From 254160691a1944db5199191e43ed5c5947059018 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Sun, 3 Sep 2017 15:30:52 +1000 Subject: [PATCH] Revert "Vulkan: Use VK_NV_glsl extension where available, and skip glslang" This reverts commit d23fd17e1a2c7c617731d619020af021a7658e93. Dynamic sampler indexing is broken in VK_NV_glsl as of 385.41. The performance gap doesn't seem to be as wide with the updated driver, so to save maintaining two code paths, it's easier to just drop the extension support completely. --- .../VideoBackends/Vulkan/ShaderCompiler.cpp | 78 +------------------ .../VideoBackends/Vulkan/ShaderCompiler.h | 8 +- .../VideoBackends/Vulkan/TextureConverter.cpp | 2 +- Source/Core/VideoBackends/Vulkan/Util.cpp | 28 +++++-- Source/Core/VideoBackends/Vulkan/Util.h | 12 ++- .../VideoBackends/Vulkan/VulkanContext.cpp | 9 +-- .../Core/VideoBackends/Vulkan/VulkanContext.h | 3 - 7 files changed, 38 insertions(+), 102 deletions(-) diff --git a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp index 1846ebbbcd..75bf99a3ba 100644 --- a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp +++ b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.cpp @@ -3,7 +3,6 @@ // Refer to the license.txt file included. #include "VideoBackends/Vulkan/ShaderCompiler.h" -#include "VideoBackends/Vulkan/VulkanContext.h" #include #include @@ -38,11 +37,6 @@ 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); -// Copy GLSL source code to a SPIRVCodeVector, for use with VK_NV_glsl_shader. -static void CopyGLSLToSPVVector(SPIRVCodeVector* out_code, 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 @@ -225,42 +219,6 @@ bool CompileShaderToSPV(SPIRVCodeVector* out_code, EShLanguage stage, const char return true; } -void CopyGLSLToSPVVector(SPIRVCodeVector* out_code, const char* stage_filename, - const char* source_code, size_t source_code_length, const char* header, - size_t header_length) -{ - std::string full_source_code; - if (header_length > 0) - { - full_source_code.reserve(header_length + source_code_length); - full_source_code.append(header, header_length); - full_source_code.append(source_code, source_code_length); - } - else - { - full_source_code.append(source_code, source_code_length); - } - - if (g_ActiveConfig.iLog & CONF_SAVESHADERS) - { - static int counter = 0; - std::string filename = StringFromFormat("%s%s_%04i.txt", File::GetUserPath(D_DUMP_IDX).c_str(), - stage_filename, counter++); - - std::ofstream stream; - File::OpenFStream(stream, filename, std::ios_base::out); - if (stream.good()) - stream << full_source_code << std::endl; - } - - size_t padding = full_source_code.size() % 4; - if (padding != 0) - full_source_code.append(4 - padding, '\n'); - - out_code->resize(full_source_code.size() / 4); - std::memcpy(out_code->data(), full_source_code.c_str(), full_source_code.size()); -} - bool InitializeGlslang() { static bool glslang_initialized = false; @@ -380,57 +338,29 @@ const TBuiltInResource* GetCompilerResourceLimits() } bool CompileVertexShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length) + size_t source_code_length, bool prepend_header) { - if (g_vulkan_context->SupportsNVGLSLExtension()) - { - CopyGLSLToSPVVector(out_code, "vs", source_code, source_code_length, SHADER_HEADER, - sizeof(SHADER_HEADER) - 1); - return true; - } - return CompileShaderToSPV(out_code, EShLangVertex, "vs", source_code, source_code_length, SHADER_HEADER, sizeof(SHADER_HEADER) - 1); } bool CompileGeometryShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length) + size_t source_code_length, bool prepend_header) { - if (g_vulkan_context->SupportsNVGLSLExtension()) - { - CopyGLSLToSPVVector(out_code, "gs", source_code, source_code_length, SHADER_HEADER, - sizeof(SHADER_HEADER) - 1); - return true; - } - return CompileShaderToSPV(out_code, EShLangGeometry, "gs", source_code, source_code_length, SHADER_HEADER, sizeof(SHADER_HEADER) - 1); } bool CompileFragmentShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length) + size_t source_code_length, bool prepend_header) { - if (g_vulkan_context->SupportsNVGLSLExtension()) - { - CopyGLSLToSPVVector(out_code, "ps", source_code, source_code_length, SHADER_HEADER, - sizeof(SHADER_HEADER) - 1); - return true; - } - return CompileShaderToSPV(out_code, EShLangFragment, "ps", source_code, source_code_length, SHADER_HEADER, sizeof(SHADER_HEADER) - 1); } bool CompileComputeShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length) + size_t source_code_length, bool prepend_header) { - if (g_vulkan_context->SupportsNVGLSLExtension()) - { - CopyGLSLToSPVVector(out_code, "cs", source_code, source_code_length, COMPUTE_SHADER_HEADER, - sizeof(COMPUTE_SHADER_HEADER) - 1); - return true; - } - return CompileShaderToSPV(out_code, EShLangCompute, "cs", source_code, source_code_length, COMPUTE_SHADER_HEADER, sizeof(COMPUTE_SHADER_HEADER) - 1); } diff --git a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.h b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.h index 84434b1ad6..197dc1787c 100644 --- a/Source/Core/VideoBackends/Vulkan/ShaderCompiler.h +++ b/Source/Core/VideoBackends/Vulkan/ShaderCompiler.h @@ -19,19 +19,19 @@ using SPIRVCodeVector = std::vector; // Compile a vertex shader to SPIR-V. bool CompileVertexShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length); + size_t source_code_length, bool prepend_header = true); // Compile a geometry shader to SPIR-V. bool CompileGeometryShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length); + size_t source_code_length, bool prepend_header = true); // Compile a fragment shader to SPIR-V. bool CompileFragmentShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length); + size_t source_code_length, bool prepend_header = true); // Compile a compute shader to SPIR-V. bool CompileComputeShader(SPIRVCodeVector* out_code, const char* source_code, - size_t source_code_length); + size_t source_code_length, bool prepend_header = true); } // namespace ShaderCompiler } // namespace Vulkan diff --git a/Source/Core/VideoBackends/Vulkan/TextureConverter.cpp b/Source/Core/VideoBackends/Vulkan/TextureConverter.cpp index 15d0ae7572..fc5a7c20f2 100644 --- a/Source/Core/VideoBackends/Vulkan/TextureConverter.cpp +++ b/Source/Core/VideoBackends/Vulkan/TextureConverter.cpp @@ -407,7 +407,7 @@ bool TextureConverter::SupportsTextureDecoding(TextureFormat format, TLUTFormat std::string shader_source = TextureConversionShader::GenerateDecodingShader(format, palette_format, APIType::Vulkan); - pipeline.compute_shader = Util::CompileAndCreateComputeShader(shader_source); + pipeline.compute_shader = Util::CompileAndCreateComputeShader(shader_source, true); if (pipeline.compute_shader == VK_NULL_HANDLE) { m_decoding_pipelines.emplace(key, pipeline); diff --git a/Source/Core/VideoBackends/Vulkan/Util.cpp b/Source/Core/VideoBackends/Vulkan/Util.cpp index e377bb32cf..9796811978 100644 --- a/Source/Core/VideoBackends/Vulkan/Util.cpp +++ b/Source/Core/VideoBackends/Vulkan/Util.cpp @@ -282,38 +282,50 @@ VkShaderModule CreateShaderModule(const u32* spv, size_t spv_word_count) return module; } -VkShaderModule CompileAndCreateVertexShader(const std::string& source_code) +VkShaderModule CompileAndCreateVertexShader(const std::string& source_code, bool prepend_header) { ShaderCompiler::SPIRVCodeVector code; - if (!ShaderCompiler::CompileVertexShader(&code, source_code.c_str(), source_code.length())) + if (!ShaderCompiler::CompileVertexShader(&code, source_code.c_str(), source_code.length(), + prepend_header)) + { return VK_NULL_HANDLE; + } return CreateShaderModule(code.data(), code.size()); } -VkShaderModule CompileAndCreateGeometryShader(const std::string& source_code) +VkShaderModule CompileAndCreateGeometryShader(const std::string& source_code, bool prepend_header) { ShaderCompiler::SPIRVCodeVector code; - if (!ShaderCompiler::CompileGeometryShader(&code, source_code.c_str(), source_code.length())) + if (!ShaderCompiler::CompileGeometryShader(&code, source_code.c_str(), source_code.length(), + prepend_header)) + { return VK_NULL_HANDLE; + } return CreateShaderModule(code.data(), code.size()); } -VkShaderModule CompileAndCreateFragmentShader(const std::string& source_code) +VkShaderModule CompileAndCreateFragmentShader(const std::string& source_code, bool prepend_header) { ShaderCompiler::SPIRVCodeVector code; - if (!ShaderCompiler::CompileFragmentShader(&code, source_code.c_str(), source_code.length())) + if (!ShaderCompiler::CompileFragmentShader(&code, source_code.c_str(), source_code.length(), + prepend_header)) + { return VK_NULL_HANDLE; + } return CreateShaderModule(code.data(), code.size()); } -VkShaderModule CompileAndCreateComputeShader(const std::string& source_code) +VkShaderModule CompileAndCreateComputeShader(const std::string& source_code, bool prepend_header) { ShaderCompiler::SPIRVCodeVector code; - if (!ShaderCompiler::CompileComputeShader(&code, source_code.c_str(), source_code.length())) + if (!ShaderCompiler::CompileComputeShader(&code, source_code.c_str(), source_code.length(), + prepend_header)) + { return VK_NULL_HANDLE; + } return CreateShaderModule(code.data(), code.size()); } diff --git a/Source/Core/VideoBackends/Vulkan/Util.h b/Source/Core/VideoBackends/Vulkan/Util.h index 6847409e9b..c99f0e968c 100644 --- a/Source/Core/VideoBackends/Vulkan/Util.h +++ b/Source/Core/VideoBackends/Vulkan/Util.h @@ -61,16 +61,20 @@ void ExecuteCurrentCommandsAndRestoreState(bool execute_off_thread, VkShaderModule CreateShaderModule(const u32* spv, size_t spv_word_count); // Compile a vertex shader and create a shader module, discarding the intermediate SPIR-V. -VkShaderModule CompileAndCreateVertexShader(const std::string& source_code); +VkShaderModule CompileAndCreateVertexShader(const std::string& source_code, + bool prepend_header = true); // Compile a geometry shader and create a shader module, discarding the intermediate SPIR-V. -VkShaderModule CompileAndCreateGeometryShader(const std::string& source_code); +VkShaderModule CompileAndCreateGeometryShader(const std::string& source_code, + bool prepend_header = true); // Compile a fragment shader and create a shader module, discarding the intermediate SPIR-V. -VkShaderModule CompileAndCreateFragmentShader(const std::string& source_code); +VkShaderModule CompileAndCreateFragmentShader(const std::string& source_code, + bool prepend_header = true); // Compile a compute shader and create a shader module, discarding the intermediate SPIR-V. -VkShaderModule CompileAndCreateComputeShader(const std::string& source_code); +VkShaderModule CompileAndCreateComputeShader(const std::string& source_code, + bool prepend_header = true); } // Utility shader vertex format diff --git a/Source/Core/VideoBackends/Vulkan/VulkanContext.cpp b/Source/Core/VideoBackends/Vulkan/VulkanContext.cpp index 8bb7a5088e..e80e391305 100644 --- a/Source/Core/VideoBackends/Vulkan/VulkanContext.cpp +++ b/Source/Core/VideoBackends/Vulkan/VulkanContext.cpp @@ -402,8 +402,7 @@ bool VulkanContext::SelectDeviceExtensions(ExtensionList* extension_list, bool e for (const auto& extension_properties : available_extension_list) INFO_LOG(VIDEO, "Available extension: %s", extension_properties.extensionName); - auto CheckForExtension = [&](const char* name, bool required, - bool* has_extension = nullptr) -> bool { + auto CheckForExtension = [&](const char* name, bool required) -> bool { if (std::find_if(available_extension_list.begin(), available_extension_list.end(), [&](const VkExtensionProperties& properties) { return !strcmp(name, properties.extensionName); @@ -411,14 +410,9 @@ bool VulkanContext::SelectDeviceExtensions(ExtensionList* extension_list, bool e { INFO_LOG(VIDEO, "Enabling extension: %s", name); extension_list->push_back(name); - if (has_extension) - *has_extension = true; return true; } - if (has_extension) - *has_extension = false; - if (required) { ERROR_LOG(VIDEO, "Vulkan: Missing required extension %s.", name); @@ -431,7 +425,6 @@ bool VulkanContext::SelectDeviceExtensions(ExtensionList* extension_list, bool e if (enable_surface && !CheckForExtension(VK_KHR_SWAPCHAIN_EXTENSION_NAME, true)) return false; - CheckForExtension(VK_NV_GLSL_SHADER_EXTENSION_NAME, false, &m_supports_nv_glsl_extension); return true; } diff --git a/Source/Core/VideoBackends/Vulkan/VulkanContext.h b/Source/Core/VideoBackends/Vulkan/VulkanContext.h index 17a4a25d2c..538573fc0a 100644 --- a/Source/Core/VideoBackends/Vulkan/VulkanContext.h +++ b/Source/Core/VideoBackends/Vulkan/VulkanContext.h @@ -81,7 +81,6 @@ public: { return m_device_features.occlusionQueryPrecise == VK_TRUE; } - bool SupportsNVGLSLExtension() const { return m_supports_nv_glsl_extension; } // Helpers for getting constants VkDeviceSize GetUniformBufferAlignment() const { @@ -126,8 +125,6 @@ private: VkPhysicalDeviceFeatures m_device_features = {}; VkPhysicalDeviceProperties m_device_properties = {}; VkPhysicalDeviceMemoryProperties m_device_memory_properties = {}; - - bool m_supports_nv_glsl_extension = false; }; extern std::unique_ptr g_vulkan_context;