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

refactor(render): Stage 3 T3.1 — delete FindCameraCell AABB grace-frame fallback

Browse source 
ComputeVisibilityFromRoot(null, …) now returns null (outdoor root) instead of
calling FindCameraCell(fallbackPos). Retail CellManager::ChangePosition
(0x004559B0) reads the transition-owned curr_cell — it does NOT re-derive from
a static position. W2a guarantees CurrCell is set from the first tick, so the
AABB fallback is dead. Deleted: FindCameraCell (389–446), _lastCameraCell,
_cellSwitchGraceFrames, CellSwitchGraceFrameCount. GetVisibleCells retains a
brute-force AABB scan for test-compat; ComputeVisibility stays for the same
reason. Updated 3 null-root tests in CellVisibilityFromRootTests to assert the
new null-returns-null behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-06-02 15:36:47 +02:00
parent fcea816391
commit 6a1fbbd44e
2 changed files with 76 additions and 177 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -1,9 +1,11 @@
// CellVisibility.cs — portal-based interior cell visibility system. // CellVisibility.cs — portal-based interior cell visibility system.
// //
// Ported from ACME EnvCellManager.cs (WorldBuilder-ACME-Edition). // Stage 3 (2026-06-02): FindCameraCell + grace-frame AABB fallback deleted.
// Key methods: FindCameraCell, PointInCell, GetVisibleCells. // The physics membership answer (CellGraph.CurrCell) is now the mandatory root;
// Constants: PointInCellEpsilon = 0.01f, CellSwitchGraceFrameCount = 3 // ComputeVisibilityFromRoot(null, …) returns null (outdoor root) rather than
// (ACME values; the original spec suggested 0.1f / 5 but ACME is ground truth). // falling back to an independent AABB position resolve. This matches retail's
// CellManager::ChangePosition (0x004559B0) which does not re-derive the cell
// from a static position — it reads the swept transition-owned CurrCell.
// //
// This file is intentionally free of GL / rendering types. It depends only on // This file is intentionally free of GL / rendering types. It depends only on
// System.Numerics so it can be unit-tested without a GPU context. // System.Numerics so it can be unit-tested without a GPU context.
@ -143,8 +145,10 @@ public struct PortalClipPlane
} }
/// <summary> /// <summary>
/// Phase U.4c flap probe (diagnostic): which branch of /// Phase U.4c flap probe (diagnostic — OBSOLETE as of Stage 3). Previously tracked
/// <see cref="CellVisibility.FindCameraCell"/> resolved the camera cell. /// which branch of FindCameraCell (now deleted) resolved the camera cell. Retained
/// for binary compatibility with the [flap-cam] probe log site in GameWindow.cs that
/// still prints <see cref="LastCameraCellResolution"/> (always None post-Stage 3).
/// </summary> /// </summary>
public enum CameraCellResolution public enum CameraCellResolution
{ {
@ -200,19 +204,12 @@ public sealed class CellVisibility
// ------------------------------------------------------------------ // ------------------------------------------------------------------
/// <summary> /// <summary>
/// Epsilon applied to AABB containment tests so that a camera sitting /// Epsilon applied to AABB containment tests so that a position sitting
/// exactly on a cell wall is still considered inside. /// exactly on a cell wall is still considered inside.
/// Source: ACME EnvCellManager.cs PointInCellEpsilon = 0.01f. /// Source: ACME EnvCellManager.cs PointInCellEpsilon = 0.01f.
/// </summary> /// </summary>
private const float PointInCellEpsilon = 0.01f; private const float PointInCellEpsilon = 0.01f;
/// <summary>
/// Number of frames to keep the previous camera cell alive after the camera
/// leaves it (prevents one-frame pop-in when crossing cell boundaries).
/// Source: ACME EnvCellManager.cs CellSwitchGraceFrameCount = 3.
/// </summary>
private const int CellSwitchGraceFrameCount = 3;
// ------------------------------------------------------------------ // ------------------------------------------------------------------
// State // State
// ------------------------------------------------------------------ // ------------------------------------------------------------------
@ -223,20 +220,13 @@ public sealed class CellVisibility
/// <summary>Full-ID lookup for O(1) neighbour resolution during BFS.</summary> /// <summary>Full-ID lookup for O(1) neighbour resolution during BFS.</summary>
private readonly Dictionary<uint, LoadedCell> _cellLookup = new(); private readonly Dictionary<uint, LoadedCell> _cellLookup = new();
/// <summary>The cell the camera was in during the last <see cref="ComputeVisibility"/> call.</summary> /// <summary>The last visibility result produced by <see cref="ComputeVisibilityFromRoot"/>.</summary>
private LoadedCell? _lastCameraCell;
/// <summary>Frames remaining in the grace period after the camera left _lastCameraCell.</summary>
private int _cellSwitchGraceFrames;
/// <summary>The last visibility result produced by <see cref="ComputeVisibility"/>.</summary>
public VisibilityResult? LastVisibilityResult { get; private set; } public VisibilityResult? LastVisibilityResult { get; private set; }
/// <summary> /// <summary>
/// Phase U.4c flap probe (diagnostic): which <see cref="FindCameraCell"/> branch /// Stage 3 (2026-06-02): always <see cref="CameraCellResolution.None"/> — the FindCameraCell
/// resolved the camera cell on the most recent call. A <see cref="CameraCellResolution.Grace"/> /// AABB grace-frame resolver was deleted; the physics membership answer is the sole root.
/// (or <see cref="CameraCellResolution.Cache"/>) result while the eye is NOT actually inside the /// Retained for the [flap-cam] probe log line in GameWindow.cs.
/// returned cell is the "stale root" signature the flap probe looks for.
/// </summary> /// </summary>
public CameraCellResolution LastCameraCellResolution { get; private set; } = CameraCellResolution.None; public CameraCellResolution LastCameraCellResolution { get; private set; } = CameraCellResolution.None;
@ -299,14 +289,6 @@ public sealed class CellVisibility
foreach (var cell in list) foreach (var cell in list)
{ {
_cellLookup.Remove(cell.CellId); _cellLookup.Remove(cell.CellId);
// If the evicted cell was cached, clear the cache so FindCameraCell
// does a fresh brute-force scan next frame.
if (_lastCameraCell?.CellId == cell.CellId)
{
_lastCameraCell = null;
_cellSwitchGraceFrames = 0;
}
} }
_cellsByLandblock.Remove(lbId); _cellsByLandblock.Remove(lbId);
@ -317,12 +299,11 @@ public sealed class CellVisibility
// ------------------------------------------------------------------ // ------------------------------------------------------------------
/// <summary> /// <summary>
/// Computes portal-based visibility from <paramref name="cameraPos"/> and /// Computes portal-based visibility from <paramref name="cameraPos"/> using the
/// caches the result in <see cref="LastVisibilityResult"/>. /// AABB FindCameraCell resolver. Retained for test compatibility only; production
/// /// code should use <see cref="ComputeVisibilityFromRoot"/> with a physics-supplied
/// Call once per frame, before the render pass. Returns null when the camera /// root (Stage 3 demotes the AABB resolver to test-only use).
/// is outside all loaded cells (outdoor — caller should fall back to frustum /// Returns null when no loaded cell contains <paramref name="cameraPos"/>.
/// culling of terrain).
/// </summary> /// </summary>
public VisibilityResult? ComputeVisibility(Vector3 cameraPos) public VisibilityResult? ComputeVisibility(Vector3 cameraPos)
{ {
@ -337,26 +318,29 @@ public sealed class CellVisibility
} }
/// <summary> /// <summary>
/// UCG W2: compute visibility from a supplied root cell (the physics membership answer) /// UCG W2/Stage 3: compute visibility from a supplied root cell (the physics membership
/// rather than resolving the root from a position. Falls back to the position-based /// answer). When <paramref name="root"/> is null (pre-spawn, or player outside all indoor
/// <see cref="ComputeVisibility(Vector3)"/> when <paramref name="root"/> is null (e.g. the /// cells), returns <c>null</c> — the caller interprets null as the outdoor root (no portal
/// cell isn't registered with this renderer yet), so it never regresses below baseline. /// frame, everything slot 0, terrain ungated). The legacy AABB FindCameraCell fallback is
/// deleted as of Stage 3; <see cref="CellGraph.CurrCell"/> is the sole authority.
/// Retail anchor: CellManager::ChangePosition @ 0x004559B0 reads the transition-owned
/// curr_cell — it does NOT re-derive from a static position.
/// </summary> /// </summary>
/// <param name="root"> /// <param name="root">
/// The render-registered <see cref="LoadedCell"/> that physics determined the player is inside, /// The render-registered <see cref="LoadedCell"/> that physics determined the player is inside,
/// or null if the physics answer isn't usable yet (no cells registered, or the physics cell id /// or null when pre-spawn or the player is in an outdoor landcell. Null → outdoor root path.
/// hasn't loaded into the render system). Null triggers the legacy <see cref="ComputeVisibility"/>
/// path, which preserves today's exact AABB-based behavior.
/// </param> /// </param>
/// <param name="fallbackPos"> /// <param name="fallbackPos">
/// Position passed to <see cref="ComputeVisibility"/> when <paramref name="root"/> is null, /// Used as the viewer position for the portal-side test in the BFS when root is non-null.
/// AND used as the viewer position for the portal-side test in the BFS when root is non-null.
/// Should be the player/physics position (stable inside the cell), not the chase-camera eye. /// Should be the player/physics position (stable inside the cell), not the chase-camera eye.
/// The name "fallback" is historical; it is no longer used as a fallback position.
/// </param> /// </param>
public VisibilityResult? ComputeVisibilityFromRoot(LoadedCell? root, Vector3 fallbackPos) public VisibilityResult? ComputeVisibilityFromRoot(LoadedCell? root, Vector3 fallbackPos)
{ {
if (root is null) if (root is null)
return ComputeVisibility(fallbackPos); return null; // outdoor root: caller handles null as "player is outside"
// Stage 3: FindCameraCell AABB grace-frame fallback deleted.
// Retail: CellManager::ChangePosition (0x004559B0) uses transition-owned CurrCell.
if (_cellLookup.Count == 0) if (_cellLookup.Count == 0)
{ {
@ -364,86 +348,22 @@ public sealed class CellVisibility
return null; return null;
} }
_lastCameraCell = root;
LastVisibilityResult = GetVisibleCellsFromRoot(root, fallbackPos); LastVisibilityResult = GetVisibleCellsFromRoot(root, fallbackPos);
return LastVisibilityResult; return LastVisibilityResult;
} }
// ------------------------------------------------------------------ // ------------------------------------------------------------------
// FindCameraCell // FindCameraCell — DELETED in Stage 3 (2026-06-02)
// ------------------------------------------------------------------
// The AABB + grace-frame camera-cell resolver was removed. Production code
// now exclusively uses ComputeVisibilityFromRoot(root, …) where root is the
// transition-owned CellGraph.CurrCell (set by ResolveCellId/Stage 2 physics).
// Retail anchor: CellManager::ChangePosition (0x004559B0) reads curr_cell
// from the sweep — it never re-derives from a static position.
//
// GetVisibleCells (used by ComputeVisibility below for test compatibility)
// still uses the brute-force AABB scan internally.
// ------------------------------------------------------------------ // ------------------------------------------------------------------
/// <summary>
/// Finds the <see cref="LoadedCell"/> the camera is currently inside, with
/// a short hysteresis window to prevent flicker at cell boundaries.
///
/// Search order:
/// 1. Cached cell fast path.
/// 2. Immediate portal neighbours of the cached cell.
/// 3. Brute-force scan of all loaded cells.
/// 4. Grace period — return the previous cell for a few frames.
/// 5. Return null (camera is outdoors).
///
/// Ported from ACME EnvCellManager.cs FindCameraCell().
/// </summary>
public LoadedCell? FindCameraCell(Vector3 cameraPos)
{
// 1. Fast path: cached cell.
if (_lastCameraCell != null && PointInCell(cameraPos, _lastCameraCell))
{
LastCameraCellResolution = CameraCellResolution.Cache;
return _lastCameraCell;
}
// 2. One-hop neighbours of the cached cell.
if (_lastCameraCell != null)
{
uint lbMask = _lastCameraCell.CellId & 0xFFFF0000u;
foreach (var portal in _lastCameraCell.Portals)
{
if (portal.OtherCellId == 0xFFFF)
continue;
uint neighbourId = lbMask | portal.OtherCellId;
if (_cellLookup.TryGetValue(neighbourId, out var neighbour) &&
PointInCell(cameraPos, neighbour))
{
_lastCameraCell = neighbour;
_cellSwitchGraceFrames = CellSwitchGraceFrameCount;
LastCameraCellResolution = CameraCellResolution.Neighbour;
return neighbour;
}
}
}
// 3. Brute-force scan.
foreach (var kvp in _cellsByLandblock)
{
foreach (var cell in kvp.Value)
{
if (PointInCell(cameraPos, cell))
{
_lastCameraCell = cell;
_cellSwitchGraceFrames = CellSwitchGraceFrameCount;
LastCameraCellResolution = CameraCellResolution.BruteForce;
return cell;
}
}
}
// 4. Grace period: keep the previous cell alive for a few frames.
if (_lastCameraCell != null && _cellSwitchGraceFrames > 0)
{
_cellSwitchGraceFrames--;
LastCameraCellResolution = CameraCellResolution.Grace;
return _lastCameraCell;
}
// 5. Camera is outside all cells.
_lastCameraCell = null;
LastCameraCellResolution = CameraCellResolution.None;
return null;
}
// ------------------------------------------------------------------ // ------------------------------------------------------------------
// PointInCell // PointInCell
@ -477,11 +397,9 @@ public sealed class CellVisibility
/// <summary> /// <summary>
/// Brute-force scan of every loaded cell to test whether /// Brute-force scan of every loaded cell to test whether
/// <paramref name="worldPoint"/> is inside any of them. Does not touch /// <paramref name="worldPoint"/> is inside any of them. Safe to call
/// the camera cache (<see cref="_lastCameraCell"/>), so this is safe /// independently of <see cref="ComputeVisibilityFromRoot"/> in the same
/// to call alongside <see cref="ComputeVisibility"/> in the same frame /// frame for a different position.
/// for a different position (e.g. player position when the camera is
/// in third-person chase mode).
/// </summary> /// </summary>
public bool IsInsideAnyCell(Vector3 worldPoint) public bool IsInsideAnyCell(Vector3 worldPoint)
{ {
@ -496,31 +414,19 @@ public sealed class CellVisibility
/// <summary> /// <summary>
/// Performs portal-based BFS visibility traversal starting from the camera /// Performs portal-based BFS visibility traversal starting from the camera
/// cell. Returns null when the camera is outside all loaded cells. /// cell found by an AABB brute-force scan. Returns null when no loaded cell
/// /// contains <paramref name="cameraPos"/>. Used only by
/// Algorithm: /// <see cref="ComputeVisibility"/> (test-compatibility path); production code
/// • Start with the camera cell in the visited set and the work queue. /// uses <see cref="ComputeVisibilityFromRoot"/> with the physics-supplied root.
/// • For each dequeued cell, iterate its portals:
/// – OtherCellId == 0xFFFF → exit portal, set HasExitPortalVisible.
/// – Already visited → skip.
/// – Not loaded → skip.
/// – Portal-side test: transform camera to cell-local space, dot with
/// clip plane; skip if camera is on the wrong side.
/// – Enqueue neighbour and add to VisibleCellIds.
///
/// Note: ACME also applies a frustum test after the portal-side test. That
/// test is omitted here because <see cref="CellVisibility"/> is a pure-logic
/// class. Callers that have a frustum can post-filter VisibleCellIds.
///
/// The landblock mask for neighbour resolution is taken from the camera
/// cell's CellId (upper 16 bits). All portals in a dungeon are assumed to
/// connect cells within the same landblock.
///
/// Ported from ACME EnvCellManager.cs GetVisibleCells().
/// </summary> /// </summary>
public VisibilityResult? GetVisibleCells(Vector3 cameraPos) public VisibilityResult? GetVisibleCells(Vector3 cameraPos)
{ {
var cameraCell = FindCameraCell(cameraPos); // Brute-force AABB scan (test-compatibility; FindCameraCell was deleted in Stage 3).
LoadedCell? cameraCell = null;
foreach (var kvp in _cellsByLandblock)
foreach (var cell in kvp.Value)
if (PointInCell(cameraPos, cell)) { cameraCell = cell; break; }
if (cameraCell == null) if (cameraCell == null)
return null; return null;
@ -529,11 +435,11 @@ public sealed class CellVisibility
/// <summary> /// <summary>
/// UCG W2: BFS visibility traversal from a pre-resolved root cell. /// UCG W2: BFS visibility traversal from a pre-resolved root cell.
/// The root is assumed to be the correct membership answer (supplied by the /// The root is the correct membership answer (supplied by the caller —
/// caller — either <see cref="GetVisibleCells"/> via <see cref="FindCameraCell"/>, /// physics CurrCell via <see cref="ComputeVisibilityFromRoot"/>, or AABB
/// or <see cref="ComputeVisibilityFromRoot"/> via the physics CurrCell answer). /// scan via <see cref="GetVisibleCells"/> for test compat).
/// ///
/// The BFS body is byte-identical to the original <see cref="GetVisibleCells"/> /// The BFS body is byte-identical to the original GetVisibleCells
/// implementation — only root acquisition was extracted out. /// implementation — only root acquisition was extracted out.
/// </summary> /// </summary>
private VisibilityResult? GetVisibleCellsFromRoot(LoadedCell cameraCell, Vector3 cameraPos) private VisibilityResult? GetVisibleCellsFromRoot(LoadedCell cameraCell, Vector3 cameraPos)

39
tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs
View file

@ -1,9 +1,9 @@
// CellVisibilityFromRootTests.cs — UCG W2 Task 2: tests for // CellVisibilityFromRootTests.cs — UCG W2 Task 2 + Stage 3: tests for
// CellVisibility.ComputeVisibilityFromRoot. // CellVisibility.ComputeVisibilityFromRoot.
// //
// Two acceptance criteria (from the W2 Task 2 spec): // Acceptance criteria (Stage 3 — W2 null-fallback deleted):
// (a) ComputeVisibilityFromRoot(null, pos) is fallback-equivalent to // (a) ComputeVisibilityFromRoot(null, pos) returns NULL (outdoor root), regardless
// ComputeVisibility(pos) — same CameraCell answer. // of whether any cells are registered. The AABB FindCameraCell fallback is gone.
// (b) ComputeVisibilityFromRoot(root, pos) with a registered root returns // (b) ComputeVisibilityFromRoot(root, pos) with a registered root returns
// a result whose CameraCell is that root, regardless of whether 'pos' // a result whose CameraCell is that root, regardless of whether 'pos'
// is geometrically inside it. // is geometrically inside it.
@ -43,60 +43,53 @@ public class CellVisibilityFromRootTests
} }
// ------------------------------------------------------------------ // ------------------------------------------------------------------
// (a) Fallback equivalence: null root → same answer as ComputeVisibility // (a) Stage 3: null root → null (outdoor root), not a position fallback
// ------------------------------------------------------------------ // ------------------------------------------------------------------
[Fact] [Fact]
public void ComputeVisibilityFromRoot_NullRoot_FallsBackToPositionBased() public void ComputeVisibilityFromRoot_NullRoot_ReturnsNull_WhenCellExists()
{ {
// Arrange: one cell covering [0,10]^3, position inside it. // Stage 3: null root → outdoor root → null result, even when a cell covers the
// fallback position. Pre-Stage 3 this called FindCameraCell(pos); now the caller
// must supply the root (physics CellGraph.CurrCell). Retail: CellManager::ChangePosition
// reads the transition-owned curr_cell — it does not re-derive from a static position.
var cv = new CellVisibility(); var cv = new CellVisibility();
var cell = MakeCell(0xA9B40101u, Vector3.Zero, new Vector3(10, 10, 10)); var cell = MakeCell(0xA9B40101u, Vector3.Zero, new Vector3(10, 10, 10));
cv.AddCell(cell); cv.AddCell(cell);
var pos = new Vector3(5, 5, 5); // inside the cell var pos = new Vector3(5, 5, 5); // inside the cell — null root overrides
// Act: null-root and direct-position paths must agree on CameraCell.
var fromPos = cv.ComputeVisibility(pos);
var fromNull = cv.ComputeVisibilityFromRoot(null, pos); var fromNull = cv.ComputeVisibilityFromRoot(null, pos);
// Assert // Stage 3: null root → null (outdoor root path).
Assert.NotNull(fromPos); Assert.Null(fromNull);
Assert.NotNull(fromNull);
Assert.Equal(fromPos!.CameraCell?.CellId, fromNull!.CameraCell?.CellId);
Assert.Equal(cell.CellId, fromNull.CameraCell?.CellId);
} }
[Fact] [Fact]
public void ComputeVisibilityFromRoot_NullRoot_NoCells_ReturnsNull() public void ComputeVisibilityFromRoot_NullRoot_NoCells_ReturnsNull()
{ {
// With no cells registered both paths return null. // With no cells registered and null root: always null (outdoor root).
var cv = new CellVisibility(); var cv = new CellVisibility();
var posOutdoors = new Vector3(100, 100, 100); var posOutdoors = new Vector3(100, 100, 100);
var fromPos = cv.ComputeVisibility(posOutdoors);
var fromNull = cv.ComputeVisibilityFromRoot(null, posOutdoors); var fromNull = cv.ComputeVisibilityFromRoot(null, posOutdoors);
Assert.Null(fromPos);
Assert.Null(fromNull); Assert.Null(fromNull);
} }
[Fact] [Fact]
public void ComputeVisibilityFromRoot_NullRoot_PositionOutsideAllCells_ReturnsNull() public void ComputeVisibilityFromRoot_NullRoot_PositionOutsideAllCells_ReturnsNull()
{ {
// Cell exists but position is outside it — both paths produce null CameraCell. // Cell exists but null root: always null regardless of position.
var cv = new CellVisibility(); var cv = new CellVisibility();
var cell = MakeCell(0xA9B40102u, Vector3.Zero, new Vector3(5, 5, 5)); var cell = MakeCell(0xA9B40102u, Vector3.Zero, new Vector3(5, 5, 5));
cv.AddCell(cell); cv.AddCell(cell);
var posOutside = new Vector3(100, 100, 100); var posOutside = new Vector3(100, 100, 100);
var fromPos = cv.ComputeVisibility(posOutside);
var fromNull = cv.ComputeVisibilityFromRoot(null, posOutside); var fromNull = cv.ComputeVisibilityFromRoot(null, posOutside);
// Both should return null (no grace frames built up yet) Assert.Null(fromNull);
Assert.Null(fromPos?.CameraCell);
Assert.Null(fromNull?.CameraCell);
} }
// ------------------------------------------------------------------ // ------------------------------------------------------------------