WebGLState: Ensure reset() resets all status variables.

[webglzhang]

webglstate needs to be completely reset

I found the problem that is webglstate is not real reset ,when I used three.js combine with webgl command. After a few days of research, I fix it and want to share..:smile:

[mrdoob]

Thanks!

[jscastro76]

Hi @webglzhang, @mrdoob, @Mugen87!

Following the recommendation from @Mugen87, bringing here a collateral effect of this change in WebGLState.reset() method of the WebGLState.

As described in StackOverflow, this change is making the shadows disappear from custom layers in Mapbox since v123 in the official example of Mapbox 3D layers using Three.js.

Apart of what is described in detail in the question from SO, the context from Mapbox is reused to draw with Three.js and reset() is called in every Custom Layer render loop. Debugging the reason, this is due to the reset to null of the variable currentBlendingEnabled. When this variable is set to null at reset, after it's been set as true by a transparent material at setMaterial in the same render loop then the shadows are not working anymore, the rest of the variables have no impact.

From this:	To this...
Here is the Fiddle v122	Here is the Fiddle v123

Exactly same source code in both.

I definitely need to call reset to avoid other render issues, but the issue is that I cannot replace the call to reset by an alternative method because they are not accessible from the page code. This is stopping me to migrate my production projects beyond v123.

Any guidance in how to solve it will be greatly appreciated, thanks in advance.

[Mugen87]

@webglzhang Can you please describe the actual problem in your app that has been fixed with this PR?

[mrdoob]

@jscastro76

the context from Mapbox is reused to draw with Three.js

Is this to avoid relying in the browser doing compositing?

[Mugen87]

@jscastro76 I've tried to reproduce the disappearing shadows with different official three.js examples by calling state.reset() before rendering like in your live example. However, shadows always work.

As long as the issue can't be reproduce with pure three.js code, it's probably better to report this issue to Mapbox. This PR itself seems correct since the additional state variables were not resetted correctly.

[jscastro76]

@jscastro76

the context from Mapbox is reused to draw with Three.js

Is this to avoid relying in the browser doing compositing?

I guess so... the official example from Mapbox uses the context instance of the layer and the canvas to create the WebGLRenderer as a layer.

var customLayer = {
    id: '3d-model',
    type: 'custom',
    renderingMode: '3d',
    onAdd: function (map, gl) {
        this.camera = new THREE.Camera();
        this.scene = new THREE.Scene();
 
        // create two three.js lights to illuminate the model
        var directionalLight = new THREE.DirectionalLight(0xffffff);
        directionalLight.position.set(0, -70, 100).normalize();
        this.scene.add(directionalLight);
 
        var directionalLight2 = new THREE.DirectionalLight(0xffffff);
        directionalLight2.position.set(0, 70, 100).normalize();
        this.scene.add(directionalLight2);
 
        // use the three.js GLTF loader to add the 3D model to the three.js scene
        var loader = new THREE.GLTFLoader();
        loader.load(
            'https://docs.mapbox.com/mapbox-gl-js/assets/34M_17/34M_17.gltf',
            function (gltf) {
                this.scene.add(gltf.scene);
            }.bind(this)
        );
        this.map = map;
 
        // use the Mapbox GL JS map canvas for three.js
        this.renderer = new THREE.WebGLRenderer({
            canvas: map.getCanvas(),
            context: gl,
            antialias: true
        });
 
        this.renderer.autoClear = false;
},

[mrdoob]

I see... I think you'll have to report this to Mapbox instead.

[jscastro76]

Thanks for your reply, but as long as their code hasn't changed at all neither the demo code and this has been working since ages, I don't think they will accept it

[mrdoob]

It was not our decision to reuse the WebGL context. They should be responsible for that decision.

[mrdoob]

This is how it should work:

1. If you have an issue with Mapbox, you report it to Mapbox.
2. Mapbox engineers investigate the issue and report the issue to us if needed.

[Mugen87]

I have to say that I recommended @jscastro76 to add a comment at this PR since I was initially unsure about whether this change affects pure three.js code or not.

[b1616]

I ran into a similar issue using another library that was sharing the WebGL context. What I found was that the other library (imgui-js) was setting gl.BLEND to true and then, with this WebGLState.Reset() change, currentBlendingEnabled was being set to null.

This caused a lot of textures in my scene to be displayed incorrectly because when setBlending is subsequently called on the WebGLState, it assumes that if the desired blending method is NoBlending and currentBlendingEnabled is null, that gl.BLEND is already disabled:

function setBlending(blending, blendEquation, blendSrc, blendDst, blendEquationAlpha, blendSrcAlpha, blendDstAlpha, premultipliedAlpha) {
    if (blending === NoBlending) {
       if (currentBlendingEnabled === false) {
          disable(3042);

However, with reset nulling the currentBlendingEnabled value each frame but not setting gl.BLEND to false, I believe this assumption is no longer correct. Looking closer, even after removing the external library I was using that sets the gl.BLEND value to true, I see that this change (#20732) is having a negative impacting some of the textures in my scene. Would it make sense to update the setBlending function to honor NoBlending requests including when currentBlendingEnabled is null, like so?

function setBlending(blending, blendEquation, blendSrc, blendDst, blendEquationAlpha, blendSrcAlpha, blendDstAlpha, premultipliedAlpha) {
    if (blending === NoBlending) {
       if (currentBlendingEnabled !== false) {
          disable(3042);

[mrdoob]

I see that this change (#20732) is having a negative impacting some of the textures in my scene.

Could you create a jsfiddle that shows the issue?

[b1616]

Thanks @Mugen87! I confirmed that fix worked for the textures that were displaying improperly in my scene. In my case, I had one particular mesh where the eyeball spheres were showing through the face. However, I have other characters and their face meshes that were displaying okay. I have been trying to repro in a jsfiddle, but I couldn't quite pin down what mesh or material properties were causing the issue when the gl state wasn't being reset.

Let me know if there is still any value in getting a jsfiddle (perhaps they are used as test cases to ensure stability before releases?) but otherwise I'll grab the fix and move along with my upgrade :)

[mrdoob]

Let me know if there is still any value in getting a jsfiddle (perhaps they are used as test cases to ensure stability before releases?) but otherwise I'll grab the fix and move along with my upgrade :)

If the PR solves the issue, we can forget about the jsfiddle 😊