From 804aa79376eb4cb216f9f7fa1b8015eff1d3ac59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Fri, 20 Oct 2017 18:09:05 +0200 Subject: [PATCH] Various Vulkan image transition fixes and related --- Common/Vulkan/VulkanContext.cpp | 10 ++-- Common/Vulkan/VulkanMemory.cpp | 8 +++- GPU/Vulkan/DrawEngineVulkan.cpp | 10 +++- GPU/Vulkan/DrawEngineVulkan.h | 2 +- GPU/Vulkan/PipelineManagerVulkan.cpp | 4 ++ GPU/Vulkan/PipelineManagerVulkan.h | 1 + GPU/Vulkan/StateMappingVulkan.cpp | 6 +-- Windows/GPU/WindowsVulkanContext.cpp | 12 +---- ext/native/thin3d/thin3d_vulkan.cpp | 72 +++++++++++++++------------- 9 files changed, 70 insertions(+), 55 deletions(-) diff --git a/Common/Vulkan/VulkanContext.cpp b/Common/Vulkan/VulkanContext.cpp index daa21b05fc..c2861285b3 100644 --- a/Common/Vulkan/VulkanContext.cpp +++ b/Common/Vulkan/VulkanContext.cpp @@ -1203,7 +1203,7 @@ void VulkanContext::DestroyDevice() { VkPipelineCache VulkanContext::CreatePipelineCache() { VkPipelineCache cache; - VkPipelineCacheCreateInfo pc = { VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO }; + VkPipelineCacheCreateInfo pc{ VK_STRUCTURE_TYPE_PIPELINE_CACHE_CREATE_INFO }; pc.pInitialData = nullptr; pc.initialDataSize = 0; pc.flags = 0; @@ -1213,7 +1213,7 @@ VkPipelineCache VulkanContext::CreatePipelineCache() { } bool VulkanContext::CreateShaderModule(const std::vector &spirv, VkShaderModule *shaderModule) { - VkShaderModuleCreateInfo sm = { VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO }; + VkShaderModuleCreateInfo sm{ VK_STRUCTURE_TYPE_SHADER_MODULE_CREATE_INFO }; sm.pCode = spirv.data(); sm.codeSize = spirv.size() * sizeof(uint32_t); sm.flags = 0; @@ -1229,7 +1229,7 @@ void TransitionImageLayout2(VkCommandBuffer cmd, VkImage image, VkImageAspectFla VkImageLayout oldImageLayout, VkImageLayout newImageLayout, VkPipelineStageFlags srcStageMask, VkPipelineStageFlags dstStageMask, VkAccessFlags srcAccessMask, VkAccessFlags dstAccessMask) { - VkImageMemoryBarrier image_memory_barrier = { VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER }; + VkImageMemoryBarrier image_memory_barrier{ VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER }; image_memory_barrier.srcAccessMask = srcAccessMask; image_memory_barrier.dstAccessMask = dstAccessMask; image_memory_barrier.oldLayout = oldImageLayout; @@ -1237,8 +1237,8 @@ void TransitionImageLayout2(VkCommandBuffer cmd, VkImage image, VkImageAspectFla image_memory_barrier.image = image; image_memory_barrier.subresourceRange.aspectMask = aspectMask; image_memory_barrier.subresourceRange.baseMipLevel = 0; - image_memory_barrier.subresourceRange.levelCount = 1; - image_memory_barrier.subresourceRange.layerCount = 1; + image_memory_barrier.subresourceRange.levelCount = VK_REMAINING_MIP_LEVELS; + image_memory_barrier.subresourceRange.layerCount = VK_REMAINING_ARRAY_LAYERS; vkCmdPipelineBarrier(cmd, srcStageMask, dstStageMask, 0, 0, nullptr, 0, nullptr, 1, &image_memory_barrier); } diff --git a/Common/Vulkan/VulkanMemory.cpp b/Common/Vulkan/VulkanMemory.cpp index c6db161c8a..03983c62a6 100644 --- a/Common/Vulkan/VulkanMemory.cpp +++ b/Common/Vulkan/VulkanMemory.cpp @@ -47,10 +47,16 @@ bool VulkanPushBuffer::AddBuffer() { return false; } + VkMemoryRequirements reqs; + vkGetBufferMemoryRequirements(device_, info.buffer, &reqs); + + // TODO: We really should use memoryTypeIndex here.. + // Okay, that's the buffer. Now let's allocate some memory for it. VkMemoryAllocateInfo alloc = { VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO }; + // TODO: Should check here that memoryTypeIndex_ matches reqs.memoryTypeBits. alloc.memoryTypeIndex = memoryTypeIndex_; - alloc.allocationSize = size_; + alloc.allocationSize = reqs.size; res = vkAllocateMemory(device_, &alloc, nullptr, &info.deviceMemory); if (VK_SUCCESS != res) { diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index 07e46aee86..50385236c4 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -884,8 +884,11 @@ void DrawEngineVulkan::DoFlush() { if (pipeline != lastPipeline_) { vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline->pipeline); lastPipeline_ = pipeline; + if (lastPipeline_ && !lastPipeline_->useBlendConstant && pipeline->useBlendConstant) { + gstate_c.Dirty(DIRTY_BLEND_STATE); + } } - ApplyDrawStateLate(cmd, false, 0); + ApplyDrawStateLate(cmd, false, 0, pipeline->useBlendConstant); gstate_c.Clean(DIRTY_BLEND_STATE | DIRTY_DEPTHSTENCIL_STATE | DIRTY_RASTER_STATE | DIRTY_VIEWPORTSCISSOR_STATE); } lastPrim_ = prim; @@ -986,8 +989,11 @@ void DrawEngineVulkan::DoFlush() { if (pipeline != lastPipeline_) { vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline->pipeline); // TODO: Avoid if same as last draw. lastPipeline_ = pipeline; + if (lastPipeline_ && !lastPipeline_->useBlendConstant && pipeline->useBlendConstant) { + gstate_c.Dirty(DIRTY_BLEND_STATE); + } } - ApplyDrawStateLate(cmd, false, 0); + ApplyDrawStateLate(cmd, false, 0, pipeline->useBlendConstant); gstate_c.Clean(DIRTY_BLEND_STATE | DIRTY_DEPTHSTENCIL_STATE | DIRTY_RASTER_STATE | DIRTY_VIEWPORTSCISSOR_STATE); } lastPrim_ = prim; diff --git a/GPU/Vulkan/DrawEngineVulkan.h b/GPU/Vulkan/DrawEngineVulkan.h index 1bb2d3295c..a5387dfb9d 100644 --- a/GPU/Vulkan/DrawEngineVulkan.h +++ b/GPU/Vulkan/DrawEngineVulkan.h @@ -171,7 +171,7 @@ public: private: struct FrameData; - void ApplyDrawStateLate(VkCommandBuffer cmd, bool applyStencilRef, uint8_t stencilRef); + void ApplyDrawStateLate(VkCommandBuffer cmd, bool applyStencilRef, uint8_t stencilRef, bool useBlendConstant); void ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManager, ShaderManagerVulkan *shaderManager, int prim, VulkanPipelineRasterStateKey &key, VulkanDynamicState &dynState); void InitDeviceObjects(); diff --git a/GPU/Vulkan/PipelineManagerVulkan.cpp b/GPU/Vulkan/PipelineManagerVulkan.cpp index d23906750c..f0822a2d45 100644 --- a/GPU/Vulkan/PipelineManagerVulkan.cpp +++ b/GPU/Vulkan/PipelineManagerVulkan.cpp @@ -122,6 +122,8 @@ static bool UsesBlendConstant(int factor) { static VulkanPipeline *CreateVulkanPipeline(VkDevice device, VkPipelineCache pipelineCache, VkPipelineLayout layout, VkRenderPass renderPass, const VulkanPipelineRasterStateKey &key, const VertexDecoder *vtxDec, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform) { + bool useBlendConstant = false; + VkPipelineColorBlendAttachmentState blend0 = {}; blend0.blendEnable = key.blendEnable; if (key.blendEnable) { @@ -166,6 +168,7 @@ static VulkanPipeline *CreateVulkanPipeline(VkDevice device, VkPipelineCache pip if (key.blendEnable && (UsesBlendConstant(key.srcAlpha) || UsesBlendConstant(key.srcColor) || UsesBlendConstant(key.destAlpha) || UsesBlendConstant(key.destColor))) { dynamicStates[numDyn++] = VK_DYNAMIC_STATE_BLEND_CONSTANTS; + useBlendConstant = true; } dynamicStates[numDyn++] = VK_DYNAMIC_STATE_SCISSOR; dynamicStates[numDyn++] = VK_DYNAMIC_STATE_VIEWPORT; @@ -285,6 +288,7 @@ static VulkanPipeline *CreateVulkanPipeline(VkDevice device, VkPipelineCache pip VulkanPipeline *vulkanPipeline = new VulkanPipeline(); vulkanPipeline->pipeline = pipeline; vulkanPipeline->uniformBlocks = UB_VS_FS_BASE; + vulkanPipeline->useBlendConstant = useBlendConstant; if (useHwTransform) { if (vs->HasLights()) { vulkanPipeline->uniformBlocks |= UB_VS_LIGHTS; diff --git a/GPU/Vulkan/PipelineManagerVulkan.h b/GPU/Vulkan/PipelineManagerVulkan.h index 7ee6770869..15e29864f1 100644 --- a/GPU/Vulkan/PipelineManagerVulkan.h +++ b/GPU/Vulkan/PipelineManagerVulkan.h @@ -64,6 +64,7 @@ enum { struct VulkanPipeline { VkPipeline pipeline; int uniformBlocks; // UB_ enum above. + bool useBlendConstant; }; class VulkanContext; diff --git a/GPU/Vulkan/StateMappingVulkan.cpp b/GPU/Vulkan/StateMappingVulkan.cpp index 107d667337..dc390ef2af 100644 --- a/GPU/Vulkan/StateMappingVulkan.cpp +++ b/GPU/Vulkan/StateMappingVulkan.cpp @@ -369,7 +369,7 @@ void DrawEngineVulkan::ConvertStateToVulkanKey(FramebufferManagerVulkan &fbManag } } -void DrawEngineVulkan::ApplyDrawStateLate(VkCommandBuffer cmd, bool applyStencilRef, uint8_t stencilRef) { +void DrawEngineVulkan::ApplyDrawStateLate(VkCommandBuffer cmd, bool applyStencilRef, uint8_t stencilRef, bool useBlendConstant) { // At this point, we know if the vertices are full alpha or not. // TODO: Set the nearest/linear here (since we correctly know if alpha/color tests are needed)? if (!gstate.isModeClear()) { @@ -401,9 +401,9 @@ void DrawEngineVulkan::ApplyDrawStateLate(VkCommandBuffer cmd, bool applyStencil if (applyStencilRef) { vkCmdSetStencilReference(cmd, VK_STENCIL_FRONT_AND_BACK, stencilRef); } - if (gstate_c.IsDirty(DIRTY_BLEND_STATE)) { + if (gstate_c.IsDirty(DIRTY_BLEND_STATE) && useBlendConstant) { float bc[4]; Uint8x4ToFloat4(bc, dynState_.blendColor); vkCmdSetBlendConstants(cmd, bc); } -} \ No newline at end of file +} diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp index 6565ceee6e..f9dd032ed9 100644 --- a/Windows/GPU/WindowsVulkanContext.cpp +++ b/Windows/GPU/WindowsVulkanContext.cpp @@ -124,17 +124,7 @@ static VkBool32 VKAPI_CALL Vulkan_Dbg(VkDebugReportFlagsEXT msgFlags, VkDebugRep } message << "[" << pLayerPrefix << "] " << ObjTypeToString(objType) << " Code " << msgCode << " : " << pMsg << "\n"; - if (msgCode == 2) // Useless perf warning - return false; - - // This seems like a bogus result when submitting two command buffers in one go, one creating the image, the other one using it. - if (msgCode == 6 && startsWith(pMsg, "Cannot submit cmd buffer using image")) - return false; - if (msgCode == 11) - return false; - // Silence "invalid reads of buffer data" - usually just uninitialized color buffers that will immediately get cleared due to our - // lacking clearing optimizations. - if (msgCode == 15 && objType == VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_MEMORY_EXT) + if (msgCode == 2) // Useless perf warning ("Vertex attribute at location X not consumed by vertex shader") return false; #ifdef _WIN32 diff --git a/ext/native/thin3d/thin3d_vulkan.cpp b/ext/native/thin3d/thin3d_vulkan.cpp index 5c619407fb..9b22558714 100644 --- a/ext/native/thin3d/thin3d_vulkan.cpp +++ b/ext/native/thin3d/thin3d_vulkan.cpp @@ -1898,56 +1898,64 @@ void VKContext::BindFramebufferAsRenderTarget(Framebuffer *fbo, const RenderPass // Now, if the image needs transitioning, let's transition. // The backbuffer does not, that's handled by VulkanContext. if (fb->color.layout != VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL) { - VkImageMemoryBarrier barrier{}; - barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; - barrier.oldLayout = fb->color.layout; - barrier.subresourceRange.layerCount = 1; - barrier.subresourceRange.levelCount = 1; - barrier.image = fb->color.image; - barrier.srcAccessMask = 0; + VkAccessFlagBits srcAccessMask; + VkPipelineStageFlagBits srcStage; switch (fb->color.layout) { case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - barrier.srcAccessMask = VK_ACCESS_SHADER_READ_BIT; + srcAccessMask = VK_ACCESS_SHADER_READ_BIT; + srcStage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; break; case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: - barrier.srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT; break; case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: - barrier.srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT; + break; + case VK_IMAGE_LAYOUT_UNDEFINED: + srcAccessMask = (VkAccessFlagBits)0; + srcStage = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; + break; + default: + assert(0); break; } - barrier.dstAccessMask = VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_COLOR_ATTACHMENT_READ_BIT; - barrier.newLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; - barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - // TODO: Double-check these flags. Should be fine. - vkCmdPipelineBarrier(cmd_, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier); - fb->color.layout = barrier.newLayout; + TransitionImageLayout2(cmd_, fb->color.image, VK_IMAGE_ASPECT_COLOR_BIT, + fb->color.layout, VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, + srcStage, VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT, + srcAccessMask, VK_ACCESS_COLOR_ATTACHMENT_READ_BIT | VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT); + fb->color.layout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL; } if (fb->depth.layout != VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL) { - VkImageMemoryBarrier barrier{}; - barrier.sType = VK_STRUCTURE_TYPE_IMAGE_MEMORY_BARRIER; - barrier.oldLayout = fb->depth.layout; - barrier.subresourceRange.layerCount = 1; - barrier.subresourceRange.levelCount = 1; - barrier.image = fb->depth.image; - barrier.srcAccessMask = 0; + VkAccessFlagBits srcAccessMask; + VkPipelineStageFlagBits srcStage; switch (fb->depth.layout) { case VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL: - barrier.srcAccessMask = VK_ACCESS_SHADER_READ_BIT; + srcAccessMask = VK_ACCESS_SHADER_READ_BIT; + srcStage = VK_PIPELINE_STAGE_FRAGMENT_SHADER_BIT; break; case VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL: - barrier.srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + srcAccessMask = VK_ACCESS_TRANSFER_READ_BIT; + srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT; break; case VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL: - barrier.srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT; + srcStage = VK_PIPELINE_STAGE_TRANSFER_BIT; + break; + case VK_IMAGE_LAYOUT_UNDEFINED: + srcAccessMask = (VkAccessFlagBits)0; + srcStage = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; + break; + default: + assert(0); break; } - barrier.dstAccessMask = VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT| VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT; - barrier.newLayout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; - barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT; - // TODO: Double-check these flags. Should be fine. - vkCmdPipelineBarrier(cmd_, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT, 0, 0, nullptr, 0, nullptr, 1, &barrier); - fb->depth.layout = barrier.newLayout; + TransitionImageLayout2(cmd_, fb->depth.image, VK_IMAGE_ASPECT_DEPTH_BIT | VK_IMAGE_ASPECT_STENCIL_BIT, + fb->depth.layout, VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL, + srcStage, VK_PIPELINE_STAGE_EARLY_FRAGMENT_TESTS_BIT | VK_PIPELINE_STAGE_LATE_FRAGMENT_TESTS_BIT, + srcAccessMask, VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_READ_BIT | VK_ACCESS_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT); + fb->depth.layout = VK_IMAGE_LAYOUT_DEPTH_STENCIL_ATTACHMENT_OPTIMAL; } renderPass = renderPasses_[RPIndex(rp.color, rp.depth)];