From 243ca6fe1f90974d15207a57e56042fede3c86f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E4=B8=9B=E8=B6=8A?= <congyue1977@gmail.com>
Date: Thu, 4 Jun 2026 12:01:41 +0800
Subject: [PATCH] [ReSTIR PT] Fix no-RcVertex neighbor replay causing surface
 corruption and biased UCW in temporal/spatial resampling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Problem

During temporal and spatial resampling, neighbor reservoirs whose path has no reconnection vertex (RcVertexLength > PathLength) are passed to ComputeHybridShift without being rejected. This causes two distinct bugs.

Bug 1 — Biased Unbiased Contribution Weight (UCW)

CombineReservoirs unconditionally accumulates M regardless of whether the candidate's TargetFunction is zero:
```
targetReservoir.M += SampleM;      // always runs
targetReservoir.WeightSum += RisWeight; // += 0 when TargetFunction == 0
```

For a no-RcVertex neighbor, ComputeHybridShift returns TargetFunction = 0 (the hybrid-shift context forces termination with Radiance = 0 in case 2 of RecordPathIntersection), so WeightSum is unaffected while M is inflated. The inflated M feeds into the MIS normalization denominator (PiSum += TemporalP * PrevSample.M), producing a UCW that is systematically too small — a darkening bias.

Bug 2 — Surface corruption crashing the entire MIS normalization

RandomReplay ends with:
`surfaceForResampling = ctx.GetRcPrevSurface();`

RcPrevSurface is initialized to RAB_EmptySurface() inside RTXDI_InitializePathTracerContext and is only written when RcVertexLength <= SelectedPathLength (case 1). For a no-RcVertex path (case 2), the write never happens, so surfaceForResampling — which is the caller's inout RAB_Surface surface — is overwritten with RAB_EmptySurface() (world position (0,0,0)).

After ResampleTemporalNeighbor returns, the Surface variable in RTXDI_PTTemporalResampling is now zeroed. BiasCorrection then calls:
```
float Jacobian = RTXDI_CalculateJacobian(
    RAB_GetSurfaceWorldPos(PrevSurface),   // valid
    RAB_GetSurfaceWorldPos(surface),        // (0,0,0) — corrupted
    targetReservoir.TranslatedWorldPosition,
    targetReservoir.WorldNormal
);
```

The resulting Jacobian is completely wrong, making every value derived from it (Pi, PiSum, final UCW) meaningless. Furthermore, BiasCorrection then calls ComputeHybridShift on targetReservoir using the corrupted PrevSurface context, potentially propagating the corruption further.

This is not merely a bias — it is a full data corruption that invalidates all resampling results for any pixel that encounters a no-RcVertex temporal neighbor.

Root Cause

A no-RcVertex path stores RcVertexLength = maxRcVertexLength (set by SetMaxRcVertexLengthIfUnset), which satisfies RcVertexLength > 2, triggering NeedToRunRandomReplayPathTracer = true. The hybrid-shift path tracer context correctly identifies the shift as non-invertible and zeroes the radiance, but the caller does not guard against the inout surface being clobbered by the subsequent ctx.GetRcPrevSurface() call, nor does it prevent the spurious M accumulation.

Fix

Reject no-RcVertex neighbors (RcVertexLength > PathLength) before any resampling or replay takes place, in both temporal and spatial resampling paths.

In TemporalResampling.hlsli, inside ValidateTemporalNeighbor:
```
// Reject no-Rc history paths (RcVertexLength > PathLength) before any mutations.
// These paths corrupt Surface via RAB_EmptySurface() in RandomReplay and produce
// a mismatched TargetFunction, causing biased WeightSum and BiasCorrection errors.
if (PrevSample.RcVertexLength > PrevSample.PathLength)
{
    return false;
}
```

In SpatialResampling.hlsli, inside ResampleNeighbors:
```
// Reject no-Rc neighbor paths (RcVertexLength > PathLength).
// RandomReplay produces an incorrect TargetFunction for these paths, which can
// cause single-frame fireflies. CachedResult will not be set, so BiasCorrection
// automatically skips the same neighbors via the CachedResult mask.
if (NeighborReservoir.RcVertexLength > NeighborReservoir.PathLength)
{
    continue;
}
```

The CachedResult bit for a skipped spatial neighbor is never set, so BiasCorrection naturally skips it as well via (CachedResult & (1u << i)) == 0, keeping forward and backward MIS coverage sets identical.

Impact

No-RcVertex paths are not portable across pixels under hybrid shift (the shift is by definition non-invertible for such paths), so excluding them from cross-pixel reuse is theoretically correct. The fix eliminates the darkening bias and the surface corruption without affecting the estimator's unbiasedness for valid reconnectable paths.
---
 Libraries/Rtxdi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Libraries/Rtxdi b/Libraries/Rtxdi
index d28e20f..bec8d17 160000
--- a/Libraries/Rtxdi
+++ b/Libraries/Rtxdi
@@ -1 +1 @@
-Subproject commit d28e20f11c6bdd5a1cca273cfbad5e8493e4ba05
+Subproject commit bec8d1700e329e4671f2fe3459c2ae6cbefaac9c