fix(VR): SSGI discrepancies

[alandtse]

Summary by CodeRabbit

• New Features

  • Added configurable VR stereo blending (depth & color-aware) with debug views.
  • Enabled stereo-consistent Screen‑Space GI for VR and integrated a VR sync/blend pass.
  • New VR settings UI controls for stereo blend parameters.

• Bug Fixes / Improvements

  • Improved cross-eye consistency for GI sampling and horizon alignment.
  • Enhanced temporal denoising with neighborhood clamping for improved stability.
  • Better handling of depth discontinuities and out-of-bounds stereo samples.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds stereo-consistent bilateral blending for VR Screen-Space GI: cross-eye sample handling and reprojection utilities, new compute shaders for stereo sync and VR stereo blend, temporal denoising and motion-aware accumulation refinements, VR settings/UI and runtime integration into the render pipeline.

Changes

Cohort / File(s)	Summary
Screen Space GI shader changes features/Screen Space GI/Shaders/ScreenSpaceGI/blur.cs.hlsl, features/Screen Space GI/Shaders/ScreenSpaceGI/gi.cs.hlsl, features/Screen Space GI/Shaders/ScreenSpaceGI/radianceDisocc.cs.hlsl	Unified cross-eye projection and sample-eye tracking; mono-noise indexing; out-of-bounds samples attempt opposite eye; sample-depth/position decode updated for sample eye; temporal denoiser: 3x3 neighborhood Y/CoCg clamping and motion-aware accumulation; early history rejection on depth mismatch.
Stereo reprojection helpers package/Shaders/Common/VR.hlsli	Added StereoBilateralResult and functions ReprojectToOtherEye / FinalizeStereoBlend to compute other-eye UV/px, bilateral depth weight, and back-check validity for stereo sampling.
New stereo sync shader features/Screen Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl	New compute shader performing stereo-consistent bilateral blending of SSGI buffers by reprojection/back-check and weighted mixing of AO/IL channels.
VR stereo blend compute shader package/Shaders/VR/StereoBlendCS.hlsl	New VR post-process compute shader implementing depth-aware reprojection-based bilateral blend between eyes with debug visualization modes and color-difference gating.
VR runtime, settings, UI src/Features/VR.h, src/Features/VR.cpp, src/Features/VR/StereoBlend.cpp, src/Features/VR/SettingsUI.cpp	Added stereo-blend settings (enable, depth sigma, max factor, color threshold, debug mode), shader compilation/resources, stereo blend CB, shader cache clear hook, DrawStereoBlend implementation, and UI panel for controls.
Screen Space GI integration & pipeline src/Features/ScreenSpaceGI.h, src/Features/ScreenSpaceGI.cpp, src/Deferred.cpp	Enable SSGI in VR by default, adjust slice presets for VR, add stereoSyncCompute shader to build/cache, bind shared CBs, and invoke VR bilateral blend after blur (post-composite) in the deferred pipeline.

Sequence Diagram

sequenceDiagram
    participant EyeCurr as Current Eye Frame
    participant GI as SSGI Shader
    participant StereoUtil as Stereo reprojection utils
    participant EyeOther as Other Eye Depth/Textures
    participant StereoSync as Stereo Sync / Blend CS
    participant Output as Final Frame

    EyeCurr->>GI: Issue GI samples (include view-space offsets)
    GI->>StereoUtil: Convert sample to stereo UV / determine sampleEyeIndex
    StereoUtil->>EyeOther: If needed, reproj & sample other-eye depth/pos
    EyeOther-->>StereoUtil: Return other-eye depth/pos (or invalid)
    StereoUtil-->>GI: Return sample eye info & possibly corrected position
    GI->>GI: Apply temporal denoise (3x3 clamp), motion-aware accumulation
    GI->>StereoSync: Emit AO/IL channels with per-sample eye metadata
    StereoSync->>StereoUtil: Reproject pixels to other eye for blending
    StereoUtil->>EyeOther: Back-check sampling for bilateral weight
    StereoUtil-->>StereoSync: Blend weight + validity
    StereoSync->>Output: Write blended AO/IL into output UAVs

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• fix: move ResolveMonoUVForEye outside VR guard #1906: Modifies VR stereo-UV/reprojection code paths used by the new reprojection helpers and cross-eye sampling.
• fix(vr): improve stereo UV handling #1899: Adjusts stereo UV/eye-resolution logic and per-sample cross-eye handling similar to changes here.
• feat(VR): separate interior/exterior options for depth buffer culling #1362: Touches VR API and settings structures that overlap with the VR::Settings and VR class changes in this PR.

