Skip to content

Commit

Permalink
Insert additional padding at the end of the struct if the size of the…
Browse files Browse the repository at this point in the history
… struct does not satisfy the alignment requirements of all its members.
  • Loading branch information
chinmaygarde authored and dnfield committed Apr 27, 2022
1 parent 866045d commit d35f10a
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 14 deletions.
8 changes: 5 additions & 3 deletions impeller/compiler/code_gen_template.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 ==============================================================
// ===========================================================================
Expand Down
51 changes: 47 additions & 4 deletions impeller/compiler/reflector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
14 changes: 7 additions & 7 deletions impeller/compiler/reflector.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand Down

0 comments on commit d35f10a

Please sign in to comment.