acdream/docs/superpowers/plans/2026-06-23-envcell-shell-batching.md
Erik fd9354f69e docs: implementation plan — EnvCell shell batching
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-23 15:49:18 +02:00

262 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# EnvCell Shell Batching Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Cut dense-town frame time by batching the per-cell `EnvCellRenderer.Render` calls (~94/frame, 24.75 ms) into one call per pass.
**Architecture:** `RetailPViewRenderer.DrawEnvCellShells` and `DrawBuildingLookIns` currently loop visible/look-in cells and call the heavy per-frame `EnvCellRenderer.Render(pass, oneCell)` once per cell × 2 passes. Replace with: opaque → one `Render(Opaque, allCells)` (z-buffer handles order; lighting is per-instance/`CellId`-keyed so cross-cell batching is safe); transparent → skip cells with no transparent geometry, keep far→near per-cell for the rest. Verified safe in the spec.
**Tech Stack:** C# / .NET 10, Silk.NET OpenGL, bindless multi-draw-indirect. Verification: `FrameProfiler` apparatus (`ACDREAM_FPS_PROF=1`) + live client + `dotnet build`/`dotnet test`.
**Spec:** `docs/superpowers/specs/2026-06-23-envcell-shell-batching-design.md`
**Verification model:** No unit tests — the draw path is heavy GL with no mockable seam, and render-perf is verified empirically (meter + visual), matching the project's render-change norm. Each task: edit → build → measure on the meter / visual → commit.
---
### Task 1: Add `CellHasTransparent` predicate to `EnvCellRenderer`
Phase 2 needs to skip the transparent `Render` for cells whose prepared snapshot has no transparent batch. Add a cheap read-only predicate mirroring the existing `[shell]` probe logic (`EnvCellRenderer.cs:1027-1034`).
**Files:**
- Modify: `src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs` (add a public method near `Render`)
- [ ] **Step 1: Add the predicate**
Add this method to `EnvCellRenderer` (e.g. just after the `Render` method, before `GetCellLightSet`):
```csharp
/// <summary>
/// True if the cell's prepared snapshot has any transparent render batch.
/// Used by the pview shell pass to skip the (heavy) transparent Render call
/// for opaque-only cells — most cell geometry is opaque walls/floors/ceilings.
/// Read-only; mirrors the [shell] probe's per-cell batch scan.
/// </summary>
public bool CellHasTransparent(uint cellId)
{
var snapshot = _activeSnapshot;
if (snapshot is null || !snapshot.BatchedByCell.TryGetValue(cellId, out var gfxDict))
return false;
foreach (var (gfxObjId, transforms) in gfxDict)
{
if (transforms.Count == 0) continue;
var rd = _meshManager.TryGetRenderData(gfxObjId);
if (rd is null) continue;
foreach (var b in rd.Batches)
if (b.IsTransparent) return true;
}
return false;
}
```
- [ ] **Step 2: Build**
Run: `dotnet build src\AcDream.App\AcDream.App.csproj -c Release`
Expected: `Build succeeded. 0 Error(s)`. (If `_activeSnapshot` / `BatchedByCell` / `_meshManager.TryGetRenderData` / `rd.Batches` / `b.IsTransparent` names differ, match them to the existing usages at `EnvCellRenderer.cs:910-934` and `:1031-1034` before building.)
- [ ] **Step 3: Commit**
```bash
git add src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs
git commit -m "feat(envcell): CellHasTransparent predicate (shell-batching prep)"
```
---
### Task 2: Batch the shell pass in `DrawEnvCellShells` (the Arwic win)
**Files:**
- Modify: `src/AcDream.App/Rendering/RetailPViewRenderer.cs` — the `_oneCell` field area + `DrawEnvCellShells` (currently `:659-665`)
- [ ] **Step 1: Add a reusable batch set field**
Find the `_oneCell` scratch field declaration (grep `_oneCell` in `RetailPViewRenderer.cs`) and add next to it:
```csharp
// Shell-batch scratch: all of a pass's cells collected for ONE Render call
// (opaque). Reused across frames + across look-in buildings. See
// docs/superpowers/specs/2026-06-23-envcell-shell-batching-design.md.
private readonly HashSet<uint> _shellBatch = new();
```
- [ ] **Step 2: Replace the per-cell loop body in `DrawEnvCellShells`**
Replace this (currently `RetailPViewRenderer.cs:658-665`):
```csharp
UseIndoorMembershipOnlyRouting();
foreach (var entry in IndoorDrawPlan.ShellPass(pvFrame))
{
_oneCell.Clear();
_oneCell.Add(entry.CellId);
_envCells.Render(WbRenderPass.Opaque, _oneCell);
_envCells.Render(WbRenderPass.Transparent, _oneCell);
}
```
with:
```csharp
UseIndoorMembershipOnlyRouting();
// Opaque: ONE batched Render for all shell cells. Opaque needs no draw
// order (z-buffer), and lighting is per-instance (CellId-keyed SSBO,
// EnvCellRenderer RenderModernMDIInternal), so cross-cell batching is
// visually identical to the old per-cell loop — at ~1 call instead of N.
_shellBatch.Clear();
foreach (var entry in IndoorDrawPlan.ShellPass(pvFrame))
_shellBatch.Add(entry.CellId);
if (_shellBatch.Count > 0)
_envCells.Render(WbRenderPass.Opaque, _shellBatch);
// Transparent: far→near order matters for compositing, so keep these
// per-cell in ShellPass order — but skip cells with no transparent
// geometry (most cells are opaque-only), which removes the bulk of the
// per-cell Render calls.
foreach (var entry in IndoorDrawPlan.ShellPass(pvFrame))
{
if (!_envCells.CellHasTransparent(entry.CellId)) continue;
_oneCell.Clear();
_oneCell.Add(entry.CellId);
_envCells.Render(WbRenderPass.Transparent, _oneCell);
}
```
- [ ] **Step 3: Build**
Run: `dotnet build src\AcDream.App\AcDream.App.csproj -c Release`
Expected: `Build succeeded. 0 Error(s)`.
- [ ] **Step 4: Measure on the meter (user-driven live run)**
Launch with `ACDREAM_FPS_PROF=1` (see Apparatus in the spec), portal to Arwic, hold the worst facing ~15 s. Read `[PASS-GPU] cells=… (calls/frame=…)`.
Expected: `calls/frame` for cells drops sharply (opaque 1 + transparent = only the few transparent cells); `cells` ms drops; frame ms drops. Visual: **no missing walls, no half-culled geometry, windows/transparency still correct.**
- [ ] **Step 5: Commit**
```bash
git add src/AcDream.App/Rendering/RetailPViewRenderer.cs
git commit -m "perf(pview): batch EnvCell shell opaque pass + skip empty transparent (dense-town FPS)"
```
---
### Task 3: Apply the same batching to `DrawBuildingLookIns` (interior roots)
The look-in path (interior root — standing inside a building looking out at others) has the identical per-cell pattern, plus a per-building aperture **punch** pass that must stay ordered before that building's shells. Batch each building's shells; keep punches per building.
**Files:**
- Modify: `src/AcDream.App/Rendering/RetailPViewRenderer.cs``DrawBuildingLookIns` pass-2 loop (currently `:348-355`)
- [ ] **Step 1: Replace the pass-2 per-cell shell loop**
Replace this (currently `RetailPViewRenderer.cs:346-355`, the "Pass 2: shells + statics, far→near." block's cell loop):
```csharp
// Pass 2: shells + statics, far→near.
UseIndoorMembershipOnlyRouting();
for (int i = frame.OrderedVisibleCells.Count - 1; i >= 0; i--)
{
uint cellId = frame.OrderedVisibleCells[i];
_oneCell.Clear();
_oneCell.Add(cellId);
_envCells.Render(WbRenderPass.Opaque, _oneCell);
_envCells.Render(WbRenderPass.Transparent, _oneCell);
```
with:
```csharp
// Pass 2: shells + statics. Opaque batched per building (this
// building's punches already ran above; z-buffer handles order).
UseIndoorMembershipOnlyRouting();
_shellBatch.Clear();
foreach (uint cellId in frame.OrderedVisibleCells)
_shellBatch.Add(cellId);
if (_shellBatch.Count > 0)
_envCells.Render(WbRenderPass.Opaque, _shellBatch);
// Transparent: far→near, skip opaque-only cells.
for (int i = frame.OrderedVisibleCells.Count - 1; i >= 0; i--)
{
uint cellId = frame.OrderedVisibleCells[i];
if (!_envCells.CellHasTransparent(cellId)) continue;
_oneCell.Clear();
_oneCell.Add(cellId);
_envCells.Render(WbRenderPass.Transparent, _oneCell);
```
> NOTE: preserve whatever follows the old `_envCells.Render(WbRenderPass.Transparent, _oneCell);` line inside the original loop body (e.g. per-cell static/entity draws) — read `RetailPViewRenderer.cs:355-...` and keep the remainder of the loop body intact, only the shell-Render calls move. If the loop body does more than the two Render calls, the transparent loop must retain that per-cell work; in that case keep the per-cell `for` loop structure and only lift the **opaque** Render out into the batched call before the loop, leaving opaque's per-cell call removed.
- [ ] **Step 2: Build**
Run: `dotnet build src\AcDream.App\AcDream.App.csproj -c Release`
Expected: `Build succeeded. 0 Error(s)`.
- [ ] **Step 3: Measure (interior-root scene)**
Launch with `ACDREAM_FPS_PROF=1`, stand INSIDE a building looking out toward other buildings (interior root). Read `[PASS-GPU] cells` calls/frame — should drop. Visual: look-in building interiors through your doorway still render correctly (no missing far-building walls, the #124 behavior intact).
- [ ] **Step 4: Commit**
```bash
git add src/AcDream.App/Rendering/RetailPViewRenderer.cs
git commit -m "perf(pview): batch EnvCell look-in shell draws (interior-root parity)"
```
---
### Task 4: Full verification
**Files:** none (verification only)
- [ ] **Step 1: Build + test green**
Run: `dotnet build` (solution) then `dotnet test tests\AcDream.App.Tests\AcDream.App.Tests.csproj -c Debug` and `dotnet test tests\AcDream.Core.Tests\AcDream.Core.Tests.csproj -c Debug`
Expected: build succeeds; test suites green (App / Core baselines).
- [ ] **Step 2: Before/after meter record**
Confirm at the same Arwic facing: cells calls/frame ~94 → low single digits, `cells` ms 24.75 → low single digits, frame ms 34 → target (~12 ms / ~80 fps). Record the numbers in the commit message / handoff.
- [ ] **Step 3: User visual gate**
User confirms at Arwic / Holtburg / Fort Tethana: dense-town fps up, no missing walls, transparency/windows correct, no flicker, interiors correct.
---
### Task 5: Strip the profiling apparatus
Once the fix is verified, remove all throwaway diagnostics (mirrors `92e95be`).
**Files:**
- Delete: `src/AcDream.App/Rendering/FrameProfiler.cs`
- Delete: `tests/AcDream.Core.Tests/Conformance/DegradeCoverageProbeTests.cs`
- Modify: `src/AcDream.App/Rendering/GameWindow.cs` — remove `_fpsProf` + `_frameProfiler` + `_msaaSamples` fields, the `_msaaSamples = startupQuality.MsaaSamples;` line, the `OnRender` BeginFrame/EndFrame hooks, the `OnUpdate` `MarkUpdateStart()` hook, and the terrain glFinish bracket in `DrawRetailPViewLandscapeSlice`.
- Modify: `src/AcDream.App/Rendering/Wb/EnvCellRenderer.cs` — remove the `Render` glFinish try/finally bracket (keep `CellHasTransparent` — it's part of the fix, not apparatus).
- [ ] **Step 1: Remove the apparatus** (edits above)
- [ ] **Step 2: Build + test green**
Run: `dotnet build` + the two `dotnet test` suites.
Expected: green, no references to `FrameProfiler` / `ACDREAM_FPS_PROF` remain (grep to confirm: `grep -rn "FrameProfiler\|ACDREAM_FPS_PROF\|MarkUpdateStart\|AddRendererGpu" src tests` → only `CellHasTransparent`-unrelated zero hits).
- [ ] **Step 3: Clean up throwaway logs**
Remove the untracked `fps-prof*.log`, `wbdiag.log`, `degrade-probe.log` from the worktree root.
- [ ] **Step 4: Commit**
```bash
git add -A
git commit -m "chore: strip dense-town FPS profiling apparatus (fix verified)"
```
---
## Notes for the implementer
- `_envCells` is the concrete `EnvCellRenderer` (field on `RetailPViewRenderer`). `Render(WbRenderPass, HashSet<uint>?)` already supports a multi-cell filter (filtered path, `EnvCellRenderer.cs:923-968`) — Task 2/3 just call it with the full set.
- Do NOT batch transparent across cells (loses far→near order → compositing artifacts). Skip-empty is the safe transparent win.
- Do NOT globally batch look-in shells across buildings — punches are per-building.
- If `DrawBuildingLookIns`'s loop body does per-cell work beyond the two Render calls, keep the per-cell `for` and only lift the opaque Render out (see Task 3 Step 1 note).
- Roadmap/issues: this is a tactical perf fix; update `docs/ISSUES.md` (note the dense-town FPS root cause + fix) when it lands.