acdream/tests/AcDream.App.Tests/Rendering/Wb/EnvCellRendererTests.cs
Erik 9559726960 fix(render): Phase A8 — pool aliasing in EnvCellRenderer (visual chaos root cause)
The post-Wave-5 indoor branch chaos (flickering, missing walls, GPU 100%,
~10 FPS) is caused by two interconnected pool-management bugs in
EnvCellRenderer that line-by-line WB comparison surfaced in 30 minutes.
Neither was found by the five post-Wave-5 speculative fixes because none
of them inspected the pool path.

Bug #1 — GetPooledList missing list.Clear():
The reuse branch returned pool lists with prior-frame data still inside.
PrepareRenderBatches' merge phase pattern `gfxDict[k] = list; list.AddRange(...)`
assumes empty lists. Without Clear(), lists grow unbounded each frame, GPU
draws cumulative instance counts, and per-instance transforms become a stew
of past + present data. Mirrors WB ObjectRenderManagerBase.cs:1221-1233.

Bug #2 — Render uses snapshot.BatchedByCell.Count instead of PostPreparePoolIndex:
The snapshot author dropped WB's PostPreparePoolIndex field calling it
"scenery-only," then "compensated" in Render by setting _poolIndex to the
cell count. The cell count has no relation to the pool — Prepare may have
used 50+ pool lists for an 18-cell scene. Render's filter-path GetPooledList
then returns lists that ARE in snapshot.BatchedByCell, corrupting the snapshot
mid-Render. Restoring PostPreparePoolIndex (WB VisibilitySnapshot.cs:31)
correctly places Render's pool cursor past the snapshot's owned region.

