From 40b0242fcc29e0784887d2772053ff8b4f153c0b Mon Sep 17 00:00:00 2001 From: Andre Weissflog Date: Mon, 1 Jul 2024 16:59:32 +0200 Subject: [PATCH 1/3] don't generate duplicate vertex attr defines/constants (fixes #133) --- src/shdc/generators/generator.cc | 10 +++--- src/shdc/generators/generator.h | 4 +-- src/shdc/generators/sokolc.cc | 8 ++--- src/shdc/generators/sokolc.h | 4 +-- src/shdc/generators/sokold.cc | 8 ++--- src/shdc/generators/sokold.h | 4 +-- src/shdc/generators/sokolnim.cc | 8 ++--- src/shdc/generators/sokolnim.h | 4 +-- src/shdc/generators/sokolodin.cc | 8 ++--- src/shdc/generators/sokolodin.h | 4 +-- src/shdc/generators/sokolrust.cc | 8 ++--- src/shdc/generators/sokolrust.h | 4 +-- src/shdc/generators/sokolzig.cc | 8 ++--- src/shdc/generators/sokolzig.h | 4 +-- src/shdc/reflection.cc | 48 ++++++++++++++++++++++++-- src/shdc/reflection.h | 6 ++-- src/shdc/types/reflection/stage_attr.h | 22 ++++++++---- 17 files changed, 107 insertions(+), 55 deletions(-) diff --git a/src/shdc/generators/generator.cc b/src/shdc/generators/generator.cc index 1b80846b..c18cd7d2 100644 --- a/src/shdc/generators/generator.cc +++ b/src/shdc/generators/generator.cc @@ -94,7 +94,7 @@ void Generator::gen_vertex_shader_info(const GenInput& gen, const ProgramReflect cbl_open("Attributes:\n"); for (const StageAttr& attr: prog.vs().inputs) { if (attr.slot >= 0) { - cbl("{} => {}\n", vertex_attr_name(prog.vs_name(), attr), attr.slot); + cbl("{} => {}\n", vertex_attr_name(attr), attr.slot); } } cbl_close(); @@ -144,11 +144,9 @@ void Generator::gen_bindings_info(const GenInput& gen, const Bindings& bindings) } void Generator::gen_vertex_attr_consts(const GenInput& gen) { - for (const ProgramReflection& prog_refl: gen.refl.progs) { - for (const StageAttr& attr: prog_refl.vs().inputs) { - if (attr.slot >= 0) { - l("{}\n", vertex_attr_definition(prog_refl.vs_name(), attr)); - } + for (const StageAttr& attr: gen.refl.unique_vs_inputs) { + if (attr.slot >= 0) { + l("{}\n", vertex_attr_definition(attr)); } } } diff --git a/src/shdc/generators/generator.h b/src/shdc/generators/generator.h index 77a943a4..aadb7049 100644 --- a/src/shdc/generators/generator.h +++ b/src/shdc/generators/generator.h @@ -76,13 +76,13 @@ class Generator { virtual std::string backend(Slang::Enum e) { assert(false && "implement me"); return ""; }; virtual std::string struct_name(const std::string& name) { assert(false && "implement me"); return ""; }; - virtual std::string vertex_attr_name(const std::string& snippet_name, const refl::StageAttr& attr) { assert(false && "implement me"); return ""; }; + virtual std::string vertex_attr_name(const refl::StageAttr& attr) { assert(false && "implement me"); return ""; }; virtual std::string image_bind_slot_name(const refl::Image& img) { assert(false && "implement me"); return ""; }; virtual std::string sampler_bind_slot_name(const refl::Sampler& smp) { assert(false && "implement me"); return ""; }; virtual std::string uniform_block_bind_slot_name(const refl::UniformBlock& ub) { assert(false && "implement me"); return ""; }; virtual std::string storage_buffer_bind_slot_name(const refl::StorageBuffer& sbuf) { assert(false && "implement me"); return ""; }; - virtual std::string vertex_attr_definition(const std::string& snippet_name, const refl::StageAttr& attr) { assert(false && "implement me"); return ""; }; + virtual std::string vertex_attr_definition(const refl::StageAttr& attr) { assert(false && "implement me"); return ""; }; virtual std::string image_bind_slot_definition(const refl::Image& img) { assert(false && "implement me"); return ""; }; virtual std::string sampler_bind_slot_definition(const refl::Sampler& smp) { assert(false && "implement me"); return ""; }; virtual std::string uniform_block_bind_slot_definition(const refl::UniformBlock& ub) { assert(false && "implement me"); return ""; }; diff --git a/src/shdc/generators/sokolc.cc b/src/shdc/generators/sokolc.cc index 59ba2fef..5c264f7f 100644 --- a/src/shdc/generators/sokolc.cc +++ b/src/shdc/generators/sokolc.cc @@ -682,8 +682,8 @@ std::string SokolCGenerator::struct_name(const std::string& name) { return fmt::format("{}{}_t", mod_prefix, name); } -std::string SokolCGenerator::vertex_attr_name(const std::string& snippet_name, const StageAttr& attr) { - return fmt::format("ATTR_{}{}_{}", mod_prefix, snippet_name, attr.name); +std::string SokolCGenerator::vertex_attr_name(const StageAttr& attr) { + return fmt::format("ATTR_{}{}_{}", mod_prefix, attr.snippet_name, attr.name); } std::string SokolCGenerator::image_bind_slot_name(const Image& img) { @@ -702,8 +702,8 @@ std::string SokolCGenerator::storage_buffer_bind_slot_name(const StorageBuffer& return fmt::format("SLOT_{}{}", mod_prefix, sbuf.struct_info.name); } -std::string SokolCGenerator::vertex_attr_definition(const std::string& snippet_name, const StageAttr& attr) { - return fmt::format("#define {} ({})", vertex_attr_name(snippet_name, attr), attr.slot); +std::string SokolCGenerator::vertex_attr_definition(const StageAttr& attr) { + return fmt::format("#define {} ({})", vertex_attr_name(attr), attr.slot); } std::string SokolCGenerator::image_bind_slot_definition(const Image& img) { diff --git a/src/shdc/generators/sokolc.h b/src/shdc/generators/sokolc.h index 188e878e..2ce8eb8d 100644 --- a/src/shdc/generators/sokolc.h +++ b/src/shdc/generators/sokolc.h @@ -40,12 +40,12 @@ class SokolCGenerator: public Generator { virtual std::string sampler_type(refl::SamplerType::Enum e); virtual std::string backend(Slang::Enum e); virtual std::string struct_name(const std::string& name); - virtual std::string vertex_attr_name(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_name(const refl::StageAttr& attr); virtual std::string image_bind_slot_name(const refl::Image& img); virtual std::string sampler_bind_slot_name(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_name(const refl::UniformBlock& ub); virtual std::string storage_buffer_bind_slot_name(const refl::StorageBuffer& sbuf); - virtual std::string vertex_attr_definition(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_definition(const refl::StageAttr& attr); virtual std::string image_bind_slot_definition(const refl::Image& img); virtual std::string sampler_bind_slot_definition(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_definition(const refl::UniformBlock& ub); diff --git a/src/shdc/generators/sokold.cc b/src/shdc/generators/sokold.cc index 264d541c..3e4d8083 100644 --- a/src/shdc/generators/sokold.cc +++ b/src/shdc/generators/sokold.cc @@ -433,8 +433,8 @@ std::string SokolDGenerator::struct_name(const std::string& name) { return to_pascal_case(name); } -std::string SokolDGenerator::vertex_attr_name(const std::string& snippet_name, const StageAttr& attr) { - return pystring::upper(fmt::format("ATTR_{}_{}", snippet_name, attr.name)); +std::string SokolDGenerator::vertex_attr_name(const StageAttr& attr) { + return pystring::upper(fmt::format("ATTR_{}_{}", attr.snippet_name, attr.name)); } static std::string slot_name(const std::string& name) { @@ -461,8 +461,8 @@ static std::string const_def(const std::string& name, int slot) { return fmt::format("enum {} = {};", name, slot); } -std::string SokolDGenerator::vertex_attr_definition(const std::string& snippet_name, const StageAttr& attr) { - return const_def(vertex_attr_name(snippet_name, attr), attr.slot); +std::string SokolDGenerator::vertex_attr_definition(const StageAttr& attr) { + return const_def(vertex_attr_name(attr), attr.slot); } std::string SokolDGenerator::image_bind_slot_definition(const Image& img) { diff --git a/src/shdc/generators/sokold.h b/src/shdc/generators/sokold.h index c9e02c6e..f87d10b5 100644 --- a/src/shdc/generators/sokold.h +++ b/src/shdc/generators/sokold.h @@ -27,12 +27,12 @@ class SokolDGenerator : public Generator { virtual std::string sampler_type(refl::SamplerType::Enum e); virtual std::string backend(Slang::Enum e); virtual std::string struct_name(const std::string& name); - virtual std::string vertex_attr_name(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_name(const refl::StageAttr& attr); virtual std::string image_bind_slot_name(const refl::Image& img); virtual std::string sampler_bind_slot_name(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_name(const refl::UniformBlock& ub); virtual std::string storage_buffer_bind_slot_name(const refl::StorageBuffer& sbuf); - virtual std::string vertex_attr_definition(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_definition(const refl::StageAttr& attr); virtual std::string image_bind_slot_definition(const refl::Image& img); virtual std::string sampler_bind_slot_definition(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_definition(const refl::UniformBlock& ub); diff --git a/src/shdc/generators/sokolnim.cc b/src/shdc/generators/sokolnim.cc index a1885176..2db7437d 100644 --- a/src/shdc/generators/sokolnim.cc +++ b/src/shdc/generators/sokolnim.cc @@ -506,8 +506,8 @@ std::string SokolNimGenerator::struct_name(const std::string& name) { return to_pascal_case(name); } -std::string SokolNimGenerator::vertex_attr_name(const std::string& snippet_name, const StageAttr& attr) { - return to_camel_case(fmt::format("ATTR_{}_{}", snippet_name, attr.name)); +std::string SokolNimGenerator::vertex_attr_name(const StageAttr& attr) { + return to_camel_case(fmt::format("ATTR_{}_{}", attr.snippet_name, attr.name)); } std::string SokolNimGenerator::image_bind_slot_name(const Image& img) { @@ -526,8 +526,8 @@ std::string SokolNimGenerator::storage_buffer_bind_slot_name(const StorageBuffer return to_camel_case(fmt::format("SLOT_{}", sbuf.struct_info.name)); } -std::string SokolNimGenerator::vertex_attr_definition(const std::string& snippet_name, const StageAttr& attr) { - return fmt::format("const {}* = {}", vertex_attr_name(snippet_name, attr), attr.slot); +std::string SokolNimGenerator::vertex_attr_definition(const StageAttr& attr) { + return fmt::format("const {}* = {}", vertex_attr_name(attr), attr.slot); } std::string SokolNimGenerator::image_bind_slot_definition(const Image& img) { diff --git a/src/shdc/generators/sokolnim.h b/src/shdc/generators/sokolnim.h index 57bfcb05..3a5c39b6 100644 --- a/src/shdc/generators/sokolnim.h +++ b/src/shdc/generators/sokolnim.h @@ -29,12 +29,12 @@ class SokolNimGenerator: public Generator { virtual std::string sampler_type(refl::SamplerType::Enum e); virtual std::string backend(Slang::Enum e); virtual std::string struct_name(const std::string& name); - virtual std::string vertex_attr_name(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_name(const refl::StageAttr& attr); virtual std::string image_bind_slot_name(const refl::Image& img); virtual std::string sampler_bind_slot_name(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_name(const refl::UniformBlock& ub); virtual std::string storage_buffer_bind_slot_name(const refl::StorageBuffer& sbuf); - virtual std::string vertex_attr_definition(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_definition(const refl::StageAttr& attr); virtual std::string image_bind_slot_definition(const refl::Image& img); virtual std::string sampler_bind_slot_definition(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_definition(const refl::UniformBlock& ub); diff --git a/src/shdc/generators/sokolodin.cc b/src/shdc/generators/sokolodin.cc index 312129f4..e3a1ebb7 100644 --- a/src/shdc/generators/sokolodin.cc +++ b/src/shdc/generators/sokolodin.cc @@ -426,8 +426,8 @@ std::string SokolOdinGenerator::struct_name(const std::string& name) { return to_ada_case(name); } -std::string SokolOdinGenerator::vertex_attr_name(const std::string& snippet_name, const StageAttr& attr) { - return fmt::format("ATTR_{}_{}", snippet_name, attr.name); +std::string SokolOdinGenerator::vertex_attr_name(const StageAttr& attr) { + return fmt::format("ATTR_{}_{}", attr.snippet_name, attr.name); } std::string SokolOdinGenerator::image_bind_slot_name(const Image& img) { @@ -446,8 +446,8 @@ std::string SokolOdinGenerator::storage_buffer_bind_slot_name(const StorageBuffe return fmt::format("SLOT_{}", sbuf.struct_info.name); } -std::string SokolOdinGenerator::vertex_attr_definition(const std::string& snippet_name, const StageAttr& attr) { - return fmt::format("{} :: {}", vertex_attr_name(snippet_name, attr), attr.slot); +std::string SokolOdinGenerator::vertex_attr_definition(const StageAttr& attr) { + return fmt::format("{} :: {}", vertex_attr_name(attr), attr.slot); } std::string SokolOdinGenerator::image_bind_slot_definition(const Image& img) { diff --git a/src/shdc/generators/sokolodin.h b/src/shdc/generators/sokolodin.h index 27ecdafd..40830423 100644 --- a/src/shdc/generators/sokolodin.h +++ b/src/shdc/generators/sokolodin.h @@ -27,12 +27,12 @@ class SokolOdinGenerator: public Generator { virtual std::string sampler_type(refl::SamplerType::Enum e); virtual std::string backend(Slang::Enum e); virtual std::string struct_name(const std::string& name); - virtual std::string vertex_attr_name(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_name(const refl::StageAttr& attr); virtual std::string image_bind_slot_name(const refl::Image& img); virtual std::string sampler_bind_slot_name(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_name(const refl::UniformBlock& ub); virtual std::string storage_buffer_bind_slot_name(const refl::StorageBuffer& sbuf); - virtual std::string vertex_attr_definition(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_definition(const refl::StageAttr& attr); virtual std::string image_bind_slot_definition(const refl::Image& img); virtual std::string sampler_bind_slot_definition(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_definition(const refl::UniformBlock& ub); diff --git a/src/shdc/generators/sokolrust.cc b/src/shdc/generators/sokolrust.cc index adabab2e..2cbdfb2d 100644 --- a/src/shdc/generators/sokolrust.cc +++ b/src/shdc/generators/sokolrust.cc @@ -440,8 +440,8 @@ std::string SokolRustGenerator::struct_name(const std::string& name) { return to_pascal_case(name); } -std::string SokolRustGenerator::vertex_attr_name(const std::string& snippet_name, const StageAttr& attr) { - return pystring::upper(fmt::format("ATTR_{}_{}", snippet_name, attr.name)); +std::string SokolRustGenerator::vertex_attr_name(const StageAttr& attr) { + return pystring::upper(fmt::format("ATTR_{}_{}", attr.snippet_name, attr.name)); } std::string SokolRustGenerator::image_bind_slot_name(const Image& img) { @@ -460,8 +460,8 @@ std::string SokolRustGenerator::storage_buffer_bind_slot_name(const StorageBuffe return pystring::upper(fmt::format("SLOT_{}", sbuf.struct_info.name)); } -std::string SokolRustGenerator::vertex_attr_definition(const std::string& snippet_name, const StageAttr& attr) { - return fmt::format("pub const {}: usize = {};", vertex_attr_name(snippet_name, attr), attr.slot); +std::string SokolRustGenerator::vertex_attr_definition(const StageAttr& attr) { + return fmt::format("pub const {}: usize = {};", vertex_attr_name(attr), attr.slot); } std::string SokolRustGenerator::image_bind_slot_definition(const Image& img) { diff --git a/src/shdc/generators/sokolrust.h b/src/shdc/generators/sokolrust.h index 44cd84cd..497507ad 100644 --- a/src/shdc/generators/sokolrust.h +++ b/src/shdc/generators/sokolrust.h @@ -27,12 +27,12 @@ class SokolRustGenerator: public Generator { virtual std::string sampler_type(refl::SamplerType::Enum e); virtual std::string backend(Slang::Enum e); virtual std::string struct_name(const std::string& name); - virtual std::string vertex_attr_name(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_name(const refl::StageAttr& attr); virtual std::string image_bind_slot_name(const refl::Image& img); virtual std::string sampler_bind_slot_name(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_name(const refl::UniformBlock& ub); virtual std::string storage_buffer_bind_slot_name(const refl::StorageBuffer& sbuf); - virtual std::string vertex_attr_definition(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_definition(const refl::StageAttr& attr); virtual std::string image_bind_slot_definition(const refl::Image& img); virtual std::string sampler_bind_slot_definition(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_definition(const refl::UniformBlock& ub); diff --git a/src/shdc/generators/sokolzig.cc b/src/shdc/generators/sokolzig.cc index 6cd7378a..2de97d46 100644 --- a/src/shdc/generators/sokolzig.cc +++ b/src/shdc/generators/sokolzig.cc @@ -437,8 +437,8 @@ std::string SokolZigGenerator::struct_name(const std::string& name) { return to_pascal_case(name); } -std::string SokolZigGenerator::vertex_attr_name(const std::string& snippet_name, const StageAttr& attr) { - return fmt::format("ATTR_{}_{}", snippet_name, attr.name); +std::string SokolZigGenerator::vertex_attr_name(const StageAttr& attr) { + return fmt::format("ATTR_{}_{}", attr.snippet_name, attr.name); } std::string SokolZigGenerator::image_bind_slot_name(const Image& img) { @@ -457,8 +457,8 @@ std::string SokolZigGenerator::storage_buffer_bind_slot_name(const refl::Storage return fmt::format("SLOT_{}", sb.struct_info.name); } -std::string SokolZigGenerator::vertex_attr_definition(const std::string& snippet_name, const StageAttr& attr) { - return fmt::format("pub const {} = {};", vertex_attr_name(snippet_name, attr), attr.slot); +std::string SokolZigGenerator::vertex_attr_definition(const StageAttr& attr) { + return fmt::format("pub const {} = {};", vertex_attr_name(attr), attr.slot); } std::string SokolZigGenerator::image_bind_slot_definition(const Image& img) { diff --git a/src/shdc/generators/sokolzig.h b/src/shdc/generators/sokolzig.h index 79f0f111..ea45b965 100644 --- a/src/shdc/generators/sokolzig.h +++ b/src/shdc/generators/sokolzig.h @@ -27,12 +27,12 @@ class SokolZigGenerator: public Generator { virtual std::string sampler_type(refl::SamplerType::Enum e); virtual std::string backend(Slang::Enum e); virtual std::string struct_name(const std::string& name); - virtual std::string vertex_attr_name(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_name(const refl::StageAttr& attr); virtual std::string image_bind_slot_name(const refl::Image& img); virtual std::string sampler_bind_slot_name(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_name(const refl::UniformBlock& ub); virtual std::string storage_buffer_bind_slot_name(const refl::StorageBuffer& sbuf); - virtual std::string vertex_attr_definition(const std::string& snippet_name, const refl::StageAttr& attr); + virtual std::string vertex_attr_definition(const refl::StageAttr& attr); virtual std::string image_bind_slot_definition(const refl::Image& img); virtual std::string sampler_bind_slot_definition(const refl::Sampler& smp); virtual std::string uniform_block_bind_slot_definition(const refl::UniformBlock& ub); diff --git a/src/shdc/reflection.cc b/src/shdc/reflection.cc index 2148ec63..6d52145c 100644 --- a/src/shdc/reflection.cc +++ b/src/shdc/reflection.cc @@ -27,12 +27,12 @@ namespace shdc::refl { // check that a program's vertex shader outputs match the fragment shader inputs // FIXME: this should also check the attribute's type -// FIXME: move all reflection validations into Reflection static ErrMsg validate_linking(const Input& inp, const Program& prog, const ProgramReflection& prog_refl) { for (int slot = 0; slot < StageAttr::Num; slot++) { const StageAttr& vs_out = prog_refl.vs().outputs[slot]; const StageAttr& fs_inp = prog_refl.fs().inputs[slot]; - if (!vs_out.equals(fs_inp)) { + // ignore snippet-name for equality check + if (!vs_out.equals(fs_inp, false)) { return inp.error(prog.line_index, fmt::format("outputs of vs '{}' don't match inputs of fs '{}' for attr #{} (vs={},fs={})\n", prog_refl.vs_name(), @@ -72,6 +72,15 @@ Reflection Reflection::build(const Args& args, const Input& inp, const std::arra res.progs.push_back(prog_refl); } + // create a set of vertex shader inputs with removed duplicates + // (or error out if the are attribute conflicts) + ErrMsg error; + res.unique_vs_inputs = merge_vs_inputs(res.progs, error); + if (error.valid()) { + res.error = inp.error(0, error.msg); + return res; + } + // create a merged set of resource bindings std::vector snippet_bindings; for (int i = 0; i < Slang::Num; i++) { @@ -82,7 +91,6 @@ Reflection Reflection::build(const Args& args, const Input& inp, const std::arra } } } - ErrMsg error; res.bindings = merge_bindings(snippet_bindings, error); if (error.valid()) { res.error = inp.error(0, error.msg); @@ -162,6 +170,7 @@ StageReflection Reflection::parse_snippet_reflection(const Compiler& compiler, c refl_attr.name = res_attr.name; refl_attr.sem_name = "TEXCOORD"; refl_attr.sem_index = refl_attr.slot; + refl_attr.snippet_name = snippet.name; refl.inputs[refl_attr.slot] = refl_attr; } for (const Resource& res_attr: shd_resources.stage_outputs) { @@ -170,6 +179,7 @@ StageReflection Reflection::parse_snippet_reflection(const Compiler& compiler, c refl_attr.name = res_attr.name; refl_attr.sem_name = "TEXCOORD"; refl_attr.sem_index = refl_attr.slot; + refl_attr.snippet_name = snippet.name; refl.outputs[refl_attr.slot] = refl_attr; } // uniform blocks @@ -265,6 +275,38 @@ StageReflection Reflection::parse_snippet_reflection(const Compiler& compiler, c return refl; } +const StageAttr* find_attr_by_name(const std::vector& attrs, const std::string& snippet_name, const std::string& attr_name) { + for (const StageAttr& attr: attrs) { + if ((attr.name == attr_name) && (attr.snippet_name == snippet_name)) { + return &attr; + } + } + return nullptr; +} + +std::vector Reflection::merge_vs_inputs(const std::vector& progs, ErrMsg& out_error) { + std::vector out_attrs; + out_error = ErrMsg(); + for (const ProgramReflection& prog: progs) { + for (const StageAttr& attr: prog.vs().inputs) { + if (attr.slot == -1) { + break; + } + const StageAttr* other_attr = find_attr_by_name(out_attrs, attr.snippet_name, attr.name); + if (other_attr) { + // take snippet-name into account for equality check + if (!attr.equals(*other_attr, true)) { + out_error = ErrMsg::error(fmt::format("conflicting vertex shader attributes found for '{}'", attr.name)); + return std::vector{}; + } + } else { + out_attrs.push_back(attr); + } + } + } + return out_attrs; +} + Bindings Reflection::merge_bindings(const std::vector& in_bindings, ErrMsg& out_error) { Bindings out_bindings; out_error = ErrMsg(); diff --git a/src/shdc/reflection.h b/src/shdc/reflection.h index 7ac2ddd5..1f49db48 100644 --- a/src/shdc/reflection.h +++ b/src/shdc/reflection.h @@ -7,6 +7,7 @@ #include "types/errmsg.h" #include "types/slang.h" #include "types/snippet.h" +#include "types/reflection/stage_attr.h" #include "types/reflection/stage_reflection.h" #include "types/reflection/program_reflection.h" #include "types/reflection/type.h" @@ -15,6 +16,7 @@ namespace shdc::refl { struct Reflection { std::vector progs; + std::vector unique_vs_inputs; Bindings bindings; ErrMsg error; @@ -26,14 +28,14 @@ struct Reflection { void dump_debug(ErrMsg::Format err_fmt) const; private: + // create a set of unique vertex shader inputs across all programs + static std::vector merge_vs_inputs(const std::vector& progs, ErrMsg& out_error); // create a set of unique resource bindings from shader snippet input bindings static Bindings merge_bindings(const std::vector& in_bindings, ErrMsg& out_error); // parse a struct static Type parse_toplevel_struct(const spirv_cross::Compiler& compiler, const spirv_cross::Resource& res, ErrMsg& out_error); // parse a struct item static Type parse_struct_item(const spirv_cross::Compiler& compiler, const spirv_cross::TypeID& type_id, const spirv_cross::TypeID& base_type_id, uint32_t item_index, ErrMsg& out_error); - // debug-dump a bindings struct - static void dump_bindings(const std::string& indent, const Bindings& bindings); }; } // namespace reflection diff --git a/src/shdc/types/reflection/stage_attr.h b/src/shdc/types/reflection/stage_attr.h index c8832ad4..fca5e624 100644 --- a/src/shdc/types/reflection/stage_attr.h +++ b/src/shdc/types/reflection/stage_attr.h @@ -9,16 +9,25 @@ struct StageAttr { std::string name; std::string sem_name; int sem_index = 0; + std::string snippet_name; - bool equals(const StageAttr& rhs) const; + bool equals(const StageAttr& rhs, bool with_snippet_name) const; void dump_debug(const std::string& indent) const; }; -inline bool StageAttr::equals(const StageAttr& rhs) const { - return (slot == rhs.slot) && - (name == rhs.name) && - (sem_name == rhs.sem_name) && - (sem_index == rhs.sem_index); +inline bool StageAttr::equals(const StageAttr& rhs, bool with_snippet_name) const { + if (with_snippet_name) { + return (slot == rhs.slot) && + (name == rhs.name) && + (sem_name == rhs.sem_name) && + (sem_index == rhs.sem_index) && + (snippet_name == rhs.snippet_name); + } else { + return (slot == rhs.slot) && + (name == rhs.name) && + (sem_name == rhs.sem_name) && + (sem_index == rhs.sem_index); + } } inline void StageAttr::dump_debug(const std::string& indent) const { @@ -28,6 +37,7 @@ inline void StageAttr::dump_debug(const std::string& indent) const { fmt::print(stderr, "{}name: {}\n", indent2, name); fmt::print(stderr, "{}sem_name: {}\n", indent2, sem_name); fmt::print(stderr, "{}sem_index: {}\n", indent2, sem_index); + fmt::print(stderr, "{}snippet_name: {}\n", indent2, snippet_name); } } // namespace From 5220ae95ffd95fb3498c32aceb16f570f02c78e3 Mon Sep 17 00:00:00 2001 From: Andre Weissflog Date: Mon, 1 Jul 2024 17:09:14 +0200 Subject: [PATCH 2/3] improve an error message, and update changelog --- CHANGELOG.md | 14 ++++++++++++++ src/shdc/reflection.cc | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fedb0203..aa113acf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,20 @@ CHANGELOG ========= +#### **01-Jul-2024** + +Fix a regression from the May-2024 refactoring: + +Linking the same vertex shader in different programs would generate duplicate +vertex attribute slot defines/constants. While this isn't a problem in C/C++, +other languages would error on the duplicate constants. + +With the fix, vertex attribute slot constants are now unique. There's also +a more tight validation in this new vertex attribute merge step which leads +to an error if a conflict is detected between two vertex attributes that should +be identical. This validation should never fail though, if it does, it would +point to an internal bug (the associated error message is `conflicting vertex shader attributes found for '[snippet_name]/[attr_name]`). + #### **24-Jun-2024** - Minor fix in the Odin code generator (see: https://github.com/floooh/sokol-tools/issues/132) diff --git a/src/shdc/reflection.cc b/src/shdc/reflection.cc index 6d52145c..8462c789 100644 --- a/src/shdc/reflection.cc +++ b/src/shdc/reflection.cc @@ -296,7 +296,7 @@ std::vector Reflection::merge_vs_inputs(const std::vector{}; } } else { From d588c5f345c3184576c5b1235b508ee6b98df1d5 Mon Sep 17 00:00:00 2001 From: Andre Weissflog Date: Mon, 1 Jul 2024 17:10:09 +0200 Subject: [PATCH 3/3] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa113acf..5eb52342 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ to an error if a conflict is detected between two vertex attributes that should be identical. This validation should never fail though, if it does, it would point to an internal bug (the associated error message is `conflicting vertex shader attributes found for '[snippet_name]/[attr_name]`). +For more details see PR https://github.com/floooh/sokol-tools/pull/134/files and issue https://github.com/floooh/sokol-tools/issues/133. + #### **24-Jun-2024** - Minor fix in the Odin code generator (see: https://github.com/floooh/sokol-tools/issues/132)