From 706fa3b65c63d0b71eba14e1f3c1b901678397e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Chr=C3=A9tien?= Date: Fri, 9 Feb 2018 18:08:58 +0100 Subject: [PATCH] Add -fauto-map-locations option See #369 --- glslc/src/main.cc | 6 +++ libshaderc/include/shaderc/shaderc.h | 5 ++ libshaderc/include/shaderc/shaderc.hpp | 6 +++ libshaderc/src/shaderc.cc | 5 ++ .../include/libshaderc_util/compiler.h | 9 ++++ libshaderc_util/src/compiler.cc | 1 + libshaderc_util/src/compiler_test.cc | 51 +++++++++++++++++++ 7 files changed, 83 insertions(+) diff --git a/glslc/src/main.cc b/glslc/src/main.cc index d7a61c97a..afcd87501 100644 --- a/glslc/src/main.cc +++ b/glslc/src/main.cc @@ -56,6 +56,10 @@ An input file of - represents standard input. Automatically assign bindings to uniform variables that don't have an explicit 'binding' layout in the shader source. + -fauto-map-locations + Automatically assign locations to uniform variables that + don't have an explicit 'location' layout in the shader + source. -fhlsl-iomap Use HLSL IO mappings for bindings. -fimage-binding-base [stage] Sets the lowest automatically assigned binding number for @@ -321,6 +325,8 @@ int main(int argc, char** argv) { } } else if (arg == "-fauto-bind-uniforms") { compiler.options().SetAutoBindUniforms(true); + } else if (arg == "-fauto-map-locations") { + compiler.options().SetAutoMapLocations(true); } else if (arg == "-fhlsl-iomap") { compiler.options().SetHlslIoMapping(true); } else if (arg == "-fhlsl-offsets") { diff --git a/libshaderc/include/shaderc/shaderc.h b/libshaderc/include/shaderc/shaderc.h index d24e67dc3..69dc09792 100644 --- a/libshaderc/include/shaderc/shaderc.h +++ b/libshaderc/include/shaderc/shaderc.h @@ -432,6 +432,11 @@ SHADERC_EXPORT void shaderc_compile_options_set_binding_base_for_stage( shaderc_compile_options_t options, shaderc_shader_kind shader_kind, shaderc_uniform_kind kind, uint32_t base); +// Sets whether the compiler should automatically assign locations to +// uniform variables that don't have explicit locations in the shader source. +SHADERC_EXPORT void shaderc_compile_options_set_auto_map_locations( + shaderc_compile_options_t options, bool auto_map); + // Sets a descriptor set and binding for an HLSL register in the given stage. // This method keeps a copy of the string data. SHADERC_EXPORT void shaderc_compile_options_set_hlsl_register_set_and_binding_for_stage( diff --git a/libshaderc/include/shaderc/shaderc.hpp b/libshaderc/include/shaderc/shaderc.hpp index 335f0df9d..f6fbeb323 100644 --- a/libshaderc/include/shaderc/shaderc.hpp +++ b/libshaderc/include/shaderc/shaderc.hpp @@ -286,6 +286,12 @@ class CompileOptions { kind, base); } + // Sets whether the compiler automatically assigns locations to + // uniform variables that don't have explicit locations. + void SetAutoMapLocations(bool auto_map) { + shaderc_compile_options_set_auto_map_locations(options_, auto_map); + } + // Sets a descriptor set and binding for an HLSL register in the given stage. // Copies the parameter strings. void SetHlslRegisterSetAndBindingForStage(shaderc_shader_kind shader_kind, diff --git a/libshaderc/src/shaderc.cc b/libshaderc/src/shaderc.cc index 38836b1d7..df9e29a2d 100644 --- a/libshaderc/src/shaderc.cc +++ b/libshaderc/src/shaderc.cc @@ -435,6 +435,11 @@ void shaderc_compile_options_set_binding_base_for_stage( GetUniformKind(kind), base); } +void shaderc_compile_options_set_auto_map_locations( + shaderc_compile_options_t options, bool auto_map) { + options->compiler.SetAutoMapLocations(auto_map); +} + void shaderc_compile_options_set_hlsl_register_set_and_binding_for_stage( shaderc_compile_options_t options, shaderc_shader_kind shader_kind, const char* reg, const char* set, const char* binding) { diff --git a/libshaderc_util/include/libshaderc_util/compiler.h b/libshaderc_util/include/libshaderc_util/compiler.h index 35c74dcd7..c94403f22 100644 --- a/libshaderc_util/include/libshaderc_util/compiler.h +++ b/libshaderc_util/include/libshaderc_util/compiler.h @@ -184,6 +184,7 @@ class Compiler { limits_(kDefaultTBuiltInResource), auto_bind_uniforms_(false), auto_binding_base_(), + auto_map_locations_(false), hlsl_iomap_(false), hlsl_offsets_(false), hlsl_legalization_enabled_(true), @@ -250,6 +251,10 @@ class Compiler { auto_binding_base_[static_cast(stage)][static_cast(kind)] = base; } + // Sets whether the compiler automatically assigns locations to + // uniform variables that don't have explicit locations. + void SetAutoMapLocations(bool auto_map) { auto_map_locations_ = auto_map; } + // Use HLSL IO mapping rules for bindings. Default is false. void SetHlslIoMapping(bool hlsl_iomap) { hlsl_iomap_ = hlsl_iomap; } @@ -439,6 +444,10 @@ class Compiler { // The default is zero. uint32_t auto_binding_base_[kNumStages][kNumUniformKinds]; + // True if the compiler should automatically map uniforms that don't + // have explicit locations. + bool auto_map_locations_; + // True if the compiler should use HLSL IO mapping rules when compiling HLSL. bool hlsl_iomap_; diff --git a/libshaderc_util/src/compiler.cc b/libshaderc_util/src/compiler.cc index da235495f..b9c062d32 100644 --- a/libshaderc_util/src/compiler.cc +++ b/libshaderc_util/src/compiler.cc @@ -210,6 +210,7 @@ std::tuple, size_t> Compiler::Compile( shader.setPreamble(preamble.c_str()); shader.setEntryPoint(entry_point_name); shader.setAutoMapBindings(auto_bind_uniforms_); + shader.setAutoMapLocations(auto_map_locations_); const auto& bases = auto_binding_base_[static_cast(used_shader_stage)]; shader.setShiftImageBinding(bases[static_cast(UniformKind::Image)]); shader.setShiftSamplerBinding(bases[static_cast(UniformKind::Sampler)]); diff --git a/libshaderc_util/src/compiler_test.cc b/libshaderc_util/src/compiler_test.cc index fa447d1f1..06b0da4b0 100644 --- a/libshaderc_util/src/compiler_test.cc +++ b/libshaderc_util/src/compiler_test.cc @@ -104,6 +104,26 @@ const char kGlslFragShaderNoExplicitBinding[] = float x = my_ubo.x; })"; +// A GLSL vertex shader with the location defined for its non-opaque uniform +// variable. +const char kGlslVertShaderExplicitLocation[] = + R"(#version 450 + layout(location = 10) uniform mat4 my_mat; + layout(location = 0) in vec4 my_vec; + void main(void) { + gl_Position = my_mat * my_vec; + })"; + +// A GLSL vertex shader without the location defined for its non-opaque uniform +// variable. +const char kGlslVertShaderNoExplicitLocation[] = + R"(#version 450 + uniform mat4 my_mat; + layout(location = 0) in vec4 my_vec; + void main(void) { + gl_Position = my_mat * my_vec; + })"; + // A GLSL vertex shader with a weirdly packed block. const char kGlslShaderWeirdPacking[] = R"(#version 450 @@ -636,6 +656,37 @@ TEST_F(CompilerTest, AutoMapBindingsSetsBindingsSetFragImageBindingBaseCompiledA EXPECT_THAT(disassembly, HasSubstr("OpDecorate %my_ubo Binding 4")); } +TEST_F(CompilerTest, NoAutoMapLocationsFailsCompilationOnOpenGLShader) { + compiler_.SetTargetEnv(Compiler::TargetEnv::OpenGL); + compiler_.SetAutoMapLocations(false); + + const auto words = SimpleCompilationBinary(kGlslVertShaderExplicitLocation, + EShLangVertex); + const auto disassembly = Disassemble(words); + EXPECT_THAT(disassembly, HasSubstr("OpDecorate %my_mat Location 10")) + << disassembly; + + EXPECT_FALSE( + SimpleCompilationSucceeds(kGlslVertShaderNoExplicitLocation, EShLangVertex)); +} + +TEST_F(CompilerTest, AutoMapLocationsSetsLocationsOnOpenGLShader) { + compiler_.SetTargetEnv(Compiler::TargetEnv::OpenGL); + compiler_.SetAutoMapLocations(true); + + const auto words_no_auto = + SimpleCompilationBinary(kGlslVertShaderExplicitLocation, EShLangVertex); + const auto disassembly_no_auto = Disassemble(words_no_auto); + EXPECT_THAT(disassembly_no_auto, HasSubstr("OpDecorate %my_mat Location 10")) + << disassembly_no_auto; + + const auto words_auto = + SimpleCompilationBinary(kGlslVertShaderNoExplicitLocation, EShLangVertex); + const auto disassembly_auto = Disassemble(words_auto); + EXPECT_THAT(disassembly_auto, HasSubstr("OpDecorate %my_mat Location 0")) + << disassembly_auto; +} + TEST_F(CompilerTest, EmitMessageTextOnlyOnce) { // Emit a warning by compiling a shader without a default entry point name. // The warning should only be emitted once even though we do parsing, linking,