Bug #3 (minor) — PopulateRecursive hardcoded isSetup:false for nested parts:
Setup IDs use high-byte 0x02 (per retail). WB ObjectRenderManagerBase.cs:813
checks `(partId >> 24) == 0x02` to detect nested Setups. Our port always
passed isSetup:false, silently dropping any nested Setup (its TryGetRenderData
returns IsSetup=true, Render's `!IsSetup` guard skips the draw). Probably
rare in EnvCells but fixed for completeness.

Regression coverage:
- GetPooledList_ReusedList_IsClearedBeforeReturn — would have failed pre-fix
- GetPooledList_FreshList_IsAlwaysEmpty — sanity check
- Snapshot_PostPreparePoolIndex_IsInitSettable — compile-time guarantee
- Snapshot_PostPreparePoolIndex_DefaultsToZero — defensive default

86/86 App tests pass. Build green. The fix is the audit's primary
deliverable; the GL state probe option-1 apparatus follows in a separate
commit as defense-in-depth for any unidentified residual issue.

Full audit + WB cross-reference in
docs/research/2026-05-28-a8-env-cell-renderer-audit-findings.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-27 19:08:49 +02:00

204 lines
8.2 KiB
C#

// Tests for EnvCellRenderer (Phase A8, 2026-05-28).
// These cover the pure data-handling portions of EnvCellRenderer.
// The GL-dependent Render() and RenderModernMDIInternal() paths require a
// GL context and are visual-verified at the render frame (Task 10).
using System.Collections.Generic;
using AcDream.App.Rendering.Wb;
using Xunit;
namespace AcDream.App.Tests.Rendering.Wb;
public class EnvCellRendererTests
{
// -----------------------------------------------------------------------
// GetEnvCellGeomId — verbatim port of WB EnvCellRenderManager.cs:94-103
// -----------------------------------------------------------------------
[Fact]
public void GetEnvCellGeomId_DedupBitSet()
{
var id = EnvCellRenderer.GetEnvCellGeomId(0x42, 7, new List<ushort> { 1, 2, 3 });
// Bit 33 (0x2_0000_0000) must be set — distinguishes dedup geom from per-cell ids.
Assert.NotEqual(0UL, id & 0x2_0000_0000UL);
}
[Fact]
public void GetEnvCellGeomId_Deterministic()
{
var s = new List<ushort> { 1, 2, 3 };
var a = EnvCellRenderer.GetEnvCellGeomId(0x42, 7, s);
var b = EnvCellRenderer.GetEnvCellGeomId(0x42, 7, s);
Assert.Equal(a, b);
}
[Fact]
public void GetEnvCellGeomId_DiffersByEnvironmentId()
{
var a = EnvCellRenderer.GetEnvCellGeomId(0x42, 7, new List<ushort> { 1 });
var b = EnvCellRenderer.GetEnvCellGeomId(0x43, 7, new List<ushort> { 1 });
Assert.NotEqual(a, b);
}
[Fact]
public void GetEnvCellGeomId_DiffersByCellStructure()
{
var a = EnvCellRenderer.GetEnvCellGeomId(0x42, 7, new List<ushort> { 1 });
var b = EnvCellRenderer.GetEnvCellGeomId(0x42, 8, new List<ushort> { 1 });
Assert.NotEqual(a, b);
}
[Fact]
public void GetEnvCellGeomId_DiffersBySurfaces()
{
var a = EnvCellRenderer.GetEnvCellGeomId(0x42, 7, new List<ushort> { 1 });
var b = EnvCellRenderer.GetEnvCellGeomId(0x42, 7, new List<ushort> { 2 });
Assert.NotEqual(a, b);
}
// -----------------------------------------------------------------------
// Constructor — pure data, no GL
// -----------------------------------------------------------------------
[Fact]
public void NewRenderer_NeedsPrepareIsTrue()
{
// GL and meshManager are null — only valid for pure-data tests (no
// Initialize() is called, so no GL calls are made).
var r = new EnvCellRenderer(gl: null!, meshManager: null!, frustum: new WbFrustum());
Assert.True(r.NeedsPrepare);
}
[Fact]
public void NewRenderer_NotDisposed()
{
var r = new EnvCellRenderer(gl: null!, meshManager: null!, frustum: new WbFrustum());
Assert.False(r.IsDisposed);
}
// -----------------------------------------------------------------------
// RemoveLandblock — pure data path
// -----------------------------------------------------------------------
[Fact]
public void RemoveLandblock_NonExistent_DoesNotThrow()
{
var r = new EnvCellRenderer(gl: null!, meshManager: null!, frustum: new WbFrustum());
// Should silently no-op.
r.RemoveLandblock(0xA9B40000u);
Assert.True(r.NeedsPrepare);
}
// -----------------------------------------------------------------------
// GetEnvCellGeomId — additional edge cases
// -----------------------------------------------------------------------
[Fact]
public void GetEnvCellGeomId_EmptySurfaces_Deterministic()
{
var a = EnvCellRenderer.GetEnvCellGeomId(1, 0, new List<ushort>());
var b = EnvCellRenderer.GetEnvCellGeomId(1, 0, new List<ushort>());
Assert.Equal(a, b);
Assert.NotEqual(0UL, a & 0x2_0000_0000UL);
}
[Fact]
public void GetEnvCellGeomId_SurfaceOrderMatters()
{
var a = EnvCellRenderer.GetEnvCellGeomId(1, 1, new List<ushort> { 10, 20 });
var b = EnvCellRenderer.GetEnvCellGeomId(1, 1, new List<ushort> { 20, 10 });
// The hash is order-sensitive (matches WB's foreach loop), so
// swapped order should produce a different id.
Assert.NotEqual(a, b);
}
// (Render() requires a GL context — visual-verified in Task 10.)
// -----------------------------------------------------------------------
// Pool-aliasing regression tests (2026-05-28 audit findings).
//
// Two interconnected bugs caused the post-Wave-5 visual chaos:
// 1. GetPooledList didn't clear reused lists, causing AddRange to grow
// pool entries unbounded across frames.
// 2. Render's pool cursor reset used `BatchedByCell.Count` (cell count,
// a small int with no relation to the pool) instead of WB's
// `PostPreparePoolIndex` (the pool high-water mark after Prepare),
// pointing Render's GetPooledList back into snapshot-owned lists.
//
// These tests use reflection to verify the fixes without widening
// EnvCellRenderer's public API. If either fix regresses, the
// corresponding test fails fast.
// -----------------------------------------------------------------------
[Fact]
public void Snapshot_PostPreparePoolIndex_IsInitSettable()
{
// Compile-time guarantee: the field exists and is init-only.
// If a future refactor renames or removes it, this test won't compile.
var s = new EnvCellVisibilitySnapshot { PostPreparePoolIndex = 42 };
Assert.Equal(42, s.PostPreparePoolIndex);
}
[Fact]
public void Snapshot_PostPreparePoolIndex_DefaultsToZero()
{
var s = new EnvCellVisibilitySnapshot();
Assert.Equal(0, s.PostPreparePoolIndex);
}
[Fact]
public void GetPooledList_ReusedList_IsClearedBeforeReturn()
{
// The bug: WB's GetPooledList clears the list before returning so
// the merge phase pattern `gfxDict[k] = list; list.AddRange(...)`
// populates fresh data. The original port omitted Clear() — each
// frame's lists grew unbounded with stale data layered on top.
//
// Reflection-based test that drives the private GetPooledList +
// _poolIndex/_listPool fields. If a future refactor removes the
// Clear() call, this test fails.
var r = new EnvCellRenderer(gl: null!, meshManager: null!, frustum: new WbFrustum());
var type = typeof(EnvCellRenderer);
var getPooledListMethod = type.GetMethod("GetPooledList",
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
Assert.NotNull(getPooledListMethod);
var poolIndexField = type.GetField("_poolIndex",
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
Assert.NotNull(poolIndexField);
// First call — creates _listPool[0], _poolIndex 0 → 1.
var first = (List<InstanceData>)getPooledListMethod!.Invoke(r, null)!;
first.Add(new InstanceData());
first.Add(new InstanceData());
Assert.Equal(2, first.Count);
// Reset cursor to 0 — simulates the start of the next prepare cycle.
poolIndexField!.SetValue(r, 0);
// Second call — returns _listPool[0] (same as first). With the fix
// it should be cleared. Without the fix the list still has 2 items.
var second = (List<InstanceData>)getPooledListMethod.Invoke(r, null)!;
Assert.Same(first, second); // reuses the same instance
Assert.Empty(second); // and the data is gone
}
[Fact]
public void GetPooledList_FreshList_IsAlwaysEmpty()
{
// Sanity check for the fresh-list branch. _poolIndex past _listPool.Count
// should produce a brand-new empty list and grow the pool.
var r = new EnvCellRenderer(gl: null!, meshManager: null!, frustum: new WbFrustum());
var type = typeof(EnvCellRenderer);
var getPooledListMethod = type.GetMethod("GetPooledList",
System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
var a = (List<InstanceData>)getPooledListMethod!.Invoke(r, null)!;
var b = (List<InstanceData>)getPooledListMethod.Invoke(r, null)!;
Assert.NotSame(a, b);
Assert.Empty(a);
Assert.Empty(b);
}
}