erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(render): Phase U.2b — resolve reciprocal portal by other_portal_id (retail 433557)

Browse source 
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) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-30 16:56:00 +02:00
parent 3916b2b23e
commit 65781f5768
7 changed files with 184 additions and 66 deletions
Download patch file Download diff file Expand all files Collapse all files

11
src/AcDream.App/Rendering/CellVisibility.cs
View file

@ -88,8 +88,17 @@ public sealed class LoadedCell
/// <summary>
/// Portal connection to a neighbouring cell.
/// OtherCellId == 0xFFFF indicates an exit portal to the outdoor world.
/// <para>
/// <see cref="OtherPortalId"/> 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
/// (<c>arg2-&gt;other_portal_id</c>, 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.
/// </para>
/// </summary>
public readonly record struct CellPortalInfo(ushort OtherCellId, ushort PolygonId, ushort Flags);
public readonly record struct CellPortalInfo(
ushort OtherCellId, ushort PolygonId, ushort Flags, ushort OtherPortalId);
/// <summary>
/// Clip plane derived from a portal polygon, in cell-local space.

3
src/AcDream.App/Rendering/GameWindow.cs
View file

@ -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)

60
src/AcDream.App/Rendering/PortalVisibilityBuilder.cs
View file

@ -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<ViewPolygon> clippedRegion, uint fromCellId, LoadedCell neighbour, Matrix4x4 viewProj)
List<ViewPolygon> 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.