Skip to content
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

Furnace Test Example: added transmission to the test #22335

Merged
merged 1 commit into from
Aug 15, 2021

Conversation

WestLangley
Copy link
Collaborator

... plus other minor fixes.

@WestLangley WestLangley added this to the r132 milestone Aug 15, 2021
@WestLangley
Copy link
Collaborator Author

It appears that orthographic camera, PMREM, and/or transmission are mutually incompatible, so I reverted the camera back to a perspective one.

@mrdoob mrdoob merged commit 3dadc41 into mrdoob:dev Aug 15, 2021
@mrdoob
Copy link
Owner

mrdoob commented Aug 15, 2021

Thanks!

@WestLangley WestLangley deleted the dev_furnace_test branch August 15, 2021 18:04
@WestLangley
Copy link
Collaborator Author

@Mugen87 If transmission is set to 1, then this example renders incorrectly on the first rendering. Move the mouse to force another render.

I expect all the promises in this example are not required, but even after removing them and simplifying the example to follow the typical three.js coding pattern, the issue remains. It seems like transmission thinks the background is black...

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 17, 2021

If transmission is set to 1, then this example renders incorrectly on the first rendering. Move the mouse to force another render.

I can reproduce. However it's not clear to me whether this is only an issue in the first frame or if the transmission implementation is frame-late in general.

@takahirox Do you mind having a look? Here is a test case: https://jsfiddle.net/psjwzyft/

@mrdoob
Copy link
Owner

mrdoob commented Aug 17, 2021

Doing this shows that it's a frame late indeed:

function render() {

  renderer.render(scene, camera);
  renderer.render(scene, camera);

}

@takahirox
Copy link
Collaborator

@takahirox Do you mind having a look? Here is a test case: https://jsfiddle.net/psjwzyft/

OK, let me try hopefully this weekend

@takahirox
Copy link
Collaborator

takahirox commented Aug 23, 2021

I investigated the problem this weekend but I couldn't find the root issue. But I found some weird behaviors. Does anyone here have any clues from them?

  • In webgl_furnace_test example with transmission=1, the problem was disappeared if I set renderer.outputEncoding = THREE.sRGBEncoding
  • In webgl_furnace_test example with transmission=1, the problem was disappeared if I set scene.background = new THREE.Color( COLOR ); instead of hdrEquirect
  • The problem can reproducible even in webgl_materials_physical_transmission example if I comment out the hdrEquirect.mapping = THREE.EquirectangularReflectionMapping; and renderer.outputEncoding = THREE.sRGBEncoding; lines

So, sounds like the problem happens with linear outputEncoding (default) and certain scene.background?

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 23, 2021

After merging #22394, the following warning appears now in the first render call of the fiddle:

[.WebGL-0x7ffe34856400]RENDER WARNING: there is no texture bound to the unit 0

The warning does not appear in the unmodified version of the furnace test.

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 23, 2021

Is seems the issue can be fixed by forcing a uniform update of the scene's background before the opaque objects are rendered for the transmissive pass. Meaning before the following line:

renderObjects( opaqueObjects, scene, camera );

I add this hack.

opaqueObjects[ 0 ].material.uniformsNeedUpdate = true; // the first render item is the skybox

Of course this is only for testing since it just works with the fiddle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants