docs(N.5): plan amendment — Task 3+4 use parallel bindless caches
Original Task 3 had Bindless* methods calling the legacy Texture2D GetOrUpload* and converting the GL name to a bindless handle — producing a sampler2D texture sampled via sampler2DArray (GLSL type mismatch). Revised: Task 3 introduces three parallel cache dictionaries (_bindlessBySurfaceId / _bindlessByOverridden / _bindlessByPalette) storing both the GL texture name and the resident handle. Bindless* methods call DecodeFromDats + UploadRgba8AsLayer1Array directly with their own caching; legacy three-cache structure mirrored exactly. Task 4 (Dispose) updated to: (1) MakeNonResident on every bindless handle FIRST, (2) DeleteTexture on every Texture2DArray name, (3) DeleteTexture on every legacy Texture2D handle. Order matters per ARB_bindless_texture spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
0b73875d39
commit
4b9a9bb721
1 changed files with 82 additions and 38 deletions
|
|
@ -259,7 +259,9 @@ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 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:**
|
**Files:**
|
||||||
- Modify: `src/AcDream.App/Rendering/TextureCache.cs`
|
- Modify: `src/AcDream.App/Rendering/TextureCache.cs`
|
||||||
|
|
@ -267,26 +269,28 @@ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||||||
|
|
||||||
- [ ] **Step 3.1: Read TextureCache constructor + cache fields**
|
- [ ] **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:
|
Add these fields to `TextureCache`, near the existing legacy cache dicts:
|
||||||
|
|
||||||
```csharp
|
|
||||||
public TextureCache(GL gl, DatCollection dats)
|
|
||||||
{
|
|
||||||
_gl = gl;
|
|
||||||
_dats = dats;
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
to:
|
|
||||||
|
|
||||||
```csharp
|
```csharp
|
||||||
private readonly Wb.BindlessSupport? _bindless;
|
private readonly Wb.BindlessSupport? _bindless;
|
||||||
private readonly Dictionary<uint, ulong> _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<uint, (uint Name, ulong Handle)> _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)
|
public TextureCache(GL gl, DatCollection dats, Wb.BindlessSupport? bindless = null)
|
||||||
{
|
{
|
||||||
_gl = gl;
|
_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**
|
- [ ] **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.
|
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:
|
Add to `src/AcDream.App/Rendering/TextureCache.cs` immediately after the existing `GetOrUploadWithPaletteOverride` overloads:
|
||||||
|
|
||||||
```csharp
|
```csharp
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// 64-bit bindless handle variant of <see cref="GetOrUpload"/>.
|
/// 64-bit bindless handle variant of <see cref="GetOrUpload"/> for the WB
|
||||||
|
/// modern rendering path. Uploads the texture as a 1-layer Texture2DArray
|
||||||
|
/// (so the shader's <c>sampler2DArray</c> 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.
|
/// Throws if BindlessSupport wasn't provided to the constructor.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public ulong GetOrUploadBindless(uint surfaceId)
|
public ulong GetOrUploadBindless(uint surfaceId)
|
||||||
{
|
{
|
||||||
uint name = GetOrUpload(surfaceId);
|
EnsureBindlessAvailable();
|
||||||
return MakeResidentHandle(name);
|
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;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>64-bit bindless variant of <see cref="GetOrUploadWithOrigTextureOverride"/>.</summary>
|
/// <summary>64-bit bindless variant of <see cref="GetOrUploadWithOrigTextureOverride"/>.
|
||||||
|
/// Uses the parallel Texture2DArray upload path.</summary>
|
||||||
public ulong GetOrUploadWithOrigTextureOverrideBindless(uint surfaceId, uint overrideOrigTextureId)
|
public ulong GetOrUploadWithOrigTextureOverrideBindless(uint surfaceId, uint overrideOrigTextureId)
|
||||||
{
|
{
|
||||||
uint name = GetOrUploadWithOrigTextureOverride(surfaceId, overrideOrigTextureId);
|
EnsureBindlessAvailable();
|
||||||
return MakeResidentHandle(name);
|
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;
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>64-bit bindless variant of <see cref="GetOrUploadWithPaletteOverride"/>
|
/// <summary>64-bit bindless variant of <see cref="GetOrUploadWithPaletteOverride"/>
|
||||||
/// taking a precomputed palette hash.</summary>
|
/// taking a precomputed palette hash. Uses the parallel Texture2DArray upload path.</summary>
|
||||||
public ulong GetOrUploadWithPaletteOverrideBindless(
|
public ulong GetOrUploadWithPaletteOverrideBindless(
|
||||||
uint surfaceId,
|
uint surfaceId,
|
||||||
uint? overrideOrigTextureId,
|
uint? overrideOrigTextureId,
|
||||||
PaletteOverride paletteOverride,
|
PaletteOverride paletteOverride,
|
||||||
ulong precomputedPaletteHash)
|
ulong precomputedPaletteHash)
|
||||||
{
|
{
|
||||||
uint name = GetOrUploadWithPaletteOverride(surfaceId, overrideOrigTextureId, paletteOverride, precomputedPaletteHash);
|
EnsureBindlessAvailable();
|
||||||
return MakeResidentHandle(name);
|
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)
|
if (_bindless is null)
|
||||||
throw new InvalidOperationException(
|
throw new InvalidOperationException(
|
||||||
"TextureCache constructed without BindlessSupport — cannot generate bindless handles. " +
|
"TextureCache constructed without BindlessSupport — cannot generate bindless handles. " +
|
||||||
"WbDrawDispatcher requires the bindless ctor overload.");
|
"WbDrawDispatcher requires the bindless-aware ctor overload (pass non-null BindlessSupport).");
|
||||||
if (_bindlessHandlesByGlName.TryGetValue(glTextureName, out var h))
|
|
||||||
return h;
|
|
||||||
h = _bindless.GetResidentHandle(glTextureName);
|
|
||||||
_bindlessHandlesByGlName[glTextureName] = h;
|
|
||||||
return h;
|
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
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**
|
- [ ] **Step 3.5: Write the failing tests**
|
||||||
|
|
||||||
Create `tests/AcDream.Core.Tests/Rendering/TextureCacheBindlessTests.cs`:
|
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()
|
public void Dispose()
|
||||||
{
|
{
|
||||||
// Release bindless handles BEFORE deleting underlying textures.
|
// 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)
|
if (_bindless is not null)
|
||||||
{
|
{
|
||||||
foreach (var h in _bindlessHandlesByGlName.Values)
|
foreach (var (name, handle) in _bindlessBySurfaceId.Values)
|
||||||
_bindless.MakeNonResident(h);
|
_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)
|
foreach (var h in _handlesBySurfaceId.Values)
|
||||||
_gl.DeleteTexture(h);
|
_gl.DeleteTexture(h);
|
||||||
_handlesBySurfaceId.Clear();
|
_handlesBySurfaceId.Clear();
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue