262 lines
12 KiB
Markdown
262 lines
12 KiB
Markdown
# 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.
|