From 174f30f176819f92cf5f0014c62610a96e5c0fde Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 3 Sep 2026 18:48:46 +0200 Subject: [PATCH] =?UTF-8?q?docs(render):=20=C2=A712=20=E2=80=94=20S3=20rev?= =?UTF-8?q?iew=20fix=20round=201=20(punch-fan=20view=20alignment,=20seal?= =?UTF-8?q?=20pin=20through=20the=20producer,=20dead=20clip=20routing,=20E?= =?UTF-8?q?C/OC=20transcript=20exactness)=20and=20the=20chunk-3=20perf=20e?= =?UTF-8?q?vidence=20correction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5.1 --- .../s3-walk-ownership-map.md | 119 ++++++++++++++++++ 1 file changed, 119 insertions(+) diff --git a/docs/research/2026-09-01-overhaul/s3-walk-ownership-map.md b/docs/research/2026-09-01-overhaul/s3-walk-ownership-map.md index 129a40c9..d3cf26c2 100644 --- a/docs/research/2026-09-01-overhaul/s3-walk-ownership-map.md +++ b/docs/research/2026-09-01-overhaul/s3-walk-ownership-map.md @@ -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`)