-
-
Notifications
You must be signed in to change notification settings - Fork 35.2k
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
Problem with Reflector affecting lighting when its texture is rendered. #12883
Comments
It is not clear to me what you are referring to... Are you aware that the highlights on the walls are specular highlights, and have nothing to do with the mirror? Change the wall material to |
I am referring to the sudden transition in the lighting on the walls as you rotate the view. Do you not see change as you rotate at the point where you are looking at the back of the mirror (and thus has no need to render the mirror texture)? |
btw. yes, i agree the lighting on the walls should not be affected by the mirror.... but in this case it does. That’s the problem... the lighting on the walls looks incorrect when camera is facing the mirror. |
OK, I see that. Here is the pattern you are implementing: scene.add( mirror );
renderer.render( scene, camera );
createMirrorTest(); // this function adds lights You are changing the number of lights after the first render, so you need to recompile the mirror's shader. mirror.material.needsUpdate = true; // you need to add this line updated fiddle: https://jsfiddle.net/9zLhq9Lf/2/ |
Ah ha. Thanks very much. I didn't know that I had to force update of materials when i add lights. In general, do i need to force update of all the materials used in a scene if I add a light after a material is first rendered? |
Yes. See the Materials section in How to Update Things in the docs. |
Thanks again. Apologies for posting what turned out to be a newbie question into the issue tracker. |
@WestLangley I'm still having trouble with this. When I change the starting camera view so that it is facing the back of the mirror, your call to |
In case it helps diagnose what's going on, I noticed that I can work around the problem if I comment out the line in Reflector.js that prevents it rendering to texture when the plane is facing away from the camera: three.js/examples/js/objects/Reflector.js Line 90 in 6d9c1f7
|
OK, then this is a more fundamental issue. Thank you for pursuing this. Lighting problems occur if the mirror is not in the camera view when the mirror is added to the scene. This bug can be reproduced in camera.position.set( 0, 75, - 160 ); and then rotating the camera until all mirrors are in view. |
Um, i wonder how the reflector's material can influence the lighting of the entire scene 🤔? For some reasons, the lights seem to be reflected along the reflection plane. It's easy to see this when you set the camera in camera.position.set( 0, - 75 , 160 ); Besides, comment out the addition of the vertical mirror. // scene.add( verticalMirror ); |
Could the issue be related to light position uniform setup in the code I referred to in my first comment? If lights are positioned in camera space, it made me wonder if for some reason the light position was using the position for the wrong camera? three.js/src/renderers/webgl/WebGLLights.js Line 262 in 14ed22d
This is just a guess based on how the light position seeming to shift when the mirror texture is rendered. |
Oh dear, i think i've found the issue 😒 The problem is the setup of light uniforms in three.js/src/renderers/WebGLRenderer.js Line 1141 in b50726a
The setup of lights is done once per frame. But when a reflector is rendered, it overwrites the lights setup because it executes a rendering of the scene in I think we have to find a way to restore the light setup after possible changes in |
One possible workaround: Set the https://jsfiddle.net/9zLhq9Lf/4/ But we definitely need a conceptual solution for this bug. |
ooo.. interesting. so presumably the problem could affect anything where onbeforerender is used to render from a different camera position? |
Yes, unfortunately. This makes the problem even more critical... |
I've created a basic PR that solves the problem. Not the best solution but at least straightforward. An other solution: Set |
Cool.. will try the PR change monday. Btw. just wondered if adding a call to a new function in WebGLRenderer risked breaking things for other renderer types that don’t implement that function? Does it need to check if the function exists before calling? Not an issue for me... I just thought it worth mentioning as it popped into my head when looking at your PR. |
That should be no problem. The PR introduces a new method that won't affect existing code. But one thing is important: The version (or release) of |
@Mugen87 I just tried your PR. Looks good to me. Thanks. Another supplementary question though: Presumably the PR only fixes Reflector, but couldn't the issue potentially affect anything that uses |
Yes, right now it is necessary to call |
Just for info: I can confirm that updating to r90 fixed the problems I was having with |
Now we usually only create one render state per scene. Instead of creating camera-specific render states, the view-dependent light uniforms are updated with a separate function whenever rendering. This helps avoid initMaterial calls when rendering cube maps or array cameras, improving performance dramatically. The only exception is handling nested render() calls done from callbacks. These are sometimes used to render the same scene from a different viewpoint, for example to generate a mirror reflection. They could also potentially modify the scene in some other ways. For these reasons they can't share the same render state. Related issues: mrdoob#12883
Now we usually only create one render state per scene. Instead of creating camera-specific render states, the view-dependent light uniforms are updated with a separate function whenever rendering. This helps avoid initMaterial calls when rendering cube maps or array cameras, improving performance dramatically. The only exception is handling nested render() calls done from callbacks. These are sometimes used to render the same scene from a different viewpoint, for example to generate a mirror reflection. They could also potentially modify the scene in some other ways. For these reasons they can't share the same render state. Related issues: mrdoob#12883
Description of the problem
I have a problem with changes to lighting when Reflector object is used.
I have distilled the problem down here:
https://jsfiddle.net/cmd4/9zLhq9Lf/
You should see that the lighting on the walls changes whenever you rotate the view, as the mirror changes between facing towards and away from the camera.
Note that to show the problem, it seems to require adding the Reflector object before the first render, and the rest of the scene after the first render. i.e. If I create the whole scene before the first render, then I don't seem to get any issues.
My guess is that the problem may be related to setting up GL state for the point light when rendered for both the main camera and the "virtual" camera created by the Reflector class.
Maybe in this function below, but here I am out of my depth:
(from https://github.com/mrdoob/three.js/blob/dev/src/renderers/webgl/WebGLLights.js)
Three.js version
Browser
OS
The text was updated successfully, but these errors were encountered: