Merge pull request #19017 from hrydgard/more-vulkan-barrier-stuff

Vulkan: More memory barrier simplification and fixes

.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

.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

.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

.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

.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

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"
 
@@ -50,30 +51,103 @@ void VulkanBarrierBatch::TransitionImage(
 	imageBarrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
 }
 
-void VulkanBarrierBatch::TransitionImageAuto(
-	VkImage image, int baseMip, int numMipLevels, int numLayers, VkImageAspectFlags aspectMask,
-	VkImageLayout oldImageLayout, 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 (oldImageLayout) {
+	switch (*imageLayout) {
+	case VK_IMAGE_LAYOUT_UNDEFINED:
+		srcAccessMask = 0;
+		srcStageMask_ |= VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT;
+		break;
 	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_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;
+		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;
+	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;
+	}
+
+	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_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_UNDEFINED:
+		srcAccessMask = 0;
+		srcStageMask_ |= VK_PIPELINE_STAGE_TOP_OF_PIPE_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;
-		}
+	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;
@@ -84,7 +158,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;
 	}
 
@@ -93,8 +167,20 @@ void VulkanBarrierBatch::TransitionImageAuto(
 		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: %d", (int)newImageLayout);
+		_assert_msg_(false, "Unexpected newLayout: %s", VulkanImageLayoutToString(newImageLayout));
 		break;
 	}
 
@@ -103,14 +189,24 @@ 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;
+	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.
 	imageBarrier.subresourceRange.baseArrayLayer = 0;
 	imageBarrier.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
 	imageBarrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED;
+
+	*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);
 }

Common/GPU/Vulkan/VulkanBarrier.h

@@ -8,6 +8,7 @@
 #include "Common/Data/Collections/TinySet.h"
 
 class VulkanContext;
+struct VKRImage;
 
 class VulkanBarrierBatch {
 public:
@@ -16,6 +17,7 @@ class VulkanBarrierBatch {
 
 	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;
@@ -44,8 +46,11 @@ class VulkanBarrierBatch {
 
 	// 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);
+	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);