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 4a91219..7989f1d 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 @@ -259,7 +259,9 @@ Co-Authored-By: Claude Opus 4.7 (1M context) --- -## Task 3: Add bindless handle cache + Bindless GetOrUpload methods +## Task 3: Add bindless GetOrUpload methods with parallel Texture2DArray cache + +**AMENDED 2026-05-08:** the original Task 3 had Bindless* methods calling the legacy Texture2D `GetOrUpload*` then converting the GL name to a bindless handle. That produces a `sampler2D` texture sampled via `sampler2DArray` in the shader — a GLSL type mismatch. Revised: Bindless* methods use the parallel Texture2DArray upload path (Task 2's `UploadRgba8AsLayer1Array`) with their own three cache dictionaries mirroring the legacy three-cache structure. **Files:** - Modify: `src/AcDream.App/Rendering/TextureCache.cs` @@ -267,26 +269,28 @@ Co-Authored-By: Claude Opus 4.7 (1M context) - [ ] **Step 3.1: Read TextureCache constructor + cache fields** -Read `src/AcDream.App/Rendering/TextureCache.cs:1-50`. Note the existing dictionaries: `_handlesBySurfaceId`, `_handlesByOverridden`, `_handlesByPalette`. +Read `src/AcDream.App/Rendering/TextureCache.cs:1-50`. Note the existing dictionaries: `_handlesBySurfaceId`, `_handlesByOverridden`, `_handlesByPalette` — these stay untouched, serving the legacy Texture2D path. -- [ ] **Step 3.2: Add BindlessSupport dependency to TextureCache constructor** +- [ ] **Step 3.2: Add BindlessSupport dependency + three parallel cache dicts** -In `src/AcDream.App/Rendering/TextureCache.cs`, change the constructor from: - -```csharp -public TextureCache(GL gl, DatCollection dats) -{ - _gl = gl; - _dats = dats; -} -``` - -to: +Add these fields to `TextureCache`, near the existing legacy cache dicts: ```csharp private readonly Wb.BindlessSupport? _bindless; -private readonly Dictionary _bindlessHandlesByGlName = new(); +// Bindless / Texture2DArray parallel caches. Keys mirror the legacy three +// caches so a surface used by both the legacy (Texture2D, sampler2D) and +// modern (Texture2DArray, sampler2DArray) paths is uploaded twice — once +// per target. Each entry stores both the GL texture name (for Dispose +// cleanup) and the resident bindless handle (returned to callers). +private readonly Dictionary _bindlessBySurfaceId = new(); +private readonly Dictionary<(uint surfaceId, uint origTexOverride), (uint Name, ulong Handle)> _bindlessByOverridden = new(); +private readonly Dictionary<(uint surfaceId, uint origTexOverride, ulong paletteHash), (uint Name, ulong Handle)> _bindlessByPalette = new(); +``` + +Change the constructor signature: + +```csharp public TextureCache(GL gl, DatCollection dats, Wb.BindlessSupport? bindless = null) { _gl = gl; @@ -295,7 +299,7 @@ public TextureCache(GL gl, DatCollection dats, Wb.BindlessSupport? bindless = nu } ``` -The optional parameter keeps backward compatibility with consumers that don't need bindless (sky, terrain, etc.). +The optional `bindless` parameter keeps backward compatibility — legacy `GetOrUpload*` keeps working without it. The Bindless* methods throw if `bindless` is null. - [ ] **Step 3.3: Update TextureCache constructor sites** @@ -305,55 +309,79 @@ Identified call site: `src/AcDream.App/Rendering/GameWindow.cs` (typically aroun Modify `GameWindow.cs` to pass the `BindlessSupport` instance — but only after Task 6 wires it up. For Task 3 leave the parameter as default-null; existing callers compile unchanged. -- [ ] **Step 3.4: Add MakeResidentHandle helper + three Bindless GetOrUpload methods** +- [ ] **Step 3.4: Add three Bindless GetOrUpload methods** Add to `src/AcDream.App/Rendering/TextureCache.cs` immediately after the existing `GetOrUploadWithPaletteOverride` overloads: ```csharp /// -/// 64-bit bindless handle variant of . +/// 64-bit bindless handle variant of for the WB +/// modern rendering path. Uploads the texture as a 1-layer Texture2DArray +/// (so the shader's sampler2DArray can sample at layer 0) and returns +/// a resident bindless handle. Caches by surfaceId in a separate dictionary +/// from the legacy Texture2D path; the same surface may be uploaded twice +/// if used by both paths (acceptable transition cost — N.6 deletes the legacy +/// path). /// Throws if BindlessSupport wasn't provided to the constructor. /// public ulong GetOrUploadBindless(uint surfaceId) { - uint name = GetOrUpload(surfaceId); - return MakeResidentHandle(name); + EnsureBindlessAvailable(); + if (_bindlessBySurfaceId.TryGetValue(surfaceId, out var entry)) + return entry.Handle; + var decoded = DecodeFromDats(surfaceId, origTextureOverride: null, paletteOverride: null); + uint name = UploadRgba8AsLayer1Array(decoded); + ulong handle = _bindless!.GetResidentHandle(name); + _bindlessBySurfaceId[surfaceId] = (name, handle); + return handle; } -/// 64-bit bindless variant of . +/// 64-bit bindless variant of . +/// Uses the parallel Texture2DArray upload path. public ulong GetOrUploadWithOrigTextureOverrideBindless(uint surfaceId, uint overrideOrigTextureId) { - uint name = GetOrUploadWithOrigTextureOverride(surfaceId, overrideOrigTextureId); - return MakeResidentHandle(name); + EnsureBindlessAvailable(); + var key = (surfaceId, overrideOrigTextureId); + if (_bindlessByOverridden.TryGetValue(key, out var entry)) + return entry.Handle; + var decoded = DecodeFromDats(surfaceId, origTextureOverride: overrideOrigTextureId, paletteOverride: null); + uint name = UploadRgba8AsLayer1Array(decoded); + ulong handle = _bindless!.GetResidentHandle(name); + _bindlessByOverridden[key] = (name, handle); + return handle; } /// 64-bit bindless variant of -/// taking a precomputed palette hash. +/// taking a precomputed palette hash. Uses the parallel Texture2DArray upload path. public ulong GetOrUploadWithPaletteOverrideBindless( uint surfaceId, uint? overrideOrigTextureId, PaletteOverride paletteOverride, ulong precomputedPaletteHash) { - uint name = GetOrUploadWithPaletteOverride(surfaceId, overrideOrigTextureId, paletteOverride, precomputedPaletteHash); - return MakeResidentHandle(name); + EnsureBindlessAvailable(); + uint origTexKey = overrideOrigTextureId ?? 0; + var key = (surfaceId, origTexKey, precomputedPaletteHash); + if (_bindlessByPalette.TryGetValue(key, out var entry)) + return entry.Handle; + var decoded = DecodeFromDats(surfaceId, origTextureOverride: overrideOrigTextureId, paletteOverride: paletteOverride); + uint name = UploadRgba8AsLayer1Array(decoded); + ulong handle = _bindless!.GetResidentHandle(name); + _bindlessByPalette[key] = (name, handle); + return handle; } -private ulong MakeResidentHandle(uint glTextureName) +private void EnsureBindlessAvailable() { - if (glTextureName == 0) return 0; if (_bindless is null) throw new InvalidOperationException( "TextureCache constructed without BindlessSupport — cannot generate bindless handles. " + - "WbDrawDispatcher requires the bindless ctor overload."); - if (_bindlessHandlesByGlName.TryGetValue(glTextureName, out var h)) - return h; - h = _bindless.GetResidentHandle(glTextureName); - _bindlessHandlesByGlName[glTextureName] = h; - return h; + "WbDrawDispatcher requires the bindless-aware ctor overload (pass non-null BindlessSupport)."); } ``` +Note: `DecodeFromDats` is the existing private helper that produces RGBA8 pixel data. It's target-agnostic — same decoded pixels go to either Texture2D (legacy) or Texture2DArray (bindless) upload. No duplication of the decode pipeline. + - [ ] **Step 3.5: Write the failing tests** Create `tests/AcDream.Core.Tests/Rendering/TextureCacheBindlessTests.cs`: @@ -436,14 +464,30 @@ Replace the existing `Dispose` in `src/AcDream.App/Rendering/TextureCache.cs` (c public void Dispose() { // Release bindless handles BEFORE deleting underlying textures. - // glDeleteTextures of a texture with resident handles is undefined behavior. + // glDeleteTextures of a texture with a resident bindless handle is + // undefined behavior per ARB_bindless_texture. if (_bindless is not null) { - foreach (var h in _bindlessHandlesByGlName.Values) - _bindless.MakeNonResident(h); + foreach (var (name, handle) in _bindlessBySurfaceId.Values) + _bindless.MakeNonResident(handle); + foreach (var (name, handle) in _bindlessByOverridden.Values) + _bindless.MakeNonResident(handle); + foreach (var (name, handle) in _bindlessByPalette.Values) + _bindless.MakeNonResident(handle); } - _bindlessHandlesByGlName.Clear(); + // Then delete the array textures backing those handles. + foreach (var (name, _) in _bindlessBySurfaceId.Values) + _gl.DeleteTexture(name); + _bindlessBySurfaceId.Clear(); + foreach (var (name, _) in _bindlessByOverridden.Values) + _gl.DeleteTexture(name); + _bindlessByOverridden.Clear(); + foreach (var (name, _) in _bindlessByPalette.Values) + _gl.DeleteTexture(name); + _bindlessByPalette.Clear(); + + // Legacy Texture2D textures. foreach (var h in _handlesBySurfaceId.Values) _gl.DeleteTexture(h); _handlesBySurfaceId.Clear();