search

Samsung / GearVRf

The GearVR framework(GearVRf) is an Open Source VR rendering library for application development on VR-supported Android devices.
http://www.gearvrf.org
Apache License 2.0
407 stars 217 forks source link

Fix bugs with adding / removing lights #1899

Closed NolaDonato closed 6 years ago

NolaDonato commented 6 years ago

Fixed faulty C++ logic for notifying components when they are removed / added from scene Delete light block when light list is cleared (from rendering thread)

This fixes intermittent failures with SIDIA lighting tests

GearVRF DCO signed off by: Nola Donato nola.donato@samsung.com

williamsma commented 6 years ago

Will this have any impact on the outstanding issue in PR https://github.com/Samsung/GearVRf/pull/1825 where lights are not turned on when loading a new X3D scene? A sample test is included with the PR.

liaxim commented 6 years ago

Planning to merge on Tuesday.

@NolaDonato Which tests in particular you expect to pass reliably. For me meshWithLightingTest fails; LightTests crash. Maybe none of these are affected by this PR; have not verified without.

LightTests crash:

06-18 10:28:35.459 23920 23920 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
06-18 10:28:35.460 23920 23920 F DEBUG   : Build fingerprint: 'samsung/greatqltesq/greatqlte:7.1.1/NMF26X/N950USQU1AQIA:user/release-keys'
06-18 10:28:35.460 23920 23920 F DEBUG   : Revision: '11'
06-18 10:28:35.460 23920 23920 F DEBUG   : ABI: 'arm'
06-18 10:28:35.460 23920 23920 F DEBUG   : pid: 23699, tid: 23893, name: GLThread 1101  >>> org.gearvrf.tester <<<
06-18 10:28:35.460 23920 23920 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
06-18 10:28:35.460 23920 23920 F DEBUG   :     r0 c622aeb4  r1 00000000  r2 c5f06600  r3 c5f06600
06-18 10:28:35.460 23920 23920 F DEBUG   :     r4 c5cc8900  r5 c3e5401c  r6 e0ac69b0  r7 cb3fde78
06-18 10:28:35.460 23920 23920 F DEBUG   :     r8 62201d24  r9 e0ac69b0  sl 62201d24  fp 00000000
06-18 10:28:35.460 23920 23920 F DEBUG   :     ip e0ac69b0  sp cb3fde50  lr c622aeb4  pc ccc57cac  cpsr a00b0030
06-18 10:28:35.463 23920 23920 F DEBUG   : 
06-18 10:28:35.463 23920 23920 F DEBUG   : backtrace:
06-18 10:28:35.463 23920 23920 F DEBUG   :     #00 pc 00360cac  /data/app/org.gearvrf.tester-1/lib/arm/libgvrf.so (_ZN3gvr9LightList9useLightsEPNS_8RendererEPNS_6ShaderE+47)
06-18 10:28:35.463 23920 23920 F DEBUG   :     #01 pc 00331a5d  /data/app/org.gearvrf.tester-1/lib/arm/libgvrf.so (_ZN3gvr10GLRenderer20renderMaterialShaderERNS_11RenderStateEPNS_10RenderDataEPNS_10ShaderDataEPNS_6ShaderE+692)
06-18 10:28:35.463 23920 23920 F DEBUG   :     #02 pc 00331c3f  /data/app/org.gearvrf.tester-1/lib/arm/libgvrf.so (_ZN3gvr10GLRenderer16renderWithShaderERNS_11RenderStateEPNS_6ShaderEPNS_10RenderDataEPNS_10ShaderDataEi+190)
06-18 10:28:35.463 23920 23920 F DEBUG   :     #03 pc 00331745  /data/app/org.gearvrf.tester-1/lib/arm/libgvrf.so (_ZN3gvr10GLRenderer10renderMeshERNS_11RenderStateEPNS_10RenderDataE+512)
06-18 10:28:35.463 23920 23920 F DEBUG   :     #04 pc 00336cdb  /data/app/org.gearvrf.tester-1/lib/arm/libgvrf.so (_ZN3gvr8Renderer16renderRenderDataERNS_11RenderStateEPNS_10RenderDataE+108)
06-18 10:28:35.463 23920 23920 F DEBUG   :     #05 pc 0032f5ff  /data/app/org.gearvrf.tester-1/lib/arm/libgvrf.so (_ZN3gvr10GLRenderer18renderRenderTargetEPNS_5SceneEP8_jobjectPNS_12RenderTargetEPNS_13ShaderManagerEPNS_13RenderTextureESA_+862)
06-18 10:28:35.463 23920 23920 F DEBUG   :     #06 pc 0038bb19  /data/app/org.gearvrf.tester-1/lib/arm/libgvrf.so (Java_org_gearvrf_NativeRenderTarget_render+256)
06-18 10:28:35.463 23920 23920 F DEBUG   :     #07 pc 00611f01  /data/app/org.gearvrf.tester-1/oat/arm/base.odex (offset 0x5ac000)
liaxim commented 6 years ago

The crash seems to be happening during the tear down of a test case. Maybe LightList::useLights needs mLightBlock nullptr check (& lock?).

liaxim commented 6 years ago

@mwitchwilliams #1825 doesn't seem to get affected by this pr.

NolaDonato commented 6 years ago

I figured out what caused the crash and fixed it.