Skip to content

Commit

Permalink
fixed the size in bytes of vec3s to match Metal standard in Metal gpu…
Browse files Browse the repository at this point in the history
… backend

Bug: skia:
Change-Id: Iba46b7eff46cfd05c3b65ba2d38f10dce16e9481
Reviewed-on: https://skia-review.googlesource.com/146764
Commit-Queue: Timothy Liang <timliang@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
  • Loading branch information
Timothy Liang authored and Skia Commit-Bot committed Aug 10, 2018
1 parent 46c178b commit 609fbe3
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 20 deletions.
26 changes: 7 additions & 19 deletions src/gpu/mtl/GrMtlUniformHandler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@
// To determine whether a current offset is aligned, we can just 'and' the lowest bits with the
// alignment mask. A value of 0 means aligned, any other value is how many bytes past alignment we
// are. This works since all alignments are powers of 2. The mask is always (alignment - 1).
// This alignment mask will give correct alignments for using the std430 block layout. If you want
// the std140 alignment, you can use this, but then make sure if you have an array type it is
// aligned to 16 bytes (i.e. has mask of 0xF).
// These are designated in the Vulkan spec, section 14.5.4 "Offset and Stride Assignment".
// https://www.khronos.org/registry/vulkan/specs/1.0-wsi_extensions/html/Mtlspec.html#interfaces-resources-layout
uint32_t grsltype_to_alignment_mask(GrSLType type) {
switch(type) {
case kByte_GrSLType: // fall through
Expand Down Expand Up @@ -90,39 +85,39 @@ uint32_t grsltype_to_alignment_mask(GrSLType type) {
return 0;
}

/** Returns the size in bytes taken up in vulkanbuffers for GrSLTypes. */
/** Returns the size in bytes taken up in Metal buffers for GrSLTypes. */
static inline uint32_t grsltype_to_mtl_size(GrSLType type) {
switch(type) {
case kByte_GrSLType:
return sizeof(int8_t);
case kByte2_GrSLType:
return 2 * sizeof(int8_t);
case kByte3_GrSLType:
return 3 * sizeof(int8_t);
return 4 * sizeof(int8_t);
case kByte4_GrSLType:
return 4 * sizeof(int8_t);
case kUByte_GrSLType:
return sizeof(uint8_t);
case kUByte2_GrSLType:
return 2 * sizeof(uint8_t);
case kUByte3_GrSLType:
return 3 * sizeof(uint8_t);
return 4 * sizeof(uint8_t);
case kUByte4_GrSLType:
return 4 * sizeof(uint8_t);
case kShort_GrSLType:
return sizeof(int16_t);
case kShort2_GrSLType:
return 2 * sizeof(int16_t);
case kShort3_GrSLType:
return 3 * sizeof(int16_t);
return 4 * sizeof(int16_t);
case kShort4_GrSLType:
return 4 * sizeof(int16_t);
case kUShort_GrSLType:
return sizeof(uint16_t);
case kUShort2_GrSLType:
return 2 * sizeof(uint16_t);
case kUShort3_GrSLType:
return 3 * sizeof(uint16_t);
return 4 * sizeof(uint16_t);
case kUShort4_GrSLType:
return 4 * sizeof(uint16_t);
case kInt_GrSLType:
Expand All @@ -137,7 +132,7 @@ static inline uint32_t grsltype_to_mtl_size(GrSLType type) {
return 2 * sizeof(float);
case kHalf3_GrSLType: // fall through
case kFloat3_GrSLType:
return 3 * sizeof(float);
return 4 * sizeof(float);
case kHalf4_GrSLType: // fall through
case kFloat4_GrSLType:
return 4 * sizeof(float);
Expand All @@ -146,7 +141,7 @@ static inline uint32_t grsltype_to_mtl_size(GrSLType type) {
case kInt2_GrSLType:
return 2 * sizeof(int32_t);
case kInt3_GrSLType:
return 3 * sizeof(int32_t);
return 4 * sizeof(int32_t);
case kInt4_GrSLType:
return 4 * sizeof(int32_t);
case kHalf2x2_GrSLType: // fall through
Expand All @@ -172,9 +167,6 @@ static inline uint32_t grsltype_to_mtl_size(GrSLType type) {
return 0;
}

// TODO: The SKSLC assumes Metal sizes and alignments are calculated by the std140 layout below.
// This will need to be changed along with the MSL generator to match the Metal standard instead.
//
// Given the current offset into the ubo, calculate the offset for the uniform we're trying to add
// taking into consideration all alignment requirements. The uniformOffset is set to the offset for
// the new uniform, and currentOffset is updated to be the offset to the end of the new uniform.
Expand All @@ -183,10 +175,6 @@ void get_ubo_aligned_offset(uint32_t* uniformOffset,
GrSLType type,
int arrayCount) {
uint32_t alignmentMask = grsltype_to_alignment_mask(type);
// We want to use the std140 layout here, so we must make arrays align to 16 bytes.
if (arrayCount || type == kFloat2x2_GrSLType) {
alignmentMask = 0xF;
}
uint32_t offsetDiff = *currentOffset & alignmentMask;
if (offsetDiff != 0) {
offsetDiff = alignmentMask - offsetDiff + 1;
Expand Down
7 changes: 6 additions & 1 deletion src/sksl/SkSLMemoryLayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ class MemoryLayout {
public:
enum Standard {
k140_Standard,
k430_Standard
k430_Standard,
kMetal_Standard
};

MemoryLayout(Standard std)
Expand All @@ -35,6 +36,7 @@ class MemoryLayout {
switch (fStd) {
case k140_Standard: return (raw + 15) & ~15;
case k430_Standard: return raw;
case kMetal_Standard: return raw;
}
ABORT("unreachable");
}
Expand Down Expand Up @@ -103,6 +105,9 @@ class MemoryLayout {
// handle it...
return 4;
case Type::kVector_Kind:
if (fStd == kMetal_Standard && type.columns() == 3) {
return 4 * this->size(type.componentType());
}
return type.columns() * this->size(type.componentType());
case Type::kMatrix_Kind: // fall through
case Type::kArray_Kind:
Expand Down
12 changes: 12 additions & 0 deletions src/sksl/SkSLMetalCodeGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,11 @@ void MetalCodeGenerator::writeInterfaceBlock(const InterfaceBlock& intf) {

void MetalCodeGenerator::writeFields(const std::vector<Type::Field>& fields, int parentOffset,
const InterfaceBlock* parentIntf) {
#ifdef SK_MOLTENVK
MemoryLayout memoryLayout(MemoryLayout::k140_Standard);
#else
MemoryLayout memoryLayout(MemoryLayout::kMetal_Standard);
#endif
int currentOffset = 0;
for (const auto& field: fields) {
int fieldOffset = field.fModifiers.fLayout.fOffset;
Expand All @@ -796,13 +800,21 @@ void MetalCodeGenerator::writeFields(const std::vector<Type::Field>& fields, int
to_string((int) alignment));
}
}
#ifdef SK_MOLTENVK
if (fieldType->kind() == Type::kVector_Kind &&
fieldType->columns() == 3) {
SkASSERT(memoryLayout.size(*fieldType) == 3);
// Pack all vec3 types so that their size in bytes will match what was expected in the
// original SkSL code since MSL has vec3 sizes equal to 4 * component type, while SkSL
// has vec3 equal to 3 * component type.

// FIXME - Packed vectors can't be accessed by swizzles, but can be indexed into. A
// combination of this being a problem which only occurs when using MoltenVK and the
// fact that we haven't swizzled a vec3 yet means that this problem hasn't been
// addressed.
this->write(PACKED_PREFIX);
}
#endif
currentOffset += memoryLayout.size(*fieldType);
std::vector<int> sizes;
while (fieldType->kind() == Type::kArray_Kind) {
Expand Down

0 comments on commit 609fbe3

Please sign in to comment.