Skip to content

Shadows: the sun's floor pages are asked only over the scene box (#763) - #779

Merged
pasquelin merged 18 commits into
developfrom
525-shadow-pixel-cost
Sep 26, 2026
Merged

pasquelin merged 18 commits into
developfrom
525-shadow-pixel-cost

Conversation

@pasquelin

Copy link
Copy Markdown
Owner

Closes #763

What changed

Why a page was evicted, then asked for again, while the frame's request fit the pool

The measurer's run (sponza 3456×2234, --camera-mobile) gave about 1 670 pages requested a frame, a pool full at 4 096 of 4 096, and about 130 refetches a frame. The pool's eviction order is sound:

  • pool.take evicts only a page that no report since the latest one named;
  • requests.ask raises requested for every page a report names, drawn or not.

What held the pool was the sun's floor. Every frame, requests.floors() asks, as if the latest report had named them, for every last-level page within the view's far distance (sunLevels.ts floorReach), whether or not anything lies under it:

  • On the bench's sponza, far is radius · 20 ≈ 356 m, and the last level has 16 m pages. That makes about 2 100 floor pages, over 30 × 14 m of scene and hundreds of metres of empty ground.
  • Those pages are never evictable, because the latest report named them. So they pinned half the pool.
  • The frame's 1 670 pages plus 2 100 floors left about 300 pages of cache. Each page the dollying camera came back to had been evicted meanwhile.
  • The ~2 100 floor pages were also all drawn in the first frame of the view.

The fix: floorReach is bounded by the scene box's rectangle on the light plane, one page around it for the normal offset and the PCF (sunLevels.ts).

  • That rectangle comes from sunBoxRect (math.ts), now shared with the staling boxes of invalidate.ts, which had their own copy.
  • No receiver lies past the box, so a floor page there held nothing anyone read.
  • A reader past the box, a sprite for example, still has its floor mapped under every page a report names (askFloor, unchanged).

A scratch model (not committed) ran the real plan and pool under the bench's trajectory: sponza's box, receivers on its floor, walls and two galleries, 3456 × 2234, and a two-frame report lag. Results:

  • Floors held: 2 162 before, 20 after.
  • Refetches: 353 counted and 156 real before; 0 after.
  • Pages drawn over 120 frames: 9 003 before, 6 705 after.

The pool's size is not the cause. The model's requests stay under 1 500 while the pool holds 4 096. So the pool is not resized and no MB is added.

What a "refetch" counts

pool.refetched is cumulative. It counts a table entry mapped again after the pool evicted what it held, at any later frame. A sun entry is a ring slot of its level: when a clipmap window pans, a different absolute page takes the slot. So the counter also counts a new page landing on a slot whose earlier page was evicted. In the model, before the fix, it read 353 where 156 pages really came back. It does not count pages released because they left the window.

The CTO's three points

  1. A page the latest report named is never evicted, and eviction takes the least recently requested first. That is already pool.take.
  2. What held the unrequested pages is the floor pre-ask above.
  3. Level hysteresis is not added. After the fix, the model refetches nothing under the bench's dolly, so no level flip is left to damp. If the measurer's run shows otherwise, it is the next round.

Loops A and B, reverted

The measurer's run on the previous round showed they did not change the stage, and they are reverted (measured first):

  • Shadows stage 13.2 / 21.1 ms against develop's 13.5 / 21.6.
  • Pages drawn per frame unchanged: mean 262, p95 1 009–1 248.
  • drive-a-car unchanged, at about 150 pages a frame.

What each did:

  • Loop A listed each light-cut request once a frame. It would lower LIST_FULL redraws, which the counters did not show.
  • Loop B held the view limit while the camera moves. It would lower light-cut-redraw pages.

Proof

  • New test by behaviour, on the real plan and pool: packages/sdk-core/src/scene/light-shadow/dollyRefetch.test.ts, a dolly over a small scene whose reads fit the pool evicts nothing it reads again.
    • A camera rocks over a 20 m scene under a 120 m far distance and reads 144 fine pages a frame from a 1 024-page pool.
    • On develop: 880 floor pages held and 1 864 refetches.
    • Here: 64 floor pages, every page over the box held, and 0 refetches.
    • It fails on develop's sunLevels.ts; I checked by putting it back.
  • sunLevels.test.ts: an empty box, a plane unbounded along x, and a box unbounded along the sun's axis leave the floor the view's whole reach (reviewer).
  • visibility/shader/spriteShadowCut.test.ts, the shadow scene box follows a pose the engine moved, with no table change (reviewer): fails on develop's sceneBox.ts.
  • The existing light-shadow tests pass unchanged (77), including invalidate.ts' staling through the shared rectangle.
  • Gates after the review and merging origin/develop (368d8e723):
    • check:changed and test:changed: 2 609 pass, 0 fail;
    • validate --group quick and --group typescript: OK;
    • check-pr-size.ts: 180 hand-written lines.

