diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs index 3b1654b..e1d4cbd 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs @@ -404,15 +404,8 @@ public sealed unsafe class WbDrawDispatcher : IDisposable // Flush-on-entity-change: if the previous entity accumulated any // batches AND this iteration is for a different entity, populate // its cache entry now and reset the scratch buffer. - if (populateEntityId.HasValue && populateEntityId.Value != entity.Id) - { - if (_populateScratch.Count > 0) - { - _cache.Populate(populateEntityId.Value, populateLandblockId, _populateScratch.ToArray()); - } - _populateScratch.Clear(); - populateEntityId = null; - } + (populateEntityId, populateLandblockId) = MaybeFlushOnEntityChange( + populateEntityId, populateLandblockId, entity.Id, _cache, _populateScratch); var entityWorld = Matrix4x4.CreateFromQuaternion(entity.Rotation) * @@ -420,6 +413,36 @@ public sealed unsafe class WbDrawDispatcher : IDisposable bool isAnimated = animatedEntityIds?.Contains(entity.Id) == true; + // Cache-hit fast path (Task 10): static entity with a populated + // cache entry skips classification entirely. Walk the cached + // (GroupKey, RestPose) flat list and append cached.RestPose * + // entityWorld to each matching group's matrices. Animated entities + // bypass the cache (collector is set null below; their entries are + // never populated in the first place). + // + // Placed AFTER the entity-change flush above so that, on a + // hit, this iteration also finishes flushing any pending + // populate state from a previous entity. Animated entities never + // enter this branch — the !isAnimated guard makes that explicit. + if (!isAnimated && _cache.TryGet(entity.Id, out var cachedEntry)) + { + ApplyCacheHit(cachedEntry!, entityWorld, AppendInstanceToGroup); + + // anyVao recovery: when the first visible entity in the frame + // takes the fast path, no slow-path lookup has populated + // anyVao yet. Look up THIS entity's first MeshRef once via + // the mesh adapter — cheap dict lookup, not a re-classify. + if (anyVao == 0) + { + var firstMeshRef = entity.MeshRefs[partIdx]; + var firstRenderData = _meshAdapter.TryGetRenderData(firstMeshRef.GfxObjId); + if (firstRenderData is not null) anyVao = firstRenderData.VAO; + } + + if (diag) _entitiesDrawn++; + continue; + } + // Compute palette-override hash ONCE per entity (perf #4). // Reused across every (part, batch) lookup so the FNV-1a fold // over SubPalettes runs once instead of N times. Zero when the @@ -449,8 +472,6 @@ public sealed unsafe class WbDrawDispatcher : IDisposable // Static entities accumulate into _populateScratch across ALL // their MeshRefs; the flush at next-entity-boundary (or // end-of-loop) commits them as a single Populate call. - // Task 10 will add the cache-hit fast path that skips slow - // classification when an entry already exists. var collector = isAnimated ? null : _populateScratch; bool drewAny = false; @@ -492,12 +513,7 @@ public sealed unsafe class WbDrawDispatcher : IDisposable // to trigger the entity-change flush, so commit its accumulated batches // here. No-op when the last entity was animated (populateEntityId stays // null) or when no entities walked at all. - if (populateEntityId.HasValue && _populateScratch.Count > 0) - { - _cache.Populate(populateEntityId.Value, populateLandblockId, _populateScratch.ToArray()); - _populateScratch.Clear(); - populateEntityId = null; - } + FinalFlushPopulate(populateEntityId, populateLandblockId, _cache, _populateScratch); // Nothing visible — skip the GL pass entirely. if (anyVao == 0) @@ -781,6 +797,116 @@ public sealed unsafe class WbDrawDispatcher : IDisposable return copy[idx]; } + // ── Tier 1 cache (#53) helpers extracted for testability ───────────────── + // + // Three pure-CPU static helpers carved out of Draw's per-entity loop so + // unit tests can exercise the populate/flush algorithm + cache-hit fast + // path without needing a real GL context. Production code (Draw) calls + // these helpers; the dispatcher integration tests in + // WbDrawDispatcherBucketingTests use them to drive the same algorithm + // through deterministic inputs. + + /// + /// Apply a cache hit's batches into the per-frame group dictionary by + /// composing cached.RestPose * entityWorld per batch and routing + /// the result through . The delegate + /// abstracts over so this helper stays + /// GL-free and unit-testable. + /// + /// + /// Matrix multiplication is non-commutative: it MUST be + /// RestPose * entityWorld, not the reverse. See + /// for the full part-world product. + /// + internal static void ApplyCacheHit( + EntityCacheEntry entry, + Matrix4x4 entityWorld, + Action appendInstance) + { + foreach (var cached in entry.Batches) + { + appendInstance(cached.Key, cached.RestPose * entityWorld); + } + } + + /// + /// Per-tuple flush check. If is set + /// AND differs from , the previous + /// entity's accumulated batches are committed to + /// and is cleared. Returns the + /// updated tracker tuple — pass these back into the field locals in the + /// caller's loop. + /// + /// + /// This is the bug-fix structure from commit 00fa8ae (per-MeshRef + /// Populate would overwrite earlier MeshRefs because the cache is + /// keyed by entity.Id; flushing only on entity boundary preserves all + /// MeshRefs' batches). _walkScratch is in entity-order so all MeshRefs + /// of one entity arrive contiguously. + /// + internal static (uint? PopulateEntityId, uint PopulateLandblockId) + MaybeFlushOnEntityChange( + uint? populateEntityId, + uint populateLandblockId, + uint currentEntityId, + EntityClassificationCache cache, + List populateScratch) + { + if (populateEntityId.HasValue && populateEntityId.Value != currentEntityId) + { + if (populateScratch.Count > 0) + { + cache.Populate(populateEntityId.Value, populateLandblockId, populateScratch.ToArray()); + } + populateScratch.Clear(); + return (null, 0u); + } + return (populateEntityId, populateLandblockId); + } + + /// + /// End-of-loop final flush. The last entity in _walkScratch has + /// no next-iteration to trigger , + /// so commit its accumulated batches here. No-op when no populate is + /// pending (the last entity was animated, or the scratch is empty). + /// + internal static void FinalFlushPopulate( + uint? populateEntityId, + uint populateLandblockId, + EntityClassificationCache cache, + List populateScratch) + { + if (populateEntityId.HasValue && populateScratch.Count > 0) + { + cache.Populate(populateEntityId.Value, populateLandblockId, populateScratch.ToArray()); + populateScratch.Clear(); + } + } + + /// + /// Instance-side helper used by . Looks up or + /// creates an for the given key in + /// _groups and appends the per-instance world matrix. + /// + private void AppendInstanceToGroup(GroupKey key, Matrix4x4 model) + { + if (!_groups.TryGetValue(key, out var grp)) + { + grp = new InstanceGroup + { + Ibo = key.Ibo, + FirstIndex = key.FirstIndex, + BaseVertex = key.BaseVertex, + IndexCount = key.IndexCount, + BindlessTextureHandle = key.BindlessTextureHandle, + TextureLayer = key.TextureLayer, + Translucency = key.Translucency, + }; + _groups[key] = grp; + } + grp.Matrices.Add(model); + } + private void ClassifyBatches( ObjectRenderData renderData, ulong gfxObjId, diff --git a/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherBucketingTests.cs b/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherBucketingTests.cs index 051dcf2..28c6ff8 100644 --- a/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherBucketingTests.cs +++ b/tests/AcDream.Core.Tests/Rendering/Wb/WbDrawDispatcherBucketingTests.cs @@ -351,4 +351,259 @@ public sealed class WbDrawDispatcherBucketingTests // AabbDirty should have been cleared by the lazy refresh. Assert.False(entity.AabbDirty); } + + // ── Tier 1 cache (#53) dispatcher integration tests ────────────────────── + // + // Tasks 9 & 10 wire the EntityClassificationCache into Draw's per-entity + // loop. These tests exercise the populate + cache-hit fast-path algorithm + // through the static helpers Draw uses (MaybeFlushOnEntityChange, + // FinalFlushPopulate, ApplyCacheHit). The helpers were extracted from + // Draw's foreach for testability — Draw calls them; tests drive them + // directly with deterministic synthesized inputs. This is the same + // pattern WalkEntities follows (extracted from Draw, tested in isolation). + // + // The tests cover spec §7.2 #11 (static populate + reuse) and #12 + // (animated bypass), plus a multi-MeshRef regression test that would + // have caught the bug fixed in commit 00fa8ae (per-MeshRef Populate + // overwrites earlier batches because the cache is keyed by entity.Id). + + /// + /// Helper: constructs a CachedBatch with stable group-key inputs so the + /// hit-path test can verify membership. Mirrors the shape ClassifyBatches + /// produces under the collector pattern. + /// + private static CachedBatch MakeCachedBatch( + uint ibo, uint firstIndex, int indexCount, ulong texHandle, Matrix4x4? restPose = null) + { + var key = new GroupKey( + Ibo: ibo, + FirstIndex: firstIndex, + BaseVertex: 0, + IndexCount: indexCount, + BindlessTextureHandle: texHandle, + TextureLayer: 0, + Translucency: TranslucencyKind.Opaque); + return new CachedBatch(key, texHandle, restPose ?? Matrix4x4.Identity); + } + + [Fact] + public void Draw_StaticEntity_PopulatesCacheOnFirstFrameAndHitsOnSecond() + { + // Spec §7.2 test #11. + // Drives Draw's populate + cache-hit algorithm through the production + // static helpers. Verifies that: + // 1. First "frame": cache is empty → populate fires once at the + // end-of-loop final flush (entity.Id=100 has 2 batches). + // 2. Second "frame": cache.TryGet(100) hits → ApplyCacheHit appends + // cached batches to a fresh _groups dict without re-populating. + // cache.Count stays at 1 (Populate is idempotent via overwrite, + // but the hit-path doesn't re-populate at all). + var cache = new EntityClassificationCache(); + var scratch = new List(); + + Assert.Equal(0, cache.Count); + + // Frame 1: simulate one foreach iteration producing 2 batches for + // entity 100 in landblock 0xA9B40000. With no prior tracker, the + // entity-change flush is a no-op. ClassifyBatches' collector adds + // to scratch. The end-of-loop FinalFlushPopulate commits. + const uint EntityId = 100; + const uint LandblockId = 0xA9B40000u; + + // First MeshRef contributes 2 batches (mimics ClassifyBatches output). + scratch.Add(MakeCachedBatch(ibo: 1, firstIndex: 0, indexCount: 6, texHandle: 0xAA)); + scratch.Add(MakeCachedBatch(ibo: 1, firstIndex: 6, indexCount: 6, texHandle: 0xBB)); + + uint? populateEntityId = null; + uint populateLandblockId = 0u; + // First-tuple boundary check: no flush, sets the tracker. + (populateEntityId, populateLandblockId) = WbDrawDispatcher.MaybeFlushOnEntityChange( + populateEntityId, populateLandblockId, EntityId, cache, scratch); + // After ClassifyBatches the loop sets the tracker (matching Draw). + populateEntityId = EntityId; + populateLandblockId = LandblockId; + + // End-of-loop final flush — this is where the cache populates. + WbDrawDispatcher.FinalFlushPopulate(populateEntityId, populateLandblockId, cache, scratch); + + // First-frame post-conditions: 1 cache entry, 2 batches in it. + Assert.Equal(1, cache.Count); + Assert.True(cache.TryGet(EntityId, out var entry)); + Assert.NotNull(entry); + Assert.Equal(2, entry!.Batches.Length); + Assert.Equal(0xAAul, entry.Batches[0].BindlessTextureHandle); + Assert.Equal(0xBBul, entry.Batches[1].BindlessTextureHandle); + + // Frame 2: cache hit. ApplyCacheHit walks the cached batches and + // appends RestPose * entityWorld to a per-frame group dict. + // Production code: this is the !isAnimated && _cache.TryGet branch + // at the top of the per-entity loop body in Draw. + var groups = new Dictionary>(); + void AppendInstance(GroupKey k, Matrix4x4 m) + { + if (!groups.TryGetValue(k, out var list)) + { + list = new List(); + groups[k] = list; + } + list.Add(m); + } + + Assert.True(cache.TryGet(EntityId, out var entryHit)); + Assert.NotNull(entryHit); + var entityWorld = Matrix4x4.CreateTranslation(new Vector3(10f, 20f, 30f)); + WbDrawDispatcher.ApplyCacheHit(entryHit!, entityWorld, AppendInstance); + + // Cache state stable — Populate didn't fire on the hit path. + Assert.Equal(1, cache.Count); + + // Both groups received exactly one matrix each (the entity is one + // instance contributing once per cached batch). + Assert.Equal(2, groups.Count); + foreach (var (_, list) in groups) + Assert.Single(list); + + // Matrix composition is RestPose * entityWorld (NOT the reverse). + // RestPose is Matrix4x4.Identity for the synthesized batches, so the + // appended matrix must equal entityWorld. + foreach (var (_, list) in groups) + Assert.Equal(entityWorld, list[0]); + } + + [Fact] + public void Draw_AnimatedEntity_DoesNotPopulateCache() + { + // Spec §7.2 test #12. + // Animated entities take the slow path with collector=null: their + // ClassifyBatches output is NOT routed into _populateScratch and the + // populate-tracking locals stay null. Result: the cache is never + // populated for animated entities, and FinalFlushPopulate is a no-op. + // + // This test models that flow: scratch stays empty, populateEntityId + // stays null, FinalFlushPopulate fires but commits nothing. + var cache = new EntityClassificationCache(); + var scratch = new List(); + + const uint AnimatedId = 7; + const uint LandblockId = 0xA9B40000u; + var animatedSet = new HashSet { AnimatedId }; + + // Even when the entity has MeshRefs that would produce batches, the + // animated-set membership means collector=null in Draw — scratch + // stays empty and the tracker stays null. Simulating that here: + // we do NOT add to scratch and we do NOT set populateEntityId. + bool isAnimated = animatedSet.Contains(AnimatedId); + Assert.True(isAnimated); + + uint? populateEntityId = null; + uint populateLandblockId = 0u; + // Boundary check still runs but is a no-op — tracker is null. + (populateEntityId, populateLandblockId) = WbDrawDispatcher.MaybeFlushOnEntityChange( + populateEntityId, populateLandblockId, AnimatedId, cache, scratch); + + // For animated entities, Draw does NOT set populateEntityId after + // ClassifyBatches (the `if (collector is not null)` guard). + // populateEntityId stays null. + + // End-of-loop flush — no-op for animated-only iterations. + WbDrawDispatcher.FinalFlushPopulate(populateEntityId, populateLandblockId, cache, scratch); + + // Cache should never be populated for animated entities. + Assert.Equal(0, cache.Count); + Assert.False(cache.TryGet(AnimatedId, out _)); + + // Suppress unused-variable warning — LandblockId is here for parity + // with the static-entity test's structure. + _ = LandblockId; + } + + [Fact] + public void Draw_MultiMeshRefStaticEntity_PopulatesAllBatchesIntoSingleCacheEntry() + { + // Regression test for the bug fixed at commit 00fa8ae: + // + // Task 9's first attempt called _cache.Populate per-(entity, + // MeshRefIndex) tuple, but the cache is keyed by entity.Id. For + // multi-MeshRef entities (multi-part Setup buildings, statues, + // NPCs), each iteration's Populate OVERWROTE the previous one + // — only the LAST MeshRef's batches survived in the cache. After + // the fix, Populate fires once per entity at the entity boundary + // (or end-of-loop), with all MeshRefs' batches accumulated into + // _populateScratch. + // + // This test simulates a 3-MeshRef static entity where each MeshRef + // contributes 2 batches (total = 6). It walks through Draw's loop + // structure tuple-by-tuple, calling MaybeFlushOnEntityChange before + // each tuple's classification and FinalFlushPopulate at end-of-loop. + // Asserts the cache entry holds ALL 6 batches, not just the last 2. + // + // If the per-MeshRef Populate bug were reintroduced, this test would + // see Batches.Length == 2 (last MeshRef only). + var cache = new EntityClassificationCache(); + var scratch = new List(); + + const uint EntityId = 200; + const uint LandblockId = 0xA9B40000u; + const int MeshRefCount = 3; + const int BatchesPerMeshRef = 2; + const int ExpectedTotalBatches = MeshRefCount * BatchesPerMeshRef; + + uint? populateEntityId = null; + uint populateLandblockId = 0u; + + // Simulate Draw's foreach over _walkScratch. _walkScratch yields + // (entity, MeshRefIndex, landblockId) — all MeshRefs of one entity + // are contiguous because the walk emits them in entity-order. + for (int meshRefIdx = 0; meshRefIdx < MeshRefCount; meshRefIdx++) + { + // Boundary check: same entity across all 3 iterations, so this + // never fires the flush. populateEntityId stays as is (null on + // first iter; EntityId on subsequent iters after we set it). + (populateEntityId, populateLandblockId) = WbDrawDispatcher.MaybeFlushOnEntityChange( + populateEntityId, populateLandblockId, EntityId, cache, scratch); + + // Mimic ClassifyBatches' collector output for THIS MeshRef: + // 2 batches with distinct (ibo, firstIndex, texHandle) so the + // ordering can be verified post-hoc. + for (int b = 0; b < BatchesPerMeshRef; b++) + { + ulong texHandle = (ulong)(0x100 + meshRefIdx * BatchesPerMeshRef + b); + scratch.Add(MakeCachedBatch( + ibo: (uint)(meshRefIdx + 1), + firstIndex: (uint)(b * 6), + indexCount: 6, + texHandle: texHandle)); + } + + // After ClassifyBatches, Draw sets the tracker (matching the + // `if (collector is not null)` block at line 482-486 in + // WbDrawDispatcher.Draw). + populateEntityId = EntityId; + populateLandblockId = LandblockId; + } + + // End-of-loop final flush. Without this call (or if Populate fired + // per-tuple inside the loop), the cache would only hold the last + // 2 batches — exactly the bug class from commit 00fa8ae. + WbDrawDispatcher.FinalFlushPopulate(populateEntityId, populateLandblockId, cache, scratch); + + // Assertions: ONE cache entry with ALL 6 batches in MeshRef order. + Assert.Equal(1, cache.Count); + Assert.True(cache.TryGet(EntityId, out var entry)); + Assert.NotNull(entry); + Assert.Equal(EntityId, entry!.EntityId); + Assert.Equal(LandblockId, entry.LandblockHint); + + // KEY ASSERTION: Batches.Length == sum across MeshRefs (6), + // NOT just the last MeshRef's batch count (2). + Assert.Equal(ExpectedTotalBatches, entry.Batches.Length); + + // Per-batch ordering check: batches arrived in MeshRef order, so + // texture handles run 0x100..0x105 in the order they were appended. + for (int i = 0; i < ExpectedTotalBatches; i++) + Assert.Equal((ulong)(0x100 + i), entry.Batches[i].BindlessTextureHandle); + + // After flush, scratch is cleared so the next entity starts fresh. + Assert.Empty(scratch); + } }