From e60268bd4274de510f5214d6eedafba30a253984 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 30 May 2019 03:07:40 -0400 Subject: VideoCommon/RenderBase: Use a std::string_view with CreateShaderFromSource() Greatly simplifies the overall interface when it comes to compiling shaders. Also allows getting rid of a std::string overload of the same name. Now std::string and const char* both go through the same function. --- .../Core/VideoBackends/Vulkan/ShaderCompiler.cpp | 44 ++++++++-------------- 1 file changed, 15 insertions(+), 29 deletions(-) (limited to 'Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp') diff --git a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp index a9c1fa2c19..e526ed2924 100644 --- a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp +++ b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp @@ -35,11 +35,6 @@ bool InitializeGlslang(); // Resource limits used when compiling shaders static const TBuiltInResource* GetCompilerResourceLimits(); -// Compile a shader to SPIR-V via glslang -static bool CompileShaderToSPV(SPIRVCodeVector* out_code, EShLanguage stage, - const char* stage_filename, const char* source_code, - size_t source_code_length, const char* header, size_t header_length); - // Regarding the UBO bind points, we subtract one from the binding index because // the OpenGL backend requires UBO #0 for non-block uniforms (at least on NV). // This allows us to share the same shaders but use bind point #0 in the Vulkan @@ -114,8 +109,7 @@ static const char SUBGROUP_HELPER_HEADER[] = R"( )"; bool CompileShaderToSPV(SPIRVCodeVector* out_code, EShLanguage stage, const char* stage_filename, - const char* source_code, size_t source_code_length, const char* header, - size_t header_length) + std::string_view source, std::string_view header) { if (!InitializeGlslang()) return false; @@ -129,16 +123,16 @@ bool CompileShaderToSPV(SPIRVCodeVector* out_code, EShLanguage stage, const char int default_version = 450; std::string full_source_code; - const char* pass_source_code = source_code; - int pass_source_code_length = static_cast(source_code_length); - if (header_length > 0) + const char* pass_source_code = source.data(); + int pass_source_code_length = static_cast(source.size()); + if (!header.empty()) { constexpr size_t subgroup_helper_header_length = ArraySize(SUBGROUP_HELPER_HEADER) - 1; - full_source_code.reserve(header_length + subgroup_helper_header_length + source_code_length); - full_source_code.append(header, header_length); + full_source_code.reserve(header.size() + subgroup_helper_header_length + source.size()); + full_source_code.append(header); if (g_vulkan_context->SupportsShaderSubgroupOperations()) full_source_code.append(SUBGROUP_HELPER_HEADER, subgroup_helper_header_length); - full_source_code.append(source_code, source_code_length); + full_source_code.append(source); pass_source_code = full_source_code.c_str(); pass_source_code_length = static_cast(full_source_code.length()); } @@ -362,32 +356,24 @@ const TBuiltInResource* GetCompilerResourceLimits() return &limits; } -bool CompileVertexShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length) +bool CompileVertexShader(SPIRVCodeVector* out_code, std::string_view source_code) { - return CompileShaderToSPV(out_code, EShLangVertex, "vs", source_code, source_code_length, - SHADER_HEADER, sizeof(SHADER_HEADER) - 1); + return CompileShaderToSPV(out_code, EShLangVertex, "vs", source_code, SHADER_HEADER); } -bool CompileGeometryShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length) +bool CompileGeometryShader(SPIRVCodeVector* out_code, std::string_view source_code) { - return CompileShaderToSPV(out_code, EShLangGeometry, "gs", source_code, source_code_length, - SHADER_HEADER, sizeof(SHADER_HEADER) - 1); + return CompileShaderToSPV(out_code, EShLangGeometry, "gs", source_code, SHADER_HEADER); } -bool CompileFragmentShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length) +bool CompileFragmentShader(SPIRVCodeVector* out_code, std::string_view source_code) { - return CompileShaderToSPV(out_code, EShLangFragment, "ps", source_code, source_code_length, - SHADER_HEADER, sizeof(SHADER_HEADER) - 1); + return CompileShaderToSPV(out_code, EShLangFragment, "ps", source_code, SHADER_HEADER); } -bool CompileComputeShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length) +bool CompileComputeShader(SPIRVCodeVector* out_code, std::string_view source_code) { - return CompileShaderToSPV(out_code, EShLangCompute, "cs", source_code, source_code_length, - COMPUTE_SHADER_HEADER, sizeof(COMPUTE_SHADER_HEADER) - 1); + return CompileShaderToSPV(out_code, EShLangCompute, "cs", source_code, COMPUTE_SHADER_HEADER); } } // namespace ShaderCompiler -- cgit v1.2.3 From d6a60050ffd667ec1bd9a1759fb103ab0af9cfce Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 30 May 2019 03:52:05 -0400 Subject: VideoVulkan/ShaderCompiler: Use a std::optional instead of bool+out variable Now that we utilize C++17, we can simply return an optional containing the code instead of using an out variable and a boolean result, essentially combining them into one. This provides a much more straightforward interface. --- .../Core/VideoBackends/Vulkan/ShaderCompiler.cpp | 35 +++++++++++----------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp') diff --git a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp index e526ed2924..107c9f4962 100644 --- a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp +++ b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp @@ -108,11 +108,11 @@ static const char SUBGROUP_HELPER_HEADER[] = R"( #define SUBGROUP_MAX(value) value = subgroupMax(value) )"; -bool CompileShaderToSPV(SPIRVCodeVector* out_code, EShLanguage stage, const char* stage_filename, - std::string_view source, std::string_view header) +std::optional CompileShaderToSPV(EShLanguage stage, const char* stage_filename, + std::string_view source, std::string_view header) { if (!InitializeGlslang()) - return false; + return std::nullopt; std::unique_ptr shader = std::make_unique(stage); std::unique_ptr program; @@ -172,7 +172,7 @@ bool CompileShaderToSPV(SPIRVCodeVector* out_code, EShLanguage stage, const char includer)) { DumpBadShader("Failed to parse shader"); - return false; + return std::nullopt; } // Even though there's only a single shader, we still need to link it to generate SPV @@ -181,18 +181,19 @@ bool CompileShaderToSPV(SPIRVCodeVector* out_code, EShLanguage stage, const char if (!program->link(messages)) { DumpBadShader("Failed to link program"); - return false; + return std::nullopt; } glslang::TIntermediate* intermediate = program->getIntermediate(stage); if (!intermediate) { DumpBadShader("Failed to generate SPIR-V"); - return false; + return std::nullopt; } + SPIRVCodeVector out_code; spv::SpvBuildLogger logger; - glslang::GlslangToSpv(*intermediate, *out_code, &logger); + glslang::GlslangToSpv(*intermediate, out_code, &logger); // Write out messages // Temporary: skip if it contains "Warning, version 450 is not yet complete; most version-specific @@ -230,11 +231,11 @@ bool CompileShaderToSPV(SPIRVCodeVector* out_code, EShLanguage stage, const char stream << "SPIR-V conversion messages: " << std::endl; stream << spv_messages; stream << "SPIR-V:" << std::endl; - spv::Disassemble(stream, *out_code); + spv::Disassemble(stream, out_code); } } - return true; + return out_code; } bool InitializeGlslang() @@ -356,24 +357,24 @@ const TBuiltInResource* GetCompilerResourceLimits() return &limits; } -bool CompileVertexShader(SPIRVCodeVector* out_code, std::string_view source_code) +std::optional CompileVertexShader(std::string_view source_code) { - return CompileShaderToSPV(out_code, EShLangVertex, "vs", source_code, SHADER_HEADER); + return CompileShaderToSPV(EShLangVertex, "vs", source_code, SHADER_HEADER); } -bool CompileGeometryShader(SPIRVCodeVector* out_code, std::string_view source_code) +std::optional CompileGeometryShader(std::string_view source_code) { - return CompileShaderToSPV(out_code, EShLangGeometry, "gs", source_code, SHADER_HEADER); + return CompileShaderToSPV(EShLangGeometry, "gs", source_code, SHADER_HEADER); } -bool CompileFragmentShader(SPIRVCodeVector* out_code, std::string_view source_code) +std::optional CompileFragmentShader(std::string_view source_code) { - return CompileShaderToSPV(out_code, EShLangFragment, "ps", source_code, SHADER_HEADER); + return CompileShaderToSPV(EShLangFragment, "ps", source_code, SHADER_HEADER); } -bool CompileComputeShader(SPIRVCodeVector* out_code, std::string_view source_code) +std::optional CompileComputeShader(std::string_view source_code) { - return CompileShaderToSPV(out_code, EShLangCompute, "cs", source_code, COMPUTE_SHADER_HEADER); + return CompileShaderToSPV(EShLangCompute, "cs", source_code, COMPUTE_SHADER_HEADER); } } // namespace ShaderCompiler -- cgit v1.2.3 From 1831dcbe6f3a2119bebc572654805045f0f0c473 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 30 May 2019 03:54:56 -0400 Subject: VideoVulkan/ShaderCompiler: Use non-member std::size instead of ArraySize() Now that we're on C++17, we can use its non-member std::size function instead of ours. This provides no functional change. --- Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp') diff --git a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp index 107c9f4962..e9551e319e 100644 --- a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp +++ b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp @@ -17,7 +17,6 @@ #include "ShaderLang.h" #include "disassemble.h" -#include "Common/CommonFuncs.h" #include "Common/FileUtil.h" #include "Common/Logging/Log.h" #include "Common/MsgHandler.h" @@ -127,7 +126,7 @@ std::optional CompileShaderToSPV(EShLanguage stage, const char* int pass_source_code_length = static_cast(source.size()); if (!header.empty()) { - constexpr size_t subgroup_helper_header_length = ArraySize(SUBGROUP_HELPER_HEADER) - 1; + constexpr size_t subgroup_helper_header_length = std::size(SUBGROUP_HELPER_HEADER) - 1; full_source_code.reserve(header.size() + subgroup_helper_header_length + source.size()); full_source_code.append(header); if (g_vulkan_context->SupportsShaderSubgroupOperations()) -- cgit v1.2.3