phase(N.5) Task 3+4 fixup: two-phase Dispose + doc consistency

Code quality review caught four issues:
- Critical: Dispose interleaved MakeNonResident + DeleteTexture per
  entry, violating ARB_bindless_texture's "all handles non-resident
  before any texture deletion" requirement. Reordered to two phases:
  Phase 1 makes ALL bindless handles non-resident; Phase 2 deletes
  ALL bindless textures; Phase 3 deletes legacy Texture2D textures.
- Important: per-call _bindless?.MakeNonResident replaced with a
  single if (_bindless is not null) guard around the whole Phase 1
  block — cleaner reasoning, one null check.
- Minor: test contract comment referenced wrong task number for
  visual gate; corrected to match current plan.
- Minor: two abbreviated XML docs (GetOrUploadWithOrigTextureOverrideBindless,
  GetOrUploadWithPaletteOverrideBindless) expanded to mention the
  throw-on-null-bindless contract for IDE readers.

This fixup also completes Task 4's Dispose work — Task 4 will be
marked complete since this commit does its full job.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erik 2026-05-08 19:59:10 +02:00
parent 0d96716825
commit 0bfe536858

View file

@ -183,8 +183,13 @@ public sealed unsafe class TextureCache : Wb.ITextureCachePerInstance, IDisposab
return handle;
}
/// <summary>64-bit bindless variant of <see cref="GetOrUploadWithOrigTextureOverride"/>.
/// Uses the parallel Texture2DArray upload path.</summary>
/// <summary>
/// 64-bit bindless handle variant of <see cref="GetOrUploadWithOrigTextureOverride"/>
/// for the WB modern rendering path. Uploads the texture as a 1-layer
/// Texture2DArray with the override SurfaceTexture id and returns a resident
/// bindless handle. Caches under a separate composite key from the legacy
/// path. Throws if BindlessSupport wasn't provided to the constructor.
/// </summary>
public ulong GetOrUploadWithOrigTextureOverrideBindless(uint surfaceId, uint overrideOrigTextureId)
{
EnsureBindlessAvailable();
@ -198,8 +203,14 @@ public sealed unsafe class TextureCache : Wb.ITextureCachePerInstance, IDisposab
return handle;
}
/// <summary>64-bit bindless variant of <see cref="GetOrUploadWithPaletteOverride"/>
/// taking a precomputed palette hash. Uses the parallel Texture2DArray upload path.</summary>
/// <summary>
/// 64-bit bindless handle variant of <see cref="GetOrUploadWithPaletteOverride"/>
/// for the WB modern rendering path. Applies the palette override on top of
/// the texture's default palette before decoding, uploads as a 1-layer
/// Texture2DArray, and returns a resident bindless handle. Takes a
/// precomputed palette hash so the WB dispatcher can compute it once per
/// entity. Throws if BindlessSupport wasn't provided to the constructor.
/// </summary>
public ulong GetOrUploadWithPaletteOverrideBindless(
uint surfaceId,
uint? overrideOrigTextureId,
@ -390,39 +401,49 @@ public sealed unsafe class TextureCache : Wb.ITextureCachePerInstance, IDisposab
public void Dispose()
{
// Phase 1: make all bindless handles non-resident BEFORE any
// DeleteTexture call. ARB_bindless_texture requires that resident
// handles be released before their backing texture is deleted —
// interleaving per-entry is UB. Single null-guard around the whole
// block (cleaner than per-call null-conditionals).
if (_bindless is not null)
{
foreach (var (_, handle) in _bindlessBySurfaceId.Values)
_bindless.MakeNonResident(handle);
foreach (var (_, handle) in _bindlessByOverridden.Values)
_bindless.MakeNonResident(handle);
foreach (var (_, handle) in _bindlessByPalette.Values)
_bindless.MakeNonResident(handle);
}
// Phase 2: delete the Texture2DArray 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();
// Phase 3: legacy Texture2D textures.
foreach (var h in _handlesBySurfaceId.Values)
_gl.DeleteTexture(h);
_handlesBySurfaceId.Clear();
foreach (var h in _handlesByOverridden.Values)
_gl.DeleteTexture(h);
_handlesByOverridden.Clear();
foreach (var h in _handlesByPalette.Values)
_gl.DeleteTexture(h);
_handlesByPalette.Clear();
if (_magentaHandle != 0)
{
_gl.DeleteTexture(_magentaHandle);
_magentaHandle = 0;
}
// Bindless caches: make handles non-resident before deleting the texture.
foreach (var (name, handle) in _bindlessBySurfaceId.Values)
{
_bindless?.MakeNonResident(handle);
_gl.DeleteTexture(name);
}
_bindlessBySurfaceId.Clear();
foreach (var (name, handle) in _bindlessByOverridden.Values)
{
_bindless?.MakeNonResident(handle);
_gl.DeleteTexture(name);
}
_bindlessByOverridden.Clear();
foreach (var (name, handle) in _bindlessByPalette.Values)
{
_bindless?.MakeNonResident(handle);
_gl.DeleteTexture(name);
}
_bindlessByPalette.Clear();
}
}