Local review before push

  • Simplification pass: the reviewer's simplify skill, 4 agents (reuse, simplification, efficiency, altitude), on the whole diff against develop.
    • Fixed: the floor's rectangle and the depth range now share one finiteness test on both depth ends, so an empty or unbounded box skips the projection; the rectangle's array is boxRect, not extent, which the clipmap already names; the floorReach doc says why no page is needed past the box (no caster there: a receiver there is lit).
    • Left, for the lead: the one-page margin is about sixteen times the normal offset plus the PCF reach (safe, not derived); a scene wider than the far distance is still unbounded (the coder's altitude note).
    • Skipped: fusing the depth loop with sunBoxRect's corner loop (about 100 ns a sun a frame); a default offset for sunBoxRect; the fixture's planFrame box.
  • Correctness review: the reviewer's code-review --fix skill, 6 findings.
    • Fixed, image: the scene box was rebuilt only on a table epoch or root-list change, but an engine pose or placement move (physics, drive-a-car) bumps the scene revision alone, so an object leaving a small scene lost its pre-asked floor for the report lag. The box now follows revisions.scene too (sceneBox.ts, encodeShadows.ts), with a test that fails on develop.
    • Fixed: a box unbounded along the sun's axis on one side gave a finite near end and a NaN rectangle, which Int32Array stored as 0; the guard now tests both ends, with a test.
    • Fixed: the docs of shadowFactorWgsl.ts, requests.ts, sceneBox.ts and lights.ts say the floor spans the box, not the view's reach.
    • Checked: sunPageMeets is exactly develop's (rects[2], rects[3] are develop's -rects[3], -rects[2]), so invalidate.ts stales the same pages; loops A and B are fully reverted (no gpu/dag file differs from develop); a receiver at the box's edge is covered, the normal offset plus the PCF reach being a few texels of a 128-texel page; transparents are selection roots, inside the box; the engine has no skinning.
    • Left: past the box the far-shadow ray answers "lit" only if the proxy does not reach outside the box; hidden or parked roots stay in the box, which only widens it. For the measurer: with the box following engine moves, a moving caster can now shift the sun's depth range, which redraws the sun; drive-a-car's pages a frame say whether it matters.
  • Auditor list (reviewer): the diff does what the issue's step asks and names its measurement; each changed behaviour has a test that fails on develop (dollyRefetch.test.ts, the scene-box test); no image loss found; §Streaming, memory and shadows: no pool or MB change, the floor bound follows rule 6 (bounded by the scene, not the view's reach). Not delivered by this branch: "Shadows within 2 ms of GPU", which only the measurer's run proves; Closes #525 stands on that run.
  • Before the review, the coder's own passes:
    • The coder's simplification pass: the real simplify skill, 4 agents (reuse, simplification, efficiency, altitude).
      • Fixed, reuse: the box-to-light-plane rectangle existed in invalidate.ts (sunRect, rows up the axis). There is now one sunBoxRect in math.ts, rows down the axis, used by both, and sunPageMeets is rewritten on it.
      • Fixed, simplification: one clamp in metres, then one floor; no scratch corner; the floor count moved after the test's loop.
      • Efficiency: nothing to change; no per-frame allocation.
      • Altitude, left for the lead: on a scene wider than the view's far distance (a terrain, a city), the pre-asked floor can still fill the pool. The general change is to ask those pages at an evictable stamp, or under a fixed page budget. That changes the floor's pre-ask guarantee, so it is outside this pull request.
      • Skipped: a box parameter on the fixture's planFrame, which would save 3 lines.
    • The coder's correctness review: the real code-review --fix skill, no correctness bug.
      • The clamp is exact, and the sunPageMeets sign is equivalent to the old code.
      • Fixed: the test now also checks that every floor page over the box is held. A new sunLevels.test.ts case covers the no-box path.
      • Left, for the lead: a sprite outside the opaque box, whose pages no report has named yet, reads the far-shadow ray for the report lag (two or three frames) instead of a pre-asked floor. Once reported, askFloor maps its floor as before, so the still image is unchanged.
      • Left: the box's corners are walked twice per sun per frame.
      • Left: a sun missing from the store keeps the last rectangle, as its other clipmap fields do.

