From: Mikko Rasa Date: Fri, 2 Apr 2021 20:05:06 +0000 (+0300) Subject: Validate location overlap and type matching for GLSL interfaces X-Git-Url: http://git.tdb.fi/?a=commitdiff_plain;h=20a86c5905e1f7527d3c9edc3f56f6b7679c268a;p=libs%2Fgl.git Validate location overlap and type matching for GLSL interfaces --- diff --git a/source/glsl/compiler.cpp b/source/glsl/compiler.cpp index 5ff003e7..e0a67606 100644 --- a/source/glsl/compiler.cpp +++ b/source/glsl/compiler.cpp @@ -82,9 +82,13 @@ void Compiler::compile(Mode mode) generate(*i); ConstantIdAssigner().apply(*module, features); + for(list::iterator i=module->stages.begin(); i!=module->stages.end(); ++i) + validate(*i); + GlobalInterfaceValidator().apply(*module); + bool valid = true; for(list::iterator i=module->stages.begin(); i!=module->stages.end(); ++i) - if(!validate(*i)) + if(!check_errors(*i)) valid = false; if(!valid) throw invalid_shader_source(get_diagnostics()); @@ -297,13 +301,17 @@ void Compiler::resolve(Stage &stage, unsigned flags) } } -bool Compiler::validate(Stage &stage) +void Compiler::validate(Stage &stage) { DeclarationValidator().apply(stage); IdentifierValidator().apply(stage); ReferenceValidator().apply(stage); ExpressionValidator().apply(stage); + StageInterfaceValidator().apply(stage); +} +bool Compiler::check_errors(Stage &stage) +{ stable_sort(stage.diagnostics, &diagnostic_line_order); for(vector::const_iterator i=stage.diagnostics.begin(); i!=stage.diagnostics.end(); ++i) diff --git a/source/glsl/compiler.h b/source/glsl/compiler.h index 674081a0..e30bd709 100644 --- a/source/glsl/compiler.h +++ b/source/glsl/compiler.h @@ -132,9 +132,13 @@ private: aspects as necessary. */ void resolve(Stage &, unsigned = RESOLVE_ALL); - /** Checks the validity of the module. If the return value is false, the - module's diagnostics list will contain additional information of errors. */ - bool validate(Stage &); + /** Runs validators on a stage. Diagnostic messages are recorded in the + stage for later inspection. */ + void validate(Stage &); + + /** Checks a stage's recorded diagnostics for errors. If any are found, + returns true. */ + bool check_errors(Stage &); static bool diagnostic_line_order(const Diagnostic &, const Diagnostic &); diff --git a/source/glsl/syntax.cpp b/source/glsl/syntax.cpp index 32a7b12c..23b8f030 100644 --- a/source/glsl/syntax.cpp +++ b/source/glsl/syntax.cpp @@ -485,6 +485,66 @@ string get_unused_variable_name(const Block &block, const string &base) } } +bool is_same_type(const TypeDeclaration &type1, const TypeDeclaration &type2) +{ + if(const BasicTypeDeclaration *basic1 = dynamic_cast(&type1)) + { + const BasicTypeDeclaration *basic2 = dynamic_cast(&type2); + if(!basic2) + return false; + + if(basic1->kind!=basic2->kind || basic1->size!=basic2->size) + return false; + + if(basic1->base_type && basic2->base_type) + return is_same_type(*basic1->base_type, *basic2->base_type); + else + return (!basic1->base_type && !basic2->base_type); + } + else if(const ImageTypeDeclaration *image1 = dynamic_cast(&type1)) + { + const ImageTypeDeclaration *image2 = dynamic_cast(&type2); + if(!image2) + return false; + + if(image1->dimensions!=image2->dimensions || image1->array!=image2->array) + return false; + if(image1->sampled!=image2->sampled || image1->shadow!=image2->shadow) + return false; + + if(image1->base_type && image2->base_type) + return is_same_type(*image1->base_type, *image2->base_type); + else + return (!image1->base_type && !image2->base_type); + } + else if(const StructDeclaration *strct1 = dynamic_cast(&type1)) + { + const StructDeclaration *strct2 = dynamic_cast(&type2); + if(!strct2) + return false; + + NodeList::const_iterator i = strct1->members.body.begin(); + NodeList::const_iterator j = strct2->members.body.begin(); + for(; (i!=strct1->members.body.end() && j!=strct2->members.body.end()); ++i, ++j) + { + const VariableDeclaration *var1 = dynamic_cast(i->get()); + const VariableDeclaration *var2 = dynamic_cast(j->get()); + if(!var1 || !var1->type_declaration || !var2 || !var2->type_declaration) + return false; + if(!is_same_type(*var1->type_declaration, *var2->type_declaration)) + return false; + if(var1->name!=var2->name || var1->array!=var2->array) + return false; + // TODO Compare array sizes + // TODO Compare layout qualifiers for interface block members + } + + return (i==strct1->members.body.end() && j==strct2->members.body.end()); + } + else + return false; +} + } // namespace SL } // namespace GL } // namespace Msp diff --git a/source/glsl/syntax.h b/source/glsl/syntax.h index 1caf4d95..d69922a7 100644 --- a/source/glsl/syntax.h +++ b/source/glsl/syntax.h @@ -551,6 +551,8 @@ struct Module std::string get_unused_variable_name(const Block &, const std::string &); +bool is_same_type(const TypeDeclaration &, const TypeDeclaration &); + } // namespace SL } // namespace GL } // namespace Msp diff --git a/source/glsl/validate.cpp b/source/glsl/validate.cpp index 154189aa..0096ff5e 100644 --- a/source/glsl/validate.cpp +++ b/source/glsl/validate.cpp @@ -609,6 +609,139 @@ void ExpressionValidator::visit(Return &ret) TraversingVisitor::visit(ret); } + +int StageInterfaceValidator::get_location(const Layout &layout) +{ + for(vector::const_iterator i=layout.qualifiers.begin(); i!=layout.qualifiers.end(); ++i) + if(i->name=="location") + return i->value; + return -1; +} + +void StageInterfaceValidator::visit(VariableDeclaration &var) +{ + int location = (var.layout ? get_location(*var.layout) : -1); + if(var.interface=="in" && var.linked_declaration) + { + const Layout *linked_layout = var.linked_declaration->layout.get(); + int linked_location = (linked_layout ? get_location(*linked_layout) : -1); + if(linked_location!=location) + { + error(var, format("Mismatched location %d for 'in %s'", location, var.name)); + add_info(*var.linked_declaration, format("Linked to 'out %s' with location %d", + var.linked_declaration->name, linked_location)); + } + if(var.type_declaration && var.linked_declaration->type_declaration) + { + const TypeDeclaration *type = var.type_declaration; + if(stage->type==Stage::GEOMETRY) + { + if(const BasicTypeDeclaration *basic = dynamic_cast(type)) + if(basic->kind==BasicTypeDeclaration::ARRAY && basic->base_type) + type = basic->base_type; + } + if(!is_same_type(*type, *var.linked_declaration->type_declaration)) + { + error(var, format("Mismatched type '%s' for 'in %s'", type->name, var.name)); + add_info(*var.linked_declaration, format("Linked to 'out %s' with type '%s'", + var.linked_declaration->name, var.linked_declaration->type_declaration->name)); + } + } + } + + if(location>=0 && !var.interface.empty()) + { + map &used = used_locations[var.interface]; + + unsigned loc_count = 1; + if(var.array) + if(const Literal *literal = dynamic_cast(var.array_size.get())) + if(literal->value.check_type()) + loc_count = literal->value.value(); + + for(unsigned i=0; i::const_iterator j = used.find(location+i); + if(j!=used.end()) + { + error(var, format("Overlapping location %d for '%s %s'", location+i, var.interface, var.name)); + add_info(*j->second, format("Previously used here for '%s %s'", j->second->interface, j->second->name)); + } + else + used[location+i] = &var; + } + } +} + + +void GlobalInterfaceValidator::apply(Module &module) +{ + for(list::iterator i=module.stages.begin(); i!=module.stages.end(); ++i) + { + stage = &*i; + i->content.visit(*this); + } +} + +void GlobalInterfaceValidator::get_binding(const Layout &layout, unsigned &desc_set, int &binding) +{ + for(vector::const_iterator i=layout.qualifiers.begin(); i!=layout.qualifiers.end(); ++i) + { + if(i->name=="set") + desc_set = i->value; + else if(i->name=="binding") + binding = i->value; + } +} + +void GlobalInterfaceValidator::check_binding(const Layout &layout, const Binding &binding) +{ + unsigned desc_set = 0; + int bind_point = -1; + get_binding(layout, desc_set, bind_point); + if(bind_point<0) + return; + + map &used = used_bindings[desc_set]; + map::const_iterator i = used.find(bind_point); + if(i!=used.end()) + { + if(i->second.name!=binding.name) + { + error(*binding.node, format("Overlapping binding %d for '%s'", bind_point, binding.name)); + add_info(*i->second.node, format("Previously used here for '%s'", i->second.name)); + } + if(i->second.type && binding.type) + { + if(!is_same_type(*i->second.type, *binding.type)) + { + string type_name = (dynamic_cast(binding.type) ? "struct type" : + format("type '%s'", binding.type->name)); + error(*binding.node, format("Mismatched %s for binding %d '%s'", type_name, bind_point, binding.name)); + + string message = "Previously used here"; + if(!dynamic_cast(i->second.type)) + message += format(" with type '%s'", i->second.type->name); + add_info(*i->second.node, message); + } + } + } + else + used.insert(make_pair(bind_point, binding)); +} + +void GlobalInterfaceValidator::visit(VariableDeclaration &var) +{ + if(var.interface=="uniform" && var.layout) + check_binding(*var.layout, var); +} + +void GlobalInterfaceValidator::visit(InterfaceBlock &iface) +{ + if(iface.interface=="uniform" && iface.layout) + check_binding(*iface.layout, iface); +} + } // namespace SL } // namespace GL } // namespace Msp diff --git a/source/glsl/validate.h b/source/glsl/validate.h index f1e3f512..63af3610 100644 --- a/source/glsl/validate.h +++ b/source/glsl/validate.h @@ -127,6 +127,48 @@ private: virtual void visit(Return &); }; +class StageInterfaceValidator: private Validator +{ +private: + std::map > used_locations; + +public: + void apply(Stage &s) { stage = &s; s.content.visit(*this); } + +private: + int get_location(const Layout &); + + virtual void visit(VariableDeclaration &); + virtual void visit(FunctionDeclaration &) { } +}; + +class GlobalInterfaceValidator: private Validator +{ +private: + struct Binding + { + Node *node; + TypeDeclaration *type; + std::string name; + + Binding(VariableDeclaration &v): node(&v), type(v.type_declaration), name(v.name) { } + Binding(InterfaceBlock &i): node(&i), type(i.struct_declaration), name(i.block_name) { } + }; + + std::map > used_bindings; + +public: + void apply(Module &); + +private: + void get_binding(const Layout &, unsigned &, int &); + void check_binding(const Layout &, const Binding &); + + virtual void visit(VariableDeclaration &); + virtual void visit(InterfaceBlock &); + virtual void visit(FunctionDeclaration &) { } +}; + } // namespace SL } // namespace GL } // namespace Msp diff --git a/tests/glsl/binding_mismatch.glsl b/tests/glsl/binding_mismatch.glsl new file mode 100644 index 00000000..fa88bf1a --- /dev/null +++ b/tests/glsl/binding_mismatch.glsl @@ -0,0 +1,43 @@ +#pragma MSP stage(vertex) +layout(binding=0) uniform Transform +{ + mat4 model; + mat4 vp; +}; +layout(binding=1) uniform Lighting +{ + vec3 position; +} light; +layout(location=0) in vec4 position; +layout(location=1) in vec3 normal; +void main() +{ + vec4 world_pos = model*position; + gl_Position = vp*world_pos; + out vec3 light_dir = world_pos.xyz-light.position; + passthrough; +} + +#pragma MSP stage(fragment) +layout(binding=1) uniform Lighting +{ + vec4 color; +} light; +layout(binding=0) uniform Material +{ + vec4 color; +}; +layout(location=0) out vec4 frag_color; +void main() +{ + frag_color = color*vec4(vec3(max(dot(normalize(normal), normalize(light_dir)), 0.0)), 1.0); +} + +/* Expected error: +:23: Mismatched struct type for binding 1 'Lighting' +:8: Previously used here +:27: Overlapping binding 0 for 'Material' +:3: Previously used here for 'Transform' +:27: Mismatched struct type for binding 0 'Material' +:3: Previously used here +*/ diff --git a/tests/glsl/linked_interface_mismatch.glsl b/tests/glsl/linked_interface_mismatch.glsl new file mode 100644 index 00000000..30a70d52 --- /dev/null +++ b/tests/glsl/linked_interface_mismatch.glsl @@ -0,0 +1,30 @@ +uniform sampler2D tex; + +#pragma MSP stage(vertex) +layout(location=0) in vec4 position; +layout(location=1) in vec4 color; +layout(location=2) in vec4 texcoord; +layout(location=0) out vec4 color_out; +layout(location=2) out vec4 tc_out; +void main() +{ + gl_Position = position; + color_out = color; + tc_out = texcoord; +} + +#pragma MSP stage(fragment) +layout(location=1) in vec4 color_out; +layout(location=2) in vec2 tc_out; +layout(location=0) out vec4 frag_color; +void main() +{ + frag_color = color_out*texture(tex, tc_out); +} + +/* Expected error: +:17: Mismatched location 1 for 'in color_out' +:7: Linked to 'out color_out' with location 0 +:18: Mismatched type 'vec2' for 'in tc_out' +:8: Linked to 'out tc_out' with type 'vec4' +*/ diff --git a/tests/glsl/location_overlap.glsl b/tests/glsl/location_overlap.glsl new file mode 100644 index 00000000..df0f0314 --- /dev/null +++ b/tests/glsl/location_overlap.glsl @@ -0,0 +1,15 @@ +#pragma MSP stage(vertex) +layout(location=0) in vec4 position; +layout(location=0) in vec2 texcoords[3]; +layout(location=2) in vec4 color; +void main() +{ + gl_Position = position; +} + +/* Expected error: +:3: Overlapping location 0 for 'in texcoords' +:2: Previously used here for 'in position' +:4: Overlapping location 2 for 'in color' +:3: Previously used here for 'in texcoords' +*/