From 65675e77a87c4c74a21a474e646a3e921f4a837a Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 3 Sep 2026 14:29:33 +0200 Subject: [PATCH] =?UTF-8?q?docs(render):=20S3=20chunk=204=20fix=20round=20?= =?UTF-8?q?2=20contract=20(=C2=A710.5)=20=E2=80=94=20the=20last=20allowed?= =?UTF-8?q?=20round?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 1 (d60ca4ea0) failed all three lenses: the draw-side weather gate is unpinned, the scissor stack survived K4 on a false comment, the sky shader still claims retail clips the sky, the K6 pin runs through an assembler overload production never calls, and K3 left a dead UBO fed per draw. L1-L9 delete the stacks outright and pin the draw gate's condition. Co-Authored-By: Claude Fable 5.1 --- .../s3-walk-ownership-map.md | 180 ++++++++++++++++++ 1 file changed, 180 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 6a1a2092..d59ca3e6 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 @@ -787,6 +787,186 @@ real leftovers. Every item lead-verified. `DrawWeatherOnce`'s comment so the order change is on record. - Gates: §10.3 unchanged; InstalledDat exactly the four known failures. +### 10.5 Chunk 4 fix round 2 — the LAST the plan allows (written 2026-09-03 after the round-1 three-lens FAIL at `d60ca4ea0`) + +**Round-1 verdicts (all three lenses FAIL, `s3-chunk4-impl` @ `d60ca4ea0`).** +Shared BLOCKING finding: K2's fix is right in production +(`RetailFrameWalk.cs:253` → `WalkFrameDriver.OnWeatherTurn` sets +`WeatherTurnFired` at the print site → `RetailPViewRenderer.cs:665` +`if (walkDriver.WeatherTurnFired) passes.DrawWeatherOnce(ctx);`) but NOTHING +pins the draw side: the only test naming the flag +(`WalkFrameDriverTranscriptTests.cs:765`) builds a `WalkFrameDriver` alone, and +the two IL pins (`RetailPViewPassExecutorTests.cs:171`, `:205`) never inspect +the branch CONDITION — restoring the pre-fix gate +`clipAssembly.OutsideViewSlices.Length != 0` leaves every lane green. The +transcript test's doc (`:725-733`) falsely claims it "doubles as the draw-side +pin". MAJOR: (a) K4 deleted `BeginDoorwayScissor` but kept +`IWorldPassSurface.BeginScissor/EndScissor`, `NdcScissorRect` and the +`EndScissor()` inside `ClearInteriorDepth` on the comment "still ends an active +scissor" (`RetailPViewPassExecutor.cs:450-459`) — no producer of a scissor +exists any more (`BeginScissor` has ZERO callers), so the comment asserts a +mechanism that does not exist, the exact C4-closeout failure class. (b) +`sky.vert:88-102` still states retail clips the sky to the exit-portal region +and cites the deleted `ClipFrame.SetTerrainClip`; the decomp +(`acclient_2013_pseudo_c.txt:267912-267918`, `GameSky::Draw(sky,0)` @0x0050633c +before `draw_check_blocks`) and K4's own doc say the opposite. (c) The K6 pin +runs through `ClipFrameAssembler.Assemble`, which has ZERO production callers +(`ClipFrameAssembler.cs:4-5`; production uses `BeginWalkFrame` @ +`RetailPViewPassExecutor.cs:156` and `ReassembleOutsideViewFromWalk` @ +`RetailPViewRenderer.cs:285`); the slice construction is DUPLICATED at +`ClipFrameAssembler.cs:394` and `:533`, so the pin proves nothing about the +producer that runs. MINOR: K3 turned one per-frame publish into one 144-byte +zeroed ring allocation + clear per terrain flush and per sky draw +(`WorldPassScope.cs:190-216` fallback, called from +`TerrainModernRenderer.Rhi.cs:259` and `SkyRenderer.Rhi.cs:258`); the retained +`ClipFrame._terrainBytes/TerrainBytes/TerrainBytesForTest` triple is dead state +kept only so `NoClip_TerrainBytes_Count0_AllZeros` compiles (unfailable by any +production edit); `ClipFrame.cs:140` names the deleted `SetTerrainClip`; +`Issue130DoorwayStripTests` (`:20`, `:36-39`, `:297-310`) pins a scissor +mechanism production no longer has; the commit message deferred the mutation +texts to an ephemeral report and said "four" for five; `DrawWalkSky` has no +loop-shape pin although K1 added one for the identical class; the round-1 +review prompt's "two #383 + TowerAscent" InstalledDat expectation was stale +(the fourth known failure is #458, pinned by §10.4 itself). + +**Rule.** This is fix round 2. A third round STOPS chunk 4 and writes it up in +§13 (plan §5); nothing here may be forced. Every item below is a deletion, a +comment correction, or a pin — no new admission/order/flag/probe. + +**L1 (BLOCKING) — the draw-side weather-gate pin.** Add to +`RetailPViewPassExecutorTests` a pin +`DrawLandscapeDynamicsPhase_GatesDrawWeatherOnceOnWalkDriverWeatherTurnFired` +that reads the private method's calls (`CompiledCallGraph.Read`) and branches +(`ReadBranches`) and asserts, with `c` = the index of the `DrawWeatherOnce` +call: (a) `calls[c-1].Target` is `WalkFrameDriver.get_WeatherTurnFired` — the +condition is the LAST call before the draw; (b) exactly one branch `b` with +`calls[c-1].Offset < b.Offset < calls[c].Offset`, `b.OpCode` in +{`brfalse`, `brfalse.s`}, and `b.TargetOffset > calls[c].Offset` — a forward +skip over the draw on a false flag. Keep round 1's loop-shape pin. THREE +mutation checks, each shown failing with its assertion text recorded: M1 +restore `if (clipAssembly.OutsideViewSlices.Length != 0)`; M2 delete the `if` +(unconditional draw); M3 invert to `if (!walkDriver.WeatherTurnFired)`. Then +correct `WalkFrameDriverTranscriptTests.cs:725-733`: that test pins the +PRINT half and the flag; the draw half is the new pin (name it). + +**L2 (MAJOR) — delete the scissor stack outright and tell the truth in the K4 +comment.** Delete `IWorldPassSurface.BeginScissor`/`EndScissor` and their +`RhiWorldPassSurface` bodies (`WorldPassSurface.cs:82-87`, `:157-171`), the +`EndScissor()` call inside `ClearInteriorDepth` (`:183`) together with the +paragraph justifying it — the pass encoder sets the full-attachment scissor +at pass begin (`VulkanGpuPassEncoder.cs:87`) and after this round nothing +narrows it (cite both facts in the replacement comment), `NdcScissorRect.cs` +and `NdcScissorRectTests.cs`, and the `NdcScissorRect.ToPixels` mention in +`VulkanViewportMapping.cs:41` / `VulkanViewportMappingTests.cs:21`. +`IGpuPassEncoder.SetScissor` STAYS (pass begin). Delete the assembly's +terrain-mode remnants whose only readers are diagnostic text: +`TerrainClipMode`, `ClipFrameAssembly.TerrainMode`, `TerrainScissorNdcAabb`, +their writers at `ClipFrameAssembler.cs:282/293/400-419/539-550`, and the two +readers `WorldRenderDiagnostics.cs:345` (`terrain=`) and `:409` (`outMode=`). +KEEP `ScissorFallbacks`, `OutsidePlaneCount`, `OutsideViewNdcAabb`, +`HasOutsideView`, `OutdoorVisible`, `OutdoorSlot` and `ClipViewSlice.NdcAabb`: +the >8-edge zero-plane slice case (`ClipFrameAssembler.cs:524-533`, +`ClipPlaneSet.cs:23-39`) is live for the punch fans +(`RetailPViewPassExecutor.WalkLeaf.cs:103-118`), and `EmitVis` reads the +counters. Rewrite `RetailPViewPassExecutor.cs:450-459` to what is true after +this round. + +**L3 (MAJOR, completes K3) — delete the dead TerrainClip UBO from the sky and +terrain shaders and everything that fed it.** No writer can ever exist again, +so the block is dead in `sky.vert:103-106` (+ loops `:169-180`), +`terrain_modern.vert:56-59` (+ `:185-190`) and `terrain_atmospheric.vert:58-61` +(+ `:204-209`). Delete the block, both `gl_ClipDistance` loops, and the +`gl_PerVertex` redeclaration where nothing else needs it; recompile with +`tools/compile-shaders.ps1` (read its header) and commit the `.spv` files and +the manifest it updates (`VulkanShaderManifestTests` pins source SHA-256 to the +compiled module; `RenderPackSpirvValidatorTests` validates the modules). +Delete `WorldFrameSectionBinding.BindTerrainClip` and `Zeroed` +(`WorldPassScope.cs:185-216`) with the two callers (`SkyRenderer.Rhi.cs:258`, +`TerrainModernRenderer.Rhi.cs:259`) and `WorldFrameSections.TerrainClip` (+ its +reset `WorldPassScope.cs:53`); delete the no-op `IWorldPassSurface.BindTerrainClip` +(`WorldPassSurface.cs:47-64`, `:143-145`) and `WorldScenePassExecutor.cs:121/235`'s +calls to it; delete `ClipFrame.TerrainUboBytes`, `_terrainBytes`, `TerrainBytes`, +`TerrainBytesForTest`, Reset's `Array.Clear(_terrainBytes)` (`ClipFrame.cs:69`, +`:85-86`, `:140-141`, `:312`, `:321`) and the stale header/K3 paragraphs +(`:16`, `:31-35`, `:233-243`); delete `NoClip_TerrainBytes_Count0_AllZeros` and +the `:183-187` comment in `ClipFrameLayoutTests`. KEEP +`ClipFrame.TerrainClipUboBinding` = 2 and `VulkanPipelineLayouts.UniformTerrainClip` += 2: `portal_depth.vert:32-34` (the exit-seal/punch clip block, a KEEP clip) +still binds there (`PortalDepthMaskRenderer.Rhi.cs:184`) — fix both consts' +docs (`ClipFrame.cs:70-77`, `VulkanPipelineLayouts.cs:377-384`) to say only +the portal-depth clip block uses binding 2 now. Update +`VulkanShaderDescriptorContractTests.cs:136-156` (terrain_modern's uniform set +becomes {SceneLighting}); its purpose (set-0/set-1 dialect regression) stays. +REQUIRED in the commit body, with `file:line`: what `VulkanGpuPassEncoder` / +the set-1 dynamic-offset path does for a declared-but-never-bound dynamic +uniform binding on a draw whose shader does not statically use it (read the +encoder's `BindUniformBuffer` and the set-1 bind; a descriptor not statically +used by the pipeline may be unwritten — cite the code path that makes the +terrain and sky draws legal). The lead re-runs the self-gate with +`ACDREAM_DEVTOOLS=1` (Vulkan validation layers) and reads the log for +validation errors; a validation error fails this round. + +**L4 (MAJOR) — one outside-view slice constructor, and the K6 pin through the +producer that runs.** Extract ONE internal static helper in +`ClipFrameAssembler` that turns a `ViewPolygon` into the appended slot + the +`ClipViewSlice` (the code at `:379-394` and `:509-533` — including the +zero-plane fallback branch and the max-plane/fallback bookkeeping) and call it +from BOTH `Assemble` and `ReassembleOutsideViewFromWalk`. Rewrite the K6 pin to +build a synthetic `WalkPortalView` holding one pixel-space polygon (the way +`WalkCopyViewTests` builds views), run `BeginWalkFrame(frame, outdoorRoot: +false, null)` then `ReassembleOutsideViewFromWalk(assembly, view, w, h)`, and +read `assembly.OutsideViewSlices[0].Planes`. Mutations: perturb `planes[0].W` +INSIDE the shared helper → fails (record the text); the pin's source contains +no `Assemble(` (grep-checked in the commit body). Fix the K6 doc comment. + +**L5 — shader comments state retail truth.** After L3, whatever comment remains +in `sky.vert` / the terrain shaders about clipping says: retail draws the sky +ONCE (`LScape::draw` 0x00506330 → `GameSky::Draw(sky,0)` @0x0050633c, before +`draw_check_blocks`, no view installed, no clip) and the landscape is +view-CULLED per cell, never GPU-clipped (`RenderDeviceD3D::DrawBlock` +0x005a17c0). Delete the "Phase W Stage 4" narrative (`sky.vert:88-102`, +`:169-176`) and the "U.3 default" clip paragraphs in both terrain shaders. + +**L6 — stale comments.** Everything L2/L3 does not already delete that names +`SetTerrainClip`, `BeginDoorwayScissor`, `BindTerrainClip`, `TerrainBytes` or +"an active scissor": a grep over `src`, `tests` and `docs/architecture` for +`SetTerrainClip`, `BeginDoorwayScissor`, `BindTerrainClip`, `TerrainBytes`, +`active scissor`, `TerrainClipMode`, `NdcScissorRect` (cs, vert, frag, md) +must return nothing after this round except the plan/research record under +`docs/plans` and `docs/research` (history stays). + +**L7 — `Issue130DoorwayStripTests`.** Delete the scissor half +(`AnySliceAdmitsScissor`, `worstScissorGapPx`, its assertion, the header's +scissor sentences at `:20`, `:36-39`, `:297-310`). Rewrite the header: the #130 +scissor mechanism was retired at S3 chunk 4 (the sky and the landscape draw +unclipped; an interior root's aperture exactness comes from the depth clear, +the exit seals and the interior repaint — `WalkFrameDriver`'s interior turn); +what remains pins the outside-view polygon pipeline (`ProjectToClip` → +`ClipToRegion` → `ClipPlaneSet.From`) that the punch fans still consume +(`DrawWalkPunchFan` reads `OutsideViewSlices[i].Planes`). If the remaining half +pins nothing production reads, delete the test and say so in the commit body. + +**L8 — `DrawWalkSky` loop-shape pin.** The K1 pin's shape applied to +`RetailPViewPassExecutor.DrawWalkSky`'s `SkyRenderer.RenderSky` call site: no +backward branch spans it. Mutation: wrap the call in a two-iteration `for` +loop → fails (record the text). Note in the K1 and L8 pins' comments that +`CompiledCallGraph.ReadBranches` ignores `switch` jump tables (no C# loop +compiles to one) — the reviewers' scope note. + +**L9 — the commit message.** Inline EVERY mutation check's failing assertion +text verbatim (M1–M3, L4, L8), as `6ba4b0b87` did; state the L3 encoder fact +with `file:line`; state the L6 grep result; last line `Co-Authored-By`. + +**Gates (unchanged from §10.3/§10.4 plus):** build 0 warnings / 0 errors; +focused filter incl. `FullyQualifiedName~Gpu`; hermetic lane fully green; +InstalledDat exactly the four known failures (2x #383, TowerAscent, #458 +doorway); `VulkanShaderDescriptorContractTests`, `VulkanShaderManifestTests`, +`RenderPackSpirvValidatorTests`, `SkyVertexLayoutTests` green; no register +row (every deletion removes an acdream-only rule); no new flag/probe; the +lead's `ACDREAM_DEVTOOLS=1` four-pose self-gate shows no validation error and +no visual delta against `logs/selfgate-20260903-104516-s3-chunk3-r2` beyond +actors/daylight. + ## 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`)