Renderer state.reset() no longer resets attributes

[cgauld]

We perform some WebGL computation using the raw WebGL context (i.e. inside our animation frame callback but before our calls to Three to render the scene etc. We need to use the same context since we use some of the output from that work in three (e.g. some textures). In order to ensure that GL state is in a relatively predictable state before and after our raw context use, we call renderer.state.reset().

This was working great before r118, but it looks like that revision moved the management of attributes out of WebGLState and into WebGLBindingStates, and this means our call to renderer.state.reset() no longer disables the enabled attributes.

We were wondering if there was a way to clear these attributes with the new structure or, failing that, if you'd be open to us taking a look at how to introduce it with a PR?

[Mugen87]

The problem is that resetting the WebGL state was never meant to be public. It should be a pure internal thing of the engine.

I'm in general no fan of opening up the internals of the renderer since it makes the introduction of new features (like VAO in r118) more complicated.

[cgauld]

Hi - thanks for the quick reply! That makes total sense. For our use case, however, it does mean a significant performance hit if we have to perform our computation in a separate context and transfer the data between the contexts. Alternatively we could maintain a fork for our users that patches in a moment/mechanism of predictable GL state, although we'd of course rather share our work if it's welcome 😄. Some other platforms we target (e.g. Unity) expose a 'GL plugin' interface that enables this sort of activity but I 100% recognise that that has a maintainability cost.

Our proposal would be to expose a function on the renderer, e.g. resetAttributes that would do something like this:

bindingStates.resetDefaultState();
bindingStates.disableUnusedAttributes();

Let me know if this is a non-starter and we'll go the fork route, or indeed if there is a route forward with a change in approach / implementation. As I say we're happy to provide a PR including docs + type defs 💪.

[Mugen87]

Thanks for you willingness to contribute! However, I still have problems to purposely provide an interface for state manipulation. Not only because of the mentioned reason but also because state management with WebGPU will be different than with WebGL. In the long term view, it will be easier for users to switch to the upcoming WebGPU if no WebGL specific logic "leaks" into applications.

[mrdoob]

I wonder what @takahirox thinks about this 🤔

[takahirox]

I agree with @Mugen87. The Renderer inside (WebGL resource management) is complex. Exposing an API which affects internal WebGL resources can make it harder to maintain. Once we expose an API, we should keep the API unchanged as much as possible, then I think there my be a chance that exposing such an API can block the change (optimization) of the renderer inside.

But, on the other hand, I know we sometimes have requests from users that they want to finely control WebGL resources by themselves. So I think it might be acceptable if the change is very simple and easy, and many users want this feature.

[cgauld]

Thanks all for your insights 😄 In terms of what a simple and easy change could look like, we'd propose adding the following function inside WebGLRenderer around line 581:

this.resetAttributes = function() {

    bindingStates.resetDefaultState();
    bindingStates.disableUnusedAttributes();

};

We'd propose adding it only to the WebGLRenderer/WebGL1Renderer (and not the base Renderer interface), so that there would be no expectation for it to exist on other renderers. Users of this function would be binding their application to a WebGL renderer in a similar way to if they're using WebGLProgram for example.

An alternative would be to modify the existing renderer.state.reset() function to restore its pre-118 behaviour of resetting attributes to a predictable state. This seems perhaps the cleanest in terms of contract - it's an already-exposed function, that would be returning to behaviour that it used to exhibit; and it's not used elsewhere in the Three codebase so modification would only affect external users of that function (who may indeed be expecting its previous behaviour). My concern with this approach is that I suspect the diff would be more complicated since that attribute state is now managed in a different object.

Let me know your thoughts! Hope you don't mind my persistence @Mugen87 - I promise to let it lie if the answer is no 😉

[Mugen87]

Sorry, but I still don't vote for this change. A resetAttributes() method is WebGL specific.

[cgauld]

Indeed - I agree 🙂 We’d only expose it on WebGLRenderer/WebGL1Renderer so there’s no expectation it would be a concept that applied to or was exposed by other renderers.

[takahirox]

Unfortunately we need to be careful to add this type of API as written in the above. So I'd like to suggest making your fork for now and revisiting in the core if we get the same feature request more.

[m-schuetz]

Hi, I'd like to vote for resetting the binding state since I also encountered an issue with binding state not being reset after an upgrade to three.js 123.

In Potree, I'm rendering point clouds with custom WebGL code using the same context as three.js. Previously, renderer.state.reset();did the trick. Now, I also have to call bindingStates.reset() but the main issue for me is that bindingStates is not accessible without modifying the three.js source code.

As a workaround, I've added this.bindingStates = bindingStates; to the WebGLRenderer so that I can use renderer.bindingStates.reset().

var xr = new WebXRManager(_this, _gl);
this.xr = xr; // shadow map

this.bindingStates = bindingStates;

var shadowMap = new WebGLShadowMap(_this, objects, capabilities.maxTextureSize);
this.shadowMap = shadowMap; // API

Would it be okay to make bindingStates publicly accessible like that? It's still up to me to make sure that whatever I do with custom WebGL code doesn't break three.js, but at least it would allow me to do custom stuff.

Edit:
Btw, a workaround that doesn't require changes to three.js is to render a dummy scene with at least one object, just to make sure that the vertex array object state gets reset. This requires that the object is actually renderer to work, i.e., no frustum culling.

// render dummy scene to reset vertex array object state
let dummyScene = new THREE.Scene();
let geometry = new THREE.SphereGeometry(0.0001, 2, 2);
let mesh = new THREE.Mesh(geometry, new THREE.MeshBasicMaterial());
dummyScene.add(mesh);
renderer.render(dummyScene, camera1);

// render actual scene
renderer.render(scene, camera2);

// do custom WebGL rendering into same three.js context

[cgauld]

Thank you :-D One thing I'd advocate for is to also call the following in the resetState function:

bindingState.disableUnusedAttributes();

If I'm reading the diff correctly then I think the state of the GL attributes is still unpredictable without it.

[Mugen87]

@takahirox How about calling WebGLBindingStates.disableUnusedAttributes() in WebGLBindingStates.reset()? To me, it seems disabling attributes should be part of the general reset operation.

[takahirox]

I guess you think of disabling all attributes rather than WebGLBindingStates.disableUnusedAttributes() which disables only the unused ones of an active VAO, don't you?

Yeah, I think disabling all the attributes sounds more proper as reset state function. But one concern. Every VAO has attribute enabled/disabled settings so we need to iterate all the VAOs and disable them. (Disposing of them with .dispose() may be another option.)

In case a lot of VAOs are created, iterating them and calling WebGL methods for each could take time? So I feel I first want to know some renderer.resetState() concrete use cases.

[Mugen87]

which disables only the unused ones of an active VAO

Right, the operation should work for all attributes. To me, it's okay if a reset operation potentially takes some time. Developers performing a manual state manipulation should know what they are doing.

[takahirox]

To me, it's okay if a reset operation potentially takes some time.

Yeah, I think so, too. But just in case I want to hear @cgauld 's comment because I'm not sure the use case.

[m-schuetz]

Would it be possible to provide this "deep-reset" functionality as a separate function call? For me, it only matters that there are no VAOs, VBOs, programs, textures, etc. currently bound and cached, so that I can bind my own resources, do some rendering with them, unbind them, and let three.js continue from there, without three.js thinking that the last VAO is still bound. The state of specific VAOs and VBOs managed by three.js don't affect me rendering my custom stuff in between. (gl.vertexAttribPointer and gl.enableVertexAttribArray are coupled with the currently bound VAO/VBO, right? So three.js calling them on one VBO should not affect me calling them on my own VBOs?))

[Mugen87]

(gl.vertexAttribPointer and gl.enableVertexAttribArray are coupled with the currently bound VAO/VBO, right? So three.js calling them on one VBO should not affect me calling them on my own VBOs?))

Correct.