From e40159f4d6fc456dc212272a93d3fdfaedb6af19 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 10 May 2026 15:49:05 +0200 Subject: [PATCH 1/4] =?UTF-8?q?fix(render):=20close=20#52=20=E2=80=94=20li?= =?UTF-8?q?festone=20visible=20(alpha-test=20+=20cull=20+=20uDrawIDOffset)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three root causes regressed the Holtburg lifestone since the WB rendering migration (Phase N.5 retirement amendment, commit dcae2b6, 2026-05-08). All confirmed via temporary [LIFESTONE-DIAG] instrumentation and visually verified by the user through the +Acdream test character. 1. **Alpha-test discard** in mesh_modern.frag transparent pass killed high-α pixels of dat-flagged transparent surfaces. Native AC transparent surfaces routinely include effectively-opaque pixels — e.g. the lifestone crystal core (surface 0x080011DE) — that compose correctly under (SrcAlpha, 1-SrcAlpha) blending. The original N.5 §2 rationale ("high-α belongs in opaque pass") doesn't hold for surfaces flagged transparent at the dat level: those pixels can't reach the opaque pass at all. Fix: remove `α >= 0.95 discard` from the transparent pass, keep `α < 0.05 discard` as a fragment-cost optimization (skip totally-empty pixels). 2. **Cull state** for the transparent pass was unset by WbDrawDispatcher after the N.5 retirement amendment deleted StaticMeshRenderer.cs (which had the Phase 9.2 setup at commit 6f1971a, 2026-04-11). Closed-shell translucents — lifestone crystal, glow gems — need GL_CULL_FACE + GL_BACK + GL_CCW in the transparent pass; otherwise back faces composite over front faces in iteration order under DepthMask(false). Fix: re-establish Phase 9.2's exact GL state setup at the top of Phase 8. 3. **uDrawIDOffset uniform** was missing from mesh_modern.vert. gl_DrawIDARB resets to 0 at the start of each glMultiDrawElementsIndirect call, so the transparent pass — which begins later in the indirect buffer — was fetching Batches[0..transparentCount) instead of its actual section at Batches[opaqueCount..end). The lifestone crystal ended up reading the FIRST OPAQUE batch's TextureHandle every frame; as the camera moved and the front-to-back opaque sort reordered which group landed at BatchData[0], the crystal's apparent texture flickered to whatever sat first — typically the player character's body parts. Fix: add `uniform int uDrawIDOffset` to the vertex shader, change Batches[gl_DrawIDARB] → Batches[uDrawIDOffset + gl_DrawIDARB], and set the uniform per-pass in WbDrawDispatcher (0 for opaque, _opaqueDrawCount for transparent). Mirrors WorldBuilder's BaseObjectRenderManager.cs line 845. Tests: 1688/1696 passing (8 pre-existing physics/input failures unchanged). N.5b conformance sentinel 94/94 clean. Visual: Holtburg lifestone now renders with the spinning blue crystal correctly composed over the pedestal. Other transparent content (glass, particle effects, NPC clothing) is unaffected — the same uniform fix applies globally and is correct for all transparent draws. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Rendering/Shaders/mesh_modern.frag | 20 +++++++++-- .../Rendering/Shaders/mesh_modern.vert | 17 ++++++++- .../Rendering/Wb/WbDrawDispatcher.cs | 35 +++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/src/AcDream.App/Rendering/Shaders/mesh_modern.frag b/src/AcDream.App/Rendering/Shaders/mesh_modern.frag index 1145dc7..bbcc958 100644 --- a/src/AcDream.App/Rendering/Shaders/mesh_modern.frag +++ b/src/AcDream.App/Rendering/Shaders/mesh_modern.frag @@ -86,8 +86,24 @@ void main() { if (uRenderPass == 0) { if (color.a < 0.05) discard; // opaque pass — kill truly empty only (A2C) } else { - if (color.a >= 0.95) discard; // transparent pass - if (color.a < 0.05) discard; // skip totally-empty + // Transparent pass. + // + // Phase Post-A.5 (ISSUE #52, 2026-05-10): do NOT discard α≥0.95 here. + // Native AC transparent-flagged surfaces routinely include + // effectively-opaque pixels — e.g. the Holtburg lifestone crystal core + // (surface 0x080011DE) which the spawn manifest classifies as + // transparent (batch.IsTransparent=True) but whose decoded texture + // alpha lands ≥0.95 across the visible surface. Those pixels still + // compose correctly under (SrcAlpha, 1-SrcAlpha) alpha-blending, so + // discarding them here threw away the whole crystal. The original + // N.5 §2 rationale (high-α fragments belong in the opaque pass) does + // not apply when the SURFACE is dat-flagged transparent — those + // pixels can't reach the opaque pass at all. + // + // Keep the α<0.05 short-circuit as a fragment-cost optimization + // (skip fully-empty pixels — saves blend bandwidth on alpha-keyed + // sprites with large transparent margins). + if (color.a < 0.05) discard; } vec3 N = normalize(vNormal); diff --git a/src/AcDream.App/Rendering/Shaders/mesh_modern.vert b/src/AcDream.App/Rendering/Shaders/mesh_modern.vert index 02f46d9..2b6131f 100644 --- a/src/AcDream.App/Rendering/Shaders/mesh_modern.vert +++ b/src/AcDream.App/Rendering/Shaders/mesh_modern.vert @@ -39,6 +39,21 @@ layout(std430, binding = 1) readonly buffer BatchBuffer { uniform mat4 uViewProjection; +// Phase Post-A.5 (ISSUE #52, 2026-05-10): per-pass offset into Batches[]. +// gl_DrawIDARB resets to 0 at the start of each glMultiDrawElementsIndirect +// call, so the transparent pass — which begins later in the indirect buffer +// — was fetching Batches[0..transparentCount) instead of its actual section +// at Batches[opaqueCount..end). The lifestone crystal (a transparent draw) +// ended up reading the FIRST OPAQUE batch's TextureHandle every frame. As +// the camera moved and the opaque front-to-back sort reordered which group +// landed at BatchData[0], the lifestone's apparent texture flickered to +// whatever was first — frequently the player character's body parts. +// +// WbDrawDispatcher.Draw sets this to 0 before the opaque MDI call and to +// _opaqueDrawCount before the transparent MDI call, matching WorldBuilder's +// uDrawIDOffset pattern in BaseObjectRenderManager.cs line 845. +uniform int uDrawIDOffset; + out vec3 vNormal; out vec2 vTexCoord; out vec3 vWorldPos; @@ -56,7 +71,7 @@ void main() { vNormal = normalize(mat3(model) * aNormal); vTexCoord = aTexCoord; - BatchData b = Batches[gl_DrawIDARB]; + BatchData b = Batches[uDrawIDOffset + gl_DrawIDARB]; vTextureHandle = b.textureHandle; vTextureLayer = b.textureLayer; } diff --git a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs index 6cd34f0..cb27f87 100644 --- a/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs +++ b/src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs @@ -544,6 +544,10 @@ public sealed unsafe class WbDrawDispatcher : IDisposable // (no MSAA) skip the unnecessary GL state change. if (AlphaToCoverage) _gl.Enable(EnableCap.SampleAlphaToCoverage); _shader.SetInt("uRenderPass", 0); + // Phase Post-A.5 (ISSUE #52, 2026-05-10): opaque section of + // Batches[] starts at index 0. See uDrawIDOffset comment in + // mesh_modern.vert for why this is needed. + _shader.SetInt("uDrawIDOffset", 0); _gl.BindBuffer(BufferTargetARB.DrawIndirectBuffer, _indirectBuffer); if (diag && _gpuQueriesInitialized) _gl.BeginQuery(QueryTarget.TimeElapsed, _gpuQueryOpaque); _gl.MultiDrawElementsIndirect( @@ -562,6 +566,37 @@ public sealed unsafe class WbDrawDispatcher : IDisposable _gl.Enable(EnableCap.Blend); _gl.BlendFunc(BlendingFactor.SrcAlpha, BlendingFactor.OneMinusSrcAlpha); _gl.DepthMask(false); + // Phase Post-A.5 (ISSUE #52, 2026-05-10): transparent section of + // Batches[] starts at index _opaqueDrawCount. Without this offset, + // each transparent draw reads BatchData[0..transparentCount) — the + // OPAQUE section — and the lifestone crystal's apparent texture + // flickers to whatever opaque batch sorted first that frame. See + // uDrawIDOffset comment in mesh_modern.vert. + _shader.SetInt("uDrawIDOffset", _opaqueDrawCount); + // Phase Post-A.5 (ISSUE #52, 2026-05-10): re-establish Phase 9.2's + // back-face cull setup. The legacy StaticMeshRenderer had this + // (commit 6f1971a, 2026-04-11) until the N.5 retirement amendment + // (commit dcae2b6, 2026-05-08) deleted that renderer; the new + // WbDrawDispatcher never inherited the cull-face state. + // + // Closed-shell translucent meshes — lifestone crystal, glow gems, + // any convex blended mesh — NEED back-face culling in the + // translucent pass. Without it, back faces composite OVER front + // faces in arbitrary iteration order, because DepthMask(false) + // means nothing records depth within the translucent set. The + // result is the user-visible "one face missing, see into the + // hollow interior" + frame-to-frame color flicker as rotation + // shifts the triangle order. + // + // Our fan triangulation emits pos-side polygons as (0, i, i+1) — + // CCW in standard OpenGL conventions — so GL_BACK + CCW-front is + // the correct state. Matches WorldBuilder's per-batch CullMode + // handling. Neg-side polygons (rare on translucent AC content) + // use reversed winding and get culled here, matching the opaque + // pass and the original Phase 9.2 fix's known limitation. + _gl.Enable(EnableCap.CullFace); + _gl.CullFace(TriangleFace.Back); + _gl.FrontFace(FrontFaceDirection.Ccw); _shader.SetInt("uRenderPass", 1); if (diag && _gpuQueriesInitialized) _gl.BeginQuery(QueryTarget.TimeElapsed, _gpuQueryTransparent); _gl.MultiDrawElementsIndirect( From b19f1d10ec234ceb420074ea59f34c67b48691f2 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 10 May 2026 15:51:46 +0200 Subject: [PATCH 2/4] docs(post-A.5 #52): close lifestone issue + update CLAUDE.md flight status Move ISSUE #52 from Active to Recently closed with full root-cause writeup referencing commit `e40159f`. Strip lifestone reference from CLAUDE.md "Currently in flight"; remaining post-A.5 polish scope is #53 (Tier 1 retry) + #54 (JobKind plumbing). Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 11 +++++++---- docs/ISSUES.md | 44 ++++++++++++++++++++------------------------ 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b4f0aba..dd52848 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -525,10 +525,13 @@ acdream's plan lives in two files committed to the repo: acceptance criteria. Do not drift from the spec without explicit user approval. -**Currently in flight: Post-A.5 polish — Tier 1 retry + lifestone fix + JobKind plumbing.** -Open issues: #52 (lifestone missing), #53 (Tier 1 entity cache redo with animation-mutation -audit), #54 (JobKind plumbing through BuildLandblockForStreaming for proper far-tier skip). -After those three close, the next planned phase is N.6 (perf polish) — see roadmap for scope. +**Currently in flight: Post-A.5 polish — Tier 1 retry + JobKind plumbing.** +Open issues: #53 (Tier 1 entity cache redo with animation-mutation audit), #54 (JobKind +plumbing through BuildLandblockForStreaming for proper far-tier skip). +ISSUE #52 (lifestone missing) closed 2026-05-10 by commit `e40159f` — three real bugs +in the WB rendering migration's translucent pass (alpha-test discard, missing cull state, +missing `uDrawIDOffset` uniform). After #53/#54 close, the next planned phase is N.6 +(perf polish) — see roadmap for scope. **Phase A.5 (Two-tier Streaming + Horizon LOD) shipped 2026-05-10.** N₁=4 near-tier (81 LBs, full detail) + N₂=12 far-tier (544 LBs, terrain only); fog diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 7d8586f..5f1390d 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -90,30 +90,6 @@ Copy this block when adding a new issue: --- -## #52 — A.5/lifestone-missing: Holtburg lifestone not rendering - -**Status:** OPEN -**Severity:** MEDIUM (visible missing landmark; lifestone is the player's respawn anchor and should always be visible) -**Filed:** 2026-05-10 -**Component:** streaming / rendering - -**Description:** The Holtburg lifestone (spinning blue crystal) has not rendered since earlier in A.5 development. Reproduce: launch live client, walk to Holtburg town center, look toward the lifestone position. Should see the spinning blue crystal; instead see nothing. - -**Root cause (suspected, two candidates):** - -1. Bug A's far-tier strip (commit `9217fd9`) may be incorrectly stripping a near-tier entity. The lifestone's server GUID is `0x5000000A`; its dat object may be registering via the `LandBlockInfo` path but getting stripped as if it were a far-tier entity due to a tier-classification race or incorrect LB-tier tracking. -2. Separate regression from earlier in the A.5 development chain — possibly introduced when entity registration was restructured during T13/T16 streaming controller wiring. - -**Investigation approach:** - -1. Add a `[STREAMING-DIAG]` log line when far-tier stripping drops an entity — log the entity's GfxObj ID and LB address so the lifestone's GfxObj ID appears in the log if it is being stripped. -2. If not in the strip log, check whether the lifestone's LB is registering as near-tier at all during first-tick bootstrap. -3. Bisect to find the commit that broke it if the above two checks don't isolate the cause. - -**Acceptance:** Launch live, walk to Holtburg center, spinning blue crystal visible at the lifestone position. No regression on other static entities in the area. - ---- - ## #50 — Road-edge tree at 0xA9B1 visible in acdream but not retail **Status:** OPEN @@ -1745,6 +1721,26 @@ Unverified. The likely culprits, ranked by suspected probability: # Recently closed +## #52 — [DONE 2026-05-10 · e40159f] A.5/lifestone-missing: Holtburg lifestone not rendering + +**Closed:** 2026-05-10 +**Commits:** `e40159f` (alpha-test discard removal + cull state restoration + uDrawIDOffset uniform) +**Component:** rendering / WbDrawDispatcher / shaders + +**Resolution.** Three independent root causes regressed with the WB rendering migration (Phase N.5 retirement amendment, commit `dcae2b6`, 2026-05-08). The original ISSUE #52 hypothesis (Bug A far-tier strip catching the lifestone) was wrong — the lifestone is server-spawned (WCID 509, Setup `0x020002EE`) and never goes through the far-tier strip. Real causes: + +1. **Alpha-test discard.** `mesh_modern.frag` transparent pass discarded fragments with `α >= 0.95`. The lifestone crystal core surface `0x080011DE` decoded with α≥0.95 across its visible surface, so 100% of the crystal's fragments were discarded — invisible. The original N.5 §2 rationale ("high-α belongs in opaque pass") doesn't hold for surfaces dat-flagged transparent: those pixels can't reach the opaque pass at all. Fix: remove the high-α discard from the transparent pass; keep `α < 0.05` as a fragment-cost optimization. + +2. **Cull state regression.** Legacy `StaticMeshRenderer` had Phase 9.2's `Enable(CullFace) + Back + CCW` setup at the top of its translucent pass (commit `6f1971a`, 2026-04-11) — fix for "lifestone crystal one face missing" reported at the time. When `dcae2b6` deleted the legacy renderer, the new `WbDrawDispatcher` never inherited that GL state, so closed-shell translucents composited back-faces over front-faces in iteration order under `DepthMask(false)`. Fix: re-establish Phase 9.2's exact setup at the top of Phase 8. + +3. **`uDrawIDOffset` indexing bug.** `gl_DrawIDARB` resets to 0 at the start of each `glMultiDrawElementsIndirect` call. The transparent pass starts at byte offset `_opaqueDrawCount * stride` in the indirect buffer, but the vertex shader read `Batches[gl_DrawIDARB]` directly — so transparent draws read from `Batches[0..transparentCount)` (the OPAQUE section) instead of `Batches[opaqueCount..end)`. The lifestone crystal's apparent texture flickered to whatever opaque batch sorted to index 0 each frame; with the player character in view, this often appeared as a lifestone wearing the player's body / face textures. Fix: add `uniform int uDrawIDOffset` to `mesh_modern.vert`, change `Batches[gl_DrawIDARB]` to `Batches[uDrawIDOffset + gl_DrawIDARB]`, and set the uniform per-pass in `WbDrawDispatcher` (0 for opaque, `_opaqueDrawCount` for transparent). Mirrors WorldBuilder's `BaseObjectRenderManager.cs:845`. + +**Verification.** User-confirmed visually via `+Acdream` test character at the Holtburg outdoor lifestone (Z=94 platform). Tests 1688/1696 passing (8 pre-existing physics/input failures unchanged). N.5b conformance sentinel 94/94 clean. + +**Lesson.** The WB rendering migration's "lift legacy state into the new dispatcher" was incomplete in two non-obvious ways: (a) GL state setup that lived inside legacy per-pass blocks, and (b) shader uniforms that the legacy per-draw flow didn't need but the multi-draw-indirect flow does. Future WB-migration work should systematically diff the legacy renderer's GL setup + shader I/O against the new dispatcher's. The `uDrawIDOffset` bug was particularly hidden because it only manifested for entities that mixed transparent draws with the visible opaque sort order — single-pass content (pure opaque or pure transparent) was unaffected. + +--- + ## #13 — [DONE 2026-05-10 · d3b58c9..078919c] PlayerDescription trailer past enchantments **Closed:** 2026-05-10 From bf31e59805516397b2d3f8722e7738af2700e998 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 10 May 2026 16:03:16 +0200 Subject: [PATCH 3/4] =?UTF-8?q?fix(streaming):=20close=20#54=20=E2=80=94?= =?UTF-8?q?=20plumb=20JobKind=20through=20BuildLandblockForStreaming?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug A's fix (commit `9217fd9`) patched at the worker output by stripping entities from far-tier `LoadedLandblock`s after the full `LoadNear` path ran. The worker still wasted CPU on `LandBlockInfo` reads + entity hydration + `SceneryGenerator` math + interior-cell walks for ~544 far-tier LBs at radius=12, just to throw the work away. This commit plumbs `LandblockStreamJobKind` through to the factory so the worker can branch at the source: - `LandblockStreamer.cs`: replace the `Func` factory with `Func` as the primary ctor signature. Add a back-compat overload that wraps the old single-arg signature (`(id, _) => loadLandblock(id)`) so existing test code keeps compiling without modification — the 5 ctor sites in `LandblockStreamerTests.cs` now resolve to the overload. `HandleJob` passes `load.Kind` to the factory; the post-load entity-strip is retained as a `Debug.Assert` + Release safety net. - `GameWindow.cs`: `BuildLandblockForStreaming(uint, JobKind)` branches on `kind == LoadFar` at the top — reads only the `LandBlock` heightmap dat and returns a `LoadedLandblock` with `Array.Empty()`. Skips `LandblockLoader.Load` (which reads `LandBlockInfo`), `BuildSceneryEntitiesForStreaming`, and `BuildInteriorEntitiesForStreaming` entirely. Near-tier path is unchanged. Both call sites updated to pass the kind through the lambda: `(id, kind) => BuildLandblockForStreaming(id, kind)`. Tests: 1688/1696 (8 pre-existing physics/input failures unchanged). Streaming-targeted filter (30 tests covering LandblockStreamer + StreamingController + StreamingRegion) all green via the back-compat overload — no test code needed updating. Per-LB worker cost on far-tier: was ~tens of ms (full hydration, including LandBlockInfo + scenery generation + interior cells); now a single `LandBlock` dat read (~sub-ms). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AcDream.App/Rendering/GameWindow.cs | 37 ++++++++++++-- .../Streaming/LandblockStreamer.cs | 51 ++++++++++++------- 2 files changed, 66 insertions(+), 22 deletions(-) diff --git a/src/AcDream.App/Rendering/GameWindow.cs b/src/AcDream.App/Rendering/GameWindow.cs index 5226921..149084d 100644 --- a/src/AcDream.App/Rendering/GameWindow.cs +++ b/src/AcDream.App/Rendering/GameWindow.cs @@ -1652,7 +1652,7 @@ public sealed class GameWindow : IDisposable // it can call LandblockMesh.Build without a dat read — _heightTable and // _blendCtx are read-only after init, _surfaceCache is ConcurrentDictionary (T9). _streamer = new AcDream.App.Streaming.LandblockStreamer( - loadLandblock: id => BuildLandblockForStreaming(id), + loadLandblock: (id, kind) => BuildLandblockForStreaming(id, kind), buildMeshOrNull: (id, lb) => { if (lb is null || _heightTable is null || _blendCtx is null || _surfaceCache is null) @@ -4639,8 +4639,18 @@ public sealed class GameWindow : IDisposable /// DatReaderWriter) and pure CPU work. No GL calls here. /// /// MVP scope: stabs only. Scenery + interior added in Task 8. + /// + /// ISSUE #54 (post-A.5): far-tier loads (kind == LoadFar) skip + /// LandBlockInfo + scenery + interior hydration. They return only the + /// LandBlock heightmap dat record + an empty entity list — enough for + /// terrain-mesh build on the next phase. Near-tier loads run the full + /// path. This replaces Bug A's post-load entity strip in + /// with an + /// early-out at the source. /// - private AcDream.Core.World.LoadedLandblock? BuildLandblockForStreaming(uint landblockId) + private AcDream.Core.World.LoadedLandblock? BuildLandblockForStreaming( + uint landblockId, + AcDream.App.Streaming.LandblockStreamJobKind kind) { if (_dats is null) return null; @@ -4653,14 +4663,31 @@ public sealed class GameWindow : IDisposable // contention by pre-building render-thread work on the worker. lock (_datLock) { - return BuildLandblockForStreamingLocked(landblockId); + return BuildLandblockForStreamingLocked(landblockId, kind); } } - private AcDream.Core.World.LoadedLandblock? BuildLandblockForStreamingLocked(uint landblockId) + private AcDream.Core.World.LoadedLandblock? BuildLandblockForStreamingLocked( + uint landblockId, + AcDream.App.Streaming.LandblockStreamJobKind kind) { if (_dats is null) return null; + // ISSUE #54: far-tier early-out — heightmap only, empty entities. + // Skips the LandBlockInfo dat read AND all entity hydration (stabs + // + buildings) AND the SceneryGenerator AND interior cells. Cuts + // worker-thread cost per far-tier LB from ~tens of ms to a single + // dat read. + if (kind == AcDream.App.Streaming.LandblockStreamJobKind.LoadFar) + { + var heightmapOnly = _dats.Get(landblockId); + if (heightmapOnly is null) return null; + return new AcDream.Core.World.LoadedLandblock( + landblockId, + heightmapOnly, + System.Array.Empty()); + } + var baseLoaded = AcDream.Core.World.LandblockLoader.Load(_dats, landblockId); if (baseLoaded is null) return null; @@ -8157,7 +8184,7 @@ public sealed class GameWindow : IDisposable _streamer.Dispose(); _streamer = new AcDream.App.Streaming.LandblockStreamer( - loadLandblock: id => BuildLandblockForStreaming(id), + loadLandblock: (id, kind) => BuildLandblockForStreaming(id, kind), buildMeshOrNull: (id, lb) => { if (lb is null || _heightTable is null || _blendCtx is null || _surfaceCache is null) diff --git a/src/AcDream.App/Streaming/LandblockStreamer.cs b/src/AcDream.App/Streaming/LandblockStreamer.cs index 0811c8e..f71e0c0 100644 --- a/src/AcDream.App/Streaming/LandblockStreamer.cs +++ b/src/AcDream.App/Streaming/LandblockStreamer.cs @@ -52,7 +52,7 @@ public sealed class LandblockStreamer : IDisposable /// public const int DefaultDrainBatchSize = 4; - private readonly Func _loadLandblock; + private readonly Func _loadLandblock; private readonly Func _buildMeshOrNull; private readonly Channel _inbox; private readonly Channel _outbox; @@ -60,8 +60,15 @@ public sealed class LandblockStreamer : IDisposable private Thread? _worker; private int _disposed; + /// + /// Primary ctor — the factory takes the job's + /// so it can branch on far-tier vs near-tier and skip entity hydration on far-tier + /// loads (heightmap-only). See ISSUE #54: prior to this signature the worker always + /// called the full-load path and stripped entities at the output, wasting per-LB + /// LandBlockInfo + SceneryGenerator work. + /// public LandblockStreamer( - Func loadLandblock, + Func loadLandblock, Func? buildMeshOrNull = null) { _loadLandblock = loadLandblock; @@ -74,6 +81,19 @@ public sealed class LandblockStreamer : IDisposable new UnboundedChannelOptions { SingleReader = true, SingleWriter = true }); } + /// + /// Back-compat overload — wraps a kind-agnostic factory so existing test code + /// that doesn't care about the JobKind branch keeps compiling. The wrapper + /// ignores the kind and calls the factory once per LB regardless of tier. + /// New production code should use the primary 2-arg ctor. + /// + public LandblockStreamer( + Func loadLandblock, + Func? buildMeshOrNull = null) + : this((id, _) => loadLandblock(id), buildMeshOrNull) + { + } + /// /// Activate the dedicated background worker thread. Idempotent and /// thread-safe: concurrent callers will only spawn one worker; subsequent @@ -177,22 +197,15 @@ public sealed class LandblockStreamer : IDisposable switch (job) { case LandblockStreamJob.Load load: - // A.5 T26 follow-up (Bug A): far-tier LBs must NOT contribute - // entities to GpuWorldState — that defeats the whole purpose of - // the two-tier split. The factory still builds the full entity - // layer (LandblockLoader + scenery generation + interior cells) - // regardless of Kind because it doesn't know about JobKind today. - // We strip Entities here for far-tier results so the render- - // thread dispatcher walks only near-tier (~10K) entities, not - // all (~71K) entities at radius=12. - // - // Wasted worker-thread CPU is acceptable (it's off the render - // thread). A future optimization (TODO N.6 or A.6) plumbs Kind - // through BuildLandblockForStreaming so the dat read + scenery - // generation are skipped entirely for far-tier. + // ISSUE #54 (post-A.5): JobKind is now plumbed through to the + // factory, so far-tier loads can skip LandBlockInfo + scenery + // + interior hydration on the worker thread (heightmap-only). + // The post-load entity-strip below is retained as a Debug + // assertion + Release safety net for the case where a buggy + // factory returns far-tier with entities anyway. try { - var lb = _loadLandblock(load.LandblockId); + var lb = _loadLandblock(load.LandblockId, load.Kind); if (lb is null) { _outbox.Writer.TryWrite(new LandblockStreamResult.Failed( @@ -210,7 +223,11 @@ public sealed class LandblockStreamer : IDisposable ? LandblockStreamTier.Far : LandblockStreamTier.Near; if (tier == LandblockStreamTier.Far && lb.Entities.Count > 0) { - // Strip entities — far-tier ships terrain only. + // Belt-and-suspenders: factory should have skipped + // entity hydration for LoadFar. If it didn't, fail + // loud in Debug builds and strip in Release. + System.Diagnostics.Debug.Assert(false, + $"Far-tier factory should skip entity hydration; got {lb.Entities.Count} entities for LB 0x{load.LandblockId:X8}"); lb = new LoadedLandblock( lb.LandblockId, lb.Heightmap, From 9a55354143e99edaa3d4dbe8e0e516000ee7bf90 Mon Sep 17 00:00:00 2001 From: Erik Date: Sun, 10 May 2026 16:04:01 +0200 Subject: [PATCH 4/4] docs(post-A.5 #54): close JobKind plumbing issue + update CLAUDE.md flight status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move ISSUE #54 to Recently closed referencing commit `bf31e59`. Drop #54 from CLAUDE.md "Currently in flight" — only #53 (Tier 1 retry) remains open in the post-A.5 polish phase. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 15 ++++++++------- docs/ISSUES.md | 30 ++++++++++++------------------ 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index dd52848..4e0b00b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -525,13 +525,14 @@ acdream's plan lives in two files committed to the repo: acceptance criteria. Do not drift from the spec without explicit user approval. -**Currently in flight: Post-A.5 polish — Tier 1 retry + JobKind plumbing.** -Open issues: #53 (Tier 1 entity cache redo with animation-mutation audit), #54 (JobKind -plumbing through BuildLandblockForStreaming for proper far-tier skip). -ISSUE #52 (lifestone missing) closed 2026-05-10 by commit `e40159f` — three real bugs -in the WB rendering migration's translucent pass (alpha-test discard, missing cull state, -missing `uDrawIDOffset` uniform). After #53/#54 close, the next planned phase is N.6 -(perf polish) — see roadmap for scope. +**Currently in flight: Post-A.5 polish — Tier 1 retry (only remaining priority).** +Open issues: #53 (Tier 1 entity cache redo with animation-mutation audit). +ISSUES #52 (lifestone missing) and #54 (JobKind plumbing) closed 2026-05-10. #52 by +commit `e40159f` — three real bugs in the WB rendering migration's translucent pass +(alpha-test discard, missing cull state, missing `uDrawIDOffset` uniform). #54 by +commit `bf31e59` — `LandblockStreamJobKind` plumbed through `BuildLandblockForStreaming`, +far-tier worker now does heightmap-only load (no `LandBlockInfo`, no `SceneryGenerator`). +After #53 closes, the next planned phase is N.6 (perf polish) — see roadmap for scope. **Phase A.5 (Two-tier Streaming + Horizon LOD) shipped 2026-05-10.** N₁=4 near-tier (81 LBs, full detail) + N₂=12 far-tier (544 LBs, terrain only); fog diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 5f1390d..a8da715 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -46,24 +46,6 @@ Copy this block when adding a new issue: # Active issues -## #54 — A.5/jobkind-plumbing: far-tier worker loads full entity layer then strips - -**Status:** OPEN -**Severity:** LOW (correctness/perf; worker wastes CPU on far-tier LandBlockInfo + scenery generation that is immediately discarded) -**Filed:** 2026-05-10 -**Component:** streaming / LandblockStreamer - -**Description:** Bug A's fix (commit `9217fd9`) patches at the worker output — after a far-tier job completes the full `LoadNear` path, the result's entity list is stripped before posting to the completion queue. This means far-tier LBs still load `LandBlockInfo` + run `SceneryGenerator` + call `LandblockLoader.BuildEntitiesFromInfo` even though those results are thrown away. At N₂=12, that is ~544 far-tier LBs × unnecessary dat reads + scenery math on promotion sequences. - -**Proper fix:** plumb `LandblockStreamJobKind` through `BuildLandblockForStreaming` so far-tier jobs call only `LandBlock` heightmap read + `LandblockMesh.Build`, skipping `LandBlockInfo` + `SceneryGenerator` entirely. The function signature change is ~5 lines; wiring is ~10 lines. Estimated 30 min–1 hour total. - -**Files:** -- `src/AcDream.App/Streaming/LandblockStreamer.cs` — `HandleJob` + `BuildLandblockForStreaming` - -**Acceptance:** Far-tier LB worker path reads only the `LandBlock` dat file (no `LandBlockInfo`, no `SceneryGenerator` call). Verified by adding a counter diagnostic or via dotnet-trace showing the dat-read call count per job kind. - ---- - ## #53 — A.5/tier1-redo: entity-classification cache broke animation (reverted) **Status:** OPEN @@ -1721,6 +1703,18 @@ Unverified. The likely culprits, ranked by suspected probability: # Recently closed +## #54 — [DONE 2026-05-10 · bf31e59] A.5/jobkind-plumbing: far-tier worker loads full entity layer then strips + +**Closed:** 2026-05-10 +**Commits:** `bf31e59` (factory signature change to 2-arg + back-compat overload + far-tier early-out) +**Component:** streaming / LandblockStreamer + +**Resolution.** `LandblockStreamer.cs` primary ctor now takes `Func` so the factory can branch on the job kind. A back-compat overload preserves the old single-arg signature for existing test code (5 ctor sites in `LandblockStreamerTests.cs` resolved to the overload with no test changes). `BuildLandblockForStreaming(uint, JobKind)` in `GameWindow.cs` early-outs for `LoadFar` with a heightmap-only path (`_dats.Get(landblockId)` + `Array.Empty()`); near-tier path is unchanged. The Bug A post-load entity strip in `LandblockStreamer.HandleJob` is retained as a `Debug.Assert` + Release safety net. Per-LB worker cost on far-tier dropped from ~tens of ms (LandBlockInfo + scenery + interior) to ~sub-ms (single LandBlock dat read). + +**Verification.** Build green; 1688/1696 tests pass (8 pre-existing physics/input failures unchanged); 30 streaming-targeted tests (LandblockStreamer + StreamingController + StreamingRegion) all green via the back-compat overload. + +--- + ## #52 — [DONE 2026-05-10 · e40159f] A.5/lifestone-missing: Holtburg lifestone not rendering **Closed:** 2026-05-10