docs(render): re-scope flap fix to retail enqueue-once traversal port (not an overlap band-aid)
Per senior-eng direction: the retail-faithful fix is to stop diverging from PView:: AddViewToPortals (first-discovery enqueue + AddToCell/FixCellList in-place growth, no re-enqueue/re-clip), removing acdream's MaxReprocessPerCell re-enqueue fixpoint and its documented per-round ProjectToClip drift. Drops the overlap-predicate approach. Viewpoint bit-stability (the ~1-8um player RenderPosition jitter) is the contingency next step only if a residual flap survives the visual gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d9d69394bb
commit
d0b65c4170
1 changed files with 79 additions and 66 deletions
|
|
@ -10,10 +10,16 @@
|
||||||
|
|
||||||
The indoor render **flap** (textures "battling" at the doorway threshold) is **portal-flood
|
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
|
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
|
deeper cell cluster frame-to-frame, redrawing a different set each frame. The fix is a **verbatim
|
||||||
membership depend on a **stable visibility predicate** (side-test + view-region overlap) instead of
|
port of retail's enqueue-once traversal** (`PView::ConstructView`/`AddViewToPortals`): a cell is
|
||||||
the **drift-prone surviving-vertex count** of the per-portal clip. Localized to
|
enqueued **only on first discovery**; later view-growth into an already-discovered cell is unioned
|
||||||
`PortalVisibilityBuilder`; no camera/movement/physics/clip-math rewrite.
|
**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
|
authoritative local position does not move). acdream diverges on **both** (re-enqueue drift + µm
|
||||||
viewpoint jitter), and the two combine at the grazing portal.
|
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
|
The fix restores retail's traversal **verbatim** — enqueue-once on first discovery, union-in-place on
|
||||||
predicate that is stable under acdream's residual drift/jitter, rather than the literal
|
growth — so acdream stops diverging from `AddViewToPortals` and the per-round re-clip drift disappears.
|
||||||
drift-sensitive vertex count.
|
No new predicate, no added robustness.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 4. The fix (design)
|
## 4. The fix (design)
|
||||||
|
|
||||||
**Principle:** set-membership is decided by a **stable** visibility predicate, not by the drift-prone
|
**Principle:** membership is set by **first discovery** in distance-priority order (retail
|
||||||
surviving-vertex count of the clip. The clip still computes the *draw* region; it no longer decides
|
`InsCellTodoList` in the `AddViewToPortals` `update_count == 0` branch, decomp `:433478`). A cell
|
||||||
*whether* a reachable cell is in the set.
|
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`):
|
bool grew = AddRegion(nview, clippedRegion); // union in place (= retail AddToCell)
|
||||||
```
|
if (grew && popCounts[neighbourId] < MaxReprocessPerCell // RE-ENQUEUE on growth ← the divergence
|
||||||
if (clippedRegion.Count == 0)
|
&& queued.Add(neighbourId))
|
||||||
{
|
todo.Insert(neighbour, dist);
|
||||||
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.
|
|
||||||
|
|
||||||
The drift-prone `clippedRegion.Count` no longer flips membership; a portal that is genuinely visible
|
New: enqueue a neighbour **only on first discovery** (no `CellViews` / `processedViewCounts` entry
|
||||||
through the accumulated view (stable side-test + stable overlap) stays in the set every frame.
|
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
|
**Change B — exit-portal / `OutsideView` contribution stays first-process.** Retail contributes a
|
||||||
projected polygon overlap any of the `activeViewPolygons`? Implemented to be stable for the
|
cell's exit-portal slice to `OutsideView` once, when the cell is processed; there is no re-enqueue
|
||||||
near/grazing case (the failure mode is `ClipToRegion` losing a vertex to float noise, NOT the gross
|
path in `AddViewToPortals` to re-contribute a grown view. acdream's `OutsideView` contribution
|
||||||
position of the sliver, which sits well inside the view region — so a robust "any-overlap" test
|
(line 256) already happens at process time, so removing the re-enqueue makes it match retail.
|
||||||
returns a steady boolean). Exact formulation is fixed in TDD (§5); candidates: (a) any portal NDC
|
**Regression watch:** the re-enqueue was added 2026-06-07 "to propagate late-discovered slices to exit
|
||||||
vertex inside the region OR any region vertex inside the portal OR any edge crossing; (b) reuse the
|
portals" — which retail does **not** do, so dropping it is faithful, but a look-in / outside-view
|
||||||
existing `EyeInsidePortalOpening` 3D near-region test generalized from "eye in opening" to "eye within
|
slice could shrink. The existing OutsideView tests (`Builder_Cellar_WindowClippedToStairwell`, the
|
||||||
the portal's view cone." The chosen formulation MUST keep the #95 guard test green.
|
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
|
**`EyeInsidePortalOpening` (line 235-244) is unchanged by this fix.** It is a separate near-degenerate
|
||||||
overlaps the full-screen region), so that ad-hoc patch is removed once the general predicate is in
|
single-clip guard (eye standing in a doorway), orthogonal to the re-enqueue, and stays as-is. **No
|
||||||
place — fewer special cases, not more.
|
overlap predicate is introduced.**
|
||||||
|
|
||||||
**#95 over-inclusion guard preserved:** an off-screen portal (2 m to the side) does not overlap the
|
**Why this is the flap fix, not a band-aid:** the re-enqueue re-clips a popped cell's portals from its
|
||||||
view region → still culled. No visible-set blowup.
|
*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)
|
## 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
|
1. **Enqueue-once correctness + termination (new).** A multi-path fixture in
|
||||||
`PortalVisibilityBuilderTests`: construct an interior portal that (a) passes the side-test, (b)
|
`PortalVisibilityBuilderTests`: a **diamond** (a cell reachable from two parents, so its view grows
|
||||||
whose projection overlaps the view region, but (c) whose `ClipToRegion` returns `<3` verts
|
after first discovery) and a **cycle** (portals looping back). Assert the flood (a) **terminates
|
||||||
(degenerate sliver — the live failure mode), and the eye is NOT standing in the opening. Assert the
|
with `MaxReprocessPerCell` removed**, (b) yields a **deduped** `OrderedVisibleCells`, and (c) each
|
||||||
neighbour **is** in `OrderedVisibleCells`. RED today (culled at line 235 because not
|
reachable cell is present exactly once. This is the property the re-enqueue cap was protecting;
|
||||||
`EyeInsidePortalOpening`); GREEN after the fix (kept because side-test + overlap). This pins the
|
enqueue-once provides it by construction. If a per-cell pop counter is cheap to surface, also assert
|
||||||
gate change without needing to reproduce the exact µm knife-edge.
|
**each cell is popped ≤ 1** (RED under the re-enqueue, GREEN after) — the direct enqueue-once signal.
|
||||||
- *Optional companion (robustness):* if a fixture can be found whose clip flips `<3 ↔ ≥3` under a
|
2. **No membership regression on known geometries.** `Build_EyeStandingInInteriorPortal_FloodsNeighbour`,
|
||||||
µ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`,
|
|
||||||
`Build_CollapsedInteriorPortalNearEyeBeyondHalfMeter_FloodsNeighbour`,
|
`Build_CollapsedInteriorPortalNearEyeBeyondHalfMeter_FloodsNeighbour`,
|
||||||
`Build_IsDeterministic_IdenticalInputsGiveIdenticalVisibleSet`, and the existing cellar/window
|
`Build_DegeneratePortalToTheSide_NotFlooded_NoOverInclusion` (#95 guard), `Build_IsDeterministic_*`,
|
||||||
clip tests.
|
and the cellar/window/look-in tests stay **green** (re-enqueue and enqueue-once agree on
|
||||||
4. **Visual gate (user).** At the cottage doorway threshold, hold still — the 2↔6 oscillation is
|
non-drifting geometry; if one changes, that is the §4 Change-B regression to handle retail's way,
|
||||||
gone; the deeper rooms render steadily through the door. Walking in/out remains seamless.
|
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.
|
`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
|
## 6. Scope / non-goals
|
||||||
|
|
||||||
- **In scope:** `PortalVisibilityBuilder` (the line-235 gate + the `PortalOverlapsView` helper),
|
- **In scope:** `PortalVisibilityBuilder` enqueue logic — enqueue-once on first discovery; remove the
|
||||||
removal of the now-subsumed `EyeInsidePortalOpening` force-flood branch, the new + existing tests.
|
`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):**
|
- **Non-goals (explicitly deferred):**
|
||||||
- No camera / movement / interpolation / physics changes (the µm viewpoint jitter is left as-is;
|
- **No overlap predicate / no added robustness** — this is a verbatim retail port, not a new
|
||||||
the fix is robust to it).
|
membership rule. `EyeInsidePortalOpening` (line 235) is untouched.
|
||||||
- No clip-math rewrite (`ProjectToClip`/`ClipToRegion` stay).
|
- **No clip-math rewrite** (`ProjectToClip`/`ClipToRegion` stay).
|
||||||
- **Restoring retail's enqueue-once traversal** (removing the re-enqueue fixpoint, eliminating the
|
- **No camera / movement / interpolation / physics changes** in this step.
|
||||||
per-round drift at its source) is a real, larger, retail-faithful improvement but a **separate
|
- **Contingency (next retail-faithful step, only if a residual flap survives the visual gate):**
|
||||||
step** — out of scope here. This fix neutralizes the drift's effect on membership without
|
bit-stabilize the viewpoint at rest. The live `[pv-input]` probe shows the player `RenderPosition`
|
||||||
restructuring the BFS.
|
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.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue