phase(N.5): retirement amendment — InstancedMeshRenderer + StaticMeshRenderer + WbFoundationFlag deleted

Final cross-cutting review of N.5 found that Task 15's deletion of
mesh_instanced.vert/.frag left InstancedMeshRenderer orphaned —
ACDREAM_USE_WB_FOUNDATION=0 silently rendered terrain+sky only with
no entities. The SHIP commit's "[x] ACDREAM_USE_WB_FOUNDATION=0 still
works" claim was inaccurate.

Resolution: formal retirement of the legacy renderer path within N.5
instead of deferring to N.6.

Deleted:
- src/AcDream.App/Rendering/InstancedMeshRenderer.cs
- src/AcDream.App/Rendering/StaticMeshRenderer.cs
- src/AcDream.App/Rendering/Wb/WbFoundationFlag.cs

GameWindow simplified — capability detection is unconditional, missing
bindless throws NotSupportedException with a clear message at startup.
WbDrawDispatcher + mesh_modern shader load are mandatory after init.
No escape hatch.

GpuWorldState simplified — WbFoundationFlag.IsEnabled guards on
AddLandblock/RemoveLandblock removed; adapter calls are unconditional
when the adapter is non-null.

PendingSpawnIntegrationTests updated — WbFoundationFlag.ForTestsOnly_ForceEnable
static ctor removed (flag is gone; adapter calls are unconditional).

The ApplyLoadedTerrain physics-data loop was also simplified: the
EnsureUploaded sub-loop that fed InstancedMeshRenderer is gone;
_pendingCellMeshes is now explicitly cleared to prevent unbounded
accumulation (the worker thread still populates it, but WB handles
EnvCell geometry through its own pipeline).

Spec §2 Decision 5 + §10 Out-of-Scope updated. Plan ship-amendment
section added. Roadmap updated (N.5 ships with retirement; N.6 scope
narrowed to perf-only). CLAUDE.md "WB integration cribs" updated.
Perf baseline doc updated. WbDrawDispatcher class summary docstring
corrected to describe the as-shipped SSBO + multi-draw-indirect path.
ISSUES.md #51 updated (terrain not in N.5 scope; deferred to N.7).

Bindless support is now a hard requirement. Modern desktop GPUs
universally expose GL_ARB_bindless_texture + GL_ARB_shader_draw_parameters;
if a user hits the NotSupportedException, that's a real bug report
worth investigating, not a silent fallback.

Build: 0 errors, 0 warnings. Tests: 71/71 (Wb+MatrixComposition+TextureCacheBindless filter).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-08 22:01:36 +02:00
parent 55ecec683f
commit dcae2b6b94
13 changed files with 211 additions and 1140 deletions

View file

@ -2561,10 +2561,10 @@ SHIP commit at Task 19.
`FullyQualifiedName~Wb|FullyQualifiedName~MatrixComposition`.
Pre-existing 8 failures in physics/input/movement tests carry
forward unchanged from before N.5.
- [x] **`ACDREAM_USE_WB_FOUNDATION=0` still works** — Task 15 confirmed
InstancedMeshRenderer remains intact as the escape hatch; if
bindless is missing, `_meshShader` stays null + `_wbDrawDispatcher`
stays null, falling through to InstancedMeshRenderer naturally.
- [N/A] **`ACDREAM_USE_WB_FOUNDATION=0` still works** — escape hatch
formally retired in N.5 ship amendment (see section below).
`InstancedMeshRenderer`, `StaticMeshRenderer`, and `WbFoundationFlag`
deleted. Missing bindless throws `NotSupportedException` at startup.
### Plan amendments captured during execution
@ -2613,7 +2613,7 @@ adjustments captured beyond the plan:
- **Persistent-mapped buffers** (Decision 7 deferral). Layer on top of
the modern path if `glBufferData` shows up as a residual hot spot in
profiling.
- **Retire `InstancedMeshRenderer`** entirely — N.6 primary scope.
- ~~**Retire `InstancedMeshRenderer`** entirely — N.6 primary scope.~~ **Done in N.5 ship amendment.**
- **WB atlas adoption** for memory savings on shared content (trees,
walls, etc).
- **GPU-side culling** via compute pre-pass.
@ -2655,3 +2655,52 @@ CLAUDE.md "WB integration cribs" updated with N.5 patterns (Task 16).
**Deleted:**
- `src/AcDream.App/Rendering/Shaders/mesh_instanced.vert`
- `src/AcDream.App/Rendering/Shaders/mesh_instanced.frag`
---
## Ship amendment — 2026-05-08
### Problem discovered in cross-cutting review
Task 15's deletion of `mesh_instanced.vert/.frag` left `InstancedMeshRenderer`
orphaned. The `_staticMesh` construction was gated on `_meshShader is not null`,
and `_meshShader` was only assigned when bindless was present. So with
`ACDREAM_USE_WB_FOUNDATION=0`, the flag path produced `_meshShader=null`
`_staticMesh=null` → terrain+sky only with no entity rendering. The SHIP
commit's `[x] ACDREAM_USE_WB_FOUNDATION=0 still works` claim was inaccurate.
### Resolution
User authorized **Option B**: formal retirement of the legacy path in N.5
instead of restoring it. Reasons: bindless + WB foundation has been default-on
since N.4, escape hatch was never exercised in practice, N.6 was already
planning to retire it — we did it now instead.
**Files deleted:**
- `src/AcDream.App/Rendering/InstancedMeshRenderer.cs`
- `src/AcDream.App/Rendering/StaticMeshRenderer.cs`
- `src/AcDream.App/Rendering/Wb/WbFoundationFlag.cs`
**GameWindow simplified:**
- `_staticMesh` field removed
- Capability detection block is unconditional (no `WbFoundationFlag.IsEnabled` guard)
- Missing bindless throws `NotSupportedException` at startup with a clear message
- `_wbMeshAdapter`, `_wbEntitySpawnAdapter`, `_wbDrawDispatcher` all construct
unconditionally after the capability check
- Draw path: `_wbDrawDispatcher!.Draw(...)` — no null-conditional, no else branch
**GpuWorldState simplified:**
- `WbFoundationFlag.IsEnabled` guards removed from `AddLandblock` /
`RemoveLandblock`; adapter calls are unconditional when adapter is non-null
**Test file updated:**
- `PendingSpawnIntegrationTests.cs`: removed `static WbFoundationFlag.ForTestsOnly_ForceEnable()` ctor
(no longer needed — `GpuWorldState` adapter calls are unconditional)
**Spec §2 Decision 5 updated:** two-way flag → mandatory modern path.
**Spec §10 Out-of-scope updated:** `InstancedMeshRenderer` deletion crossed off (done).
**Roadmap updated:** N.5 entry notes retirement; N.6 scope narrowed.
**Perf baseline doc updated:** acceptance gate row corrected to N/A.
**CLAUDE.md updated:** WB integration cribs no longer reference WbFoundationFlag.
Build: green (0 errors, 0 warnings). Tests: 71/71 in Wb+MatrixComposition+TextureCacheBindless filter.