Lead verification

  • The floor pre-ask bounded by the scene: delivered in packages/sdk-core/src/scene/light-shadow/sunLevels.ts (floorReach bounded by the scene box's rectangle on the light plane, plus one page), with math.ts sunBoxRect shared with invalidate.ts. The scene box also follows engine moves, in packages/sdk-browser/src/webgpu/shadow/sceneBox.ts and encodeShadows.ts. Proved by dollyRefetch.test.ts:25 "a dolly over a small scene whose reads fit the pool evicts nothing it reads again" (fails on develop), the sunLevels.test.ts unbounded-box case, and spriteShadowCut.test.ts:77 "the shadow scene box follows a pose the engine moved, with no table change" (fails on develop).
  • Duplicate asks and the drop state (loops A and B): reverted, because the measurer's run showed no effect. Written on The sun's floor shadow pages are asked only inside the scene, never across the whole far distance #763 before the merge; no gpu/dag file differs from develop.
  • The measurer's branch proof (2e66ba6 against develop 368d8e7):
    • refetches per frame, mean, 5 446 → 2 051 (−62 %);
    • pages drawn 268 → 211, pool in use 4 069 → 3 678;
    • image 0 px, plain and TAA sides;
    • drive-a-car GPU p95 not above develop: not reproduced on the re-run, and the one high run sat in composition and TAA, not in a shadow pass.

Before merge:

  1. The whole promise: the floor bound and every Proof item of The sun's floor shadow pages are asked only inside the scene, never across the whole far distance #763 hold; A and B are dropped by a measured decision written on the issue.
  2. Tests that bite: dollyRefetch.test.ts and the scene-box test fail on develop's sources. They use the real plan and pool, with no fixed delays.
  3. No image loss: 0 px on sponza. Past the scene box no caster exists: the box is the caster box the depth range spans, so a reader there (a sprite, water, fog) reads lit through the far-shadow ray until its own asked page is drawn. The scene box now follows moving objects, closing a latent develop gap where a caster could leave the depth range.
  4. Reuse: sunBoxRect replaces invalidate.ts's own copy; sunPageMeets is proved equivalent (rects[2]/[3] are develop's -rects[3]/-rects[2]). There is no other new export.
  5. Docs: no public member changes; the comments in shadowFactorWgsl.ts, requests.ts, sceneBox.ts and lights.ts now say the floor spans the box. This body describes this diff and closes The sun's floor shadow pages are asked only inside the scene, never across the whole far distance #763.
  6. Measured first: the cut acts on the measured refetches (130 a frame, half the pages drawn); the loops without a measured effect were reverted.
  7. Path: the split from Streaming without holes 8/12 (cont.): shadows within 2 ms of GPU on the reference scenes #525 and the revert of A and B are written on Streaming without holes 8/12 (cont.): shadows within 2 ms of GPU on the reference scenes #525 and The sun's floor shadow pages are asked only inside the scene, never across the whole far distance #763. Labels: in review now, to measure at the hand-over.

Not proven / left out

The measurer proves the branch before the pull request opens, sponza, branch against develop:

node bench/runner/bench.ts --moteur webgpu --apres dist --scene sponza --soleil --textures cache --pixelError 0 --vues generale --largeur 3456 --hauteur 2234 --camera-mobile --images 120

Read per frame, from ombresParImage:

  • shadowPagesRefetched per frame: the key number. Target: refetches per frame about equal to the newly exposed pages, near 0 on this dolly.
  • shadowPagesDrawn (mean, p95, max);
  • shadowPoolPages against 4 096;
  • the Shadows stage p50 / p95, against the 2 ms target.

Also:

  • The counter over-counts a panned clipmap slot (above). If refetches stay high, the measurer's shadowPagesDrawn settles it.
  • drive-a-car: pages drawn per frame, with the 2026-09-25 19:56 protocol.
  • Still image A/A 0 px against develop.

If the refetches fall and the stage stays over 2 ms, the remaining exact route is per-page occlusion culling of casters, which goes to the CTO for a split.

pasquelin and others added 18 commits September 26, 2026 00:44
… requestPage, and its word offset is pinned to the layout (#525)
…o a dropped frame cannot lose it; an empty catalogue writes no stamp (#525)
@pasquelin
pasquelin merged commit 073e1b0 into develop Sep 26, 2026
7 checks passed
@pasquelin
pasquelin deleted the 525-shadow-pixel-cost branch September 26, 2026 06:56
@pasquelin pasquelin added the audited merged pull request re-read by the auditor label Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audited merged pull request re-read by the auditor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant