Hubs-Foundation / hubs

Duck-themed multi-user virtual spaces in WebVR. Built with A-Frame.
https://hubsfoundation.org
Mozilla Public License 2.0
2.13k stars 1.41k forks source link

WebGLProgram warning : pow(f, e) won't work for negative f #1086

Open johnshaughnessy opened 5 years ago

johnshaughnessy commented 5 years ago

THREE.WebGLProgram: gl.getProgramInfoLog() C:\fakepath(382,23-154): warning X3571: pow(f, e) will not work for negative f, use abs(f) or conditionally handle negative values if you expect them C:\fakepath(807,16-119): warning X3571: pow(f, e) will not work for negative f, use abs(f) or conditionally handle negative values if you expect them C:\fakepath(378,18-93): warning X3571: pow(f, e) will not work for negative f, use abs(f) or conditionally handle negative values if you expect them

johnshaughnessy commented 5 years ago

If we know we never use a negative f and we don't want to make an unnecessary call to abs that's fine -- but in that case we should consider disabling this warning somehow.

netpro2k commented 5 years ago

Looks like its the Skybox shader doing this.

johnshaughnessy commented 5 years ago
src/renderers/shaders/ShaderChunk/bsdfs.glsl.js:11: float distanceFalloff = 1.0 / max( pow( lightDistance, decayExponent ), 0.01 );
src/renderers/shaders/ShaderChunk/bsdfs.glsl.js:25:     return pow( saturate( -lightDistance / cutoffDistance + 1.0 ), decayExponent );
src/renderers/shaders/ShaderChunk/bsdfs.glsl.js:44: // float fresnel = pow( 1.0 - dotLH, 5.0 );
src/renderers/shaders/ShaderChunk/bsdfs.glsl.js:266:    return RECIPROCAL_PI * ( shininess * 0.5 + 1.0 ) * pow( dotNH, shininess );
src/renderers/shaders/ShaderChunk/envmap_physical_pars_fragment.glsl.js:46:     //float envMapWidth = pow( 2.0, maxMIPLevelScalar );
src/renderers/shaders/ShaderChunk/encodings_pars_fragment.glsl.js:9:    return vec4( pow( value.rgb, vec3( gammaFactor ) ), value.a );
src/renderers/shaders/ShaderChunk/encodings_pars_fragment.glsl.js:13:   return vec4( pow( value.rgb, vec3( 1.0 / gammaFactor ) ), value.a );
src/renderers/shaders/ShaderChunk/encodings_pars_fragment.glsl.js:17:   return vec4( mix( pow( value.rgb * 0.9478672986 + vec3( 0.0521327014 ), vec3( 2.4 ) ), value.rgb * 0.0773993808, vec3( lessThanEqual( value.rgb, vec3( 0.04045 ) ) ) ), value.a );
src/renderers/shaders/ShaderChunk/encodings_pars_fragment.glsl.js:21:   return vec4( mix( pow( value.rgb, vec3( 0.41666 ) ) * 1.055 - vec3( 0.055 ), value.rgb * 12.92, vec3( lessThanEqual( value.rgb, vec3( 0.0031308 ) ) ) ), value.a );
src/renderers/shaders/ShaderChunk/lights_physical_pars_fragment.glsl.js:21: return DEFAULT_SPECULAR_COEFFICIENT + ( 1.0 - DEFAULT_SPECULAR_COEFFICIENT ) * ( pow( 1.0 - dotNL, 5.0 ) * pow( 1.0 - roughness, 2.0 ) );
src/renderers/shaders/ShaderChunk/lights_physical_pars_fragment.glsl.js:134:    return saturate( pow( dotNV + ambientOcclusion, exp2( - 16.0 * roughness - 1.0 ) ) - 1.0 + ambientOcclusion );
src/renderers/shaders/ShaderChunk/tonemapping_pars_fragment.glsl.js:40: return pow( ( color * ( 6.2 * color + 0.5 ) ) / ( color * ( 6.2 * color + 1.7 ) + 0.06 ), vec3( 2.2 ) );

I'm not sure what to do about this. I don't think we want to call abs every time we use pow, since (as far as I know) we do not expect to ever pass a negative value in. We could tell THREE not to log this warning, but since we don't want to silence other warnings, doing so also comes with a perf cost. What I think we actually want it to tell the browsers, "We know we can't send negative values to pow. If something bad happens if you do, just let the bad thing happen."

We might just opt to not log warnings to the console. The warnings are still available in the programLogs, so if we noticed something suspicious happening we could inspect it. We might only do this in production, or only do it in production on mobile, where we are most performance sensitive.