From 97a487ee8cddc97215765bdf8ca2ce6a9e8b8c43 Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Thu, 21 May 2026 22:46:28 -0400 Subject: [PATCH 01/11] Queue guard callbacks that can make UploadEx efficiently thread-safe in conjunction with guarded suballocator callbacks. --- lib/include/ktxvulkan.h | 45 +++++++++++++++++++++ lib/src/vkloader.c | 89 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 128 insertions(+), 6 deletions(-) diff --git a/lib/include/ktxvulkan.h b/lib/include/ktxvulkan.h index 47ae45b37d..8b05d141c0 100644 --- a/lib/include/ktxvulkan.h +++ b/lib/include/ktxvulkan.h @@ -134,6 +134,9 @@ typedef VkResult(*ktxVulkanTexture_subAllocatorBindImageFuncPtr)(VkImage image, typedef VkResult(*ktxVulkanTexture_subAllocatorMemoryMapFuncPtr)(uint64_t allocId, uint64_t pageNumber, VkDeviceSize *mapLength, void** dataPtr); typedef void (*ktxVulkanTexture_subAllocatorMemoryUnmapFuncPtr)(uint64_t allocId, uint64_t pageNumber); typedef void (*ktxVulkanTexture_subAllocatorFreeMemFuncPtr)(uint64_t allocId); +typedef void (*ktxVulkanTexture_queueLockFuncPtr)(); +typedef void (*ktxVulkanTexture_queueUnlockFuncPtr)(); + /** * @class ktxVulkanTexture_subAllocatorCallbacks * @~English @@ -234,6 +237,32 @@ KTX_API void KTX_APIENTRY ktxVulkanDeviceInfo_Destruct(ktxVulkanDeviceInfo* This); KTX_API void KTX_APIENTRY ktxVulkanDeviceInfo_Destroy(ktxVulkanDeviceInfo* This); + + +/** + * @class ktxVulkanTexture_queueGuardCallbacks + * @~English + * @brief Struct that contains callbacks necessary for guarding access to the supplied queue in vdi. + * + * Needs to be used in conjunction with suballocator callbacks that are also guarded for thread-safety. Together they can make UploadEx fully thread-safe in an efficient manner. + * The suballocator callbacks need to guard around memory objects and if sparse binding support is introduced, will need to guard around both memory and queue objects. + * As of now, guarding for external bookkeeping mechanisms are also a necessity in a threaded environment. + * + */ +typedef struct { + ktxVulkanTexture_queueLockFuncPtr queueLockFuncPtr; /*!< Pointer to function for commencing mutually exclusive access to the vdi queue. */ + ktxVulkanTexture_queueUnlockFuncPtr queueUnlockFuncPtr; /*!< Pointer to function for ending mutually exclusive access to the vdi queue. */ +} ktxVulkanTexture_QueueGuardCallbacks; + +KTX_API KTX_error_code KTX_APIENTRY +ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanDeviceInfo* vdi, + ktxVulkanTexture* vkTexture, + VkImageTiling tiling, + VkImageUsageFlags usageFlags, + VkImageLayout finalLayout, + ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks, + ktxVulkanTexture_QueueGuardCallbacks* queueMutexCallbacks); + KTX_API KTX_error_code KTX_APIENTRY ktxTexture_VkUploadEx_WithSuballocator(ktxTexture* This, ktxVulkanDeviceInfo* vdi, ktxVulkanTexture* vkTexture, @@ -251,6 +280,14 @@ KTX_API KTX_error_code KTX_APIENTRY ktxTexture_VkUpload(ktxTexture* texture, ktxVulkanDeviceInfo* vdi, ktxVulkanTexture *vkTexture); KTX_API KTX_error_code KTX_APIENTRY +ktxTexture1_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture1* This, ktxVulkanDeviceInfo* vdi, + ktxVulkanTexture* vkTexture, + VkImageTiling tiling, + VkImageUsageFlags usageFlags, + VkImageLayout finalLayout, + ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks, + ktxVulkanTexture_QueueGuardCallbacks* queueMutexCallbacks); +KTX_API KTX_error_code KTX_APIENTRY ktxTexture1_VkUploadEx_WithSuballocator(ktxTexture1* This, ktxVulkanDeviceInfo* vdi, ktxVulkanTexture* vkTexture, VkImageTiling tiling, @@ -267,6 +304,14 @@ KTX_API KTX_error_code KTX_APIENTRY ktxTexture1_VkUpload(ktxTexture1* texture, ktxVulkanDeviceInfo* vdi, ktxVulkanTexture *vkTexture); KTX_API KTX_error_code KTX_APIENTRY +ktxTexture2_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture2* This, ktxVulkanDeviceInfo* vdi, + ktxVulkanTexture* vkTexture, + VkImageTiling tiling, + VkImageUsageFlags usageFlags, + VkImageLayout finalLayout, + ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks, + ktxVulkanTexture_QueueGuardCallbacks* queueMutexCallbacks); +KTX_API KTX_error_code KTX_APIENTRY ktxTexture2_VkUploadEx_WithSuballocator(ktxTexture2* This, ktxVulkanDeviceInfo* vdi, ktxVulkanTexture* vkTexture, VkImageTiling tiling, diff --git a/lib/src/vkloader.c b/lib/src/vkloader.c index e46f0cb80c..d1aeaa93b3 100644 --- a/lib/src/vkloader.c +++ b/lib/src/vkloader.c @@ -794,6 +794,12 @@ linearTilingPadCallback(int miplevel, int face, * They use a uint64_t stored in the @c allocationId * field of the structure pointed at by @a vkTexture * to reference allocated page(s). + * @param [in] queueMutexCallbacks If used in conjunction with suballocator callbacks + * that guard against simultaneous access to memory + * (or additionally queue if sparse binding support + * is added) and external suballocation managements + * objects, it can make UploadEx fully thread-safe + * and efficiently so. * * @return KTX_SUCCESS on success, other KTX_* enum values on error. * @@ -819,12 +825,13 @@ linearTilingPadCallback(int miplevel, int face, * @sa @ref ktxVulkanDeviceInfo::ktxVulkanDeviceInfo\_Construct "ktxVulkanDeviceInfo_Construct()" */ KTX_error_code -ktxTexture_VkUploadEx_WithSuballocator(ktxTexture* This, ktxVulkanDeviceInfo* vdi, - ktxVulkanTexture* vkTexture, - VkImageTiling tiling, - VkImageUsageFlags usageFlags, - VkImageLayout finalLayout, - ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks) +ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanDeviceInfo* vdi, + ktxVulkanTexture* vkTexture, + VkImageTiling tiling, + VkImageUsageFlags usageFlags, + VkImageLayout finalLayout, + ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks, + ktxVulkanTexture_QueueGuardCallbacks* queueMutexCallbacks) { KTX_error_code kResult; VkFilter blitFilter = VK_FILTER_LINEAR; @@ -853,6 +860,7 @@ ktxTexture_VkUploadEx_WithSuballocator(ktxTexture* This, ktxVulkanDeviceInfo* vd ktx_uint32_t elementSize = ktxTexture_GetElementSize(This); ktx_bool_t canUseFasterPath; ktx_bool_t useSuballocator = false; + ktx_bool_t useQueueMutex = false; if (subAllocatorCallbacks) { if (subAllocatorCallbacks->allocMemFuncPtr && subAllocatorCallbacks->bindBufferFuncPtr && @@ -864,6 +872,13 @@ ktxTexture_VkUploadEx_WithSuballocator(ktxTexture* This, ktxVulkanDeviceInfo* vd else return KTX_INVALID_VALUE; } + if (queueMutexCallbacks) { + if (queueMutexCallbacks->queueLockFuncPtr && + queueMutexCallbacks->queueUnlockFuncPtr) + useQueueMutex = true; + else + return KTX_INVALID_VALUE; + } if (!vdi || !This || !vkTexture) { return KTX_INVALID_VALUE; @@ -1293,8 +1308,10 @@ ktxTexture_VkUploadEx_WithSuballocator(ktxTexture* This, ktxVulkanDeviceInfo* vd submitInfo.commandBufferCount = 1; submitInfo.pCommandBuffers = &vdi->cmdBuffer; + if (useQueueMutex) queueMutexCallbacks->queueLockFuncPtr(); VK_CHECK_RESULT( vdi->vkFuncs.vkQueueSubmit(vdi->queue, 1, &submitInfo, copyFence)); + if (useQueueMutex) queueMutexCallbacks->queueUnlockFuncPtr(); VK_CHECK_RESULT( vdi->vkFuncs.vkWaitForFences(vdi->device, 1, ©Fence, @@ -1445,12 +1462,26 @@ ktxTexture_VkUploadEx_WithSuballocator(ktxTexture* This, ktxVulkanDeviceInfo* vd submitInfo.commandBufferCount = 1; submitInfo.pCommandBuffers = &vdi->cmdBuffer; + if (useQueueMutex) queueMutexCallbacks->queueLockFuncPtr(); VK_CHECK_RESULT(vdi->vkFuncs.vkQueueSubmit(vdi->queue, 1, &submitInfo, nullFence)); VK_CHECK_RESULT(vdi->vkFuncs.vkQueueWaitIdle(vdi->queue)); + if (useQueueMutex) queueMutexCallbacks->queueUnlockFuncPtr(); } return KTX_SUCCESS; } +KTX_error_code +ktxTexture_VkUploadEx_WithSuballocator(ktxTexture* This, ktxVulkanDeviceInfo* vdi, + ktxVulkanTexture* vkTexture, + VkImageTiling tiling, + VkImageUsageFlags usageFlags, + VkImageLayout finalLayout, + ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks) +{ + return ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(This, vdi, vkTexture, tiling, usageFlags, + finalLayout, subAllocatorCallbacks, NULL); +} + /** @memberof ktxTexture * @~English * @brief Create a Vulkan image object from a ktxTexture object. @@ -1490,6 +1521,29 @@ ktxTexture_VkUpload(ktxTexture* texture, ktxVulkanDeviceInfo* vdi, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); } +/** @memberof ktxTexture1 + * @~English + * @brief Create a Vulkan image object from a ktxTexture1 object. + * + * This simply calls @ref ktxTexture::ktxTexture\_VkUploadEx_WithSuballocatorAndQueueGuard + * "ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard()" + * + * @copydetails ktxTexture::ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard + */ +KTX_error_code +ktxTexture1_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture1* This, ktxVulkanDeviceInfo* vdi, + ktxVulkanTexture* vkTexture, + VkImageTiling tiling, + VkImageUsageFlags usageFlags, + VkImageLayout finalLayout, + ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks, + ktxVulkanTexture_QueueGuardCallbacks* queueMutexCallbacks) +{ + return ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture(This), vdi, vkTexture, + tiling, usageFlags, finalLayout, + subAllocatorCallbacks, queueMutexCallbacks); +} + /** @memberof ktxTexture1 * @~English * @brief Create a Vulkan image object from a ktxTexture1 object. @@ -1548,6 +1602,29 @@ ktxTexture1_VkUpload(ktxTexture1* texture, ktxVulkanDeviceInfo* vdi, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL); } +/** @memberof ktxTexture2 + * @~English + * @brief Create a Vulkan image object from a ktxTexture2 object. + * + * This simplly calls @ref ktxTexture::ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard + * "ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard()". + * + * @copydetails ktxTexture::ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard + */ +KTX_error_code +ktxTexture2_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture2* This, ktxVulkanDeviceInfo* vdi, + ktxVulkanTexture* vkTexture, + VkImageTiling tiling, + VkImageUsageFlags usageFlags, + VkImageLayout finalLayout, + ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks, + ktxVulkanTexture_QueueGuardCallbacks* queueMutexCallbacks) +{ + return ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture(This), vdi, vkTexture, + tiling, usageFlags, finalLayout, + subAllocatorCallbacks, queueMutexCallbacks); +} + /** @memberof ktxTexture2 * @~English * @brief Create a Vulkan image object from a ktxTexture2 object. From 9b042a2d75af305233027975ba340f8ad9bbdabe Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Sun, 24 May 2026 18:33:59 -0400 Subject: [PATCH 02/11] Transfer queue support --- lib/src/vkloader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/vkloader.c b/lib/src/vkloader.c index d1aeaa93b3..effb85bd39 100644 --- a/lib/src/vkloader.c +++ b/lib/src/vkloader.c @@ -1870,7 +1870,7 @@ setImageLayout( = VK_ACCESS_HOST_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT; } imageMemoryBarrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT; - destStageFlags = VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT | VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT; + destStageFlags = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; break; default: From 86ae1abecffb84803ba9c1dd1a242788063d413c Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Mon, 25 May 2026 02:35:18 -0400 Subject: [PATCH 03/11] Don't leave the memory mapped on the way out --- lib/src/vkloader.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/src/vkloader.c b/lib/src/vkloader.c index effb85bd39..bd44c4ccc9 100644 --- a/lib/src/vkloader.c +++ b/lib/src/vkloader.c @@ -1171,7 +1171,13 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD pMappedStagingBuffer, (ktx_size_t)memAllocInfo.allocationSize); if (kResult != KTX_SUCCESS) + { + if (!useSuballocator) + vdi->vkFuncs.vkUnmapMemory(vdi->device, stagingMemory); + else + subAllocatorCallbacks->memoryUnmapFuncPtr(stagingAllocId, 0ull); return kResult; + } } // Iterate over mip levels to set up the copy regions. From 03dc28a7373b4c7692168b638f3f78ee0d7a3a8f Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Wed, 29 Jul 2026 22:55:02 -0400 Subject: [PATCH 04/11] Reverting commit for transfer queue support. Changing Vulkan usage instead at the source. --- lib/src/vkloader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/vkloader.c b/lib/src/vkloader.c index bd44c4ccc9..3352ac5bf3 100644 --- a/lib/src/vkloader.c +++ b/lib/src/vkloader.c @@ -1876,7 +1876,7 @@ setImageLayout( = VK_ACCESS_HOST_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT; } imageMemoryBarrier.dstAccessMask = VK_ACCESS_SHADER_READ_BIT; - destStageFlags = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; + destStageFlags = VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT | VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT; break; default: From 82a5bad1acf38a7154084eb7fe708bee79f0bef2 Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Sat, 1 Aug 2026 03:16:57 -0400 Subject: [PATCH 05/11] Sample usage for tests. These only serve as a guide and do not stress test the feature. An actual test case would need multiple textures simultaneously uploaded to and the entire test environment to re-use the mutexes provided in relevant scenarios. Such scenarios include other simultaneous accesses to the queue creating textures or arena `VkDeviceMemory`s. --- tests/loadtests/vkloadtests/Texture.cpp | 29 +++++-- tests/loadtests/vkloadtests/Texture.h | 6 ++ .../vkloadtests/VulkanLoadTestSample.cpp | 82 +++++++++++++++++++ .../vkloadtests/VulkanLoadTestSample.h | 29 +++++++ .../loadtests/vkloadtests/VulkanLoadTests.cpp | 4 + 5 files changed, 145 insertions(+), 5 deletions(-) diff --git a/tests/loadtests/vkloadtests/Texture.cpp b/tests/loadtests/vkloadtests/Texture.cpp index 122b01be7d..64eb02bcdf 100644 --- a/tests/loadtests/vkloadtests/Texture.cpp +++ b/tests/loadtests/vkloadtests/Texture.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include "argparser.h" #include "Texture.h" @@ -65,7 +66,8 @@ Texture::Texture(VulkanContext& vkctx, rotation = { 0.0f, 15.0f, 0.0f }; tiling = vk::ImageTiling::eOptimal; useSubAlloc = UseSuballocator::No; - rgbcolor upperLeftColor{ 0.7f, 0.1f, 0.2f }; + useQueueGuard = UseQueueGuard::No; + rgbcolor upperLeftColor{0.7f, 0.1f, 0.2f}; rgbcolor lowerLeftColor{ 0.8f, 0.9f, 0.3f }; rgbcolor upperRightColor{ 0.4f, 1.0f, 0.5f }; rgbcolor lowerRightColor{ 0.0f, 0.6f, 0.1f }; @@ -123,10 +125,26 @@ Texture::Texture(VulkanContext& vkctx, VkInstance vkInst = vkctx.instance; VMA_CALLBACKS::InitVMA(vdi.physicalDevice, vdi.device, vkInst, vdi.deviceMemoryProperties); - ktxresult = ktxTexture_VkUploadEx_WithSuballocator(kTexture, &vdi, &texture, - static_cast(tiling), - VK_IMAGE_USAGE_SAMPLED_BIT, - VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, &subAllocatorCallbacks); + if (useQueueGuard == UseQueueGuard::Yes) + { + std::thread uploaderThread([&]() { + ktxresult = ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(kTexture, &vdi, &texture, + static_cast(tiling), + VK_IMAGE_USAGE_SAMPLED_BIT, + VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, + &subAllocatorCallbacksGuarded, &queueGuardCallbacks); + }); + uploaderThread.join(); // We really have no choice here. This crucible/test-environment only loads one texture at a time. + // Additionally `QUEUE_GUARD_CALLBACKS::queueAccessGuard` and `QUEUE_GUARD_CALLBACKS::memoryAccessGuard` would + // need to be re-used where arena `VkDeviceMemory`s or the same `VkQueue`s are being accessed simultaneously. + } + else + { + ktxresult = ktxTexture_VkUploadEx_WithSuballocator(kTexture, &vdi, &texture, + static_cast(tiling), + VK_IMAGE_USAGE_SAMPLED_BIT, + VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, &subAllocatorCallbacks); + } } else // Keep separate call so ktxTexture_VkUploadEx is also tested. ktxresult = ktxTexture_VkUploadEx(kTexture, &vdi, &texture, @@ -236,6 +254,7 @@ Texture::processArgs(std::string sArgs) {"external", argparser::option::no_argument, &externalFile, 1}, {"linear-tiling", argparser::option::no_argument, (int*)&tiling, (int)vk::ImageTiling::eLinear}, {"use-vma", argparser::option::no_argument, (int*)&useSubAlloc, (int)UseSuballocator::Yes}, + {"guard-queue", argparser::option::no_argument, (int*)&useQueueGuard, (int)UseQueueGuard::Yes}, {"qcolor", argparser::option::required_argument, NULL, 1}, {NULL, argparser::option::no_argument, NULL, 0} }; diff --git a/tests/loadtests/vkloadtests/Texture.h b/tests/loadtests/vkloadtests/Texture.h index 63664f6c7d..bcfc843c52 100644 --- a/tests/loadtests/vkloadtests/Texture.h +++ b/tests/loadtests/vkloadtests/Texture.h @@ -38,6 +38,11 @@ class Texture : public VulkanLoadTestSample No = 0, Yes }; + enum class UseQueueGuard + { + No = 0, + Yes + }; std::string filename; ktxVulkanTexture texture; @@ -45,6 +50,7 @@ class Texture : public VulkanLoadTestSample vk::ImageView imageView; vk::ImageTiling tiling; UseSuballocator useSubAlloc; + UseQueueGuard useQueueGuard; vk::ComponentMapping swizzle; struct { diff --git a/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp b/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp index ff19df07bf..f452c59031 100644 --- a/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp +++ b/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp @@ -20,6 +20,7 @@ #include #include +#include #define VMA_IMPLEMENTATION #define VMA_VULKAN_VERSION 1000000 @@ -131,6 +132,87 @@ namespace VMA_CALLBACKS vmaFreeMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation); AllocMemCWrapperDirectory.erase(allocId); } + + std::mutex memoryAccessGuard; + uint64_t AllocMemCWrapperGuarded(VkMemoryAllocateInfo* allocInfo, VkMemoryRequirements* memReq, uint64_t* numPages) + { + uint64_t allocId = mt64(); + VmaAllocationCreateInfo pCreateInfo = {}; + if ((cachedDevMemProps.memoryTypes[allocInfo->memoryTypeIndex].propertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) || + (cachedDevMemProps.memoryTypes[allocInfo->memoryTypeIndex].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) + { + pCreateInfo.usage = VMA_MEMORY_USAGE_CPU_TO_GPU; + pCreateInfo.flags = VMA_ALLOCATION_CREATE_HOST_ACCESS_SEQUENTIAL_WRITE_BIT; + } + else + { + pCreateInfo.usage = VMA_MEMORY_USAGE_GPU_ONLY; + } + pCreateInfo.memoryTypeBits = memReq->memoryTypeBits; + + { + std::lock_guard lk(memoryAccessGuard); + VmaAllocation allocation; + VkResult result = vmaAllocateMemory(vmaAllocator, memReq, &pCreateInfo, &allocation, VMA_NULL); + if (result != VK_SUCCESS) + { + return 0ull; + } + + AllocMemCWrapperDirectory[allocId].allocation = allocation; + AllocMemCWrapperDirectory[allocId].mapSize = memReq->size; + } + *numPages = 1ull; + + return allocId; + } + + VkResult BindBufferMemoryCWrapperGuarded(VkBuffer buffer, uint64_t allocId) + { + std::lock_guard lk(memoryAccessGuard); + return vmaBindBufferMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation, buffer); + } + + VkResult BindImageMemoryCWrapperGuarded(VkImage image, uint64_t allocId) + { + std::lock_guard lk(memoryAccessGuard); + return vmaBindImageMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation, image); + } + + VkResult MapMemoryCWrapperGuarded(uint64_t allocId, uint64_t, VkDeviceSize* mapLength, void** dataPtr) + { + memoryAccessGuard.lock(); // Locked for the duration of access to VkDeviceMemory + *mapLength = AllocMemCWrapperDirectory[allocId].mapSize; + return vmaMapMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation, dataPtr); + } + + void UnmapMemoryCWrapperGuarded(uint64_t allocId, uint64_t) + { + vmaUnmapMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation); + memoryAccessGuard.unlock(); // Done with VkDeviceMemory access + } + + void FreeMemCWrapperGuarded(uint64_t allocId) + { + std::lock_guard lk(memoryAccessGuard); + vmaFreeMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation); + AllocMemCWrapperDirectory.erase(allocId); + } +} + +namespace QUEUE_GUARD_CALLBACKS +{ + std::mutex queueAccessGuard; + + void LockQueue() + { + queueAccessGuard.lock(); + } + + void UnlockQueue() + { + queueAccessGuard.unlock(); + } } VulkanLoadTestSample::~VulkanLoadTestSample() diff --git a/tests/loadtests/vkloadtests/VulkanLoadTestSample.h b/tests/loadtests/vkloadtests/VulkanLoadTestSample.h index 2d46ab09c1..7974d88a79 100644 --- a/tests/loadtests/vkloadtests/VulkanLoadTestSample.h +++ b/tests/loadtests/vkloadtests/VulkanLoadTestSample.h @@ -25,12 +25,27 @@ namespace VMA_CALLBACKS void InitVMA(VkPhysicalDevice& physicalDevice, VkDevice& device, VkInstance& instance, VkPhysicalDeviceMemoryProperties& devMemProps); void DestroyVMA(); + uint64_t AllocMemCWrapper(VkMemoryAllocateInfo* allocInfo, VkMemoryRequirements* memReq, uint64_t* numPages); VkResult BindBufferMemoryCWrapper(VkBuffer buffer, uint64_t allocId); VkResult BindImageMemoryCWrapper(VkImage image, uint64_t allocId); VkResult MapMemoryCWrapper(uint64_t allocId, uint64_t, VkDeviceSize* mapLength, void** dataPtr); void UnmapMemoryCWrapper(uint64_t allocId, uint64_t); void FreeMemCWrapper(uint64_t allocId); + + uint64_t AllocMemCWrapperGuarded(VkMemoryAllocateInfo* allocInfo, VkMemoryRequirements* memReq, + uint64_t* numPages); + VkResult BindBufferMemoryCWrapperGuarded(VkBuffer buffer, uint64_t allocId); + VkResult BindImageMemoryCWrapperGuarded(VkImage image, uint64_t allocId); + VkResult MapMemoryCWrapperGuarded(uint64_t allocId, uint64_t, VkDeviceSize* mapLength, void** dataPtr); + void UnmapMemoryCWrapperGuarded(uint64_t allocId, uint64_t); + void FreeMemCWrapperGuarded(uint64_t allocId); +} + +namespace QUEUE_GUARD_CALLBACKS +{ + void LockQueue(); + void UnlockQueue(); } #define ARRAY_LEN(a) (sizeof(a) / sizeof(a[0])) @@ -79,6 +94,20 @@ class VulkanLoadTestSample : public LoadTestSample { VMA_CALLBACKS::FreeMemCWrapper }; + ktxVulkanTexture_subAllocatorCallbacks subAllocatorCallbacksGuarded = { + VMA_CALLBACKS::AllocMemCWrapperGuarded, + VMA_CALLBACKS::BindBufferMemoryCWrapperGuarded, + VMA_CALLBACKS::BindImageMemoryCWrapperGuarded, + VMA_CALLBACKS::MapMemoryCWrapperGuarded, + VMA_CALLBACKS::UnmapMemoryCWrapperGuarded, + VMA_CALLBACKS::FreeMemCWrapperGuarded + }; + + ktxVulkanTexture_QueueGuardCallbacks queueGuardCallbacks = { + QUEUE_GUARD_CALLBACKS::LockQueue, + QUEUE_GUARD_CALLBACKS::UnlockQueue + }; + virtual void keyPressed(uint32_t /*keyCode*/) { } virtual void viewChanged() { } bool gpuSupportsSwizzle() { diff --git a/tests/loadtests/vkloadtests/VulkanLoadTests.cpp b/tests/loadtests/vkloadtests/VulkanLoadTests.cpp index ba75c5381d..e89047ae64 100644 --- a/tests/loadtests/vkloadtests/VulkanLoadTests.cpp +++ b/tests/loadtests/vkloadtests/VulkanLoadTests.cpp @@ -399,6 +399,10 @@ const VulkanLoadTests::sampleInvocation siSamples[] = { "--use-vma Iron_Bars_001_normal_uastc_zstd_10.ktx2", "KTX2: UASTC+zstd Compressed XY Normal Map Mipmapped Using VMA," }, + { Texture::create, + "--use-vma --guard-queue Iron_Bars_001_normal_uastc_zstd_10.ktx2", + "KTX2: UASTC+zstd Compressed XY Normal Map Mipmapped Using VMA and Queue Guard," + }, { Texture::create, "color_grid_zstd_5.ktx2", "KTX2: Zstd Compressed RGBA8 Non-mipmapped" From b1170cb67670d15aff936ec5654d64388399fd80 Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Mon, 10 Aug 2026 00:13:13 -0400 Subject: [PATCH 06/11] Other than map and unmap, VMA is thread-safe: memory guards were too heavy handed. --- .../vkloadtests/VulkanLoadTestSample.cpp | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp b/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp index f452c59031..e09a75d13a 100644 --- a/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp +++ b/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp @@ -150,15 +150,15 @@ namespace VMA_CALLBACKS } pCreateInfo.memoryTypeBits = memReq->memoryTypeBits; + VmaAllocation allocation; + VkResult result = vmaAllocateMemory(vmaAllocator, memReq, &pCreateInfo, &allocation, VMA_NULL); + if (result != VK_SUCCESS) { - std::lock_guard lk(memoryAccessGuard); - VmaAllocation allocation; - VkResult result = vmaAllocateMemory(vmaAllocator, memReq, &pCreateInfo, &allocation, VMA_NULL); - if (result != VK_SUCCESS) - { - return 0ull; - } + return 0ull; + } + { + std::lock_guard lk(memoryAccessGuard); AllocMemCWrapperDirectory[allocId].allocation = allocation; AllocMemCWrapperDirectory[allocId].mapSize = memReq->size; } @@ -169,34 +169,46 @@ namespace VMA_CALLBACKS VkResult BindBufferMemoryCWrapperGuarded(VkBuffer buffer, uint64_t allocId) { - std::lock_guard lk(memoryAccessGuard); - return vmaBindBufferMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation, buffer); + VmaAllocation allocation; + { + std::lock_guard lk(memoryAccessGuard); + allocation = AllocMemCWrapperDirectory[allocId].allocation; + } + return vmaBindBufferMemory(vmaAllocator, allocation, buffer); } VkResult BindImageMemoryCWrapperGuarded(VkImage image, uint64_t allocId) { - std::lock_guard lk(memoryAccessGuard); - return vmaBindImageMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation, image); + VmaAllocation allocation; + { + std::lock_guard lk(memoryAccessGuard); + allocation = AllocMemCWrapperDirectory[allocId].allocation; + } + return vmaBindImageMemory(vmaAllocator, allocation, image); } VkResult MapMemoryCWrapperGuarded(uint64_t allocId, uint64_t, VkDeviceSize* mapLength, void** dataPtr) { - memoryAccessGuard.lock(); // Locked for the duration of access to VkDeviceMemory + std::lock_guard lk(memoryAccessGuard); *mapLength = AllocMemCWrapperDirectory[allocId].mapSize; return vmaMapMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation, dataPtr); } void UnmapMemoryCWrapperGuarded(uint64_t allocId, uint64_t) { + std::lock_guard lk(memoryAccessGuard); vmaUnmapMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation); - memoryAccessGuard.unlock(); // Done with VkDeviceMemory access } void FreeMemCWrapperGuarded(uint64_t allocId) { - std::lock_guard lk(memoryAccessGuard); - vmaFreeMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation); - AllocMemCWrapperDirectory.erase(allocId); + VmaAllocation allocation; + { + std::lock_guard lk(memoryAccessGuard); + allocation = AllocMemCWrapperDirectory[allocId].allocation; + AllocMemCWrapperDirectory.erase(allocId); + } + vmaFreeMemory(vmaAllocator, allocation); } } From 339c04731e38806a288caf7ca4820accdfd08af6 Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Mon, 10 Aug 2026 00:50:31 -0400 Subject: [PATCH 07/11] Addressing comment issues. Making the first set more succinct. --- lib/include/ktxvulkan.h | 8 +++++--- tests/loadtests/vkloadtests/Texture.cpp | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/include/ktxvulkan.h b/lib/include/ktxvulkan.h index 8b05d141c0..764f7f700a 100644 --- a/lib/include/ktxvulkan.h +++ b/lib/include/ktxvulkan.h @@ -244,9 +244,11 @@ ktxVulkanDeviceInfo_Destroy(ktxVulkanDeviceInfo* This); * @~English * @brief Struct that contains callbacks necessary for guarding access to the supplied queue in vdi. * - * Needs to be used in conjunction with suballocator callbacks that are also guarded for thread-safety. Together they can make UploadEx fully thread-safe in an efficient manner. - * The suballocator callbacks need to guard around memory objects and if sparse binding support is introduced, will need to guard around both memory and queue objects. - * As of now, guarding for external bookkeeping mechanisms are also a necessity in a threaded environment. + * Needs to be used in conjunction with suballocator callbacks that are also thread-safe. + * Together they can make UploadEx fully thread-safe in an efficient manner. + * The suballocator callbacks need to guard around memory objects and if sparse binding + * support is introduced, will need to guard around both memory and queue objects. + * Thread-safety for the external bookkeeping mechanism is also a necessity. * */ typedef struct { diff --git a/tests/loadtests/vkloadtests/Texture.cpp b/tests/loadtests/vkloadtests/Texture.cpp index 64eb02bcdf..0ab93f8437 100644 --- a/tests/loadtests/vkloadtests/Texture.cpp +++ b/tests/loadtests/vkloadtests/Texture.cpp @@ -136,7 +136,7 @@ Texture::Texture(VulkanContext& vkctx, }); uploaderThread.join(); // We really have no choice here. This crucible/test-environment only loads one texture at a time. // Additionally `QUEUE_GUARD_CALLBACKS::queueAccessGuard` and `QUEUE_GUARD_CALLBACKS::memoryAccessGuard` would - // need to be re-used where arena `VkDeviceMemory`s or the same `VkQueue`s are being accessed simultaneously. + // need to be re-used when arena `VkDeviceMemory`s or the same `VkQueue`s are being accessed simultaneously. } else { From 6c155ad62a5fc77964795fa0e17defad43adb718 Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Sat, 15 Aug 2026 03:50:28 -0400 Subject: [PATCH 08/11] * Noting that a per-thread VDI is a necessary prerequisite for thread-safety. * Passing the queue to be guarded to the lock/unlock callbacks. * `mt64()` needs guarding, but VMA (unless initialized otherwise) is thread-safe by default. * Much better clean-up on `ktxTexture_LoadImageData()`'s failure. --- lib/include/ktxvulkan.h | 5 ++-- lib/src/vkloader.c | 22 ++++++++++---- tests/loadtests/vkloadtests/Texture.cpp | 2 +- .../vkloadtests/VulkanLoadTestSample.cpp | 30 ++++++++++++------- .../vkloadtests/VulkanLoadTestSample.h | 4 +-- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/lib/include/ktxvulkan.h b/lib/include/ktxvulkan.h index 764f7f700a..df27d9b296 100644 --- a/lib/include/ktxvulkan.h +++ b/lib/include/ktxvulkan.h @@ -134,8 +134,8 @@ typedef VkResult(*ktxVulkanTexture_subAllocatorBindImageFuncPtr)(VkImage image, typedef VkResult(*ktxVulkanTexture_subAllocatorMemoryMapFuncPtr)(uint64_t allocId, uint64_t pageNumber, VkDeviceSize *mapLength, void** dataPtr); typedef void (*ktxVulkanTexture_subAllocatorMemoryUnmapFuncPtr)(uint64_t allocId, uint64_t pageNumber); typedef void (*ktxVulkanTexture_subAllocatorFreeMemFuncPtr)(uint64_t allocId); -typedef void (*ktxVulkanTexture_queueLockFuncPtr)(); -typedef void (*ktxVulkanTexture_queueUnlockFuncPtr)(); +typedef void (*ktxVulkanTexture_queueLockFuncPtr)(VkQueue queueToLock); +typedef void (*ktxVulkanTexture_queueUnlockFuncPtr)(VkQueue queueToUnlock); /** * @class ktxVulkanTexture_subAllocatorCallbacks @@ -244,6 +244,7 @@ ktxVulkanDeviceInfo_Destroy(ktxVulkanDeviceInfo* This); * @~English * @brief Struct that contains callbacks necessary for guarding access to the supplied queue in vdi. * + * As a prerequisite, a per-thread VDI is necessary for thread-safety. * Needs to be used in conjunction with suballocator callbacks that are also thread-safe. * Together they can make UploadEx fully thread-safe in an efficient manner. * The suballocator callbacks need to guard around memory objects and if sparse binding diff --git a/lib/src/vkloader.c b/lib/src/vkloader.c index 3352ac5bf3..84bc156581 100644 --- a/lib/src/vkloader.c +++ b/lib/src/vkloader.c @@ -799,7 +799,8 @@ linearTilingPadCallback(int miplevel, int face, * (or additionally queue if sparse binding support * is added) and external suballocation managements * objects, it can make UploadEx fully thread-safe - * and efficiently so. + * and efficiently so. This is assuming that a + * per-thread VDI is used as well. * * @return KTX_SUCCESS on success, other KTX_* enum values on error. * @@ -1172,10 +1173,21 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD (ktx_size_t)memAllocInfo.allocationSize); if (kResult != KTX_SUCCESS) { + // TODO: This should be a common clean-up path for all failure cases, + // complete with checks for partial initialization. + VK_CHECK_RESULT(vdi->vkFuncs.vkEndCommandBuffer(vdi->cmdBuffer)); + free(copyRegions); if (!useSuballocator) + { vdi->vkFuncs.vkUnmapMemory(vdi->device, stagingMemory); + vdi->vkFuncs.vkFreeMemory(vdi->device, stagingMemory, vdi->pAllocator); + } else + { subAllocatorCallbacks->memoryUnmapFuncPtr(stagingAllocId, 0ull); + subAllocatorCallbacks->freeMemFuncPtr(stagingAllocId); + } + vdi->vkFuncs.vkDestroyBuffer(vdi->device, stagingBuffer, vdi->pAllocator); return kResult; } } @@ -1314,10 +1326,10 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD submitInfo.commandBufferCount = 1; submitInfo.pCommandBuffers = &vdi->cmdBuffer; - if (useQueueMutex) queueMutexCallbacks->queueLockFuncPtr(); + if (useQueueMutex) queueMutexCallbacks->queueLockFuncPtr(vdi->queue); VK_CHECK_RESULT( vdi->vkFuncs.vkQueueSubmit(vdi->queue, 1, &submitInfo, copyFence)); - if (useQueueMutex) queueMutexCallbacks->queueUnlockFuncPtr(); + if (useQueueMutex) queueMutexCallbacks->queueUnlockFuncPtr(vdi->queue); VK_CHECK_RESULT( vdi->vkFuncs.vkWaitForFences(vdi->device, 1, ©Fence, @@ -1468,10 +1480,10 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD submitInfo.commandBufferCount = 1; submitInfo.pCommandBuffers = &vdi->cmdBuffer; - if (useQueueMutex) queueMutexCallbacks->queueLockFuncPtr(); + if (useQueueMutex) queueMutexCallbacks->queueLockFuncPtr(vdi->queue); VK_CHECK_RESULT(vdi->vkFuncs.vkQueueSubmit(vdi->queue, 1, &submitInfo, nullFence)); VK_CHECK_RESULT(vdi->vkFuncs.vkQueueWaitIdle(vdi->queue)); - if (useQueueMutex) queueMutexCallbacks->queueUnlockFuncPtr(); + if (useQueueMutex) queueMutexCallbacks->queueUnlockFuncPtr(vdi->queue); } return KTX_SUCCESS; } diff --git a/tests/loadtests/vkloadtests/Texture.cpp b/tests/loadtests/vkloadtests/Texture.cpp index 0ab93f8437..e2a8f99e7e 100644 --- a/tests/loadtests/vkloadtests/Texture.cpp +++ b/tests/loadtests/vkloadtests/Texture.cpp @@ -136,7 +136,7 @@ Texture::Texture(VulkanContext& vkctx, }); uploaderThread.join(); // We really have no choice here. This crucible/test-environment only loads one texture at a time. // Additionally `QUEUE_GUARD_CALLBACKS::queueAccessGuard` and `QUEUE_GUARD_CALLBACKS::memoryAccessGuard` would - // need to be re-used when arena `VkDeviceMemory`s or the same `VkQueue`s are being accessed simultaneously. + // need to be re-used when arena allocations or the same `VkQueue`s are being accessed simultaneously. } else { diff --git a/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp b/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp index e09a75d13a..476a500d22 100644 --- a/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp +++ b/tests/loadtests/vkloadtests/VulkanLoadTestSample.cpp @@ -136,7 +136,6 @@ namespace VMA_CALLBACKS std::mutex memoryAccessGuard; uint64_t AllocMemCWrapperGuarded(VkMemoryAllocateInfo* allocInfo, VkMemoryRequirements* memReq, uint64_t* numPages) { - uint64_t allocId = mt64(); VmaAllocationCreateInfo pCreateInfo = {}; if ((cachedDevMemProps.memoryTypes[allocInfo->memoryTypeIndex].propertyFlags & VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT) || (cachedDevMemProps.memoryTypes[allocInfo->memoryTypeIndex].propertyFlags & VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)) @@ -157,8 +156,10 @@ namespace VMA_CALLBACKS return 0ull; } + uint64_t allocId; { std::lock_guard lk(memoryAccessGuard); + allocId = mt64(); AllocMemCWrapperDirectory[allocId].allocation = allocation; AllocMemCWrapperDirectory[allocId].mapSize = memReq->size; } @@ -189,15 +190,23 @@ namespace VMA_CALLBACKS VkResult MapMemoryCWrapperGuarded(uint64_t allocId, uint64_t, VkDeviceSize* mapLength, void** dataPtr) { - std::lock_guard lk(memoryAccessGuard); - *mapLength = AllocMemCWrapperDirectory[allocId].mapSize; - return vmaMapMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation, dataPtr); + VmaAllocation allocation; + { + std::lock_guard lk(memoryAccessGuard); + allocation = AllocMemCWrapperDirectory[allocId].allocation; + *mapLength = AllocMemCWrapperDirectory[allocId].mapSize; + } + return vmaMapMemory(vmaAllocator, allocation, dataPtr); } void UnmapMemoryCWrapperGuarded(uint64_t allocId, uint64_t) { - std::lock_guard lk(memoryAccessGuard); - vmaUnmapMemory(vmaAllocator, AllocMemCWrapperDirectory[allocId].allocation); + VmaAllocation allocation; + { + std::lock_guard lk(memoryAccessGuard); + allocation = AllocMemCWrapperDirectory[allocId].allocation; + } + vmaUnmapMemory(vmaAllocator, allocation); } void FreeMemCWrapperGuarded(uint64_t allocId) @@ -215,15 +224,14 @@ namespace VMA_CALLBACKS namespace QUEUE_GUARD_CALLBACKS { std::mutex queueAccessGuard; - - void LockQueue() + void LockQueue(VkQueue) { - queueAccessGuard.lock(); + queueAccessGuard.lock(); // Only a single queue at the moment } - void UnlockQueue() + void UnlockQueue(VkQueue) { - queueAccessGuard.unlock(); + queueAccessGuard.unlock(); // Only a single queue at the moment } } diff --git a/tests/loadtests/vkloadtests/VulkanLoadTestSample.h b/tests/loadtests/vkloadtests/VulkanLoadTestSample.h index 7974d88a79..245f94d98c 100644 --- a/tests/loadtests/vkloadtests/VulkanLoadTestSample.h +++ b/tests/loadtests/vkloadtests/VulkanLoadTestSample.h @@ -44,8 +44,8 @@ namespace VMA_CALLBACKS namespace QUEUE_GUARD_CALLBACKS { - void LockQueue(); - void UnlockQueue(); + void LockQueue(VkQueue queue); + void UnlockQueue(VkQueue queue); } #define ARRAY_LEN(a) (sizeof(a) / sizeof(a[0])) From 130a53690c80dce933b883e73f455bd377538064 Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Mon, 17 Aug 2026 18:43:45 -0400 Subject: [PATCH 09/11] * Common path for cleaning up partially initialized resources during a failed texture upload. --- lib/src/vkloader.c | 97 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 80 insertions(+), 17 deletions(-) diff --git a/lib/src/vkloader.c b/lib/src/vkloader.c index 84bc156581..93ed183751 100644 --- a/lib/src/vkloader.c +++ b/lib/src/vkloader.c @@ -735,6 +735,38 @@ linearTilingPadCallback(int miplevel, int face, return KTX_SUCCESS; } +/** + * @internal + * @~English + * @brief Clean-up call for texture upload failures. + * + * Frees partially initialized resources before exiting. + */ +void +freeUploadResources(ktxVulkanDeviceInfo* vdi, VkBufferImageCopy** copyRegions, bool isRecording, + bool hasAllocated, bool hasCreatedBuffer, bool useSuballocator, + VkDeviceMemory stagingMemory, uint64_t stagingAllocId, + ktxVulkanTexture_subAllocatorCallbacks* subAllocatorCallbacks, + VkBuffer stagingBuffer) { + if (isRecording && vdi) { + VK_CHECK_RESULT(vdi->vkFuncs.vkEndCommandBuffer(vdi->cmdBuffer)); + } + if (copyRegions && *copyRegions) { + free(*copyRegions); + *copyRegions = NULL; + } + if (hasAllocated) { + if (!useSuballocator && vdi) { + vdi->vkFuncs.vkFreeMemory(vdi->device, stagingMemory, vdi->pAllocator); + } else if (subAllocatorCallbacks) { + subAllocatorCallbacks->freeMemFuncPtr(stagingAllocId); + } + } + if (hasCreatedBuffer && vdi) { + vdi->vkFuncs.vkDestroyBuffer(vdi->device, stagingBuffer, vdi->pAllocator); + } +} + /** * @memberof ktxTexture * @~English @@ -870,22 +902,32 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD subAllocatorCallbacks->memoryUnmapFuncPtr && subAllocatorCallbacks->freeMemFuncPtr) useSuballocator = true; - else + else { + freeUploadResources(vdi, NULL, false, false, false, useSuballocator, VK_NULL_HANDLE, 0, + subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_INVALID_VALUE; + } } if (queueMutexCallbacks) { if (queueMutexCallbacks->queueLockFuncPtr && queueMutexCallbacks->queueUnlockFuncPtr) useQueueMutex = true; - else + else { + freeUploadResources(vdi, NULL, false, false, false, useSuballocator, VK_NULL_HANDLE, 0, + subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_INVALID_VALUE; + } } if (!vdi || !This || !vkTexture) { + freeUploadResources(vdi, NULL, false, false, false, useSuballocator, VK_NULL_HANDLE, 0, + subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_INVALID_VALUE; } if (!This->pData && !ktxTexture_isActiveStream(This)) { + freeUploadResources(vdi, NULL, false, false, false, useSuballocator, VK_NULL_HANDLE, 0, + subAllocatorCallbacks, VK_NULL_HANDLE); /* Nothing to upload. */ return KTX_INVALID_OPERATION; } @@ -928,6 +970,8 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD vkFormat = ktxTexture_GetVkFormat(This); if (vkFormat == VK_FORMAT_UNDEFINED) { + freeUploadResources(vdi, NULL, false, false, false, useSuballocator, VK_NULL_HANDLE, 0, + subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_INVALID_OPERATION; } @@ -948,9 +992,13 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD createFlags, &imageFormatProperties); if (vResult == VK_ERROR_FORMAT_NOT_SUPPORTED) { + freeUploadResources(vdi, NULL, false, false, false, useSuballocator, VK_NULL_HANDLE, 0, + subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_INVALID_OPERATION; } if (This->numLayers > imageFormatProperties.maxArrayLayers) { + freeUploadResources(vdi, NULL, false, false, false, useSuballocator, VK_NULL_HANDLE, 0, + subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_INVALID_OPERATION; } @@ -984,6 +1032,8 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD } if (numImageLevels > imageFormatProperties.maxMipLevels) { + freeUploadResources(vdi, NULL, false, false, false, useSuballocator, VK_NULL_HANDLE, 0, + subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_INVALID_OPERATION; } @@ -1032,7 +1082,7 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD if (tiling == VK_IMAGE_TILING_OPTIMAL) { // Create a host-visible staging buffer that contains the raw image data - VkBuffer stagingBuffer; + VkBuffer stagingBuffer = VK_NULL_HANDLE; VkDeviceMemory stagingMemory = VK_NULL_HANDLE; VkBufferImageCopy* copyRegions; VkDeviceSize textureSize; @@ -1084,6 +1134,8 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD copyRegions = (VkBufferImageCopy*)malloc(sizeof(VkBufferImageCopy) * numCopyRegions); if (copyRegions == NULL) { + freeUploadResources(vdi, ©Regions, true, false, false, useSuballocator, + stagingMemory, 0, subAllocatorCallbacks, stagingBuffer); return KTX_OUT_OF_MEMORY; } @@ -1113,7 +1165,9 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD vResult = vdi->vkFuncs.vkAllocateMemory(vdi->device, &memAllocInfo, vdi->pAllocator, &stagingMemory); if (vResult != VK_SUCCESS) { - free(copyRegions); + freeUploadResources(vdi, ©Regions, true, false, true, useSuballocator, + stagingMemory, stagingAllocId, subAllocatorCallbacks, + stagingBuffer); return KTX_OUT_OF_MEMORY; } VK_CHECK_RESULT( @@ -1129,11 +1183,15 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD uint64_t numPages = 0ull; stagingAllocId = subAllocatorCallbacks->allocMemFuncPtr(&memAllocInfo, &memReqs, &numPages); if (stagingAllocId == 0ull) { - free(copyRegions); + freeUploadResources(vdi, ©Regions, true, false, true, useSuballocator, + stagingMemory, stagingAllocId, subAllocatorCallbacks, + stagingBuffer); return KTX_OUT_OF_MEMORY; } if (numPages > 1ull) { // Sparse binding of KTX textures is unsupported for the moment - free(copyRegions); + freeUploadResources(vdi, ©Regions, true, true, true, useSuballocator, + stagingMemory, stagingAllocId, subAllocatorCallbacks, + stagingBuffer); return KTX_UNSUPPORTED_FEATURE; } VK_CHECK_RESULT( @@ -1173,21 +1231,13 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD (ktx_size_t)memAllocInfo.allocationSize); if (kResult != KTX_SUCCESS) { - // TODO: This should be a common clean-up path for all failure cases, - // complete with checks for partial initialization. - VK_CHECK_RESULT(vdi->vkFuncs.vkEndCommandBuffer(vdi->cmdBuffer)); - free(copyRegions); if (!useSuballocator) - { vdi->vkFuncs.vkUnmapMemory(vdi->device, stagingMemory); - vdi->vkFuncs.vkFreeMemory(vdi->device, stagingMemory, vdi->pAllocator); - } else - { subAllocatorCallbacks->memoryUnmapFuncPtr(stagingAllocId, 0ull); - subAllocatorCallbacks->freeMemFuncPtr(stagingAllocId); - } - vdi->vkFuncs.vkDestroyBuffer(vdi->device, stagingBuffer, vdi->pAllocator); + freeUploadResources(vdi, ©Regions, true, true, true, useSuballocator, + stagingMemory, stagingAllocId, subAllocatorCallbacks, + stagingBuffer); return kResult; } } @@ -1261,9 +1311,16 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD uint64_t numPages = 0ull; vkTexture->allocationId = subAllocatorCallbacks->allocMemFuncPtr(&memAllocInfo, &memReqs, &numPages); if (vkTexture->allocationId == 0ull) { + freeUploadResources(vdi, ©Regions, true, true, true, useSuballocator, + stagingMemory, stagingAllocId, subAllocatorCallbacks, + stagingBuffer); return KTX_OUT_OF_MEMORY; } if(numPages > 1ull) { // Sparse binding of KTX textures is unsupported for the moment + freeUploadResources(vdi, ©Regions, true, true, true, useSuballocator, + stagingMemory, stagingAllocId, subAllocatorCallbacks, + stagingBuffer); + subAllocatorCallbacks->freeMemFuncPtr(vkTexture->allocationId); return KTX_UNSUPPORTED_FEATURE; } VK_CHECK_RESULT( @@ -1393,6 +1450,8 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD vResult = vdi->vkFuncs.vkAllocateMemory(vdi->device, &memAllocInfo, vdi->pAllocator, &mappableMemory); if (vResult != VK_SUCCESS) { + freeUploadResources(vdi, NULL, true, false, false, useSuballocator, mappableMemory, + vkTexture->allocationId, subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_OUT_OF_MEMORY; } VK_CHECK_RESULT( @@ -1403,9 +1462,13 @@ ktxTexture_VkUploadEx_WithSuballocatorAndQueueGuard(ktxTexture* This, ktxVulkanD uint64_t numPages = 0ull; vkTexture->allocationId = subAllocatorCallbacks->allocMemFuncPtr(&memAllocInfo, &memReqs, &numPages); if (vkTexture->allocationId == 0ull) { + freeUploadResources(vdi, NULL, true, false, false, useSuballocator, mappableMemory, + vkTexture->allocationId, subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_OUT_OF_MEMORY; } if (numPages > 1ull) { // Sparse binding of KTX textures is unsupported for the moment + freeUploadResources(vdi, NULL, true, true, false, useSuballocator, mappableMemory, + vkTexture->allocationId, subAllocatorCallbacks, VK_NULL_HANDLE); return KTX_UNSUPPORTED_FEATURE; } VK_CHECK_RESULT( From 6da6d8eb3cf1bf9f076fdd8b1ee6b825c64bc83c Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Sat, 22 Aug 2026 21:26:17 -0400 Subject: [PATCH 10/11] * Better comments with more emphasis on the necessity of a per-thread VDI for threaded uploads. --- lib/src/vkloader.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/src/vkloader.c b/lib/src/vkloader.c index 93ed183751..d8864725a6 100644 --- a/lib/src/vkloader.c +++ b/lib/src/vkloader.c @@ -810,7 +810,11 @@ freeUploadResources(ktxVulkanDeviceInfo* vdi, VkBufferImageCopy** copyRegions, b * @param[in] This pointer to the ktxTexture from which to upload. * @param [in] vdi pointer to a ktxVulkanDeviceInfo structure providing * information about the Vulkan device onto which to - * load the texture. + * load the texture. If subAllocatorCallbacks and + * queueMutexCallbacks are provided, this operation + * is assumed to be taking place in a threaded + * environment requiring a per-thread value for + * this parameter. * @param [in,out] vkTexture pointer to a ktxVulkanTexture structure into which * the function writes information about the created * VkImage. @@ -829,10 +833,10 @@ freeUploadResources(ktxVulkanDeviceInfo* vdi, VkBufferImageCopy** copyRegions, b * @param [in] queueMutexCallbacks If used in conjunction with suballocator callbacks * that guard against simultaneous access to memory * (or additionally queue if sparse binding support - * is added) and external suballocation managements - * objects, it can make UploadEx fully thread-safe - * and efficiently so. This is assuming that a - * per-thread VDI is used as well. + * is added) and external suballocation management + * objects, it can make this function fully + * thread-safe and efficiently so. As noted, a + * per-thread VDI is a necessity when this is supplied. * * @return KTX_SUCCESS on success, other KTX_* enum values on error. * From 4f7d8577027e6003126ac7cdb33456c092ab6c3d Mon Sep 17 00:00:00 2001 From: Baktash Abdollah-shamshir-saz Date: Sat, 29 Aug 2026 16:54:32 -0400 Subject: [PATCH 11/11] * Further clarifications on `VDI` and `queueMutexCallbacks` parameters based on feedback. --- lib/src/vkloader.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/src/vkloader.c b/lib/src/vkloader.c index d8864725a6..c2fea87c3b 100644 --- a/lib/src/vkloader.c +++ b/lib/src/vkloader.c @@ -810,11 +810,11 @@ freeUploadResources(ktxVulkanDeviceInfo* vdi, VkBufferImageCopy** copyRegions, b * @param[in] This pointer to the ktxTexture from which to upload. * @param [in] vdi pointer to a ktxVulkanDeviceInfo structure providing * information about the Vulkan device onto which to - * load the texture. If subAllocatorCallbacks and - * queueMutexCallbacks are provided, this operation - * is assumed to be taking place in a threaded - * environment requiring a per-thread value for - * this parameter. + * load the texture. If calling this function from + * multiple threads, a per-thread value is required for + * this parameter. This will be the case when + * subAllocatorCallbacks and queueMutexCallbacks are + * provided but normally a single thread is used. * @param [in,out] vkTexture pointer to a ktxVulkanTexture structure into which * the function writes information about the created * VkImage. @@ -830,13 +830,16 @@ freeUploadResources(ktxVulkanDeviceInfo* vdi, VkBufferImageCopy** copyRegions, b * They use a uint64_t stored in the @c allocationId * field of the structure pointed at by @a vkTexture * to reference allocated page(s). - * @param [in] queueMutexCallbacks If used in conjunction with suballocator callbacks - * that guard against simultaneous access to memory - * (or additionally queue if sparse binding support - * is added) and external suballocation management - * objects, it can make this function fully - * thread-safe and efficiently so. As noted, a - * per-thread VDI is a necessity when this is supplied. + * @param [in] queueMutexCallbacks Pointer to a set of queue-mutex callbacks which + * the function uses to protect accesses to the queue. + * When used in conjunction with suballocator callbacks + * that guard against simultaneous access to memory and + * suballocation management objects, these will make the + * function fully thread-safe and efficiently so. + * Note that if sparse binding support is added, the + * suballocator callbacks must also guard against + * simultaneous access to the queue. Note also that a + * per-thread VDI is necessary when using these callbacks. * * @return KTX_SUCCESS on success, other KTX_* enum values on error. *