Quick fix nexus5 point/spot light issues #9948

Merged
merged 4 commits into from Oct 27, 2016

Projects

None yet

2 participants

@bhouston
Contributor
bhouston commented Oct 26, 2016 edited

This PR fixes the issue with more than one point or spot light not working on Nexus 5 devices, addressing long standing issues #9100 #9871.

To fix this I removed the testLightInRange function which was added in this commit: 3adb436

I doubt this is really the best fix. I know that the testLightInRange function does hvae issues in that it doesn't look at decayExponent in the same way that it is used in punctualLightIntensityToIrradianceFactor. Basically testLightInRange is a more stringent of a test.

To be specific in punctualLightIntensityToIrradianceFactor, decayExponent must be > 0 for the cutoffDistance to matter. But testLightInRange always uses cutoffDistance whether or not the light is decaying.

I think there is something else going on at the same time. I will investigate a bit more today to see if there is a better fix -- I suspect that an array of uniforms (either cutoffDistance or decayExponent) is not being initialized properly, otherwise why would this only affect the lights after the first....

bhouston added some commits Oct 26, 2016
@bhouston bhouston remove testLightInRange, not equivalent to calcLightAttenuation. fc15517
@bhouston bhouston Merge branch 'fix_nexus5_pointLights' into dev
# Conflicts:
#	src/renderers/shaders/ShaderChunk/bsdfs.glsl
#	src/renderers/shaders/ShaderChunk/lights_pars.glsl
70bc5d0
@bhouston
Contributor
@bhouston
Contributor

I've tried to figure out the real underlying issue in testLightInRange and I can not figure it out. The parameters seem correct, the check just seems to fail on N > 0 for lights no matter what I set, unless I set it to return true.

I'm going to move on if people find htis PR sufficient.

@mrdoob
Owner
mrdoob commented Oct 26, 2016

To fix this I removed the testLightInRange function which was added in this commit: 3adb436

Eek! Sorry about that 😞

- }
+ directLight.color = pointLight.color;
+ directLight.color *= punctualLightIntensityToIrradianceFactor( lightDistance, pointLight.distance, pointLight.decay );
+ directLight.visible = ( directLight.color != vec3( 0.0, 0.0, 0.0 ) );
@mrdoob
mrdoob Oct 26, 2016 Owner

Would there be any benefit of adding #define COLOR_BLACK vec3( 0.0 ) in common.glsl for this?

@bhouston
bhouston Oct 27, 2016 edited Contributor

The issue with COLOR_BLACK is that color can be RGB or RGBA, so we would need 2 defines RGB_BLACK and RGBA_BLACK but really it is only saving a few characters. I did replace my usage of vec3( 0.0, 0.0, 0.0 ) with vec3( 0.0 ) which is what I should have done in the first place.

@bhouston
Contributor

This is ready for merging. I tested all examples and they seem good.

@mrdoob mrdoob merged commit a56405c into mrdoob:dev Oct 27, 2016
@mrdoob
Owner
mrdoob commented Oct 27, 2016

Many many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment