feat(VR): add edge detection for stereo blending

[alandtse]

Summary by CodeRabbit

• New Features

  • Added "Edge Detection" debug visualization mode for stereo blending.
  • Edge-aware stereo handling: skip synchronization/blending near depth discontinuities to reduce incorrect cross-eye artifacts.

• Tests

  • Added extensive unit tests for depth-difference calculations and stereo clamping/UV behavior.

• Documentation

  • Updated stereo debug tooltips to describe new edge-detection and refined back-check/blend visualizations.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds edge-aware stereo reprojection and blending: cross-pattern neighbor depth sampling, MaxDepthDiff in shared VR utilities, conditional Passthrough to skip blending near depth discontinuities, destination-edge checks, expanded debug modes and an edge-detection debug shader, plus unit tests for the new utilities.

Changes

Cohort / File(s)	Summary
Stereo Sync Shaders features/Screen Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl, features/Screen-Space Shadows/Shaders/ScreenSpaceShadows/StereoSyncCS.hlsl	Added SampleCrossDepths (cross-pattern neighbor sampling) and Passthrough flow; replaced direct neighbor reads with cross-depth sampling; moved edge/back-check logic to use shared MaxDepthDiff; added depth-margin edge checks and early passthroughs.
Shared VR Utilities package/Shaders/Common/VR.hlsli	Added Stereo::MaxDepthDiff, Stereo::ClampToEyeUV, and Stereo::ClampToEyeBounds to centralize depth-difference computation and eye-local clamping for packed stereo buffers.
Stereo Blend Shaders & Debug package/Shaders/VR/StereoBlendCS.hlsl, src/Features/VR.cpp, src/Features/VR.h, src/Features/VR/StereoBlend.cpp, src/Features/VR/SettingsUI.cpp	Gated reprojection/blending with source and destination edge checks, added kEdgeMargin/thresholds, new debug state values and DEBUG_EDGE_DETECTION path, wired a new compute shader (stereoBlendDebugEdgeDetectionCS) and UI debug mode (Edge Detection).
Shader Tests package/Shaders/Tests/TestVR.hlsl	Added extensive unit tests for Stereo::MaxDepthDiff, ClampToEyeBounds, and ClampToEyeUV including boundary clamping cases. Note: test blocks for MaxDepthDiff are duplicated within the file.

Sequence Diagram

sequenceDiagram
    participant Src as SourceEye (depth buffer)
    participant SrcEdge as SourceEdgeCheck
    participant Reproj as ReprojectionLogic
    participant Dst as DestinationEye (other-eye depth)
    participant DstEdge as DestinationEdgeCheck
    participant Blend as StereoBlend/Output

    Src->>SrcEdge: Sample center + cross neighbors (SampleCrossDepths)
    alt source is edge (MaxDepthDiff > threshold)
        SrcEdge->>Blend: Passthrough (skip reproj/blend)
    else source passes
        SrcEdge->>Reproj: Reproject sample to other eye
        alt reprojection invalid / OOB
            Reproj->>Blend: Mark out-of-bounds (debugState)
        else reprojection valid
            Reproj->>Dst: Sample other-eye neighbors (SampleCrossDepths with kEdgeMargin)
            Dst->>DstEdge: Check destination depth discontinuity (MaxDepthDiff)
            alt destination is edge
                DstEdge->>Blend: Passthrough (skip blend)
            else destination passes
                DstEdge->>Blend: Perform stereo blend, back-check, color gate
            end
        end
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• fix(VR): screen space shadows desync #1946 — Modifies stereo sync/shader pipeline and adds cross-depth sampling and edge-aware stereo sync code paths.
• fix(VR): SSGI discrepancies #1926 — Changes stereo reprojection depth-discontinuity logic and shared helpers like SampleCrossDepths/MaxDepthDiff.
• fix(VR): disable hmd menu for incompatible openvr #1309 — Related VR shader resource setup and compute shader initialization patterns (shader wiring overlaps).

Suggested reviewers

• doodlum
• jiayev
• davo0411

Poem

🐇✨ I hop along pixels, cross-sampling near seams,
I sniff for depth edges and skip risky reams,
When silhouettes glimmer I whisper “Passthrough,”
Else blend with care, a careful rabbit view,
Hooray for clearer stereo dreams! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately summarizes the main change: adding edge detection functionality for stereo blending in VR, which is reflected across multiple shader files and supporting infrastructure.

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

✨ Finishing Touches

🧪 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]

No actionable suggestions for changed features.

[coderabbitai]

🧹 Nitpick comments (3)

package/Shaders/VR/StereoBlendCS.hlsl (1)

34-42: Clamp cross-neighbor reads to eye-local bounds to avoid seam contamination.

At Line 35, neighbor reads can cross the packed-eye seam when center.x is within offset pixels of the half-width boundary, which can over-mark debugState 1/2 near nasal edges.

Proposed refactor

+int2 ClampToEyeBounds(int2 px, uint eyeIndex)
+{
+	int halfWidth = (int)FrameDim.x / 2;
+	int minX = (eyeIndex == 0) ? 0 : halfWidth;
+	int maxX = (eyeIndex == 0) ? (halfWidth - 1) : ((int)FrameDim.x - 1);
+	px.x = clamp(px.x, minX, maxX);
+	px.y = clamp(px.y, 0, (int)FrameDim.y - 1);
+	return px;
+}
+
-float4 SampleCrossDepths(int2 center, int offset)
+float4 SampleCrossDepths(int2 center, int offset, uint eyeIndex)
 {
+	int2 px0 = ClampToEyeBounds(center + int2(offset, 0), eyeIndex);
+	int2 px1 = ClampToEyeBounds(center + int2(-offset, 0), eyeIndex);
+	int2 px2 = ClampToEyeBounds(center + int2(0, offset), eyeIndex);
+	int2 px3 = ClampToEyeBounds(center + int2(0, -offset), eyeIndex);
 	return float4(
-		DepthTexture[center + int2(offset, 0)],
-		DepthTexture[center + int2(-offset, 0)],
-		DepthTexture[center + int2(0, offset)],
-		DepthTexture[center + int2(0, -offset)]);
+		DepthTexture[px0],
+		DepthTexture[px1],
+		DepthTexture[px2],
+		DepthTexture[px3]);
 }

Also applies to: 72-85

🤖 Prompt for AI Agents

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

In `@package/Shaders/VR/StereoBlendCS.hlsl` around lines 34 - 42,
SampleCrossDepths (and the analogous four-neighbor sampler later in the file)
can read across the packed-eye seam; fix by clamping neighbor reads to the
eye-local X bounds before indexing. Determine eye region from center.x (if
center.x < halfWidth use eyeMinX=0 and eyeMaxX=halfWidth-1 else
eyeMinX=halfWidth and eyeMaxX=fullWidth-1), then compute neighborX =
clamp(center.x + dx, eyeMinX, eyeMaxX) and use int2(neighborX, center.y) for
each sample instead of raw center + int2(...); apply the same clamp logic to the
other cross-sampling function at 72-85.

package/Shaders/Tests/TestVR.hlsl (2)

160-169: Tighten TestMaxDepthDiffArmWorldCase to an exact expectation.

The current > 0.05 / < 0.5 bounds are broad and can hide regressions. Assert the exact expected diff (|worldDepth - armDepth|) instead.

Proposed test tightening

 	float armDepth = 0.75;
 	float worldDepth = 1.0;
 	float result = Stereo::MaxDepthDiff(armDepth, float4(worldDepth, armDepth, armDepth, armDepth));
-	ASSERT(IsTrue, result > 0.05);  // exceeds kEdgeDepthThreshold used in SSS
-	ASSERT(IsTrue, result < 0.5);   // not a false positive at some larger threshold
+	float expected = abs(worldDepth - armDepth);
+	ASSERT(IsTrue, abs(result - expected) < kEps);

🤖 Prompt for AI Agents

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

In `@package/Shaders/Tests/TestVR.hlsl` around lines 160 - 169, The test
TestMaxDepthDiffArmWorldCase uses broad bounds; replace the two range asserts
with a single exact expectation that Stereo::MaxDepthDiff returns the absolute
difference between worldDepth and armDepth (0.25). Update the assertions in
TestMaxDepthDiffArmWorldCase to assert equality against abs(worldDepth -
armDepth) (or the literal 0.25) using a small floating-point tolerance (epsilon)
instead of >/< checks so the test detects regressions precisely.

---

1-1: Consider adding an issue reference in the PR description.

Since this is a feature PR, adding Implements #<id> or Addresses #<id> improves traceability.

As per coding guidelines: "Issue References (if PR fixes bugs or implements features): Suggest adding appropriate GitHub keywords."

🤖 Prompt for AI Agents

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

In `@package/Shaders/Tests/TestVR.hlsl` at line 1, This PR adds a new feature to
package/Shaders/Tests/TestVR.hlsl but lacks an issue reference in the PR
description; update the PR description to include a GitHub issue keyword such as
"Implements #<id>" or "Addresses #<id>" (replace <id> with the relevant issue
number) at the top of the PR description to satisfy the coding guideline on
Issue References and improve traceability for the TestVR.hlsl changes.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@package/Shaders/Tests/TestVR.hlsl`:
- Around line 160-169: The test TestMaxDepthDiffArmWorldCase uses broad bounds;
replace the two range asserts with a single exact expectation that
Stereo::MaxDepthDiff returns the absolute difference between worldDepth and
armDepth (0.25). Update the assertions in TestMaxDepthDiffArmWorldCase to assert
equality against abs(worldDepth - armDepth) (or the literal 0.25) using a small
floating-point tolerance (epsilon) instead of >/< checks so the test detects
regressions precisely.
- Line 1: This PR adds a new feature to package/Shaders/Tests/TestVR.hlsl but
lacks an issue reference in the PR description; update the PR description to
include a GitHub issue keyword such as "Implements #<id>" or "Addresses #<id>"
(replace <id> with the relevant issue number) at the top of the PR description
to satisfy the coding guideline on Issue References and improve traceability for
the TestVR.hlsl changes.

In `@package/Shaders/VR/StereoBlendCS.hlsl`:
- Around line 34-42: SampleCrossDepths (and the analogous four-neighbor sampler
later in the file) can read across the packed-eye seam; fix by clamping neighbor
reads to the eye-local X bounds before indexing. Determine eye region from
center.x (if center.x < halfWidth use eyeMinX=0 and eyeMaxX=halfWidth-1 else
eyeMinX=halfWidth and eyeMaxX=fullWidth-1), then compute neighborX =
clamp(center.x + dx, eyeMinX, eyeMaxX) and use int2(neighborX, center.y) for
each sample instead of raw center + int2(...); apply the same clamp logic to the
other cross-sampling function at 72-85.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 249b0e6 and 1cd16cf.

📒 Files selected for processing (9)

• features/Screen Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl
• features/Screen-Space Shadows/Shaders/ScreenSpaceShadows/StereoSyncCS.hlsl
• package/Shaders/Common/VR.hlsli
• package/Shaders/Tests/TestVR.hlsl
• package/Shaders/VR/StereoBlendCS.hlsl
• src/Features/VR.cpp
• src/Features/VR.h
• src/Features/VR/SettingsUI.cpp
• src/Features/VR/StereoBlend.cpp

[github-actions]

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

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

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

113-115: Back-check disabled via parameter - document the trade-off.

Passing 0.0 for backCheckThreshold disables the back-check validation. The comment explains the rationale (edge detection covers the cases), but consider whether this should be a tunable constant like the other parameters for future adjustment.

🤖 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 113 - 115, The call to Stereo::FinalizeStereoBlend currently hardcodes
backCheckThreshold as 0.0 (disabling back-check); make this a named tunable
constant or shader keyword (similar to kDepthSigma and kMaxBlend) and replace
the literal 0.0 with that symbol, update the comment to document the trade-off
(why back-check is disabled by default and when to enable it), and ensure the
new symbol is declared/initialized near the other stereo parameters so it can be
adjusted later without changing the call site.

package/Shaders/Common/VR.hlsli (1)

186-190: Boundary at x=0.5 is included in both eyes' valid ranges.

For the left eye, ClampToEyeUV clamps x to [0.0, 0.5], while GetEyeIndexFromTexCoord returns 1 (right eye) for x >= 0.5. This means a left-eye sample clamped to exactly 0.5 would technically be at the right eye's boundary.

In practice this is unlikely to cause issues since the seam represents the same physical edge, but if strict eye separation is needed, consider using 0.5 - epsilon for the left eye's upper bound.

🤖 Prompt for AI Agents

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

In `@package/Shaders/Common/VR.hlsli` around lines 186 - 190, ClampToEyeUV
currently allows x==0.5 for the left eye which overlaps GetEyeIndexFromTexCoord
behavior; update ClampToEyeUV to use a strict upper bound for the left eye
(e.g., 0.5 - epsilon) so boundaries are non-overlapping: adjust the clamp
expression in ClampToEyeUV to use a small epsilon (e.g., 1e-6) for the left-eye
upper bound while keeping the right-eye lower bound at 0.5 (or similarly
subtract epsilon for the comparison side), referencing ClampToEyeUV and
GetEyeIndexFromTexCoord when making the change.

package/Shaders/Tests/TestVR.hlsl (1)

282-291: Consider adding edge case test for Y clamping in ClampToEyeBounds.

TestClampToEyeUVYUnchanged correctly verifies that Y is not modified by ClampToEyeUV. However, there's no corresponding test verifying that ClampToEyeBounds does clamp Y when out-of-bounds in both directions (the existing TestClampToEyeBoundsY only tests one direction per assertion).

📝 Suggested additional assertions for TestClampToEyeBoundsY

// Verify right eye Y clamping works the same as left eye
int2 topRight = Stereo::ClampToEyeBounds(int2(1536, -1), 1, frameDim);
ASSERT(AreEqual, topRight.y, 0);

int2 bottomRight = Stereo::ClampToEyeBounds(int2(1536, 1025), 1, frameDim);
ASSERT(AreEqual, bottomRight.y, 1023);

🤖 Prompt for AI Agents

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

In `@package/Shaders/Tests/TestVR.hlsl` around lines 282 - 291, Add assertions to
TestClampToEyeBoundsY to verify Y clamping for the right eye as well as the
left: call Stereo::ClampToEyeBounds with int2(1536, -1) and eye index 1,
asserting the returned .y equals 0, and call Stereo::ClampToEyeBounds with
int2(1536, 1025) and eye index 1, asserting the returned .y equals frameDim.y -
1 (e.g. 1023); place these alongside the existing left-eye checks inside
TestClampToEyeBoundsY so both directions are validated for both eyes.

🤖 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 42-50: In SampleCrossDepths the center UV is clamped once before
applying offsets, which can push neighbor samples into the other-eye region;
instead compute each neighbor UV by adding its offset to centerUV and then call
Stereo::ClampToEyeUV on that summed UV (e.g., for each of the four offsets)
before multiplying by texScale and calling srcDepth.SampleLevel; update the four
SampleLevel calls to clamp per-neighbor UVs while keeping RES_MIP and
samplerPointClamp unchanged.

In `@package/Shaders/VR/StereoBlendCS.hlsl`:
- Around line 88-101: Stereo::FinalizeStereoBlend is being invoked without the
backCheckThreshold override (using the default 8.0) which makes its back-check
behavior inconsistent with stereoSync.cs.hlsl (which passes 0.0); either call
Stereo::FinalizeStereoBlend(r, uv, centerDepth, otherDepth, eyeIndex, FrameDim,
DepthSigma, MaxBlendFactor, 0.0) to explicitly disable back-check for this pass,
or if back-check is intentionally desired here, add a concise comment next to
the Stereo::FinalizeStereoBlend call explaining why this pass must use the
default/stronger back-check compared to SSGI.

---

Nitpick comments:
In `@features/Screen` Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl:
- Around line 113-115: The call to Stereo::FinalizeStereoBlend currently
hardcodes backCheckThreshold as 0.0 (disabling back-check); make this a named
tunable constant or shader keyword (similar to kDepthSigma and kMaxBlend) and
replace the literal 0.0 with that symbol, update the comment to document the
trade-off (why back-check is disabled by default and when to enable it), and
ensure the new symbol is declared/initialized near the other stereo parameters
so it can be adjusted later without changing the call site.

In `@package/Shaders/Common/VR.hlsli`:
- Around line 186-190: ClampToEyeUV currently allows x==0.5 for the left eye
which overlaps GetEyeIndexFromTexCoord behavior; update ClampToEyeUV to use a
strict upper bound for the left eye (e.g., 0.5 - epsilon) so boundaries are
non-overlapping: adjust the clamp expression in ClampToEyeUV to use a small
epsilon (e.g., 1e-6) for the left-eye upper bound while keeping the right-eye
lower bound at 0.5 (or similarly subtract epsilon for the comparison side),
referencing ClampToEyeUV and GetEyeIndexFromTexCoord when making the change.

In `@package/Shaders/Tests/TestVR.hlsl`:
- Around line 282-291: Add assertions to TestClampToEyeBoundsY to verify Y
clamping for the right eye as well as the left: call Stereo::ClampToEyeBounds
with int2(1536, -1) and eye index 1, asserting the returned .y equals 0, and
call Stereo::ClampToEyeBounds with int2(1536, 1025) and eye index 1, asserting
the returned .y equals frameDim.y - 1 (e.g. 1023); place these alongside the
existing left-eye checks inside TestClampToEyeBoundsY so both directions are
validated for both eyes.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1cd16cf and 4d2f254.

📒 Files selected for processing (5)

• features/Screen Space GI/Shaders/ScreenSpaceGI/stereoSync.cs.hlsl
• features/Screen-Space Shadows/Shaders/ScreenSpaceShadows/StereoSyncCS.hlsl
• package/Shaders/Common/VR.hlsli
• package/Shaders/Tests/TestVR.hlsl
• package/Shaders/VR/StereoBlendCS.hlsl