Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't generate duplicate vertex attr defines/constants (fixes #133) #134

Merged
merged 3 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
10 changes: 4 additions & 6 deletions src/shdc/generators/generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/shdc/generators/generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""; };
Expand Down
8 changes: 4 additions & 4 deletions src/shdc/generators/sokolc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/shdc/generators/sokolc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/shdc/generators/sokold.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/shdc/generators/sokold.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/shdc/generators/sokolnim.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/shdc/generators/sokolnim.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/shdc/generators/sokolodin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/shdc/generators/sokolodin.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/shdc/generators/sokolrust.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/shdc/generators/sokolrust.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/shdc/generators/sokolzig.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/shdc/generators/sokolzig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading