From 354ca746ad64e17a62d0d0a8bd4c440d9ea66ad3 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 30 May 2026 18:16:21 +0200 Subject: [PATCH] =?UTF-8?q?test(render):=20Phase=20U.4=20=E2=80=94=20cover?= =?UTF-8?q?=20ResolveEntitySlot=20clip-slot=20resolution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review flagged the gate-critical per-instance slot resolution as untested. Add RED→GREEN cases (live=unclipped slot 0, cell-static→cell slot, non-visible→cull, outdoor-stab→OutsideView/cull, routing-inactive→all slot 0). Note the full-cell-id-space invariant at ResolveEntitySlot; fix a stale RenderInsideOut comment in EnvCellRenderer. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Rendering/Wb/EnvCellRenderer.cs | 7 +- .../Rendering/Wb/WbDrawDispatcher.cs | 83 ++++++-- .../Wb/WbDrawDispatcherClipSlotTests.cs | 191 ++++++++++++++++++ 3 files changed, 259 insertions(+), 22 deletions(-) create mode 100644 tests/AcDream.App.Tests/Rendering/Wb/WbDrawDispatcherClipSlotTests.cs diff --git a/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs b/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs index 136ffcd..68ba289 100644 --- a/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs +++ b/src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs @@ -764,8 +764,11 @@ public sealed unsafe class EnvCellRenderer : IDisposable /// /// Draws all visible EnvCells (and their static objects) for the given pass. /// When is non-null, only cells whose CellId is in - /// the set are drawn — used for indoor RenderInsideOut to restrict to camera- - /// building cells. + /// the set are drawn. As of Phase U.4 this is the portal-visibility SHELL + /// filter (the drawable visible cells from the PView traversal; each cell's + /// shell instances are clip-gated to its CellClip slot by the caller's + /// binding=3 map). NOTE: this is NOT the old two-pipe RenderInsideOut approach + /// — that flat camera-inside-building stencil pass was deleted in Phase U.1. /// Source: WB EnvCellRenderManager.cs:399-511 (verbatim minus selection highlights). /// public void Render(WbRenderPass renderPass, HashSet? filter) diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs index 3159025..d2cbd9c 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs @@ -346,7 +346,9 @@ public sealed unsafe class WbDrawDispatcher : IDisposable // Phase U.4 CULL sentinel returned by ResolveEntitySlot: the entity's instances // are dropped entirely (not emitted into the binding=0 instance buffer NOR the // binding=3 slot buffer), matching the existing frustum / visible-cell cull. - private const int ClipSlotCull = -1; + // Internal (not private) so the clip-slot unit tests can assert against it + // directly — see WbDrawDispatcherClipSlotTests. + internal const int ClipSlotCull = -1; /// /// Phase U.4: resolve the clip slot for one entity per the slot/gate policy. @@ -355,27 +357,74 @@ public sealed unsafe class WbDrawDispatcher : IDisposable /// ServerGuid != 0 (live dynamic: player / NPC / items / doors) ⇒ slot 0 /// (UNCLIPPED — retail draws live-dynamic unclipped; depth only). /// ParentCellId != null (cell static) ⇒ the cell's slot, or CULL when the - /// cell isn't in cellIdToSlot (not visible / nothing-visible). + /// cell isn't in (not visible / nothing-visible). /// ParentCellId == null (outdoor scenery / building shell) ⇒ the OutsideView - /// slot when outdoorVisible, else CULL. + /// slot when , else CULL. /// /// Only called when _clipRoutingActive (indoor root). On the U.3 / outdoor - /// path every instance is slot 0 and nothing is culled. + /// path every instance is slot 0 and nothing is culled — see + /// , which gates on that flag. + /// + /// INVARIANT: and the keys of + /// MUST live in the same FULL cell-id space + /// (lbMask | OtherCellId, e.g. 0xA9B40164). A bare-low-byte + /// ParentCellId (e.g. 0x64) would never match a full-id key and would + /// silently CULL every indoor stab — cf. the L.2e bare-low-byte finding in + /// CLAUDE.md where player CellId was tracked without its landblock prefix. + /// + /// + /// internal static + pure (reads no instance state) so the clip-slot + /// unit tests exercise every branch without a GL context. The caller hands in + /// the routing fields it would otherwise read from _cellIdToSlot etc. + /// /// - private int ResolveEntitySlot(WorldEntity entity) + internal static int ResolveEntitySlot( + uint serverGuid, + uint? parentCellId, + IReadOnlyDictionary cellIdToSlot, + int outdoorSlot, + bool outdoorVisible) { // Live-dynamic entities render unclipped regardless of cell — retail draws // the player / NPCs / dropped items through the depth buffer without portal // clipping. ServerGuid is the live-dynamic marker (0 for dat-hydrated). - if (entity.ServerGuid != 0) + if (serverGuid != 0) return 0; - if (entity.ParentCellId is uint parentCell) - return _cellIdToSlot!.TryGetValue(parentCell, out int slot) ? slot : ClipSlotCull; + if (parentCellId is uint parentCell) + return cellIdToSlot.TryGetValue(parentCell, out int slot) ? slot : ClipSlotCull; // Outdoor scenery / building shell (no ParentCellId). Indoor root: gate to // the OutsideView slot, or cull when nothing outdoors is visible. - return _outdoorVisible ? _outdoorSlot : ClipSlotCull; + return outdoorVisible ? outdoorSlot : ClipSlotCull; + } + + /// + /// Phase U.4: the call-site clip-slot decision for one entity, returning the + /// (Slot, Culled) pair the per-entity loop body consumes. Wraps + /// with the + /// gate: when routing is INACTIVE (outdoor root / no portal frame), every entity + /// is slot 0 and nothing is clip-culled — the bit-identical-to-U.3 property, so + /// the resolver (and ) is bypassed entirely. + /// When active, a CULL sentinel maps to (0, culled=true) — the slot value + /// is never emitted for a culled entity. + /// internal static + pure so the whole policy (including the routing- + /// inactive branch) is unit-testable — see WbDrawDispatcherClipSlotTests. + /// + internal static (uint Slot, bool Culled) ResolveSlotForFrame( + bool clipRoutingActive, + uint serverGuid, + uint? parentCellId, + IReadOnlyDictionary? cellIdToSlot, + int outdoorSlot, + bool outdoorVisible) + { + if (!clipRoutingActive) + return (0u, false); + + int resolved = ResolveEntitySlot(serverGuid, parentCellId, cellIdToSlot!, outdoorSlot, outdoorVisible); + bool culled = resolved == ClipSlotCull; + return (culled ? 0u : (uint)resolved, culled); } public static Matrix4x4 ComposePartWorldMatrix( @@ -775,17 +824,11 @@ public sealed unsafe class WbDrawDispatcher : IDisposable // Phase U.4: resolve this entity's clip slot ONCE per entity // (constant across its tuples). On the U.3 / outdoor path // (_clipRoutingActive false) every entity is slot 0, never culled. - if (_clipRoutingActive) - { - int resolved = ResolveEntitySlot(entity); - _currentEntityCulled = resolved == ClipSlotCull; - _currentEntitySlot = _currentEntityCulled ? 0u : (uint)resolved; - } - else - { - _currentEntityCulled = false; - _currentEntitySlot = 0u; - } + // The whole decision (including the routing-active gate) lives in + // the pure ResolveSlotForFrame helper so it's unit-testable. + (_currentEntitySlot, _currentEntityCulled) = ResolveSlotForFrame( + _clipRoutingActive, entity.ServerGuid, entity.ParentCellId, + _cellIdToSlot, _outdoorSlot, _outdoorVisible); } prevTupleEntityId = entity.Id; diff --git a/tests/AcDream.App.Tests/Rendering/Wb/WbDrawDispatcherClipSlotTests.cs b/tests/AcDream.App.Tests/Rendering/Wb/WbDrawDispatcherClipSlotTests.cs new file mode 100644 index 0000000..9a25811 --- /dev/null +++ b/tests/AcDream.App.Tests/Rendering/Wb/WbDrawDispatcherClipSlotTests.cs @@ -0,0 +1,191 @@ +// Tests for WbDrawDispatcher's Phase U.4 per-instance clip-slot resolution +// (ResolveEntitySlot / ResolveSlotForFrame). Code review of the U.4 commit +// (7993e06) flagged this gate-critical routing as untested: if it breaks, +// every indoor instance is sent to the wrong clip slot (or wrongly culled), +// producing total visual garbage at the portal-visibility gate. The logic is +// a pure function of (ServerGuid, ParentCellId, the clip-routing state), so we +// extract it to internal static helpers and test the branches directly — no GL +// context required. +// +// Branch map (ResolveSlotForFrame, the call-site policy): +// routing inactive (outdoor root) → slot 0, NOT culled (≡ U.3) +// ServerGuid != 0 (live dynamic) → slot 0, NOT culled (unclipped) +// ParentCellId in cellIdToSlot → that cell's slot +// ParentCellId NOT in cellIdToSlot → CULL +// ParentCellId == null, outdoorVisible → outdoorSlot +// ParentCellId == null, !outdoorVisible → CULL + +using System.Collections.Generic; +using AcDream.App.Rendering.Wb; +using Xunit; + +namespace AcDream.App.Tests.Rendering.Wb; + +public sealed class WbDrawDispatcherClipSlotTests +{ + // Full cell-id space keys (lbMask | OtherCellId). 0xA9B4 is the Holtburg + // landblock prefix used throughout the indoor-walking work; the low word is + // the EnvCell index. ParentCellId on a cell static is the SAME full id — see + // the L.2e bare-low-byte finding (a 0x29 low-byte key would cull everything). + private const uint VisibleCellA = 0xA9B4_0164u; + private const uint VisibleCellB = 0xA9B4_0165u; + private const uint NotVisibleCell = 0xA9B4_0999u; + + private const int SlotA = 3; + private const int SlotB = 7; + private const int OutsideViewSlot = 11; + + private static IReadOnlyDictionary Routing() => new Dictionary + { + [VisibleCellA] = SlotA, + [VisibleCellB] = SlotB, + }; + + // ── Raw resolver (ResolveEntitySlot): only reached when routing is active ── + + [Fact] + public void RawResolve_LiveEntity_IsUnclippedSlot0_WhenParentCellNull() + { + // ServerGuid != 0 ⇒ unclipped (slot 0) regardless of cell state. + int slot = WbDrawDispatcher.ResolveEntitySlot( + serverGuid: 0x5000_000Au, parentCellId: null, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.Equal(0, slot); + } + + [Fact] + public void RawResolve_LiveEntity_IsUnclippedSlot0_EvenWhenParentCellVisible() + { + // A live entity whose ParentCellId IS a visible cell still goes to slot 0, + // NOT SlotA — the live-dynamic check must precede the cell lookup. + int slot = WbDrawDispatcher.ResolveEntitySlot( + serverGuid: 0x5000_000Au, parentCellId: VisibleCellA, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.Equal(0, slot); + Assert.NotEqual(SlotA, slot); // guards against ordering regression + } + + [Fact] + public void RawResolve_CellStatic_InVisibleSet_GetsThatCellSlot() + { + int slot = WbDrawDispatcher.ResolveEntitySlot( + serverGuid: 0u, parentCellId: VisibleCellB, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.Equal(SlotB, slot); + } + + [Fact] + public void RawResolve_CellStatic_NotInVisibleSet_IsCulled() + { + int slot = WbDrawDispatcher.ResolveEntitySlot( + serverGuid: 0u, parentCellId: NotVisibleCell, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.Equal(WbDrawDispatcher.ClipSlotCull, slot); + } + + [Fact] + public void RawResolve_OutdoorStab_OutdoorsVisible_GetsOutsideViewSlot() + { + int slot = WbDrawDispatcher.ResolveEntitySlot( + serverGuid: 0u, parentCellId: null, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.Equal(OutsideViewSlot, slot); + } + + [Fact] + public void RawResolve_OutdoorStab_OutdoorsNotVisible_IsCulled() + { + int slot = WbDrawDispatcher.ResolveEntitySlot( + serverGuid: 0u, parentCellId: null, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: false); + + Assert.Equal(WbDrawDispatcher.ClipSlotCull, slot); + } + + // ── Call-site policy (ResolveSlotForFrame): adds the clipRoutingActive gate ── + // Cases mirror the raw resolver but return the (slot, culled) pair the loop + // body consumes, and add the routing-inactive (outdoor-root) branch. + + [Fact] + public void ForFrame_RoutingInactive_EveryEntityIsSlot0AndNotCulled() + { + // The bit-identical-to-U.3 property: when the camera is at an outdoor root + // (ClearClipRouting), ResolveEntitySlot is never consulted — every entity + // maps to slot 0 and nothing is clip-culled. Exercised here for BOTH a + // live entity and a cell static that would otherwise cull, with a null + // routing map to prove the resolver is bypassed entirely. + + var live = WbDrawDispatcher.ResolveSlotForFrame( + clipRoutingActive: false, serverGuid: 0x5000_000Au, parentCellId: null, + cellIdToSlot: null, outdoorSlot: OutsideViewSlot, outdoorVisible: true); + Assert.Equal(0u, live.Slot); + Assert.False(live.Culled); + + var wouldCull = WbDrawDispatcher.ResolveSlotForFrame( + clipRoutingActive: false, serverGuid: 0u, parentCellId: NotVisibleCell, + cellIdToSlot: null, outdoorSlot: OutsideViewSlot, outdoorVisible: false); + Assert.Equal(0u, wouldCull.Slot); + Assert.False(wouldCull.Culled); + } + + [Fact] + public void ForFrame_RoutingActive_LiveEntity_Slot0NotCulled() + { + var r = WbDrawDispatcher.ResolveSlotForFrame( + clipRoutingActive: true, serverGuid: 0x5000_000Au, parentCellId: VisibleCellA, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.Equal(0u, r.Slot); + Assert.False(r.Culled); + } + + [Fact] + public void ForFrame_RoutingActive_CellStaticVisible_GetsCellSlotNotCulled() + { + var r = WbDrawDispatcher.ResolveSlotForFrame( + clipRoutingActive: true, serverGuid: 0u, parentCellId: VisibleCellA, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.Equal((uint)SlotA, r.Slot); + Assert.False(r.Culled); + } + + [Fact] + public void ForFrame_RoutingActive_CellStaticNotVisible_Culled() + { + var r = WbDrawDispatcher.ResolveSlotForFrame( + clipRoutingActive: true, serverGuid: 0u, parentCellId: NotVisibleCell, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.True(r.Culled); + // When culled the loop body forces slot 0 (the value is never emitted). + Assert.Equal(0u, r.Slot); + } + + [Fact] + public void ForFrame_RoutingActive_OutdoorStabVisible_GetsOutsideViewSlot() + { + var r = WbDrawDispatcher.ResolveSlotForFrame( + clipRoutingActive: true, serverGuid: 0u, parentCellId: null, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: true); + + Assert.Equal((uint)OutsideViewSlot, r.Slot); + Assert.False(r.Culled); + } + + [Fact] + public void ForFrame_RoutingActive_OutdoorStabNotVisible_Culled() + { + var r = WbDrawDispatcher.ResolveSlotForFrame( + clipRoutingActive: true, serverGuid: 0u, parentCellId: null, + cellIdToSlot: Routing(), outdoorSlot: OutsideViewSlot, outdoorVisible: false); + + Assert.True(r.Culled); + Assert.Equal(0u, r.Slot); + } +}