Redesign texture pipeline around one canonical handle - #751
Merged
Conversation
RRM's own ImageHandle/m_image_slots texture system had zero real consumers and was disconnected from what actually renders (materials sample via a raw bindless index into Device->GlobalTextures). Delete it in favor of Rendering::Textures::TextureHandle everywhere, and fix the three structural gaps that came with it: - No working texture disposal: TextureHandleToDispose had a consumer but no producer. VulkanDevice::DestroyTexture is now the sole producer, timeline- gating both the VkImage free and the bindless slot reclaim in Present(). - No real hot-reload trigger: AssetManager::IngestTexture's dedup silently blocked re-ingest. A new TextureImporter routes texture files through ImportCoordinator like every other asset type, and a dedup hit now calls RenderResourceManager::ScheduleTextureReload instead of no-op'ing. - No reference safety: materials stored texture refs as a bare uint64_t index with no generation. AssetManager::ReleaseTexture/FlushTextureReleases patch every referencing material to the INVALID_MAP_HANDLE sentinel before the underlying bindless slot can ever be reused. Also: VulkanDevice::ReconstructTexture generalizes the in-place resize pattern (same handle, same slot) previously duplicated inline in RenderGraph::Resize; TextureHandleToDispose is now a lock-free SPSC queue since producer and consumer are both render-thread only; AssetRegistry:: InferTypeFromExtension recognizes all 8 raster extensions TextureImporter claims plus the pre-existing .exr gap; Image2DBuffer renamed to ImageBuffer (it holds 2D, cube, and array images, not just 2D); and the fully dead Texture2D.h/.cpp (a superseded Ref<T>-based texture class, zero callers) is removed. Adversarially reviewed in 4 parallel passes; 2 real bugs found and fixed (an unlocked concurrent read, and a missing arena Clear() that would have grown unboundedly and crashed on exhaustion). 554/554 tests passing, 5 new (AssetRegistry extension coverage + OnRemoved callback firing, TextureImporter::CanImport coverage). Verified live in Obelisk under an aggressive resize stress test with no leaks or crashes.
This was referenced Sep 5, 2026
JeanPhilippeKernel
added a commit
that referenced
this pull request
Sep 5, 2026
…e redesign (#752) The doc still described RRM's ImageHandle/GPUImage/ScheduleSwap(ImageHandle,...) system as if it existed. PR #751 deleted that system outright (zero real consumers) in favor of Rendering::Textures::TextureHandle everywhere. Update the status header, naming note, and the two most misleading checklist items to point at the real current API (IngestTexture, ScheduleTextureReload, ReleaseTexture, GetTexture, VulkanDevice::DestroyTexture) instead of the removed one. Sections 2-10's prose is left as-is and explicitly scoped as accurate for buffers/meshes only, consistent with this doc's existing correction-callout convention.
JeanPhilippeKernel
added a commit
that referenced
this pull request
Sep 5, 2026
… fallback (#754) * docs(rrm): correct render-resource-manager.md for the texture pipeline redesign The doc still described RRM's ImageHandle/GPUImage/ScheduleSwap(ImageHandle,...) system as if it existed. PR #751 deleted that system outright (zero real consumers) in favor of Rendering::Textures::TextureHandle everywhere. Update the status header, naming note, and the two most misleading checklist items to point at the real current API (IngestTexture, ScheduleTextureReload, ReleaseTexture, GetTexture, VulkanDevice::DestroyTexture) instead of the removed one. Sections 2-10's prose is left as-is and explicitly scoped as accurate for buffers/meshes only, consistent with this doc's existing correction-callout convention. * fix(vulkan): guard QueueSubmit's optional args, fix GetQueue transfer fallback VulkanDevice::QueueSubmit(wait_stage_flag, command_buffer, signal_semaphore, fence) declares signal_semaphore/fence with nullptr defaults but dereferenced both unconditionally — any caller relying on the documented default crashed. Guard every dereference; skip the fence wait when no fence was given instead of trying to wait on nothing. VulkanDevice::GetQueue(TRANSFER_QUEUE) computed the family index with the correct HasSeperateTransfertQueueFamily fallback, but looked up the queue *handle* using the unadjusted type, which m_queue_map never has an entry for on devices without a separate transfer queue family — a hard abort via UnorderedHashMap::at. Mirror QueueWait's existing type-reassignment guard. Closes #741, #743.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RRM's own
ImageHandle/m_image_slotstexture system had zero real consumers and was disconnected from what actually renders (materials sample via a raw bindless index intoDevice->GlobalTextures). This deletes it in favor ofRendering::Textures::TextureHandleeverywhere, and fixes the three structural gaps that came with it:TextureHandleToDisposehad a consumer but no producer.VulkanDevice::DestroyTextureis now the sole producer, timeline-gating both theVkImagefree and the bindless slot reclaim inPresent().AssetManager::IngestTexture's dedup silently blocked re-ingest. A newTextureImporterroutes texture files throughImportCoordinatorlike every other asset type, and a dedup hit now callsRenderResourceManager::ScheduleTextureReloadinstead of no-op'ing.uint64_tindex with no generation.AssetManager::ReleaseTexture/FlushTextureReleasespatch every referencing material to theINVALID_MAP_HANDLEsentinel before the underlying bindless slot can ever be reused.Also included:
VulkanDevice::ReconstructTexturegeneralizes the in-place resize pattern (same handle, same slot) that was previously duplicated inline inRenderGraph::Resize.TextureHandleToDisposeis now a lock-free SPSC queue since producer and consumer are both render-thread only.AssetRegistry::InferTypeFromExtensionrecognizes all 8 raster extensionsTextureImporterclaims, plus the pre-existing.exrgap.Image2DBufferrenamed toImageBuffer— it holds 2D, cube, and array images, not just 2D.Texture2D.h/.cpp(a supersededRef<T>-based texture class, zero callers anywhere).Adversarially reviewed across 4 parallel passes; 2 real bugs found and fixed (an unlocked concurrent read of
AssetManager::Textures[], and a missing arenaClear()inTextureImporterthat would have grown unboundedly and crashed on exhaustion).Test plan
zEngineLib,Obelisk,ZEngineTestsall build and link cleanlyAssetRegistryextension-classification +OnRemovedcallback firing,TextureImporter::CanImportcoverage)VulkanDevicetest fixture needed to un-skip the 4RenderResourceManagerHotReloadTestcases is still open)