-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WebGLState: Ensure reset() resets all status variables. #20732
Conversation
Thanks! |
Hi @webglzhang, @mrdoob, @Mugen87! Following the recommendation from @Mugen87, bringing here a collateral effect of this change in 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
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 Any guidance in how to solve it will be greatly appreciated, thanks in advance. |
@webglzhang Can you please describe the actual problem in your app that has been fixed with this PR? |
Is this to avoid relying in the browser doing compositing? |
@jscastro76 I've tried to reproduce the disappearing shadows with different official As long as the issue can't be reproduce with pure |
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;
}, |
I see... I think you'll have to report this to Mapbox instead. |
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 |
It was not our decision to reuse the WebGL context. They should be responsible for that decision. |
This is how it should work:
|
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 |
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:
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?
|
Could you create a jsfiddle that shows the issue? |
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 :) |
If the PR solves the issue, we can forget about the jsfiddle 😊 |
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: