From 56e71c1dad35919fd4db2800746fd91e84785aad Mon Sep 17 00:00:00 2001 From: Hans-Kristian Arntzen Date: Mon, 31 Jul 2017 11:17:02 +0200 Subject: [PATCH] Do not emit layout(location) qualifiers for older GLSL targets. ESSL 300 and GLSL <410 do not support this along with legacy targets. --- README.md | 6 +++--- spirv_glsl.cpp | 30 +++++++++++++++++++++++------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 32606ad1..9c6e8746 100644 --- a/README.md +++ b/README.md @@ -236,9 +236,9 @@ This can be done with `Compiler::set_decoration(id, spv::DecorationDescriptorSet #### Linking by name for targets which do not support explicit locations (legacy GLSL/ESSL) -Modern GLSL and HLSL sources will rely on explicit layout(location) qualifiers to guide the linking process, -but legacy GLSL relies on symbol names to perform the linking. When emitting legacy shaders, these layout statements will be dropped, -so it is important that the API user ensures that the names of I/O variables are sanitized to ensure that linking will work properly. +Modern GLSL and HLSL sources (and SPIR-V) relies on explicit layout(location) qualifiers to guide the linking process between shader stages, +but older GLSL relies on symbol names to perform the linking. When emitting shaders with older versions, these layout statements will be removed, +so it is important that the API user ensures that the names of I/O variables are sanitized so that linking will work properly. The reflection API can rename variables, struct types and struct members to deal with these scenarios using `Compiler::set_name` and friends. ## Contributing diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp index 1dd2485b..19ec0df9 100644 --- a/spirv_glsl.cpp +++ b/spirv_glsl.cpp @@ -965,14 +965,30 @@ string CompilerGLSL::layout_for_variable(const SPIRVariable &var) if (flags & (1ull << DecorationLocation)) { - uint64_t combined_decoration = 0; - for (uint32_t i = 0; i < meta[type.self].members.size(); i++) - combined_decoration |= combined_decoration_for_member(type, i); + bool can_use_varying_location = true; - // If our members have location decorations, we don't need to - // emit location decorations at the top as well (looks weird). - if ((combined_decoration & (1ull << DecorationLocation)) == 0) - attr.push_back(join("location = ", dec.location)); + // Location specifiers are must have in SPIR-V, but they aren't really supported in earlier versions of GLSL. + // Be very explicit here about how to solve the issue. + if ((get_execution_model() != ExecutionModelVertex && var.storage == StorageClassInput) || + (get_execution_model() != ExecutionModelFragment && var.storage == StorageClassOutput)) + { + if (!options.es && options.version < 410 && !options.separate_shader_objects) + can_use_varying_location = false; + else if (options.es && options.version < 310) + can_use_varying_location = false; + } + + if (can_use_varying_location) + { + uint64_t combined_decoration = 0; + for (uint32_t i = 0; i < meta[type.self].members.size(); i++) + combined_decoration |= combined_decoration_for_member(type, i); + + // If our members have location decorations, we don't need to + // emit location decorations at the top as well (looks weird). + if ((combined_decoration & (1ull << DecorationLocation)) == 0) + attr.push_back(join("location = ", dec.location)); + } } // set = 0 is the default. Do not emit set = decoration in regular GLSL output, but