chore(VR): disable stereoopt by default#2097
Conversation
|
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughVR stereo-optimization gating was tightened and reworked: shader define injection and D3D11 hook installation are now conditioned differently, VR resource setup checks additional runtime flags, and several defaults for stereo settings were changed to disabled states. Changes
Sequence Diagram(s)sequenceDiagram
participant Init as Process startup
participant Globals as globals::InstallD3DHooks
participant VR as VR::SetupResources
participant Deferred as Deferred::GetComputeMainComposite
participant GPU as Shader compile / dispatch
rect rgba(100,150,240,0.5)
Init->>Globals: call InstallD3DHooks
Globals->>VR: check isVR && stereoMode != Off
end
rect rgba(120,200,140,0.5)
Init->>VR: call SetupResources
VR->>VR: if isVR && stereoMode != Off => stereoOpt.SetupResources() / loaded=true
VR->>VR: else stereoOpt.loaded = false
end
rect rgba(240,180,80,0.5)
Deferred->>VR: query HasShaderDefine(Type::Utility)
VR->>Deferred: return stereoOpt.CanDispatchStencil() (or false)
Deferred->>GPU: include `VR_STEREO_OPT` define if REL::Module::IsVR() (and predicate allows)
GPU->>GPU: compile/dispatch shader
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Deferred.cpp`:
- Around line 589-590: Change the compile-time VR define to use the stable VR
presence check instead of the transient readiness flag: replace uses of
globals::features::vr.IsStereoOptimizationCullingReady() when pushing the
"VR_STEREO_OPT" define (the code around defines.push_back({ "VR_STEREO_OPT",
nullptr })) with REL::Module::IsVR() so mainCompositeCS and
mainCompositeInteriorCS compile with the correct VR permutation; keep runtime
behavior of SRV binding (pass null when IsStereoOptimizationCullingReady() is
false) and rely on DeferredCompositeCS shader branch logic to handle the
not-ready case without recompiling.
In `@src/Globals.cpp`:
- Around line 361-364: The detour installation is incorrectly gated by
globals::features::vr.IsStereoOptimizationCullingReady(), causing hooks never to
be installed because resources are created later; update InstallD3DHooks (the
block that checks globals::game::isVR) to remove the
IsStereoOptimizationCullingReady() condition and always call the
stl::detour_vfunc<36, ID3D11DeviceContext_OMSetDepthStencilState>(a_context) and
stl::detour_vfunc<53, ID3D11DeviceContext_ClearDepthStencilView>(a_context) when
globals::game::isVR is true, relying on the existing runtime guards in the hook
thunks to protect functionality until VR::SetupResources() runs.
🪄 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: 8af8f479-f370-4b6c-b974-aed36da5f612
📒 Files selected for processing (5)
src/Deferred.cppsrc/Features/VR.cppsrc/Features/VR.hsrc/Features/VRStereoOptimizations.hsrc/Globals.cpp
|
✅ A pre-release build is available for this PR: |
|
check ai and i will approve |
Summary by CodeRabbit
Bug Fixes
Defaults