acdream/src/AcDream.App/Rendering/Wb/EnvCellVisibilitySnapshot.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

48 lines
2.4 KiB
C#

using System.Collections.Generic;
namespace AcDream.App.Rendering.Wb;
/// <summary>
/// Phase A8 (2026-05-28): EnvCell-scoped visibility snapshot. Direct port of
/// WB's <c>VisibilitySnapshot</c> at
/// <c>references/WorldBuilder/Chorizite.OpenGLSDLBackend/Lib/VisibilitySnapshot.cs:1-36</c>,
/// narrowed to the fields <see cref="EnvCellRenderer"/> actually consumes
/// (<c>BatchedByCell</c> + <c>VisibleLandblocks</c> + <c>PostPreparePoolIndex</c>).
/// The scenery-side <c>VisibleGroups</c> / <c>VisibleGfxObjIds</c> /
/// <c>IntersectingLandblocks</c> are dropped — we render scenery through
/// <see cref="WbDrawDispatcher"/>, not through this snapshot.
///
/// <para>Used as an immutable snapshot atomically swapped under the
/// renderer's render lock so PrepareRenderBatches (worker-driven) and
/// Render (render-thread-driven) can't race on a half-populated dict.</para>
/// </summary>
public sealed class EnvCellVisibilitySnapshot
{
/// <summary>Landblocks fully or partially inside the frustum at prepare time.</summary>
public List<EnvCellLandblock> VisibleLandblocks { get; init; } = new();
/// <summary>
/// Grouped instance data by full 32-bit cell id.
/// Outer key: <c>CellId</c>. Inner key: <c>GfxObjId</c> (ulong; bit 33 set for
/// deduplicated cell geometry per <see cref="EnvCellRenderer.GetEnvCellGeomId"/>).
/// Value: list of per-instance transforms (one per cell or per static object
/// inside that cell).
/// </summary>
public Dictionary<uint, Dictionary<ulong, List<InstanceData>>> BatchedByCell { get; init; } = new();
/// <summary>
/// Pool-index high-water mark after PrepareRenderBatches' merge phase.
/// Mirrors WB <c>VisibilitySnapshot.PostPreparePoolIndex</c> at
/// <c>references/WorldBuilder/.../VisibilitySnapshot.cs:31</c>.
/// <para>Read by <see cref="EnvCellRenderer.Render"/> to set the pool
/// cursor to a safe region past the snapshot's owned lists, so any
/// <c>GetPooledList</c> calls inside Render don't trample data the
/// snapshot still references. Dropping this field caused the post-Wave-5
/// visual chaos — see
/// <c>docs/research/2026-05-28-a8-env-cell-renderer-audit-findings.md</c>.</para>
/// </summary>
public int PostPreparePoolIndex { get; init; }
/// <summary>True when no visible cells were produced this prepare cycle.</summary>
public bool IsEmpty => VisibleLandblocks.Count == 0 && BatchedByCell.Count == 0;
}