docs(render): S3 chunk 3 fix round 1 — slot key, cross-block deferred batching, per-frame diagnostic, LandCell pins

Three-lens review of 671eb3ad: the terrain slot map is keyed by the DAT id
(0xXXYYFFFF) while the walk passes 0xXXYY0000 (no terrain drawn); the
adjacent-same-landblock batching can never merge because retail's own
DrawSortCell interposes a particle turn after every land cell; the timing
diagnostic moved inside the per-batch loop. Round 1 of two.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-09-03 08:58:43 +02:00
parent 51c6bfcd96
commit eb80c80eb6

View file

@ -518,6 +518,89 @@ against the chunk-2 run: the ONLY expected delta is the punch region at
Holtburg (fragments gone); dense-outdoor p50/p99 recorded before/after with
the existing frame-time diagnostics (no new counters).
### 9.6 Chunk 3 fix round 1 (lead, 2026-09-03, after the three-lens review of `671eb3ad`)
Verdicts: build/test FAIL (blocking: the per-batch timing diagnostic), retail
FAIL (blocking: the terrain slot key), driver FAIL (blocking: batching is
inert). Lead-verified against the source; every item below is real. Fix in the
same worktree on top of `671eb3ad`; this is round 1 of the two the plan allows.
- **F1 — slot key (blocking).** `TerrainModernRenderer._idToSlot` is keyed by
the DAT landblock id `0xXXYYFFFF` (`LandblockRenderPublisher.LandblockId =>
Build.Landblock.LandblockId`; the old walk filter compared
`data.LandblockId & 0xFFFF0000`), while the walk hands `0xXXYY0000`
(`WalkLandBlock.LandblockId = bx<<24 | by<<16`). `DrawLandCellRuns` therefore
misses every lookup and the walk path draws NO terrain. Normalize at the
terrain entry: look up `(landblockId & 0xFFFF0000u) | 0xFFFFu`; add a unit
test that `AddLandblock(0xA9B4FFFF, …)` is found by a `0xA9B40000` lookup and
that an unknown block is a no-op.
- **F2 — batching across landblocks, deferred across no-op turns (blocking).**
Retail's `DrawSortCell` follows every `DrawLandCell`, so `HandleLandscapeCellTurn`
always appends a `StaticParticles` event (and, for cells with content, a
`StreamMark`) between two `LandCell` events — the "adjacent, same landblock"
rule never merges. The terrain index buffer is ONE buffer with
`FirstIndex = slot*384 + start`, and the deleted whole-stage draw already
issued one `MultiDrawIndexedIndirect` spanning every visible slot, so a
batch may span LANDBLOCKS. New rule, still order-preserving by construction:
Replay keeps a PENDING terrain batch (list of `(landblockId, side,
cellIndex)`, cleared at frame start); a `LandCell` event appends to it; before
any leaf call that WILL submit GPU work the pending batch is flushed
(`DrawLandCells` once); a particle turn whose cell has no renderable emitter
submits nothing and does NOT flush — add
`ParticleSystem.HasRenderableEmittersInCell(pass, cellId)` (a lookup in the
existing per-pass cell→handles index, no allocation) and ask it in Replay's
`StaticParticles`/`CellParticles` arms BEFORE flushing; every other event kind
(`StreamMark`, `AlphaBarrier`, `LandscapeFlush`, `ClearInteriorDepth`,
`ExitSeals`, `PunchFan`, `CellShell`, `Sky`) flushes first, and the end of
Replay flushes the remainder. The GPU submission order is then IDENTICAL to
the unbatched order (a skipped particle turn submits nothing either way).
`TerrainModernRenderer.DrawLandCellRuns` becomes `DrawLandCells(viewProjection,
IReadOnlyList<(uint LandblockId, int SideCellCount, int CellIndex)>)`:
resolve each entry's slot (F1), skip unknown slots, build the run commands in
list order, ONE indirect draw. Expected at terrace-edge frame 2: tens of
submissions, not 578; report the count from a driver test and in perfNote.
- **F3 — the terrain diagnostic (blocking).** One sample per FRAME, not per
batch: the leaf accumulates each batch's elapsed `Stopwatch.GetTimestamp()`
delta (no allocation) and the executor/renderer pushes ONE sample at the
end of the walk replay (where the old whole-stage `Complete()` semantics
lived); `TerrainRenderDiagnosticFacts.VisibleSlots` = distinct slots
submitted this frame (a per-frame bit set/scratch cleared at
`BeginFrame`), `draws` = batches submitted. The `[TERRAIN-DIAG]` line keeps
its meaning (cpu_us per frame) so §9.5's before/after compare is valid.
- **F4 — driver pins for the LandCell position (major).** The deleted
`TERRAIN:0` pins are REPLACED, not dropped: the fake leaf logs
`LANDCELL:<lb>:<side>:<idx>[,…]` per batch; add (a) an outdoor-root
`RunFrame` test with a synthetic in-view block whose sequence is
`SKY, LANDCELL…` with the block's cell turns after their terrain, (b) an
interior-root test with one exit view and one block: `SKY, LANDCELL…,
LFLUSH, SEALS, SHELL…`, (c) the batching pin from T4 re-expressed for the
new rule: two land cells of DIFFERENT landblocks with only an empty particle
turn between them merge into one `LANDCELL` batch; a `StreamMark` (a cell
with statics) or a building barrier between them splits; the fake leaf's
particle turn reports "has emitters" from a test-set so both arms are
covered.
- **F5 — interior-root terrain extent (major, answered, no code change).**
Retail's `DrawLandCell` fires only for in-view cells (`DrawBlock` @0x005a197d;
capture: `f418` 16 LC of 64) — the per-cell admission IS retail; the FW4
under-paint came from the GPU clip of terrain to the portal polygon, which
no longer exists. Keep the in-view gate; the cathedral self-gate poses are
the check (recorded in the ledger either way).
- **F6 — minor/notes.** Frustum cull: the walk's `CheckBlocks` admission is the
authority (retail has no separate terrain frustum test beyond
`landcell_check`); document that in `DrawLandCells`' comment. The cathedral
order-trace token prints `:LC<lb>/<side>:<idx>`. Fix the inverted comment in
`HandleLandscapeTurn` (a FARTHER building's punch survives because NEARER
terrain was drawn before it; the interleave draws that terrain after). Fix
the "directional-shadow receivers" claim (the only non-walk caller is the
flat-terrain path). Replace T2's vacuous `TERRAIN` assertion with the
driver-level pins of F4. Retire the stale "Confirmed OH5 defect" row in
`oh1-construction-landscape-contract.md` (line ≈310) with a one-line
"FIXED by S3 chunk 3 (`<sha>`)".
- Gates unchanged (§9.5); the implementer additionally reports, from the
F4(c)-style driver test over a synthetic terrace-like frame, the batch
count for the terrace-edge cell sequence if it can be driven cheaply, else
the reasoning.
## 10. Chunk 4 contract — deletions (lead draft, 2026-09-03; re-locate every owner at chunk time, after chunks 2 and 3 land)
### 10.1 What retail clips, and what it does not (verified)