Insert additional padding at the end of the struct if the size of the struct does not satisfy the alignment requirements of all its members.

Author: chinmaygarde

impeller/compiler/code_gen_template.h

@@ -39,23 +39,25 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader {
 
   struct {{def.name}} {
 {% for member in def.members %}
-    {{member.type}} {{member.name}};
+    {{member.type}} {{member.name}}; // (offset {{member.offset}}, size {{member.byte_length}})
 {% endfor %}
-  }; // struct {{def.name}}
+  }; // struct {{def.name}} (size {{def.byte_length}})
 {% endfor %}
 {% endif %}
 {% if length(uniform_buffers) > 0 %}
+
   // ===========================================================================
   // Stage Uniforms ============================================================
   // ===========================================================================
 {% for uniform in uniform_buffers %}
 
   static constexpr auto kResource{{camel_case(uniform.name)}} = ShaderUniformSlot<{{uniform.name}}> { // {{uniform.name}}
     "{{uniform.name}}",     // name
-    {{uniform.binding}}u,   // binding
+    {{uniform.msl_res_0}}u,   // binding
   };
 {% endfor %}
 {% endif %}
+
   // ===========================================================================
   // Stage Inputs ==============================================================
   // ===========================================================================

impeller/compiler/reflector.cc

@@ -372,14 +372,36 @@ static std::optional<KnownType> ReadKnownScalarType(
   return std::nullopt;
 }
 
-std::vector<Reflector::StructMember> Reflector::ReadStructMembers(
+//------------------------------------------------------------------------------
+/// @brief      Get the reflected struct size. In the vast majority of the
+///             cases, this is the same as the declared struct size as given by
+///             the compiler. But, additional padding may need to be introduced
+///             after the end of the struct to keep in line with the alignment
+///             requirement of the individual struct members. This method
+///             figures out the actual size of the reflected struct that can be
+///             referenced in native code.
+///
+/// @param[in]  members  The members
+///
+/// @return     The reflected structure size.
+///
+static size_t GetReflectedStructSize(const std::vector<StructMember>& members) {
+  auto struct_size = 0u;
+  for (const auto& member : members) {
+    struct_size += member.byte_length;
+  }
+  return struct_size;
+}
+
+std::vector<StructMember> Reflector::ReadStructMembers(
     const spirv_cross::TypeID& type_id) const {
   const auto& struct_type = compiler_->get_type(type_id);
   FML_CHECK(struct_type.basetype == spirv_cross::SPIRType::BaseType::Struct);
 
   std::vector<StructMember> result;
 
   size_t current_byte_offset = 0;
+  size_t max_member_alignment = 0;
 
   for (size_t i = 0; i < struct_type.member_types.size(); i++) {
     const auto& member = compiler_->get_type(struct_type.member_types[i]);
@@ -390,14 +412,18 @@ std::vector<Reflector::StructMember> Reflector::ReadStructMembers(
       const auto alignment_pad = struct_member_offset - current_byte_offset;
       result.emplace_back(StructMember{
           .type = TypeNameWithPaddingOfSize(alignment_pad),
-          .name = SPrintF("_align_%s",
+          .name = SPrintF("_PADDING_%s_",
                           GetMemberNameAtIndex(struct_type, i).c_str()),
           .offset = current_byte_offset,
           .byte_length = alignment_pad,
       });
       current_byte_offset += alignment_pad;
     }
 
+    max_member_alignment =
+        std::max<size_t>(max_member_alignment,
+                         (member.width / 8) * member.columns * member.vecsize);
+
     FML_CHECK(current_byte_offset == struct_member_offset);
 
     // Tightly packed 4x4 Matrix is special cased as we know how to work with
@@ -499,6 +525,20 @@ std::vector<Reflector::StructMember> Reflector::ReadStructMembers(
       continue;
     }
   }
+
+  const auto struct_length = current_byte_offset;
+  {
+    const auto padding = struct_length % max_member_alignment;
+    if (padding != 0) {
+      result.emplace_back(StructMember{
+          .type = TypeNameWithPaddingOfSize(padding),
+          .name = "_PADDING_",
+          .offset = current_byte_offset,
+          .byte_length = padding,
+      });
+    }
+  }
+
   return result;
 }
 
@@ -514,10 +554,13 @@ std::optional<Reflector::StructDefinition> Reflector::ReflectStructDefinition(
     return std::nullopt;
   }
 
+  auto struct_members = ReadStructMembers(type_id);
+  auto reflected_struct_size = GetReflectedStructSize(struct_members);
+
   StructDefinition struc;
   struc.name = struct_name;
-  struc.byte_length = compiler_->get_declared_struct_size(type);
-  struc.members = ReadStructMembers(type_id);
+  struc.byte_length = reflected_struct_size;
+  struc.members = std::move(struct_members);
   return struc;
 }
 

impeller/compiler/reflector.h

@@ -16,6 +16,13 @@
 namespace impeller {
 namespace compiler {
 
+struct StructMember {
+  std::string type;
+  std::string name;
+  size_t offset = 0u;
+  size_t byte_length = 0u;
+};
+
 class Reflector {
  public:
   struct Options {
@@ -38,13 +45,6 @@ class Reflector {
   std::shared_ptr<fml::Mapping> GetReflectionCC() const;
 
  private:
-  struct StructMember {
-    std::string type;
-    std::string name;
-    size_t offset = 0u;
-    size_t byte_length = 0u;
-  };
-
   struct StructDefinition {
     std::string name;
     size_t byte_length = 0u;