SAOPass: Wrong behavior when using two canvases

[cnspaha]

Description of the problem

When using SAOPass in several canvas it won't work, see jsfiddle.

https://jsfiddle.net/ptgwhemb/6/ (I just copied the initializing code so its rather nasty but does its job ;) )

What I expect:
A green cube with SAO on the first canvas and a red cube with SAO on the second canvas.

What I get:
Only a red cube with SAO on the second canvas.

Possible reasons:
After some debugging I found out, that, for both canvases, in the SAOPass render function the texture uniforms of the materials are pointing to the same texture even though their member renderTargets and their textures are fine.
So in this case the first WebGLRenderer tries to access a texture which isn't in its context but in the other WebGL Context.
With spector.js I could verify that, on the first canvas, the texture uniforms tDepth and tNormal of the saomaterial referencing to an empty texture with width=height=1 and the uuid of them is the same as the textures from the second canvas...
I don't know three.js internally good enough to know why this is happening, but probably some of you guys know where this bug comes from?

Three.js version

• Dev

Browser

• All of them

OS

• All of them

[Mugen87]

The problem is that the shader material in SAOPass is not setup correctly. Uniforms are not cloned so more than one instance of SAOPass will lead to invalidation of uniforms of other passes. The following code fixes the issue.

this.saoMaterial = new THREE.ShaderMaterial( {
   defines: THREE.SAOShader.defines,
   fragmentShader: THREE.SAOShader.fragmentShader,
   vertexShader: THREE.SAOShader.vertexShader,
   uniforms: THREE.UniformsUtils.clone( THREE.SAOShader.uniforms ) // fix
} );

Demo: https://jsfiddle.net/ptgwhemb/25/

It's also necessary to clone the defines object since these values can differ, too.

[Mugen87]

@cnspaha Do you wanna make a PR with the fix? 😇

[cnspaha]

@Mugen87
Thanks for the fix, will test it later and then do the PR :)

[cnspaha]

@Mugen87

Do we really need this line in the SAOPass.js?

  this.saoMaterial.extensions.drawBuffers = true;

As far as I could check we don't use MRT so why add the drawBuffers Extension?

Beside that there is almost no support for it (at least on mobile devices)

[cnspaha]

PR Done
#13399

[Mugen87]

As far as I could check we don't use MRT so why add the drawBuffers Extension?

Not sure about that but it seems obsolete.

[cnspaha]

@Ludobaka
you are the author of the SAOPass, aren't you?
can you tell us why you add the WEBGL_draw_buffers extension? we don't need it, do we?

[Ludobaka]

Actually, my contribution was an update of a former contribution that hadn't been merged with ThreeJS. So my job was mainly to update the code to fit ThreeJS version at this time. It could be a copy/paste error from this previous work. If there is no need for drawBuffers, do not hesitate to remove it.

[cnspaha]

Ah okay.
Just wanted to make sure that there is no intention of using the drawbuffer extension. Seems to work without it so I will open a PR for removing it