fix(N.4) Adjustment 2: revert Task 9 renderer-level routing
Smoke test flag-on showed characters/NPCs disappearing along with static scenery. Root cause: Task 9 routed all InstancedMeshRenderer.EnsureUploaded calls through WB. But that renderer is used for BOTH tiers in production — character per-part spawn (line 2302, per-instance) AND streaming-loader spawns (lines 5137 + 5155, atlas). The renderer is tier-blind by design. Tier-routing belongs at the spawn-callback layer per the spec's data-flow section: - LandblockSpawnAdapter (Task 11) calls IncrementRefCount per unique GfxObj — atlas-tier only. - EntitySpawnAdapter (Task 17) routes through per-instance path via TextureCache.GetOrUploadWithPaletteOverride. This commit removes the sentinel pattern + 4 sentinel-skip checks from InstancedMeshRenderer. Kept the _wbMeshAdapter constructor parameter (unused for now) so GameWindow's wire-up doesn't shift. Kept all the real WB pipeline construction in WbMeshAdapter (it's the substrate routing will use in Week 2). Verified flag-on === flag-off post-revert. Plan updated with Adjustment 2 explaining the discovery + correct architectural placement for routing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
c49c6edde5
commit
4f318bcbba
2 changed files with 53 additions and 38 deletions
|
|
@ -837,6 +837,47 @@ writing a real bridge that shares index caches with our `DatCollection`.
|
||||||
|
|
||||||
**No work for this task — skip and proceed to Task 7.**
|
**No work for this task — skip and proceed to Task 7.**
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Adjustment 2 (2026-05-08): Task 9 routing reverted — tier decision belongs at spawn-callback layer
|
||||||
|
|
||||||
|
**Discovered during Week 1 visual smoke test**: with flag on, characters /
|
||||||
|
NPCs disappeared along with static scenery. Root cause: Task 9 routed
|
||||||
|
**all** `InstancedMeshRenderer.EnsureUploaded` calls through
|
||||||
|
`WbMeshAdapter.IncrementRefCount` and marked their cache entries with
|
||||||
|
`WbManagedSentinel`. But `InstancedMeshRenderer` is used for both tiers
|
||||||
|
in production:
|
||||||
|
|
||||||
|
- **Atlas-tier** call sites: `_pendingCellMeshes` drain
|
||||||
|
([GameWindow.cs:5137](../../../src/AcDream.App/Rendering/GameWindow.cs:5137)),
|
||||||
|
per-MeshRef GfxObj loop on `lb.Entities`
|
||||||
|
([:5155](../../../src/AcDream.App/Rendering/GameWindow.cs:5155)).
|
||||||
|
- **Per-instance-tier** call sites: per-part loop in spawn handling
|
||||||
|
([:2302](../../../src/AcDream.App/Rendering/GameWindow.cs:2302)) — this is
|
||||||
|
character / creature rendering driven by server `CreateObject`.
|
||||||
|
|
||||||
|
The renderer is **tier-blind by design**: it doesn't know spawn source.
|
||||||
|
Putting routing logic there violates separation of concerns. The spec's
|
||||||
|
Data-Flow section already specifies the right placement — routing happens
|
||||||
|
at the **spawn-callback layer**:
|
||||||
|
|
||||||
|
- `LandblockSpawnAdapter.OnLandblockLoaded(...)` (Task 11) calls
|
||||||
|
`IncrementRefCount` per unique GfxObj — atlas-tier only.
|
||||||
|
- `EntitySpawnAdapter.OnCreate(entity)` (Task 17) routes through
|
||||||
|
per-instance path (`TextureCache.GetOrUploadWithPaletteOverride`) —
|
||||||
|
never calls `IncrementRefCount` for atlas.
|
||||||
|
|
||||||
|
**Resolution:** reverted Task 9's renderer-level routing. Removed the
|
||||||
|
sentinel logic and the 4 sentinel-skip checks in
|
||||||
|
`InstancedMeshRenderer`. **Kept** the `_wbMeshAdapter` constructor
|
||||||
|
parameter (unused for now) so `GameWindow.cs` doesn't shift when
|
||||||
|
later tasks need adapter access. Kept all the real WB pipeline
|
||||||
|
construction in `WbMeshAdapter` (verified working under flag-off).
|
||||||
|
|
||||||
|
**Week 1 endpoint shifts:** "WB infrastructure constructed; flag-on and
|
||||||
|
flag-off visually identical." Routing arrives in Week 2 (Task 11) at
|
||||||
|
the correct layer. Smoke verification is now: flag-on === flag-off.
|
||||||
|
|
||||||
### Task 6 (original — kept for history)
|
### Task 6 (original — kept for history)
|
||||||
|
|
||||||
**Files:**
|
**Files:**
|
||||||
|
|
|
||||||
|
|
@ -35,19 +35,16 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable
|
||||||
private readonly TextureCache _textures;
|
private readonly TextureCache _textures;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Optional WB adapter. When non-null and <see cref="WbFoundationFlag.IsEnabled"/>,
|
/// Optional WB adapter. Held but currently unused — Phase N.4 Adjustment 2
|
||||||
/// <see cref="EnsureUploaded"/> hands the GfxObj ref to the WB pipeline instead of
|
/// (2026-05-08) reverted Task 9's renderer-level routing. Tier-routing decisions
|
||||||
/// uploading into our own VAO pool. The draw loop skips sentinel entries — Task 22's
|
/// (atlas vs per-instance) belong at the spawn-callback layer (Task 11
|
||||||
/// WbDrawDispatcher will eventually draw them.
|
/// LandblockSpawnAdapter for atlas-tier; Task 17 EntitySpawnAdapter for
|
||||||
|
/// per-instance), not in the renderer which is intentionally tier-blind. The
|
||||||
|
/// constructor parameter is preserved so GameWindow's wire-up doesn't shift
|
||||||
|
/// when later tasks need adapter access.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
private readonly WbMeshAdapter? _wbMeshAdapter;
|
private readonly WbMeshAdapter? _wbMeshAdapter;
|
||||||
|
|
||||||
// Sentinel: a GfxObj that has been handed to the WB pipeline gets this list
|
|
||||||
// stored in _gpuByGfxObj. The Draw loop recognises it by reference identity
|
|
||||||
// (object.ReferenceEquals) and skips it — no legacy VAO draw for WB-managed
|
|
||||||
// objects until Task 22 wires up WbDrawDispatcher.
|
|
||||||
private static readonly List<SubMeshGpu> WbManagedSentinel = new(0);
|
|
||||||
|
|
||||||
// One GPU bundle per unique GfxObj id. Each GfxObj can have multiple sub-meshes.
|
// One GPU bundle per unique GfxObj id. Each GfxObj can have multiple sub-meshes.
|
||||||
private readonly Dictionary<uint, List<SubMeshGpu>> _gpuByGfxObj = new();
|
private readonly Dictionary<uint, List<SubMeshGpu>> _gpuByGfxObj = new();
|
||||||
|
|
||||||
|
|
@ -100,17 +97,11 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable
|
||||||
if (_gpuByGfxObj.ContainsKey(gfxObjId))
|
if (_gpuByGfxObj.ContainsKey(gfxObjId))
|
||||||
return;
|
return;
|
||||||
|
|
||||||
// Phase N.4 Task 9: when the WB foundation flag is on and we have an
|
// Phase N.4 Adjustment 2 (2026-05-08): renderer is tier-blind. Tier-routing
|
||||||
// adapter, hand this GfxObj to the WB pipeline instead of uploading our
|
// (atlas vs per-instance) lives at the spawn-callback layer (Tasks 11 + 17),
|
||||||
// own VAO. The sentinel entry marks "this GfxObj lives in WB now" so the
|
// not here. Smoke-test of the original Task 9 routing showed it caught
|
||||||
// draw loop knows to skip it. Task 22's WbDrawDispatcher will draw them.
|
// characters / NPCs (server-spawned, per-instance tier) along with static
|
||||||
if (WbFoundationFlag.IsEnabled && _wbMeshAdapter is not null)
|
// scenery, because EnsureUploaded is called from both spawn paths.
|
||||||
{
|
|
||||||
_wbMeshAdapter.IncrementRefCount(gfxObjId);
|
|
||||||
_gpuByGfxObj[gfxObjId] = WbManagedSentinel;
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
var list = new List<SubMeshGpu>(subMeshes.Count);
|
var list = new List<SubMeshGpu>(subMeshes.Count);
|
||||||
foreach (var sm in subMeshes)
|
foreach (var sm in subMeshes)
|
||||||
list.Add(UploadSubMesh(sm));
|
list.Add(UploadSubMesh(sm));
|
||||||
|
|
@ -245,11 +236,6 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable
|
||||||
if (!_gpuByGfxObj.TryGetValue(key.GfxObjId, out var subMeshes))
|
if (!_gpuByGfxObj.TryGetValue(key.GfxObjId, out var subMeshes))
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
// WB-managed GfxObjs have a sentinel entry; Task 22 (WbDrawDispatcher)
|
|
||||||
// will draw them. Skip here to avoid drawing with stale/null VAO data.
|
|
||||||
if (object.ReferenceEquals(subMeshes, WbManagedSentinel))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
bool hasOpaqueSubMesh = false;
|
bool hasOpaqueSubMesh = false;
|
||||||
foreach (var sub in subMeshes)
|
foreach (var sub in subMeshes)
|
||||||
{
|
{
|
||||||
|
|
@ -325,10 +311,6 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable
|
||||||
if (!_gpuByGfxObj.TryGetValue(key.GfxObjId, out var subMeshes))
|
if (!_gpuByGfxObj.TryGetValue(key.GfxObjId, out var subMeshes))
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
// WB-managed GfxObjs — skip; Task 22 will draw them.
|
|
||||||
if (object.ReferenceEquals(subMeshes, WbManagedSentinel))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
bool hasTranslucentSubMesh = false;
|
bool hasTranslucentSubMesh = false;
|
||||||
foreach (var sub in subMeshes)
|
foreach (var sub in subMeshes)
|
||||||
{
|
{
|
||||||
|
|
@ -458,9 +440,6 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable
|
||||||
{
|
{
|
||||||
if (!_gpuByGfxObj.TryGetValue(meshRef.GfxObjId, out var cachedMeshes))
|
if (!_gpuByGfxObj.TryGetValue(meshRef.GfxObjId, out var cachedMeshes))
|
||||||
continue;
|
continue;
|
||||||
// WB-managed GfxObjs don't go through our instance pipeline.
|
|
||||||
if (object.ReferenceEquals(cachedMeshes, WbManagedSentinel))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
var model = meshRef.PartTransform * entityRoot;
|
var model = meshRef.PartTransform * entityRoot;
|
||||||
|
|
||||||
|
|
@ -565,11 +544,6 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable
|
||||||
{
|
{
|
||||||
foreach (var subs in _gpuByGfxObj.Values)
|
foreach (var subs in _gpuByGfxObj.Values)
|
||||||
{
|
{
|
||||||
// WB-managed entries use the sentinel — no GL resources to free here;
|
|
||||||
// ObjectMeshManager owns those resources.
|
|
||||||
if (object.ReferenceEquals(subs, WbManagedSentinel))
|
|
||||||
continue;
|
|
||||||
|
|
||||||
foreach (var sub in subs)
|
foreach (var sub in subs)
|
||||||
{
|
{
|
||||||
_gl.DeleteBuffer(sub.Vbo);
|
_gl.DeleteBuffer(sub.Vbo);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue