Efficient thread-safe ktxTexture_VkUploadEx_WithSuballocator() with queue guard callbacks

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,34 @@ 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 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 {
+    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 +282,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 +306,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,

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;
@@ -1156,7 +1171,13 @@ ktxTexture_VkUploadEx_WithSuballocator(ktxTexture* This, ktxVulkanDeviceInfo* vd
                                       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.
@@ -1293,8 +1314,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, &copyFence,
@@ -1445,12 +1468,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 +1527,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 +1608,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.

tests/loadtests/vkloadtests/Texture.cpp

@@ -30,6 +30,7 @@
 #include <assert.h>
 #include <exception>
 #include <vector>
+#include <thread>
 
 #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<VkImageTiling>(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<VkImageTiling>(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 when arena `VkDeviceMemory`s or the same `VkQueue`s are being accessed simultaneously.
+        }
+        else
+        {
+            ktxresult = ktxTexture_VkUploadEx_WithSuballocator(kTexture, &vdi, &texture,
+                                                               static_cast<VkImageTiling>(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}
     };

tests/loadtests/vkloadtests/Texture.h

@@ -38,13 +38,19 @@ class Texture : public VulkanLoadTestSample
         No = 0,
         Yes
     };
+    enum class UseQueueGuard
+    {
+        No = 0,
+        Yes
+    };
 
     std::string filename;
     ktxVulkanTexture texture;
     vk::Sampler sampler;
     vk::ImageView imageView;
     vk::ImageTiling tiling;
     UseSuballocator useSubAlloc;
+    UseQueueGuard useQueueGuard;
     vk::ComponentMapping swizzle;
 
     struct {