Merge pull request #134 from floooh/fix/issue-133_duplicate_vertex_at…

…tr_slots Don't generate duplicate vertex attr defines/constants (fixes #133)
floooh · Jul 1, 2024 · fbfd3b2 · fbfd3b2
2 parents 190830c + d588c5f
commit fbfd3b2
Show file tree

Hide file tree

Showing 18 changed files with 123 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,22 @@
 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]`).
+
+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)

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);