From 306cdb069ca0c205fb3d4e500e17da74a0bb24fe Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 30 May 2026 16:30:41 +0200 Subject: [PATCH] =?UTF-8?q?docs(render):=20Phase=20U.2a=20review=20fixups?= =?UTF-8?q?=20=E2=80=94=20LIFO-on-ties=20comment=20+=20ISSUES=20#102?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review minor follow-ups: correct the CellTodoList comments (ties are LIFO, not FIFO — an equal-distance newcomer lands at the tail and pops first, matching retail's break-on-first-not-greater + pop-from-tail). Update ISSUES #102 to record that U.2a closes I-1/I-2 (under-count + duplicate accumulation) via the enqueue-once gate, narrowing the residual to diamond-topology clip-completeness (AddToCell onward re-propagation, tracked under U.6). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/ISSUES.md | 25 +++++++++++++++++-- .../Rendering/PortalVisibilityBuilder.cs | 6 +++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 50c65f1..bbc4e06 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -84,11 +84,32 @@ Related: #102 (builder dungeon-scaling fixpoint). ## #102 — A8.F PortalVisibilityBuilder — port retail update_count fixpoint (replace MaxReprocessPerCell cap) -**Status:** OPEN -**Severity:** MEDIUM +**Status:** PARTIALLY RESOLVED (Phase U.2a, 2026-05-30, commit `d880775`) +**Severity:** MEDIUM → LOW (residual is diamond-topology clip-completeness only) **Filed:** 2026-05-29 **Component:** rendering, visibility, EnvCell portal traversal +**U.2a resolution (2026-05-30):** Reading the decomp showed retail does NOT +re-enqueue on view-growth: `AddViewToPortals` (433446) enqueues a cell via +`InsCellTodoList` ONLY in the first-discovery branch (`ecx_5 == 0`); later +growth goes through `AddToCell` (433050) in place and never re-enqueues. U.2a +replaced the `MaxReprocessPerCell` cap with an **enqueue-once gate** (a `seen` +set = retail `cell_view_done`, 433784) + a distance-priority work list (retail +`InsCellTodoList`). This **closes I-1 and I-2**: the clip-region union into a +neighbour now runs UNCONDITIONALLY before the enqueue gate, so >4-portal cells +no longer under-count (I-1 gone), and each cell processes its exit portals +exactly once, so cyclic graphs no longer accumulate duplicate polygons (I-2 +gone). The new `Build_CyclicHub_TerminatesAndBounds` test enforces the +acceptance (4-room ring ⇒ ≤5 cells, no dups). **Residual scope:** retail's +`AddToCell` ONWARD re-propagation of late growth (a cell reached via a longer +path AFTER it was drawn gets its own `CellView` unioned but does not +re-propagate that growth to ITS children) is NOT ported — this affects only +clip-region completeness on **diamond** topologies, never the visible cell set +or draw order. It overlaps the M-4 `OtherPortalClip` stub below; track both +under U.6 (dungeon-scale validation). A naive count-watermark re-enqueue is NOT +a valid fix (it never terminates, because `CellView.Add` appends without +merging) — the faithful fix is the in-place slice re-propagation. + **Description:** A8.F Task 4 shipped a bounded-BFS port of retail's `PView::ConstructView` → `ClipPortals` → `AddViewToPortals` in [`src/AcDream.App/Rendering/PortalVisibilityBuilder.cs`](../src/AcDream.App/Rendering/PortalVisibilityBuilder.cs). diff --git a/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs b/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs index 1ec315a..0b8aeff 100644 --- a/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs +++ b/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs @@ -268,7 +268,8 @@ public static class PortalVisibilityBuilder /// the tail; removes the tail — giving closest-first traversal exactly /// as ConstructView's pop-from-(cell_todo_num-1) does (433767-433769). The insertion only shifts /// entries strictly farther than the newcomer (retail's flag test breaks on the first - /// not-greater entry), so ties preserve insertion order (stable, FIFO among equal distances). + /// not-greater entry), so an equal-distance newcomer lands at the tail and pops FIRST — + /// LIFO on ties, matching retail's break-on-first-not-greater + pop-from-tail. /// private sealed class CellTodoList { @@ -280,7 +281,8 @@ public static class PortalVisibilityBuilder { // Find the slot: scan from the tail (nearest) toward the head while existing entries are // strictly nearer than `distance`, so the newcomer lands just ABOVE every entry that is - // farther-or-equal — i.e. nearest-at-tail order, FIFO on ties. + // farther-or-equal — i.e. nearest-at-tail order, LIFO on ties (an equal-distance + // newcomer inserts at the tail and pops first). int idx = _items.Count; while (idx > 0 && _items[idx - 1].Distance < distance) idx--;