From b60c58b1bc98ea823daf61901bf10ec752d731a2 Mon Sep 17 00:00:00 2001 From: cosmonaut Date: Sat, 13 Jan 2024 01:45:08 +0000 Subject: [PATCH] Fix potential sync hazards (#49) We now do certain image layout transitions in the render pass instead of a barrier in EndRenderPass. There is also an additional barrier on buffer uploads to prevent write-after-write hazard. It's a kludge on the fact that we're only tracking the most recent resource access. Reviewed-on: https://gitea.moonside.games/MoonsideGames/Refresh/pulls/49 --- src/Refresh_Driver_Vulkan.c | 179 ++++++++++++++++++++++++++---------- 1 file changed, 129 insertions(+), 50 deletions(-) diff --git a/src/Refresh_Driver_Vulkan.c b/src/Refresh_Driver_Vulkan.c index 3e00de1..f9639fa 100644 --- a/src/Refresh_Driver_Vulkan.c +++ b/src/Refresh_Driver_Vulkan.c @@ -1,4 +1,4 @@ -/* Refresh - XNA-inspired 3D Graphics Library with modern capabilities +/* Refresh - XNA-inspired 3D Graphics Library with modern capabilities * * Copyright (c) 2020 Evan Hemsley * @@ -561,8 +561,8 @@ static const VulkanResourceAccessInfo AccessMap[RESOURCE_ACCESS_TYPES_COUNT] = /* RESOURCE_ACCESS_ANY_SHADER_READ_SAMPLED_IMAGE */ { - VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, - VK_ACCESS_SHADER_READ_BIT, + VK_PIPELINE_STAGE_VERTEX_SHADER_BIT | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT | VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT, + VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_INPUT_ATTACHMENT_READ_BIT, VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL }, @@ -1015,6 +1015,7 @@ typedef struct RenderPassHash uint32_t colorAttachmentCount; RenderPassDepthStencilTargetDescription depthStencilTargetDescription; Refresh_SampleCount colorAttachmentSampleCount; + VkImageLayout finalLayout; } RenderPassHash; typedef struct RenderPassHashMap @@ -1046,6 +1047,11 @@ static inline uint8_t RenderPassHash_Compare( return 0; } + if (a->finalLayout != b->finalLayout) + { + return 0; + } + for (i = 0; i < a->colorAttachmentCount; i += 1) { if (a->colorTargetDescriptions[i].format != b->colorTargetDescriptions[i].format) @@ -5955,6 +5961,41 @@ static VulkanRenderTarget* VULKAN_INTERNAL_FetchRenderTarget( return renderTarget; } +static VkImageLayout VULKAN_INTERNAL_GetRenderPassFinalLayout( + VulkanTexture *texture +) { + VkImageLayout finalLayout; + + if (IsDepthFormat(texture->format)) + { + if (texture->usageFlags & VK_IMAGE_USAGE_SAMPLED_BIT) + { + finalLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; + } + else + { + finalLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; + } + } + else + { + if (texture->usageFlags & VK_IMAGE_USAGE_SAMPLED_BIT) + { + finalLayout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; + } + else if (texture->usageFlags & VK_IMAGE_USAGE_STORAGE_BIT) + { + finalLayout = VK_IMAGE_LAYOUT_GENERAL; + } + else + { + finalLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; + } + } + + return finalLayout; +} + static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( VulkanRenderer *renderer, VulkanCommandBuffer *commandBuffer, @@ -5969,6 +6010,7 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( VkAttachmentReference depthStencilAttachmentReference; VkRenderPassCreateInfo renderPassCreateInfo; VkSubpassDescription subpass; + VkSubpassDependency dep[2]; VkRenderPass renderPass; uint32_t i; @@ -6021,7 +6063,7 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( attachmentDescriptions[attachmentDescriptionCount].initialLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; attachmentDescriptions[attachmentDescriptionCount].finalLayout = - VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; + VULKAN_INTERNAL_GetRenderPassFinalLayout(texture); resolveReferences[resolveReferenceCount].attachment = attachmentDescriptionCount; @@ -6077,8 +6119,7 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( attachmentDescriptions[attachmentDescriptionCount].initialLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; attachmentDescriptions[attachmentDescriptionCount].finalLayout = - VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; - + VULKAN_INTERNAL_GetRenderPassFinalLayout(texture); colorAttachmentReferences[colorAttachmentReferenceCount].attachment = attachmentDescriptionCount; colorAttachmentReferences[colorAttachmentReferenceCount].layout = @@ -6126,7 +6167,7 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( attachmentDescriptions[attachmentDescriptionCount].initialLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; attachmentDescriptions[attachmentDescriptionCount].finalLayout = - VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; + VULKAN_INTERNAL_GetRenderPassFinalLayout(texture); depthStencilAttachmentReference.attachment = attachmentDescriptionCount; @@ -6148,6 +6189,37 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( subpass.pResolveAttachments = NULL; } + const VkPipelineStageFlags graphicsStages = 0 + | VK_PIPELINE_STAGE_DRAW_INDIRECT_BIT + | VK_PIPELINE_STAGE_VERTEX_INPUT_BIT + | VK_PIPELINE_STAGE_VERTEX_SHADER_BIT + | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT + | VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT + | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT + | VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT + ; + const VkPipelineStageFlags outsideStages = 0 + | graphicsStages + | VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT + | VK_PIPELINE_STAGE_TRANSFER_BIT + ; + + dep[0].srcSubpass = VK_SUBPASS_EXTERNAL; + dep[0].dstSubpass = 0; + dep[0].srcStageMask = outsideStages; + dep[0].dstStageMask = graphicsStages; + dep[0].srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT; + dep[0].dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT; + dep[0].dependencyFlags = 0; + + dep[1].srcSubpass = 0; + dep[1].dstSubpass = VK_SUBPASS_EXTERNAL; + dep[1].srcStageMask = graphicsStages; + dep[1].dstStageMask = outsideStages; + dep[1].srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT; + dep[1].dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT; + dep[1].dependencyFlags = 0; + renderPassCreateInfo.sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO; renderPassCreateInfo.pNext = NULL; renderPassCreateInfo.flags = 0; @@ -6155,8 +6227,8 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( renderPassCreateInfo.attachmentCount = attachmentDescriptionCount; renderPassCreateInfo.subpassCount = 1; renderPassCreateInfo.pSubpasses = &subpass; - renderPassCreateInfo.dependencyCount = 0; - renderPassCreateInfo.pDependencies = NULL; + renderPassCreateInfo.dependencyCount = 2; + renderPassCreateInfo.pDependencies = dep; vulkanResult = renderer->vkCreateRenderPass( renderer->logicalDevice, @@ -6185,6 +6257,7 @@ static VkRenderPass VULKAN_INTERNAL_CreateTransientRenderPass( VkAttachmentReference depthStencilAttachmentReference; Refresh_ColorAttachmentDescription attachmentDescription; VkSubpassDescription subpass; + VkSubpassDependency dep[2]; VkRenderPassCreateInfo renderPassCreateInfo; VkRenderPass renderPass; VkResult result; @@ -6195,6 +6268,7 @@ static VkRenderPass VULKAN_INTERNAL_CreateTransientRenderPass( uint32_t resolveReferenceCount = 0; uint32_t i; + /* Note: Render pass compatibility does not care about layout */ for (i = 0; i < attachmentInfo.colorAttachmentCount; i += 1) { attachmentDescription = attachmentInfo.colorAttachmentDescriptions[i]; @@ -6328,6 +6402,37 @@ static VkRenderPass VULKAN_INTERNAL_CreateTransientRenderPass( subpass.pResolveAttachments = NULL; } + const VkPipelineStageFlags graphicsStages = 0 + | VK_PIPELINE_STAGE_DRAW_INDIRECT_BIT + | VK_PIPELINE_STAGE_VERTEX_INPUT_BIT + | VK_PIPELINE_STAGE_VERTEX_SHADER_BIT + | VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT + | VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT + | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT + | VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT + ; + const VkPipelineStageFlags outsideStages = 0 + | graphicsStages + | VK_PIPELINE_STAGE_COMPUTE_SHADER_BIT + | VK_PIPELINE_STAGE_TRANSFER_BIT + ; + + dep[0].srcSubpass = VK_SUBPASS_EXTERNAL; + dep[0].dstSubpass = 0; + dep[0].srcStageMask = outsideStages; + dep[0].dstStageMask = graphicsStages; + dep[0].srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT; + dep[0].dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT; + dep[0].dependencyFlags = 0; + + dep[1].srcSubpass = 0; + dep[1].dstSubpass = VK_SUBPASS_EXTERNAL; + dep[1].srcStageMask = graphicsStages; + dep[1].dstStageMask = outsideStages; + dep[1].srcAccessMask = VK_ACCESS_MEMORY_WRITE_BIT; + dep[1].dstAccessMask = VK_ACCESS_MEMORY_READ_BIT | VK_ACCESS_MEMORY_WRITE_BIT; + dep[1].dependencyFlags = 0; + renderPassCreateInfo.sType = VK_STRUCTURE_TYPE_RENDER_PASS_CREATE_INFO; renderPassCreateInfo.pNext = NULL; renderPassCreateInfo.flags = 0; @@ -6335,8 +6440,8 @@ static VkRenderPass VULKAN_INTERNAL_CreateTransientRenderPass( renderPassCreateInfo.attachmentCount = attachmentDescriptionCount; renderPassCreateInfo.subpassCount = 1; renderPassCreateInfo.pSubpasses = &subpass; - renderPassCreateInfo.dependencyCount = 0; - renderPassCreateInfo.pDependencies = NULL; + renderPassCreateInfo.dependencyCount = 2; + renderPassCreateInfo.pDependencies = dep; result = renderer->vkCreateRenderPass( renderer->logicalDevice, @@ -7777,6 +7882,14 @@ static void VULKAN_SetBufferData( vulkanBuffer ); + // this janky call will wait for transfer writes to finish! + VULKAN_INTERNAL_BufferMemoryBarrier( + renderer, + vulkanCommandBuffer->commandBuffer, + RESOURCE_ACCESS_TRANSFER_WRITE, + vulkanBuffer + ); + bufferCopy.srcOffset = transferBuffer->offset; bufferCopy.dstOffset = offsetInBytes; bufferCopy.size = (VkDeviceSize) dataLength; @@ -8481,6 +8594,8 @@ static VkRenderPass VULKAN_INTERNAL_FetchRenderPass( hash.depthStencilTargetDescription.stencilStoreOp = depthStencilAttachmentInfo->stencilStoreOp; } + hash.finalLayout = VULKAN_INTERNAL_GetRenderPassFinalLayout(texture); + renderPass = RenderPassHashArray_Fetch( &renderer->renderPassHashArray, &hash @@ -9039,35 +9154,11 @@ static void VULKAN_EndRenderPass( if (currentTexture->usageFlags & VK_IMAGE_USAGE_SAMPLED_BIT) { - VULKAN_INTERNAL_ImageMemoryBarrier( - renderer, - vulkanCommandBuffer->commandBuffer, - RESOURCE_ACCESS_ANY_SHADER_READ_SAMPLED_IMAGE, - currentTexture->aspectFlags, - 0, - currentTexture->layerCount, - 0, - currentTexture->levelCount, - 0, - currentTexture->image, - ¤tTexture->resourceAccessType - ); + currentTexture->resourceAccessType = RESOURCE_ACCESS_ANY_SHADER_READ_SAMPLED_IMAGE; } else if (currentTexture->usageFlags & VK_IMAGE_USAGE_STORAGE_BIT) { - VULKAN_INTERNAL_ImageMemoryBarrier( - renderer, - vulkanCommandBuffer->commandBuffer, - RESOURCE_ACCESS_COMPUTE_SHADER_STORAGE_IMAGE_READ_WRITE, - currentTexture->aspectFlags, - 0, - currentTexture->layerCount, - 0, - currentTexture->levelCount, - 0, - currentTexture->image, - ¤tTexture->resourceAccessType - ); + currentTexture->resourceAccessType = RESOURCE_ACCESS_COMPUTE_SHADER_STORAGE_IMAGE_READ_WRITE; } } vulkanCommandBuffer->renderPassColorTargetCount = 0; @@ -9078,19 +9169,7 @@ static void VULKAN_EndRenderPass( if (currentTexture->usageFlags & VK_IMAGE_USAGE_SAMPLED_BIT) { - VULKAN_INTERNAL_ImageMemoryBarrier( - renderer, - vulkanCommandBuffer->commandBuffer, - RESOURCE_ACCESS_ANY_SHADER_READ_SAMPLED_IMAGE, - currentTexture->aspectFlags, - 0, - currentTexture->layerCount, - 0, - currentTexture->levelCount, - 0, - currentTexture->image, - ¤tTexture->resourceAccessType - ); + currentTexture->resourceAccessType = RESOURCE_ACCESS_ANY_SHADER_READ_SAMPLED_IMAGE; } } vulkanCommandBuffer->renderPassDepthTexture = NULL;