feat(render): steps 1-3 of the upscaler — jitter phases, 3x3 dilation, disocclusion - #870
Merged
Merged
Conversation
FSR's rule with our base: the phase count scales with the SQUARE of the render-to-display ratio, because what a low-discrepancy sequence has to cover is an area. Scaling it linearly starves the reconstruction exactly where the upscaler works hardest. JITTER_PERIOD is replaced by phase_count(render_width, display_width). Two choices are ours rather than AMD's, both one line to reverse: the base is 16 where FSR's is 8, because at 1:1 and a minimum blend rate of 1/64 eight points get averaged seven times over instead of covering more of the pixel; and there is a ceiling of 144, since a zero render width squares to millions of phases and a period that large never repeats. The resolution split itself is not built. Vbuf64Stage passes its own width as both arguments, so today's answer is 16 and the second argument is the single thing that changes when step 4 separates them.
…soccluded history Steps 2-3 of #481. Both improve the TAA that ships today, whether or not the upscaler after them lands. Dilation: nine taps at one texel, where this took five at two. At two texels the closest-depth winner can be a surface that does not touch this pixel, so a thin foreground object hands its velocity to a two-pixel skirt of background around it. Step 4 would make that worse by construction: after the resolution split one input texel is 1.5 output pixels. Disocclusion: the variance clip catches a history whose colour no longer fits. It cannot catch one whose colour fits perfectly and belongs to a surface at another distance -- a wall revealed from behind a pillar, in front of a wall of the same shade -- which then ghosts for the twenty frames the blend takes to forget it. The confidence target grows a second channel holding the reversed-Z depth the pixel was written at, and the next frame compares it at the reprojected address. Reversed-Z pays for itself: the infinite-far projection maps z = near/distance exactly, so the RATIO of two depths is the ratio of the two distances. The test is a divide -- no linearisation, no near plane in a uniform, nothing to keep in sync with the camera. The tolerance is 10 %, deliberately loose: it has to separate a foreground silhouette from its background without firing on a camera walking forward. Also fixes the confidence counter being read at the pixel's own address instead of the reprojected one, which asked how long whatever sits here NOW has been still. Tests: a_depth_jump_drops_the_history pins the mask by the 99th percentile -- 6 with it, 44 without, against 612 for the two viewpoints -- because a mean over the whole frame passes either way. a_slow_pan_keeps_accumulating pins the other side, where a tolerance too tight returns the raw frame and still bills for both passes. The binary is serialised behind a mutex: four cases against the one shared device segfault radv intermittently, and pass under --test-threads=1.
The plan said the resolve used a neighbourhood clamp. It has clipped against a YCoCg mean +/- sigma AABB since it was written; the step was described from memory rather than from the shader. What was missing was the disocclusion mask.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Steps 1, 2 and 3 of #481, in the order the user picked (option C). One PR rather than three, per the standing rule against micro-splits — and because two PRs open at once bump to the same version (#869).
feat:is a MINOR under the repo's rule, and 0.2.48 + minor is 0.3.0.Step 1 — the jitter period is a function of the ratio
JITTER_PERIOD = 8is gone. In its placejitter::phase_count(render_width, display_width): FSR's rule with our base, where the count scales with the square of the ratio, because what the sequence covers is an area. At 1.5× every output pixel is fed by 1/2.25 of an input one, so the same sub-pixel density takes 2.25× the samples — 36, not 24. Scaling it linearly is the plausible-looking mistake and it reads as "FSR is soft".Two choices are ours rather than AMD's, both one line to reverse:
JITTER_BASE_PHASESback to 8 if the transliterated passes disagree — their accumulation constants are tuned against AMD's count, and that count is one of the things in the port that cannot be guessed.JITTER_MAX_PHASES= 144 (3×, FSR's Ultra Performance). Not tidying: a render width of zero — a minimised window, a target queried a frame early — squares to tens of millions of phases, and a period that large is a sequence that never repeats.The resolution split is deliberately not built:
Vbuf64Stage::jitter_phasespasses its own width as both arguments, so the second argument is the single thing step 4 changes and there is no dead field to desynchronise meanwhile.Step 2 — the dilation narrowed, from 5 taps at 2 texels to 9 at 1
Karis' wide cross is tuned for a resolve at display resolution. At two texels the closest-depth winner can be a surface that does not touch this pixel at all, so a thin foreground object hands its velocity to a two-pixel skirt of background around it — the object drags a halo. Step 4 makes that worse by construction: after the resolution split one input texel is 1.5 output pixels, so a two-texel reach is three.
Step 3 — the disocclusion mask, and a premise in the plan that was wrong
🔴 The plan said "today's resolve uses a neighbourhood clamp, which is the crude version of this". It does not, and never did. It has clipped against a YCoCg mean ± σ AABB since it was written — Playdead's, clip and not clamp, at one sigma, with the measurement behind that sigma in
taa.wgsl's header. The step was written from memory rather than from the shader. #481's body is corrected.What was genuinely missing is the mask. The clip catches a history whose colour no longer fits. It cannot catch one whose colour fits perfectly and belongs to a surface at a different distance — a wall revealed from behind a pillar, in front of a wall of the same shade — which then ghosts for the twenty frames the blend takes to forget it.
🎯 Reversed-Z pays for itself. The infinite-far projection maps
z = near / distanceexactly, so the ratio of two depths is the ratio of the two distances. The whole test is a divide: no linearisation, no near plane in a uniform, nothing to keep in sync with the camera. The depth rides in the confidence target's second channel —R16Float→Rg16Float, two bytes a pixel, no new attachment. Half precision is right here because the test is a ratio, so it needs relative accuracy: fp16 carries ~0.05 % against a 10 % tolerance.🔴 This is the cheap test, not FSR's. FSR reconstructs the previous depth into the current grid with an extra atomic pass and is exact under any camera motion. Ours compares against this pixel's own depth, so axial camera motion is a small error everywhere at once — which is what the loose tolerance absorbs, and the thing to revisit if it misbehaves on the device.
Fixed in passing: the confidence counter was read at the pixel's own address instead of the reprojected one, which asks how long whatever sits here now has been still — a different surface every frame under a turning camera.
Tests
a_depth_jump_drops_the_historypins the mask, and its threshold was measured on both sides rather than chosen: 99th-percentile difference is 6 with the mask and 44 without, against 612 for the two viewpoints. Verified by stubbingis_disoccludedout — the test fails, with that message.🔴 Its first version used a mean and passed either way (0.33 vs 0.75, against a threshold of 17). A disocclusion touches the pixels a silhouette uncovers, and averaged over the frame that is a rounding error next to the lighting. Same failure recorded in the memory of this project, made again.
a_slow_pan_keeps_accumulatingpins the other side: consecutive resolved frames must be closer together than consecutive unresolved ones. It fails if the tolerance is ever tightened enough to reject on ordinary motion.🔴 And a harness trap:
tests/commonhands every case in a binary the same device, so four cases at once segfault radv intermittently — and pass reliably under--test-threads=1, the worst way to find out.temporal_motionis now serialised behind a mutex, the patterngpu_scopes.rsalready used. Three consecutive parallel runs green.cargo test -p kooch_renderfully green (340 lib + every integration binary). Workspace builds.Not measured on the device
Steps 2–3 are the ones whose payoff is visual, and the handheld run has not happened. No frame-time change is expected from step 1; steps 2–3 trade four extra depth taps for two extra bytes a pixel of traffic.
Docs ship with it:
docs/ROADMAP.mdhas all three marked built, with every reversible choice and the corrected premise written down.Refs #481