fix(vulkan): guard QueueSubmit's optional args, fix GetQueue transfer fallback - #754
Merged
JeanPhilippeKernel merged 2 commits intoSep 5, 2026
Conversation
…e 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.
… 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.
JeanPhilippeKernel
added
enhancement
JeanPhilippeKernel
deleted the
fix/queuesubmit-getqueue-null-and-transfer-fallback
branch
September 5, 2026 15:26
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
1 participant
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
Two small, unrelated null-deref/abort bugs in
VulkanDevice:QueueSubmit(wait_stage_flag, command_buffer, signal_semaphore, fence)declaressignal_semaphore/fencewithnullptrdefaults but dereferenced both unconditionally. Any caller relying on the documented default (submit without a signal semaphore/fence) crashed. Guarded every dereference; when no fence is given, the function now skips the wait/reset step and returnstrueimmediately after submit instead of trying to wait on nothing.GetQueue(TRANSFER_QUEUE)computed the queue family index with the correctHasSeperateTransfertQueueFamilyfallback, but looked up the queue handle using the unadjustedtype—m_queue_mapnever has aTRANSFER_QUEUEentry on a device with no separate transfer queue family, so this was a hard abort (UnorderedHashMap::at) rather than a graceful fallback. MirrorsQueueWait's existing type-reassignment guard, which already had this right.Neither path has any call site today (confirmed via repo-wide grep before fixing) — these are latent bugs in public API surface, not currently triggered — but both are cheap, well-scoped fixes worth closing before something starts relying on the documented defaults.
Test plan
zEngineLib,Obelisk,ZEngineTestsall build cleanlyVulkanDeviceto exercise, which is blocked on Headless VulkanDevice test fixture for RRM/texture-pipeline GPU-level tests #753 (headlessVulkanDevicefixture), same as the RRM hot-reload tests