Repository navigation
Conversation
|
With ReSTIR the way Q2RTX implements it (pairwise neighbour sharing, 8-byte reservoirs, A-SVGF), this is as close to correct as the method gets. What this PR changes is that the remaining error is no longer hidden by a second error. The sample accounting is now honest. Decision needed: default value of
|
| return vec; | ||
| uvec4 vec; | ||
| r.W = r.y == RESTIR_INVALID_ID ? 0.0 : r.W; | ||
| // Clamp to fp16 max: values above 65504 would become Inf in |
There was a problem hiding this comment.
Hmm, perhaps check that clamping W here doesn't possibly introduce inaccuracies elsewhere,
as it's used to reconstruct weights...
Or maybe it doesn't matter that much, just storing as a float16 already introduces inaccuracies.
There was a problem hiding this comment.
...then again, clamping is probably preferable to inf/getting zeroed.
There was a problem hiding this comment.
Yes, you've got a point. I saw dark vertical stripes in a dark spot of base2 with pt_restir 3 and pt_restir_spatial 0. After the clamp, this was gone. So, my conclusion was that the clamp is still better.
| // v2 packing of the second u32: | ||
| // bits 0.. 7 : y_pos.x, quantized to [0, 1] in 256 steps | ||
| // bits 8..15 : y_pos.y, quantized to [0, 1] in 256 steps | ||
| // bit 16 : vis_verified flag (groundwork for shadow reuse) |
There was a problem hiding this comment.
Did you consider just using the sign bit of W to store vis_verified?
After all, weights should be positive, and it's a bit that's otherwise unused...
There was a problem hiding this comment.
There's no bit deficit to solve. The packed v2 layout already stores vis_verified in bit 16 of vec.y, and M above 32767 is unreachable in practice. The only gain would be debuggability loss (NaN sign pitfall, harder raw-bit inspection).
| // bits 17..31 : real M (the actual sample count this reservoir represents) | ||
| uint pos_qx = uint(clamp(r.y_pos.x, 0.0, 1.0) * 255.0 + 0.5); | ||
| uint pos_qy = uint(clamp(r.y_pos.y, 0.0, 1.0) * 255.0 + 0.5); | ||
| uint m = (r.y == RESTIR_INVALID_ID) ? 0u : min(r.M, 32767u); |
There was a problem hiding this comment.
Did you find a tangible difference with M being stored?
I tried an explicit M as well, but in the end, it didn't seem to actually make a difference...
There was a problem hiding this comment.
Also, IIRC the weights were some point normalized to the M cap value, and that would be used on unpacking, hence it didn't have to be stored.
There was a problem hiding this comment.
In average scenes there's no visible difference. But the difference concentrates exactly at shadow edges (~3× honest-vs-fake M ratio), which is the bridge-shadow behaviour I mentioned above, and likely also explains your own puzzle *. Real M is also what makes the cheap-mode fix possible.
*) direct_lightning.rgen:
* (Unfortunately, I haven't quite figured out why the light weight W seems to end up too
* high after spatial sampling, so it's quite possible there's a less hacky way to deal
* with that.)
| // Reservoirs with W == 0 (the light turned out to be shadowed there last | ||
| // frame) carry no weight; merging them would still add their M to the | ||
| // denominator of W = w_sum / (p_hat * M) and dim the new estimate. | ||
| if(prev_r.y != RESTIR_INVALID_ID && prev_r.W > 0.0 && prev_r.vis_verified) |
There was a problem hiding this comment.
Be careful. Throwing away samples may mess up the maths and introduce bias.
A sample being in shadow is relevant, after all... after all, it may really just be a rather dim spot with a lot of samples in the shadow.
There was a problem hiding this comment.
Do any of the cited references perhaps say something about this?
There was a problem hiding this comment.
Reservoirs store unshadowed lighting; shadows are applied once, at the end, by the pixel's own shadow ray. A neighbour's W=0 means "that neighbour is occluded". Merging it in would smear someone else's shadow across this pixel. The zero-weight merge only adds phantom M, which is exactly the documented "everything gets darker" bias. So, the skip is correct: it removes fake credit, not real information.
About the references:
https://benedikt-bitterli.me/restir/bitterli20restir.pdf
§4.1 — the reweighting factor p̂q(r.y) · r.W · r.M and the role of W as an estimate of 1/p(y), with the key sentence: "In order for Eq. (6) to be unbiased, the expected value of W(x,z) should be equal to 1/p(y)... this is not always the case when combining samples from neighboring pixels, which is the source of bias."
§4.2 "Biased RIS" — the formal derivation of that bias for multi-distribution RIS (each candidate from a different source PDF, exactly the cross-pixel case).
https://intro-to-restir.cwyman.org/presentations/2023ReSTIR_Course_Notes.pdf
§2.4 "Unbiased Contribution Weights" and §3.2 "MIS weights": the contribution-weighting machinery that's supposed to keep merges honest when M differs;
§4.2 "Spatiotemporal reuse": where the p̂q · W · M merge weight itself is defined;
§7.1 "Sampler optimization": the weight family in practice: §7.1.1 neighbor rejection, §7.1.2 Contribution MIS weights, §7.1.3 Pairwise MIS weights, §7.1.4 Biased MIS weights.
Also note: I couldn't have done this without the guidance of AI. I am not a mathematician. So, I needed the info in laymen terms. I'm learning about ReSTIR and used what I learned to make this PR.
2 participants
I have been working on improving the ReSTIR implementation. Especially on the cheap modes (pt_restir 2 and 3) where shadows were not stable.
Files adapted:
I also added fixes for compile warnings.
Demo test
master branch with pt_restir 1
restir_update_2 branch with pt_restir 1
Check the shadow of the bridge on the wall left.
About the shadow difference in the second screenshot
The shorter bridge shadow in the second picture is not a rendering error in this branch — it's an old quirk in the light estimation that this branch stops masking.
The shadow estimation in the restir-update base has two long-standing quirks that partly cancel each other out:
A converged reference image (many accumulated frames, denoiser off) shows the true shadow — and it looks like the first picture. Removing quirk 1 is correct, but it also stops compensating quirk 2, so the bridge shadow now ends a bit earlier.
If you want the reference look, setting pt_restir_spatial 0 disables the neighbor reuse and makes the shadow look like the first picture again. A principled fix for quirk 2 (properly weighting reused samples) is possible but is a larger change, planned as future work.
I'll be addressing this later.