From 65781f57684740c34dece7b280007d1b314b2c19 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 30 May 2026 16:56:00 +0200 Subject: [PATCH] =?UTF-8?q?fix(render):=20Phase=20U.2b=20=E2=80=94=20resol?= =?UTF-8?q?ve=20reciprocal=20portal=20by=20other=5Fportal=5Fid=20(retail?= =?UTF-8?q?=20433557)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review caught a CRITICAL under-inclusion: ApplyReciprocalClip scanned for the first OtherCellId match, so a cell with two portals to the same neighbour clipped both near-side openings against the FIRST reciprocal polygon — hiding geometry through the second opening (real on Holtburg cellar cells 0x148<->0x149). Plumb the dat's OtherPortalId back-link through CellPortalInfo + BuildLoadedCell and index the reciprocal directly (retail arg2->other_portal_id, 433557). Skip (degrade to over-include) when the index is unresolvable — never clip against a guessed polygon. Adds a disjoint two-back- portal regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/ISSUES.md | 38 +++-- src/AcDream.App/Rendering/CellVisibility.cs | 11 +- src/AcDream.App/Rendering/GameWindow.cs | 3 +- .../Rendering/PortalVisibilityBuilder.cs | 60 ++++---- .../CellVisibilityPortalPolygonsTests.cs | 4 +- .../Rendering/PortalVisibilityBuilderTests.cs | 132 +++++++++++++++--- .../Rendering/Wb/BuildingLoaderTests.cs | 2 +- 7 files changed, 184 insertions(+), 66 deletions(-) diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 8903051..58b6485 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -146,21 +146,33 @@ 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 — 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 +**Related M-4 stub — CLOSED (Phase U.2b, 2026-05-30; reciprocal-resolution +fix 2026-05-30):** the neighbour-side `OtherPortalClip` (decomp:433524) is +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 **by direct index via the dat's +`CellPortal.OtherPortalId` back-link** (retail `arg2->other_portal_id`, +005a54b2), 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 +clip region is the intersection of the opening seen from BOTH sides. The +reciprocal is `neighbour.PortalPolygons[portal.OtherPortalId]`, NOT a scan +for the first `OtherCellId` match. The direct index is load-bearing: a cell +with TWO portals to the same neighbour (real on the Holtburg cellar — +`0x148` has two portals to `0x149`, polys 40/41, and `0x149` has two +reciprocals back to `0x148`) clips each opening against its OWN reciprocal. +The earlier scan-by-first-match resolved both near-side openings to the +FIRST reciprocal, and disjoint apertures then intersected to empty — +HIDING the geometry through the second opening (under-inclusion). The fix +plumbs `OtherPortalId` through `CellPortalInfo` + `BuildLoadedCell`. Guards +degrade to over-include (never clip against a guessed polygon) when the +index is out of range, the polygon is missing/degenerate, or it projects +behind the camera. Can only TIGHTEN. 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.) +(reciprocal tightening) + `…_DegradesGracefully_WhenNoBackPortal` +(over-include degrade) + `…_MultiplePortalsToSameNeighbour_EachResolvesOwnReciprocal` +(the disjoint two-back-portal regression). (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/CellVisibility.cs b/src/AcDream.App/Rendering/CellVisibility.cs index 6f1b803..074aaa4 100644 --- a/src/AcDream.App/Rendering/CellVisibility.cs +++ b/src/AcDream.App/Rendering/CellVisibility.cs @@ -88,8 +88,17 @@ public sealed class LoadedCell /// /// Portal connection to a neighbouring cell. /// OtherCellId == 0xFFFF indicates an exit portal to the outdoor world. +/// +/// is the dat's reciprocal back-link: the index of +/// the portal WITHIN the neighbour cell's portal list that points back through +/// this same opening. Retail indexes the reciprocal directly via this field +/// (arg2->other_portal_id, decomp:433557) rather than scanning — which +/// is what lets a cell with TWO portals to the same neighbour resolve each +/// opening against its OWN reciprocal polygon instead of the first match. +/// /// -public readonly record struct CellPortalInfo(ushort OtherCellId, ushort PolygonId, ushort Flags); +public readonly record struct CellPortalInfo( + ushort OtherCellId, ushort PolygonId, ushort Flags, ushort OtherPortalId); /// /// Clip plane derived from a portal polygon, in cell-local space. diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index fde8056..375f54c 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -5594,7 +5594,8 @@ public sealed class GameWindow : IDisposable portals.Add(new CellPortalInfo( portal.OtherCellId, portal.PolygonId, - (ushort)portal.Flags)); + (ushort)portal.Flags, + portal.OtherPortalId)); // Phase U.2b: dat back-link → reciprocal portal index (retail 433557) // Build clip plane from the portal polygon. if (cellStruct.Polygons.TryGetValue(portal.PolygonId, out var poly) diff --git a/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs b/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs index e44aa88..5276196 100644 --- a/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs +++ b/src/AcDream.App/Rendering/PortalVisibilityBuilder.cs @@ -197,9 +197,14 @@ public static class PortalVisibilityBuilder // 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); + // degrades to the prior near-side-only region when the reciprocal is unresolvable + // (over-include is the safe default). The reciprocal is the portal at index + // `portal.OtherPortalId` in the NEIGHBOUR's portal list — retail's direct back-link + // (arg2->other_portal_id, 433557), NOT a scan for the first OtherCellId match. The + // direct index is what lets a cell with TWO portals to the same neighbour clip each + // opening against its OWN reciprocal instead of the first one. Mutates clippedRegion + // in place before the union below. + ApplyReciprocalClip(clippedRegion, portal.OtherPortalId, neighbour, viewProj); if (clippedRegion.Count == 0) continue; // reciprocal opening doesn't overlap → not visible // Union the clipped region into the neighbour's accumulated view. @@ -251,37 +256,34 @@ public static class PortalVisibilityBuilder } // 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. + // Resolves the neighbour's reciprocal back-portal by DIRECT INDEX (`otherPortalId`), 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. // - // 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. + // `otherPortalId` is the near-side portal's reciprocal back-link, straight from the dat's + // CellPortal.OtherPortalId. Retail indexes the neighbour's portal array with it directly — + // `portals->portal[arg2->other_portal_id ...]` at 005a54b2/005a54f6 — rather than scanning for + // the first OtherCellId match. A scan picks the FIRST back-portal for EVERY near-side portal to + // the same neighbour, so a cell with two openings into one neighbour clips both against the same + // (first) reciprocal — hiding the second opening when the apertures are disjoint (under-inclusion + // bug #102 M-4). The direct index gives each opening its own reciprocal. + // + // GUARDS — degrade to over-include (leave `clippedRegion` untouched), NEVER clip against a + // guessed polygon: the index is out of range, OR the indexed polygon is missing/degenerate + // (< 3 verts), OR it projects entirely behind the camera. Over-inclusion is the safe default; + // mis-resolution is the bug this method exists to remove. PortalPolygons is in lockstep with + // Portals, so index `otherPortalId` selects the reciprocal polygon. NEVER throws. private static void ApplyReciprocalClip( - List clippedRegion, uint fromCellId, LoadedCell neighbour, Matrix4x4 viewProj) + List clippedRegion, ushort otherPortalId, 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) + // Direct back-link index (retail arg2->other_portal_id). Out-of-range → over-include. + if (otherPortalId >= neighbour.PortalPolygons.Count) return; + Vector3[]? reciprocalPoly = neighbour.PortalPolygons[otherPortalId]; + if (reciprocalPoly == null || reciprocalPoly.Length < 3) return; // missing/degenerate → over-include // 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. diff --git a/tests/AcDream.App.Tests/Rendering/CellVisibilityPortalPolygonsTests.cs b/tests/AcDream.App.Tests/Rendering/CellVisibilityPortalPolygonsTests.cs index 8c8008d..881a8d7 100644 --- a/tests/AcDream.App.Tests/Rendering/CellVisibilityPortalPolygonsTests.cs +++ b/tests/AcDream.App.Tests/Rendering/CellVisibilityPortalPolygonsTests.cs @@ -30,8 +30,8 @@ public class CellVisibilityPortalPolygonsTests { Portals = new() { - new CellPortalInfo(0xFFFF, 100, 0), // exit portal, has geometry - new CellPortalInfo(0x0102, 101, 0), // inner portal, no geometry resolved + new CellPortalInfo(0xFFFF, 100, 0, 0), // exit portal, has geometry + new CellPortalInfo(0x0102, 101, 0, 0), // inner portal, no geometry resolved }, ClipPlanes = new() { default, default }, PortalPolygons = new() diff --git a/tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs b/tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs index 380b550..a9fb2c9 100644 --- a/tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs +++ b/tests/AcDream.App.Tests/Rendering/PortalVisibilityBuilderTests.cs @@ -21,6 +21,15 @@ public class PortalVisibilityBuilderTests new Vector3(cx + halfW, cy + halfH, z), new Vector3(cx - halfW, cy + halfH, z), }; + // Full-height (y in [-0.9, 0.9]) quad spanning [minX, maxX] in X at plane z. + // Used by the multi-back-portal fixture where the X span is the only thing + // distinguishing the LEFT and RIGHT apertures. + private static Vector3[] QuadX(float minX, float maxX, float z) => new[] + { + new Vector3(minX, -0.9f, z), new Vector3(maxX, -0.9f, z), + new Vector3(maxX, 0.9f, z), new Vector3(minX, 0.9f, z), + }; + private static LoadedCell Cell(uint id, params CellPortalInfo[] portals) => new LoadedCell { CellId = id, WorldTransform = Matrix4x4.Identity, InverseWorldTransform = Matrix4x4.Identity, @@ -33,9 +42,9 @@ public class PortalVisibilityBuilderTests [Fact] public void Builder_Cellar_WindowClippedToStairwell_NotFullWindow() { - var cam = Cell(0x0001, new CellPortalInfo(0x0002, 0, 0)); + var cam = Cell(0x0001, new CellPortalInfo(0x0002, 0, 0, 0)); cam.PortalPolygons.Add(Quad(0f, 0f, 0.1f, 1.0f, -3f)); // narrow stairwell - var ground = Cell(0x0002, new CellPortalInfo(0xFFFF, 0, 0)); + var ground = Cell(0x0002, new CellPortalInfo(0xFFFF, 0, 0, 0)); ground.PortalPolygons.Add(Quad(0f, 0f, 1.0f, 1.0f, -6f)); // wide window var all = new Dictionary { [0x0001] = cam, [0x0002] = ground }; @@ -52,7 +61,7 @@ public class PortalVisibilityBuilderTests [Fact] public void Builder_SealedCellar_NoExitPortal_OutsideViewEmpty() { - var cam = Cell(0x0001, new CellPortalInfo(0x0002, 0, 0)); + var cam = Cell(0x0001, new CellPortalInfo(0x0002, 0, 0, 0)); cam.PortalPolygons.Add(Quad(0, 0, 0.1f, 1f, -3f)); var inner = Cell(0x0002); // no portals at all var all = new Dictionary { [0x0001] = cam, [0x0002] = inner }; @@ -62,7 +71,7 @@ public class PortalVisibilityBuilderTests [Fact] public void Builder_CameraCellWithDirectExit_OutsideViewIsFullWindow() { - var cam = Cell(0x0001, new CellPortalInfo(0xFFFF, 0, 0)); + var cam = Cell(0x0001, new CellPortalInfo(0xFFFF, 0, 0, 0)); cam.PortalPolygons.Add(Quad(0, 0, 1f, 1f, -6f)); var all = new Dictionary { [0x0001] = cam }; var frame = Build(cam, all); @@ -74,11 +83,11 @@ public class PortalVisibilityBuilderTests public void Builder_BackFacingPortal_NotTraversed() { // Portal to 0x0002, but its clip plane puts the camera (origin) on the OUTSIDE. - var cam = Cell(0x0001, new CellPortalInfo(0x0002, 0, 0)); + var cam = Cell(0x0001, new CellPortalInfo(0x0002, 0, 0, 0)); cam.PortalPolygons.Add(Quad(0, 0, 0.5f, 0.5f, -3f)); cam.ClipPlanes.Add(new PortalClipPlane { Normal = new Vector3(0, 0, 1), D = -1f, InsideSide = 0 }); // dot = (0,0,1)·origin + (-1) = -1 < 0; InsideSide==0 requires dot >= -eps → camera OUTSIDE → skip. - var ground = Cell(0x0002, new CellPortalInfo(0xFFFF, 0, 0)); + var ground = Cell(0x0002, new CellPortalInfo(0xFFFF, 0, 0, 0)); ground.PortalPolygons.Add(Quad(0, 0, 1f, 1f, -6f)); var all = new Dictionary { [0x0001] = cam, [0x0002] = ground }; @@ -95,7 +104,7 @@ public class PortalVisibilityBuilderTests { new Vector3(-1, -1, -6), new Vector3(-1, 1, -6), new Vector3(1, 1, -6), new Vector3(1, -1, -6), }; - var cam = Cell(0x0001, new CellPortalInfo(0xFFFF, 0, 0)); + var cam = Cell(0x0001, new CellPortalInfo(0xFFFF, 0, 0, 0)); cam.PortalPolygons.Add(cwQuad); var all = new Dictionary { [0x0001] = cam }; var frame = Build(cam, all); @@ -110,9 +119,9 @@ public class PortalVisibilityBuilderTests public void Builder_CyclicGraph_TerminatesWithBoundedPolys() { // A <-> B cycle; B also has an exit window. Must terminate and not blow up. - var a = Cell(0x0001, new CellPortalInfo(0x0002, 0, 0)); + var a = Cell(0x0001, new CellPortalInfo(0x0002, 0, 0, 0)); a.PortalPolygons.Add(Quad(0f, 0f, 0.5f, 0.5f, -3f)); - var b = Cell(0x0002, new CellPortalInfo(0x0001, 0, 0), new CellPortalInfo(0xFFFF, 1, 0)); + var b = Cell(0x0002, new CellPortalInfo(0x0001, 0, 0, 0), new CellPortalInfo(0xFFFF, 1, 0, 0)); b.PortalPolygons.Add(Quad(0f, 0f, 0.5f, 0.5f, -2f)); // back to A b.PortalPolygons.Add(Quad(0f, 0f, 1.0f, 1.0f, -6f)); // exit window var all = new Dictionary { [0x0001] = a, [0x0002] = b }; @@ -134,11 +143,11 @@ public class PortalVisibilityBuilderTests private static (LoadedCell[] cells, Dictionary lookup) SyntheticChain() { const uint A = 0x0001, B = 0x0002, C = 0x0003; - var a = Cell(A, new CellPortalInfo((ushort)B, 0, 0)); + var a = Cell(A, new CellPortalInfo((ushort)B, 0, 0, 0)); a.PortalPolygons.Add(Quad(0f, 0f, 0.6f, 0.6f, -2f)); // portal A->B at z=-2 (nearer) - var b = Cell(B, new CellPortalInfo((ushort)C, 0, 0)); + var b = Cell(B, new CellPortalInfo((ushort)C, 0, 0, 0)); b.PortalPolygons.Add(Quad(0f, 0f, 0.6f, 0.6f, -5f)); // portal B->C at z=-5 (farther) - var c = Cell(C, new CellPortalInfo(0xFFFF, 0, 0)); + var c = Cell(C, new CellPortalInfo(0xFFFF, 0, 0, 0)); c.PortalPolygons.Add(Quad(0f, 0f, 0.6f, 0.6f, -8f)); // exit window var all = new Dictionary { [A] = a, [B] = b, [C] = c }; return (new[] { a, b, c }, all); @@ -163,14 +172,14 @@ public class PortalVisibilityBuilderTests uint[] rooms = { 0x0011, 0x0012, 0x0013, 0x0014 }; // Hub has one portal to each room; rooms sit at distinct depths so ordering is deterministic. var hub = Cell(HUB, - new CellPortalInfo((ushort)rooms[0], 0, 0), new CellPortalInfo((ushort)rooms[1], 1, 0), - new CellPortalInfo((ushort)rooms[2], 2, 0), new CellPortalInfo((ushort)rooms[3], 3, 0)); + new CellPortalInfo((ushort)rooms[0], 0, 0, 0), new CellPortalInfo((ushort)rooms[1], 1, 0, 0), + new CellPortalInfo((ushort)rooms[2], 2, 0, 0), new CellPortalInfo((ushort)rooms[3], 3, 0, 0)); for (int i = 0; i < 4; i++) hub.PortalPolygons.Add(Quad(0f, 0f, 0.6f, 0.6f, -2f - i)); // -2,-3,-4,-5 var all = new Dictionary { [HUB] = hub }; for (int i = 0; i < 4; i++) { - var room = Cell(rooms[i], new CellPortalInfo((ushort)HUB, 0, 0)); // links back to hub → cycle + var room = Cell(rooms[i], new CellPortalInfo((ushort)HUB, 0, 0, 0)); // links back to hub → cycle room.PortalPolygons.Add(Quad(0f, 0f, 0.6f, 0.6f, -2f - i)); all[rooms[i]] = room; } @@ -205,13 +214,14 @@ public class PortalVisibilityBuilderTests 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's portal into B: wide opening (half-width 0.9) at z = -3. Its + // reciprocal back-portal lives at index 0 in B (OtherPortalId = 0). + var a = Cell(A, new CellPortalInfo((ushort)B, 0, 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)); + var b = Cell(B, new CellPortalInfo((ushort)A, 0, 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); @@ -253,7 +263,7 @@ public class PortalVisibilityBuilderTests // 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)); + var a = Cell(A, new CellPortalInfo((ushort)B, 0, 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 }; @@ -270,6 +280,90 @@ public class PortalVisibilityBuilderTests $"(got {area}, near-side {nearSideArea}) — degrade must not tighten or expand"); } + // ----------------------------------------------------------------------- + // Phase U.2b CRITICAL — multiple portals to the SAME neighbour must each + // resolve their OWN reciprocal back-portal (retail arg2->other_portal_id, + // decomp:433557), NOT the first OtherCellId match. This is the real + // Holtburg-cellar shape: cell 0x148 has two portals to 0x149 (poly 40, 41) + // and 0x149 has two reciprocals back to 0x148. A scan-by-first-match clips + // BOTH near-side openings against the FIRST reciprocal — if the apertures + // are disjoint, the second opening's intersection underflows to empty and + // its geometry is HIDDEN (under-inclusion). Direct-index resolution clips + // each opening against the matching reciprocal, so both survive. + // ----------------------------------------------------------------------- + + // Camera in A looking down -Z. A has TWO portals to the SAME neighbour B, + // with DISJOINT openings: a LEFT aperture (x in [-0.9,-0.3]) and a RIGHT + // aperture (x in [0.3,0.9]). B has two reciprocals back to A, one matching + // each side. A.Portals[0].OtherPortalId = 0 (B's LEFT reciprocal), + // A.Portals[1].OtherPortalId = 1 (B's RIGHT reciprocal). + // + // Scan-by-first-match resolves BOTH A-portals to B.Portals[0] (LEFT), + // so the RIGHT near-side region (x>0) intersects an x<0 reciprocal → empty + // → B's CellView never reaches the RIGHT aperture. Direct-index keeps it. + private static (LoadedCell camCell, LoadedCell neighbour, Dictionary lookup) + SyntheticMultiBackPortalPair() + { + const uint A = 0x0001, B = 0x0002; + // A: portal[0] → B through the LEFT opening, portal[1] → B through the RIGHT opening. + // The OtherPortalId back-links pick the matching reciprocal in B (0 = LEFT, 1 = RIGHT). + var a = Cell(A, + new CellPortalInfo((ushort)B, PolygonId: 0, Flags: 0, OtherPortalId: 0), // LEFT → B recip[0] + new CellPortalInfo((ushort)B, PolygonId: 1, Flags: 0, OtherPortalId: 1)); // RIGHT → B recip[1] + a.PortalPolygons.Add(QuadX(-0.9f, -0.3f, -3f)); // LEFT near-side aperture + a.PortalPolygons.Add(QuadX(0.3f, 0.9f, -3f)); // RIGHT near-side aperture + + // B: two reciprocals back to A. Index 0 covers the LEFT opening, index 1 + // the RIGHT opening — disjoint, same plane (z=-3) as the near side so each + // reciprocal exactly overlaps its matching A aperture. + var b = Cell(B, + new CellPortalInfo((ushort)A, PolygonId: 0, Flags: 0, OtherPortalId: 0), + new CellPortalInfo((ushort)A, PolygonId: 1, Flags: 0, OtherPortalId: 1)); + b.PortalPolygons.Add(QuadX(-0.9f, -0.3f, -3f)); // reciprocal[0] = LEFT + b.PortalPolygons.Add(QuadX(0.3f, 0.9f, -3f)); // reciprocal[1] = RIGHT + + var all = new Dictionary { [A] = a, [B] = b }; + return (a, b, all); + } + + [Fact] + public void Build_MultiplePortalsToSameNeighbour_EachResolvesOwnReciprocal() + { + var (camCell, neighbour, lookup) = SyntheticMultiBackPortalPair(); + 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 RIGHT reciprocal opening (B.PortalPolygons[1]) projects to a region + // with strictly positive NDC X. Geometry visible through the SECOND opening + // survives ONLY if A.Portals[1] was clipped against B's RIGHT reciprocal + // (index 1) rather than the LEFT one (index 0). + var rightNdc = PortalProjection.ProjectToNdc(neighbour.PortalPolygons[1], Matrix4x4.Identity, vp); + float rightMinX = float.MaxValue, rightMaxX = float.MinValue; + foreach (var v in rightNdc) { if (v.X < rightMinX) rightMinX = v.X; if (v.X > rightMaxX) rightMaxX = v.X; } + Assert.True(rightMinX > 0f, $"fixture sanity: RIGHT reciprocal must project to positive NDC X (got minX {rightMinX})"); + + // The neighbour CellView must extend into the RIGHT aperture. With the + // scan-by-first-match bug both near-side openings clip against the LEFT + // reciprocal (x<0), so the CellView's MaxX stays well left of rightMinX + // and this assertion FAILS (the RIGHT opening's geometry is hidden). + var bView = f.CellViews[0x0002]; + Assert.True(bView.MaxX >= rightMinX - 1e-4f, + $"neighbour CellView MaxX {bView.MaxX} must reach the RIGHT reciprocal opening " + + $"[{rightMinX}, {rightMaxX}] — under scan-by-first-match the second opening is clipped " + + $"against the LEFT reciprocal and HIDDEN (under-inclusion bug #102 M-4)."); + + // And the LEFT aperture must still be present (the first opening was never + // in question) — guards against a fix that accidentally drops the LEFT side. + var leftNdc = PortalProjection.ProjectToNdc(neighbour.PortalPolygons[0], Matrix4x4.Identity, vp); + float leftMinX = float.MaxValue; + foreach (var v in leftNdc) if (v.X < leftMinX) leftMinX = v.X; + Assert.True(bView.MinX <= leftMinX + 1e-4f, + $"neighbour CellView MinX {bView.MinX} must still cover the LEFT reciprocal opening (minX {leftMinX})"); + } + // Signed-area-magnitude (shoelace) sum over a CellView's polygons in NDC. private static float CellViewArea(CellView view) { diff --git a/tests/AcDream.App.Tests/Rendering/Wb/BuildingLoaderTests.cs b/tests/AcDream.App.Tests/Rendering/Wb/BuildingLoaderTests.cs index 4edb778..e308449 100644 --- a/tests/AcDream.App.Tests/Rendering/Wb/BuildingLoaderTests.cs +++ b/tests/AcDream.App.Tests/Rendering/Wb/BuildingLoaderTests.cs @@ -136,7 +136,7 @@ public class BuildingLoaderTests CellId = 0xA9B40150u, Portals = new List { - new(0xFFFF, 0, 0), + new(0xFFFF, 0, 0, 0), }, PortalPolygons = new List {