feat(VR): enable upscaling#1507
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRemoves VR stereo SSGI sampling/blending from DeferredCompositeCS.hlsl; adds Upscaling::IsUpscalingActive and GetFrameGenerationFrameTime; centralizes VR depth-buffer culling via VR::UpdateDepthBufferCulling and disables culling UI during external upscaling; replaces hardcoded render-target/depth-stencil loop bounds with Util helpers; bumps VR address library/submodule. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Settings UI
participant VR as VR::UpdateDepthBufferCulling
participant U as Upscaling
participant R as Renderer
User->>UI: Toggle depth-buffer culling (desired)
UI->>VR: UpdateDepthBufferCulling(desired)
VR->>U: IsUpscalingActive()
U-->>VR: true / false
alt Upscaling active
VR->>R: Force-disable depth-buffer culling
Note right of R #dfe7f7: Prevents VR occlusion with external upscalers
else Upscaling inactive
VR->>R: Apply desired culling state
end
sequenceDiagram
autonumber
participant Shader as DeferredCompositeCS.hlsl
participant G as GBuffer/SSGI
participant Out as Framebuffer
Note over Shader: SSGI now uses single-eye sampling (no stereo branch)
Shader->>G: Sample SSGI AO & specular (single-eye)
G-->>Shader: AO, specular
Shader->>Out: Compose final irradiance/specular (no stereo blend)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (13)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.{cpp,cxx,cc,h,hpp,hxx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧬 Code graph analysis (3)src/FrameAnnotations.cpp (1)
src/Features/VR.h (1)
src/Features/VR.cpp (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (21)
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 |
There was a problem hiding this comment.
Pull Request Overview
Enable basic VR upscaling and ensure compatibility by disabling depth-based occlusion culling when downscaling upscalers are active. Also updates the minimum VR Address Library requirement.
- Add IsUpscalingActive() and wire VR to auto-disable depth buffer culling when downscaling is used
- Expose VR upscaling settings in UI and disable culling toggles while upscaling is active
- Adjust shaders to use depth-based masking in VR and remove stereo SSGI mixing paths; update hooks to support VR
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/XSEPlugin.cpp | Bump VR Address Library requirement to 0.193.0 to support new VR hooks. |
| src/Features/VR.cpp | Disable depth buffer culling when upscaling is active; update UI to reflect and lock settings accordingly. |
| src/Features/Upscaling.h | Add IsUpscalingActive() to expose active downscaling state. |
| src/Features/Upscaling.cpp | Enable VR in upscaling pipeline, add VR-safe checks, force-disable VR depth culling when needed, and revise hooks; add IsUpscalingActive() logic. |
| package/Shaders/DeferredCompositeCS.hlsl | Remove VR stereo SSGI mixing path to align with upscaling changes. |
| package/Shaders/AmbientCompositeCS.hlsl | Remove VR stereo SSGI mixing path to align with upscaling changes. |
| features/Upscaling/Shaders/Upscaling/DepthRefractionUpscalePS.hlsl | Replace VR stencil-based discard with depth-based discard; avoid filtered depth reads via Load. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Features/Upscaling.cpp (1)
47-52: Null-check pAdapter before GetDesc to avoid a crash.D3D11CreateDeviceAndSwapChain can be called with
pAdapter == nullptr.- DXGI_ADAPTER_DESC adapterDesc; - pAdapter->GetDesc(&adapterDesc); - globals::state->SetAdapterDescription(adapterDesc.Description); + if (pAdapter) { + DXGI_ADAPTER_DESC adapterDesc{}; + pAdapter->GetDesc(&adapterDesc); + globals::state->SetAdapterDescription(adapterDesc.Description); + } else { + globals::state->SetAdapterDescription(L"<default adapter>"); + }
🧹 Nitpick comments (5)
src/Features/Upscaling.h (1)
89-96: Make IsUpscalingActive a const, [[nodiscard]] query.Pure accessor; align with IsFrameGenerationActive() const.
- bool IsUpscalingActive(); + [[nodiscard]] bool IsUpscalingActive() const;features/Upscaling/Shaders/Upscaling/DepthRefractionUpscalePS.hlsl (1)
41-60: VR depth fetch via Load() is correct; minor guard suggestion.The texel clamp is good. Consider guarding against zero-sized BufferDim to avoid undefined cast if pipeline state is transient.
- uint2 texSize = (uint2)SharedData::BufferDim.xy; + uint2 texSize = max((uint2)1, (uint2)SharedData::BufferDim.xy);src/Features/Upscaling.cpp (3)
389-390: FXAA detour: add VR guard to be safe.- stl::detour_thunk<BSImageSpace_Init_FXAA>(REL::RelocationID(98974, 105626)); + if (!REL::Module::IsVR()) + stl::detour_thunk<BSImageSpace_Init_FXAA>(REL::RelocationID(98974, 105626));
782-801: Stencil state writes 0 to bound DSV; verify no later pass depends on main stencil.Given
OMSetDepthStencilState(..., 0x00)and REPLACE/ALWAYS, the main depth’s stencil will be zeroed wherever we draw. Confirm nothing consumes main stencil later, or switch to a DSV without stencil for this pass.Proposed safer tweak if available:
- Bind a depth-only (no stencil) DSV for the pass, or
- Change
StencilEnable=falsefor VR here if stencil is no longer used by the shader.
1242-1256: Match header: make IsUpscalingActive const.Also future-proof tiny epsilon.
-bool Upscaling::IsUpscalingActive() +bool Upscaling::IsUpscalingActive() const { auto method = GetUpscaleMethod(); // ... - return resolutionScale.x < .99f; + return resolutionScale.x < 0.99f; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
features/Upscaling/Shaders/Upscaling/DepthRefractionUpscalePS.hlsl(2 hunks)package/Shaders/AmbientCompositeCS.hlsl(0 hunks)package/Shaders/DeferredCompositeCS.hlsl(0 hunks)src/Features/Upscaling.cpp(9 hunks)src/Features/Upscaling.h(1 hunks)src/Features/VR.cpp(3 hunks)src/XSEPlugin.cpp(1 hunks)
💤 Files with no reviewable changes (2)
- package/Shaders/DeferredCompositeCS.hlsl
- package/Shaders/AmbientCompositeCS.hlsl
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
features/Upscaling/Shaders/Upscaling/DepthRefractionUpscalePS.hlslsrc/Features/Upscaling.hsrc/XSEPlugin.cppsrc/Features/Upscaling.cppsrc/Features/VR.cpp
features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
features/*/Shaders/**/*.{hlsl,hlsli,fx,fxh}: Place all feature shaders under features/YourFeature/Shaders/
Avoid GPU register/buffer conflicts in HLSL; verify register usage (e.g., with hlslkit buffer scanning)
Files:
features/Upscaling/Shaders/Upscaling/DepthRefractionUpscalePS.hlsl
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/Upscaling.hsrc/XSEPlugin.cppsrc/Features/Upscaling.cppsrc/Features/VR.cpp
🧠 Learnings (1)
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.
Applied to files:
features/Upscaling/Shaders/Upscaling/DepthRefractionUpscalePS.hlsl
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build plugin and addons
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
🔇 Additional comments (8)
features/Upscaling/Shaders/Upscaling/DepthRefractionUpscalePS.hlsl (1)
4-6: Include order LGTM; ensures EPSILON and BufferDim are defined before use.No issues spotted.
src/Features/VR.cpp (3)
112-131: Correctly disable depth-buffer culling when upscaling is active.Pointer checks + no-log-spam logic look good.
137-154: EarlyPrepass guard mirrors DataLoaded appropriately.Good restoration on interiors/exteriors; minimal log churn.
Please confirm EarlyPrepass is executed on every frame in VR so runtime toggling of upscaler switches culling promptly after loading.
593-631: UI disables culling toggles when upscaling is active—nice UX.Tooltips are clear. No functional issues.
src/Features/Upscaling.cpp (3)
192-200: Preset label mapping LGTM.Indexes align with qualityMode range; no issues.
731-748: Nice safety: force-disable VR depth culling when downscaling is active.Matches the PR objective and prevents bad occlusion.
1239-1240: FG disabled in VR—LGTM.Clear conservative guard.
src/XSEPlugin.cpp (1)
147-149: Surface VR Address Library version failure to the user (fail fast)If REL::Module::IsVR() is true, check the boolean return of REL::IDDatabase::get().IsVRAddressLibraryAtLeastVersion(...) and, on false, log an error and push it into errors so the UI flow cannot proceed.
File: src/XSEPlugin.cpp (around lines 147–149)
- if (REL::Module::IsVR()) { - REL::IDDatabase::get().IsVRAddressLibraryAtLeastVersion("0.193.0", true); - } + if (REL::Module::IsVR()) { + if (!REL::IDDatabase::get().IsVRAddressLibraryAtLeastVersion("0.193.0", false)) { + auto errorMessage = std::string("VR Address Library >= 0.193.0 is required"); + logger::error("{}", errorMessage); + errors.push_back(errorMessage); + } + }Also confirm release notes/docs mention VR Address Library >= 0.193.0.
|
✅ A pre-release build is available for this PR: |
|
This needs to be rebased |
alandtse
force-pushed
the
vr_upscaling_fix
branch
from
88c88e8 to
b352d28
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Features/Upscaling.cpp (1)
747-763: Use VR::UpdateDepthBufferCulling() instead of writing the global directly.Centralize behavior via the new API to avoid duplication and missed restores.
- if (globals::game::isVR) { - auto& vr = globals::features::vr; - if (IsUpscalingActive()) { - if (vr.gDepthBufferCulling) { - if (*vr.gDepthBufferCulling) { - *vr.gDepthBufferCulling = false; - logger::info("[Upscaling] VR detected - forcing depth buffer culling OFF due to active downscaling upscaler (scale={})", resolutionScale.x); - } - } else { - logger::warn("[Upscaling] VR depth buffer culling pointer is null, cannot force disable"); - } - } - } + if (globals::game::isVR && IsUpscalingActive()) { + auto& vr = globals::features::vr; + // Pass desired state; VR helper will force-disable when upscaling is active. + bool desired = false; + if (auto tes = RE::TES::GetSingleton()) { + desired = tes->interiorCell ? vr.settings.EnableDepthBufferCullingInterior + : vr.settings.EnableDepthBufferCullingExterior; + } else { + desired = vr.settings.EnableDepthBufferCullingExterior; + } + vr.UpdateDepthBufferCulling(desired); + }src/Features/VR.cpp (1)
1573-1587: Harden null-pointer handling and logging.Add explicit else-path logging when gDepthBufferCulling is null to aid diagnostics; early-return to reduce branching.
void VR::UpdateDepthBufferCulling(bool desired) { - if (globals::features::upscaling.IsUpscalingActive()) { - if (gDepthBufferCulling && *gDepthBufferCulling) { - logger::info("Upscaling detected, disabling incompatible depth buffer culling."); - *gDepthBufferCulling = false; - } - } else { - if (gDepthBufferCulling && *gDepthBufferCulling != desired) { - *gDepthBufferCulling = desired; - logger::info("VR depth buffer culling restored to {}", desired); - } - } + if (!gDepthBufferCulling) { + logger::warn("VR depth buffer culling pointer is null; cannot update"); + return; + } + if (globals::features::upscaling.IsUpscalingActive()) { + if (*gDepthBufferCulling) { + logger::info("Upscaling detected, disabling incompatible depth buffer culling."); + *gDepthBufferCulling = false; + } + return; + } + if (*gDepthBufferCulling != desired) { + *gDepthBufferCulling = desired; + logger::info("VR depth buffer culling restored to {}", desired); + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package/Shaders/DeferredCompositeCS.hlsl(0 hunks)src/Features/Upscaling.cpp(9 hunks)src/Features/Upscaling.h(1 hunks)src/Features/VR.cpp(4 hunks)src/Features/VR.h(1 hunks)src/XSEPlugin.cpp(1 hunks)
💤 Files with no reviewable changes (1)
- package/Shaders/DeferredCompositeCS.hlsl
🚧 Files skipped from review as they are similar to previous changes (2)
- src/XSEPlugin.cpp
- src/Features/Upscaling.h
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Features/Upscaling.cppsrc/Features/VR.hsrc/Features/VR.cpp
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/Upscaling.cppsrc/Features/VR.hsrc/Features/VR.cpp
🧠 Learnings (1)
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
PR: doodlum/skyrim-community-shaders#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to src/**/*.{cpp,cxx,cc,h,hpp,hxx} : Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Applied to files:
src/Features/Upscaling.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build plugin and addons
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
🔇 Additional comments (14)
src/Features/Upscaling.cpp (9)
184-212: Preset/UI logic looks solid.Clamped preset index prevents OOB; single SliderInt with context label is clean.
700-704: Vendor scale selection LGTM.Input scale sourced from the active backend is correct.
792-792: Stencil enablement for VR is reasonable.Good separation of VR vs non‑VR depth-stencil config.
1053-1054: Correctly excludes VR from frame generation.Matches PR goal and avoids unsupported paths in VR.
402-414: PC-only detours must be gated off on VR.These relocations can be absent/different on VR; patching unconditionally risks crashes. Wrap in a VR runtime guard.
[ suggest_essential_refactor ]
- // Patches RSSetScissorRect calls to use dynamic resolution - // This is a PC-specific function hence it was missing - stl::detour_thunk<SetScissorRect>(REL::RelocationID(75564, 77365)); - - // Patches facegen texture generation to not use dynamic resolution - stl::detour_thunk<BSFaceGenManager_UpdatePendingCustomizationTextures>(REL::RelocationID(26455, 27041)); - - // Patches precipitation camera to not use dynamic resolution - stl::write_thunk_call<Main_RenderPrecipitation>(REL::RelocationID(35560, 36559).address() + REL::Relocate(0x3A1, 0x3A1, 0x2FA)); - - // Forces FXAA off - stl::detour_thunk<BSImageSpace_Init_FXAA>(REL::RelocationID(98974, 105626)); + // PC-only hooks; skip on VR + if (!REL::Module::IsVR()) { + // Patches RSSetScissorRect calls to use dynamic resolution + stl::detour_thunk<SetScissorRect>(REL::RelocationID(75564, 77365)); + // Patches facegen texture generation to not use dynamic resolution + stl::detour_thunk<BSFaceGenManager_UpdatePendingCustomizationTextures>(REL::RelocationID(26455, 27041)); + // Patches precipitation camera to not use dynamic resolution + stl::write_thunk_call<Main_RenderPrecipitation>(REL::RelocationID(35560, 36559).address() + REL::Relocate(0x3A1, 0x3A1, 0x2FA)); + // Forces FXAA off + stl::detour_thunk<BSImageSpace_Init_FXAA>(REL::RelocationID(98974, 105626)); + }Based on learnings
682-685: Null-check INI setting before dereference.RE::GetINISetting can return nullptr across SE/AE/VR variants.
- auto fDRClampOffset = RE::GetINISetting("fDRClampOffset:Display"); - fDRClampOffset->data.f = 0.0f; + if (auto fDRClampOffset = RE::GetINISetting("fDRClampOffset:Display")) { + fDRClampOffset->data.f = 0.0f; + } else { + logger::debug("INI fDRClampOffset:Display not found; skipping clamp offset reset"); + }
743-743: Fix typo in comment.“explictly” → “explicitly”.
- // Disable dynamic resolution unless the game explictly enables it + // Disable dynamic resolution unless the game explicitly enables it
58-59: Guard FLIP_DISCARD to flip-compatible swapchains only.Unconditional FLIP_DISCARD can fail when MSAA is enabled or BufferCount < 2. Add guards and fallback to DISCARD.
[ suggest_essential_refactor ]
Apply:- // Use better swap effect to prevent tearing and improve performance - pSwapChainDesc->SwapEffect = DXGI_SWAP_EFFECT_FLIP_DISCARD; + // Prefer FLIP_DISCARD when flip-compatible; otherwise, use a compatible fallback + if (pSwapChainDesc->SampleDesc.Count <= 1 && pSwapChainDesc->SampleDesc.Quality == 0) { + if (pSwapChainDesc->BufferCount < 2) { + pSwapChainDesc->BufferCount = 2; + } + pSwapChainDesc->SwapEffect = DXGI_SWAP_EFFECT_FLIP_DISCARD; + } else { + pSwapChainDesc->SwapEffect = DXGI_SWAP_EFFECT_DISCARD; + }Based on learnings
1056-1070: Verify resolutionScale default initialization
Ensure resolutionScale.x is explicitly initialized (e.g. to 1.0f in the Upscaling constructor) or add a guard in IsUpscalingActive to prevent using an uninitialized value.src/Features/VR.h (1)
285-286: Public API addition looks good.Clear name and signature for centralizing depth-culling control.
src/Features/VR.cpp (4)
6-6: Include dependency is appropriate.Needed to query upscaling state.
114-118: Good: centralizing depth-culling on load.Respects user preference but defers to upscaling state.
124-128: Good: re-evaluates per-scene (interior/exterior) early.Matches intended behavior.
567-606: UI disablement tied to upscaling is well-implemented.Clear tooltips and proper BeginDisabled/EndDisabled pairs.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Features/Upscaling.cpp(9 hunks)src/Features/Upscaling.h(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/Features/Upscaling.cppsrc/Features/Upscaling.h
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/Features/Upscaling.cppsrc/Features/Upscaling.h
🧠 Learnings (1)
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
PR: doodlum/skyrim-community-shaders#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to src/**/*.{cpp,cxx,cc,h,hpp,hxx} : Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Applied to files:
src/Features/Upscaling.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Build plugin and addons
🔇 Additional comments (1)
src/Features/Upscaling.h (1)
93-93: MakeIsUpscalingActive()constWe previously flagged this: the accessor doesn’t mutate object state, so please mark both the declaration and definition
constto keep the API const-correct.Apply this diff to the declaration:
- bool IsUpscalingActive(); + bool IsUpscalingActive() const;…and remember to mirror the change in the definition.
Enables basic upscaling for VR. This requires disabling depth based culling when any downscaling is used. Requires VR Address Library 0.193.0
During RenderShadowMasks the shadowmasks is being clipped by SetScissorRect. This avoids the clipping but probably causes a performance hit. Known issue is the in game menu sometimes disappears depending on upscaling setting.
alandtse
force-pushed
the
vr_upscaling_fix
branch
from
9f7487f to
f8e278d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
extern/CommonLibSSE-NG(1 hunks)package/Shaders/DeferredCompositeCS.hlsl(0 hunks)src/Features/Upscaling.cpp(9 hunks)src/Features/Upscaling.h(2 hunks)src/Features/VR.cpp(4 hunks)src/Features/VR.h(1 hunks)src/FrameAnnotations.cpp(2 hunks)src/Utils/D3D.cpp(4 hunks)src/Utils/D3D.h(1 hunks)src/XSEPlugin.cpp(1 hunks)
💤 Files with no reviewable changes (1)
- package/Shaders/DeferredCompositeCS.hlsl
✅ Files skipped from review due to trivial changes (1)
- extern/CommonLibSSE-NG
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Features/VR.h
- src/Features/Upscaling.h
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not include TODO/FIXME placeholders; provide complete, working solutions
Files:
src/XSEPlugin.cppsrc/FrameAnnotations.cppsrc/Utils/D3D.hsrc/Features/VR.cppsrc/Features/Upscaling.cppsrc/Utils/D3D.cpp
src/**/*.{cpp,cxx,cc,h,hpp,hxx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{cpp,cxx,cc,h,hpp,hxx}: Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Include robust error handling and resource management with graceful degradation in the plugin code
Files:
src/XSEPlugin.cppsrc/FrameAnnotations.cppsrc/Utils/D3D.hsrc/Features/VR.cppsrc/Features/Upscaling.cppsrc/Utils/D3D.cpp
🧠 Learnings (1)
📚 Learning: 2025-08-17T18:37:35.839Z
Learnt from: CR
PR: doodlum/skyrim-community-shaders#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-17T18:37:35.839Z
Learning: Applies to src/**/*.{cpp,cxx,cc,h,hpp,hxx} : Ensure SE/AE/VR runtime compatibility; use runtime detection patterns (e.g., REL::RelocateMember())
Applied to files:
src/Features/Upscaling.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Build plugin and addons
🔇 Additional comments (16)
src/XSEPlugin.cpp (1)
148-149: VR Address Library min version bump looks correct; confirm failure path.If
IsVRAddressLibraryAtLeastVersion("0.193.0", true)does not hard-fail internally, consider surfacing a user-facing error (push toerrorsand disable features) for consistency with the other DLL checks. Please confirm the function’s behavior on older versions at runtime.Based on learnings
src/Features/VR.cpp (5)
6-6: Include dependency OK.Directly including Upscaling to query state is reasonable here.
114-118: Good: centralizing initial depth-culling state via UpdateDepthBufferCulling.Removes duplication and ensures a single code path decides culling based on upscaling.
124-128: Good: per-frame reconciliation in EarlyPrepass.This keeps depth-culling synced with runtime upscaling changes.
567-606: UI disable/tooltip logic looks solid.Controls are correctly disabled when upscaling is active; tooltips explain why.
1573-1588: Central helper is right; remove duplicate enforcement in Upscaling.cpp.Now that VR owns the policy, delete the duplicate force‑disable block in
Upscaling::ConfigureUpscaling()(lines 758‑775 there) and just call this helper when needed from Upscaling.Apply in Upscaling.cpp:
@@ - // If running in VR and an external upscaler is active, force-disable - // the engine's depth-buffer culling immediately. This ensures that - // enabling upscaling at runtime (after game load) does not leave the - // VR depth-buffer culling enabled which can cause incorrect occlusion. - if (globals::game::isVR) { - auto& vr = globals::features::vr; - if (IsUpscalingActive()) { - if (vr.gDepthBufferCulling) { - if (*vr.gDepthBufferCulling) { - *vr.gDepthBufferCulling = false; - logger::info("[Upscaling] VR detected - forcing depth buffer culling OFF due to active downscaling upscaler (scale={})", resolutionScale.x); - } - } else { - logger::warn("[Upscaling] VR depth buffer culling pointer is null, cannot force disable"); - } - } - } + // Defer policy to VR helper to avoid duplication/divergence. + if (globals::game::isVR) { + globals::features::vr.UpdateDepthBufferCulling( + globals::features::vr.settings.EnableDepthBufferCullingExterior); + }Based on learnings
src/Features/Upscaling.cpp (10)
9-9: Include OK.VR linkage used only for helpers; fine.
184-215: Preset index/labels logic is safe and clamps correctly.No functional concerns.
704-708: Method-based resolution scale selection looks correct.Clean split between DLSS and FSR.
754-757: Comment clarifies intent; OK.No change requested.
758-775: Remove VR depth-culling enforcement here; use VR::UpdateDepthBufferCulling.This duplicates logic now centralized in VR.cpp and risks divergence. See suggested diff in VR.cpp comment.
Based on learnings
803-821: Depth/stencil config gated by VR: sensible.Stencil path only in VR; matches usage below.
1064-1065: Frame generation disabled on VR: good.Prevents proxy path on VR.
1067-1081: IsUpscalingActive heuristic is fine.Treating vendor downscaling as “active” using < 0.99x is pragmatic.
686-689: Null-check INI setting before dereference.
RE::GetINISetting("fDRClampOffset:Display")may return null across SE/AE/VR.Apply:
- auto fDRClampOffset = RE::GetINISetting("fDRClampOffset:Display"); - fDRClampOffset->data.f = 0.0f; + if (auto fDRClampOffset = RE::GetINISetting("fDRClampOffset:Display")) { + fDRClampOffset->data.f = 0.0f; + } else { + logger::debug("INI fDRClampOffset:Display not found; skipping clamp offset reset"); + }Based on learnings
58-60: Don’t force FLIP_DISCARD unconditionally; can break swap-chain creation.Flip-model requires BufferCount ≥ 2 and SampleDesc.Count == 1 (and often Quality == 0). Guard it and fall back to DISCARD. Also handle null
pSwapChainDesc.Apply:
- // Use better swap effect to prevent tearing and improve performance - pSwapChainDesc->SwapEffect = DXGI_SWAP_EFFECT_FLIP_DISCARD; + // Use flip model when compatible; otherwise keep legacy discard + if (pSwapChainDesc) { + if (pSwapChainDesc->SampleDesc.Count <= 1 && pSwapChainDesc->SampleDesc.Quality == 0) { + if (pSwapChainDesc->BufferCount < 2) { + pSwapChainDesc->BufferCount = 2; + } + pSwapChainDesc->SwapEffect = DXGI_SWAP_EFFECT_FLIP_DISCARD; + } else { + pSwapChainDesc->SwapEffect = DXGI_SWAP_EFFECT_DISCARD; + } + }Additionally, earlier in this function
pAdapter->GetDescis called without a null check; D3D11 allowspAdapter == nullptr. Consider guarding that in a follow-up.
Based on learnings
|
Woot, many thanks |
* chore(ui): update discord banner (community-shaders#1493) * fix: use proper filename settingsuser.json (community-shaders#1491) * chore(upscaling): increase fsr sharpness * chore: rename d3d12interop to d3d12SwapChainActive (community-shaders#1494) * feat(llf): remove particle lights (community-shaders#1495) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat(llf): move llf to core (community-shaders#1496) * fix: remove water clamp (community-shaders#1497) * fix(upscaling): more upscaling fixes (community-shaders#1498) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix: fix some internal errors when debugging (community-shaders#1500) * fix(ui): fix save settings conflicts & welcome screen (community-shaders#1501) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(ui): add constraints for discord banner size (community-shaders#1463) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(VR): fix exiting menu using controllers (community-shaders#1502) * build: fix warnings (community-shaders#1505) * feat(UI): allow tooltips for disabled elements (community-shaders#1503) * feat(upscaling): add downscale percentages (community-shaders#1506) * perf(ssgi): optimize (community-shaders#1499) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(ui): font size and perf overlay improvements (community-shaders#1511) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore: remove unused hooks (community-shaders#1510) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix: adjust IsInterior to consider kNoSky or kFixedDimensions flags (community-shaders#1512) * fix(hair): correct hair indirect normal, marschner by default (community-shaders#1515) Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore: mostly revert ISHDR to 1.3.6 (community-shaders#1516) * chore(upscaling): simplify interop and upscale methods (community-shaders#1514) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(hair): typo in code (community-shaders#1517) * feat(ibl): lerp sky ibl using skylighting (community-shaders#1519) * fix(sss): burley artifacts with effect blend (community-shaders#1518) * fix(upscaling): fix screenshots when upscaling enabled (community-shaders#1520) * fix(upscaling): fix mipbias sometimes being wrong (community-shaders#1521) * fix: fix compile error if snow shader on (community-shaders#1522) * chore(upscaling): revert fsr to typical settings (community-shaders#1523) * fix: fix minor ui issues (community-shaders#1524) * chore(grass collision): simpler grass collision (community-shaders#1525) * fix: update skylighting and version * fix(pbr): fix inconsistencies (community-shaders#1526) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: jiayev <l936249247@hotmail.com> * feat(upscaling): sharpening slider (community-shaders#1527) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore: bump versions * fix(ibl): add ibl to reflection normalization (community-shaders#1528) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(hair): remove pbr lighting mult for hair (community-shaders#1531) * chore(upscaling): add back upscale multiplier (community-shaders#1532) * fix(upscaling): fix minor upscaling issues (community-shaders#1536) * chore: gamma space normalisation (community-shaders#1535) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(grass collision): implement with texture and history (community-shaders#1539) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * chore(grass collision): less aggressive (community-shaders#1546) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(skylighting): fix cell id casting (community-shaders#1544) * chore(emat): auto detect terrain parallax (community-shaders#1545) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore: update versions * feat(VR): enable upscaling (community-shaders#1507) * fix(terrain shadows): fix brightened lods (community-shaders#1547) * chore(upscaling): reduce ghosting near camera (community-shaders#1548) * fix: fix grass not animating (community-shaders#1549) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix(grass collision): fix non-standard timescales (community-shaders#1550) * build: deploy only updated files (community-shaders#1556) * feat: add Clear Shader Cache to Advanced (community-shaders#1555) * chore(featureissues): default collapse testing menu (community-shaders#1554) * fix(VR): use only supported shaders from cache (community-shaders#1553) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * build: use gersemi cmake formatter (community-shaders#1557) * fix(terrain): vanilla diffuse in pbr terrain cell too bright due to wrong color space (community-shaders#1558) * docs: add new feature development template guide (community-shaders#1529) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * docs(UI): remove duplicate GPL license statement (community-shaders#1561) * feat: add renderdoc for debugging (community-shaders#1560) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@gmail.com> * fix(ui): welcome popup size issues (community-shaders#1573) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore(grass collision): minor tweaks (community-shaders#1568) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(terrain helper): fix conflicting bit (community-shaders#1566) Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(UI): separate theme settings, UI refactor, font support (community-shaders#1571) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * chore: bump versions * build: fix zipping aio (community-shaders#1579) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(grass collision): clamp maximum depth of grass (community-shaders#1578) * feat(UI): enhance shader blocking (community-shaders#1564) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@gmail.com> * fix: remove duplicate buffer setup (community-shaders#1586) * feat: update shader compile elapsed time every second (community-shaders#1587) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * build: add cmake install commands (community-shaders#1372) * feat(perf-overlay): add size controls (community-shaders#1591) * fix(perf-overlay): fix infinite draw calls table height (community-shaders#1590) * refactor(perf-overlay): remove collapsible headers (community-shaders#1572) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(perf-overlay): removed ImGuiTableFlags_ScrollX/Y for scroll bar issues (community-shaders#1594) * build: fix shader copying to relative paths (community-shaders#1603) * fix(ibl): apply ibl to cubemap normalisation for non deferred (community-shaders#1604) * fix(grass): use correct light direction (community-shaders#1602) * fix(welcome-popup): adjust font size & window spacing (community-shaders#1592) * feat(lod): add gamma sliders (community-shaders#1588) * build: correct CodeRabbit schema syntax (community-shaders#1608) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> * build: add compile-time validation of GPU buffers (community-shaders#1427) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> * ci: run shader validation on CMake and CI config changes (community-shaders#1606) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> * feat: procedural sun * limb darkening * another darkening * build(deps): remove orphaned Intel XeSS dependency (community-shaders#1611) Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> * fix: accumulate sunlight color in pixel shader output * fix(ui): enter key now behaves properly when first time popup is open (community-shaders#1615) * feat(ui): add tabs to advanced settings & PBR search (community-shaders#1599) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * build: add HLSL intellisense (community-shaders#1614) * refactor(UI): move light limit visualization into debug (community-shaders#1619) * refactor(ui): add settings for shader block hotkeys (community-shaders#1624) Co-authored-by: Bruce <44987693+brucenguyen@users.noreply.github.com> * fix(ui): anchor reset settings button position (community-shaders#1621) Co-authored-by: Giovanni Correia <Gistix@users.noreply.github.com> * fix(hair): use indirect normal for deferred marschner hair (community-shaders#1626) * build: fix Package-AIO-Manual for fresh pulls (community-shaders#1625) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(snow): use world space vectors (community-shaders#1618) * feat(UI): add gaussian blur shader core files (community-shaders#1595) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat(ui): add test conditions button (community-shaders#1637) * fix(ui): blocked shader info overflow in Shader Debug tab (community-shaders#1632) * fix(upscaling): replace NIS with RCAS for DLSS (community-shaders#1620) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix(dynamic cubemaps): add a check for timeskip (community-shaders#1639) * refactor: restructure lighting (community-shaders#1633) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * feat(ui): add themes & fonts (community-shaders#1596) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat(water): add flowmap parallax (community-shaders#1636) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * fix cloud shadow setting saving --------- Co-authored-by: zxcvbn <66063766+zndxcvbn@users.noreply.github.com> Co-authored-by: davo0411 <davidkehoe0411@outlook.com> Co-authored-by: doodlum <15017472+doodlum@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@users.noreply.github.com> Co-authored-by: soda <130315225+soda3000@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: ThePagi <32794457+ThePagi@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: alandtse <7086117+alandtse@users.noreply.github.com> Co-authored-by: Alan Tse <alandtse@gmail.com> Co-authored-by: Yupeng Zhang <ArcEarth@outlook.com> Co-authored-by: kuplion <kuplion@hotmail.com> Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Co-authored-by: Giovanni Correia <Gistix@users.noreply.github.com> Co-authored-by: Bruce <44987693+brucenguyen@users.noreply.github.com> Co-authored-by: Midona <106106405+midona-rhel@users.noreply.github.com>
Enables basic upscaling for VR. This requires disabling depth based culling when any downscaling is used (#1508).
Requires VR Address Library 0.193.0
Known Issues:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores