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

WebGLRenderer: Fix feedback loop in transmission pass for WebGL 2. #26177

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jun 1, 2023

Fixed #25990.

Description

#25502 introduced a feedback loop during the back-side transmission pass since the active render target is equal to the texture which is assigned to transmissionSamplerMap. The feedback loop produces a WebGL warning but also the result of the back-side pass is missing in the render target (so the transmissive surface looks like before #25502 was added). The warning looks like so:

GL_INVALID_OPERATION: Feedback loop formed between Framebuffer and active Texture.

This issue does not pop-up when antialias is set to true which is why the issue was overlooked initially. This happens because setting antialias to true means the transmissive render target is multisampled. Multisampled render targets do have more than one framebuffer so using it as the active render target and as a texture does not produce a feedback loop.

Solving this issue without multisampling is difficult because you need a separate code path that copies the contents of the transmissive render target via copyTexSubImage2D() into a texture and then uses it as the value for transmissionSamplerMap. When trying to get this to work, I have realized this will noticeably increase the code complexity with unclear performance implications (since you have to call copyTexSubImage2D() two times inside renderTransmissionPass()).

For simplicity reasons, I suggest to always use a multisampled render target for the transmission pass when using WebGL 2 which fixes the issue.

@Mugen87 Mugen87 marked this pull request as ready for review June 1, 2023 11:14
@github-actions
Copy link

github-actions bot commented Jun 1, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
644.1 kB (159.5 kB) 644.1 kB (159.5 kB) -8 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
434.1 kB (105.1 kB) 434.1 kB (105.1 kB) -8 B

@Mugen87 Mugen87 added this to the r154 milestone Jun 21, 2023
@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 24, 2023

@mrdoob Are you okay with merging this PR? Otherwise transmission is broken for all users who are not setting antialias to true.

Considering the complexity of a larger refactoring, it seems to me the proposed solution here is best.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jun 27, 2023

I'm going ahead and merge this PR so we have a hotfix for r154.

@Mugen87 Mugen87 merged commit c1e558c into mrdoob:dev Jun 27, 2023
@mrdoob
Copy link
Owner

mrdoob commented Jun 28, 2023

@mrdoob Are you okay with merging this PR? Otherwise transmission is broken for all users who are not setting antialias to true.

Considering the complexity of a larger refactoring, it seems to me the proposed solution here is best.

Sounds good!

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.

GL_INVALID_OPERATION: Feedback loop formed between Framebuffer and active Texture.
2 participants