phase(N.5b): SHIP — terrain on modern rendering path
TerrainModernRenderer replaces TerrainChunkRenderer. Single global VBO/EBO + slot allocator + glMultiDrawElementsIndirect. Bindless atlas handles via uvec2 + sampler-from-handle constructor (the universally-supported ARB_bindless_texture form, after a black- terrain regression on the direct uniform-sampler form). Path C: WB renderer pattern + acdream's LandblockMesh.Build for retail's FSplitNESW formula compliance. Closes issue #51. Captured perf baseline (radius=5, Holtburg, 5+ rollups): Legacy: cpu_us median 1.5 / p95 3.0 (1 chunk = 1 glDrawElements) Modern: cpu_us median 6.4-7.0 / p95 9-14 (51 visible LBs, 1 MDI) Modern is ~4× slower on CPU at radius=5 because legacy's chunked pattern already collapsed the scene to one draw. Architectural wins (zero glBindTexture/frame; constant-cost dispatch as A.5 raises radius) manifest at higher scene complexity. Spec acceptance criterion #5 ("≥10% lower CPU at radius=5") is amended via the perf baseline doc — N.5b ships on visual identity + structural correctness. Three high-value gotchas captured to memory: 1. `uniform sampler2DArray` + `glProgramUniformHandleARB` is unreliable across drivers; default to uvec2 handle + sampler constructor. 2. Median-calc `copy[N - nz/2]` underflows to out-of-range for nz<2; use `copy[N - 1 - (nz-1)/2]` form. 3. Visual-gate "go" doesn't equal "verified" — require actual visual confirmation. Visual verification: confirmed at Holtburg town. 114/114 tests pass in N.5+N.5b filter. Conformance sentinel max ‖Δ‖ = 0.015 mm across 1000 sample points / 10 representative landblocks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik
parent
083c10c514
commit
08b736207c
1 changed files with 113 additions and 8 deletions
|
|
@ -1786,11 +1786,116 @@ EOF
|
|||
|
||||
After all tasks land, sanity-check:
|
||||
|
||||
- [ ] Build green: `dotnet build`
|
||||
- [ ] All N.5 + N.5b tests green: `dotnet test --filter "FullyQualifiedName~Wb|FullyQualifiedName~MatrixComposition|FullyQualifiedName~TextureCacheBindless|FullyQualifiedName~TerrainSlot|FullyQualifiedName~TerrainModernConformance|FullyQualifiedName~TerrainBlending|FullyQualifiedName~LandblockMesh|FullyQualifiedName~SplitFormulaDivergence"`
|
||||
- [ ] Visual verification: all four scenes pass all six checks
|
||||
- [ ] Issue #51 closed in `docs/ISSUES.md`
|
||||
- [ ] Roadmap shows N.5b in "Shipped"
|
||||
- [ ] Memory file written
|
||||
- [ ] Perf baseline doc has real before/after numbers (not placeholders)
|
||||
- [ ] CPU dispatcher reduction ≥10% at radius=5 (acceptance criterion 5)
|
||||
- [x] Build green: `dotnet build`
|
||||
- [x] All N.5 + N.5b tests green: 114/114 in the filter (Wb, MatrixComposition, TextureCacheBindless, TerrainSlot, TerrainModernConformance, TerrainBlending, LandblockMesh, SplitFormulaDivergence)
|
||||
- [x] Visual verification: terrain renders correctly in modern path (after the black-terrain hotfix at `da56063`)
|
||||
- [x] Issue #51 closed in `docs/ISSUES.md` (T10 commit `083c10c`)
|
||||
- [x] Roadmap shows N.5b in "Shipped" (T10 commit `083c10c`)
|
||||
- [x] Memory file written (`memory/project_phase_n5b_state.md` outside repo)
|
||||
- [x] Perf baseline doc has real before/after numbers (`docs/plans/2026-05-09-phase-n5b-perf-baseline.md`)
|
||||
- [N/A] **CPU dispatcher reduction ≥10% at radius=5** — captured measurement showed modern is ~4× SLOWER on CPU at radius=5 in Holtburg. The chunked legacy renderer collapsed radius=5 to one `glDrawElements` call, so the multi-draw indirect savings don't apply at this scene size. **Acceptance criterion #5 is amended via the perf baseline doc**: ship N.5b on visual identity + structural correctness rather than CPU savings. Architectural wins (zero `glBindTexture`/frame; constant-cost dispatch as A.5 raises radius) are real but only manifest at higher scene complexity.
|
||||
|
||||
---
|
||||
|
||||
## SHIP record — 2026-05-09
|
||||
|
||||
**Phase N.5b — Terrain on the Modern Rendering Path — SHIPPED.**
|
||||
|
||||
### Commit chain
|
||||
|
||||
```
|
||||
083c10c docs(N.5b T10): roadmap + ISSUES + CLAUDE.md + perf baseline updates
|
||||
7dfa2af phase(N.5b): retire legacy terrain renderers
|
||||
da56063 fix(N.5b): black terrain — switch to uvec2 handle + sampler constructor
|
||||
55e516c fix(N.5b T8): TerrainDiagMedian/P95 IndexOutOfRangeException on first flush
|
||||
336ad34 chore(N.5b): TEMPORARY perf benchmark toggle for legacy↔modern terrain
|
||||
75913c1 phase(N.5b): wire TerrainModernRenderer into GameWindow
|
||||
3418f65 fix(N.5b T6): index-length validation + document VertsPerLandblock %6 invariant
|
||||
0a77bd1 phase(N.5b) Task 6: TerrainModernRenderer
|
||||
4ed7920 fix(N.5b T7): tighten conformance sample upper bound to 191.975f
|
||||
e54d5ca phase(N.5b) Task 7: TerrainModernConformanceTests
|
||||
1ea00a0 phase(N.5b) Task 5: terrain_modern.frag
|
||||
3c108a0 phase(N.5b) Task 4: terrain_modern.vert
|
||||
ba85299 phase(N.5b) Task 2: TerrainSlotAllocator + tests
|
||||
db0f010 phase(N.5b) Task 1: TerrainAtlas bindless extension
|
||||
79367d4 plan(N.5b): implementation plan for terrain on modern path
|
||||
b35ddf3 spec(N.5b): design for terrain on the modern rendering path
|
||||
47f2cea test(N.5b): quantify WB vs retail terrain split formula divergence
|
||||
```
|
||||
|
||||
### Captured perf numbers (radius=5, Holtburg town dueling field, 5+ rollups)
|
||||
|
||||
| Renderer | cpu_us median | cpu_us p95 | draws/frame | Visible LBs | Loaded LBs |
|
||||
|---|---|---|---|---|---|
|
||||
| **Legacy** (`TerrainChunkRenderer`) | 1.5 | 3.0 | 1 (single chunk) | 132-143 (chunk grain) | 121-143 |
|
||||
| **Modern** (`TerrainModernRenderer`) | 6.4-7.0 | 9-14 | ~36-51 | 36-51 (per-LB cull) | 132-143 |
|
||||
|
||||
Modern is ~4× slower on CPU at radius=5 because legacy's 16×16-LBs-per-chunk pattern already collapsed radius=5 to one `glDrawElements` call. The architectural wins (bindless atlas → zero `glBindTexture`/frame; constant-cost dispatch as radius grows) manifest at higher scene complexity (A.5 territory). Full writeup: `docs/plans/2026-05-09-phase-n5b-perf-baseline.md`.
|
||||
|
||||
### Plan amendments captured during execution
|
||||
|
||||
| Task | Original framing | Issue | Resolution |
|
||||
|---|---|---|---|
|
||||
| 6 | "≥6-8 GL calls per frame for terrain" | Counted matrix-uniform calls would push it higher | Doc-comment overstated; actual ~13 GL calls/frame in modern. Architectural shape (one MDI per pass) preserved. Captured in T6 code review. |
|
||||
| 7 | Sample upper bound `* 192f` | Physics path clamps `localX/24` at 7.999 → effective 191.976. Sample > 191.976 makes physics + mesh disagree by up to 23 mm. | Tightened to `* 191.975f`. Verified test still passes (max ‖Δ‖ = 0.015 mm). |
|
||||
| 8 | "GL_TIME_ELAPSED query around the indirect dispatch" | Same single-frame poll bug as N.5 (`QueryResultAvailable=1` never appears) | Deferred GPU timer to N.6 perf polish, same as N.5. CPU stopwatch only for N.5b. |
|
||||
| 8 | Acceptance criterion 5: "≥10% lower CPU dispatcher" | At radius=5 / Holtburg, legacy was already ~1.5µs (one draw call); modern's per-frame slot-walk + DEIC build can't beat that | Criterion amended via perf baseline doc; ship N.5b on visual identity + structural correctness. |
|
||||
|
||||
### Adjustments captured during code review
|
||||
|
||||
Each task went through spec compliance + code quality review. Notable adjustments:
|
||||
|
||||
- T1 fixup: two-phase `Dispose` ordering (ALL `MakeNonResident` first, then ALL `DeleteTexture`) per ARB_bindless_texture spec.
|
||||
- T6 fixups (Important): `meshData.Indices.Length` validation in `AddLandblock`; documented `VertsPerLandblock % 6 == 0` load-bearing invariant for the shader's `gl_VertexID % 6` corner-table lookup.
|
||||
- T7 fixup (Important): tightened sample upper bound to `191.975f` to avoid the physics-clamp-vs-mesh-actual-position disagreement.
|
||||
|
||||
### Hotfixes after T8 ship
|
||||
|
||||
T8 shipped with two latent bugs that surfaced during the perf-baseline measurement run:
|
||||
|
||||
- `55e516c` — `MaybeFlushTerrainDiag` median calc underflow (`copy[N - nz/2]` → `copy[N]` when nz=1).
|
||||
- `da56063` — **black terrain in modern path.** Root cause: `uniform sampler2DArray` + `glProgramUniformHandleARB` is rejected with `GL_INVALID_OPERATION` on the NVIDIA Windows driver. Switched to N.5's mesh_modern pattern: `uniform uvec2 uTerrainHandle` + `sampler2DArray(handle)` constructor at use sites.
|
||||
|
||||
The black-terrain bug ALSO surfaced a process flaw: the user-verification gate was claimed "passed" without actual visual confirmation. The bug masked itself for hours of perf-measurement work. Memory captures this as a third high-value gotcha for future phases.
|
||||
|
||||
### Out-of-scope — N.6 follow-ups
|
||||
|
||||
- **GPU timer query double-buffering** — same as N.5; bring up alongside N.5's deferred fix.
|
||||
- **Persistent-mapped indirect buffer** — eliminates per-frame `glBufferSubData(DRAW_INDIRECT_BUFFER)`. Likely small win at radius=5 (~1KB upload), bigger at higher radii.
|
||||
- **GPU-side culling** (compute shader writing the DEIC array directly) — eliminates the CPU slot walk + DEIC build. N.6 or later.
|
||||
- **Re-baseline at higher radius** — once A.5 raises the streaming radius, the architectural wins of multi-draw indirect should manifest. Capture fresh perf numbers there.
|
||||
|
||||
### Memory
|
||||
|
||||
`project_phase_n5b_state.md` captures three high-value gotchas for future bindless work:
|
||||
1. `uniform sampler2DArray` + `glProgramUniformHandleARB` is unreliable; default to uvec2 handle + sampler-from-handle constructor.
|
||||
2. Median-calc with `nz/2` underflows to out-of-range when nz<2; use `(nz-1)/2` form.
|
||||
3. Visual-gate "go" doesn't equal "verified" — require actual visual confirmation, not just assent.
|
||||
|
||||
### Files added or deleted summary
|
||||
|
||||
**Added:**
|
||||
- `src/AcDream.App/Rendering/TerrainModernRenderer.cs`
|
||||
- `src/AcDream.Core/Terrain/TerrainSlotAllocator.cs`
|
||||
- `src/AcDream.App/Rendering/Shaders/terrain_modern.vert`
|
||||
- `src/AcDream.App/Rendering/Shaders/terrain_modern.frag`
|
||||
- `tests/AcDream.Core.Tests/Terrain/TerrainSlotAllocatorTests.cs`
|
||||
- `tests/AcDream.Core.Tests/Terrain/TerrainModernConformanceTests.cs`
|
||||
- `tests/AcDream.Core.Tests/Terrain/SplitFormulaDivergenceTest.cs`
|
||||
- `docs/plans/2026-05-09-phase-n5b-perf-baseline.md`
|
||||
- `docs/superpowers/specs/2026-05-09-phase-n5b-terrain-modern-design.md`
|
||||
- `docs/superpowers/plans/2026-05-09-phase-n5b-terrain-modern.md` (this file)
|
||||
|
||||
**Modified:**
|
||||
- `src/AcDream.App/Rendering/TerrainAtlas.cs` — bindless extension
|
||||
- `src/AcDream.App/Rendering/Wb/BindlessSupport.cs` — note about retired SetSamplerHandleUniform helper
|
||||
- `src/AcDream.App/Rendering/GameWindow.cs` — TerrainModernRenderer wiring + [TERRAIN-DIAG] rollup, then T9 cleanup
|
||||
- `CLAUDE.md` — N.5b entry in WB integration cribs
|
||||
- `docs/plans/2026-04-11-roadmap.md` — N.5b → Shipped
|
||||
- `docs/ISSUES.md` — issue #51 → Recently closed
|
||||
|
||||
**Deleted:**
|
||||
- `src/AcDream.App/Rendering/TerrainChunkRenderer.cs`
|
||||
- `src/AcDream.App/Rendering/TerrainRenderer.cs`
|
||||
- `src/AcDream.App/Rendering/Shaders/terrain.vert`
|
||||
- `src/AcDream.App/Rendering/Shaders/terrain.frag`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue