docs(render): §12 — S3 review fix round 1 (punch-fan view alignment, seal pin through the producer, dead clip routing, EC/OC transcript exactness) and the chunk-3 perf evidence correction
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
5bc4b36879
commit
174f30f176
1 changed files with 119 additions and 0 deletions
|
|
@ -1056,6 +1056,125 @@ with `ScissorNdcAabb|UseScissorFallback|TerrainClipMode.Scissor|
|
|||
ndcClipAabb|terrainUploadCount` returns nothing under `src`/`tests`; no
|
||||
register row (AD-17's correction is the lead's, in the same landing).
|
||||
|
||||
## 12. S3 review fix round 1 (lead, 2026-09-03 evening — after the two-lens slice review of `51d532320..e6f5a156b`, both FAIL)
|
||||
|
||||
**Verdicts.** Retail lens FAIL: one BLOCKING (F1), one major (F2), one minor
|
||||
(F3), three notes (F4a/F4b, and the weather-gate/print cell mismatch, logged
|
||||
below as an open question). Shader/perf lens FAIL: one BLOCKING on the
|
||||
ledger's chunk-3 perf evidence (E1, the LEAD's item), two majors (F5, F3),
|
||||
two minors (perf reductions available — S5 candidates, logged), one note
|
||||
(the two KEEP clips' >8-edge overflow branches lack equivalence pins — folded
|
||||
into F2).
|
||||
|
||||
**F1 (BLOCKING) — punch fans clipped by the wrong view.** `RetailFrameWalk.
|
||||
DrawBuilding` pins `ActiveViewIndex = v` over the walk's views (retail
|
||||
`building_view = portal_view_num` @0x0059f3bf); the leaf indexes
|
||||
`clipAssembly.OutsideViewSlices[activeViewIndex]` — but
|
||||
`ReassembleOutsideViewFromWalk` skips a view whose polygon collapses
|
||||
(`AppendOutsideSlice` returns false on `IsNothingVisible`), so the slice array
|
||||
can be SHORTER than the view list: every later fan takes view v+1's planes
|
||||
and the last takes `default` = UNCLIPPED. Fix: the reassembly keeps ONE slice
|
||||
per walk view, index-aligned (a collapsed view yields a slice flagged
|
||||
nothing-visible: `ClipViewSlice.NothingVisible`, slot 0, empty planes, empty
|
||||
AABB); `DrawWalkPunchFan` draws NOTHING for a nothing-visible slice (retail:
|
||||
`polyClipFinish` under a degenerate view leaves <3 vertices → no fan,
|
||||
`0x59BDBC–0x59BDBF`) and FAILS LOUD (exception, fail-loud rule) when
|
||||
`activeViewIndex >= OutsideViewSlices.Length` instead of drawing unclipped.
|
||||
`OutsidePlaneCount`/`ScissorFallbacks` bookkeeping unchanged. Tests: a
|
||||
synthetic two-view `WalkPortalView` whose FIRST polygon collapses (area under
|
||||
`MinPolygonArea`) → `OutsideViewSlices.Length == 2`, `[0].NothingVisible`,
|
||||
`[1].Planes` equal the second polygon's edge planes (through
|
||||
`BeginWalkFrame` + `ReassembleOutsideViewFromWalk`); the punch leaf with
|
||||
`activeViewIndex = 1` uses `[1]` (recording surface: the fan submitted with
|
||||
those planes); with `activeViewIndex = 0` submits nothing; with index 2
|
||||
throws. MUTATION: revert the skip (append nothing) → the index test fails;
|
||||
restore the `: default` fallback → the throw test fails.
|
||||
|
||||
**F2 (major) — the exit-seal equivalence pin through the producer that
|
||||
runs, and both overflow branches.** Production seal planes come from
|
||||
`WalkFrameDriver.AppendClipSlot` (pixel→NDC, winding selection, normalize,
|
||||
the >8-vertex NDC-AABB branch) read back by `InteriorFloodViewClipPlanesAt`
|
||||
→ `ClipFrame.GetSlotPlanes`; the current pin drives
|
||||
`ClipFrame.AppendSlot(ClipPlaneSet)`, which has zero production callers.
|
||||
Rewrite it to drive `CaptureViews`/`AppendClipSlot` on a synthetic
|
||||
`WalkPortalView` (the K6 pin's construction) and read back through
|
||||
`InteriorFloodViewClipPlanesAt`; add the two overflow cases: a 9-vertex
|
||||
view → the seal slot carries exactly the 4 AABB planes that contain every
|
||||
vertex (over-include); a 9-vertex outside view → the punch slice has zero
|
||||
planes and the fan draws unclipped. Mutations: flip `ccw`, drop the
|
||||
normalize, delete the AABB branch — each must fail (record all three).
|
||||
|
||||
**F3 (minor/major, both lenses) — the false bracket comment** at
|
||||
`RetailPViewPassExecutor.cs:453-457`: neither KEEP clip brackets anything
|
||||
with Enable/DisableClipDistances (the six `DisableClipDistances()` calls sit
|
||||
around the unclipped particle draws; the surface's bodies are no-ops on
|
||||
Vulkan). Rewrite to the truth; then DELETE the six no-op
|
||||
`DisableClipDistances()` calls and the private wrapper (a method that does
|
||||
nothing on the only backend is a false mechanism too), keeping the
|
||||
`IWorldPassSurface` members only if `WorldScenePassExecutor` still needs
|
||||
them (it calls them — leave the interface, delete this file's use).
|
||||
|
||||
**F4 (notes, transcript exactness) —** (a) `EmitFloodTurns` prints one `EC`
|
||||
per flood cell; retail's `DrawEnvCell` breakpoint fires once per (cell ×
|
||||
live view) (`PView::DrawCells` @0x005a4ab1–0x005a4acc, the `setup_view` /
|
||||
`DrawEnvCell` loop). Print `EC` once per live view of the cell
|
||||
(`InteriorFloodViewSliceCountAt`), draw unchanged (the stamp dedupes).
|
||||
(b) `WalkTraceConformanceTests.Recorder` lacks `OnWeatherTurn`, so the
|
||||
eight-kind signature drops the weather `OC` on both sides; implement it and
|
||||
stop dropping the trailing OC on the oracle side — the signature then pins
|
||||
the trailing weather turn. Every fixture row must still pass (the captures
|
||||
carry the OC literally).
|
||||
|
||||
**F5 (major) — the dead per-cell clip machinery.** `WbDrawDispatcher.
|
||||
SetClipRouting(...)` (the only writer of `_clipRoutingActive`) has ZERO
|
||||
callers; `EnvCellRenderer.SetClipRouting` is called only with `null`. So no
|
||||
path can arm the per-cell clip-region table — §10.6's "live for the flat
|
||||
path" was wrong, and five pipelines (`mesh_modern`, `mesh_atmospheric`,
|
||||
`mesh_detail`, `particle`, `particle_mesh`) still fetch `clipRegions[slot]`
|
||||
and write eight always-passing `gl_ClipDistance`s per vertex. Delete: the
|
||||
routing state and methods (`SetClipRouting` both overloads,
|
||||
`ClearClipRouting` remnants, `_clipRoutingActive`, `ResolveSlotForFrame`'s
|
||||
routing arm — it becomes "slot 0, never culled"), the `ProbeClipRouteEnabled
|
||||
&& _clipRoutingActive` branch, the five shaders' `clipRegions` fetch +
|
||||
`gl_ClipDistance` loops + `gl_PerVertex` redeclarations (recompile; re-pin
|
||||
the manifest), and the `ClipRegions` SSBO publication/binding IF no other
|
||||
shader reads it (`portal_depth.vert` takes its planes per draw through the
|
||||
UBO at binding 2 — verify; if `ClipRegions` has no reader, delete
|
||||
`PrepareClipFrame`'s publish, `WorldFrameSections.ClipRegions`,
|
||||
`BindClipRegions`, and `ClipFrame`'s region packing, keeping only
|
||||
`AppendSlot`/`GetSlotPlanes` as the CPU plane store the seals read). Keep
|
||||
the vertex layouts' `aClipSlot`/`instanceClipSlot` inputs THIS round (the
|
||||
instance-buffer layout is S5's), fed 0; note the residue in the commit body.
|
||||
AD-17 is corrected by the lead in the landing ("dormant in every path;
|
||||
deleted").
|
||||
|
||||
**E1 (LEAD, BLOCKING on evidence) — the chunk-3 perf gate.** The ledger's
|
||||
"every destination within the §9.7 gate" quoted only the `turn` phase of the
|
||||
two report files; the `stationary` phase has Aerlinthe 4.00→5.80 ms GPU p50
|
||||
(1.45×, +1.8 ms; p95 4.20→6.00) — outside the gate — and two destinations
|
||||
(Sawato revisit/plateau) were omitted. The lead re-reads both files, records
|
||||
the COMPLETE table, and re-measures the current tip against the S2-final
|
||||
binary in the same session (`run-connected-r6-soak.ps1 -SkipBuild`, both
|
||||
worktrees, back-to-back); chunk 3's status becomes STOPPED-LANDED with an
|
||||
open perf item if the regression holds (the plan's own rule), and the
|
||||
order-preserving reductions the shader lens listed (skip the terrain flush
|
||||
when the alpha queue is empty; coalesce contiguous per-cell runs into fewer
|
||||
indirect commands) become the S5 perf chunk's contract.
|
||||
|
||||
**Open question logged (not this round):** the weather gate reads the
|
||||
PLAYER cell (`is_player_outside`) while the `OC` line prints the VIEWER cell
|
||||
(retail's `after_sky_cell` = the P cell). A camera inside a cell with the
|
||||
player outdoors would print an EnvCell id; no capture exercises it. Settle
|
||||
with one retail capture at such a pose before S5's transcript closeout.
|
||||
|
||||
**Gates:** build 0 warnings; hermetic fully green; InstalledDat exactly the
|
||||
four known; every fixture row of `WalkTraceConformanceTests` unchanged
|
||||
except the OC now compared; the shader tests (descriptor contract, manifest,
|
||||
SPIR-V validator, sky layout) green; the L6+H6 grep sweeps plus
|
||||
`SetClipRouting|_clipRoutingActive|clipRegions|instanceClipSlot` (the last
|
||||
two allowed only in the vertex-layout residue named in the body); no new
|
||||
flag/probe; the lead's four-pose validation self-gate afterwards.
|
||||
|
||||
## 11. Chunk 1 contract — transcript kinds, fixtures, and the print-only emitter (lead, 2026-09-03; runs AFTER chunk 3, because `LC`/`SC` only exist once terrain is per cell)
|
||||
|
||||
### 11.1 What exists (verified in the tree at `542b049b`)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue