Skip to content

TSL: Add MRT support for traaPass() #31361

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

Merged
merged 3 commits into from
Jul 11, 2025
Merged

TSL: Add MRT support for traaPass() #31361

merged 3 commits into from
Jul 11, 2025

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Jul 3, 2025

Related issue: #31354 (comment)

Description

Creates a new MRT to transfer the remaining textures from the SampleRenderTarget to the final RenderTarget.

Copy link

github-actions bot commented Jul 3, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 337.69
78.76
337.69
78.76
+0 B
+0 B
WebGPU 557.69
154.41
557.8
154.44
+113 B
+34 B
WebGPU Nodes 556.61
154.19
556.73
154.23
+113 B
+33 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 468.91
113.45
468.91
113.45
+0 B
+0 B
WebGPU 633.32
171.43
633.32
171.43
+0 B
+0 B
WebGPU Nodes 588.45
160.78
588.45
160.78
+0 B
+0 B

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 4, 2025

So with this PR only beauty is resolved, right? Other color attachments like normal, emissive are not resolved?

It seems there is an issue that I've mentioned previosuly in #29636 (comment). The sample (the rendering stored in _sampleRenderTarget) is rendered with a jitter. An output produced with a jitter must be resolved otherwise you see temporal instabilities when visually debugging the texture.

That's the main issue that concerned me initially and blocked an implementation since I'm not sure how other engines are handle this aspect. Do they resolve all attachments? If not, attachments like normal or emissive must render with a camera that has no jitter applied. Which means we would need more than one render pass or different camera uniforms.

@sunag
Copy link
Collaborator Author

sunag commented Jul 4, 2025

Hmm... It transfers the other attachments from sampleRenderTarget to renderTarget, so I think the current approach of copying the depth texture from sampleRenderTarget to renderTarget is also temporally unstable if this is used in another process?

@sunag
Copy link
Collaborator Author

sunag commented Jul 4, 2025

I think that we could use textureLoad() instead of texture() with screenCoordinate as uv and remove the _invSize since sampler is not necessary?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 4, 2025

the current approach of copying the depth texture from sampleRenderTarget to renderTarget is also temporally unstable if this is used in another process?

Yes, this isn't correct at the moment (at least I believe it isn't right). I was trying to investigate how other engines handle this topic but I found no MRT specific discussions. Most articles in the web only focus on the beauty pass.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 4, 2025

I think that we could use textureLoad() instead of texture() with screenCoordinate as uv and remove the _invSize since sampler is not necessary?

Sounds good!

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 8, 2025

After talking to a colleague from the university, reading multiple web sources and a chat with Gemini the situation is a bit more clear now.

  • It seems it is quite unusual to resolve all color attachments from the sample render target. This would only slow down the TAA especially on weaker devices (mobile) similar to MSAA.
  • When applying TAA when producing the beauty pass, the Jitter affects all attachments and depth. If these attachments are not resolved, subsequent passes might show temporal instability. This issue could be mitigate by applying accumulation and smoothing in subsequent effects but it seems more common to apply TAA at a later point instead (see next point).
  • We currently assume traaPass() is an alternative to pass() but in other engines TAA is executed at a later point in the effect chain. So e.g. when using effects like SSR, it is common to apply TAA after it. This will make things more complicated though since the camera modification required for TAA (the Jitter) must be applied much earlier in the pass chain. That would require a deeper integration of TAA into our post processing system. However, this setup would automatically fix the instability issues from the previous point since we only have to resolve a single color attachment (which is also better for performance).

So I think the PR is a good as it is although the suggestion from #31361 (comment) should be implemented. Using textureLoad() instead of texture() is a plus performance-wise.

For the future, we should implement something like traa() that can be used similar to fxaa() and smaa() at different positions in the pass chain. A good default position for FX seems to me similar to SMAA after tone mapping but before sRGB output encoding. If you use stuff like motion blur, TAA might come earlier but such details could be covered in the documentation later.

@sunag sunag marked this pull request as ready for review July 11, 2025 19:34
@sunag sunag merged commit 42048f8 into mrdoob:dev Jul 11, 2025
12 checks passed
@sunag sunag added this to the r179 milestone Jul 11, 2025
@sunag sunag deleted the dev-mrt-traa branch July 11, 2025 19:39
Mugen87 added a commit that referenced this pull request Jul 15, 2025
Mugen87 added a commit that referenced this pull request Jul 15, 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.

2 participants