WebGLLights state and its hashing produce unintended material reinitialization

[Usnul]

WebGLLights is used to store lighting state.
WebGLLights.hash property is used to determine whether or not a material needs to be re-built.

three.js/src/renderers/WebGLRenderer.js

Lines 1479 to 1482 in bd63a6c

	} else if ( materialProperties.lightsHash !== lights.state.hash ) {
	properties.update( material, 'lightsHash', lights.state.hash );
	programChange = false;

The way hash is built, it includes unique reference to WebGLLights instance:

three.js/src/renderers/webgl/WebGLLights.js

Line 319 in efeb95f

state.hash = state.id + ',' + directionalLength + ',' + pointLength + ',' + spotLength + ',' + rectAreaLength + ',' + hemiLength + ',' + shadows.length;

constructor:

three.js/src/renderers/webgl/WebGLLights.js

Lines 103 to 111 in efeb95f

	var count = 0;
	function WebGLLights() {
	var cache = new UniformsCache();
	var state = {
	id: count ++,

this means that if you have 2 instances of WebGLLights and they are identical, except for id - you will end up rebuilding materials as you switch between these instances - inefficient.

Another problem with the current approach is that it is probabilistic, it's possible that hash remains the same even though lighting has changed.

I propose following:

• remove id from hash calculation
• use a fast hashing function to compute 64bit hash(javascript number is IEEE 64-bit Float) instead of using a string
• pass more than just number of lights into the hash (as long as that makes sense).

[mrdoob]

this means that if you have 2 instances of WebGLLights and they are identical,

When would that happen? 2 different WebGLRenderers rendering the same scene?

[Usnul]

not quite, it depends on scene.id and camera.id
Here:

three.js/src/renderers/webgl/WebGLRenderStates.js

Lines 71 to 86 in bd63a6c

	function get( scene, camera ) {
	var hash = scene.id + ',' + camera.id;
	var renderState = renderStates[ hash ];
	if ( renderState === undefined ) {
	renderState = new WebGLRenderState();
	renderStates[ hash ] = renderState;
	}
	return renderState;
	}

[Usnul]

lets say you want to render the same scene from 2 points of view. A simple example would be Mirror class. But there are a lot of other examples where having multiple cameras could be useful. Scene ID makes me skeptical also, the fact that scene ID is the same means very little for the content of that scene between frames, or indeed having two scenes that use the same materials and lighting setup.

[WestLangley]

or indeed having two scenes that use the same materials and lighting setup.

This is the use case in #14477.

In that case, the work-around is to clone the material, and not share it:

var mesh1 = new THREE.Mesh( geometry, material );
scene1.add( mesh1 );

var mesh2 = new THREE.Mesh( geometry, material.clone() );
scene2.add( mesh2 );

This is either a limitation or a bug. If we consider this a limitation, we should document both the limitation and the work-around somewhere.

I'm thinking it would be preferable to consider this a bug.

[Mugen87]

Avoiding calls of initMaterial() would be nice but the flexibility of three.js makes it hard to provide an approach that works without causing fatal rendering problems like mentioned in #12883. I'm not sure how such a solution could look like.

I personally consider this issue as a limitation of three.js in order to ensure correct rendering.

[Mugen87]

BTW: The root cause of the issue is the fact that it's possible to perform nested render calls via hooks like onBeforeRender(). The current implementation of WebGLRenderer ensures a correct result for this kind of rendering.

[Usnul]

@Mugen87
The hashing is bogus. If you generate a unique ID and hash that too - what's the point really? I'm not sure who wrote that hashing function but it could use some improvements.

[Mugen87]

I'm not sure who wrote that hashing function but it could use some improvements.

I suggest you do a PR that demonstrates a concrete improvement. Make sure to check all examples that utilize onBeforeRender() before submitting.

[carstenschwede]

Do I understand correctly that this applies to a simple CubeCamera-setting as well, i.e. in a scene with only light-based material and a cube-camera (jsFiddle):

This won't cause light state changes (and calls to .initMaterial()) at every frame:

function animate() {
    requestAnimationFrame( animate ); 
    renderer.render( scene, camera );
}

But this does:

function animate() {
    requestAnimationFrame( animate ); 
    cubeCamera.update( renderer, scene );
}

because of CubeCamera rendering 6 different PerspectiveCameras? However, this is not the case for ArrayCamera. Are ArrayCameras exempt because their cameras are assumed to be

• close to each other
• pointing in similar direction

and thus are less susceptible to missing light updates? I think I still have troubles understanding why .initMaterials() calls are needed for some camera updates but not for others - if they are in general, is there an option to make hash calculation faster? .cloneUniforms() is really dragging my fps.

[Mugen87]

and thus are less susceptible to missing light updates?

The problem is that each camera has a different view matrix and thus affecting light computations.

[carstenschwede]

Exactly. So the light calculations in the THREE.ArrayCamera are actually wrong? Cause there are no calls to .initMaterial() for THREE.ArrayCamera even though they all have different view matrices.

[Mugen87]

Any chances to demonstrate this with a fiddle?

So the light calculations in the THREE.ArrayCamera are actually wrong?

No, the renderer updates the render state's light setup here for all subcamera.

three.js/src/renderers/WebGLRenderer.js

Line 1441 in 92ad8c5

currentRenderState.setupLights( camera2 );

[carstenschwede]

Thanks for taking a look at this. I ran the THREE.js ArrayCamera example and my performance log doesn't show repeated calls to .initMaterial() which is why I wondered why they are required for CubeCamera but not for ArrayCamera. (.initMaterial() is slow for my app due to .cloneUniforms()). If .setupLights() is sufficient instead of .initMaterial()., shouldn't CubeCamera be handled similarly?

[Mugen87]

.initMaterial() is slow for my app due to .cloneUniforms()

Can you please show the code or call stack that triggers cloneUniforms() from initMaterial()?

[carstenschwede]

The CubeCamera from above exhibits this.

Call stack is .initMaterial()->.getParameters()->.getShaderObject()->.cloneUniforms():

Rendering to CubeCamera with 6 cameras:

vs. rendering to ArrayCamera with 36 cameras:

[Mugen87]

See #19673.

[Mugen87]

Technically, comparing CubeCamera with ArrayCamera right now is like comparing apples and oranges. The problem is that CubeCamera has to perform calls of WebGLRenderer.render() for each camera since it has to alter the destination render target for each camera. This is not true for array camera which needs only a single call of WebGLRenderer.render() since it's explicitly designed for single-pass rendering.

That said, there is still room for performance improvements for CubeCamera since redundant logic is executed when producing the cube render target.

@mrdoob Have you ever considered to derive CubeCamera form ArrayCamera?

[carstenschwede]

This would be awesome if ArrayCamera had the option to not only render to different viewports, but to different CubeFaces.

Devil is probably in the details, e.g. ArrayCamera currently only has one frustum and would need frustum updates before rendering subcameras.

I wonder if this is the reason why StereoCamera is not derived from ArrayCamera? I.e. you would have to set a larger combined frustum on the ArrayCamera to make sure the subcameras do not cull differently.

[Mugen87]

Devil is probably in the details, e.g. ArrayCamera currently only has one frustum and would need frustum updates before rendering subcameras.

Yeah, the problem is you still want a specific view frustum culling per sub camera. On the other hand, other tasks are redundant like updating the world matrices of the scene graph or recomputing shadow maps per sub camera.

[Mugen87]

Anyway, I have the feeling this discussion went a bit off-topic. Besides, the implementation of WebGLLights has slightly changed. It does not compare hashes anymore (materialProperties.lightsHash !== lights.state.hash) but just a single version number.

I actually like the idea of making initMaterial() faster when no shader compilation is required. Such optimizations might be valuable for #15047.

[Mugen87]

The renderer has changed at various places since this issue was originally reported. The performance of initMaterial() was improved via #19673, render states (and lists) are only created per scene now (#20422, #21254) and light uniforms can now be updated without triggering initMaterial() via WebGLLights.setupView(). WebGLLights also performs no string concatenation anymore (#14588) and simplified the usage of the internal hash (#16581).

Overall I think it's best to close the issue and open a new one if there are still performance issues.