diff --git a/docs/superpowers/specs/2026-06-08-portal-flood-membership-stability-design.md b/docs/superpowers/specs/2026-06-08-portal-flood-membership-stability-design.md index 90bb5b98..08621cc2 100644 --- a/docs/superpowers/specs/2026-06-08-portal-flood-membership-stability-design.md +++ b/docs/superpowers/specs/2026-06-08-portal-flood-membership-stability-design.md @@ -10,10 +10,16 @@ The indoor render **flap** (textures "battling" at the doorway threshold) is **portal-flood set-membership instability**: from a *stable* viewer cell, the PView BFS includes or excludes a -deeper cell cluster frame-to-frame, redrawing a different set each frame. The fix makes set -membership depend on a **stable visibility predicate** (side-test + view-region overlap) instead of -the **drift-prone surviving-vertex count** of the per-portal clip. Localized to -`PortalVisibilityBuilder`; no camera/movement/physics/clip-math rewrite. +deeper cell cluster frame-to-frame, redrawing a different set each frame. The fix is a **verbatim +port of retail's enqueue-once traversal** (`PView::ConstructView`/`AddViewToPortals`): a cell is +enqueued **only on first discovery**; later view-growth into an already-discovered cell is unioned +**in place** (retail `AddToCell`/`FixCellList`) and **never re-enqueues or re-clips** that cell's +portals. This removes acdream's `MaxReprocessPerCell` **re-enqueue fixpoint** — the documented +per-round `ProjectToClip` **drift** that lets µm viewpoint jitter re-discover/undiscover the deep +cluster. Localized to `PortalVisibilityBuilder`; no overlap-predicate, no added robustness, no +camera/movement/physics/clip-math change. (Contingency: if a residual flap survives — the deep +portal's *first* clip being knife-edge under µm jitter independent of drift — the next +retail-faithful step is bit-stabilizing the viewpoint at rest; see §6.) --- @@ -100,77 +106,79 @@ cell **once** (enqueue-once; no re-clip drift) and (b) its viewpoint is **bit-st authoritative local position does not move). acdream diverges on **both** (re-enqueue drift + µm viewpoint jitter), and the two combine at the grazing portal. -The fix restores retail's **intent** — "the portal is visible through the accumulated view" — with a -predicate that is stable under acdream's residual drift/jitter, rather than the literal -drift-sensitive vertex count. +The fix restores retail's traversal **verbatim** — enqueue-once on first discovery, union-in-place on +growth — so acdream stops diverging from `AddViewToPortals` and the per-round re-clip drift disappears. +No new predicate, no added robustness. --- ## 4. The fix (design) -**Principle:** set-membership is decided by a **stable** visibility predicate, not by the drift-prone -surviving-vertex count of the clip. The clip still computes the *draw* region; it no longer decides -*whether* a reachable cell is in the set. +**Principle:** membership is set by **first discovery** in distance-priority order (retail +`InsCellTodoList` in the `AddViewToPortals` `update_count == 0` branch, decomp `:433478`). A cell +already discovered is **never re-enqueued and never re-clipped**; later view-growth into it is unioned +**in place** and only refines that cell's own draw clip / draw-list position (retail `AddToCell` + +`FixCellList`, `:433492-433502`). The drift-prone re-clip loop is deleted, so µm viewpoint jitter can +no longer re-discover/undiscover a cell. -**Change — localized to `PortalVisibilityBuilder` (the line-235 gate):** +**Change A — enqueue-once (the core fix), `PortalVisibilityBuilder.cs` ~308-327.** +Today a neighbour is RE-enqueued whenever its view `grew`, capped by `MaxReprocessPerCell`: -- Today (`PortalVisibilityBuilder.cs:235-244`): - ``` - if (clippedRegion.Count == 0) - { - if (!EyeInsidePortalOpening(poly, cell.WorldTransform, cameraPos)) continue; // cull - foreach (var vp in activeViewPolygons) clippedRegion.Add(clone(vp)); // flood with parent view - } - ``` -- New: when `clippedRegion.Count == 0` but the portal **passed the side-test** (already computed: - `sideAllowed`, the stable plane-side test) **and its projection still overlaps the current view - region** (a stable convex-overlap predicate — true for a thin grazing sliver inside the region, - false for an off-screen portal), keep the neighbour by flooding it with the parent's view (the same - substitution the `EyeInsidePortalOpening` branch already does). Otherwise cull as today. + bool grew = AddRegion(nview, clippedRegion); // union in place (= retail AddToCell) + if (grew && popCounts[neighbourId] < MaxReprocessPerCell // RE-ENQUEUE on growth ← the divergence + && queued.Add(neighbourId)) + todo.Insert(neighbour, dist); -The drift-prone `clippedRegion.Count` no longer flips membership; a portal that is genuinely visible -through the accumulated view (stable side-test + stable overlap) stays in the set every frame. +New: enqueue a neighbour **only on first discovery** (no `CellViews` / `processedViewCounts` entry +yet). On growth into an already-discovered neighbour, union in place (keep `AddRegion`) and update its +draw-list position if already drawn (port `FixCellList`), but **do not** re-insert it into the todo +list. Remove `MaxReprocessPerCell`, `popCounts`, and the per-pop cap — enqueue-once terminates by +construction (≤ N cells), matching retail's `cell_view_done` guarantee (`:433784`). -**The stable overlap predicate** (`PortalOverlapsView`, new small helper): does the portal's -projected polygon overlap any of the `activeViewPolygons`? Implemented to be stable for the -near/grazing case (the failure mode is `ClipToRegion` losing a vertex to float noise, NOT the gross -position of the sliver, which sits well inside the view region — so a robust "any-overlap" test -returns a steady boolean). Exact formulation is fixed in TDD (§5); candidates: (a) any portal NDC -vertex inside the region OR any region vertex inside the portal OR any edge crossing; (b) reuse the -existing `EyeInsidePortalOpening` 3D near-region test generalized from "eye in opening" to "eye within -the portal's view cone." The chosen formulation MUST keep the #95 guard test green. +**Change B — exit-portal / `OutsideView` contribution stays first-process.** Retail contributes a +cell's exit-portal slice to `OutsideView` once, when the cell is processed; there is no re-enqueue +path in `AddViewToPortals` to re-contribute a grown view. acdream's `OutsideView` contribution +(line 256) already happens at process time, so removing the re-enqueue makes it match retail. +**Regression watch:** the re-enqueue was added 2026-06-07 "to propagate late-discovered slices to exit +portals" — which retail does **not** do, so dropping it is faithful, but a look-in / outside-view +slice could shrink. The existing OutsideView tests (`Builder_Cellar_WindowClippedToStairwell`, the +look-in tests) must stay green; if one shrinks, the fix is retail's `AddToCell`/`FixCellList` ordering, +**not** reinstating the re-enqueue. -**This subsumes the `EyeInsidePortalOpening` special-case** (a portal the eye stands in trivially -overlaps the full-screen region), so that ad-hoc patch is removed once the general predicate is in -place — fewer special cases, not more. +**`EyeInsidePortalOpening` (line 235-244) is unchanged by this fix.** It is a separate near-degenerate +single-clip guard (eye standing in a doorway), orthogonal to the re-enqueue, and stays as-is. **No +overlap predicate is introduced.** -**#95 over-inclusion guard preserved:** an off-screen portal (2 m to the side) does not overlap the -view region → still culled. No visible-set blowup. +**Why this is the flap fix, not a band-aid:** the re-enqueue re-clips a popped cell's portals from its +*grown* (drifted) view and can therefore **add or drop** the deep `0172-0175` cluster as the drift +walks across the clip boundary under µm jitter. Enqueue-once decides the cluster's membership **once**, +at first discovery, from the cell's clean first-accumulated view — the same decision retail makes. --- ## 5. Verification (TDD) -Write the failing test first, then the fix. +The flap itself is float-drift-dependent (it manifests only under live µm jitter at a specific grazing +geometry), so the **visual gate is the acceptance**; the unit layer pins enqueue-once correctness and +guards regressions. -1. **RED → GREEN — degenerate-clip membership.** New deterministic test in - `PortalVisibilityBuilderTests`: construct an interior portal that (a) passes the side-test, (b) - whose projection overlaps the view region, but (c) whose `ClipToRegion` returns `<3` verts - (degenerate sliver — the live failure mode), and the eye is NOT standing in the opening. Assert the - neighbour **is** in `OrderedVisibleCells`. RED today (culled at line 235 because not - `EyeInsidePortalOpening`); GREEN after the fix (kept because side-test + overlap). This pins the - gate change without needing to reproduce the exact µm knife-edge. - - *Optional companion (robustness):* if a fixture can be found whose clip flips `<3 ↔ ≥3` under a - µm eye nudge, add a test asserting `OrderedVisibleCells` is identical across the nudge. Skip if - it proves too geometry-sensitive to construct stably — the deterministic test above is the gate. -2. **Stays GREEN — #95 over-inclusion guard.** `Build_DegeneratePortalToTheSide_NotFlooded_NoOverInclusion` - (off-screen portal stays culled). -3. **Stays GREEN — existing behavior.** `Build_EyeStandingInInteriorPortal_FloodsNeighbour`, +1. **Enqueue-once correctness + termination (new).** A multi-path fixture in + `PortalVisibilityBuilderTests`: a **diamond** (a cell reachable from two parents, so its view grows + after first discovery) and a **cycle** (portals looping back). Assert the flood (a) **terminates + with `MaxReprocessPerCell` removed**, (b) yields a **deduped** `OrderedVisibleCells`, and (c) each + reachable cell is present exactly once. This is the property the re-enqueue cap was protecting; + enqueue-once provides it by construction. If a per-cell pop counter is cheap to surface, also assert + **each cell is popped ≤ 1** (RED under the re-enqueue, GREEN after) — the direct enqueue-once signal. +2. **No membership regression on known geometries.** `Build_EyeStandingInInteriorPortal_FloodsNeighbour`, `Build_CollapsedInteriorPortalNearEyeBeyondHalfMeter_FloodsNeighbour`, - `Build_IsDeterministic_IdenticalInputsGiveIdenticalVisibleSet`, and the existing cellar/window - clip tests. -4. **Visual gate (user).** At the cottage doorway threshold, hold still — the 2↔6 oscillation is - gone; the deeper rooms render steadily through the door. Walking in/out remains seamless. + `Build_DegeneratePortalToTheSide_NotFlooded_NoOverInclusion` (#95 guard), `Build_IsDeterministic_*`, + and the cellar/window/look-in tests stay **green** (re-enqueue and enqueue-once agree on + non-drifting geometry; if one changes, that is the §4 Change-B regression to handle retail's way, + NOT by reinstating the re-enqueue). +3. **Visual gate (user) — the acceptance.** At the cottage doorway threshold, hold still: the 2↔6 + oscillation is gone; the deeper rooms render steadily through the door; walking in/out stays + seamless. Re-run the `[pv-input]`/`[render-sig]` probes to confirm `ids=`/flood is stable while + standing still. `dotnet build` + `dotnet test` green before the visual gate. @@ -178,16 +186,21 @@ Write the failing test first, then the fix. ## 6. Scope / non-goals -- **In scope:** `PortalVisibilityBuilder` (the line-235 gate + the `PortalOverlapsView` helper), - removal of the now-subsumed `EyeInsidePortalOpening` force-flood branch, the new + existing tests. +- **In scope:** `PortalVisibilityBuilder` enqueue logic — enqueue-once on first discovery; remove the + `MaxReprocessPerCell` re-enqueue, `popCounts`, and the per-pop cap; union-in-place + draw-list + re-position on growth (port retail `AddToCell`/`FixCellList`); the new + existing tests. - **Non-goals (explicitly deferred):** - - No camera / movement / interpolation / physics changes (the µm viewpoint jitter is left as-is; - the fix is robust to it). - - No clip-math rewrite (`ProjectToClip`/`ClipToRegion` stay). - - **Restoring retail's enqueue-once traversal** (removing the re-enqueue fixpoint, eliminating the - per-round drift at its source) is a real, larger, retail-faithful improvement but a **separate - step** — out of scope here. This fix neutralizes the drift's effect on membership without - restructuring the BFS. + - **No overlap predicate / no added robustness** — this is a verbatim retail port, not a new + membership rule. `EyeInsidePortalOpening` (line 235) is untouched. + - **No clip-math rewrite** (`ProjectToClip`/`ClipToRegion` stay). + - **No camera / movement / interpolation / physics changes** in this step. +- **Contingency (next retail-faithful step, only if a residual flap survives the visual gate):** + bit-stabilize the viewpoint at rest. The live `[pv-input]` probe shows the player `RenderPosition` + carries ~1–8 µm float noise at rest (e.g. Z `94.000000 ↔ 94.000008`), which retail's authoritative + local position does not. If enqueue-once leaves a residual flicker (the deep portal's *first* clip is + knife-edge under that jitter), trace the jitter to its source (interpolation residual vs physics + contact-settling) and make the local-player viewpoint bit-stable at rest, matching retail. Scoped as + a separate step because it touches the movement/physics path; do it only if measured necessary. ---