Skip to content

Redesign texture pipeline around one canonical handle - #751

Merged
JeanPhilippeKernel merged 1 commit into
developfrom
feat/texture-pipeline-redesign
Sep 4, 2026
Merged

Redesign texture pipeline around one canonical handle#751
JeanPhilippeKernel merged 1 commit into
developfrom
feat/texture-pipeline-redesign

Conversation

@JeanPhilippeKernel

Copy link
Copy Markdown
Owner

Summary

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). This deletes it in favor of Rendering::Textures::TextureHandle everywhere, and fixes the three structural gaps that came with it:

  • No working texture disposalTextureHandleToDispose 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 triggerAssetManager::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 included:

  • VulkanDevice::ReconstructTexture generalizes the in-place resize pattern (same handle, same slot) that was 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.
  • Removed the fully dead Texture2D.h/.cpp (a superseded Ref<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 arena Clear() in TextureImporter that would have grown unboundedly and crashed on exhaustion).

Test plan

  • zEngineLib, Obelisk, ZEngineTests all build and link cleanly
  • 554/554 tests passing (5 new: AssetRegistry extension-classification + OnRemoved callback firing, TextureImporter::CanImport coverage)
  • Verified live in Obelisk under an aggressive resize stress test (3 escalating rounds, ~100+ resize events including near-zero dimensions) — no leaks, no crashes, same PID throughout
  • Manual texture hot-reload verification in the running editor (tracked separately — the headless VulkanDevice test fixture needed to un-skip the 4 RenderResourceManagerHotReloadTest cases is still open)

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.
@JeanPhilippeKernel JeanPhilippeKernel added this to the Stable Core (1.0.0) milestone Sep 4, 2026
@JeanPhilippeKernel JeanPhilippeKernel self-assigned this Sep 4, 2026
@JeanPhilippeKernel JeanPhilippeKernel added enhancement New feature or request area-rendering area-linux Work on Linux system area-window Work on Window system area-macOS Work on macOS system labels Sep 4, 2026
@JeanPhilippeKernel
JeanPhilippeKernel merged commit 6580f7d into develop Sep 4, 2026
17 checks passed
@JeanPhilippeKernel
JeanPhilippeKernel deleted the feat/texture-pipeline-redesign branch September 4, 2026 12:01
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-linux Work on Linux system area-macOS Work on macOS system area-rendering area-window Work on Window system enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant