fix(vr): full-resolution underwater mask#34
Conversation
(cherry picked from commit b2a9b0e)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpscaleDepth was refactored to centralize depth-upscale gating, add finer resource validation, value-initialize jitter CB, reorganize depth-copy/stencil/render-target steps for active vs VR-repair flows, tighten VR propagation conditions, and update Main_PostProcessing dispatch routing. ChangesUpscale Depth Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.21.0)OpenGrep fatal error (exit code 2): [00.12][ERROR]: Error: exception Unix_error: No such file or directory stat src/Features/Upscaling.cpp 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
Fixes a VR-specific rendering regression where the underwater mask repair pass could bail out when vendor upscaling wasn’t “active” (DLAA / TAA / no upscaler), leaving the underwater tint/mask broken on those full-resolution paths. The change fits into the Upscaling feature’s postprocessing pipeline by ensuring the depth source used by the underwater-mask repair is refreshed even when no depth upscaling draw is performed.
Changes:
- Extends
UpscaleDepth()to run on VR full-resolution mask paths (kNONE / kTAA / DLAA/NativeAA) even whenIsUpscalingActive()is false, with split resource validation depending on whether depth upscaling is actually active. - Gates the wide-kernel hysteresis + depth-refinement draw behind
depthUpscaleActive, and adds a full-resolution branch that refreshesdepthCopyfromdepth. - Updates
Main_PostProcessingto callUpscaleDepth()on the VR kNONE/kTAA path wherePerformUpscaling()is not invoked.
The non-upscale branch already refreshes depthCopy from depth before running the underwater mask draw; the unconditional VR propagate block then copied the same resource a second time per frame. Gate the propagate on depthUpscaleActive so the full-resolution path issues exactly one CopyResource.
alandtse
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@src/Features/Upscaling.cpp`:
- Around line 1867-1869: The early return 'if (isVR && !depthCopy.stencilSRV) {
return; }' incorrectly applies to the full-resolution repair path (which only
updates depthCopy.texture and uses depthCopy.depthSRV for the underwater-mask
pass), so narrow this guard to only the VR upscale path: move or change the
condition so the check for depthCopy.stencilSRV runs only when executing the
upscale path (e.g., wrap it inside the upscale branch or add an isUpscalePath
flag check), leaving the full-resolution repair branch able to proceed even when
stencilSRV is unavailable; reference symbols: isVR, depthCopy.stencilSRV,
depthCopy.texture, depthCopy.depthSRV, underwater-mask pass, and the upscale
branch.
🪄 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
Run ID: a963c803-bb87-4e7d-8b5f-6cec21da91b1
📒 Files selected for processing (1)
src/Features/Upscaling.cpp
|
✅ A pre-release build is available for this PR: |
The full-resolution underwater mask repair samples depthCopy.depthSRV only; it never binds depthCopy.stencilSRV (an upscale-path input) or depth.views[0] (DSV for the depth-refinement draw). Requiring them in the shared early-exit disabled the VR repair on setups where stencil SRV creation isn't available, even though that branch can still succeed. Collapse both into one upscale-path guard.
Two Copilot review findings: 1. UpscaleDepth() can now be invoked standalone from Main_PostProcessing on the kNONE/kTAA VR path, bypassing Upscale()'s explicit RT unbind. The first CopyResource on depth/depthCopy can then race a still-bound DSV. Mirror Upscale()'s precondition by unbinding RTs at entry. 2. Use the existing local isVR flag in the propagate gate instead of re-reading globals::game::isVR.
Summary
Cherry-picks
b2a9b0e49fromParticleTroned/cs-1.5-PL-VR— the underwater mask repair pass currently bails whenIsUpscalingActive()is false (DLAA, TAA, or no upscaler), so VR users on those paths see a broken underwater mask. ParticleTroned's fix letsUpscaleDepth()run in the full-resolution mask path (vendor upscaler atqualityMode=0/ TAA / kNONE in VR) to refresh the depth source the analytical mask needs, then dispatches the existing underwater-mask repair draw.Authorship preserved via
git cherry-pick -x— original commit is by @ParticleTroned, the cherry-pick adds a(cherry picked from commit b2a9b0e49)trailer.What it changes
src/Features/Upscaling.cpp::UpscaleDepth:if (!IsUpscalingActive()) return;. Now also runs whenisVR && fullResolutionMaskPath && !depthUpscaleActive, wherefullResolutionMaskPathcovers kNONE / kTAA / vendor-upscaler-at-DLAA.upscaleDepthStencilState,refractionNormals.*,saoCameraZ.RTV, anddepth.views[0]are only required whendepthUpscaleActive(the full-res mask path doesn't draw the depth-refinement pass, just the mask draw).if (depthUpscaleActive).elsebranch refreshesdepthCopy.texturefromdepth.textureso the underwater mask PS gets fresh depth.Main_PostProcessingnow also callsUpscaleDepth()in the VR-kNONE / VR-kTAA branch (wherePerformUpscalingisn't called).Why this isn't already on dev
b2a9b0e49lives only on ParticleTroned'scs-1.5-PL-VR/cs-1.5-PL-VR-unleashedbranches and was never opened upstream. Surfacing it here so the fork has it on dev.Test plan
depthUpscaleActivebranch)isVR &&-gated)Credit
@ParticleTroned for the original fix.
Summary by CodeRabbit