diff --git a/src/AcDream.App/Rendering/CellVisibility.cs b/src/AcDream.App/Rendering/CellVisibility.cs index fc4b5a1..80477c0 100644 --- a/src/AcDream.App/Rendering/CellVisibility.cs +++ b/src/AcDream.App/Rendering/CellVisibility.cs @@ -1,9 +1,11 @@ // CellVisibility.cs — portal-based interior cell visibility system. // -// Ported from ACME EnvCellManager.cs (WorldBuilder-ACME-Edition). -// Key methods: FindCameraCell, PointInCell, GetVisibleCells. -// Constants: PointInCellEpsilon = 0.01f, CellSwitchGraceFrameCount = 3 -// (ACME values; the original spec suggested 0.1f / 5 but ACME is ground truth). +// Stage 3 (2026-06-02): FindCameraCell + grace-frame AABB fallback deleted. +// The physics membership answer (CellGraph.CurrCell) is now the mandatory root; +// ComputeVisibilityFromRoot(null, …) returns null (outdoor root) rather than +// 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 // System.Numerics so it can be unit-tested without a GPU context. @@ -143,8 +145,10 @@ public struct PortalClipPlane } /// -/// Phase U.4c flap probe (diagnostic): which branch of -/// resolved the camera cell. +/// Phase U.4c flap probe (diagnostic — OBSOLETE as of Stage 3). Previously tracked +/// 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 (always None post-Stage 3). /// public enum CameraCellResolution { @@ -200,19 +204,12 @@ public sealed class CellVisibility // ------------------------------------------------------------------ /// - /// 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. /// Source: ACME EnvCellManager.cs PointInCellEpsilon = 0.01f. /// private const float PointInCellEpsilon = 0.01f; - /// - /// 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. - /// - private const int CellSwitchGraceFrameCount = 3; - // ------------------------------------------------------------------ // State // ------------------------------------------------------------------ @@ -223,20 +220,13 @@ public sealed class CellVisibility /// Full-ID lookup for O(1) neighbour resolution during BFS. private readonly Dictionary _cellLookup = new(); - /// The cell the camera was in during the last call. - private LoadedCell? _lastCameraCell; - - /// Frames remaining in the grace period after the camera left _lastCameraCell. - private int _cellSwitchGraceFrames; - - /// The last visibility result produced by . + /// The last visibility result produced by . public VisibilityResult? LastVisibilityResult { get; private set; } /// - /// Phase U.4c flap probe (diagnostic): which branch - /// resolved the camera cell on the most recent call. A - /// (or ) result while the eye is NOT actually inside the - /// returned cell is the "stale root" signature the flap probe looks for. + /// Stage 3 (2026-06-02): always — the FindCameraCell + /// AABB grace-frame resolver was deleted; the physics membership answer is the sole root. + /// Retained for the [flap-cam] probe log line in GameWindow.cs. /// public CameraCellResolution LastCameraCellResolution { get; private set; } = CameraCellResolution.None; @@ -299,14 +289,6 @@ public sealed class CellVisibility foreach (var cell in list) { _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); @@ -317,12 +299,11 @@ public sealed class CellVisibility // ------------------------------------------------------------------ /// - /// Computes portal-based visibility from and - /// caches the result in . - /// - /// Call once per frame, before the render pass. Returns null when the camera - /// is outside all loaded cells (outdoor — caller should fall back to frustum - /// culling of terrain). + /// Computes portal-based visibility from using the + /// AABB FindCameraCell resolver. Retained for test compatibility only; production + /// code should use with a physics-supplied + /// root (Stage 3 demotes the AABB resolver to test-only use). + /// Returns null when no loaded cell contains . /// public VisibilityResult? ComputeVisibility(Vector3 cameraPos) { @@ -337,26 +318,29 @@ public sealed class CellVisibility } /// - /// UCG W2: compute visibility from a supplied root cell (the physics membership answer) - /// rather than resolving the root from a position. Falls back to the position-based - /// when is null (e.g. the - /// cell isn't registered with this renderer yet), so it never regresses below baseline. + /// UCG W2/Stage 3: compute visibility from a supplied root cell (the physics membership + /// answer). When is null (pre-spawn, or player outside all indoor + /// cells), returns null — the caller interprets null as the outdoor root (no portal + /// frame, everything slot 0, terrain ungated). The legacy AABB FindCameraCell fallback is + /// deleted as of Stage 3; is the sole authority. + /// Retail anchor: CellManager::ChangePosition @ 0x004559B0 reads the transition-owned + /// curr_cell — it does NOT re-derive from a static position. /// /// /// The render-registered 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 - /// hasn't loaded into the render system). Null triggers the legacy - /// path, which preserves today's exact AABB-based behavior. + /// or null when pre-spawn or the player is in an outdoor landcell. Null → outdoor root path. /// /// - /// Position passed to when is null, - /// AND used as the viewer position for the portal-side test in the BFS when root is non-null. + /// 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. + /// The name "fallback" is historical; it is no longer used as a fallback position. /// public VisibilityResult? ComputeVisibilityFromRoot(LoadedCell? root, Vector3 fallbackPos) { 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) { @@ -364,86 +348,22 @@ public sealed class CellVisibility return null; } - _lastCameraCell = root; LastVisibilityResult = GetVisibleCellsFromRoot(root, fallbackPos); 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. // ------------------------------------------------------------------ - - /// - /// Finds the 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(). - /// - 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 @@ -477,11 +397,9 @@ public sealed class CellVisibility /// /// Brute-force scan of every loaded cell to test whether - /// is inside any of them. Does not touch - /// the camera cache (), so this is safe - /// to call alongside in the same frame - /// for a different position (e.g. player position when the camera is - /// in third-person chase mode). + /// is inside any of them. Safe to call + /// independently of in the same + /// frame for a different position. /// public bool IsInsideAnyCell(Vector3 worldPoint) { @@ -496,31 +414,19 @@ public sealed class CellVisibility /// /// Performs portal-based BFS visibility traversal starting from the camera - /// cell. Returns null when the camera is outside all loaded cells. - /// - /// Algorithm: - /// • Start with the camera cell in the visited set and the work queue. - /// • 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 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(). + /// cell found by an AABB brute-force scan. Returns null when no loaded cell + /// contains . Used only by + /// (test-compatibility path); production code + /// uses with the physics-supplied root. /// 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) return null; @@ -529,11 +435,11 @@ public sealed class CellVisibility /// /// UCG W2: BFS visibility traversal from a pre-resolved root cell. - /// The root is assumed to be the correct membership answer (supplied by the - /// caller — either via , - /// or via the physics CurrCell answer). + /// The root is the correct membership answer (supplied by the caller — + /// physics CurrCell via , or AABB + /// scan via for test compat). /// - /// The BFS body is byte-identical to the original + /// The BFS body is byte-identical to the original GetVisibleCells /// implementation — only root acquisition was extracted out. /// private VisibilityResult? GetVisibleCellsFromRoot(LoadedCell cameraCell, Vector3 cameraPos) diff --git a/tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs b/tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs index 682c080..a9e4b6f 100644 --- a/tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs +++ b/tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs @@ -1,9 +1,9 @@ -// CellVisibilityFromRootTests.cs — UCG W2 Task 2: tests for +// CellVisibilityFromRootTests.cs — UCG W2 Task 2 + Stage 3: tests for // CellVisibility.ComputeVisibilityFromRoot. // -// Two acceptance criteria (from the W2 Task 2 spec): -// (a) ComputeVisibilityFromRoot(null, pos) is fallback-equivalent to -// ComputeVisibility(pos) — same CameraCell answer. +// Acceptance criteria (Stage 3 — W2 null-fallback deleted): +// (a) ComputeVisibilityFromRoot(null, pos) returns NULL (outdoor root), regardless +// of whether any cells are registered. The AABB FindCameraCell fallback is gone. // (b) ComputeVisibilityFromRoot(root, pos) with a registered root returns // a result whose CameraCell is that root, regardless of whether 'pos' // 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] - 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 cell = MakeCell(0xA9B40101u, Vector3.Zero, new Vector3(10, 10, 10)); 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); - // Assert - Assert.NotNull(fromPos); - Assert.NotNull(fromNull); - Assert.Equal(fromPos!.CameraCell?.CellId, fromNull!.CameraCell?.CellId); - Assert.Equal(cell.CellId, fromNull.CameraCell?.CellId); + // Stage 3: null root → null (outdoor root path). + Assert.Null(fromNull); } [Fact] 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 posOutdoors = new Vector3(100, 100, 100); - var fromPos = cv.ComputeVisibility(posOutdoors); var fromNull = cv.ComputeVisibilityFromRoot(null, posOutdoors); - Assert.Null(fromPos); Assert.Null(fromNull); } [Fact] 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 cell = MakeCell(0xA9B40102u, Vector3.Zero, new Vector3(5, 5, 5)); cv.AddCell(cell); var posOutside = new Vector3(100, 100, 100); - var fromPos = cv.ComputeVisibility(posOutside); var fromNull = cv.ComputeVisibilityFromRoot(null, posOutside); - // Both should return null (no grace frames built up yet) - Assert.Null(fromPos?.CameraCell); - Assert.Null(fromNull?.CameraCell); + Assert.Null(fromNull); } // ------------------------------------------------------------------