fix(VR): skip kSAO_CAMERAZ RTV in depth pass#2192
Conversation
kSAO_CAMERAZ is allocated at per-eye (half-stereo) resolution in VR but the fullscreen draw uses a full-stereo viewport, causing the PS to write garbage into the left-eye region. Pass nullptr for RTV slot 1 in VR to match the engine's own ISAOCameraZ pass behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
🧹 Nitpick comments (1)
src/Features/Upscaling.cpp (1)
1934-1938: Fix looks correct; consider tightening the comment wording.The nullptr RTV in VR correctly avoids writing the full-stereo viewport into the per-eye
kSAO_CAMERAZtarget, andISSAOCameraZalready populates it. Behavior-wise this matches the PR intent.Minor nit: the comment says
kSAO_CAMERAZis at "quarter-stereo resolution" and that corruption hits "the top-left quarter", but per the PR description the RT is per-eye (half-width × full-height of the stereo target), and the observed artifact is in the left-eye region. Consider aligning the wording with the PR description to avoid future confusion.✏️ Suggested comment tweak
- // kSAO_CAMERAZ is at quarter-stereo resolution in VR; the full-stereo viewport would - // corrupt only the top-left quarter. The engine's ISSAOCameraZ pass populates it correctly. + // kSAO_CAMERAZ is allocated at per-eye (half-stereo) resolution in VR, so a full-stereo + // viewport would write garbage into the left-eye region. The engine's ISSAOCameraZ pass + // already populates this target correctly, so skip binding it here in VR. ID3D11RenderTargetView* rtvs[] = { refractionNormals.RTV, globals::game::isVR ? nullptr : saoCameraZ.RTV }; context->OMSetRenderTargets(2, rtvs, depth.views[0]);Also, as per coding guidelines, consider shortening the PR title to fit the 50-char limit, e.g.
fix(vr): skip kSAO_CAMERAZ RTV in depth upscale(lowercase scope, ≤50 chars).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Features/Upscaling.cpp` around lines 1934 - 1938, The comment above the rtvs initialization is slightly misleading about kSAO_CAMERAZ resolution and artifact location; update the comment near the ID3D11RenderTargetView* rtvs[] block (references: rtvs, saoCameraZ.RTV, kSAO_CAMERAZ, ISSAOCameraZ, globals::game::isVR) to state that kSAO_CAMERAZ is a per-eye target (half-width × full-height of the stereo target) and the observed corruption appears in the left-eye region, and note that the ISSAOCameraZ pass already populates the target so we intentionally pass nullptr for VR to avoid writing the full-stereo viewport. Also apply the suggested shorter PR title when creating the changelist (e.g., "fix(vr): skip kSAO_CAMERAZ RTV in depth upscale").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Features/Upscaling.cpp`:
- Around line 1934-1938: The comment above the rtvs initialization is slightly
misleading about kSAO_CAMERAZ resolution and artifact location; update the
comment near the ID3D11RenderTargetView* rtvs[] block (references: rtvs,
saoCameraZ.RTV, kSAO_CAMERAZ, ISSAOCameraZ, globals::game::isVR) to state that
kSAO_CAMERAZ is a per-eye target (half-width × full-height of the stereo target)
and the observed corruption appears in the left-eye region, and note that the
ISSAOCameraZ pass already populates the target so we intentionally pass nullptr
for VR to avoid writing the full-stereo viewport. Also apply the suggested
shorter PR title when creating the changelist (e.g., "fix(vr): skip kSAO_CAMERAZ
RTV in depth upscale").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 20428c3d-1724-44e9-aa48-f53ce5b393f8
📒 Files selected for processing (1)
src/Features/Upscaling.cpp
|
✅ A pre-release build is available for this PR: |
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> (cherry picked from commit a8f2e52)
Summary
nullptrfor RTV slot 1 in VR, matching the engine's own pass behaviorTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit