fix(TAA): banding and oversharpening#2399
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRewrites ISTemporalAA.hlsl from a phase-based 9-tap TAA to a packed 3x3 neighbor pipeline with dynamic-resolution UV clamping, depth-guided reprojection, luminance bracket/flicker scoring, temporal blend/clamp/sharpen, and motion/disocclusion/mask rejection; PS entry/output remain unchanged. ChangesTemporal Anti-Aliasing Rewrite
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Pull request overview
This PR rewrites the BSImagespaceShaderISTemporalAA pixel shader to more closely mirror the vanilla/decompiled TAA implementation, aiming to fix banding artifacts and unintended oversharpening when Imagespace is toggled (issues #2371 / #2375 context).
Changes:
- Replaces the prior higher-level/annotated TAA implementation with “verbatim” decompile-style math and register packing/swizzle behavior.
- Reworks neighborhood sampling, history reprojection, and rejection logic (depth-guided selection, mask/alpha gating, flicker score, sharpening stages).
- Refactors HDR conversion handling by adding local PQ conversion helpers and applying them around sampling/output.
Comments suppressed due to low confidence (1)
package/Shaders/ISTemporalAA.hlsl:88
- ClampHistoryUV mirrors FrameBuffer::GetPreviousDynamicResolutionAdjustedScreenPosition but does not apply VR eye-half clamping. In VR this can allow history sampling from the opposite eye, which typically shows up as temporal smear/ghosting at the eye boundary. Suggest using the FrameBuffer helper or incorporating the same VR clamp logic.
float2 ClampHistoryUV(float2 reprojectedUV)
{
float2 uv = max(FrameBuffer::DynamicResolutionParams1.zw * reprojectedUV, float2(0, 0));
uv.x = min(FrameBuffer::DynamicResolutionParams2.w, uv.x);
uv.y = min(FrameBuffer::DynamicResolutionParams1.w, uv.y);
return uv;
}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package/Shaders/ISTemporalAA.hlsl`:
- Around line 455-467: The alpha-coverage gating logic for motionReject misses
the drUVMax/corner sample from SelectDepthGuidedUV (loaded into corner/drUVMax),
so a fully-covered path can be taken when only that corner is uncovered; update
the AlphaCoverageMask checks (the sequence using AlphaCoverageMask on
drNeighborsA/B/C and sampleUV.z) to also call AlphaCoverageMask on the
drUVMax/corner UVs and AND that result into motionReject.x (and/or
motionReject.w as appropriate) so the corner tap is required before allowing the
covered path. Ensure you reference motionReject, AlphaCoverageMask,
drUVMax/corner, and SelectDepthGuidedUV when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eb2472cb-6743-4cca-9302-997e6c36c5d3
📒 Files selected for processing (1)
package/Shaders/ISTemporalAA.hlsl
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package/Shaders/ISTemporalAA.hlsl`:
- Around line 439-449: The HDR luma-difference scale is wrong: in the HDR branch
where motionReject.z is computed and lumaDiffScaled is set, replace the
multiplier 0.05 with 2.5 so the downstream history.xw computation (which uses
lumaDiffScaled.xx * float2(20,100)) produces the intended ~50 and ~250
coefficients; update the scalar in the block that computes lumaDiffScaled (refer
to motionReject.z, history.xw, centerMeta.x) accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c579a1d-e061-49bf-955a-778f78f881fb
📒 Files selected for processing (1)
package/Shaders/ISTemporalAA.hlsl
|
lgtm |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
fixes bugs in TAA. Rewritten from raw .hlsl again, similar to alandtse's pr but I could not address the bugs in that pr so I started over.

Dev has above lines etc.
this pr fixes it
closes #2371 & #2375
Summary by CodeRabbit