View file

@ -40,7 +40,7 @@ This section records the brainstorm outcomes that the rest of the doc relies on.
| 2 | Translucent rendering | **WB's two-pass alpha-test** (opaque pass discards `α<0.95`, transparent pass discards `α≥0.95`) | Single blend mode per pass enables one indirect call per pass. Loses native `Additive` blend on GfxObj surfaces; sky + particles have own renderers and aren't affected. Falsifiable at visual verification — if we see a regression, add an additive sub-pass (~30-min fix). |
| 3 | Per-instance + per-draw data delivery | **All-SSBO**: `Instances[]` at binding=0 (mat4 per instance), `Batches[]` at binding=1 (texture handle + layer + flags per group) | Matches WB's modern shader. SSBOs avoid the 16-attrib stride limit, scale to large instance counts, give clean per-draw indexing via `gl_DrawIDARB`. |
| 4 | Bindless handle residency | **Resident on upload, never release** | acdream's content set is bounded (~1-5K unique textures per session). Handles persist for process lifetime; no eviction code in N.5. Diagnostic logging of handle count under `ACDREAM_WB_DIAG=1` to spot growth. |
| 5 | Escape hatch | **Two-way flag (no change)**. `ACDREAM_USE_WB_FOUNDATION=0/1` controls `WbFoundationFlag`; flag-on is the N.5 modern path; flag-off falls back to legacy `InstancedMeshRenderer`. N.4's draw method is replaced in place. | N.4's grouped-instanced draw is not preserved as an A/B fallback; legacy `InstancedMeshRenderer` is the existing safety net for "modern rendering broken on this GPU." |
| 5 | Escape hatch | **Modern path mandatory (N.5 ship amendment)**. `WbFoundationFlag` and `ACDREAM_USE_WB_FOUNDATION` env var have been deleted. Missing `GL_ARB_bindless_texture` or `GL_ARB_shader_draw_parameters` throws `NotSupportedException` at startup with a clear error message. No fallback. | Escape hatch was never exercised after N.4 ship. Legacy `InstancedMeshRenderer` + `StaticMeshRenderer` deleted in the N.5 retirement commit. N.6 scope narrowed accordingly. |
| 6 | Perf measurement | **CPU stopwatch + GL timer queries** logged via `[WB-DIAG]` | Captures both CPU dispatcher time and GPU rendering time. Acceptance gate compares before/after numbers in fixed Holtburg/Foundry scenes. |
| 7 | Persistent-mapped buffers | **Defer to N.6** | Bindless+indirect win is 70-80% of achievable savings. Persistent-mapped + ring + sync is the last 5-10% with non-trivial sync-fence complexity; not worth the risk in N.5's 2-3 week budget. Add post-N.5 if profiling shows residual `glBufferData` cost. |
| 8 | Per-instance highlight (selection blink) | **Defer to a Phase B.4 follow-up** | Retail pulses click targets as visual confirmation; the right mechanism is per-instance highlight color (NOT WB's global `uHighlightColor` which would tint everything in our single-indirect-call design). Field is reserved in design (extend `InstanceData` to include `vec4 highlightColor`); N.5 ships without the field, future phase plumbs it without shader rewrite. |
@ -540,7 +540,7 @@ The following are NOT N.5 work. They become possible follow-ons.
- **GPU-side culling (compute pre-pass).** Future phase.
- **Texture array repacking for multi-layer per-instance composites.** Future, if many palette-overrides actually share dimensions and could be packed.
- **Selection-blink highlight color.** Decision 8. Phase B.4 follow-up. Field reserved in `InstanceData` design (extend stride to 80 bytes when implementing).
- **Deletion of legacy `InstancedMeshRenderer`.** N.6.
- ~~**Deletion of legacy `InstancedMeshRenderer`.** N.6.~~ **Done in N.5 ship amendment**`InstancedMeshRenderer`, `StaticMeshRenderer`, and `WbFoundationFlag` were deleted in the retirement commit.
- **Terrain wiring through WB.** Future.
---