From 4f318bcbba813c175d38b758ee9af6b137f884f1 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 8 May 2026 13:48:30 +0200 Subject: [PATCH] fix(N.4) Adjustment 2: revert Task 9 renderer-level routing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...026-05-08-phase-n4-rendering-foundation.md | 41 +++++++++++++++ .../Rendering/InstancedMeshRenderer.cs | 50 +++++-------------- 2 files changed, 53 insertions(+), 38 deletions(-) diff --git a/docs/superpowers/plans/2026-05-08-phase-n4-rendering-foundation.md b/docs/superpowers/plans/2026-05-08-phase-n4-rendering-foundation.md index ba2337d..f31e820 100644 --- a/docs/superpowers/plans/2026-05-08-phase-n4-rendering-foundation.md +++ b/docs/superpowers/plans/2026-05-08-phase-n4-rendering-foundation.md @@ -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.** +--- + +### 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) **Files:** diff --git a/src/AcDream.App/Rendering/InstancedMeshRenderer.cs b/src/AcDream.App/Rendering/InstancedMeshRenderer.cs index 2ba5093..5b0c9eb 100644 --- a/src/AcDream.App/Rendering/InstancedMeshRenderer.cs +++ b/src/AcDream.App/Rendering/InstancedMeshRenderer.cs @@ -35,19 +35,16 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable private readonly TextureCache _textures; /// - /// Optional WB adapter. When non-null and , - /// hands the GfxObj ref to the WB pipeline instead of - /// uploading into our own VAO pool. The draw loop skips sentinel entries — Task 22's - /// WbDrawDispatcher will eventually draw them. + /// Optional WB adapter. Held but currently unused — Phase N.4 Adjustment 2 + /// (2026-05-08) reverted Task 9's renderer-level routing. Tier-routing decisions + /// (atlas vs per-instance) belong at the spawn-callback layer (Task 11 + /// 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. /// 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 WbManagedSentinel = new(0); - // One GPU bundle per unique GfxObj id. Each GfxObj can have multiple sub-meshes. private readonly Dictionary> _gpuByGfxObj = new(); @@ -100,17 +97,11 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable if (_gpuByGfxObj.ContainsKey(gfxObjId)) return; - // Phase N.4 Task 9: when the WB foundation flag is on and we have an - // adapter, hand this GfxObj to the WB pipeline instead of uploading our - // own VAO. The sentinel entry marks "this GfxObj lives in WB now" so the - // draw loop knows to skip it. Task 22's WbDrawDispatcher will draw them. - if (WbFoundationFlag.IsEnabled && _wbMeshAdapter is not null) - { - _wbMeshAdapter.IncrementRefCount(gfxObjId); - _gpuByGfxObj[gfxObjId] = WbManagedSentinel; - return; - } - + // Phase N.4 Adjustment 2 (2026-05-08): renderer is tier-blind. Tier-routing + // (atlas vs per-instance) lives at the spawn-callback layer (Tasks 11 + 17), + // not here. Smoke-test of the original Task 9 routing showed it caught + // characters / NPCs (server-spawned, per-instance tier) along with static + // scenery, because EnsureUploaded is called from both spawn paths. var list = new List(subMeshes.Count); foreach (var sm in subMeshes) list.Add(UploadSubMesh(sm)); @@ -245,11 +236,6 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable if (!_gpuByGfxObj.TryGetValue(key.GfxObjId, out var subMeshes)) 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; foreach (var sub in subMeshes) { @@ -325,10 +311,6 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable if (!_gpuByGfxObj.TryGetValue(key.GfxObjId, out var subMeshes)) continue; - // WB-managed GfxObjs — skip; Task 22 will draw them. - if (object.ReferenceEquals(subMeshes, WbManagedSentinel)) - continue; - bool hasTranslucentSubMesh = false; foreach (var sub in subMeshes) { @@ -458,9 +440,6 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable { if (!_gpuByGfxObj.TryGetValue(meshRef.GfxObjId, out var cachedMeshes)) continue; - // WB-managed GfxObjs don't go through our instance pipeline. - if (object.ReferenceEquals(cachedMeshes, WbManagedSentinel)) - continue; var model = meshRef.PartTransform * entityRoot; @@ -565,11 +544,6 @@ public sealed unsafe class InstancedMeshRenderer : IDisposable { 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) { _gl.DeleteBuffer(sub.Vbo);