From e1a41c1c97b3d86293b17a13d54bc3b454ddb179 Mon Sep 17 00:00:00 2001
From: Evan Hemsley <[EMAIL REDACTED]>
Date: Tue, 29 Apr 2025 15:43:28 -0700
Subject: [PATCH] GPU Vulkan: Add locks for layout object lookups (#12924)
---
src/gpu/vulkan/SDL_gpu_vulkan.c | 37 ++++++++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/src/gpu/vulkan/SDL_gpu_vulkan.c b/src/gpu/vulkan/SDL_gpu_vulkan.c
index 5de1e62b8cfbe..a1e234d6d2c30 100644
--- a/src/gpu/vulkan/SDL_gpu_vulkan.c
+++ b/src/gpu/vulkan/SDL_gpu_vulkan.c
@@ -1178,6 +1178,9 @@ struct VulkanRenderer
SDL_Mutex *acquireUniformBufferLock;
SDL_Mutex *renderPassFetchLock;
SDL_Mutex *framebufferFetchLock;
+ SDL_Mutex *graphicsPipelineLayoutFetchLock;
+ SDL_Mutex *computePipelineLayoutFetchLock;
+ SDL_Mutex *descriptorSetLayoutFetchLock;
SDL_Mutex *windowLock;
Uint8 defragInProgress;
@@ -3698,10 +3701,13 @@ static DescriptorSetLayout *VULKAN_INTERNAL_FetchDescriptorSetLayout(
key.writeStorageBufferCount = writeStorageBufferCount;
key.uniformBufferCount = uniformBufferCount;
+ SDL_LockMutex(renderer->descriptorSetLayoutFetchLock);
+
if (SDL_FindInHashTable(
renderer->descriptorSetLayoutHashTable,
(const void *)&key,
(const void **)&layout)) {
+ SDL_UnlockMutex(renderer->descriptorSetLayoutFetchLock);
return layout;
}
@@ -3784,7 +3790,10 @@ static DescriptorSetLayout *VULKAN_INTERNAL_FetchDescriptorSetLayout(
NULL,
&descriptorSetLayout);
- CHECK_VULKAN_ERROR_AND_RETURN(vulkanResult, vkCreateDescriptorSetLayout, NULL);
+ if (vulkanResult != VK_SUCCESS) {
+ SDL_UnlockMutex(renderer->descriptorSetLayoutFetchLock);
+ CHECK_VULKAN_ERROR_AND_RETURN(vulkanResult, vkCreateDescriptorSetLayout, NULL);
+ }
layout = SDL_malloc(sizeof(DescriptorSetLayout));
layout->descriptorSetLayout = descriptorSetLayout;
@@ -3806,6 +3815,7 @@ static DescriptorSetLayout *VULKAN_INTERNAL_FetchDescriptorSetLayout(
(const void *)allocedKey,
(const void *)layout, true);
+ SDL_UnlockMutex(renderer->descriptorSetLayoutFetchLock);
return layout;
}
@@ -3826,10 +3836,14 @@ static VulkanGraphicsPipelineResourceLayout *VULKAN_INTERNAL_FetchGraphicsPipeli
key.fragmentStorageTextureCount = fragmentShader->numStorageTextures;
key.fragmentStorageBufferCount = fragmentShader->numStorageBuffers;
key.fragmentUniformBufferCount = fragmentShader->numUniformBuffers;
+
+ SDL_LockMutex(renderer->graphicsPipelineLayoutFetchLock);
+
if (SDL_FindInHashTable(
renderer->graphicsPipelineResourceLayoutHashTable,
(const void *)&key,
(const void **)&pipelineResourceLayout)) {
+ SDL_UnlockMutex(renderer->graphicsPipelineLayoutFetchLock);
return pipelineResourceLayout;
}
@@ -3912,6 +3926,7 @@ static VulkanGraphicsPipelineResourceLayout *VULKAN_INTERNAL_FetchGraphicsPipeli
if (vulkanResult != VK_SUCCESS) {
VULKAN_INTERNAL_DestroyGraphicsPipelineResourceLayout(renderer, pipelineResourceLayout);
+ SDL_UnlockMutex(renderer->graphicsPipelineLayoutFetchLock);
CHECK_VULKAN_ERROR_AND_RETURN(vulkanResult, vkCreatePipelineLayout, NULL);
}
@@ -3923,6 +3938,7 @@ static VulkanGraphicsPipelineResourceLayout *VULKAN_INTERNAL_FetchGraphicsPipeli
(const void *)allocedKey,
(const void *)pipelineResourceLayout, true);
+ SDL_UnlockMutex(renderer->graphicsPipelineLayoutFetchLock);
return pipelineResourceLayout;
}
@@ -3941,10 +3957,13 @@ static VulkanComputePipelineResourceLayout *VULKAN_INTERNAL_FetchComputePipeline
key.readWriteStorageBufferCount = createinfo->num_readwrite_storage_buffers;
key.uniformBufferCount = createinfo->num_uniform_buffers;
+ SDL_LockMutex(renderer->computePipelineLayoutFetchLock);
+
if (SDL_FindInHashTable(
renderer->computePipelineResourceLayoutHashTable,
(const void *)&key,
(const void **)&pipelineResourceLayout)) {
+ SDL_UnlockMutex(renderer->computePipelineLayoutFetchLock);
return pipelineResourceLayout;
}
@@ -4013,6 +4032,7 @@ static VulkanComputePipelineResourceLayout *VULKAN_INTERNAL_FetchComputePipeline
if (vulkanResult != VK_SUCCESS) {
VULKAN_INTERNAL_DestroyComputePipelineResourceLayout(renderer, pipelineResourceLayout);
+ SDL_UnlockMutex(renderer->computePipelineLayoutFetchLock);
CHECK_VULKAN_ERROR_AND_RETURN(vulkanResult, vkCreatePipelineLayout, NULL);
}
@@ -4024,6 +4044,7 @@ static VulkanComputePipelineResourceLayout *VULKAN_INTERNAL_FetchComputePipeline
(const void *)allocedKey,
(const void *)pipelineResourceLayout, true);
+ SDL_UnlockMutex(renderer->computePipelineLayoutFetchLock);
return pipelineResourceLayout;
}
@@ -4914,6 +4935,9 @@ static void VULKAN_DestroyDevice(
SDL_DestroyMutex(renderer->acquireUniformBufferLock);
SDL_DestroyMutex(renderer->renderPassFetchLock);
SDL_DestroyMutex(renderer->framebufferFetchLock);
+ SDL_DestroyMutex(renderer->graphicsPipelineLayoutFetchLock);
+ SDL_DestroyMutex(renderer->computePipelineLayoutFetchLock);
+ SDL_DestroyMutex(renderer->descriptorSetLayoutFetchLock);
SDL_DestroyMutex(renderer->windowLock);
renderer->vkDestroyDevice(renderer->logicalDevice, NULL);
@@ -11761,6 +11785,9 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
renderer->acquireUniformBufferLock = SDL_CreateMutex();
renderer->renderPassFetchLock = SDL_CreateMutex();
renderer->framebufferFetchLock = SDL_CreateMutex();
+ renderer->graphicsPipelineLayoutFetchLock = SDL_CreateMutex();
+ renderer->computePipelineLayoutFetchLock = SDL_CreateMutex();
+ renderer->descriptorSetLayoutFetchLock = SDL_CreateMutex();
renderer->windowLock = SDL_CreateMutex();
/*
@@ -11821,7 +11848,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
renderer->renderPassHashTable = SDL_CreateHashTable(
0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful.
- false, // manually synchronized due to timing
+ false, // manually synchronized due to lookup timing
VULKAN_INTERNAL_RenderPassHashFunction,
VULKAN_INTERNAL_RenderPassHashKeyMatch,
VULKAN_INTERNAL_RenderPassHashDestroy,
@@ -11837,7 +11864,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
renderer->graphicsPipelineResourceLayoutHashTable = SDL_CreateHashTable(
0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful.
- true, // thread-safe
+ false, // manually synchronized due to lookup timing
VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashFunction,
VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashKeyMatch,
VULKAN_INTERNAL_GraphicsPipelineResourceLayoutHashDestroy,
@@ -11845,7 +11872,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
renderer->computePipelineResourceLayoutHashTable = SDL_CreateHashTable(
0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful.
- true, // thread-safe
+ false, // manually synchronized due to lookup timing
VULKAN_INTERNAL_ComputePipelineResourceLayoutHashFunction,
VULKAN_INTERNAL_ComputePipelineResourceLayoutHashKeyMatch,
VULKAN_INTERNAL_ComputePipelineResourceLayoutHashDestroy,
@@ -11853,7 +11880,7 @@ static SDL_GPUDevice *VULKAN_CreateDevice(bool debugMode, bool preferLowPower, S
renderer->descriptorSetLayoutHashTable = SDL_CreateHashTable(
0, // !!! FIXME: a real guess here, for a _minimum_ if not a maximum, could be useful.
- true, // thread-safe
+ false, // manually synchronized due to lookup timing
VULKAN_INTERNAL_DescriptorSetLayoutHashFunction,
VULKAN_INTERNAL_DescriptorSetLayoutHashKeyMatch,
VULKAN_INTERNAL_DescriptorSetLayoutHashDestroy,