Suggested reviewers

• doodlum
• jiayev

Poem

🐰 I hopped from left to right and found,

Pixels crossed and depths that bound.
With reproj, blend, and gentle care,
VR seams vanish into air.
A bunny's code — now stereo fair!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix(VR): SSGI discrepancies' is related to the changeset but is overly vague. It mentions fixing VR-related SSGI issues without specifying what the discrepancies are or how they're fixed.	Consider a more descriptive title such as 'fix(VR): improve SSGI stereo consistency with bilateral blending' or 'fix(VR): add stereo-aware SSGI sampling and cross-eye synchronization' to clearly convey the main changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Using provided base ref: 3ecc7df
Using base ref: 3ecc7df
Base commit date: 2026-02-21T17:31:06-08:00 (Saturday, February 21, 2026 05:31 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Features/ScreenSpaceGI.h (1)

11-11: ⚠️ Potential issue | 🟡 Minor

SupportsVR() must return true for VR-specific features to load in VR mode.

The framework filters out features with SupportsVR() = false in production VR builds (see Feature.cpp:252). If this PR adds VR support—including stereo sync shaders and VR-specific presets—the method must return true, otherwise the entire feature is disabled in VR and the new VR code is unreachable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/ScreenSpaceGI.h` at line 11, The SupportsVR() override in
ScreenSpaceGI (in src/Features/ScreenSpaceGI.h) currently returns false which
prevents VR-specific features from loading; change the SupportsVR()
implementation in the ScreenSpaceGI class to return true so the framework will
include this feature in VR builds and allow the VR shaders/presets to be
reachable.

🧹 Nitpick comments (4)

features/Screen Space GI/Shaders/ScreenSpaceGI/radianceDisocc.cs.hlsl (1)

152-166: Motion-aware accumulation is a solid improvement for VR temporal stability.

The approach handles the three key scenarios well:

• Disocclusion: soft halving instead of hard reset (reduces flashing)
• Fast motion: reduced max accumulation prevents ghosting from stale history
• Static: full accumulation preserved

Minor observation: motionVec on line 162 effectively re-derives the motion vector that was already sampled from srcMotionVec at line 83. If the motion vector texture is already bound/available in this scope, you could reuse it directly. Not a correctness issue.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@features/Screen` Space GI/Shaders/ScreenSpaceGI/radianceDisocc.cs.hlsl around
lines 152 - 166, The code recomputes motionVec as (prev_screen_pos - screen_pos)
even though the shader already samples the motion vector into srcMotionVec
earlier (see srcMotionVec at line ~83); update the accumulation logic to reuse
the sampled motion vector instead of recomputing it — replace uses of motionVec
with the existing sampled motion vector (srcMotionVec or whatever local name
holds the sampled motion) when computing motionLen and motionMaxAccum and keep
the rest of the accumulation logic (prevAccum handling and accum_frames clamp)
unchanged.

src/Features/ScreenSpaceGI.cpp (1)

586-590: stereoSyncCompute always compiled with FRAMEBUFFER define.

This is correct since the stereo sync shader always needs the FrameBuffer matrices for cross-eye reprojection. However, this shader will also be compiled for non-VR (the VR define is added conditionally at line 593), though it's never dispatched on non-VR (line 874 guards on REL::Module::IsVR()). This is harmless but wastes a tiny bit of compile time on non-VR.

Skip compiling stereoSync on non-VR

You could conditionally include it in the shaderInfos vector only when REL::Module::IsVR(), similar to how the dispatch is guarded. Not critical since the compile cost is negligible.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/ScreenSpaceGI.cpp` around lines 586 - 590, The shaderInfos list
currently always includes the stereoSyncCompute entry causing an unnecessary
compile when not running VR; modify the code that builds the shaderInfos vector
(the initializer containing &giCompute, &blurCompute, &stereoSyncCompute,
&upsampleCompute in ScreenSpaceGI.cpp) to only push or include the
stereoSyncCompute entry when REL::Module::IsVR() is true (same condition used
for the dispatch guarded at REL::Module::IsVR()), so that stereoSync.cs.hlsl
(with the FRAMEBUFFER define) is compiled only for VR builds.

features/Screen Space GI/Shaders/ScreenSpaceGI/gi.cs.hlsl (1)

372-401: History clamping uses previous-frame neighborhood rather than current-frame neighborhood.

The comment references SVGF (Schied et al. 2017), which typically clamps history to the current frame's spatial neighborhood. Here, the min/max bounds are computed from srcPrevY[np] / srcPrevCoCg[np] (lines 385-386), which are the previous frame's accumulated results. This is a reasonable pragmatic choice since the current frame's GI is only available for the center pixel at this point, but it's worth noting the bounds will lag one frame behind.

Also, GI_SPECULAR at line 400 doesn't get the same clamping treatment — presumably intentional given its experimental status.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@features/Screen` Space GI/Shaders/ScreenSpaceGI/gi.cs.hlsl around lines 372 -
401, The history clamping currently builds min/max bounds from the
previous-frame buffers srcPrevY/srcPrevCoCg (used in the loop that computes
nMinY/nMaxY and nMinCoCg/nMaxCoCg), which lags the bounds one frame; change the
neighbor sample sources inside that loop to the current-frame per-pixel GI
samples (i.e., sample the current-frame Y/CoCg buffer for np instead of
srcPrevY[np]/srcPrevCoCg[np]) so nMinY/nMaxY and nMinCoCg/nMaxCoCg are computed
from the current-frame neighborhood around pxCoord; keep the final clamp/lerp
logic that writes prevY/prevCoCg -> currY/currCoCg, and optionally apply the
same neighborhood clamping to srcPrevGISpecular/currGIAOSpecular if you want
consistent treatment for GI_SPECULAR.

features/Screen Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl (1)

23-25: Consider whether kMaxBlend = 0.5 is intentionally much higher than the post-composite blend.

StereoBlendCS.hlsl uses a configurable default of 0.1, while this SSGI-specific sync uses a hardcoded 0.5 — a 5× more aggressive blend. If this is intentional (SSGI buffers carry less parallax), a brief comment would help future readers understand the rationale. Also, unlike StereoBlendCS, there's no luminance-difference gate here, so even pixels where both eyes agree will be blended.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@features/Screen` Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl:
- Around line 62-66: Guard against otherLinearDepth being zero before converting
to raw depth: if otherLinearDepth == 0, set otherRawDepth to a safe finite value
(e.g., 0) instead of performing the division so you don't produce ±INF; this
prevents feeding infinite depth into Stereo::FinalizeStereoBlend (and its
ConvertMonoUVToOtherEye path). Update the block computing otherRawDepth (which
uses SharedData::CameraData) to mirror the existing early-out on line 41 by
checking otherLinearDepth and assigning a safe fallback before calling
Stereo::FinalizeStereoBlend.

In `@src/Features/VR.cpp`:
- Line 100: stereoBlendCB is allocated with raw new in SetupResources() and
never freed, causing leaks when shaders are recompiled; replace the raw
allocation with a smart pointer (use eastl::unique_ptr or
eastl::make_unique<ConstantBuffer>(ConstantBufferDesc<StereoBlendCB>()) for
stereoBlendCB) and update the member type accordingly, or if you prefer the
existing pattern add explicit cleanup in ClearShaderCache() to Release()/delete
or reset the pointer (reset the eastl::unique_ptr or call
stereoBlendCB->Release()/delete and null it) so stereoBlendCB is properly freed
between SetupResources() calls; ensure references to ConstantBuffer,
ConstantBufferDesc<StereoBlendCB>, SetupResources(), and ClearShaderCache() are
updated to match the new ownership model.

In `@src/Features/VR.h`:
- Around line 357-372: The member stereoBlendCB is a raw ConstantBuffer* that is
allocated in SetupResources() and never deleted, causing a leak and inconsistent
ownership; change its declaration to eastl::unique_ptr<ConstantBuffer>
stereoBlendCB and update all places that assign or reset it (e.g.,
SetupResources() where new ConstantBuffer(...) is used and ClearShaderCache()
where other resources are released) to use make_unique/reset/release semantics
so the buffer is automatically freed; ensure construction matches the existing
API (use eastl::make_unique or new wrapped immediately) and that any usages of
stereoBlendCB->... compile after switching to pointer-like unique_ptr access.

---

Outside diff comments:
In `@src/Features/ScreenSpaceGI.h`:
- Line 11: The SupportsVR() override in ScreenSpaceGI (in
src/Features/ScreenSpaceGI.h) currently returns false which prevents VR-specific
features from loading; change the SupportsVR() implementation in the
ScreenSpaceGI class to return true so the framework will include this feature in
VR builds and allow the VR shaders/presets to be reachable.

---

Nitpick comments:
In `@features/Screen` Space GI/Shaders/ScreenSpaceGI/gi.cs.hlsl:
- Around line 372-401: The history clamping currently builds min/max bounds from
the previous-frame buffers srcPrevY/srcPrevCoCg (used in the loop that computes
nMinY/nMaxY and nMinCoCg/nMaxCoCg), which lags the bounds one frame; change the
neighbor sample sources inside that loop to the current-frame per-pixel GI
samples (i.e., sample the current-frame Y/CoCg buffer for np instead of
srcPrevY[np]/srcPrevCoCg[np]) so nMinY/nMaxY and nMinCoCg/nMaxCoCg are computed
from the current-frame neighborhood around pxCoord; keep the final clamp/lerp
logic that writes prevY/prevCoCg -> currY/currCoCg, and optionally apply the
same neighborhood clamping to srcPrevGISpecular/currGIAOSpecular if you want
consistent treatment for GI_SPECULAR.

In `@features/Screen` Space GI/Shaders/ScreenSpaceGI/radianceDisocc.cs.hlsl:
- Around line 152-166: The code recomputes motionVec as (prev_screen_pos -
screen_pos) even though the shader already samples the motion vector into
srcMotionVec earlier (see srcMotionVec at line ~83); update the accumulation
logic to reuse the sampled motion vector instead of recomputing it — replace
uses of motionVec with the existing sampled motion vector (srcMotionVec or
whatever local name holds the sampled motion) when computing motionLen and
motionMaxAccum and keep the rest of the accumulation logic (prevAccum handling
and accum_frames clamp) unchanged.

In `@src/Features/ScreenSpaceGI.cpp`:
- Around line 586-590: The shaderInfos list currently always includes the
stereoSyncCompute entry causing an unnecessary compile when not running VR;
modify the code that builds the shaderInfos vector (the initializer containing
&giCompute, &blurCompute, &stereoSyncCompute, &upsampleCompute in
ScreenSpaceGI.cpp) to only push or include the stereoSyncCompute entry when
REL::Module::IsVR() is true (same condition used for the dispatch guarded at
REL::Module::IsVR()), so that stereoSync.cs.hlsl (with the FRAMEBUFFER define)
is compiled only for VR builds.

[github-actions]

✅ A pre-release build is available for this PR:
Download

[coderabbitai]

🧹 Nitpick comments (3)

features/Screen Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl (1)

23-25: Hardcoded constants vs. CB-driven tuning.

kDepthSigma, kMaxBlend, and kBackCheckThreshold are static const rather than driven by a constant buffer. This means they can't be tuned at runtime (unlike the StereoBlendCS which uses CB parameters). If this is intentional for SSGI-specific fixed tuning, this is fine — but if future runtime adjustability is desired, these could be moved to the SSGI CB.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@features/Screen` Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl around
lines 23 - 25, Currently kDepthSigma, kMaxBlend, and kBackCheckThreshold are
hardcoded as static const in stereoSync.cs.hlsl which prevents runtime tuning;
change these to be driven from the SSGI constant buffer (add float fields with
the same names or clear equivalents in the SSGI CB used by StereoBlendCS),
replace the static const usages in stereoSync.cs.hlsl with references to the CB
fields, and provide the same default values in the CB so behavior is unchanged
unless overridden at runtime.

src/Features/ScreenSpaceGI.h (1)

62-69: Verify the VR default changes against user expectations.

Enabled is now unconditionally true, and NumSlices for VR jumped from 1→3. Combined with EnableGI = false for VR (AO-only), this means SSGI AO will be active by default for all VR users. The ResolutionMode = 1 (half-res) helps, but consider whether first-time VR users with lower-end hardware might experience unexpected performance hits with the feature auto-enabled.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/ScreenSpaceGI.h` around lines 62 - 69, The default settings
enable SSGI AO for VR users unexpectedly because Enabled is unconditionally
true, EnableGI is false for VR, and NumSlices jumps to 3 for VR; revert sensible
VR defaults by changing Enabled and NumSlices to preserve prior VR behavior
(e.g., set Enabled = REL::Module::IsVR() ? false : true and NumSlices =
REL::Module::IsVR() ? 1u : 4u) or add a clear config flag to opt-in VR users,
and ensure NumSteps/ResolutionMode remain conservative for VR (NumSteps =
REL::Module::IsVR() ? 6u : 8u, ResolutionMode = 1) while documenting the
decision in the ScreenSpaceGI settings comment so first-time VR users aren’t
auto-enabled with higher-cost AO.

src/Features/VR.cpp (1)

74-85: Add error logging for stereo blend shader compilation failures to match codebase patterns.

The code silently skips attachment if Util::CompileShader returns nullptr, leaving shaders uninitialized. While DrawStereoBlend() includes null-checks before use (graceful degradation), other features like TruePBR.cpp log compilation failures with logger::error(). Adding consistent logging helps diagnose missing functionality and aligns with codebase conventions.

Suggested pattern

 	if (auto rawPtr = reinterpret_cast<ID3D11ComputeShader*>(Util::CompileShader(L"Data\\Shaders\\VR\\StereoBlendCS.hlsl", defines, "cs_5_0")))
 		stereoBlendCS.attach(rawPtr);
+	else
+		logger::warn("VR: Failed to compile StereoBlendCS");

Apply similarly for DEBUG_BACKCHECK and DEBUG_BLEND_WEIGHT variants.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/VR.cpp` around lines 74 - 85, The shader compilation blocks for
stereoBlendCS, stereoBlendDebugBackCheckCS, and stereoBlendDebugBlendWeightCS
call Util::CompileShader and silently skip on nullptr; update each block to
detect a null return and call logger::error(...) with a clear message including
the shader path ("Data\\Shaders\\VR\\StereoBlendCS.hlsl") and the defines used
(e.g., the contents of defines/backCheckDefines/blendWeightDefines) so failures
are logged consistently with other modules like TruePBR.cpp while still
preserving the attach-on-success behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@features/Screen` Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl:
- Around line 23-25: Currently kDepthSigma, kMaxBlend, and kBackCheckThreshold
are hardcoded as static const in stereoSync.cs.hlsl which prevents runtime
tuning; change these to be driven from the SSGI constant buffer (add float
fields with the same names or clear equivalents in the SSGI CB used by
StereoBlendCS), replace the static const usages in stereoSync.cs.hlsl with
references to the CB fields, and provide the same default values in the CB so
behavior is unchanged unless overridden at runtime.

In `@src/Features/ScreenSpaceGI.h`:
- Around line 62-69: The default settings enable SSGI AO for VR users
unexpectedly because Enabled is unconditionally true, EnableGI is false for VR,
and NumSlices jumps to 3 for VR; revert sensible VR defaults by changing Enabled
and NumSlices to preserve prior VR behavior (e.g., set Enabled =
REL::Module::IsVR() ? false : true and NumSlices = REL::Module::IsVR() ? 1u :
4u) or add a clear config flag to opt-in VR users, and ensure
NumSteps/ResolutionMode remain conservative for VR (NumSteps =
REL::Module::IsVR() ? 6u : 8u, ResolutionMode = 1) while documenting the
decision in the ScreenSpaceGI settings comment so first-time VR users aren’t
auto-enabled with higher-cost AO.

In `@src/Features/VR.cpp`:
- Around line 74-85: The shader compilation blocks for stereoBlendCS,
stereoBlendDebugBackCheckCS, and stereoBlendDebugBlendWeightCS call
Util::CompileShader and silently skip on nullptr; update each block to detect a
null return and call logger::error(...) with a clear message including the
shader path ("Data\\Shaders\\VR\\StereoBlendCS.hlsl") and the defines used
(e.g., the contents of defines/backCheckDefines/blendWeightDefines) so failures
are logged consistently with other modules like TruePBR.cpp while still
preserving the attach-on-success behavior.