test(render): Phase U.4 — cover ResolveEntitySlot clip-slot resolution

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) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-30 18:16:21 +02:00
parent 7993e064a0
commit 354ca746ad
3 changed files with 259 additions and 22 deletions

View file

@ -764,8 +764,11 @@ public sealed unsafe class EnvCellRenderer : IDisposable
/// <summary>
/// Draws all visible EnvCells (and their static objects) for the given pass.
/// When <paramref name="filter"/> 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).
/// </summary>
public void Render(WbRenderPass renderPass, HashSet<uint>? filter)

View file

@ -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;
/// <summary>
/// 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
/// <item>ServerGuid != 0 (live dynamic: player / NPC / items / doors) ⇒ slot 0
/// (UNCLIPPED — retail draws live-dynamic unclipped; depth only).</item>
/// <item>ParentCellId != null (cell static) ⇒ the cell's slot, or CULL when the
/// cell isn't in <c>cellIdToSlot</c> (not visible / nothing-visible).</item>
/// cell isn't in <paramref name="cellIdToSlot"/> (not visible / nothing-visible).</item>
/// <item>ParentCellId == null (outdoor scenery / building shell) ⇒ the OutsideView
/// slot when <c>outdoorVisible</c>, else CULL.</item>
/// slot when <paramref name="outdoorVisible"/>, else CULL.</item>
/// </list>
/// Only called when <c>_clipRoutingActive</c> (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
/// <see cref="ResolveSlotForFrame"/>, which gates on that flag.
/// <para>
/// INVARIANT: <paramref name="parentCellId"/> and the keys of
/// <paramref name="cellIdToSlot"/> MUST live in the same FULL cell-id space
/// (<c>lbMask | OtherCellId</c>, e.g. <c>0xA9B40164</c>). A bare-low-byte
/// ParentCellId (e.g. <c>0x64</c>) 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.
/// </para>
/// <para>
/// <c>internal static</c> + 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 <c>_cellIdToSlot</c> etc.
/// </para>
/// </summary>
private int ResolveEntitySlot(WorldEntity entity)
internal static int ResolveEntitySlot(
uint serverGuid,
uint? parentCellId,
IReadOnlyDictionary<uint, int> 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;
}
/// <summary>
/// Phase U.4: the call-site clip-slot decision for one entity, returning the
/// <c>(Slot, Culled)</c> pair the per-entity loop body consumes. Wraps
/// <see cref="ResolveEntitySlot"/> with the <paramref name="clipRoutingActive"/>
/// 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 <paramref name="cellIdToSlot"/>) is bypassed entirely.
/// When active, a CULL sentinel maps to <c>(0, culled=true)</c> — the slot value
/// is never emitted for a culled entity.
/// <c>internal static</c> + pure so the whole policy (including the routing-
/// inactive branch) is unit-testable — see WbDrawDispatcherClipSlotTests.
/// </summary>
internal static (uint Slot, bool Culled) ResolveSlotForFrame(
bool clipRoutingActive,
uint serverGuid,
uint? parentCellId,
IReadOnlyDictionary<uint, int>? 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;

View file

@ -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<uint, int> Routing() => new Dictionary<uint, int>
{
[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);
}
}