docs(render): S3 chunk 4 fix round 2 contract (§10.5) — the last allowed round
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 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
b3d4be6ab3
commit
65675e77a8
1 changed files with 180 additions and 0 deletions
|
|
@ -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`)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue