Skip to content

Conversation

@cabanier
Copy link
Contributor

Move more logic to resolveDepthBuffer instead of relying on an undefined depth buffer

This contribution is funded by Meta

@github-actions
Copy link

github-actions bot commented Feb 28, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 336.16
78.31
336.01
78.26
-144 B
-52 B
WebGPU 522.67
145.12
522.67
145.12
+0 B
+0 B
WebGPU Nodes 522.14
145.01
522.14
145.01
+0 B
+0 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 465.22
112.18
465.07
112.13
-144 B
-53 B
WebGPU 593.09
160.72
593.09
160.72
+0 B
+0 B
WebGPU Nodes 548.22
150.16
548.22
150.16
+0 B
+0 B

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2025

It seems this PR breaks transmission. There is no refraction anymore in webgl_loader_gltf_transmission:

image

@cabanier
Copy link
Contributor Author

cabanier commented Feb 28, 2025

@Mugen87 Strange, it looks correct on my end with my change.

Screenshot 2025-02-28 at 9 39 15 AM

Maybe I need to rebase?

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2025

I have tested on macOS with Chrome. The CI uses puppeteer + Windows. Here are the actual and expected screenshots from the CI for webgl_loader_gltf_transmission:

Actual:
webgl_loader_gltf_transmission-actual

Expected:
webgl_loader_gltf_transmission-expected

Other affected demos are webgl_loader_gltf_anisotropy and webgl_loader_gltf_iridescence.

Maybe I need to rebase?

This can't hurt. On which device/browser have you tested with?

@cabanier
Copy link
Contributor Author

Maybe I need to rebase?

This can't hurt. On which device/browser have you tested with?

I'm using Chrome on mac

@cabanier
Copy link
Contributor Author

@Mugen87 the transmission render target has resolveDepthBuffer set to false. Shouldn't that be true? Don't we need the depth?

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 28, 2025

When I recall correctly only the color is needed. The depth discard was originally implemented in #28132 and later refactored via #28170 so resolveDepthBuffer is used.

/cc @mrxz

@cabanier
Copy link
Contributor Author

I removed the change in WebGLTextures.js that caused this regression.

@mrxz
Copy link
Contributor

mrxz commented Mar 1, 2025

When I recall correctly only the color is needed.

Indeed, only the color is used for transmission. Even when refracting, the refracted ray is just used to approximate where to sample (see transmission_pars_fragment.glsl.js#L210-L218)

@Mugen87 Mugen87 added this to the r175 milestone Mar 1, 2025
@Mugen87 Mugen87 merged commit 4944ad7 into mrdoob:dev Mar 1, 2025
12 checks passed
@Mugen87 Mugen87 changed the title Move more logic to resolveDepthBuffer WebGLRenderer: Refactor setRenderTargetTextures(). Mar 1, 2025
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.

3 participants