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.