From aba2cfc3b68bac86db61924c957e0962ee9768b9 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 8 May 2026 19:42:18 +0200 Subject: [PATCH] =?UTF-8?q?docs(N.5):=20plan=20amendment=20=E2=80=94=20Tas?= =?UTF-8?q?k=202=20uses=20parallel=20upload=20path,=20not=20replace?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implementer caught that the original Task 2 (replace UploadRgba8 target with Texture2DArray) would break four legacy consumers whose shaders sample via sampler2D: WbDrawDispatcher (pre-rewrite path), StaticMeshRenderer, InstancedMeshRenderer (legacy escape hatch), ParticleRenderer. Revised: Task 2 ADDS a parallel UploadRgba8AsLayer1Array. Existing UploadRgba8 (Texture2D) stays for legacy callers. Task 3's Bindless* methods will call the new array path with their own cache dictionaries. Same surface may be uploaded twice during transition; bounded cost. N.6 cleanup deletes the legacy path. Task 3 will be amended at dispatch time to reflect parallel caches. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-08-phase-n5-modern-rendering.md | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/docs/superpowers/plans/2026-05-08-phase-n5-modern-rendering.md b/docs/superpowers/plans/2026-05-08-phase-n5-modern-rendering.md index 05caf50..4a91219 100644 --- a/docs/superpowers/plans/2026-05-08-phase-n5-modern-rendering.md +++ b/docs/superpowers/plans/2026-05-08-phase-n5-modern-rendering.md @@ -176,23 +176,32 @@ Co-Authored-By: Claude Opus 4.7 (1M context) --- -## Task 2: Switch TextureCache uploads to Texture2DArray (depth=1) +## Task 2: Add parallel Texture2DArray upload path to TextureCache **Files:** - Modify: `src/AcDream.App/Rendering/TextureCache.cs` -This task is structurally a no-op for callers — `GetOrUpload` still returns `uint`. Internally we change the GL target from `Texture2D` to `Texture2DArray`. Sky / terrain / debug consumers continue using their own `glBindTexture(Texture2D, ...)` patterns; we only change the WB-modern-path consumers later. **Wait — that creates a binding-target mismatch.** The same texture object can't be bound to both `Texture2D` and `Texture2DArray` targets. This task therefore only switches the upload target; we then audit consumers in Step 2.4 below to confirm none of them do a raw `glBindTexture(Texture2D, returnedName)`. +**AMENDED 2026-05-08** after first-pass implementation surfaced a flaw. Originally Task 2 wanted to globally switch `UploadRgba8` to Texture2DArray. Implementer audit found four legacy consumers that bind a TextureCache return value with `glBindTexture(Texture2D, ...)`: `WbDrawDispatcher.cs:363` (rewritten in Task 10 — but breaks meanwhile), `StaticMeshRenderer.cs:126,223`, `InstancedMeshRenderer.cs:282,361` (legacy escape hatch — must keep working under foundation flag-off), and `ParticleRenderer.cs:162`. A texture has ONE GL target — can't be both Texture2D and Texture2DArray. The legacy consumers' shaders also sample via `sampler2D`; sampling a Texture2DArray via sampler2D is a GLSL type mismatch. + +**Revised approach:** ADD a parallel `UploadRgba8AsLayer1Array` method. Don't touch the existing `UploadRgba8`. Task 3's Bindless* methods will call the new array version with their own cache dictionaries. Legacy callers stay on the Texture2D path, untouched. WB modern dispatcher (Task 10) uses the array path. + +Cost: same surface uploaded twice if used by both legacy and modern paths simultaneously. In practice the overlap is small, and N.6 deletes the legacy path entirely. Acceptable transition cost. - [ ] **Step 2.1: Read existing UploadRgba8 in TextureCache.cs** Read `src/AcDream.App/Rendering/TextureCache.cs:256-280`. Confirm it uses `TextureTarget.Texture2D` + `TexImage2D`. -- [ ] **Step 2.2: Replace UploadRgba8 with Texture2DArray version** +- [ ] **Step 2.2: ADD UploadRgba8AsLayer1Array method (do NOT replace UploadRgba8)** -Replace the `UploadRgba8` method body in `src/AcDream.App/Rendering/TextureCache.cs` with: +ADD this NEW method to `src/AcDream.App/Rendering/TextureCache.cs` immediately after the existing `UploadRgba8` (which stays untouched): ```csharp -private uint UploadRgba8(DecodedTexture decoded) +/// +/// Variant of that uploads pixel data as a 1-layer +/// Texture2DArray. Required by the WB modern rendering path which samples via +/// sampler2DArray in its bindless shader. Pixel data is identical. +/// +private uint UploadRgba8AsLayer1Array(DecodedTexture decoded) { uint tex = _gl.GenTexture(); _gl.BindTexture(TextureTarget.Texture2DArray, tex); @@ -220,31 +229,30 @@ private uint UploadRgba8(DecodedTexture decoded) } ``` -- [ ] **Step 2.3: Audit consumers for stale Texture2D bindings** - -Run: `Grep` for `BindTexture\(.*Texture2D[^A]` in `src/AcDream.App/Rendering` (excluding `Texture2DArray`). - -Expected: only `SkyRenderer.cs`, `TerrainAtlas.cs`, `DebugLineRenderer.cs`, `TextRenderer.cs`, `ParticleRenderer.cs` should appear. NONE of these should bind a `TextureCache.GetOrUpload*`-returned name (they own their own GL textures). - -If any consumer DOES bind a `TextureCache` return value with `Texture2D`: that consumer needs migration to `Texture2DArray` with layer 0 sampling. Note for follow-up; for N.5 the WB-modern dispatcher is the only intended consumer of the new format. - -- [ ] **Step 2.4: Build + run all tests** +- [ ] **Step 2.3: Build + run tests** Run: `dotnet build` -Expected: PASS. +Expected: PASS. The new method is unused at this point, but that's fine — Task 3 wires the bindless variants to call it. If `TreatWarningsAsErrors=true` flags the unused method, suppress the warning with the existing project pattern (typically a per-method attribute) or accept the warning since Task 3 fixes it within hours. Run: `dotnet test --filter "FullyQualifiedName~TextureCache"` -Expected: existing tests PASS (TextureCache tests don't bind in shaders). +Expected: existing tests PASS (no behavior change for legacy callers). -- [ ] **Step 2.5: Commit** +- [ ] **Step 2.4: Commit** ``` -phase(N.5) Task 2: TextureCache uploads as 1-layer Texture2DArray +phase(N.5) Task 2: parallel Texture2DArray upload path in TextureCache -Switches UploadRgba8 from glTexImage2D → glTexImage3D with depth=1 so -every TextureCache upload is a single-layer texture array. Required for -Task 5's mesh_modern.frag which samples via sampler2DArray. Pixel data -is identical — only target + bookkeeping changes. +Adds UploadRgba8AsLayer1Array — uploads pixel data as a 1-layer +Texture2DArray. Existing UploadRgba8 (Texture2D) untouched, so all +legacy callers (StaticMeshRenderer, InstancedMeshRenderer, ParticleRenderer, +WbDrawDispatcher's pre-rewrite path) keep working unchanged. + +Required for Task 3's Bindless* methods which need the Texture2DArray +target so the WB modern shader can sample via sampler2DArray. Same +surface may be uploaded both ways during the N.5/N.6 transition; +doubling is bounded and acceptable. After N.6 retires legacy +renderers entirely, the legacy UploadRgba8 becomes unused and is +deleted. Co-Authored-By: Claude Opus 4.7 (1M context) ```