From d52651f1d29d1cc99f721dfd845e12ad986202e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 19:54:18 +0200 Subject: [PATCH 01/12] Replace links to Orphis buildbot with /devbuilds --- .github/ISSUE_TEMPLATE/bug.yml | 2 +- .github/ISSUE_TEMPLATE/build.yml | 2 +- .github/ISSUE_TEMPLATE/feature.yml | 2 +- .github/ISSUE_TEMPLATE/graphics.yml | 2 +- .github/ISSUE_TEMPLATE/performance.yml | 2 +- README.md | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index 95dd9e21d5..7ee80bae84 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -127,7 +127,7 @@ body: attributes: label: Checklist options: - - label: "Test in the [latest git build](https://buildbot.orphis.net/ppsspp/) in case it's already fixed." + - label: "Test in the [latest git build](https://www.ppsspp.org/devbuilds) in case it's already fixed." required: true - label: "[Search for other reports](https://github.com/hrydgard/ppsspp/search?q=my+issue&type=issues) of the same issue." required: true diff --git a/.github/ISSUE_TEMPLATE/build.yml b/.github/ISSUE_TEMPLATE/build.yml index 01830ac853..675fee7b33 100644 --- a/.github/ISSUE_TEMPLATE/build.yml +++ b/.github/ISSUE_TEMPLATE/build.yml @@ -84,7 +84,7 @@ body: attributes: label: Checklist options: - - label: "Test in the [latest git build](https://buildbot.orphis.net/ppsspp/) in case it's already fixed." + - label: "Test in the [latest git build](https://www.ppsspp.org/devbuilds) in case it's already fixed." required: true - label: "Make sure to run `git submodule update --init --recursive` before building." required: true diff --git a/.github/ISSUE_TEMPLATE/feature.yml b/.github/ISSUE_TEMPLATE/feature.yml index 8af90a3f0c..bc107fb505 100644 --- a/.github/ISSUE_TEMPLATE/feature.yml +++ b/.github/ISSUE_TEMPLATE/feature.yml @@ -68,7 +68,7 @@ body: attributes: label: Checklist options: - - label: "Check the [latest git build](https://buildbot.orphis.net/ppsspp/) in case it's already implemented." + - label: "Check the [latest git build](https://www.ppsspp.org/devbuilds) in case it's already implemented." required: true - label: "[Search for other requests](https://github.com/hrydgard/ppsspp/search?q=my+issue&type=issues) of the same feature." required: true diff --git a/.github/ISSUE_TEMPLATE/graphics.yml b/.github/ISSUE_TEMPLATE/graphics.yml index 1c473ec033..6ac8e5f1ac 100644 --- a/.github/ISSUE_TEMPLATE/graphics.yml +++ b/.github/ISSUE_TEMPLATE/graphics.yml @@ -141,7 +141,7 @@ body: attributes: label: Checklist options: - - label: "Test in the [latest git build](https://buildbot.orphis.net/ppsspp/) in case it's already fixed." + - label: "Test in the [latest git build](https://www.ppsspp.org/devbuilds) in case it's already fixed." required: true - label: "[Search for other reports](https://github.com/hrydgard/ppsspp/search?q=my+issue&type=issues) of the same issue." required: true diff --git a/.github/ISSUE_TEMPLATE/performance.yml b/.github/ISSUE_TEMPLATE/performance.yml index 7ee87a8543..737048b70f 100644 --- a/.github/ISSUE_TEMPLATE/performance.yml +++ b/.github/ISSUE_TEMPLATE/performance.yml @@ -125,7 +125,7 @@ body: attributes: label: Checklist options: - - label: "Test in the [latest git build](https://buildbot.orphis.net/ppsspp/) in case it's already fixed." + - label: "Test in the [latest git build](https://www.ppsspp.org/devbuilds) in case it's already fixed." required: true - label: "[Search for other reports](https://github.com/hrydgard/ppsspp/search?q=my+issue&type=issues) of the same issue." required: true diff --git a/README.md b/README.md index 4039c3dd04..42755c2e4e 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ For documentation of all kinds (usage, reference, development), see the [documen For build instructions and other development tutorials, see [the wiki](https://github.com/hrydgard/ppsspp/wiki). -If you want to download regularly updated builds for Android, Windows x86 and x64, [visit Orphis' buildbot](https://buildbot.orphis.net/ppsspp/) +To download fresh development builds for Android, Windows and Mac, [go to the /devbuilds page](https://www.ppsspp.org/devbuilds) For game compatibility, see [community compatibility feedback](https://report.ppsspp.org/games). @@ -399,4 +399,4 @@ Credit goes to: [#18822]: https://github.com/hrydgard/ppsspp/issues/18822 "Avoid vertically merging the two main framebuffers, even if FramebufferAllowLargeVerticalOffset is on." [#18813]: https://github.com/hrydgard/ppsspp/issues/18813 "Mali: Turn off any depth writes in the shader if depth test == NEVER" [#18819]: https://github.com/hrydgard/ppsspp/issues/18819 "Temporarily disable MSAA on Adreno GPUs" -[#18823]: https://github.com/hrydgard/ppsspp/issues/18823 "Memory exception handler: Don't disassemble if ignoring the exception" \ No newline at end of file +[#18823]: https://github.com/hrydgard/ppsspp/issues/18823 "Memory exception handler: Don't disassemble if ignoring the exception" From 0bdbffd12f316dfb2c1c2306dc39efe07b0ac5e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 15:15:24 +0200 Subject: [PATCH 02/12] Revert minor optimization that needs some more work first --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index a0164c03ad..085ab52e88 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -1443,7 +1443,7 @@ VKRRenderPass *VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKR } TransitionColorToOptimal(cmd, fb->color.image, fb->color.layout, fb->numLayers, &recordBarrier_); - if (fb->depth.image && RenderPassTypeHasDepth(step.render.renderPassType)) { + if (fb->depth.image) { // } && RenderPassTypeHasDepth(step.render.renderPassType)) { TransitionDepthToOptimal(cmd, fb->depth.image, fb->depth.layout, fb->numLayers, &recordBarrier_); } From 6ac5a21846c75456f5f8c7715dfbb77f29628f06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 15:17:02 +0200 Subject: [PATCH 03/12] Update the layout from within TransitionImageAuto --- Common/GPU/Vulkan/VulkanBarrier.cpp | 12 +++++++----- Common/GPU/Vulkan/VulkanBarrier.h | 2 +- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 6 ++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanBarrier.cpp b/Common/GPU/Vulkan/VulkanBarrier.cpp index c99a48a10e..2e3b23c8aa 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.cpp +++ b/Common/GPU/Vulkan/VulkanBarrier.cpp @@ -52,12 +52,12 @@ void VulkanBarrierBatch::TransitionImage( void VulkanBarrierBatch::TransitionImageAuto( VkImage image, int baseMip, int numMipLevels, int numLayers, VkImageAspectFlags aspectMask, - VkImageLayout oldImageLayout, VkImageLayout newImageLayout) { + VkImageLayout *imageLayout, VkImageLayout newImageLayout) { _dbg_assert_(image != VK_NULL_HANDLE); VkAccessFlags srcAccessMask = 0; VkAccessFlags dstAccessMask = 0; - switch (oldImageLayout) { + switch (*imageLayout) { case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: // Assert aspect here? srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; @@ -84,7 +84,7 @@ void VulkanBarrierBatch::TransitionImageAuto( srcStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; break; default: - _assert_msg_(false, "Unexpected oldLayout: %d", (int)oldImageLayout); + _assert_msg_(false, "Unexpected oldLayout: %s", VulkanImageLayoutToString(*imageLayout)); break; } @@ -94,7 +94,7 @@ void VulkanBarrierBatch::TransitionImageAuto( dstStageMask_ |= VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; break; default: - _assert_msg_(false, "Unexpected newLayout: %d", (int)newImageLayout); + _assert_msg_(false, "Unexpected newLayout: %s", VulkanImageLayoutToString(newImageLayout)); break; } @@ -103,7 +103,7 @@ void VulkanBarrierBatch::TransitionImageAuto( imageBarrier.pNext = nullptr; imageBarrier.srcAccessMask = srcAccessMask; imageBarrier.dstAccessMask = dstAccessMask; - imageBarrier.oldLayout = oldImageLayout; + imageBarrier.oldLayout = *imageLayout; imageBarrier.newLayout = newImageLayout; imageBarrier.image = image; imageBarrier.subresourceRange.aspectMask = aspectMask; @@ -113,4 +113,6 @@ void VulkanBarrierBatch::TransitionImageAuto( imageBarrier.subresourceRange.baseArrayLayer = 0; imageBarrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; imageBarrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; + + *imageLayout = newImageLayout; } diff --git a/Common/GPU/Vulkan/VulkanBarrier.h b/Common/GPU/Vulkan/VulkanBarrier.h index 824515786c..677f99b2ee 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.h +++ b/Common/GPU/Vulkan/VulkanBarrier.h @@ -45,7 +45,7 @@ public: // Automatically determines access and stage masks from layouts. // Not universally usable, but works for PPSSPP's use. void TransitionImageAuto(VkImage image, int baseMip, int numMipLevels, int numLayers, VkImageAspectFlags aspectMask, - VkImageLayout oldImageLayout, VkImageLayout newImageLayout); + VkImageLayout *imageLayout, VkImageLayout newImageLayout); void Flush(VkCommandBuffer cmd); diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index 085ab52e88..3d7a10c7d8 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -1090,10 +1090,9 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c 1, iter.fb->numLayers, VK_IMAGE_ASPECT_COLOR_BIT, - iter.fb->color.layout, + &iter.fb->color.layout, iter.targetLayout ); - iter.fb->color.layout = iter.targetLayout; } else if (iter.fb->depth.image != VK_NULL_HANDLE && (iter.aspect & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) && iter.fb->depth.layout != iter.targetLayout) { recordBarrier_.TransitionImageAuto( iter.fb->depth.image, @@ -1101,10 +1100,9 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c 1, iter.fb->numLayers, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, - iter.fb->depth.layout, + &iter.fb->depth.layout, iter.targetLayout ); - iter.fb->depth.layout = iter.targetLayout; } } From 58fea04b8a58b00de150cc29a49d5b0e9fe8361f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 15:29:45 +0200 Subject: [PATCH 04/12] Split the auto image layout transition function (color/depth-stencil) --- Common/GPU/Vulkan/VulkanBarrier.cpp | 78 ++++++++++++++++++++----- Common/GPU/Vulkan/VulkanBarrier.h | 4 +- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 18 +++--- 3 files changed, 73 insertions(+), 27 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanBarrier.cpp b/Common/GPU/Vulkan/VulkanBarrier.cpp index 2e3b23c8aa..d467e3b948 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.cpp +++ b/Common/GPU/Vulkan/VulkanBarrier.cpp @@ -50,11 +50,9 @@ void VulkanBarrierBatch::TransitionImage( imageBarrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; } -void VulkanBarrierBatch::TransitionImageAuto( - VkImage image, int baseMip, int numMipLevels, int numLayers, VkImageAspectFlags aspectMask, - VkImageLayout *imageLayout, VkImageLayout newImageLayout) { +void VulkanBarrierBatch::TransitionColorImageAuto( + VkImage image, VkImageLayout *imageLayout, VkImageLayout newImageLayout, int baseMip, int numMipLevels, int numLayers) { _dbg_assert_(image != VK_NULL_HANDLE); - VkAccessFlags srcAccessMask = 0; VkAccessFlags dstAccessMask = 0; switch (*imageLayout) { @@ -63,17 +61,9 @@ void VulkanBarrierBatch::TransitionImageAuto( srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; srcStageMask_ |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; break; - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: - // Assert aspect here? - srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT; - srcStageMask_ |= VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; - break; case VK_IMAGE_LAYOUT_UNDEFINED: - // Actually this seems wrong? - if (aspectMask == VK_IMAGE_ASPECT_COLOR_BIT) { - srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; - srcStageMask_ |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; - } + srcAccessMask = 0; + srcStageMask_ |= VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; break; case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; @@ -106,7 +96,65 @@ void VulkanBarrierBatch::TransitionImageAuto( imageBarrier.oldLayout = *imageLayout; imageBarrier.newLayout = newImageLayout; imageBarrier.image = image; - imageBarrier.subresourceRange.aspectMask = aspectMask; + imageBarrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; + imageBarrier.subresourceRange.baseMipLevel = baseMip; + imageBarrier.subresourceRange.levelCount = numMipLevels; + imageBarrier.subresourceRange.layerCount = numLayers; // NOTE: We could usually use VK_REMAINING_ARRAY_LAYERS/VK_REMAINING_MIP_LEVELS, but really old Mali drivers have problems with those. + imageBarrier.subresourceRange.baseArrayLayer = 0; + imageBarrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; + imageBarrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; + + *imageLayout = newImageLayout; +} + +void VulkanBarrierBatch::TransitionDepthStencilImageAuto( + VkImage image, VkImageLayout *imageLayout, VkImageLayout newImageLayout, int baseMip, int numMipLevels, int numLayers) { + _dbg_assert_(image != VK_NULL_HANDLE); + + VkAccessFlags srcAccessMask = 0; + VkAccessFlags dstAccessMask = 0; + switch (*imageLayout) { + case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: + // Assert aspect here? + srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT; + srcStageMask_ |= VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; + break; + case VK_IMAGE_LAYOUT_UNDEFINED: + srcAccessMask = 0; + srcStageMask_ |= VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; + break; + case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: + srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + srcStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; + break; + case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: + srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + srcStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; + break; + default: + _assert_msg_(false, "Unexpected oldLayout: %s", VulkanImageLayoutToString(*imageLayout)); + break; + } + + switch (newImageLayout) { + case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: + dstAccessMask = VK_ACCESS_SHADER_READ_BIT; + dstStageMask_ |= VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; + break; + default: + _assert_msg_(false, "Unexpected newLayout: %s", VulkanImageLayoutToString(newImageLayout)); + break; + } + + VkImageMemoryBarrier &imageBarrier = imageBarriers_.push_uninitialized(); + imageBarrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; + imageBarrier.pNext = nullptr; + imageBarrier.srcAccessMask = srcAccessMask; + imageBarrier.dstAccessMask = dstAccessMask; + imageBarrier.oldLayout = *imageLayout; + imageBarrier.newLayout = newImageLayout; + imageBarrier.image = image; + imageBarrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; imageBarrier.subresourceRange.baseMipLevel = baseMip; imageBarrier.subresourceRange.levelCount = numMipLevels; imageBarrier.subresourceRange.layerCount = numLayers; // NOTE: We could usually use VK_REMAINING_ARRAY_LAYERS/VK_REMAINING_MIP_LEVELS, but really old Mali drivers have problems with those. diff --git a/Common/GPU/Vulkan/VulkanBarrier.h b/Common/GPU/Vulkan/VulkanBarrier.h index 677f99b2ee..831c2fca63 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.h +++ b/Common/GPU/Vulkan/VulkanBarrier.h @@ -44,8 +44,8 @@ public: // Automatically determines access and stage masks from layouts. // Not universally usable, but works for PPSSPP's use. - void TransitionImageAuto(VkImage image, int baseMip, int numMipLevels, int numLayers, VkImageAspectFlags aspectMask, - VkImageLayout *imageLayout, VkImageLayout newImageLayout); + void TransitionColorImageAuto(VkImage image, VkImageLayout *imageLayout, VkImageLayout newImageLayout, int baseMip, int numMipLevels, int numLayers); + void TransitionDepthStencilImageAuto(VkImage image, VkImageLayout *imageLayout, VkImageLayout newImageLayout, int baseMip, int numMipLevels, int numLayers); void Flush(VkCommandBuffer cmd); diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index 3d7a10c7d8..4c949a8fd9 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -1084,24 +1084,22 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c for (size_t i = 0; i < step.preTransitions.size(); i++) { const TransitionRequest &iter = step.preTransitions[i]; if (iter.aspect == VK_IMAGE_ASPECT_COLOR_BIT && iter.fb->color.layout != iter.targetLayout) { - recordBarrier_.TransitionImageAuto( + recordBarrier_.TransitionColorImageAuto( iter.fb->color.image, + &iter.fb->color.layout, + iter.targetLayout, 0, 1, - iter.fb->numLayers, - VK_IMAGE_ASPECT_COLOR_BIT, - &iter.fb->color.layout, - iter.targetLayout + iter.fb->numLayers ); } else if (iter.fb->depth.image != VK_NULL_HANDLE && (iter.aspect & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) && iter.fb->depth.layout != iter.targetLayout) { - recordBarrier_.TransitionImageAuto( + recordBarrier_.TransitionDepthStencilImageAuto( iter.fb->depth.image, + &iter.fb->depth.layout, + iter.targetLayout, 0, 1, - iter.fb->numLayers, - VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, - &iter.fb->depth.layout, - iter.targetLayout + iter.fb->numLayers ); } } From ada76e30ca3689119c9cc8dc4e16d5fc35ddf75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 15:36:02 +0200 Subject: [PATCH 05/12] Add new capabilities to the auto image layout transition functions --- Common/GPU/Vulkan/VulkanBarrier.cpp | 57 ++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanBarrier.cpp b/Common/GPU/Vulkan/VulkanBarrier.cpp index d467e3b948..f0fac28484 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.cpp +++ b/Common/GPU/Vulkan/VulkanBarrier.cpp @@ -56,15 +56,24 @@ void VulkanBarrierBatch::TransitionColorImageAuto( VkAccessFlags srcAccessMask = 0; VkAccessFlags dstAccessMask = 0; switch (*imageLayout) { - case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: - // Assert aspect here? - srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; - srcStageMask_ |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; - break; case VK_IMAGE_LAYOUT_UNDEFINED: srcAccessMask = 0; srcStageMask_ |= VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; break; + case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: + srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; + srcStageMask_ |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; + break; + case VK_IMAGE_LAYOUT_GENERAL: + // We came from the Mali workaround, and are transitioning back to COLOR_ATTACHMENT_OPTIMAL. + srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; + srcStageMask_ |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; + break; + case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: + // We only texture from images in the fragment shader, so can do this simply. + srcAccessMask = VK_ACCESS_SHADER_READ_BIT; + srcStageMask_ |= VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; + break; case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; srcStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; @@ -83,6 +92,18 @@ void VulkanBarrierBatch::TransitionColorImageAuto( dstAccessMask = VK_ACCESS_SHADER_READ_BIT; dstStageMask_ |= VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; break; + case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: + dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + dstStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; + break; + case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: + dstAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + dstStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; + break; + case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: + dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; + dstStageMask_ |= VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; + break; default: _assert_msg_(false, "Unexpected newLayout: %s", VulkanImageLayoutToString(newImageLayout)); break; @@ -114,15 +135,19 @@ void VulkanBarrierBatch::TransitionDepthStencilImageAuto( VkAccessFlags srcAccessMask = 0; VkAccessFlags dstAccessMask = 0; switch (*imageLayout) { - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: - // Assert aspect here? - srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT; - srcStageMask_ |= VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; - break; case VK_IMAGE_LAYOUT_UNDEFINED: srcAccessMask = 0; srcStageMask_ |= VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT; break; + case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: + srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT; + srcStageMask_ |= VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; + break; + case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: + // We only texture from images in the fragment shader, so can do this simply. + srcAccessMask = VK_ACCESS_SHADER_READ_BIT; + srcStageMask_ |= VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; + break; case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; srcStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; @@ -141,6 +166,18 @@ void VulkanBarrierBatch::TransitionDepthStencilImageAuto( dstAccessMask = VK_ACCESS_SHADER_READ_BIT; dstStageMask_ |= VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; break; + case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: + dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + dstStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; + break; + case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: + dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + dstStageMask_ |= VK_PIPELINE_STAGE_TRANSFER_BIT; + break; + case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: + dstAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT; + dstStageMask_ |= VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; + break; default: _assert_msg_(false, "Unexpected newLayout: %s", VulkanImageLayoutToString(newImageLayout)); break; From 9c4dd32db037d2323124dfa08279515db3028b95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 20:04:56 +0200 Subject: [PATCH 06/12] Replace the "ToOptimal" layout transitions with the auto ones --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 81 +------------------------ 1 file changed, 2 insertions(+), 79 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index 4c949a8fd9..d6ce4b0dd2 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -931,83 +931,6 @@ void VulkanQueueRunner::LogReadbackImage(const VKRStep &step) { INFO_LOG(G3D, "%s", StepToString(vulkan_, step).c_str()); } -static void TransitionColorToOptimal(VkCommandBuffer cmd, VkImage colorImage, VkImageLayout colorLayout, int numLayers, VulkanBarrierBatch *recordBarrier) { - VkPipelineStageFlags srcStageMask = 0; - VkAccessFlags srcAccessMask = 0; - switch (colorLayout) { - case VK_IMAGE_LAYOUT_UNDEFINED: - // No need to specify stage or access. - break; - case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: - // Already the right color layout. Unclear that we need to do a lot here.. - return; - case VK_IMAGE_LAYOUT_GENERAL: - // We came from the Mali workaround, and are transitioning back to COLOR_ATTACHMENT_OPTIMAL. - srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; - srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; - break; - case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - srcAccessMask = VK_ACCESS_SHADER_READ_BIT; - srcStageMask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: - srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; - srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: - srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - default: - _dbg_assert_msg_(false, "TransitionColorToOptimal: Unexpected layout %d", (int)colorLayout); - break; - } - recordBarrier->TransitionImage( - colorImage, 0, 1, numLayers, VK_IMAGE_ASPECT_COLOR_BIT, - colorLayout, - VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, - srcAccessMask, - VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, - srcStageMask, - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT); -} - -static void TransitionDepthToOptimal(VkCommandBuffer cmd, VkImage depthStencilImage, VkImageLayout depthStencilLayout, int numLayers, VulkanBarrierBatch *recordBarrier) { - VkPipelineStageFlags srcStageMask = 0; - VkAccessFlags srcAccessMask = 0; - switch (depthStencilLayout) { - case VK_IMAGE_LAYOUT_UNDEFINED: - // No need to specify stage or access. - break; - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: - // Already the right depth layout. Unclear that we need to do a lot here.. - return; - case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - srcAccessMask = VK_ACCESS_SHADER_READ_BIT; - srcStageMask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: - srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: - srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; - srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - default: - _dbg_assert_msg_(false, "TransitionDepthToOptimal: Unexpected layout %d", (int)depthStencilLayout); - break; - } - recordBarrier->TransitionImage( - depthStencilImage, 0, 1, numLayers, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, - depthStencilLayout, - VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, - srcAccessMask, - VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT, - srcStageMask, - VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT); -} - static void TransitionColorFromOptimal(VkCommandBuffer cmd, VkImage colorImage, VkImageLayout colorLayout, int numLayers, VulkanBarrierBatch *recordBarrier) { VkPipelineStageFlags dstStageMask = 0; VkAccessFlags dstAccessMask = 0; @@ -1438,9 +1361,9 @@ VKRRenderPass *VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKR fb->color.layout = VK_IMAGE_LAYOUT_GENERAL; } - TransitionColorToOptimal(cmd, fb->color.image, fb->color.layout, fb->numLayers, &recordBarrier_); + recordBarrier_.TransitionColorImageAuto(fb->color.image, &fb->color.layout, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, 0, 1, fb->numLayers); if (fb->depth.image) { // } && RenderPassTypeHasDepth(step.render.renderPassType)) { - TransitionDepthToOptimal(cmd, fb->depth.image, fb->depth.layout, fb->numLayers, &recordBarrier_); + recordBarrier_.TransitionDepthStencilImageAuto(fb->depth.image, &fb->depth.layout, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, 0, 1, fb->numLayers); } // The transition from the optimal format happens after EndRenderPass, now that we don't From 3e91095313596ad3aa90e4564214c3414398b3dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 20:06:04 +0200 Subject: [PATCH 07/12] Replace the "FromOptimal" layout transitions with auto. This fixes some rendering errors on NV. --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 76 +------------------------ 1 file changed, 2 insertions(+), 74 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index d6ce4b0dd2..b1a22c416f 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -931,78 +931,6 @@ void VulkanQueueRunner::LogReadbackImage(const VKRStep &step) { INFO_LOG(G3D, "%s", StepToString(vulkan_, step).c_str()); } -static void TransitionColorFromOptimal(VkCommandBuffer cmd, VkImage colorImage, VkImageLayout colorLayout, int numLayers, VulkanBarrierBatch *recordBarrier) { - VkPipelineStageFlags dstStageMask = 0; - VkAccessFlags dstAccessMask = 0; - // And the final transition. - // Don't need to transition it if VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL. - switch (colorLayout) { - case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - dstAccessMask = VK_ACCESS_SHADER_READ_BIT; - dstStageMask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: - dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - dstStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: - dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - dstStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - case VK_IMAGE_LAYOUT_UNDEFINED: - case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: - // Nothing to do. - return; - default: - _dbg_assert_msg_(false, "TransitionFromOptimal: Unexpected final color layout %d", (int)colorLayout); - break; - } - recordBarrier->TransitionImage( - colorImage, 0, 1, numLayers, VK_IMAGE_ASPECT_COLOR_BIT, - VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, - colorLayout, - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, // srcAccessMask - dstAccessMask, - VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, - dstStageMask - ); -} - -static void TransitionDepthFromOptimal(VkCommandBuffer cmd, VkImage depthStencilImage, VkImageLayout depthStencilLayout, int numLayers, VulkanBarrierBatch *recordBarrier) { - VkPipelineStageFlags dstStageMask = 0; - VkAccessFlags dstAccessMask = 0; - switch (depthStencilLayout) { - case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - dstAccessMask = VK_ACCESS_SHADER_READ_BIT; - dstStageMask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: - dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - dstStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: - dstAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - dstStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - case VK_IMAGE_LAYOUT_UNDEFINED: - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: - // Nothing to do. - return; - default: - _dbg_assert_msg_(false, "TransitionFromOptimal: Unexpected final depth layout %d", (int)depthStencilLayout); - break; - } - recordBarrier->TransitionImage( - depthStencilImage, 0, 1, numLayers, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, - VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, - depthStencilLayout, - VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT, // srcAccessMask - dstAccessMask, - VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT, - dstStageMask - ); -} - void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer cmd, int curFrame, QueueProfileContext &profile) { for (size_t i = 0; i < step.preTransitions.size(); i++) { const TransitionRequest &iter = step.preTransitions[i]; @@ -1303,9 +1231,9 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c if (fb) { // If the desired final layout aren't the optimal layout needed next, transition. - TransitionColorFromOptimal(cmd, fb->color.image, step.render.finalColorLayout, fb->numLayers, &recordBarrier_); + recordBarrier_.TransitionColorImageAuto(fb->color.image, &fb->color.layout, step.render.finalColorLayout, 0, 1, fb->numLayers); if (fb->depth.image) { - TransitionDepthFromOptimal(cmd, fb->depth.image, step.render.finalDepthStencilLayout, fb->numLayers, &recordBarrier_); + recordBarrier_.TransitionDepthStencilImageAuto(fb->depth.image, &fb->depth.layout, step.render.finalDepthStencilLayout, 0, 1, fb->numLayers); } recordBarrier_.Flush(cmd); From 7769bf06f13b37ee81c4b42f7baf945050a8242f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 21:13:55 +0200 Subject: [PATCH 08/12] Add a simplified wrapper for Transition*ImageAuto that takes a VKRImage --- Common/GPU/Vulkan/VulkanBarrier.cpp | 9 +++++++++ Common/GPU/Vulkan/VulkanBarrier.h | 4 ++++ Common/GPU/Vulkan/VulkanQueueRunner.cpp | 24 ++++++++---------------- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanBarrier.cpp b/Common/GPU/Vulkan/VulkanBarrier.cpp index f0fac28484..fa0a98bd16 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.cpp +++ b/Common/GPU/Vulkan/VulkanBarrier.cpp @@ -1,6 +1,7 @@ #include "VulkanLoader.h" #include "VulkanContext.h" #include "VulkanBarrier.h" +#include "VulkanFramebuffer.h" #include "Common/Log.h" @@ -201,3 +202,11 @@ void VulkanBarrierBatch::TransitionDepthStencilImageAuto( *imageLayout = newImageLayout; } + +void VulkanBarrierBatch::TransitionColorImageAuto(VKRImage *image, VkImageLayout newImageLayout) { + TransitionColorImageAuto(image->image, &image->layout, newImageLayout, 0, 1, image->numLayers); +} + +void VulkanBarrierBatch::TransitionDepthStencilImageAuto(VKRImage *image, VkImageLayout newImageLayout) { + TransitionDepthStencilImageAuto(image->image, &image->layout, newImageLayout, 0, 1, image->numLayers); +} diff --git a/Common/GPU/Vulkan/VulkanBarrier.h b/Common/GPU/Vulkan/VulkanBarrier.h index 831c2fca63..74a25459e5 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.h +++ b/Common/GPU/Vulkan/VulkanBarrier.h @@ -8,6 +8,7 @@ #include "Common/Data/Collections/TinySet.h" class VulkanContext; +struct VKRImage; class VulkanBarrierBatch { public: @@ -47,6 +48,9 @@ public: void TransitionColorImageAuto(VkImage image, VkImageLayout *imageLayout, VkImageLayout newImageLayout, int baseMip, int numMipLevels, int numLayers); void TransitionDepthStencilImageAuto(VkImage image, VkImageLayout *imageLayout, VkImageLayout newImageLayout, int baseMip, int numMipLevels, int numLayers); + void TransitionColorImageAuto(VKRImage *image, VkImageLayout newImageLayout); + void TransitionDepthStencilImageAuto(VKRImage *image, VkImageLayout newImageLayout); + void Flush(VkCommandBuffer cmd); private: diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index b1a22c416f..e45f141afe 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -936,21 +936,13 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c const TransitionRequest &iter = step.preTransitions[i]; if (iter.aspect == VK_IMAGE_ASPECT_COLOR_BIT && iter.fb->color.layout != iter.targetLayout) { recordBarrier_.TransitionColorImageAuto( - iter.fb->color.image, - &iter.fb->color.layout, - iter.targetLayout, - 0, - 1, - iter.fb->numLayers + &iter.fb->color, + iter.targetLayout ); } else if (iter.fb->depth.image != VK_NULL_HANDLE && (iter.aspect & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) && iter.fb->depth.layout != iter.targetLayout) { recordBarrier_.TransitionDepthStencilImageAuto( - iter.fb->depth.image, - &iter.fb->depth.layout, - iter.targetLayout, - 0, - 1, - iter.fb->numLayers + &iter.fb->depth, + iter.targetLayout ); } } @@ -1231,9 +1223,9 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c if (fb) { // If the desired final layout aren't the optimal layout needed next, transition. - recordBarrier_.TransitionColorImageAuto(fb->color.image, &fb->color.layout, step.render.finalColorLayout, 0, 1, fb->numLayers); + recordBarrier_.TransitionColorImageAuto(&fb->color, step.render.finalColorLayout); if (fb->depth.image) { - recordBarrier_.TransitionDepthStencilImageAuto(fb->depth.image, &fb->depth.layout, step.render.finalDepthStencilLayout, 0, 1, fb->numLayers); + recordBarrier_.TransitionDepthStencilImageAuto(&fb->depth, step.render.finalDepthStencilLayout); } recordBarrier_.Flush(cmd); @@ -1289,9 +1281,9 @@ VKRRenderPass *VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKR fb->color.layout = VK_IMAGE_LAYOUT_GENERAL; } - recordBarrier_.TransitionColorImageAuto(fb->color.image, &fb->color.layout, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, 0, 1, fb->numLayers); + recordBarrier_.TransitionColorImageAuto(&fb->color, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL); if (fb->depth.image) { // } && RenderPassTypeHasDepth(step.render.renderPassType)) { - recordBarrier_.TransitionDepthStencilImageAuto(fb->depth.image, &fb->depth.layout, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, 0, 1, fb->numLayers); + recordBarrier_.TransitionDepthStencilImageAuto(&fb->depth, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL); } // The transition from the optimal format happens after EndRenderPass, now that we don't From d949331f67583cb889d45be41b5a5823177e5b1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 21:24:15 +0200 Subject: [PATCH 09/12] More use of the auto image layout transition functions --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 38 ++++++++++--------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index e45f141afe..b9ac70246a 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -1370,18 +1370,16 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { // First source barriers. if (step.copy.aspectMask & VK_IMAGE_ASPECT_COLOR_BIT) { - SetupTransitionToTransferSrc(src->color, VK_IMAGE_ASPECT_COLOR_BIT, &recordBarrier_); - SetupTransitionToTransferDst(dst->color, VK_IMAGE_ASPECT_COLOR_BIT, &recordBarrier_); + recordBarrier_.TransitionColorImageAuto(&src->color, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); } // We can't copy only depth or only stencil unfortunately - or can we?. if (step.copy.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) { _dbg_assert_(src->depth.image != VK_NULL_HANDLE); - SetupTransitionToTransferSrc(src->depth, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, &recordBarrier_); + recordBarrier_.TransitionDepthStencilImageAuto(&src->depth, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); if (dst->depth.layout != VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL) { - SetupTransitionToTransferDst(dst->depth, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, &recordBarrier_); - _dbg_assert_(dst->depth.layout == VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); + recordBarrier_.TransitionDepthStencilImageAuto(&dst->depth, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); } else { // Kingdom Hearts: Subsequent copies twice to the same depth buffer without any other use. // Not super sure how that happens, but we need a barrier to pass sync validation. @@ -1395,17 +1393,17 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { // If both the targets are multisampled, copy the msaa targets too. // For that, we need to transition them from their normally permanent VK_*_ATTACHMENT_OPTIMAL layouts, and then back. if (step.copy.aspectMask & VK_IMAGE_ASPECT_COLOR_BIT) { - SetupTransitionToTransferSrc(src->msaaColor, VK_IMAGE_ASPECT_COLOR_BIT, &recordBarrier_); + recordBarrier_.TransitionColorImageAuto(&src->msaaColor, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); recordBarrier_.Flush(cmd); - SetupTransitionToTransferDst(dst->msaaColor, VK_IMAGE_ASPECT_COLOR_BIT, &recordBarrier_); + recordBarrier_.TransitionColorImageAuto(&dst->msaaColor, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); recordBarrier_.Flush(cmd); } if (step.copy.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) { // Kingdom Hearts: Subsequent copies to the same depth buffer without any other use. // Not super sure how that happens, but we need a barrier to pass sync validation. - SetupTransitionToTransferSrc(src->msaaDepth, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, &recordBarrier_); + recordBarrier_.TransitionDepthStencilImageAuto(&src->msaaDepth, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); recordBarrier_.Flush(cmd); - SetupTransitionToTransferDst(dst->msaaDepth, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, &recordBarrier_); + recordBarrier_.TransitionDepthStencilImageAuto(&dst->msaaDepth, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); recordBarrier_.Flush(cmd); } } @@ -1526,16 +1524,16 @@ void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) { // First source barriers. if (step.blit.aspectMask & VK_IMAGE_ASPECT_COLOR_BIT) { - SetupTransitionToTransferSrc(src->color, VK_IMAGE_ASPECT_COLOR_BIT, &recordBarrier_); - SetupTransitionToTransferDst(dst->color, VK_IMAGE_ASPECT_COLOR_BIT, &recordBarrier_); + recordBarrier_.TransitionColorImageAuto(&src->color, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); + recordBarrier_.TransitionColorImageAuto(&dst->color, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); } // We can't copy only depth or only stencil unfortunately. if (step.blit.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) { _assert_(src->depth.image != VK_NULL_HANDLE); _assert_(dst->depth.image != VK_NULL_HANDLE); - SetupTransitionToTransferSrc(src->depth, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, &recordBarrier_); - SetupTransitionToTransferDst(dst->depth, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, &recordBarrier_); + recordBarrier_.TransitionDepthStencilImageAuto(&src->depth, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); + recordBarrier_.TransitionDepthStencilImageAuto(&dst->depth, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL); } recordBarrier_.Flush(cmd); @@ -1762,17 +1760,15 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd VKRImage *srcImage; if (step.readback.aspectMask & VK_IMAGE_ASPECT_COLOR_BIT) { srcImage = &step.readback.src->color; + recordBarrier_.TransitionColorImageAuto(srcImage, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); } else if (step.readback.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) { srcImage = &step.readback.src->depth; + recordBarrier_.TransitionDepthStencilImageAuto(srcImage, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL); _dbg_assert_(srcImage->image != VK_NULL_HANDLE); } else { _dbg_assert_msg_(false, "No image aspect to readback?"); return; } - - if (srcImage->layout != VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL) { - SetupTransitionToTransferSrc(*srcImage, step.readback.aspectMask, &recordBarrier_); - } image = srcImage->image; copyLayout = srcImage->layout; } @@ -1831,12 +1827,8 @@ void VulkanQueueRunner::PerformReadback(const VKRStep &step, VkCommandBuffer cmd void VulkanQueueRunner::PerformReadbackImage(const VKRStep &step, VkCommandBuffer cmd) { // TODO: Clean this up - just reusing `SetupTransitionToTransferSrc`. - VKRImage srcImage{}; - srcImage.image = step.readback_image.image; - srcImage.layout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; - srcImage.numLayers = 1; - - SetupTransitionToTransferSrc(srcImage, VK_IMAGE_ASPECT_COLOR_BIT, &recordBarrier_); + VkImageLayout layout = VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL; + recordBarrier_.TransitionColorImageAuto(step.readback_image.image, &layout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, 0, 1, 1); recordBarrier_.Flush(cmd); ResizeReadbackBuffer(&syncReadback_, sizeof(uint32_t) * step.readback_image.srcRect.extent.width * step.readback_image.srcRect.extent.height); From 78c0be3d8a3e2de1b739025a66bb19893773e0e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 21:25:04 +0200 Subject: [PATCH 10/12] Delete now-unused code --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 105 ------------------------ Common/GPU/Vulkan/VulkanQueueRunner.h | 2 - 2 files changed, 107 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index b9ac70246a..875e4dbbdf 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -1581,111 +1581,6 @@ void VulkanQueueRunner::PerformBlit(const VKRStep &step, VkCommandBuffer cmd) { } } -void VulkanQueueRunner::SetupTransitionToTransferSrc(VKRImage &img, VkImageAspectFlags aspect, VulkanBarrierBatch *recordBarrier) { - if (img.layout == VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL) { - return; - } - VkImageAspectFlags imageAspect = aspect; - VkAccessFlags srcAccessMask = 0; - VkPipelineStageFlags srcStageMask = 0; - switch (img.layout) { - case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: - srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; - srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; - break; - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: - srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; - srcStageMask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: - srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; - srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - srcAccessMask = VK_ACCESS_SHADER_READ_BIT; - srcStageMask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; - break; - default: - _dbg_assert_msg_(false, "Transition from this layout to transfer src not supported (%d)", (int)img.layout); - break; - } - - if (img.format == VK_FORMAT_D16_UNORM_S8_UINT || img.format == VK_FORMAT_D24_UNORM_S8_UINT || img.format == VK_FORMAT_D32_SFLOAT_S8_UINT) { - // Barrier must specify both for combined depth/stencil buffers. - imageAspect = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; - } else { - imageAspect = aspect; - } - - recordBarrier->TransitionImage( - img.image, - 0, - 1, - img.numLayers, - imageAspect, - img.layout, - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - srcAccessMask, - VK_ACCESS_TRANSFER_READ_BIT, - srcStageMask, - VK_PIPELINE_STAGE_TRANSFER_BIT - ); - img.layout = VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL; -} - -void VulkanQueueRunner::SetupTransitionToTransferDst(VKRImage &img, VkImageAspectFlags aspect, VulkanBarrierBatch *recordBarrier) { - if (img.layout == VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL) { - return; - } - VkImageAspectFlags imageAspect = aspect; - VkAccessFlags srcAccessMask = 0; - VkPipelineStageFlags srcStageMask = 0; - if (img.format == VK_FORMAT_D16_UNORM_S8_UINT || img.format == VK_FORMAT_D24_UNORM_S8_UINT || img.format == VK_FORMAT_D32_SFLOAT_S8_UINT) { - // Barrier must specify both for combined depth/stencil buffers. - imageAspect = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; - } else { - imageAspect = aspect; - } - - switch (img.layout) { - case VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL: - srcAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT; - srcStageMask = VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT; - break; - case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: - srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; - srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT; - break; - case VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL: - srcAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT; - srcStageMask = VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT; - break; - case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - srcAccessMask = VK_ACCESS_SHADER_READ_BIT; - srcStageMask = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; - break; - default: - _dbg_assert_msg_(false, "Transition from this layout to transfer dst not supported (%d)", (int)img.layout); - break; - } - - recordBarrier->TransitionImage( - img.image, - 0, - 1, - img.numLayers, - aspect, - img.layout, - VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL, - srcAccessMask, - VK_ACCESS_TRANSFER_WRITE_BIT, - srcStageMask, - VK_PIPELINE_STAGE_TRANSFER_BIT - ); - - img.layout = VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL; -} - void VulkanQueueRunner::SetupTransferDstWriteAfterWrite(VKRImage &img, VkImageAspectFlags aspect, VulkanBarrierBatch *recordBarrier) { VkImageAspectFlags imageAspect = aspect; VkAccessFlags srcAccessMask = 0; diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.h b/Common/GPU/Vulkan/VulkanQueueRunner.h index ab39c7b06f..b489c111a5 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.h +++ b/Common/GPU/Vulkan/VulkanQueueRunner.h @@ -289,8 +289,6 @@ private: void ApplySonicHack(std::vector &steps); void ApplyRenderPassMerge(std::vector &steps); - static void SetupTransitionToTransferSrc(VKRImage &img, VkImageAspectFlags aspect, VulkanBarrierBatch *recordBarrier); - static void SetupTransitionToTransferDst(VKRImage &img, VkImageAspectFlags aspect, VulkanBarrierBatch *recordBarrier); static void SetupTransferDstWriteAfterWrite(VKRImage &img, VkImageAspectFlags aspect, VulkanBarrierBatch *recordBarrier); VulkanContext *vulkan_; From 60f350056c55ea988bcdc459eb2f00d152e33efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 21:43:06 +0200 Subject: [PATCH 11/12] Vulkan: Optimize away a barrier in case of no depth buffer --- Common/GPU/Vulkan/VulkanBarrier.h | 1 + Common/GPU/Vulkan/VulkanQueueRunner.cpp | 11 ++++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanBarrier.h b/Common/GPU/Vulkan/VulkanBarrier.h index 74a25459e5..884e0919c7 100644 --- a/Common/GPU/Vulkan/VulkanBarrier.h +++ b/Common/GPU/Vulkan/VulkanBarrier.h @@ -17,6 +17,7 @@ public: bool empty() const { return imageBarriers_.empty(); } + // TODO: Replace this with TransitionImage. VkImageMemoryBarrier *Add(VkImage image, VkPipelineStageFlags srcStageMask, VkPipelineStageFlags dstStageMask, VkDependencyFlags dependencyFlags) { srcStageMask_ |= srcStageMask; dstStageMask_ |= dstStageMask; diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index 875e4dbbdf..f30979ee84 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -958,7 +958,6 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c } // Write-after-write hazards. Fixed flicker in God of War on ARM (before we added another fix that removed these). - // These aren't so common so not bothering to combine the barrier with the pretransition one. if (step.render.framebuffer) { int n = 0; int stage = 0; @@ -976,7 +975,7 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT - ); + ); } if (step.render.framebuffer->depth.image != VK_NULL_HANDLE && step.render.framebuffer->depth.layout == VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL) { recordBarrier_.TransitionImage( @@ -995,8 +994,8 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c } } - // This reads the layout of the color and depth images, and chooses a render pass using them that - // will transition to the desired final layout. + // This chooses a render pass according to the load/store attachment state. We no longer transition + // image layouts as part of the passes. // // NOTE: Unconditionally flushes recordBarrier_. VKRRenderPass *renderPass = PerformBindFramebufferAsRenderTarget(step, cmd); @@ -1282,7 +1281,9 @@ VKRRenderPass *VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKR } recordBarrier_.TransitionColorImageAuto(&fb->color, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL); - if (fb->depth.image) { // } && RenderPassTypeHasDepth(step.render.renderPassType)) { + + // If the render pass doesn't touch depth, we can avoid a layout transition of the depth buffer. + if (fb->depth.image && RenderPassTypeHasDepth(step.render.renderPassType)) { recordBarrier_.TransitionDepthStencilImageAuto(&fb->depth, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL); } From 036c386b359f36da22f6958374325c161b136b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sun, 7 Apr 2024 22:20:37 +0200 Subject: [PATCH 12/12] More barrier optimizations --- Common/GPU/Vulkan/VulkanQueueRunner.cpp | 30 +++++++++++++------------ 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/Common/GPU/Vulkan/VulkanQueueRunner.cpp b/Common/GPU/Vulkan/VulkanQueueRunner.cpp index f30979ee84..2da4894fa7 100644 --- a/Common/GPU/Vulkan/VulkanQueueRunner.cpp +++ b/Common/GPU/Vulkan/VulkanQueueRunner.cpp @@ -248,7 +248,6 @@ void VulkanQueueRunner::DestroyBackBuffers() { INFO_LOG(G3D, "Backbuffers destroyed"); } - // Self-dependency: https://github.com/gpuweb/gpuweb/issues/442#issuecomment-547604827 // Also see https://www.khronos.org/registry/vulkan/specs/1.3-extensions/html/vkspec.html#synchronization-pipeline-barriers-subpass-self-dependencies VKRRenderPass *VulkanQueueRunner::GetRenderPass(const RPKey &key) { @@ -958,6 +957,9 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c } // Write-after-write hazards. Fixed flicker in God of War on ARM (before we added another fix that removed these). + // NOTE: These are commented out because the normal barriers no longer check for equality, effectively generating these + // barriers automatically. This is safe, but sometimes I think can be improved on. + /* if (step.render.framebuffer) { int n = 0; int stage = 0; @@ -988,11 +990,11 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT, VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT, - VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT, - VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT + VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT, + VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT, ); } - } + }*/ // This chooses a render pass according to the load/store attachment state. We no longer transition // image layouts as part of the passes. @@ -1221,15 +1223,13 @@ void VulkanQueueRunner::PerformRenderPass(const VKRStep &step, VkCommandBuffer c _dbg_assert_(recordBarrier_.empty()); if (fb) { - // If the desired final layout aren't the optimal layout needed next, transition. - recordBarrier_.TransitionColorImageAuto(&fb->color, step.render.finalColorLayout); - if (fb->depth.image) { + // If the desired final layout aren't the optimal layout needed next, early-transition the image. + if (step.render.finalColorLayout != fb->color.layout) { + recordBarrier_.TransitionColorImageAuto(&fb->color, step.render.finalColorLayout); + } + if (fb->depth.image && step.render.finalDepthStencilLayout != fb->depth.layout) { recordBarrier_.TransitionDepthStencilImageAuto(&fb->depth, step.render.finalDepthStencilLayout); } - recordBarrier_.Flush(cmd); - - fb->color.layout = step.render.finalColorLayout; - fb->depth.layout = step.render.finalDepthStencilLayout; } } @@ -1245,7 +1245,8 @@ VKRRenderPass *VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKR VkSampleCountFlagBits sampleCount; - recordBarrier_.Flush(cmd); + // Can be used to separate the final*Layout barrier from the rest for debugging in renderdoc. + // recordBarrier_.Flush(cmd); if (step.render.framebuffer) { _dbg_assert_(step.render.finalColorLayout != VK_IMAGE_LAYOUT_UNDEFINED); @@ -1272,6 +1273,7 @@ VKRRenderPass *VulkanQueueRunner::PerformBindFramebufferAsRenderTarget(const VKR step.render.colorLoad == VKRRenderPassLoadAction::CLEAR && vulkan_->GetPhysicalDeviceProperties().properties.driverVersion == 0xaa9c4b29; if (maliBugWorkaround) { + // A little suboptimal but let's go for maximum safety here. recordBarrier_.TransitionImage(fb->color.image, 0, 1, fb->numLayers, VK_IMAGE_ASPECT_COLOR_BIT, fb->color.layout, VK_IMAGE_LAYOUT_GENERAL, VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT, @@ -1463,6 +1465,7 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT ); + src->msaaColor.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; recordBarrier_.TransitionImage( dst->msaaColor.image, 0, @@ -1476,7 +1479,6 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT ); - src->msaaColor.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; dst->msaaColor.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; } if (step.copy.aspectMask & (VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT)) { @@ -1493,6 +1495,7 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT ); + src->msaaDepth.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; recordBarrier_.TransitionImage( dst->msaaDepth.image, 0, @@ -1506,7 +1509,6 @@ void VulkanQueueRunner::PerformCopy(const VKRStep &step, VkCommandBuffer cmd) { VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT ); - src->msaaDepth.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; dst->msaaDepth.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; } recordBarrier_.Flush(cmd);