From 0bfe536858cb9512eb91c6884fefcfc30d582b35 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 8 May 2026 19:59:10 +0200 Subject: [PATCH] phase(N.5) Task 3+4 fixup: two-phase Dispose + doc consistency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/AcDream.App/Rendering/TextureCache.cs | 69 +++++++++++++++-------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/src/AcDream.App/Rendering/TextureCache.cs b/src/AcDream.App/Rendering/TextureCache.cs index dcc9557..78eef29 100644 --- a/src/AcDream.App/Rendering/TextureCache.cs +++ b/src/AcDream.App/Rendering/TextureCache.cs @@ -183,8 +183,13 @@ public sealed unsafe class TextureCache : Wb.ITextureCachePerInstance, IDisposab return handle; } - /// 64-bit bindless variant of . - /// Uses the parallel Texture2DArray upload path. + /// + /// 64-bit bindless handle variant of + /// 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. + /// public ulong GetOrUploadWithOrigTextureOverrideBindless(uint surfaceId, uint overrideOrigTextureId) { EnsureBindlessAvailable(); @@ -198,8 +203,14 @@ public sealed unsafe class TextureCache : Wb.ITextureCachePerInstance, IDisposab return handle; } - /// 64-bit bindless variant of - /// taking a precomputed palette hash. Uses the parallel Texture2DArray upload path. + /// + /// 64-bit bindless handle variant of + /// 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. + /// 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(); } }