From 387d12db42da8a0c04032e47d33480d3d9f144ec Mon Sep 17 00:00:00 2001 From: Jordan Justen Date: Fri, 21 Jan 2022 01:24:45 -0800 Subject: [PATCH] glsl_shaders: Update setEnvInput() call for non-SPIR-V GL shaders Ref: https://github.com/KhronosGroup/glslang/issues/2872 Ref: https://github.com/KhronosGroup/glslang/commit/81cc10a498b25a90147cccd6e8939493c1e9e20e After discussions with the glslang developers, it appears that the parameters in glslang's setEnvInput() function are a bit more geared towards SPIRV that the documentation implied. After the glslang commit above, and discussions with the developer, it seems that the 3rd parameter to setEnvInput() should only be set to EShClientOpenGL if you want to enable the ARB_gl_spirv extension. The 4th parameter to setEnvInput() is version, but it is used to indicate the version of either the KHR_vulkan_glsl extention for vulkan, or the ARB_gl_spirv extension for OpenGL with SPIR-V. So, if the shader type is neither ShaderType::Vulkan, nor ShaderType::GLSPIRV, we should specify glslang::EShClientNone for the 3rd parameter and 0 for the 4th parameter of setEnvInput(). This doesn't actually impact renderdoc with the currently bundled glslang 8.13.3743, because they failed to pass the environment through during pre-processing. But, this changed in this glslang commit in 11.2.0: * 6274ec5c ("Pass environment through PreprocessDeferred") * https://github.com/KhronosGroup/glslang/commit/6274ec5c After this change, renderdoc passes the environment through to the pre-processor, and with the current parameters to setEnvInput(), glslang will start to define GL_SPIRV in the shaders during pre-processing. Then, in renderdoc's glsl_globals.h, GL_SPIRV causes the pre-processor to set: * #define IO_LOCATION(l) layout(location = l) which then causes Mesa to produce this error when compiling the shader: * GL_VERTEX_SHADER compile error: 0:239(2): error: shader output explicit location requires GL_ARB_separate_shader_objects extension or GLSL 4.20 Signed-off-by: Jordan Justen --- renderdoc/data/glsl_shaders.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/renderdoc/data/glsl_shaders.cpp b/renderdoc/data/glsl_shaders.cpp index 24d517a4c..d11cd0b88 100644 --- a/renderdoc/data/glsl_shaders.cpp +++ b/renderdoc/data/glsl_shaders.cpp @@ -111,12 +111,15 @@ rdcstr GenerateGLSLShader(const rdcstr &shader, ShaderType type, int version, co const char *c_src = combined.c_str(); glslang::EShClient client = - type == ShaderType::Vulkan ? glslang::EShClientVulkan : glslang::EShClientOpenGL; + type == ShaderType::Vulkan ? glslang::EShClientVulkan : type == ShaderType::GLSPIRV + ? glslang::EShClientOpenGL + : glslang::EShClientNone; glslang::EShTargetClientVersion targetversion = type == ShaderType::Vulkan ? glslang::EShTargetVulkan_1_0 : glslang::EShTargetOpenGL_450; + int inputVersion = client != glslang::EShClientNone ? 100 : 0; sh.setStrings(&c_src, 1); - sh.setEnvInput(glslang::EShSourceGlsl, EShLangFragment, client, 100); + sh.setEnvInput(glslang::EShSourceGlsl, EShLangFragment, client, inputVersion); sh.setEnvClient(client, targetversion); sh.setEnvTarget(glslang::EShTargetNone, glslang::EShTargetSpv_1_0);