From a2eaad544559a52f5bf5075217010576e7fa178e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 12 Sep 2022 11:59:08 +0200 Subject: [PATCH 1/4] Debug-name shader modules with their shader desc strings --- GPU/Vulkan/ShaderManagerVulkan.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/GPU/Vulkan/ShaderManagerVulkan.cpp b/GPU/Vulkan/ShaderManagerVulkan.cpp index 9cb91e00c6..bcbc6356fe 100644 --- a/GPU/Vulkan/ShaderManagerVulkan.cpp +++ b/GPU/Vulkan/ShaderManagerVulkan.cpp @@ -44,7 +44,7 @@ // Most drivers treat vkCreateShaderModule as pretty much a memcpy. What actually // takes time here, and makes this worthy of parallelization, is GLSLtoSPV. -Promise *CompileShaderModuleAsync(VulkanContext *vulkan, VkShaderStageFlagBits stage, const char *code) { +static Promise *CompileShaderModuleAsync(VulkanContext *vulkan, VkShaderStageFlagBits stage, const char *code, const char *tag) { auto compile = [=] { PROFILE_THIS_SCOPE("shadercomp"); @@ -75,6 +75,9 @@ Promise *CompileShaderModuleAsync(VulkanContext *vulkan, VkShade #ifdef SHADERLOG OutputDebugStringA("OK"); #endif + if (tag) { + vulkan->SetDebugName(shaderModule, VK_OBJECT_TYPE_SHADER_MODULE, tag); + } } return shaderModule; @@ -92,7 +95,7 @@ Promise *CompileShaderModuleAsync(VulkanContext *vulkan, VkShade VulkanFragmentShader::VulkanFragmentShader(VulkanContext *vulkan, FShaderID id, FragmentShaderFlags flags, const char *code) : vulkan_(vulkan), id_(id), flags_(flags) { source_ = code; - module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_FRAGMENT_BIT, source_.c_str()); + module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_FRAGMENT_BIT, source_.c_str(), FragmentShaderDesc(id).c_str()); if (!module_) { failed_ = true; } else { @@ -122,7 +125,7 @@ std::string VulkanFragmentShader::GetShaderString(DebugShaderStringType type) co VulkanVertexShader::VulkanVertexShader(VulkanContext *vulkan, VShaderID id, const char *code, bool useHWTransform) : vulkan_(vulkan), useHWTransform_(useHWTransform), id_(id) { source_ = code; - module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_VERTEX_BIT, source_.c_str()); + module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_VERTEX_BIT, source_.c_str(), VertexShaderDesc(id).c_str()); if (!module_) { failed_ = true; } else { From d9989ffc16ab0b4f6c0ddda4abd14e6e5f432310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 12 Sep 2022 12:17:32 +0200 Subject: [PATCH 2/4] Don't start blending in the shader just because there's bitmasks, if blend is disabled. --- GPU/Common/GPUStateUtils.cpp | 4 +++- GPU/Common/GPUStateUtils.h | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/GPU/Common/GPUStateUtils.cpp b/GPU/Common/GPUStateUtils.cpp index 9efde97ae6..f801524b43 100644 --- a/GPU/Common/GPUStateUtils.cpp +++ b/GPU/Common/GPUStateUtils.cpp @@ -1058,8 +1058,10 @@ static void ConvertBlendState(GenericBlendState &blendState, bool forceReplaceBl ReplaceBlendType replaceBlend = ReplaceBlendWithShader(gstate_c.framebufFormat); if (forceReplaceBlend) { - replaceBlend = REPLACE_BLEND_READ_FRAMEBUFFER; + // Enforce blend replacement if enabled. If not, shouldn't do anything of course. + replaceBlend = blendState.blendEnabled ? REPLACE_BLEND_READ_FRAMEBUFFER : REPLACE_BLEND_NO; } + blendState.replaceBlend = replaceBlend; blendState.simulateLogicOpType = SimulateLogicOpShaderTypeIfNeeded(); diff --git a/GPU/Common/GPUStateUtils.h b/GPU/Common/GPUStateUtils.h index 2055da4657..bc0c9ae7b1 100644 --- a/GPU/Common/GPUStateUtils.h +++ b/GPU/Common/GPUStateUtils.h @@ -245,7 +245,9 @@ struct ComputedPipelineState { void Convert(bool shaderBitOpsSupported); bool FramebufferRead() const { - return blendState.applyFramebufferRead; + // If blending is off, its applyFramebufferRead can be false even after state propagation. + // So it's not enough to check just that one. + return blendState.applyFramebufferRead || maskState.applyFramebufferRead || logicState.applyFramebufferRead; } }; From 4ad345d78da066136058ac8d17a2772e8fa8c8d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 12 Sep 2022 14:02:15 +0200 Subject: [PATCH 3/4] Better shader compile logging --- GPU/Vulkan/ShaderManagerVulkan.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/GPU/Vulkan/ShaderManagerVulkan.cpp b/GPU/Vulkan/ShaderManagerVulkan.cpp index bcbc6356fe..f0028cbac6 100644 --- a/GPU/Vulkan/ShaderManagerVulkan.cpp +++ b/GPU/Vulkan/ShaderManagerVulkan.cpp @@ -59,12 +59,13 @@ static Promise *CompileShaderModuleAsync(VulkanContext *vulkan, } else { ERROR_LOG(G3D, "Error in shader compilation!"); } + std::string numberedSource = LineNumberString(code); ERROR_LOG(G3D, "Messages: %s", errorMessage.c_str()); - ERROR_LOG(G3D, "Shader source:\n%s", code); -#ifdef SHADERLOG - OutputDebugStringA(LineNumberString(code).c_str()); + ERROR_LOG(G3D, "Shader source:\n%s", numberedSource.c_str()); +#if PPSSPP_PLATFORM(WINDOWS) OutputDebugStringA("Error messages:\n"); OutputDebugStringA(errorMessage.c_str()); + OutputDebugStringA(numberedSource.c_str()); #endif Reporting::ReportMessage("Vulkan error in shader compilation: info: %s / code: %s", errorMessage.c_str(), code); } From b86648b352125d66ea128df4301f4a5ddd2ddcbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 12 Sep 2022 16:30:08 +0200 Subject: [PATCH 4/4] Address feedback --- GPU/Common/GPUStateUtils.cpp | 2 +- GPU/Vulkan/ShaderManagerVulkan.cpp | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/GPU/Common/GPUStateUtils.cpp b/GPU/Common/GPUStateUtils.cpp index f801524b43..7690156d9d 100644 --- a/GPU/Common/GPUStateUtils.cpp +++ b/GPU/Common/GPUStateUtils.cpp @@ -1059,7 +1059,7 @@ static void ConvertBlendState(GenericBlendState &blendState, bool forceReplaceBl ReplaceBlendType replaceBlend = ReplaceBlendWithShader(gstate_c.framebufFormat); if (forceReplaceBlend) { // Enforce blend replacement if enabled. If not, shouldn't do anything of course. - replaceBlend = blendState.blendEnabled ? REPLACE_BLEND_READ_FRAMEBUFFER : REPLACE_BLEND_NO; + replaceBlend = gstate.isAlphaBlendEnabled() ? REPLACE_BLEND_READ_FRAMEBUFFER : REPLACE_BLEND_NO; } blendState.replaceBlend = replaceBlend; diff --git a/GPU/Vulkan/ShaderManagerVulkan.cpp b/GPU/Vulkan/ShaderManagerVulkan.cpp index f0028cbac6..132b4fba20 100644 --- a/GPU/Vulkan/ShaderManagerVulkan.cpp +++ b/GPU/Vulkan/ShaderManagerVulkan.cpp @@ -44,7 +44,8 @@ // Most drivers treat vkCreateShaderModule as pretty much a memcpy. What actually // takes time here, and makes this worthy of parallelization, is GLSLtoSPV. -static Promise *CompileShaderModuleAsync(VulkanContext *vulkan, VkShaderStageFlagBits stage, const char *code, const char *tag) { +// Takes ownership over tag. +static Promise *CompileShaderModuleAsync(VulkanContext *vulkan, VkShaderStageFlagBits stage, const char *code, std::string *tag) { auto compile = [=] { PROFILE_THIS_SCOPE("shadercomp"); @@ -77,7 +78,8 @@ static Promise *CompileShaderModuleAsync(VulkanContext *vulkan, OutputDebugStringA("OK"); #endif if (tag) { - vulkan->SetDebugName(shaderModule, VK_OBJECT_TYPE_SHADER_MODULE, tag); + vulkan->SetDebugName(shaderModule, VK_OBJECT_TYPE_SHADER_MODULE, tag->c_str()); + delete tag; } } @@ -96,7 +98,7 @@ static Promise *CompileShaderModuleAsync(VulkanContext *vulkan, VulkanFragmentShader::VulkanFragmentShader(VulkanContext *vulkan, FShaderID id, FragmentShaderFlags flags, const char *code) : vulkan_(vulkan), id_(id), flags_(flags) { source_ = code; - module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_FRAGMENT_BIT, source_.c_str(), FragmentShaderDesc(id).c_str()); + module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_FRAGMENT_BIT, source_.c_str(), new std::string(FragmentShaderDesc(id))); if (!module_) { failed_ = true; } else { @@ -126,7 +128,7 @@ std::string VulkanFragmentShader::GetShaderString(DebugShaderStringType type) co VulkanVertexShader::VulkanVertexShader(VulkanContext *vulkan, VShaderID id, const char *code, bool useHWTransform) : vulkan_(vulkan), useHWTransform_(useHWTransform), id_(id) { source_ = code; - module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_VERTEX_BIT, source_.c_str(), VertexShaderDesc(id).c_str()); + module_ = CompileShaderModuleAsync(vulkan, VK_SHADER_STAGE_VERTEX_BIT, source_.c_str(), new std::string(VertexShaderDesc(id).c_str())); if (!module_) { failed_ = true; } else {