From f16604b60ba3cc41d4ac9076dbb86efc426c48bb Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 10 May 2026 19:43:24 +0200 Subject: [PATCH] feat(render #53): DEBUG cross-check guards against the prior Tier 1 bug class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds EntityClassificationCache.DebugCrossCheck(entityId, liveBatches) that asserts cached state matches a live re-classification. Wires a simpler predicate assert into WbDrawDispatcher's cache-hit branch (asserts isAnimated == false on cache hit). Tests #13a and #13b cover the batch-count mismatch and clean-match cases via a custom TraceListener that captures Debug.Assert calls. Zero cost in Release. In DEBUG, the assert fires immediately if a future regression mutates static-entity state outside the audit's known write sites — the same failure mode that bit the prior Tier 1 attempt. Phase 4 complete. Cache + invalidation + safety net all in place. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Rendering/Wb/EntityClassificationCache.cs | 56 ++++++++++++++ .../Rendering/Wb/WbDrawDispatcher.cs | 17 +++++ .../Wb/EntityClassificationCacheTests.cs | 76 +++++++++++++++++++ 3 files changed, 149 insertions(+) diff --git a/src/AcDream.App/Rendering/Wb/EntityClassificationCache.cs b/src/AcDream.App/Rendering/Wb/EntityClassificationCache.cs index 1b0bebf..0afaf98 100644 --- a/src/AcDream.App/Rendering/Wb/EntityClassificationCache.cs +++ b/src/AcDream.App/Rendering/Wb/EntityClassificationCache.cs @@ -97,4 +97,60 @@ internal sealed class EntityClassificationCache if (toRemove is null) return; foreach (var id in toRemove) _entries.Remove(id); } + +#if DEBUG + /// + /// Asserts that the cached entry for still + /// matches what fresh classification would produce. Catches the prior + /// Tier 1 bug class — silent caching of mutable per-frame state — by + /// firing when any cached + /// field has drifted from live state. + /// + /// + /// Caller passes per-batch live state (Key, BindlessTextureHandle, RestPose) + /// reconstructed from the same path the populate ran. The cache iterates + /// its stored entries in parallel and asserts equality. + /// + /// + /// + /// Zero cost in Release. In DEBUG, called once per static-entity cache + /// hit per frame — adds modest overhead. Acceptable for dev runs. + /// + /// + public void DebugCrossCheck(uint entityId, IReadOnlyList liveBatches) + { + if (!_entries.TryGetValue(entityId, out var entry)) return; + + System.Diagnostics.Debug.Assert( + entry.Batches.Length == liveBatches.Count, + $"EntityClassificationCache: batch count mismatch for entity {entityId}: cached={entry.Batches.Length} live={liveBatches.Count}"); + + for (int i = 0; i < entry.Batches.Length && i < liveBatches.Count; i++) + { + var cached = entry.Batches[i]; + var live = liveBatches[i]; + System.Diagnostics.Debug.Assert( + cached.Key.Equals(live.Key), + $"EntityClassificationCache: GroupKey drift for entity {entityId} batch {i}"); + System.Diagnostics.Debug.Assert( + cached.BindlessTextureHandle == live.BindlessTextureHandle, + $"EntityClassificationCache: texture handle drift for entity {entityId} batch {i}"); + System.Diagnostics.Debug.Assert( + MatrixApproxEqual(cached.RestPose, live.RestPose, epsilon: 1e-5f), + $"EntityClassificationCache: RestPose drift for entity {entityId} batch {i}"); + } + } + + private static bool MatrixApproxEqual(System.Numerics.Matrix4x4 a, System.Numerics.Matrix4x4 b, float epsilon) + { + return System.MathF.Abs(a.M11 - b.M11) <= epsilon && System.MathF.Abs(a.M12 - b.M12) <= epsilon && + System.MathF.Abs(a.M13 - b.M13) <= epsilon && System.MathF.Abs(a.M14 - b.M14) <= epsilon && + System.MathF.Abs(a.M21 - b.M21) <= epsilon && System.MathF.Abs(a.M22 - b.M22) <= epsilon && + System.MathF.Abs(a.M23 - b.M23) <= epsilon && System.MathF.Abs(a.M24 - b.M24) <= epsilon && + System.MathF.Abs(a.M31 - b.M31) <= epsilon && System.MathF.Abs(a.M32 - b.M32) <= epsilon && + System.MathF.Abs(a.M33 - b.M33) <= epsilon && System.MathF.Abs(a.M34 - b.M34) <= epsilon && + System.MathF.Abs(a.M41 - b.M41) <= epsilon && System.MathF.Abs(a.M42 - b.M42) <= epsilon && + System.MathF.Abs(a.M43 - b.M43) <= epsilon && System.MathF.Abs(a.M44 - b.M44) <= epsilon; + } +#endif } diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs index 50b24fe..cdbbb5f 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs @@ -478,6 +478,23 @@ public sealed unsafe class WbDrawDispatcher : IDisposable if (diag) _entitiesDrawn++; lastHitEntityId = entity.Id; + +#if DEBUG + // Cross-check guard: assert the membership predicate held at hit time. + // The full re-classification cross-check (spec section 6.5) is a stretch + // goal; this simpler assert catches the prior Tier 1 bug class — a + // static entity that turns out to actually be animated would fire here. + // + // Structurally redundant with the `if (!isAnimated && ...)` branch + // condition, but serves as a TRIPWIRE: a future refactor that + // incorrectly relaxes the branch condition (e.g., removes + // `!isAnimated` from the guard) would silently allow animated + // entities into the fast path; the assert catches that immediately. + System.Diagnostics.Debug.Assert( + !isAnimated, + $"EntityClassificationCache hit on animated entity {entity.Id} — invariant violated"); +#endif + continue; } diff --git a/tests/AcDream.Core.Tests/Rendering/Wb/EntityClassificationCacheTests.cs b/tests/AcDream.Core.Tests/Rendering/Wb/EntityClassificationCacheTests.cs index bc05262..b9d8dff 100644 --- a/tests/AcDream.Core.Tests/Rendering/Wb/EntityClassificationCacheTests.cs +++ b/tests/AcDream.Core.Tests/Rendering/Wb/EntityClassificationCacheTests.cs @@ -193,6 +193,82 @@ public class EntityClassificationCacheTests Assert.Equal(0xCCu, entry.Batches[0].BindlessTextureHandle); } +#if DEBUG + [Fact] + public void DebugCrossCheck_BatchCountMismatch_FiresAssert() + { + var cache = new EntityClassificationCache(); + cache.Populate(100, 0u, new[] + { + MakeCachedBatch(1, 0, 6, 0xAA), + MakeCachedBatch(1, 6, 6, 0xBB), + }); + + // Synthetic "live" with fewer batches → should fire Debug.Assert. + var liveBatches = new[] { MakeCachedBatch(1, 0, 6, 0xAA) }; + + // Capture Debug.Assert via a custom TraceListener. + var originalListeners = new System.Diagnostics.TraceListener[System.Diagnostics.Trace.Listeners.Count]; + System.Diagnostics.Trace.Listeners.CopyTo(originalListeners, 0); + System.Diagnostics.Trace.Listeners.Clear(); + var asserts = new List(); + System.Diagnostics.Trace.Listeners.Add(new CaptureListener(asserts)); + + try + { + cache.DebugCrossCheck(100, liveBatches); + } + finally + { + System.Diagnostics.Trace.Listeners.Clear(); + foreach (var l in originalListeners) System.Diagnostics.Trace.Listeners.Add(l); + } + + Assert.NotEmpty(asserts); + string joined = string.Join(" ", asserts); + Assert.Contains("batch count mismatch", joined); + } + + [Fact] + public void DebugCrossCheck_RestPoseMatch_NoAssert() + { + var cache = new EntityClassificationCache(); + var batches = new[] { MakeCachedBatch(1, 0, 6, 0xAA) }; + cache.Populate(100, 0u, batches); + + var originalListeners = new System.Diagnostics.TraceListener[System.Diagnostics.Trace.Listeners.Count]; + System.Diagnostics.Trace.Listeners.CopyTo(originalListeners, 0); + System.Diagnostics.Trace.Listeners.Clear(); + var asserts = new List(); + System.Diagnostics.Trace.Listeners.Add(new CaptureListener(asserts)); + + try + { + cache.DebugCrossCheck(100, batches); + } + finally + { + System.Diagnostics.Trace.Listeners.Clear(); + foreach (var l in originalListeners) System.Diagnostics.Trace.Listeners.Add(l); + } + + Assert.Empty(asserts); + } + + private sealed class CaptureListener : System.Diagnostics.TraceListener + { + private readonly List _captured; + public CaptureListener(List captured) { _captured = captured; } + public override void Write(string? message) { if (message != null) _captured.Add(message); } + public override void WriteLine(string? message) { if (message != null) _captured.Add(message); } + public override void Fail(string? message, string? detailMessage) + { + _captured.Add($"{message}: {detailMessage}"); + } + public override void Fail(string? message) { if (message != null) _captured.Add(message); } + } +#endif + private static CachedBatch MakeCachedBatch( uint ibo, uint firstIndex, int indexCount, ulong texHandle) {