diff --git a/docs/superpowers/specs/2026-05-31-phase-u4c-stabilize-portal-visibility-design.md b/docs/superpowers/specs/2026-05-31-phase-u4c-stabilize-portal-visibility-design.md new file mode 100644 index 0000000..1389f1f --- /dev/null +++ b/docs/superpowers/specs/2026-05-31-phase-u4c-stabilize-portal-visibility-design.md @@ -0,0 +1,339 @@ +# Phase U.4c — Stabilize portal visibility (fix the threshold "flap") — design spec + +**Status:** design approved 2026-05-31 (brainstorm). +**Milestone:** M1.5 — "Indoor world feels right." +**Predecessor:** Phase U (unified retail-faithful render pipeline) shipped through U.4; indoor +rendering visually verified correct. This is the one residual. +**Handoff / decision context:** +[`docs/research/2026-05-30-phase-u4-shipped-and-flap-handoff.md`](../../research/2026-05-30-phase-u4-shipped-and-flap-handoff.md); +parent spec [`docs/superpowers/specs/2026-05-30-phase-u-unified-render-pipeline-design.md`](2026-05-30-phase-u-unified-render-pipeline-design.md). + +--- + +## 1. The problem (recap, precisely root-caused) + +Crossing a Holtburg cottage doorway (cellar → ground floor → outside), terrain + building-shells +briefly vanish, leaving only un-gated geometry (particles + live entities) over the bluish clear +color. This is the **"flap."** + +It is **not** a mystery. `ACDREAM_PROBE_VIS=1` `[vis]` lines for the *same* camera cell across +adjacent frames: + +``` +root=0xA9B40171 cells=4 ids=[...,0xA9B40170] outside(polys=1,planes=4) ← window cell reached → terrain draws +root=0xA9B40171 cells=3 ids=[0xA9B40171,75,74] outside(polys=0,planes=0) ← window cell dropped → terrain SKIPPED +``` + +Over one cellar traversal: **10 empty-`OutsideView` frames interleaved with 16 non-empty**, for +the same cells. The ground-floor cell `0xA9B40170` (which holds the window / `0xFFFF` exit portal) +**flickers in and out of the visible set** as the camera moves a few centimetres. + +### Mechanism in our code + +[`PortalVisibilityBuilder.Build`](../../../src/AcDream.App/Rendering/PortalVisibilityBuilder.cs) +discovers the visible-cell **set** purely by a per-frame portal-graph walk. Each hop is gated by +`CameraOnInteriorSide` (line 237) — a hard plane-side test (`Dot(planeN, localCam)+D` vs a ±0.01 +epsilon). When the camera sits near a portal plane, a centimetre of motion flips that test; the +sole multi-hop chain reaching `0xA9B40170` breaks; the cell drops from the set; its `0xFFFF` +portal never contributes to `OutsideView`; +[`ClipFrameAssembler.Assemble`](../../../src/AcDream.App/Rendering/ClipFrameAssembler.cs) maps the +empty `OutsideView` to `TerrainClipMode.Skip` **and** `OutdoorVisible=false` (lines 173-179) → +terrain *and* building-shells flap off. + +The root is structural: **we rebuild the cell set every frame from a pose-brittle walk.** Retail +does not. + +--- + +## 2. Goal & non-goals + +**Goal.** Make the visible-cell **set** — and therefore `OutsideView` and the terrain-draw +decision — **stable** across camera pose, the retail way: ground it in the per-cell precomputed +PVS (`stab_list`) refreshed on cell entry. The per-frame portal-clip walk stays per-frame; it +refines *where* each cell draws, never *which* cells exist. The threshold crossing becomes seamless. + +**Non-goals (this phase).** +- Any functional change to indoor cell-shell / entity / terrain **rendering** (`EnvCellRenderer`, + `WbDrawDispatcher`, `TerrainModernRenderer`, `ClipFrame`/`ClipFrameAssembler`, the mesh/terrain + clip shaders, the two U.4 GL-state fixes). U.4c is visible-**set** stability only — it changes + *what feeds* those consumers, not the consumers. (The one exception is the explicitly-optional, + behaviour-neutral cosmetic sweep of `AppendSlot`'s 3-state collapse in §8, taken only if trivial.) +- **U.5** (outdoor-camera → building-interior peering) and **U.6** (dungeon-scale validation, + #95 / residual #102). Deferred, separately tracked. +- A hysteresis / last-frame-region band-aid. **Explicitly forbidden** (workaround). The set must + be made stable by construction, not smoothed after the fact. + +--- + +## 3. The retail oracle (what we port) + +All line numbers in `docs/research/named-retail/acclient_2013_pseudo_c.txt`; struct lines in +`docs/research/named-retail/acclient.h`. Read during the U.4c brainstorm; this is the evidence the +design rests on. + +### 3.1 The two stable anchors, set on cell entry — NOT per frame + +- **`CEnvCell::grab_visible_cells` (311878).** On the camera cell: `add_visible_cell(self)`, then + `add_visible_cell(stab_list[i])` for every stab; then `if (seen_outside == 0) return;` else + `LScape::grab_visible_cells(...)`. It populates the static `visible_cell_table` (which + `CEnvCell::GetVisible` reads) from the cell's `stab_list`, and keeps the landscape grabbed only + when `seen_outside`. +- **`CellManager::ChangePosition` (94601)** is the *only* caller, on **position/cell change** + (94646 / 94653 / 94659). The landscape keep-vs-release decision keys on the stable + `seen_outside` flag (94649 keep, 94658 `LScape::release_all`). So the PVS + the + landscape-loaded decision are refreshed on cell entry and held stable between entries. +- **`SmartBox::RenderNormalMode` (92649)** — top-level draw dispatch: the draw-landscape flag + `ebx_1 = (… || viewer_cell->seen_outside != 0)` reads the **stable** per-cell `seen_outside`, + then calls `DrawInside(viewer_cell)`. + +### 3.2 The per-frame view is *seeded* from the stable PVS + +`PView::DrawInside` (433793), the per-frame indoor entry, in order: +``` +CEnvCell::curr_view_push(arg2) // push a view accumulator on the camera cell +PView::add_views(this, num_stabs, stab_list) // SEED: push an accumulator on every PVS cell +… +ConstructView(this, arg2, 0xffff) // per-frame portal-clip walk (refine regions) +DrawCells(this, …) // draw; landscape iff outside_view non-empty +PView::remove_views(this, num_stabs, stab_list) // teardown +``` +- **`PView::add_views` (433382):** for each stab id, `cell = CEnvCell::GetVisible(id); if (cell) curr_view_push(cell)`. + It makes **every PVS cell a live participant** with its own view accumulator before the walk — + not just cells the walk reaches. (It seeds an *empty* accumulator, not full-screen.) + +### 3.3 The per-frame walk (what stays per-frame) + +- **`ConstructView` (433750):** `InitCell` → `InsCellTodoList(0)` → loop {pop nearest from the + distance-priority `cell_todo_list`, append to `cell_draw_list`, `ClipPortals`, `AddViewToPortals`}. +- **`InitCell` (432896):** per-portal **sidedness** classification (sets each portal's `seen` + flag) + nearest-vertex distance for the todo key. This is the same family as our + `CameraOnInteriorSide`. +- **`ClipPortals` (433572):** for each `seen && !inflag` portal, `GetClip` projects + clips the + opening against the cell's current view. If the portal is `0xFFFF` and `draw_landscape`, the + clipped region is `copy_view`-ed into `outside_view` (433662-433676). Otherwise `OtherPortalClip` + (reciprocal) then `copy_view` into the neighbour. +- **`AddViewToPortals` (433446):** enqueue a neighbour **on first discovery** (`ecx_5==0` → + `InitCell` + `InsCellTodoList`); on **view-growth** (`ecx_5 != view_count`) → `AddToCell` + + `FixCellList` (re-incorporate in place + advance the `update_count` watermark). This is the + fixpoint: a cell accumulates view from multiple incoming portals and is re-clipped when its + region grows. +- **`DrawCells` (432709):** landscape (`LScape::draw`) is drawn **iff + `outside_view.view_count > 0`** (432715). So retail's terrain decision is per-frame and keyed on + `outside_view` emptiness — **exactly like ours.** Retail does not flap because its `outside_view` + does not spuriously empty, *because the set it walks is grounded in the stable PVS.* + +### 3.4 Struct truth (resolves a decomp-name hazard) + +`acclient.h` `CEnvCell` (~30925): `unsigned int num_stabs; unsigned int *stab_list; int seen_outside;` +— three distinct fields. `seen_outside` is a genuine `int` boolean. (The Binary Ninja pseudo-C at +311044 assigns a `Frame` array to a field it *labels* `seen_outside`; that is BN aliasing a +different offset — cf. project memory `bn-decomp-field-names`. Trust `acclient.h`.) + +### 3.5 The data is already in our process + +Both stable inputs exist in-process today; only the render path drops them: +- **`stab_list`** = `envCell.VisibleCells` (`List`, landblock-local). + [`PhysicsDataCache`](../../../src/AcDream.Core/Physics/PhysicsDataCache.cs) already reads it into + full ids (lines 178-186) and notes it "reserved for the optional find_cell_list visibility filter." +- **`seen_outside`** = `envCell.Flags.HasFlag(EnvCellFlags.SeenOutside)` — a direct dat flag. + WB's `EnvCellLandblock.SeenOutsideCells` already tracks it; `tools/A8CellAudit/Program.cs:200` + already reads it. + +So U.4c is **plumbing existing data + grounding logic**, not new dat parsing and not guessing. + +--- + +## 4. Architecture — three layers + +``` + On cell entry (camera changes cell): + LoadedCell already hydrated with VisibleCells (PVS, full ids) + SeenOutside (Layer 1) + │ + Per frame: ▼ + PortalVisibilityBuilder.Build(cameraCell, …) + • seed participants from cameraCell.VisibleCells (Layer 2 — the add_views analog) + • closest-first portal-clip walk refines each region (unchanged) + • OutsideView accumulates exit-portal contributions + ▼ + ClipFrameAssembler.Assemble(...) (UNCHANGED — already consumes CellViews + OutsideView) + ▼ + ACDREAM_PROBE_VIS [vis] — now stable: OutsideView non-empty + narrowing, no polys=0/1 flap +``` + +Each layer is independently testable; the interfaces below are the contract. + +--- + +## 5. Components + +### 5.1 Layer 1 — `LoadedCell` carries the stable inputs (data plumbing) + +Add to [`LoadedCell`](../../../src/AcDream.App/Rendering/CellVisibility.cs): + +```csharp +/// The stab_list PVS as full (landblock-prefixed) cell ids — retail +/// CEnvCell.stab_list. The stable set of cells potentially visible from this cell, +/// precomputed by the AC content tools. Refreshed only at hydration (cell entry). +public IReadOnlyList VisibleCells = System.Array.Empty(); + +/// Retail CEnvCell.seen_outside: this cell sees the exterior (an exit portal +/// is reachable from it). Gates whether the landscape is drawn for this camera cell. +public bool SeenOutside; +``` + +Populated at the existing hydration site +([`GameWindow.cs:5696`](../../../src/AcDream.App/Rendering/GameWindow.cs:5696), the EnvCell-build +method) from data already on `envCell`: +- `VisibleCells` ← `envCell.VisibleCells` each OR-ed with the landblock mask + (`envCellId & 0xFFFF0000u`), identical to the prefix logic `PhysicsDataCache` already uses. +- `SeenOutside` ← `envCell.Flags.HasFlag(EnvCellFlags.SeenOutside)`. + +This adds two fields + ~4 lines to an existing method — within Code Structure Rule 1 ("a handful +of fields and a one-paragraph method to wire an extracted class in is fine"). No new dat read. + +### 5.2 Layer 2 — `PortalVisibilityBuilder` seeds from the PVS (the `add_views` analog) + +Before the portal-clip walk, the builder makes **every cell in `cameraCell.VisibleCells`** a +participant (a keyed entry in `CellViews` with a live accumulator), resolving each via the existing +`lookup`. The existing closest-first walk + `OtherPortalClip` then refine each participant's clip +region and accumulate `OutsideView` from exit portals. The window cell `0xA9B40170` is therefore +present every frame regardless of whether a transient `CameraOnInteriorSide` flip broke its +reaching chain. + +The builder signature and `PortalVisibilityFrame` shape are unchanged; the seeding is internal. +`ClipFrameAssembler`, `ClipFrame`, the shaders, `EnvCellRenderer`, and terrain are untouched — they +already consume `CellViews` / `OutsideView` correctly. + +**The one load-bearing detail, resolved in Task 1 (not guessed here):** retail's `add_views` seeds +*empty* accumulators (`curr_view_push`), and the fixpoint (`AddToCell`/`FixCellList` on view-growth, +§3.3) lets a cell accumulate region from multiple incoming portals and re-clip its outgoing +(including `0xFFFF`) portals when its view grows. Our current builder enqueues-once and unions +growth into `CellViews` **without re-clipping** the grown cell's outgoing portals +(`PortalVisibilityBuilder.cs:210-226`). Two sub-hypotheses for *why* retail's `OutsideView` stays +non-empty where ours empties: + +- **H1 — set grounding:** the window cell stays a participant via `add_views` / `visible_cell_table` + even when the per-frame chain breaks; once it is a guaranteed participant, its exit portal + contributes. Fix = seed participants from the PVS (+ port the growth re-clip so a multi-path + cell's exit portal re-contributes against its grown region). +- **H2 — stable side test:** retail's chain does not break because `InitCell`'s sidedness is + computed on a more stable quantity / convention than our `CameraOnInteriorSide`; fix = also make + our side test robust (epsilon / reciprocal-aware), with PVS grounding as the structural net. + +**Task 1 disambiguates these on a live `ACDREAM_PROBE_VIS` capture at the cottage threshold, +porting the exact `add_views` / `ClipPortals` / `AddToCell` / `FixCellList` semantics, BEFORE any +further wiring.** The implementation follows the evidence; both sub-hypotheses share the same +primary change (PVS grounding), so Task 1 is a refinement of *one* design, not a fork. + +### 5.3 Layer 3 — the terrain/shell decision is anchored + +With the set grounded (Layer 2), `OutsideView` empties only when genuinely no exit portal is in +view (facing a wall), never spuriously. The camera cell's stable `SeenOutside` is the +retail-faithful anchor (it matches `RenderNormalMode` / `grab_visible_cells` gating the landscape +*data*) and yields two falsifiable invariants for tests (§7): +1. A camera cell whose PVS contains no exit-portal cell and is not itself `SeenOutside` must + produce an empty `OutsideView` (terrain `Skip`) — terrain is correctly absent in a windowless + interior. +2. A `SeenOutside` camera cell crossing the threshold must produce a stable non-empty `OutsideView` + across pose — no `polys=0`/`polys=1` interleave. + +Layer 3 is an **anchor + test oracle**, not a new draw gate. We do **not** make terrain ignore +`OutsideView` (that would diverge from `DrawCells` §3.3). The flap is killed by making +`OutsideView` stable (Layer 2), not by floating the terrain decision off it. + +--- + +## 6. Error handling / safe direction + +- **Camera cell with empty / missing `VisibleCells`** (degenerate or pre-PVS dat): fall back to + today's pure-walk behaviour — no regression, no crash. +- **PVS cell not currently loaded** (`lookup` returns null): skip it (it cannot draw). +- **Genuinely degenerate exit-portal data** (the safe-direction backstop, *not* the common-case + mechanism): over-include (terrain draws slightly wide) rather than vanish — a vanish is the flap; + over-draw is benign under the depth test. This applies **only** to missing/degenerate data, never + as a substitute for the faithful grounding of §5.2. +- **8-plane cap / scissor fallbacks:** unchanged — already handled in + `ClipPlaneSet` / `ClipFrameAssembler`. + +--- + +## 7. Testing strategy + +- **Unit (GL-free), `PortalVisibilityBuilderTests`:** + - **Flap regression:** a synthetic cottage chain (camera cell → mid cell → exit cell with a + `0xFFFF` portal) where the mid→exit reaching path is deliberately broken by a back-facing + intermediate portal. Assert the exit cell stays in `OrderedVisibleCells` and `OutsideView` + stays non-empty. This is the RED→GREEN test for the flap. + - **PVS-empty fallback:** camera cell with empty `VisibleCells` → behaviour identical to the + current pure-walk builder (pin no regression). + - **`SeenOutside` invariants (§5.3):** windowless interior → empty `OutsideView`; threshold cell + → stable non-empty across a swept camera pose. +- **The real gate is visual + the runtime probe** (unit tests on synthetic data did not catch #103): + at the Holtburg cottage doorway, cellar → ground → out, from several angles and zooms — + `ACDREAM_PROBE_VIS=1` shows `OutsideView` non-empty and **narrowing** (no `polys=0`/`polys=1` + interleave for the same cell), and the threshold is **seamless**: no terrain or building-shell + flicker. **This is the acceptance gate.** +- **No regression** to the indoor case (walls solid, no terrain bleed) or the outdoor default. + +--- + +## 8. Implementation staging + +Build + test green at every stage. Branch: `claude/thirsty-goldberg-51bb9b` (continue; preserve the +two `git stash` entries). + +U.4c is entirely CPU-side (the builder + `LoadedCell` data); there is no new GPU/shader work — the +GPU gate shipped in U.3/U.4. So "validate before wiring" means: **characterize the flap and port +the retail semantics to pseudocode first; validate the implementation on a live `[vis]` capture +before declaring the builder correct.** + +| Stage | Deliverable | Gate | +|-------|-------------|------| +| **U.4c-1** | **Characterize + pseudocode (oracle first).** Live `ACDREAM_PROBE_VIS` capture of the *current* flap → confirm it is the set-drop and identify which portal/cell side-test flips (sharpens H1 vs H2, §5.2). Port `add_views` / `ClipPortals` / `AddToCell` / `FixCellList` to a short pseudocode note (grep→decompile→pseudocode→port). | Pseudocode note committed; capture confirms the set-drop mechanism | +| **U.4c-2** | **Layer 1** — `LoadedCell.VisibleCells` + `SeenOutside`; hydrate at GameWindow:5696. | Build green; fields populated (probe/unit) | +| **U.4c-3** | **Layer 2** — builder seeds participants from the PVS + the Task-1 semantics; flap-regression unit test RED→GREEN. If a live `[vis]` capture still flaps, the side test is the residual (H2) → add the robust side test here. | `dotnet test` green incl. the new regression test **AND** a live `[vis]` capture at the threshold shows non-empty + narrowing `OutsideView` (no `polys=0`/`polys=1` interleave) — the apparatus gate | +| **U.4c-4** | **Layer 3** — `SeenOutside` invariant tests; confirm `ClipFrameAssembler` consumes the stabilized frame unchanged. | Unit green | +| **U.4c-5** | **Visual gate** at the Holtburg cottage threshold (several angles / zooms). | **Seamless threshold — acceptance** | + +Optional sweeps **only if trivial and in-area** (defer anything non-trivial): `AppendSlot`'s +3-`Count==0`-state collapse (branch `IsNothingVisible` / `UseScissorFallback` before the call), +orphaned `LandblockEntriesWithoutAnimatedIndex`, dead `BuildingShellAnchorPass/Reject` counters. + +--- + +## 9. Risks + +- **#103/#98 recurrence (designing on a half-read).** Medium → mitigated. The flap is characterized + on a live `[vis]` capture and the retail semantics ported to pseudocode in Task 1 (oracle first); + the builder is **not declared correct until a live `[vis]` capture shows a non-empty, narrowing + `OutsideView`** at the threshold (U.4c-3 gate). Apparatus before fix (project memory + `apparatus-for-physics-bugs`). +- **PVS seeding over-includes cells** (draws cells the camera cannot actually see). Low — gated by + the per-frame clip (an unreached participant with an empty refined region draws nothing); worst + case is benign over-draw under the depth test, never a hidden-geometry or flap regression. +- **Growth re-clip changes traversal cost.** Low — M1.5 interior chains are short (≤ ~15 cells); + the fixpoint watermark bounds re-processing (already the U.2a termination guarantee). +- **`SeenOutside` dat flag absent on some cells.** Low — falls back to the §6 pure-walk path; the + flag is present on Holtburg cottage/inn cells (verified available via `A8CellAudit`). + +--- + +## 10. Reference index + +- **Handoff:** [`docs/research/2026-05-30-phase-u4-shipped-and-flap-handoff.md`](../../research/2026-05-30-phase-u4-shipped-and-flap-handoff.md) +- **Parent spec:** [`docs/superpowers/specs/2026-05-30-phase-u-unified-render-pipeline-design.md`](2026-05-30-phase-u-unified-render-pipeline-design.md) +- **Retail decomp** (`acclient_2013_pseudo_c.txt`): `RenderNormalMode` 92649; `CellManager::ChangePosition` + 94601 (grab calls 94646/94653/94659, keep/release 94649/94658); `grab_visible_cells` 311878; + `PView::DrawInside` 433793; `add_views` 433382; `remove_views` 432319; `ConstructView` 433750; + `InitCell` 432896; `ClipPortals` 433572 (`0xFFFF`→`outside_view` 433662-433676); + `AddViewToPortals` 433446 (fixpoint `AddToCell`/`FixCellList` 433494-433502); `OtherPortalClip` + 433524; `DrawCells` 432709 (landscape gate 432715). Struct: `CEnvCell` `acclient.h` ~30925 + (`num_stabs` / `stab_list` / `seen_outside`). +- **acdream anchors:** `PortalVisibilityBuilder.cs` (walk 116-228, `CameraOnInteriorSide` 237); + `CellVisibility.cs` (`LoadedCell` 24); `ClipFrameAssembler.cs` (empty→Skip 173-179); + `GameWindow.cs` (cell hydration 5696); `PhysicsDataCache.cs` (VisibleCells read 178-186); + `EnvCellSceneryInstance.cs` (`SeenOutsideCells` 91); `tools/A8CellAudit/Program.cs:200`. +- **Project memory:** `bn-decomp-field-names` (trust `acclient.h` over BN labels); + `apparatus-for-physics-bugs` (live capture before fix); `render-self-contained-gl-state` + (don't touch the renderers). +- **Related issues:** #103 (superseded arc), #78 (inn through-floor — relate), #95 / #102 (U.6).