From 3916b2b23efbf5ca14d6e2d1d08a531d0bab37ac Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 30 May 2026 16:37:14 +0200 Subject: [PATCH] =?UTF-8?q?feat(render):=20Phase=20U.2b=20=E2=80=94=20reci?= =?UTF-8?q?procal=20OtherPortalClip=20(retail=20433524)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clip the portal opening against the neighbour's matching back-portal polygon before propagating, so a cell's clip region is the intersection of the opening seen from both sides. Closes the M-4 stub in ISSUES #102. Can only tighten, never under-include; degrades to prior behavior when no back-portal is found. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/ISSUES.md | 32 +++-- .../Rendering/PortalVisibilityBuilder.cs | 65 +++++++++- .../Rendering/PortalVisibilityBuilderTests.cs | 111 ++++++++++++++++++ 3 files changed, 194 insertions(+), 14 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index bbc4e06..8903051 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -105,10 +105,12 @@ acceptance (4-room ring ⇒ ≤5 cells, no dups). **Residual scope:** retail's 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. +or draw order. Track under U.6 (dungeon-scale validation). (The M-4 +`OtherPortalClip` stub noted below is now CLOSED by Phase U.2b — a separate +concern from this onward-re-propagation gap.) 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 @@ -144,13 +146,21 @@ Retail anchors (`docs/research/named-retail/acclient_2013_pseudo_c.txt`): - `InitCell` — per-cell timestamp init - `AddViewToPortals` 433446 — change-detection that drives the fixpoint -**Related M-4 stub:** the neighbour-side `OtherPortalClip` -(decomp:433524) is also not yet ported — the builder clips the portal -opening against the *current* cell's view but does NOT additionally clip -against the *neighbour's* matching portal polygon. Its absence can only -ever OVER-include (mark cells/regions visible that retail would cull), -never under-include, so it's deferred. There is a `TODO(A8.F)` marker at -the relevant site in `PortalVisibilityBuilder.cs`. +**Related M-4 stub — CLOSED (Phase U.2b, 2026-05-30):** the +neighbour-side `OtherPortalClip` (decomp:433524) is now ported. After a +portal's near-side opening is clipped against the current cell's view, +`PortalVisibilityBuilder.ApplyReciprocalClip` resolves the neighbour's +matching back-portal (scan `neighbour.Portals` for the entry whose +`OtherCellId` == the near cell's low-16-bits id; `PortalPolygons` is in +lockstep), projects it through the neighbour's `WorldTransform`, and +intersects it into the propagated region before the union — so a cell's +clip region is the intersection of the opening seen from BOTH sides. Can +only TIGHTEN; degrades to prior near-side-only behavior when no +back-portal is found. The `TODO(A8.F)` marker is removed. Covered by +`PortalVisibilityBuilderTests.Build_AppliesReciprocalOtherPortalClip` +(reciprocal tightening) + `…_DegradesGracefully_WhenNoBackPortal`. +(The diamond-topology onward re-propagation of late growth remains out of +scope here — tracked under U.6.) **Files:** - `src/AcDream.App/Rendering/PortalVisibilityBuilder.cs` — replace the diff --git a/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs b/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs index 0b8aeff..e44aa88 100644 --- a/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs +++ b/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs @@ -175,12 +175,12 @@ public static class PortalVisibilityBuilder continue; } - // TODO(A8.F): neighbour-side OtherPortalClip (decomp:433524) — also clip the - // interior portal against the neighbour's matching portal polygon. Not implemented - // here; add if multi-cell conformance shows over-inclusion. uint neighbourId = lbMask | portal.OtherCellId; // Cross-building boundary: route to CrossBuildingViews, don't continue in-building BFS. + // (Cross-building entry is retail's CBldPortal/AddToCell channel, not OtherPortalClip; + // the reciprocal clip below is interior-cell-to-cell only, matching the OtherPortalClip + // call inside ConstructView at decomp:433692.) if (buildingMembership != null && !buildingMembership(neighbourId)) { var xview = GetOrCreate(frame.CrossBuildingViews, neighbourId); @@ -191,6 +191,17 @@ public static class PortalVisibilityBuilder var neighbour = lookup(neighbourId); if (neighbour == null) continue; + // Phase U.2b — neighbour-side OtherPortalClip (retail PView::OtherPortalClip + // decomp:433524). The portal opening seen from THIS cell may be wider than the + // SAME opening seen from the neighbour (skewed/oblique apertures), so retail + // re-clips the already-near-side-clipped region against the neighbour's matching + // (reciprocal) portal polygon — the propagated region is the intersection of the + // opening "seen from A" AND "seen from B". This can only TIGHTEN, never widen, and + // degrades to the prior near-side-only region when no back-portal is found (data + // gap). Mutates clippedRegion in place before the union below. + ApplyReciprocalClip(clippedRegion, cell.CellId, neighbour, viewProj); + if (clippedRegion.Count == 0) continue; // reciprocal opening doesn't overlap → not visible + // Union the clipped region into the neighbour's accumulated view. var nview = GetOrCreate(frame.CellViews, neighbourId); foreach (var cp in clippedRegion) nview.Add(cp); @@ -239,6 +250,54 @@ public static class PortalVisibilityBuilder if (area2 < 0f) Array.Reverse(poly); } + // Phase U.2b — reciprocal OtherPortalClip (retail PView::OtherPortalClip decomp:433524). + // Finds the neighbour's portal that points BACK to `fromCellId`, projects that reciprocal + // polygon through the NEIGHBOUR's world transform to NDC, and intersects it into every + // polygon of `clippedRegion` (already clipped against the near-side opening + current view). + // The net region is "opening seen from the near cell" ∩ "opening seen from the neighbour" — + // a strict tightening that prevents over-inclusion through skewed apertures. Degrades to a + // no-op (leaves `clippedRegion` untouched) when no matching back-portal exists or the + // reciprocal polygon is missing/degenerate/projects behind the camera — NEVER throws. + // + // Retail resolves the reciprocal via an explicit back-link (arg2->other_portal_id at 005a54b2); + // our LoadedCell data model lacks that index, so we recover it by scanning the neighbour's + // Portals for the entry whose OtherCellId equals the near cell's low-16-bits id. PortalPolygons + // is in lockstep with Portals, so the matched index j gives PortalPolygons[j] as the reciprocal. + private static void ApplyReciprocalClip( + List clippedRegion, uint fromCellId, LoadedCell neighbour, Matrix4x4 viewProj) + { + if (clippedRegion.Count == 0) return; + + // Neighbour portals store the 16-bit OtherCellId; match against the near cell's low word + // (the builder composes full ids as lbMask | OtherCellId, so the low word is the key). + ushort backTarget = (ushort)(fromCellId & 0xFFFFu); + + Vector3[]? reciprocalPoly = null; + for (int j = 0; j < neighbour.Portals.Count; j++) + { + if (neighbour.Portals[j].OtherCellId != backTarget) continue; + if (j >= neighbour.PortalPolygons.Count) break; + var candidate = neighbour.PortalPolygons[j]; + if (candidate != null && candidate.Length >= 3) reciprocalPoly = candidate; + break; + } + if (reciprocalPoly == null) return; // data gap → keep near-side-only region (degrade gracefully) + + // Project the reciprocal opening through the NEIGHBOUR's transform (retail positionPush(3, + // &other_cell_ptr->pos) at 005a54d2), then normalize winding for the CCW-only clipper. + Vector2[] reciprocalNdc = PortalProjection.ProjectToNdc(reciprocalPoly, neighbour.WorldTransform, viewProj); + if (reciprocalNdc.Length < 3) return; // reciprocal entirely behind camera / degenerate → no-op + EnsureCcw(reciprocalNdc); + + // Intersect the reciprocal opening into each near-side polygon; drop any that fall away. + for (int k = clippedRegion.Count - 1; k >= 0; k--) + { + var tightened = ScreenPolygonClip.Intersect(reciprocalNdc, clippedRegion[k].Vertices); + if (tightened.Length >= 3) clippedRegion[k] = new ViewPolygon(tightened); + else clippedRegion.RemoveAt(k); + } + } + private static CellView GetOrCreate(Dictionary map, uint key) { if (!map.TryGetValue(key, out var v)) { v = new CellView(); map[key] = v; } diff --git a/tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs b/tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs index af275ec..380b550 100644 --- a/tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs +++ b/tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs @@ -187,6 +187,117 @@ public class PortalVisibilityBuilderTests $"hub + 4 rooms expected, got {f.OrderedVisibleCells.Count} — fixpoint failed to converge"); Assert.Equal(f.OrderedVisibleCells.Count, f.OrderedVisibleCells.Distinct().Count()); // no dup cells } + + // ----------------------------------------------------------------------- + // Phase U.2b: reciprocal OtherPortalClip (retail PView::OtherPortalClip + // decomp:433524). When a portal leads to a loaded neighbour, the + // propagated region must ALSO be clipped against the neighbour's matching + // (reciprocal) back-portal polygon — the result is the intersection of the + // opening seen from BOTH sides. This can only tighten, never widen. + // ----------------------------------------------------------------------- + + // Camera in A looking down -Z through a WIDE near-side portal (A->B). B's + // matching back-portal (B->A) is a NARROW opening at the same plane, so the + // reciprocal opening projects to a strictly smaller NDC region. Without the + // reciprocal clip, B's CellView equals the wide near-side projection; with + // it, B's CellView is bounded by the narrow reciprocal opening. + private static (LoadedCell camCell, LoadedCell neighbour, Dictionary lookup) + SyntheticReciprocalPair() + { + const uint A = 0x0001, B = 0x0002; + // A's portal into B: wide opening (half-width 0.9) at z = -3. + var a = Cell(A, new CellPortalInfo((ushort)B, 0, 0)); + a.PortalPolygons.Add(Quad(0f, 0f, 0.9f, 0.9f, -3f)); + // B's reciprocal portal back to A: NARROW opening (half-width 0.3), + // same height and plane, so it projects fully inside the near-side rect + // but covers only ~1/3 of its width. + var b = Cell(B, new CellPortalInfo((ushort)A, 0, 0)); + b.PortalPolygons.Add(Quad(0f, 0f, 0.3f, 0.9f, -3f)); + var all = new Dictionary { [A] = a, [B] = b }; + return (a, b, all); + } + + [Fact] + public void Build_AppliesReciprocalOtherPortalClip() + { + var (camCell, neighbour, lookup) = SyntheticReciprocalPair(); + var vp = ViewProj(); + var f = PortalVisibilityBuilder.Build( + camCell, Vector3.Zero, id => lookup.TryGetValue(id, out var c) ? c : null, vp); + + Assert.True(f.CellViews.ContainsKey(0x0002), "neighbour cell view should be populated"); + + // The reciprocal opening's area in NDC: project B's narrow back-portal + // polygon and take its (CCW-magnitude) shoelace area. This is the + // tightest the neighbour region can be — the propagated region must not + // exceed it once both-sides clipping is applied. + float reciprocalArea = ProjectedPolygonArea(neighbour.PortalPolygons[0], vp); + + float area = CellViewArea(f.CellViews[0x0002]); + const float eps = 1e-4f; + Assert.True(area <= reciprocalArea + eps, + $"neighbour CellView area {area} must be clipped to the narrower reciprocal opening " + + $"{reciprocalArea} (without OtherPortalClip it equals the WIDE near-side projection)"); + + // Falsifiability guard: the near-side projection is genuinely WIDER than + // the reciprocal, so a no-op clip would have failed the assertion above. + float nearSideArea = ProjectedPolygonArea(camCell.PortalPolygons[0], vp); + Assert.True(nearSideArea > reciprocalArea * 1.5f, + $"fixture sanity: near-side area {nearSideArea} must dominate reciprocal {reciprocalArea}"); + } + + [Fact] + public void Build_ReciprocalClip_DegradesGracefully_WhenNoBackPortal() + { + // Same wide A->B opening, but B has NO portal pointing back to A (data gap). + // The reciprocal clip must no-op, so B's CellView equals the WIDE near-side + // projection — proving degradation never under-includes (and never throws). + const uint A = 0x0001, B = 0x0002; + var a = Cell(A, new CellPortalInfo((ushort)B, 0, 0)); + a.PortalPolygons.Add(Quad(0f, 0f, 0.9f, 0.9f, -3f)); + var b = Cell(B); // no portals at all → no back-portal to match + var all = new Dictionary { [A] = a, [B] = b }; + var vp = ViewProj(); + + var f = PortalVisibilityBuilder.Build( + a, Vector3.Zero, id => all.TryGetValue(id, out var c) ? c : null, vp); + + Assert.True(f.CellViews.ContainsKey(B), "neighbour must still be reached when no back-portal exists"); + float nearSideArea = ProjectedPolygonArea(a.PortalPolygons[0], vp); + float area = CellViewArea(f.CellViews[B]); + Assert.True(System.MathF.Abs(area - nearSideArea) < 1e-3f, + $"with no reciprocal portal the region must equal the near-side projection " + + $"(got {area}, near-side {nearSideArea}) — degrade must not tighten or expand"); + } + + // Signed-area-magnitude (shoelace) sum over a CellView's polygons in NDC. + private static float CellViewArea(CellView view) + { + float total = 0f; + foreach (var poly in view.Polygons) total += ShoelaceAbs(poly.Vertices); + return total; + } + + // Project a cell-local polygon (identity world transform in these fixtures) + // to NDC via the same path the builder uses, then take its area magnitude. + private static float ProjectedPolygonArea(Vector3[] localPoly, Matrix4x4 vp) + { + var ndc = PortalProjection.ProjectToNdc(localPoly, Matrix4x4.Identity, vp); + return ShoelaceAbs(ndc); + } + + private static float ShoelaceAbs(Vector2[] poly) + { + if (poly == null || poly.Length < 3) return 0f; + float area2 = 0f; + for (int i = 0; i < poly.Length; i++) + { + var p = poly[i]; + var q = poly[(i + 1) % poly.Length]; + area2 += p.X * q.Y - q.X * p.Y; + } + return System.MathF.Abs(area2) * 0.5f; + } } internal static class PortalFrameTestHelper