From a06226f9a25fb486baa7a5bfcf041a8118351d62 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 2 Jun 2026 15:13:35 +0200 Subject: [PATCH] =?UTF-8?q?docs(render):=20Phase=20W=20render-rewrite=20pl?= =?UTF-8?q?an=20(Stages=203-5)=20=E2=80=94=20grounded,=20per-step?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-step subagent-driven plan for the render half: T0 test-hygiene baseline, Stage 3 render-root unification (root at CellGraph.CurrCell + seen_outside, drop the FindCameraCell grace-frame fallback), Stage 4 PView seal (sky/landscape inside the portal-clip bracket + conditional doorway Z-clear = no blue-hole; EnvCellRenderer GL_BLEND verify), Stage 5 entity/particle cell-clip. Key reframe from grounding the plan in the actual code: the PView infra (PortalVisibilityBuilder BFS + OutsideView, ClipFrame, EnvCellRenderer GL_BLEND fix, WbDrawDispatcher cell gate) ALREADY EXISTS and the A8 stencil split is already gone — so the render half is wire-and-fill-gaps, not a from-scratch port. Execution policy: no intermediate user gates, single final visual verification, full suite green at verification. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../2026-06-02-phase-w-render-rewrite.md | 782 ++++++++++++++++++ 1 file changed, 782 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-02-phase-w-render-rewrite.md diff --git a/docs/superpowers/plans/2026-06-02-phase-w-render-rewrite.md b/docs/superpowers/plans/2026-06-02-phase-w-render-rewrite.md new file mode 100644 index 0000000..199e482 --- /dev/null +++ b/docs/superpowers/plans/2026-06-02-phase-w-render-rewrite.md @@ -0,0 +1,782 @@ +# Phase W Render Rewrite — Stages 3-5 Implementation Plan + +**Goal:** Complete the render half of Phase W: root visibility at the physics `CurrCell` ++ `seen_outside` (Stage 3), port the retail PView portal traversal producing one +`cell_draw_list` + `OutsideView` that draws sky/rain through exit portals with no blue +hole (Stage 4), and clip entities/particles to the PView visible set (Stage 5). +Result: cottage interior is sealed, outdoor sky visible through the door, no transparent +walls, no entity bleed — at both cottage and dungeon. + +**Architecture (2-3 sentences):** Retail uses one cell graph and one portal-visibility +traversal (`PView`) rooted at the committed player cell (`CellGraph.CurrCell`); the +inside/outside decision is a single `(id & 0xFFFF) < 0x100` + `seen_outside` predicate, +and landscape is drawn through exit-portal clip regions by `DrawCells`, not as a separate +stencil pass. acdream already has `PortalVisibilityBuilder` (a faithful `PView` port), +`EnvCellRenderer` (cell-shell draw), `ClipFrame`/`ClipFrameAssembler` (the outdoor clip +machinery), and `CellGraph.CurrCell` (the physics membership answer) — Stage 3 deletes +the `FindCameraCell` grace-frame path and wires `CurrCell` as the mandatory root; Stage 4 +uses the existing `OutsideView` planes to drive terrain + sky draw indoors; Stage 5 uses +the `PortalVisibilityFrame.OrderedVisibleCells` set to clip entity/particle draws. + +**Tech stack:** .NET 10 / C# / OpenGL 4.3 + bindless / Silk.NET. Tests in +`tests/AcDream.Core.Tests/` (unit + replay) and `tests/AcDream.App.Tests/` (app-layer); +no new test projects. + +**REQUIRED SUB-SKILL:** `superpowers:subagent-driven-development` — each stage should +be dispatched as a bounded implementation chunk with a clear spec and acceptance criteria. + +**EXECUTION POLICY (user directive 2026-06-02):** NO intermediate user visual gates. The +per-stage "visual gate" sections below are demoted to **internal build+test-green +checkpoints** (the orchestrator launches/inspects only if a probe is needed to settle a +question). The user performs a **single final visual verification** after ALL stages land, +and the **full Core+App test suite must be GREEN** at that point (no broken/red tests). +Interim states (e.g. Stage 3's full-screen sky before Stage 4 clips it) are acceptable +because no user sees them before the final gate. The `DoorwayMembershipReplayTests` must +also use a **committed** fixture (a trimmed subset of `doorway-capture.jsonl`), not the +untracked 364K capture, so the suite stays green/portable (folded into T0). + +--- + +## Prerequisite reading (implementer must read before coding) + +Before any Stage 3+ work: +1. `docs/superpowers/specs/2026-06-02-phase-w-transition-membership-and-pview-render-design.md` + §2 (target architecture), §5 (risks), §6 (acceptance). +2. This plan, from top to bottom. +3. Current render loop: `GameWindow.cs` lines 7139–7513 — the visibility, terrain, entity, + and particle draws. Confirm line numbers before editing (they shift with every commit). +4. `src/AcDream.App/Rendering/CellVisibility.cs` — `FindCameraCell` (line 389), grace + counter (line 214), `ComputeVisibilityFromRoot` (line 356), `GetVisibleCellsFromRoot` + (line 539). +5. `src/AcDream.App/Rendering/PortalVisibilityBuilder.cs` — BFS + `OutsideView` handling + (lines 1–239). + +--- + +## File structure (created / modified) + +| File | Action | Responsibility | +|------|--------|----------------| +| `src/AcDream.App/Rendering/GameWindow.cs` | Modify (lines ~7139–7513) | Stage 3: remove `FindCameraCell` grace-frame fallback; unify the indoor/outdoor gate; Stage 4: drive terrain+sky from `OutsideView` indoors; Stage 5: entity/particle clip. | +| `src/AcDream.App/Rendering/CellVisibility.cs` | Modify (lines ~389–446) | Stage 3: delete or stub the AABB `FindCameraCell` grace-frame band-aid; promote `ComputeVisibilityFromRoot` to the sole path; expose `SeenOutside` of the root cell. | +| `src/AcDream.App/Rendering/PortalVisibilityBuilder.cs` | Possibly extend | Stage 4: confirm `OutsideView` polygons are already propagated for the no-blue-hole case; add landscape-viewpoint helper if absent. | +| `tests/AcDream.Core.Tests/Rendering/CellGraphRootTests.cs` | Create | Stage 3 unit tests: root-selection logic + `seen_outside` terrain/sky gate. | +| `tests/AcDream.Core.Tests/Rendering/PViewBfsTests.cs` | Create or extend | Stage 4 unit tests: `OutsideView` non-empty for a cell with an exit portal; sealed-dungeon `OutsideView`-empty. | +| `tests/AcDream.Core.Tests/Rendering/EntityClipTests.cs` | Create | Stage 5: cell-clip predicate unit tests. | + +> **No new production-code files** except the three new test files above. All render changes +> are surgical edits to existing files. + +--- + +## Test-hygiene task (run FIRST, before any Stage 3 code) + +**Purpose:** establish a deterministic green baseline so Stage 3–5 additions don't mask +pre-existing noise. + +### Pre-existing known failures (documented; do NOT fix unless noted) + +| Test class | Failure kind | Root cause | Action | +|---|---|---|---| +| `PhysicsResolveCapture` statics | 8–19 flaky failures per run | Static env-var read shared across test classes in the same process | Fix test isolation (see task T0.1 below). | +| `CellarUpTrajectoryReplayTests.LiveCompare_FirstCap_FixClosesCottageFloorCap` | "document-the-bug" — passes while bug exists | Documents the cottage-floor cap; the Stage 2 membership fix is expected to make this test flip to FAILING, which means the bug is now fixed. Then update it to assert the fixed behavior. | After Stage 2 lands, update this test to assert the fix. | +| `DoorBugTrajectoryReplayTests.Apparatus_Grounded_50cmOffCenter_FrontApproach_DocumentsBug` | Same pattern | Documents the door squeeze-through | After Stage 4 seals the pipeline, revisit and update to assert fixed behavior if applicable. | +| `BSPStepUpTests` (some variants) | Intermittent | Static BSP state | Same static-isolation fix. | + +### T0.1 — Fix static-leak test isolation + +- [ ] **Find the static fields causing test leakage.** Search for `static readonly` or + `static` initializers in `PhysicsResolveCapture.cs` and `PhysicsDiagnostics.cs` that + read env vars at class-init time (before any test sets them). The symptom is that a test + class that sets `ACDREAM_CAPTURE_RESOLVE` or `ACDREAM_PROBE_*` poisons the shared + statics for downstream test classes in the same process. + - Files: `src/AcDream.Core/Physics/PhysicsResolveCapture.cs`, + `src/AcDream.Core/Physics/PhysicsDiagnostics.cs`. + - Pattern to find: `static readonly bool s_xxx = Environment.GetEnvironmentVariable(...)`. + +- [ ] **Add `[Collection(nameof(PhysicsResolveCapture))]` isolation OR convert static + env-var reads to instance-time reads with a reset method.** The simplest retail-faithful + fix: add a `ResetForTest()` static method that re-reads all flags, call it in + `IDisposable.Dispose` of any test class that sets env vars. Alternatively annotate the + affected test classes with `[Collection("IsolatedPhysicsCapture")]` to force sequential + execution (xUnit's isolation model). + - MUST NOT change production behavior — only test teardown. + +- [ ] **Run `dotnet test -c Debug` twice in a row and confirm the failure set is + deterministic** (same tests, same count, or fully green). The pre-existing documented-bug + tests (`LiveCompare_FirstCap_FixClosesCottageFloorCap`, etc.) are allowed to remain red + until their stage lands — record their names in a comment so Stage 4 implementers know + to flip them. + +Commit: `test: fix PhysicsResolveCapture static-leak isolation` + +--- + +## Stage 3 — Render-root unification + +**Goal:** root render visibility at `CellGraph.CurrCell` + `seen_outside`; remove the +AABB `FindCameraCell` grace-frame source-of-truth; port the `CellManager::ChangePosition` +landscape/sky policy; camera offset via graph child lookup. Visual gate: no render-branch +strobe; landscape policy correct indoors/outdoors/dungeon. + +**Retail anchors:** +- `SmartBox::RenderNormalMode @ 0x00453aa0 (pseudo_c:92635)` — the single indoor/outdoor + decision (viewer landcell → `LScape::draw`; viewer EnvCell → `DrawInside`). +- `CellManager::ChangePosition @ 0x004559b0(pseudo_c:94601)` — landscape kept live iff + `seen_outside || isLandCell`. +- `CEnvCell::find_visible_child_cell @ 0x0052dc50 (pseudo_c:311397)` — camera-offset + child cell lookup (rooted at the player cell, not the eye). + +**Key current-code map:** +- `GameWindow.cs:7162–7166` — `physicsRoot` derivation from `CellGraph.CurrCell`; already + exists but only used as a FALLBACK to `ComputeVisibility`. +- `GameWindow.cs:7166` — `ComputeVisibilityFromRoot` call (already the main path via W2a). +- `GameWindow.cs:7185–7187` — `playerInsideCell` computation (calls `IsInsideAnyCell`, + which is an independent AABB scan — this is a separate issue from `FindCameraCell` but + related). +- `GameWindow.cs:7267` — `bool renderSky = !cameraInsideCell` — the current sky gate, + keyed off `visibility?.CameraCell is not null`. +- `GameWindow.cs:7406` — `if (terrainClipMode == TerrainClipMode.Skip)` — the current + terrain gate (no draw when `Skip`). +- `CellVisibility.cs:389` — `FindCameraCell` (AABB, with grace-frame). +- `CellVisibility.cs:214` — `CellSwitchGraceFrameCount = 3`. +- `CellVisibility.cs:356` — `ComputeVisibilityFromRoot` — already the production path. + +### T3.1 — Eliminate the `FindCameraCell` fallback path + +**Current code (CellVisibility.cs, ~line 356-369):** +```csharp +public VisibilityResult? ComputeVisibilityFromRoot(LoadedCell? root, Vector3 fallbackPos) +{ + if (root is null) + return ComputeVisibility(fallbackPos); // <-- this calls FindCameraCell(fallbackPos) + ... +} +``` + +- [ ] **Read `CellVisibility.cs:356–370` and `ComputeVisibility` (line ~521-527)** to + confirm the fallback chain: `ComputeVisibilityFromRoot(null, pos)` calls + `ComputeVisibility(pos)` which calls `FindCameraCell(pos)` with the AABB + grace-frame + logic. + +- [ ] **Change `ComputeVisibilityFromRoot` so a null `root` returns `null` instead of + calling `ComputeVisibility`.** The caller at `GameWindow.cs:7166` already handles a + null result as the "outdoor root" path (no interior portal frame, everything slot 0). + The fallback was the old bridge for the transition from `FindCameraCell` to + `CurrCell`-based root; it is now dead because W2a ensures `CurrCell` is set from the + first tick. + ```csharp + // BEFORE (CellVisibility.cs:~359-361): + if (root is null) + return ComputeVisibility(fallbackPos); + // AFTER: + if (root is null) + return null; // outdoor root: caller handles null as "player is outside" + ``` + +- [ ] **Confirm the fallback for `physicsRoot == null` in `GameWindow.cs:7162–7166`:** + when `CellGraph.CurrCell` is null (pre-spawn), `physicsRoot` is null, and + `ComputeVisibilityFromRoot(null, …)` returns null — the outdoor path runs, which is + correct (pre-spawn, no indoor draws). No additional change needed here. + +- [ ] **Delete (or comment-out-for-reference, then delete in the same commit) the + `FindCameraCell` method** (`CellVisibility.cs:389–446`) and the grace-frame counter + (`_cellSwitchGraceFrames` field, `_lastCameraCell` field). If `ComputeVisibility` (the + non-root variant, line 521) has other callers, check via Grep first — if it is ONLY + called from `ComputeVisibilityFromRoot`, delete it too; otherwise stub it to call + `GetVisibleCellsFromRoot(null, …)` and note the debt. + + **Confirm in Step 1:** search for all callers of `FindCameraCell` and + `ComputeVisibility` in the App project before deleting. Expected: zero callers outside + `CellVisibility.cs` itself and the one `GameWindow.cs` chain. If any other callers exist + note them and do not delete until they are migrated. + +Commit: `refactor(render): Stage 3 — delete FindCameraCell AABB grace-frame fallback` + +--- + +### T3.2 — Port `seen_outside` terrain/sky gate (retail `CellManager::ChangePosition`) + +**The retail policy (verified, `pseudo_c:94649`):** +``` +if (seen_outside || keep_lscape_loaded) + keep landscape + terrain +else + LScape::release_all (dungeon) +``` + +The current acdream code uses `cameraInsideCell` (any non-null `CameraCell` from +`ComputeVisibilityFromRoot`) to gate both sky and terrain. This has two problems: +1. It gates sky/terrain on "camera is inside ANY cell" — not on `seen_outside`. A dungeon + cell has `SeenOutside = false` but `cameraInsideCell = true`, and the current code + suppresses sky (correct for dungeon) but does NOT actively gate terrain for a dungeon + that has accidentally non-empty `OutsideView` (edge case). +2. Sky and terrain should be gated SEPARATELY: `seen_outside` controls whether terrain is + available at all; the `OutsideView` from the BFS controls whether it draws THIS frame. + +- [ ] **Extract `seenOutside` from the PVS root cell.** After the + `ComputeVisibilityFromRoot` call at `GameWindow.cs:7166`, add: + ```csharp + // Retail CellManager::ChangePosition:94649 — keep landscape iff seen_outside. + bool rootSeenOutside = physicsRoot?.SeenOutside ?? true; // outdoor root (null) → always seen_outside + ``` + (`LoadedCell.SeenOutside` is already populated at `GameWindow.cs:5718` from + `envCell.Flags.HasFlag(EnvCellFlags.SeenOutside)`.) + +- [ ] **Replace the `playerInsideCell` AABB scan with `seenOutside`-derived logic.** + Currently (`GameWindow.cs:7185–7187`): + ```csharp + bool playerInsideCell = (_playerMode && _playerController is not null) + ? _cellVisibility.IsInsideAnyCell(_playerController.Position) + : cameraInsideCell; + ``` + This calls `IsInsideAnyCell` (brute-force AABB scan of all cells every frame). Replace + with: `bool playerInsideCell = cameraInsideCell && !rootSeenOutside;` — i.e., player + is "fully indoor" (no sky/sun) only when inside a cell AND that cell cannot see outside + (a dungeon). Building interiors with `seen_outside` keep the sun because the sky is + visible through the door. Document the retail anchor (`CellManager::ChangePosition + @ 0x004559b0, pseudo_c:94649`). **Confirm in Step 1**: verify that `UpdateSunFromSky` + (called at `GameWindow.cs:7202`) behaves correctly with this new semantics — `true` = + kill sunlight (dungeon); `false` = keep sunlight (outdoor or building interior). + If the sun logic inverts this flag, invert accordingly. + +- [ ] **Replace `bool renderSky = !cameraInsideCell` (`GameWindow.cs:7267`) with:** + ```csharp + // Sky suppressed when inside a sealed interior (no exit portals). Building interiors + // with seen_outside still draw sky — it appears through the doorway via OutsideView + // (Stage 4). Outdoor root: always render sky. Retail RenderNormalMode:92649. + bool renderSky = !cameraInsideCell || rootSeenOutside; + ``` + For a building interior: `cameraInsideCell = true`, `rootSeenOutside = true` → + `renderSky = true` (sky is drawn, clipped to the doorway by Stage 4). + For a dungeon: `cameraInsideCell = true`, `rootSeenOutside = false` → + `renderSky = false`. + For outdoor: `cameraInsideCell = false` → `renderSky = true`. + **Note:** This means sky now renders indoors for `seen_outside` cells. Until Stage 4 + is landed, sky will draw full-screen in building interiors (wrong but expected interim + regression). Stage 4's `OutsideView` clipping will confine it to the doorway opening. + Document this as an expected interim state; do not gate on a TODO flag. + +- [ ] Similarly, update the weather gate at `GameWindow.cs:7506` (`if (!cameraInsideCell)`) + to use `renderSky` instead, so rain also follows the same policy. + +Commit: `feat(render): Stage 3 — seen_outside terrain/sky gate per CellManager::ChangePosition` + +--- + +### T3.3 — Camera-offset child-cell lookup (retail `find_visible_child_cell`) + +**Context:** In 3rd-person chase mode, the camera eye drifts outside the player cell +(U.4c). The render root is already pinned to the player cell (`visRootPos` at +`GameWindow.cs:7153`). This task ensures that when the camera is slightly outside the +player's cell, we use a graph/BSP child lookup rather than an AABB reclassification to +find the camera's cell for the projection. + +**Retail anchor:** `CEnvCell::find_visible_child_cell @ 0x0052dc50 (pseudo_c:311397)` — +walks the cell's `stab_list` looking for the first cell whose `PointInCell` contains the +query point. It is used by `AdjustPosition` (pc:280028) for the camera position, not a +fresh AABB scan. + +**Current state:** `GameWindow.cs:7153–7165` already roots the BFS at the PLAYER cell +(U.4c). The AABB `FindCameraCell` (now deleted by T3.1) is the old camera-cell resolver. +After T3.1, there is no AABB camera reclassification. The player's `CurrCell` is used as +the PVS root, and that is all we need for the portal traversal. + +- [ ] **Add `CellGraph.FindVisibleChildCell(Vector3 worldPoint)` to `CellGraph.cs` + (new method, ~10 lines):** + ```csharp + /// Retail find_visible_child_cell (pseudo_c:311397): walk CurrCell's StabList + self, + /// return the first EnvCell whose PointInCell is true for worldPoint. + /// Used to resolve the camera cell in 3rd-person from the physics cell graph, + /// NOT from a fresh AABB scan. + public EnvCell? FindVisibleChildCell(uint rootId, Vector3 worldPoint) + { + if (!_envCells.TryGetValue(rootId, out var root)) return null; + if (root.PointInCell(worldPoint)) return root; + foreach (var stabId in root.StabList) + if (_envCells.TryGetValue(stabId, out var stab) && stab.PointInCell(worldPoint)) + return stab; + return null; + } + ``` + File: `src/AcDream.Core/World/Cells/CellGraph.cs` (append after `GetVisible`). + +- [ ] **Wire it in `GameWindow.cs`**: After W2a lands, the only use of `FindCameraCell` + was for the `physicsRoot` FALLBACK. That fallback is now deleted (T3.1). The + `FindVisibleChildCell` is used for the VISUAL projection override when the chase camera + is outside the player cell — for example, to drive `envCellViewProj` from the correct + cell for portal-side tests in `PortalVisibilityBuilder.Build`. Currently `visRootPos` + (the player pos) is passed as the side-test anchor (already correct per U.4c). This + task is **low-risk**: the projection is driven from `camera.View * camera.Projection` + (the actual eye), which is unchanged. The side-test anchor (`visRootPos`) is already + the player pos. **Confirm in Step 1:** verify no current code site calls + `FindCameraCell` for the camera projection. If there is none, this T3.3 may reduce to + the `FindVisibleChildCell` method addition only (needed by Stage 4's camera-outside-door + scenario). + +Commit: `feat(core): Stage 3 — CellGraph.FindVisibleChildCell (retail find_visible_child_cell)` + +--- + +### T3.4 — Unit tests for render-root selection + +File: `tests/AcDream.Core.Tests/Rendering/CellGraphRootTests.cs` (new). + +- [ ] **Test: `RootSelection_OutdoorRoot_NullCurrCell_ReturnsFalseSeenOutside`** + When `CurrCell == null` (pre-spawn), `seenOutside = true` (outdoor default), + `renderSky = true`, `playerInsideCell = false`. + +- [ ] **Test: `RootSelection_BuildingInterior_SeenOutside_SkyRendered`** + `CurrCell = EnvCell with SeenOutside=true` → `rootSeenOutside = true` → + `renderSky = true`, `playerInsideCell = false`. + +- [ ] **Test: `RootSelection_Dungeon_NoSeenOutside_SkyNotRendered`** + `CurrCell = EnvCell with SeenOutside=false` → `rootSeenOutside = false` → + `renderSky = false`, `playerInsideCell = true`. + +- [ ] **Test: `FindVisibleChildCell_PlayerCellContains_ReturnsPlayerCell`** + Player in cell A; query point inside A → returns A. + +- [ ] **Test: `FindVisibleChildCell_StabListContains_ReturnsNeighbour`** + Player in cell A with StabList=[B]; query point outside A but inside B → returns B. + +- [ ] **Test: `FindVisibleChildCell_NeitherContains_ReturnsNull`** + Query point outside all cells → null. + +These tests are pure-logic (no GL). Run: `dotnet test --filter +"FullyQualifiedName~CellGraphRootTests" -c Debug`. + +Commit: `test(render): Stage 3 — CellGraphRootTests` + +--- + +### Stage 3 visual gate + +After T3.1–T3.4 green + `dotnet build` green: + +> Ask the user to launch the client, walk to the Holtburg cottage, enter it, and confirm: +> - **Outdoor**: terrain + sky draw as before. No regression. +> - **Building interior (seen_outside=true)**: walls/floors render; sky may draw +> full-screen (interim regression until Stage 4); no strobe between indoor/outdoor state. +> - **Dungeon** (if accessible): no sky, no terrain; walls render. +> - Cell ping-pong `0170↔0031` is gone (already fixed by Stages 1–2; confirm no +> regression). + +--- + +## Stage 4 — PView traversal + seamless seal + +**This is the big one.** Goal: draw landscape through exit portals clipped to the doorway +(kills the blue hole); seal ceilings; fix the `EnvCellRenderer` GL_BLEND regression. +Visual gate: interior sealed, sky/rain through the door, no blue-hole, no transparent walls. + +**Retail anchors:** +- `PView::ConstructView @ 0x005a57b0 (pseudo_c:433750)` — BFS, already ported to + `PortalVisibilityBuilder.Build`. +- `PView::DrawCells @ 0x005a4840 (pseudo_c:432709)` — `if outside_view.view_count > 0 → + LScape::draw first; conditional Z-clear (NOT color); then draw indoor cells`. +- `PView::DrawInside @ 0x005a5860 (pseudo_c:433793)` — top-level indoor entry; calls + `ConstructView` then `DrawCells`. +- `PView::ClipPortals @ 0x005a5520 (pseudo_c:433662)` — exit portal → + `outside_view.view_count += 1`, clip region registered. +- `SmartBox::RenderNormalMode @ 0x00453aa0 (pseudo_c:92635,92667)` — if `seen_outside`, + call `LScape::update_viewpoint(get_outside_cell_id(&viewer))` BEFORE `DrawInside`. + +**Current state:** +- `PortalVisibilityBuilder.Build` already produces `frame.OutsideView` with exit-portal + clip polygons (`PortalVisibilityBuilder.cs:163–175`). +- `ClipFrameAssembler.Assemble` already extracts `OutsideView` planes and calls + `clipFrame.SetTerrainClip(outsidePlanes)` when planes exist (the binding=2 terrain UBO). +- `GameWindow.cs:7406–7431` already has the `TerrainClipMode.Skip/Scissor/Planes` terrain + gate — terrain is SKIPPED when the player is indoor and `OutsideView` is empty. +- **GAP:** When `OutsideView` is non-empty (exit portal visible from inside), terrain and + sky should draw (clipped to the doorway). Currently sky is still suppressed by + `renderSky = !cameraInsideCell` (fixed in T3.2), but the terrain is gated correctly + (Planes mode if OutsideView has planes). However, sky is drawn BEFORE the portal frame + is assembled, so even with T3.2's `renderSky = true`, the sky draws FULL-SCREEN before + the clip bracket — it is not clipped to the doorway opening. + +### T4.1 — Move sky draw inside the portal-clip bracket + +**Problem:** Sky (`_skyRenderer?.RenderSky`) draws at `GameWindow.cs:7268–7275`, BEFORE +the clip bracket (`glEnable(ClipDistance)` at line ~7387). This means sky is never +clipped by the `gl_ClipDistance` planes, so it bleeds full-screen indoors. + +**Fix:** Move the sky draw to AFTER the terrain draw but BEFORE the entity draw, inside +the clip bracket. Sky must write to depth so entities z-test against it. + +- [ ] **Read `GameWindow.cs:7255–7292`** (sky draw block) and `7377–7390` (clip bracket + open). Confirm current order: sky → `IsLiveModeWaitingForLogin` goto → clip bracket + opens → terrain → cells → entities. + +- [ ] **Move `_skyRenderer?.RenderSky(...)` and the `SkyPreScene` particle pass to + inside the clip bracket**, just before the terrain draw (`GameWindow.cs:~7405`). + Structure becomes: + ``` + [clip bracket opens: glEnable(ClipDistance)] + sky draw (RenderSky + SkyPreScene particles) — now inside clip so gated to OutsideView + terrain draw + EnvCellRenderer opaque + entity dispatcher + EnvCellRenderer transparent + [clip bracket closes] + particles (scene pass, not sky) + weather + SkyPostScene (still outside-gated by renderSky) + ``` + **Confirm in Step 1:** verify `gl_ClipDistance` writes in `sky.vert` — if the sky + shader does NOT write `gl_ClipDistance[i]`, enabling clip while sky draws is harmless + (undefined behavior for an unwritten clip plane yields no clipping per the spec note + in the code comment at `GameWindow.cs:~1097`). However, for the sky to be clipped TO + the doorway we NEED the sky shader to write `gl_ClipDistance`. Check + `src/AcDream.App/Rendering/Shaders/sky.vert` — if it does not write clip distances, + add them mirroring `mesh_modern.vert`'s pattern. If that is too large a shader change + for Stage 4, use the Scissor fallback: before the sky draw, set a scissor rectangle + from `clipAssembly.TerrainScissorNdcAabb` (already computed for terrain), draw sky, + then disable scissor. The scissor approach is simpler and works without shader changes. + +Commit: `feat(render): Stage 4 — move sky draw inside portal-clip bracket` + +--- + +### T4.2 — Landscape viewpoint pre-position (retail `LScape::update_viewpoint`) + +**Retail:** Before `DrawInside`, when `seen_outside`, retail calls +`LScape::update_viewpoint(lscape, Position::get_outside_cell_id(&viewer))` (`RenderNormalMode:92667`). +This sets the terrain system's viewpoint to the OUTDOOR landcell corresponding to the +indoor camera position — so the terrain, when drawn through the doorway, is correctly +positioned. + +**Current state:** Terrain draw at `GameWindow.cs:7425/7430` calls +`_terrain?.Draw(camera, frustum, …)`. The terrain system's viewpoint is managed by +`LandblockStreamer` based on the player's landblock (streaming center). This is +separate from the per-frame projection the terrain shader sees. + +- [ ] **Confirm in Step 1:** does `TerrainModernRenderer.Draw` use the camera's + `view * projection` matrix from the `camera` argument, or a separately-tracked + "landscape viewpoint"? If the shader reads the matrix from the scene UBO (which has + the real camera view-proj), no change is needed — the terrain already projects from the + correct eye position. If there is a separate "landscape eye" or terrain-specific + viewpoint that gets reset to the player position rather than the camera position, it + needs to be set to `Position.get_outside_cell_id(&viewer)` equivalent before the draw. + Most likely acdream does NOT have this divergence (the terrain uses `camera.View * + camera.Projection` just like everything else), so this task may be a no-op. Confirm + and either add a `// verified: no viewpoint override needed` comment or fix it. + +Commit (or no-op annotation): `docs(render): Stage 4 — verify terrain viewpoint is camera-relative` + +--- + +### T4.3 — Conditional Z-clear for the doorway (retail `DrawCells:432731`) + +**Retail:** After drawing the landscape (terrain + sky), retail does a CONDITIONAL Z-clear: +```c +if (forceClear || D3DPolyRender::portalsDrawnCount != 0) + RenderDevice->Clear(4, 0x820fc0, 1.0); // flag 4 = Z-buffer ONLY, NOT color +``` +This clears depth ONLY WHERE the portal geometry was drawn (exit portal polygons), so +indoor geometry draws on top of the landscape without z-fighting through the doorway. +It is a Z-clear, NOT a color-clear — so there is no "blue hole" painted. + +**Current state:** acdream does not perform this clear. The current terrain `TerrainClipMode.Skip` +path means terrain never draws indoors, so z-fighting has not been observed. Once terrain +draws through the doorway (T4.1 fixed), z-fighting may appear at the doorway plane. + +- [ ] **After the terrain/sky draw block (after T4.1's sky+terrain), add a conditional + depth-only clear gated on `clipAssembly is not null && frame.OutsideView.Polygons.Count > 0`:** + ```csharp + // Retail PView::DrawCells:432731 — conditional Z-clear after landscape, before indoor geometry. + // Clears depth in the doorway region so indoor walls draw over the terrain. + // Z-buffer only (glClear(GL_DEPTH_BUFFER_BIT)) — NOT color — so no blue hole. + if (clipAssembly is not null && pvFrame.OutsideView.Polygons.Count > 0) + { + // Scissor to the doorway AABB before clearing to avoid clearing depth for + // the entire screen (only the exit-portal region needs it). + var fb = _window!.FramebufferSize; + float nx0 = System.Math.Clamp(terrainScissorNdc.X, -1f, 1f); + float ny0 = System.Math.Clamp(terrainScissorNdc.Y, -1f, 1f); + float nx1 = System.Math.Clamp(terrainScissorNdc.Z, -1f, 1f); + float ny1 = System.Math.Clamp(terrainScissorNdc.W, -1f, 1f); + int px = (int)System.MathF.Floor((nx0 * 0.5f + 0.5f) * fb.X); + int py = (int)System.MathF.Floor((ny0 * 0.5f + 0.5f) * fb.Y); + int pw = (int)System.MathF.Ceiling((nx1 - nx0) * 0.5f * fb.X); + int ph = (int)System.MathF.Ceiling((ny1 - ny0) * 0.5f * fb.Y); + _gl.Enable(EnableCap.ScissorTest); + _gl.Scissor(px, py, (uint)System.Math.Max(1, pw), (uint)System.Math.Max(1, ph)); + _gl.Clear(ClearBufferMask.DepthBufferBit); + _gl.Disable(EnableCap.ScissorTest); + } + ``` + This requires `pvFrame` to be in scope. Currently `pvFrame` is declared inside the + `if (clipRoot is not null)` block (`GameWindow.cs:7315`). Either hoist it or use + `clipAssembly.HasOutsideView` (a property to add to `ClipFrameAssembly` if needed). + **Confirm in Step 1:** check whether `ClipFrameAssembly` already exposes + `OutsideView.Polygons.Count` or an equivalent. Add if absent. + +Commit: `feat(render): Stage 4 — conditional Z-clear at doorway portal (retail DrawCells:432731)` + +--- + +### T4.4 — Verify and document ceilings-are-capped (no code change expected) + +**Retail:** Ceilings are capped BY CONSTRUCTION — each EnvCell's `drawing_bsp` is a +closed mesh (floor, 4 walls, ceiling authored in the dat). There is no explicit "cap +ceiling" step. `EnvCellRenderer.Render` draws each visible cell's mesh. + +- [ ] **Confirm in Step 1:** Verify that `EnvCellRenderer.RegisterCell` stores the full + cell mesh including ceiling polygons (not just floor + walls). The dat baking happens + in `GameWindow.cs:5456–5502` (the `BuildLoadedCell`/`CellMesh.Build` call). If ceiling + polygons are included in `CellMesh.Build`, the ceiling is capped. If they are + explicitly excluded, add them. + +- [ ] **Add a comment** in `EnvCellRenderer.Render` documenting that ceiling is present + by construction (retail `DrawCells:432745`, `cell->structure->drawing_bsp` draws all + cell surfaces including ceiling). No code change if already correct. + +Commit (or annotation): `docs(render): Stage 4 — document ceiling sealed by EnvCell dat` + +--- + +### T4.5 — Verify `EnvCellRenderer` GL_BLEND fix is complete and extends to the ceiling pass + +**Current state (ALREADY FIXED):** `EnvCellRenderer.cs:1004–1023` already sets +`_gl.Disable(EnableCap.Blend); _gl.DepthMask(true)` for the opaque pass and +`_gl.Enable(EnableCap.Blend); _gl.DepthMask(false)` for the transparent pass. This fix +is the U.4 root-cause fix for the transparent-walls regression (noted in the comment at +`EnvCellRenderer.cs:1004`). + +- [ ] **Verify the fix covers the call order after T4.1's sky-inside-clip-bracket + change.** After T4.1, the order is: sky → terrain → `EnvCellRenderer.Render(Opaque)` + → entity dispatcher → `EnvCellRenderer.Render(Transparent)`. Sky may leave GL state + dirty. The fix at line 1004 already re-establishes Blend + DepthMask at the TOP of + `Render(…)`, so it is robust against any preceding state. Confirm no new state is + needed (e.g. `DepthFunc`, `CullFace` — these are set at `GameWindow.cs:7030` before + the clip bracket). + +- [ ] **Add a `_gl.DepthMask(true)` reset at the END of `EnvCellRenderer.Render`** if + the transparent pass leaves `DepthMask(false)`. Currently the code at line 1012 says + "Restored to opaque defaults at the end of the draw loop" — verify this restoration + actually exists (search for the matching `_gl.DepthMask(true)` after the draw loop in + `EnvCellRenderer.cs:1025–1239`). If missing, add it before the method returns. + +Commit: `fix(render): Stage 4 — verify EnvCellRenderer GL state restoration after transparent pass` + +--- + +### T4.6 — Unit tests for PView BFS + OutsideView behavior + +File: `tests/AcDream.Core.Tests/Rendering/PViewBfsTests.cs` (new; tests use +`PortalVisibilityBuilder.Build` directly with synthetic `LoadedCell` fixtures). + +- [ ] **Test: `OutsideView_NonEmpty_WhenExitPortalVisible`** + Construct a `LoadedCell` with one portal whose `OtherCellId = 0xFFFF` (exit portal) + and a non-degenerate polygon facing the camera. Call `PortalVisibilityBuilder.Build` + from a camera position on the interior side. Assert `frame.OutsideView.Polygons.Count > 0`. + +- [ ] **Test: `OutsideView_Empty_WhenNoExitPortal`** + A cell with portals connecting to other interior cells only (`OtherCellId != 0xFFFF`). + Assert `frame.OutsideView.Polygons.Count == 0`. + +- [ ] **Test: `VisibleSet_ContainsRootCell_Always`** + Any cell graph → root cell is always in `frame.OrderedVisibleCells` and is first. + +- [ ] **Test: `VisibleSet_MultiCell_OrderedClosestFirst`** + Root cell with portal to neighbour farther away → root appears at index 0. + +- [ ] **Test: `BFS_Terminates_OnCyclicPortalGraph`** + Root A→B, B→A (cycle). BFS must terminate with exactly 2 cells in `OrderedVisibleCells` + (no infinite loop). This is the #95 dungeon-blowup guard. + +Run: `dotnet test --filter "FullyQualifiedName~PViewBfsTests" -c Debug`. + +Commit: `test(render): Stage 4 — PViewBfsTests (OutsideView + BFS termination)` + +--- + +### Stage 4 visual gate + +After T4.1–T4.6 green + `dotnet build` green: + +> Ask the user to launch the client at Holtburg cottage: +> - **Through the doorway from inside**: sky + rain visible in the doorway opening, +> NOT full-screen. Outside terrain/buildings visible through the door. +> - **No blue clear-color hole** in the doorway. +> - **Ceiling is present** (no "no ceiling" regression). +> - **Walls are opaque** (the transparent-walls regression is not re-introduced). +> - **Dungeon** (if accessible): no terrain, no sky, walls/floors render — PVS BFS +> converges without blowup. + +--- + +## Stage 5 — Entity / particle cell clipping + +**Goal:** Clip entities/particles to the PView visible cell set, not the world frustum. +Kills NPC/door/smoke bleed-through. + +**Retail anchor:** +- `PView::DrawCells @ pseudo_c:432868–432882` — iterates `cell_draw_list` and for each + calls `DrawObjCellForDummies(cell)`. Objects in a non-visible cell are never iterated. +- `CObjCell::object_list` — objects live in their cell's object list (from `enter_cell`/ + `leave_cell`). Entity→cell membership comes from the physics shadow lists. + +**Current state:** `WbDrawDispatcher.Draw` at `GameWindow.cs:7473–7476` already takes +`visibleCellIds: visibility?.VisibleCellIds`. The `WbDrawDispatcher` uses this to gate +entity draws (ParentCellId ∈ visibleCellIds). This is largely correct already. The +gap: entities OUTSIDE a visible cell but INSIDE the world frustum may still draw. + +### T5.1 — Verify entity-clip path is fully wired + +- [ ] **Grep `WbDrawDispatcher.Draw` signature and its `visibleCellIds` parameter usage.** + Find `src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs`, search for `visibleCellIds`. + Confirm: when `visibleCellIds != null`, entities whose `ParentCellId` is NOT in the set + are culled. If not: add the cull inside the dispatcher's entity loop. + +- [ ] **Confirm `ParentCellId` is populated correctly for EnvCell-static objects.** + Static objects inside a cell (inn furniture, door) have `ParentCellId` set to the + cell's id. Verify at `GameWindow.BuildInteriorEntitiesForStreaming` or the matching + `AddEntitiesToExistingLandblock` site that `ParentCellId` is set. If null or zero for + static objects, the cell-clip gate misses them. + +- [ ] **Use `OrderedVisibleCells` instead of `VisibleCellIds` for entity ordering.** + `visibility.VisibleCellIds` is a `HashSet` (unordered). For strict retail-faithful + entity draw ordering, the dispatcher should iterate in `frame.OrderedVisibleCells` order + (closest first). This is an enhancement, not a blocker. Add a TODO comment if deferred. + +Commit: `fix(render): Stage 5 — verify entity-clip visibleCellIds gate in WbDrawDispatcher` + +--- + +### T5.2 — Particle cell clipping + +**Current state:** `_particleRenderer.Draw(_particleSystem, camera, camPos, +AcDream.Core.Vfx.ParticleRenderPass.Scene)` at `GameWindow.cs:7494–7496` does NOT take +a `visibleCellIds` filter. + +- [ ] **Check if `ParticleRenderer.Draw` has a cell-filter parameter.** If yes, pass + `visibility?.VisibleCellIds`. If no, add it — particles in an invisible cell (NPC + smoke from behind a sealed wall) should not draw. + +- [ ] **Gate `SkyPreScene` and `SkyPostScene` particle passes by `renderSky`** (already + gated at `GameWindow.cs:7506` for weather; confirm `SkyPreScene` at line 7272 is also + gated by `renderSky`). This is the particle equivalent of Stage 3's sky gate. + +Commit: `fix(render): Stage 5 — particle cell-clip via visibleCellIds` + +--- + +### T5.3 — Unit tests for entity/particle cell-clip predicate + +File: `tests/AcDream.Core.Tests/Rendering/EntityClipTests.cs` (new). + +- [ ] **Test: `EntityClip_ParentInVisibleSet_IsIncluded`** + Entity with `ParentCellId = 0xA9B40170` (the cottage cell); `visibleCellIds = + {0xA9B40170}` → entity included. + +- [ ] **Test: `EntityClip_ParentNotInVisibleSet_IsExcluded`** + Entity with `ParentCellId = 0xA9B40172` (sealed room not in PVS); `visibleCellIds = + {0xA9B40170}` → entity excluded. + +- [ ] **Test: `EntityClip_NullVisibleSet_IncludesAll`** + `visibleCellIds = null` (outdoor root) → all entities included (no gate). + +Run: `dotnet test --filter "FullyQualifiedName~EntityClipTests" -c Debug`. + +Commit: `test(render): Stage 5 — EntityClipTests` + +--- + +### Stage 5 visual gate + +> Ask the user to launch and walk to the Holtburg cottage door: +> - **NPC just outside the door**: should be visible through the door when looking out, +> but NOT visible through the wall from inside looking sideways (not through the portal +> opening). +> - **Smoke/particles from objects in invisible cells** do not bleed through walls. +> - **No regression** on entity visibility in outdoor scenes (all entities visible when +> `visibleCellIds` is null). + +--- + +## Final acceptance — combined visual gate + +After ALL stages green + full `dotnet test -c Debug` green: + +> Ask the user to verify the full acceptance criteria (spec §6): +> +> **Cottage (Holtburg):** +> - [ ] Walk through the doorway: no strobe (Stage 1–2, already done). +> - [ ] Inside the cottage: interior sealed (walls, floor, ceiling all present). +> - [ ] Looking at the door from inside: sky + rain visible through the doorway opening, +> NOT full-screen. No blue clear-color hole. +> - [ ] No transparent walls. No terrain bleeding through the floor. +> - [ ] No entity/particle bleed through sealed walls. +> +> **Dungeon (if accessible):** +> - [ ] No terrain, no sky (sealed dungeon). +> - [ ] Walls/floors/ceilings render. Portal traversal converges (no FPS drop from BFS +> blowup — issue #95 confirmed bounded by `seen.Add` gate). +> +> **`dotnet build` green.** +> **`dotnet test -c Debug` — full suite green** (or the documented static-leak subset +> fixed by T0.1; no new deterministic failures vs pre-Phase-W baseline). + +--- + +## Roadmap update (implementer: do this in the same commit that clears the final visual gate) + +- [ ] Update `docs/plans/2026-04-11-roadmap.md` — move Phase W to "shipped" with the + commit SHA. +- [ ] Update CLAUDE.md "Currently working toward" section to the next milestone. +- [ ] Add a memory note to `memory/` if there are durable lessons (e.g. the sky-inside- + clip-bracket pattern, the Z-clear for doorway depth). + +--- + +## Self-review + +### Spec coverage + +| Spec §2 item | Covered by | +|---|---| +| "Membership is transition-owned" | Stage 1–2 (already done/shipped) | +| "Render roots at `CurrCell` + `seen_outside`" | T3.1, T3.2 | +| "Camera offset via graph/BSP child lookup" | T3.3 | +| "One PView portal traversal, `OutsideView`" | T4.1–T4.5 (PortalVisibilityBuilder already exists; Stage 4 wires the draw consequences) | +| "`MaxReprocessPerCell` → `update_count` watermark" | Already done in `PortalVisibilityBuilder.cs:74–84` (`seen` HashSet = enqueue-once, equivalent to `cell_view_done`). No code change. | +| "Draw landscape through exit portals; no blue hole" | T4.1 (sky inside clip), T4.2 (terrain viewpoint), T4.3 (Z-clear) | +| "Cap ceilings" | T4.4 (verification only — already capped by construction) | +| "`EnvCellRenderer` GL_BLEND fix" | T4.5 (already shipped in U.4; verify + extend) | +| "Entity/particle clip to PView visible set" | T5.1, T5.2 | +| "Dungeon: no terrain/sky" | T3.2 (`seen_outside=false → renderSky=false`), T4.6 test | + +### Placeholder scan + +All "Confirm in Step 1" items are explicitly labeled and have a concrete investigative +action. They are marked as genuinely-uncertain integration points because the exact +behavior depends on current shader and method bodies that would require reading additional +files. They are NOT implementation-blocking — each has a fallback or a "no-op if already +correct" path. + +### Type consistency + +- `physicsRoot` is `LoadedCell?` (render type) — correct; `CellGraph.CurrCell` is + `ObjCell?` (Core type), and the conversion at `GameWindow.cs:7163–7165` (TryGetCell) + bridges them. No new type conversions introduced. +- `SeenOutside` on `LoadedCell` is `bool` (line 104 of `CellVisibility.cs`) — used as + `physicsRoot?.SeenOutside ?? true` which is null-safe with the correct outdoor default. +- `OutsideView.Polygons.Count` returns `int` (a `List.Count`) — used in + both `int > 0` checks and `ReadOnlySpan` plane extraction. No type mismatch. + +### Stage 4 scope note + +Stage 4 is large but all the underlying machinery (BFS, clip planes, terrain UBO, +`ClipFrameAssembler`) is already in production. The stage is 5 concrete tasks (T4.1–T4.5) +plus tests (T4.6). T4.1 (move sky inside clip bracket) is the most impactful; T4.3 (Z- +clear) is the "no blue hole" fix; T4.4/T4.5 are verification. Estimated total: 2–3 hours +for an implementer who has read this plan and the prerequisite files. + +### Issue #102 note + +Issue #102 (portal-graph BFS termination — too many reprocesses) is already closed by the +`seen` HashSet in `PortalVisibilityBuilder.cs:84` (enqueue-once guarantee). No additional +work needed for this plan. If the dungeon BFS still blows up in practice (Stage 4 visual +gate), investigate whether `seen.Add(neighbourId)` is being bypassed; it should not be.