#119 ROOT CAUSE: interior-id X-byte collision + player-landblock cache hints = cross-entity batch serving
The decisive probe (3cf6bcc) caught it live in ONE session: a 43-part
staircase entity (src=0x020003F2, healthy MeshRefs tZ=[0.35..15.15])
drew with cache=hit:3 restZero=3 - THREE batches belonging to a 1-part
entity - then under a different hint the correct hit:119. Two
compounding bugs:
1. interiorIdBase = 0x40000000 | (landblockId & 0x00FFFF00) resolved to
0x40YYFF00 for landblock keys 0xXXYYFFFF - the landblock X byte
DISCARDED. Every landblock in a map Y-row shared one id space:
Holtburg town A9B3's 9th interior stab == the AAB3 tower's spiral
staircase, both 0x40B3FF09. Fixed to 0x40000000|(lbX<<16)|(lbY<<8)
(the scenery 0x80XXYY## scheme).
2. The Tier-1 classification cache's #53 tuple key (EntityId,
LandblockHint) was fed the PLAYER's landblock at bucket-draw time
(RetailPViewRenderer.DrawEntityBucket fabricates its tuple with
ctx.PlayerLandblockId), so colliding ids from different landblocks
shared a key: whichever entity classified first under a hint won,
and the loser wore its batches all session (static fast path never
re-classifies). Also: bucket-hinted entries were never swept by
InvalidateLandblock(owner) - stale entries survived owner unload.
Fixed: ResolveCacheLandblockHint derives the hint from the entity's
owning cell (ParentCellId landblock, canonical 0xXXYYFFFF), falling
back to the tuple id for ownerless paths (outdoor stabs/scenery,
where the tuple IS the owner).
Explains the session-shaped repro exactly: town-login + run to the
tower hydrates/classifies town interiors first -> the tower staircase
cache-hits the town twin's batches (stairs missing/partial + a wrong
object near the floor - the "water barrel"); login-inside classifies
the tower first -> usually clean. meshMissing=0 / entSeen==entDrawn
both ways (everything draws, wrong batches). Likely also feeds #113's
distance-dependent phantom staircase (the town twin wearing the
tower's staircase batches).
3 new cache tests pin the collision contract + hint derivation.
Suites: App green / Core 1430+2skip / UI 420 / Net 294.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik
parent
3cf6bcc219
commit
2163308032
4 changed files with 122 additions and 10 deletions
|
|
@ -5514,9 +5514,24 @@ public sealed class GameWindow : IDisposable
|
|||
(lbY - _liveCenterY) * 192f,
|
||||
0f);
|
||||
|
||||
// Per-landblock id namespace: 0x40000000 | (lbId & 0x00FFFF00) | local_counter.
|
||||
// Distinct from scenery (0x80000000+) and stabs (ids from LandblockLoader).
|
||||
uint interiorIdBase = 0x40000000u | (landblockId & 0x00FFFF00u);
|
||||
// Per-landblock id namespace: 0x40000000 | (lbX << 16) | (lbY << 8) | local_counter —
|
||||
// the same 0xNNXXYY## scheme scenery uses (0x80XXYY##). Distinct from scenery
|
||||
// (0x80000000+) and stabs (ids from LandblockLoader).
|
||||
//
|
||||
// #119 ROOT-CAUSE FIX (2026-06-11): this used to be
|
||||
// `0x40000000 | (landblockId & 0x00FFFF00)`, which for landblock keys 0xXXYYFFFF
|
||||
// resolves to 0x40YYFF00 — the landblock X byte DISCARDED. Every landblock in a
|
||||
// map Y-row produced the same id base, so interior statics collided across
|
||||
// landblocks (Holtburg town A9B3's 9th stab == the AAB3 tower's 43-part spiral
|
||||
// staircase, both 0x40B3FF09). The Tier-1 classification cache then served one
|
||||
// entity's batches to the other (the cache hint at bucket-draw time was the
|
||||
// player's landblock, identical for both) — the session-sticky "broken stairs +
|
||||
// water barrel". Counter overflow past 0xFF still bleeds into the lbY byte (the
|
||||
// documented #53 scenery-namespace caveat); the cache's (EntityId, owner-derived
|
||||
// LandblockHint) tuple key disambiguates that residual case.
|
||||
uint interiorLbX = (landblockId >> 24) & 0xFFu;
|
||||
uint interiorLbY = (landblockId >> 16) & 0xFFu;
|
||||
uint interiorIdBase = 0x40000000u | (interiorLbX << 16) | (interiorLbY << 8);
|
||||
uint localCounter = 0;
|
||||
|
||||
uint firstCellId = (landblockId & 0xFFFF0000u) | 0x0100u;
|
||||
|
|
|
|||
|
|
@ -12,12 +12,19 @@ namespace AcDream.App.Rendering.Wb;
|
|||
/// <b>Key composition:</b> entries are keyed by the tuple
|
||||
/// <c>(EntityId, LandblockHint)</c>, NOT by <c>EntityId</c> alone. Issue #53
|
||||
/// uncovered that <c>entity.Id</c> is NOT globally unique across all
|
||||
/// static-entity hydration paths: scenery (<c>0x80LLBB00 + localIndex</c>)
|
||||
/// and interior cells (<c>0x40LLBB00 + localCounter</c>) overflow at >256
|
||||
/// static-entity hydration paths: scenery (<c>0x80XXYY00 + localIndex</c>)
|
||||
/// and interior cells (<c>0x40XXYY00 + localCounter</c>, X-byte fixed
|
||||
/// 2026-06-11 — it used to be discarded entirely, #119) overflow at >256
|
||||
/// items per landblock, wrapping into the <c>lbY</c> byte and producing
|
||||
/// cross-LB collisions in dense forest/urban LBs outside Holtburg. Keying
|
||||
/// by the tuple is correct-by-construction regardless of any hydration
|
||||
/// path's id strategy.
|
||||
/// by the tuple is correct-by-construction ONLY when the hint identifies the
|
||||
/// entity's OWNING landblock — callers must derive it via
|
||||
/// <c>WbDrawDispatcher.ResolveCacheLandblockHint</c> (the entity's
|
||||
/// ParentCellId landblock when present, canonicalized <c>0xXXYYFFFF</c>),
|
||||
/// never a call-site landblock. The #119 "broken stairs + water barrel" was
|
||||
/// exactly this: the bucket draw path hinted every entity with the PLAYER's
|
||||
/// landblock, so colliding ids from different landblocks shared a key and
|
||||
/// served each other's batches.
|
||||
/// </para>
|
||||
///
|
||||
/// <para>
|
||||
|
|
|
|||
|
|
@ -736,6 +736,28 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
|||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// #119 ROOT-CAUSE FIX (2026-06-11): the Tier-1 cache hint must identify the
|
||||
/// entity's OWNING landblock, not the Draw call's tuple landblock.
|
||||
/// <c>RetailPViewRenderer.DrawEntityBucket</c> fabricates its tuple with the
|
||||
/// PLAYER's landblock id, so every bucket entity that frame shared one hint —
|
||||
/// and colliding entity ids from different landblocks (the pre-fix
|
||||
/// <c>0x40YYFF00</c> interior namespace discarded the landblock X byte) mapped
|
||||
/// to the SAME cache key and served each other's batches: the AAB3 tower's
|
||||
/// 43-part staircase drew a 1-part entity's 3 zero-RestPose batches
|
||||
/// (captured live, tower-dump-launch1.log) — the session-sticky "broken
|
||||
/// stairs + water barrel". Interior statics carry their owning cell; derive
|
||||
/// the hint from it, canonicalized to the same <c>0xXXYYFFFF</c> key format
|
||||
/// the streaming entries and <see cref="EntityClassificationCache.InvalidateLandblock"/>
|
||||
/// use — which also makes owner-unload invalidation actually hit these
|
||||
/// entries (bucket-hinted entries were previously orphaned forever).
|
||||
/// Entities without a ParentCellId (outdoor stabs / scenery / building
|
||||
/// shells via GpuWorldState entries) keep the tuple id, which IS their
|
||||
/// owner on those paths.
|
||||
/// </summary>
|
||||
internal static uint ResolveCacheLandblockHint(WorldEntity entity, uint tupleLandblockId)
|
||||
=> entity.ParentCellId is uint pc ? ((pc & 0xFFFF0000u) | 0xFFFFu) : tupleLandblockId;
|
||||
|
||||
/// <summary>
|
||||
/// #119 decisive probe: rate-limited <c>[dump-entity] WALK-REJECT</c> line
|
||||
/// for an <c>ACDREAM_DUMP_ENTITY</c>-targeted entity that the walk filtered
|
||||
|
|
@ -1005,6 +1027,11 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
|||
// re-mark the entity "complete" and let partial data populate
|
||||
// the cache. Trees with [trunk valid, branches null, leaves
|
||||
// valid] hit this exactly — branches never recover.
|
||||
// #119 root-cause fix: cache operations key on the entity's OWNING
|
||||
// landblock, never the Draw call's tuple landblock (which is the
|
||||
// PLAYER's landblock on the bucket path). See ResolveCacheLandblockHint.
|
||||
uint cacheLb = ResolveCacheLandblockHint(entity, landblockId);
|
||||
|
||||
bool isNewEntity = !prevTupleEntityId.HasValue || prevTupleEntityId.Value != entity.Id;
|
||||
if (isNewEntity)
|
||||
{
|
||||
|
|
@ -1029,7 +1056,7 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
|||
// #119 decisive probe: one-shot dump (+ change re-emission) for
|
||||
// ACDREAM_DUMP_ENTITY-targeted entities. Before the culled-continue
|
||||
// so a routed-out entity still reports its state.
|
||||
MaybeEmitEntityDump(entity, landblockId, _currentEntityCulled);
|
||||
MaybeEmitEntityDump(entity, cacheLb, _currentEntityCulled);
|
||||
}
|
||||
prevTupleEntityId = entity.Id;
|
||||
|
||||
|
|
@ -1072,7 +1099,7 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
|||
// ApplyCacheHit, sets lastHitEntityId, and continues. Subsequent
|
||||
// tuples of the same entity short-circuit at the top of the loop
|
||||
// body via the lastHitEntityId == entity.Id check above.
|
||||
if (!isAnimated && !_tier1CacheDisabled && _cache.TryGet(entity.Id, landblockId, out var cachedEntry))
|
||||
if (!isAnimated && !_tier1CacheDisabled && _cache.TryGet(entity.Id, cacheLb, out var cachedEntry))
|
||||
{
|
||||
ApplyCacheHit(cachedEntry!, entityWorld, AppendInstanceToGroup);
|
||||
|
||||
|
|
@ -1254,10 +1281,13 @@ public sealed unsafe class WbDrawDispatcher : IDisposable
|
|||
// Track THIS entity for the next iteration's flush check. Only
|
||||
// when collector is non-null (entity is static); animated entities
|
||||
// leave the tracker null so we don't try to flush them.
|
||||
// #119: the populate commits under the OWNER-derived hint so the
|
||||
// entry is found by the same key on the next frame's TryGet and
|
||||
// swept by InvalidateLandblock when the OWNING landblock unloads.
|
||||
if (collector is not null)
|
||||
{
|
||||
populateEntityId = entity.Id;
|
||||
populateLandblockId = landblockId;
|
||||
populateLandblockId = cacheLb;
|
||||
}
|
||||
|
||||
if (diag && drewAny) _entitiesDrawn++;
|
||||
|
|
|
|||
|
|
@ -49,6 +49,66 @@ public class EntityClassificationCacheTests
|
|||
Assert.Equal(0xCCu, entry.Batches[0].BindlessTextureHandle);
|
||||
}
|
||||
|
||||
// ── #119 root-cause regression (2026-06-11): colliding entity ids across
|
||||
// landblocks must never share a cache entry. The bucket draw path used to
|
||||
// hint every entity with the PLAYER's landblock, so the AAB3 tower's
|
||||
// 43-part staircase (id 0x40B3FF09) cache-hit the batches of Holtburg
|
||||
// town A9B3's 9th interior stab (same id under the old 0x40YYFF00
|
||||
// namespace) — "broken stairs + water barrel". The hint must be derived
|
||||
// from the entity's OWNER via WbDrawDispatcher.ResolveCacheLandblockHint.
|
||||
|
||||
[Fact]
|
||||
public void ResolveCacheLandblockHint_InteriorEntity_DerivesOwnerLandblock()
|
||||
{
|
||||
var towerStairs = MakeEntity(id: 0x40AAB309u, parentCellId: 0xAAB30107u);
|
||||
// Tuple landblock = the PLAYER's landblock (the bucket path) — must be ignored.
|
||||
uint hint = WbDrawDispatcher.ResolveCacheLandblockHint(towerStairs, tupleLandblockId: 0xA9B3FFFFu);
|
||||
Assert.Equal(0xAAB3FFFFu, hint);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ResolveCacheLandblockHint_NoParentCell_KeepsTupleLandblock()
|
||||
{
|
||||
var outdoorStab = MakeEntity(id: 0x00001234u, parentCellId: null);
|
||||
uint hint = WbDrawDispatcher.ResolveCacheLandblockHint(outdoorStab, tupleLandblockId: 0xA9B4FFFFu);
|
||||
Assert.Equal(0xA9B4FFFFu, hint);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CollidingEntityIds_UnderOwnerHints_KeepDistinctBatchSets()
|
||||
{
|
||||
// Same entity id (the residual >256-counter overlap the tuple key exists
|
||||
// for), two different owning landblocks → two entries, each serving its
|
||||
// own batches. Pre-fix, both would have been keyed under one player-lb
|
||||
// hint and the second entity would draw the first one's batches.
|
||||
var cache = new EntityClassificationCache();
|
||||
var townBatches = new[] { MakeCachedBatch(1, 0, 6, 0xAA) };
|
||||
var towerBatches = new[] { MakeCachedBatch(2, 0, 12, 0xBB), MakeCachedBatch(2, 12, 6, 0xCC) };
|
||||
|
||||
cache.Populate(0x40B3FF09u, 0xA9B3FFFFu, townBatches);
|
||||
cache.Populate(0x40B3FF09u, 0xAAB3FFFFu, towerBatches);
|
||||
|
||||
Assert.True(cache.TryGet(0x40B3FF09u, 0xA9B3FFFFu, out var town));
|
||||
Assert.True(cache.TryGet(0x40B3FF09u, 0xAAB3FFFFu, out var tower));
|
||||
Assert.Equal(townBatches, town!.Batches);
|
||||
Assert.Equal(towerBatches, tower!.Batches);
|
||||
|
||||
// Owner-landblock invalidation sweeps ONLY the owner's entry.
|
||||
cache.InvalidateLandblock(0xA9B3FFFFu);
|
||||
Assert.False(cache.TryGet(0x40B3FF09u, 0xA9B3FFFFu, out _));
|
||||
Assert.True(cache.TryGet(0x40B3FF09u, 0xAAB3FFFFu, out _));
|
||||
}
|
||||
|
||||
private static AcDream.Core.World.WorldEntity MakeEntity(uint id, uint? parentCellId) => new()
|
||||
{
|
||||
Id = id,
|
||||
SourceGfxObjOrSetupId = 0x020003F2u,
|
||||
Position = Vector3.Zero,
|
||||
Rotation = Quaternion.Identity,
|
||||
MeshRefs = new List<AcDream.Core.World.MeshRef>(),
|
||||
ParentCellId = parentCellId,
|
||||
};
|
||||
|
||||
[Fact]
|
||||
public void Count_TracksLiveEntries()
|
||||
{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue