From 2a3ec09a98087cb21ce7d65bd8c3dcf4b1994c0e Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 31 Aug 2026 04:46:20 +0200 Subject: [PATCH] refactor(render): delete the dead cell visibility bfs --- src/AcDream.App/Rendering/CellVisibility.cs | 217 +----------------- .../Rendering/CellVisibilityFromRootTests.cs | 143 ------------ 2 files changed, 8 insertions(+), 352 deletions(-) delete mode 100644 tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs diff --git a/src/AcDream.App/Rendering/CellVisibility.cs b/src/AcDream.App/Rendering/CellVisibility.cs index 26034263..eac7bf94 100644 --- a/src/AcDream.App/Rendering/CellVisibility.cs +++ b/src/AcDream.App/Rendering/CellVisibility.cs @@ -1,11 +1,8 @@ // CellVisibility.cs — portal-based interior cell visibility system. // -// 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. +// Campaign FW4 (2026-08-31): the obsolete per-frame portal BFS is deleted. +// The committed cell registry remains the production walk's authoritative +// source. Physics supplies the root and RetailFrameWalk owns visibility. // // 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. @@ -192,36 +189,9 @@ public enum CameraCellResolution } /// -/// Result of a portal-based visibility BFS from the camera cell. -/// -public sealed class VisibilityResult -{ - /// Full cell IDs (e.g. 0x01D90105) that should be rendered this frame. - public HashSet VisibleCellIds { get; init; } = new(); - - /// - /// True when at least one exit portal (OtherCellId == 0xFFFF) was reached during - /// traversal. The caller should render outdoor terrain when this is set. - /// - public bool HasExitPortalVisible { get; set; } - - /// The cell the camera is currently inside. - public LoadedCell? CameraCell { get; set; } -} - -// --------------------------------------------------------------------------- -// CellVisibility -// --------------------------------------------------------------------------- - -/// -/// Pure-logic portal visibility system for EnvCell interior rooms. -/// -/// Maintains a per-landblock registry of objects and -/// performs a BFS through portal connections each frame to determine which cells -/// should be rendered given the current camera position. -/// -/// Ported faithfully from ACME's EnvCellManager.cs portal-visibility region. -/// Constants and control flow match the ACME implementation. +/// Committed EnvCell registry shared by streaming, physics diagnostics, and the +/// retail frame walk. It owns no visibility algorithm; +/// is the sole per-frame visibility authority. /// public sealed class CellVisibility { @@ -243,12 +213,9 @@ public sealed class CellVisibility /// Per-landblock lists of loaded cells. Key = upper 16 bits of a cell ID. private readonly Dictionary> _cellsByLandblock = new(); - /// Full-ID lookup for O(1) neighbour resolution during BFS. + /// Full-ID lookup used by the production frame walk. private readonly Dictionary _cellLookup = new(); - /// The last visibility result produced by . - public VisibilityResult? LastVisibilityResult { get; private set; } - /// /// Stage 3 (2026-06-02): always — the FindCameraCell /// AABB grace-frame resolver was deleted; the physics membership answer is the sole root. @@ -346,77 +313,6 @@ public sealed class CellVisibility _cellsByLandblock.Remove(lbId); } - // ------------------------------------------------------------------ - // Per-frame entry points - // ------------------------------------------------------------------ - - /// - /// 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) - { - if (_cellLookup.Count == 0) - { - LastVisibilityResult = null; - return null; - } - - LastVisibilityResult = GetVisibleCells(cameraPos); - return LastVisibilityResult; - } - - /// - /// 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 when pre-spawn or the player is in an outdoor landcell. Null → outdoor root path. - /// - /// - /// 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 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) - { - LastVisibilityResult = null; - return null; - } - - LastVisibilityResult = GetVisibleCellsFromRoot(root, fallbackPos); - return LastVisibilityResult; - } - - // ------------------------------------------------------------------ - // 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. - // ------------------------------------------------------------------ - // ------------------------------------------------------------------ // PointInCell // ------------------------------------------------------------------ @@ -449,9 +345,7 @@ public sealed class CellVisibility /// /// Brute-force scan of every loaded cell to test whether - /// is inside any of them. Safe to call - /// independently of in the same - /// frame for a different position. + /// is inside any of them. /// public bool IsInsideAnyCell(Vector3 worldPoint) { @@ -460,99 +354,4 @@ public sealed class CellVisibility return false; } - // ------------------------------------------------------------------ - // GetVisibleCells (BFS) - // ------------------------------------------------------------------ - - /// - /// Performs portal-based BFS visibility traversal starting from the camera - /// 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) - { - // 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; - - return GetVisibleCellsFromRoot(cameraCell, cameraPos); - } - - /// - /// UCG W2: BFS visibility traversal from a pre-resolved root cell. - /// 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 GetVisibleCells - /// implementation — only root acquisition was extracted out. - /// - private VisibilityResult? GetVisibleCellsFromRoot(LoadedCell cameraCell, Vector3 cameraPos) - { - var result = new VisibilityResult { CameraCell = cameraCell }; - var visited = new HashSet(); - var queue = new Queue(); - - visited.Add(cameraCell.CellId); - result.VisibleCellIds.Add(cameraCell.CellId); - queue.Enqueue(cameraCell); - - // All portals in a dungeon connect cells in the same landblock. - uint lbMask = cameraCell.CellId & 0xFFFF0000u; - - while (queue.Count > 0) - { - var cell = queue.Dequeue(); - - for (int i = 0; i < cell.Portals.Count; i++) - { - var portal = cell.Portals[i]; - - // Exit portal → outdoor terrain should be visible. - if (portal.OtherCellId == 0xFFFF) - { - result.HasExitPortalVisible = true; - continue; - } - - uint neighbourId = lbMask | portal.OtherCellId; - - if (visited.Contains(neighbourId)) - continue; - - if (!_cellLookup.TryGetValue(neighbourId, out var neighbour)) - continue; - - // Portal-side test: camera must be on the interior side of the - // portal clip plane to see through into the neighbouring cell. - if (i < cell.ClipPlanes.Count) - { - var plane = cell.ClipPlanes[i]; - var localCam = Vector3.Transform(cameraPos, cell.InverseWorldTransform); - float dot = Vector3.Dot(plane.Normal, localCam) + plane.D; - - // InsideSide == 0 → inside is positive half-space; reject if dot < -ε. - // InsideSide == 1 → inside is negative half-space; reject if dot > ε. - // Source: ACME EnvCellManager.cs lines 1458-1459. - if (plane.InsideSide == 0 && dot < -PointInCellEpsilon) - continue; - if (plane.InsideSide == 1 && dot > PointInCellEpsilon) - continue; - } - - visited.Add(neighbourId); - result.VisibleCellIds.Add(neighbourId); - queue.Enqueue(neighbour); - } - } - - return result; - } } diff --git a/tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs b/tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs deleted file mode 100644 index a9e4b6f9..00000000 --- a/tests/AcDream.App.Tests/Rendering/CellVisibilityFromRootTests.cs +++ /dev/null @@ -1,143 +0,0 @@ -// CellVisibilityFromRootTests.cs — UCG W2 Task 2 + Stage 3: tests for -// CellVisibility.ComputeVisibilityFromRoot. -// -// 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. -// -// CellVisibility is intentionally free of GL types — it can be unit-tested -// without a GPU context (confirmed: only System.Numerics dependency). - -using System.Numerics; -using AcDream.App.Rendering; -using Xunit; - -namespace AcDream.App.Tests.Rendering; - -public class CellVisibilityFromRootTests -{ - // ------------------------------------------------------------------ - // Helpers - // ------------------------------------------------------------------ - - /// - /// Build a minimal LoadedCell with an axis-aligned bounding box and identity - /// transform so PointInCell works for a position inside the box. - /// - private static LoadedCell MakeCell(uint cellId, Vector3 boundsMin, Vector3 boundsMax) - { - return new LoadedCell - { - CellId = cellId, - WorldTransform = Matrix4x4.Identity, - InverseWorldTransform = Matrix4x4.Identity, - LocalBoundsMin = boundsMin, - LocalBoundsMax = boundsMax, - Portals = new(), - ClipPlanes = new(), - PortalPolygons = new(), - }; - } - - // ------------------------------------------------------------------ - // (a) Stage 3: null root → null (outdoor root), not a position fallback - // ------------------------------------------------------------------ - - [Fact] - public void ComputeVisibilityFromRoot_NullRoot_ReturnsNull_WhenCellExists() - { - // 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 — null root overrides - - var fromNull = cv.ComputeVisibilityFromRoot(null, pos); - - // Stage 3: null root → null (outdoor root path). - Assert.Null(fromNull); - } - - [Fact] - public void ComputeVisibilityFromRoot_NullRoot_NoCells_ReturnsNull() - { - // With no cells registered and null root: always null (outdoor root). - var cv = new CellVisibility(); - var posOutdoors = new Vector3(100, 100, 100); - - var fromNull = cv.ComputeVisibilityFromRoot(null, posOutdoors); - - Assert.Null(fromNull); - } - - [Fact] - public void ComputeVisibilityFromRoot_NullRoot_PositionOutsideAllCells_ReturnsNull() - { - // 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 fromNull = cv.ComputeVisibilityFromRoot(null, posOutside); - - Assert.Null(fromNull); - } - - // ------------------------------------------------------------------ - // (b) Supplied root is used as BFS root → CameraCell == root - // ------------------------------------------------------------------ - - [Fact] - public void ComputeVisibilityFromRoot_RegisteredRoot_CameraCellIsSuppliedRoot() - { - // Arrange: cell registered in CellVisibility. - var cv = new CellVisibility(); - var cell = MakeCell(0xA9B40103u, Vector3.Zero, new Vector3(10, 10, 10)); - cv.AddCell(cell); - - // The position can be OUTSIDE the cell — physics already determined membership - // via BSP, we just trust that answer. - var posAnywhere = new Vector3(999, 999, 999); - - // Act - var result = cv.ComputeVisibilityFromRoot(cell, posAnywhere); - - // Assert: CameraCell is the supplied root. - Assert.NotNull(result); - Assert.Same(cell, result!.CameraCell); - } - - [Fact] - public void ComputeVisibilityFromRoot_RegisteredRoot_IncludesRootInVisibleCells() - { - var cv = new CellVisibility(); - var cell = MakeCell(0xA9B40104u, Vector3.Zero, new Vector3(10, 10, 10)); - cv.AddCell(cell); - - var result = cv.ComputeVisibilityFromRoot(cell, Vector3.Zero); - - Assert.NotNull(result); - Assert.Contains(cell.CellId, result!.VisibleCellIds); - } - - [Fact] - public void ComputeVisibilityFromRoot_RegisteredRoot_LastVisibilityResultUpdated() - { - var cv = new CellVisibility(); - var cell = MakeCell(0xA9B40105u, Vector3.Zero, new Vector3(10, 10, 10)); - cv.AddCell(cell); - - var result = cv.ComputeVisibilityFromRoot(cell, Vector3.Zero); - - Assert.Same(result, cv.LastVisibilityResult); - } -}