fix(render #53): skip cache populate when classification is incomplete
User reported: the drudge statue on top of the Foundry (a multi-part live-spawned entity with AnimPartChange + texChanges) renders only PARTIALLY — some parts visible, some missing. Root cause: the dispatcher's slow path skips a MeshRef when _meshAdapter.TryGetRenderData returns null (mesh still async-decoding via ObjectMeshManager.PrepareMeshDataAsync). The classified-batches collector accumulates only the MeshRefs that DID resolve. At entity boundary, the cache populates with the PARTIAL set. Frame-2 cache hits serve that partial entry forever — even after the missing mesh loads, the cache continues to skip those parts because classification never reruns for cached entities. Fix: track currentEntityIncomplete during the foreach. Set it true on any null renderData. At entity boundary (and at end-of-loop), if the flag is set, DROP the accumulated populate scratch instead of writing it to the cache. The slow path retries on the next frame; once all meshes have loaded, the populate fires correctly with the complete classification. Adds a regression test pinning the contract — incomplete entities produce zero cache entries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
95ebbf3004
commit
c55acdc3d5
2 changed files with 107 additions and 0 deletions
|
|
@ -409,6 +409,20 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
||||||
// entity; subsequent tuples skip via this tracker.
|
// entity; subsequent tuples skip via this tracker.
|
||||||
uint? lastHitEntityId = null;
|
uint? lastHitEntityId = null;
|
||||||
|
|
||||||
|
// Tier 1 cache (#53) — incomplete-entity guard. When any MeshRef of
|
||||||
|
// the current entity has _meshAdapter.TryGetRenderData return null
|
||||||
|
// (mesh still async-decoding via ObjectMeshManager.PrepareMeshDataAsync),
|
||||||
|
// we mark the entity incomplete and DROP the accumulated populate
|
||||||
|
// scratch at entity boundary instead of writing it to the cache.
|
||||||
|
// Otherwise the cache would hold a partial classification (some parts
|
||||||
|
// missing), and frame-2 cache hits would persist that partial render
|
||||||
|
// even after the missing mesh loads — every subsequent frame sees the
|
||||||
|
// cache hit and skips re-classification, so the missing parts never
|
||||||
|
// recover. User-visible symptom: the drudge statue on top of the
|
||||||
|
// Foundry (multi-part Setup entity with AnimPartChange) renders with
|
||||||
|
// some parts missing permanently. Reset on entity change.
|
||||||
|
bool currentEntityIncomplete = false;
|
||||||
|
|
||||||
foreach (var (entity, partIdx, landblockId) in _walkScratch)
|
foreach (var (entity, partIdx, landblockId) in _walkScratch)
|
||||||
{
|
{
|
||||||
if (diag) _entitiesSeen++;
|
if (diag) _entitiesSeen++;
|
||||||
|
|
@ -433,6 +447,20 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
||||||
lastHitEntityId = null;
|
lastHitEntityId = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Tier 1 cache (#53) — drop the previous entity's accumulated
|
||||||
|
// populate scratch BEFORE MaybeFlushOnEntityChange runs. If the
|
||||||
|
// previous entity ended incomplete (≥1 null renderData), we MUST
|
||||||
|
// NOT cache its partial classification: clear scratch and null
|
||||||
|
// the tracker so MaybeFlushOnEntityChange sees the cleaned state
|
||||||
|
// and no-ops for this entity. Reset the incomplete flag for the
|
||||||
|
// new entity so each one gets a fresh measurement.
|
||||||
|
if (populateEntityId.HasValue && populateEntityId.Value != entity.Id && currentEntityIncomplete)
|
||||||
|
{
|
||||||
|
_populateScratch.Clear();
|
||||||
|
populateEntityId = null;
|
||||||
|
}
|
||||||
|
currentEntityIncomplete = false;
|
||||||
|
|
||||||
// Flush-on-entity-change: if the previous entity accumulated any
|
// Flush-on-entity-change: if the previous entity accumulated any
|
||||||
// batches AND this iteration is for a different entity, populate
|
// batches AND this iteration is for a different entity, populate
|
||||||
// its cache entry now and reset the scratch buffer.
|
// its cache entry now and reset the scratch buffer.
|
||||||
|
|
@ -518,6 +546,14 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
||||||
var renderData = _meshAdapter.TryGetRenderData(gfxObjId);
|
var renderData = _meshAdapter.TryGetRenderData(gfxObjId);
|
||||||
if (renderData is null)
|
if (renderData is null)
|
||||||
{
|
{
|
||||||
|
// Tier 1 cache (#53): mesh data is still async-decoding via
|
||||||
|
// WB's ObjectMeshManager.PrepareMeshDataAsync. Flag the entity
|
||||||
|
// as incomplete so the entity-boundary check (or end-of-loop
|
||||||
|
// check) drops the accumulated populate scratch instead of
|
||||||
|
// caching a partial classification. The slow path retries on
|
||||||
|
// the next frame; once all this entity's meshes have loaded,
|
||||||
|
// the populate fires with the complete batch set.
|
||||||
|
currentEntityIncomplete = true;
|
||||||
if (diag) _meshesMissing++;
|
if (diag) _meshesMissing++;
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
@ -564,6 +600,17 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
||||||
if (diag && drewAny) _entitiesDrawn++;
|
if (diag && drewAny) _entitiesDrawn++;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Tier 1 cache (#53) — drop the accumulated populate scratch if the
|
||||||
|
// LAST entity in the loop ended incomplete (had ≥1 null renderData).
|
||||||
|
// Same reason as the entity-boundary handling above: avoid caching a
|
||||||
|
// partial classification. The slow path will retry on the next frame
|
||||||
|
// and populate correctly once all meshes have loaded.
|
||||||
|
if (currentEntityIncomplete)
|
||||||
|
{
|
||||||
|
_populateScratch.Clear();
|
||||||
|
populateEntityId = null;
|
||||||
|
}
|
||||||
|
|
||||||
// Final flush: the last entity in _walkScratch has no "next iteration"
|
// Final flush: the last entity in _walkScratch has no "next iteration"
|
||||||
// to trigger the entity-change flush, so commit its accumulated batches
|
// to trigger the entity-change flush, so commit its accumulated batches
|
||||||
// here. No-op when the last entity was animated (populateEntityId stays
|
// here. No-op when the last entity was animated (populateEntityId stays
|
||||||
|
|
|
||||||
|
|
@ -603,6 +603,66 @@ public sealed class WbDrawDispatcherBucketingTests
|
||||||
Assert.Empty(scratch);
|
Assert.Empty(scratch);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Cache_Populate_SkipsEntityWithIncompleteClassification()
|
||||||
|
{
|
||||||
|
// Regression test for the bug where an entity with >=1 MeshRef whose
|
||||||
|
// mesh data was still async-decoding at populate time would have a
|
||||||
|
// PARTIAL set of batches written to the cache. Subsequent frame
|
||||||
|
// cache-hits served the partial entry indefinitely, leaving parts of
|
||||||
|
// multi-part entities (drudge statue, etc.) permanently missing.
|
||||||
|
//
|
||||||
|
// The fix: track currentEntityIncomplete during the foreach. If any
|
||||||
|
// tuple's TryGetRenderData returned null, drop the accumulated
|
||||||
|
// populate scratch at entity boundary instead of caching it. The
|
||||||
|
// slow path retries on the next frame; once all meshes have loaded,
|
||||||
|
// the populate fires correctly with the complete classification.
|
||||||
|
//
|
||||||
|
// This test simulates Draw's inner-loop logic: 3 MeshRef tuples for
|
||||||
|
// one entity where tuple 0 produces null renderData (flag the entity
|
||||||
|
// incomplete + continue, no batches), and tuples 1 and 2 produce
|
||||||
|
// valid renderData (classify + accumulate). End-of-loop check drops
|
||||||
|
// scratch + nulls populateEntityId BEFORE FinalFlushPopulate, so the
|
||||||
|
// cache stays empty for this entity.
|
||||||
|
var cache = new EntityClassificationCache();
|
||||||
|
const uint EntityId = 100;
|
||||||
|
const uint LandblockId = 0xA9B40000u;
|
||||||
|
|
||||||
|
// Simulate Draw's per-entity inner-loop logic.
|
||||||
|
var scratch = new List<CachedBatch>();
|
||||||
|
bool currentEntityIncomplete = false;
|
||||||
|
uint? populateEntityId = null;
|
||||||
|
uint populateLandblockId = 0u;
|
||||||
|
|
||||||
|
// Tuple 0 (MeshRef[0]): renderData null -> flag incomplete, skip classify.
|
||||||
|
currentEntityIncomplete = true;
|
||||||
|
|
||||||
|
// Tuple 1 (MeshRef[1]): renderData valid -> classify, accumulate.
|
||||||
|
scratch.Add(MakeCachedBatch(ibo: 1, firstIndex: 0, indexCount: 6, texHandle: 0xAAul));
|
||||||
|
populateEntityId = EntityId;
|
||||||
|
populateLandblockId = LandblockId;
|
||||||
|
|
||||||
|
// Tuple 2 (MeshRef[2]): renderData valid -> classify, accumulate.
|
||||||
|
scratch.Add(MakeCachedBatch(ibo: 2, firstIndex: 0, indexCount: 6, texHandle: 0xBBul));
|
||||||
|
populateEntityId = EntityId;
|
||||||
|
populateLandblockId = LandblockId;
|
||||||
|
|
||||||
|
// End of loop: check incomplete flag, drop scratch + null tracker
|
||||||
|
// BEFORE FinalFlushPopulate so the helper sees the cleaned state.
|
||||||
|
if (currentEntityIncomplete)
|
||||||
|
{
|
||||||
|
scratch.Clear();
|
||||||
|
populateEntityId = null;
|
||||||
|
}
|
||||||
|
WbDrawDispatcher.FinalFlushPopulate(populateEntityId, populateLandblockId, cache, scratch);
|
||||||
|
|
||||||
|
// Cache should NOT have an entry for this entity — partial population
|
||||||
|
// would be worse than no cache (cache hit would persist the partial
|
||||||
|
// render forever; cache miss retries and gets it right next frame).
|
||||||
|
Assert.Equal(0, cache.Count);
|
||||||
|
Assert.False(cache.TryGet(EntityId, LandblockId, out _));
|
||||||
|
}
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
public void ApplyCacheHit_PerTupleAmplification_DoesNotOccur()
|
public void ApplyCacheHit_PerTupleAmplification_DoesNotOccur()
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue