GPUShaderCreateInfo: Add validation for overlapping vertex attribute

Those are usually not supported but some driver silently fix them and most
just silently fail (rendering error).

5 files changed, 51 insertions, 12 deletions

diff --git a/source/blender/draw/engines/overlay/shaders/infos/armature_info.hh b/source/blender/draw/engines/overlay/shaders/infos/armature_info.hh
index f417fbe1fd4..d89272a50cf 100644
--- a/source/blender/draw/engines/overlay/shaders/infos/armature_info.hh
+++ b/source/blender/draw/engines/overlay/shaders/infos/armature_info.hh
@@ -43,8 +43,8 @@ GPU_SHADER_CREATE_INFO(overlay_armature_sphere_solid)
     .do_static_compilation(true)
     .vertex_in(0, Type::VEC2, "pos")
     /* Per instance. */
-    .vertex_in(1, Type::MAT4, "inst_obmat")
-    .vertex_in(2, Type::VEC4, "color")
+    .vertex_in(1, Type::VEC4, "color")
+    .vertex_in(2, Type::MAT4, "inst_obmat")
     // .depth_layout(DepthLayout::GREATER) /* TODO */
     .vertex_out(overlay_armature_sphere_solid_iface)
     .vertex_source("armature_sphere_solid_vert.glsl")
diff --git a/source/blender/draw/engines/overlay/shaders/infos/extra_info.hh b/source/blender/draw/engines/overlay/shaders/infos/extra_info.hh
index 1ab9ab3b23f..a765d881682 100644
--- a/source/blender/draw/engines/overlay/shaders/infos/extra_info.hh
+++ b/source/blender/draw/engines/overlay/shaders/infos/extra_info.hh
@@ -16,8 +16,8 @@ GPU_SHADER_CREATE_INFO(overlay_extra)
     .vertex_in(0, Type::VEC3, "pos")
     .vertex_in(1, Type::INT, "vclass")
     /* Instance attributes. */
-    .vertex_in(2, Type::MAT4, "inst_obmat")
-    .vertex_in(3, Type::VEC4, "color")
+    .vertex_in(2, Type::VEC4, "color")
+    .vertex_in(3, Type::MAT4, "inst_obmat")
     .vertex_out(overlay_extra_iface)
     .fragment_out(0, Type::VEC4, "fragColor")
     .fragment_out(1, Type::VEC4, "lineOutput")
diff --git a/source/blender/gpu/intern/gpu_shader_create_info.cc b/source/blender/gpu/intern/gpu_shader_create_info.cc
index 6e82201b424..f5b90989481 100644
--- a/source/blender/gpu/intern/gpu_shader_create_info.cc
+++ b/source/blender/gpu/intern/gpu_shader_create_info.cc
@@ -41,6 +41,8 @@ void ShaderCreateInfo::finalize()
 
   Set<StringRefNull> deps_merged;
 
+  validate_vertex_attributes();
+
   for (auto &info_name : additional_infos_) {
     const ShaderCreateInfo &info = *reinterpret_cast<const ShaderCreateInfo *>(
         gpu_shader_create_info_get(info_name.c_str()));
@@ -55,6 +57,8 @@ void ShaderCreateInfo::finalize()
     vertex_out_interfaces_.extend(info.vertex_out_interfaces_);
     geometry_out_interfaces_.extend(info.geometry_out_interfaces_);
 
+    validate_vertex_attributes(&info);
+
     push_constants_.extend(info.push_constants_);
     defines_.extend(info.defines_);
 
@@ -69,7 +73,7 @@ void ShaderCreateInfo::finalize()
       depth_write_ = info.depth_write_;
     }
 
-    validate(info);
+    validate_merge(info);
 
     auto assert_no_overlap = [&](const bool test, const StringRefNull error) {
       if (!test) {
@@ -164,7 +168,7 @@ std::string ShaderCreateInfo::check_error() const
   return error;
 }
 
-void ShaderCreateInfo::validate(const ShaderCreateInfo &other_info)
+void ShaderCreateInfo::validate_merge(const ShaderCreateInfo &other_info)
 {
   if (!auto_resource_location_) {
     /* Check same bind-points usage in OGL. */
@@ -220,8 +224,42 @@ void ShaderCreateInfo::validate(const ShaderCreateInfo &other_info)
       }
     }
   }
-  {
-    /* TODO(@fclem): Push constant validation. */
+}
+
+void ShaderCreateInfo::validate_vertex_attributes(const ShaderCreateInfo *other_info)
+{
+  uint32_t attr_bits = 0;
+  for (auto &attr : vertex_inputs_) {
+    if (attr.index >= 16 || attr.index < 0) {
+      std::cout << name_ << ": \"" << attr.name
+                << "\" : Type::MAT3 unsupported as vertex attribute." << std::endl;
+      BLI_assert(0);
+    }
+    if (attr.index >= 16 || attr.index < 0) {
+      std::cout << name_ << ": Invalid index for attribute \"" << attr.name << "\"" << std::endl;
+      BLI_assert(0);
+    }
+    uint32_t attr_new = 0;
+    if (attr.type == Type::MAT4) {
+      for (int i = 0; i < 4; i++) {
+        attr_new |= 1 << (attr.index + i);
+      }
+    }
+    else {
+      attr_new |= 1 << attr.index;
+    }
+
+    if ((attr_bits & attr_new) != 0) {
+      std::cout << name_ << ": Attribute \"" << attr.name
+                << "\" overlap one or more index from another attribute."
+                   " Note that mat4 takes up 4 indices.";
+      if (other_info) {
+        std::cout << " While merging " << other_info->name_ << std::endl;
+      }
+      std::cout << std::endl;
+      BLI_assert(0);
+    }
+    attr_bits |= attr_new;
   }
 }
 
diff --git a/source/blender/gpu/intern/gpu_shader_create_info.hh b/source/blender/gpu/intern/gpu_shader_create_info.hh
index 7c827e237eb..4927ef75a75 100644
--- a/source/blender/gpu/intern/gpu_shader_create_info.hh
+++ b/source/blender/gpu/intern/gpu_shader_create_info.hh
@@ -801,7 +801,8 @@ struct ShaderCreateInfo {
   std::string check_error() const;
 
   /** Error detection that some backend compilers do not complain about. */
-  void validate(const ShaderCreateInfo &other_info);
+  void validate_merge(const ShaderCreateInfo &other_info);
+  void validate_vertex_attributes(const ShaderCreateInfo *other_info = nullptr);
 
   /** \} */
 
diff --git a/source/blender/gpu/shaders/infos/gpu_shader_instance_varying_color_varying_size_info.hh b/source/blender/gpu/shaders/infos/gpu_shader_instance_varying_color_varying_size_info.hh
index 4885efc66c2..fb7bbcbc3e5 100644
--- a/source/blender/gpu/shaders/infos/gpu_shader_instance_varying_color_varying_size_info.hh
+++ b/source/blender/gpu/shaders/infos/gpu_shader_instance_varying_color_varying_size_info.hh
@@ -10,9 +10,9 @@
 
 GPU_SHADER_CREATE_INFO(gpu_shader_instance_varying_color_varying_size)
     .vertex_in(0, Type::VEC3, "pos")
-    .vertex_in(1, Type::MAT4, "InstanceModelMatrix")
-    .vertex_in(2, Type::VEC4, "color")
-    .vertex_in(3, Type::FLOAT, "size")
+    .vertex_in(1, Type::VEC4, "color")
+    .vertex_in(2, Type::FLOAT, "size")
+    .vertex_in(3, Type::MAT4, "InstanceModelMatrix")
     .vertex_out(flat_color_iface)
     .fragment_out(0, Type::VEC4, "fragColor")
     .push_constant(Type::MAT4, "ViewProjectionMatrix")