WebGLRenderer compile async and possible wrong shader cache key

https://github.com/mrdoob/three.js/blob/5a7e8bcfa57faddca4f73962fd92a207d6254bee/src/renderers/webgl/WebGLPrograms.js#L407

The above line uses WebGLRenderer.outputColorSpace current value for the shader cache key even if a different value is set in the parameters object in this line:

https://github.com/mrdoob/three.js/blob/5a7e8bcfa57faddca4f73962fd92a207d6254bee/src/renderers/webgl/WebGLPrograms.js#L202

Is this behaviour correct?

It seems to trigger an unwanted shader recompilation while using the api in this way and the render loop is running:

const toneMapping = this.renderer.toneMapping;
const outputColorSpace = this.renderer.outputColorSpace;

// Settings used by cube camera
this.renderer.toneMapping = THREE.NoToneMapping;
this.renderer.outputColorSpace = THREE.LinearSRGBColorSpace;       

// Start compile
const task = this.renderer.compileAsync(this.scene, this.cubeCamera.children[0]);        

// Restore original settings
this.renderer.toneMapping = toneMapping;
this.renderer.outputColorSpace = outputColorSpace;

await task;
1 Like

I didn’t check the renderer code, but immediately changing the variable back for a process that is async (therefore we don’t know when that async process needs them) could cause the original settings to be used.

I’d expect to do this instead:

const task = this.renderer.compileAsync(this.scene, this.cubeCamera.children[0]);        

await task;

// Restore original settings (after task that depends on the vars is done)
this.renderer.toneMapping = toneMapping;
this.renderer.outputColorSpace = outputColorSpace;

I believe there is a possible bug in shader cache key calculation because it uses both a computed outputColorSpace value from parameters “snapshot” and the straight value from the renderer.
This can cause a mismatch when rendering to a render target (like cube camera does).
I found that setting explicitly outputColorSpace to THREE.LinearSRGBColorSpace before updating the cube camera avoid the mismatch in the shader cache key.

You might be right, but it’s important to note that if you’re changing state in the middle of the async operation, you may be causing a conflict called a “race condition”. I’m not saying that is the issue for sure, but worth avoiding.

Does the issue still happen if you arrange your code to wait for async operations to finish before reverting state back?

I understand what you say. The reason I’m doing that way is in fact to avoid a race condition with the main render loop, that would render in linear otherwise. Looking at compileAsync source seems this use case should be supported. It internally calls the sync version of compile and then waits. So the parameters for the cache key are always synchronous