Align the size of Uniform and Storage buffer to 16 bytes in Metal
On Metal, it requires the size of the constant buffer to be no less
than the size of the constant buffer block defined in shader, and
the overall size of the constant buffer must be aligned to the
largest alignment of its members.
BUG=dawn:139
Change-Id: I37730b1415baecb6638aaaacec87789decf07606
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10920
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Hao Li <hao.x.li@intel.com>
diff --git a/src/dawn_native/metal/BufferMTL.mm b/src/dawn_native/metal/BufferMTL.mm
index dfe96f3..076e2b8 100644
--- a/src/dawn_native/metal/BufferMTL.mm
+++ b/src/dawn_native/metal/BufferMTL.mm
@@ -14,9 +14,13 @@
#include "dawn_native/metal/BufferMTL.h"
+#include "common/Math.h"
#include "dawn_native/metal/DeviceMTL.h"
namespace dawn_native { namespace metal {
+ // The size of uniform buffer and storage buffer need to be aligned to 16 bytes which is the
+ // largest alignment of supported data types
+ static constexpr uint32_t kMinUniformOrStorageBufferAlignment = 16u;
Buffer::Buffer(Device* device, const BufferDescriptor* descriptor)
: BufferBase(device, descriptor) {
@@ -27,7 +31,15 @@
storageMode = MTLResourceStorageModePrivate;
}
- mMtlBuffer = [device->GetMTLDevice() newBufferWithLength:GetSize() options:storageMode];
+ uint32_t currentSize = GetSize();
+ // Metal validation layer requires the size of uniform buffer and storage buffer to be no
+ // less than the size of the buffer block defined in shader, and the overall size of the
+ // buffer must be aligned to the largest alignment of its members.
+ if (GetUsage() & (dawn::BufferUsage::Uniform | dawn::BufferUsage::Storage)) {
+ currentSize = Align(currentSize, kMinUniformOrStorageBufferAlignment);
+ }
+
+ mMtlBuffer = [device->GetMTLDevice() newBufferWithLength:currentSize options:storageMode];
}
Buffer::~Buffer() {
diff --git a/src/tests/end2end/ComputeIndirectTests.cpp b/src/tests/end2end/ComputeIndirectTests.cpp
index 3e35bb2..60fe850 100644
--- a/src/tests/end2end/ComputeIndirectTests.cpp
+++ b/src/tests/end2end/ComputeIndirectTests.cpp
@@ -105,21 +105,11 @@
// Test basic indirect
TEST_P(ComputeIndirectTests, Basic) {
- // TODO(hao.x.li@intel.com): Test failing on Metal with validation layer on, which blocks
- // end2end tests run with validation layer in bots. Suppress this while we're fixing.
- // See https://bugs.chromium.org/p/dawn/issues/detail?id=139
- DAWN_SKIP_TEST_IF(IsMetal() && IsBackendValidationEnabled());
-
BasicTest({2, 3, 4}, 0);
}
// Test indirect with buffer offset
TEST_P(ComputeIndirectTests, IndirectOffset) {
- // TODO(hao.x.li@intel.com): Test failing on Metal with validation layer on, which blocks
- // end2end tests run with validation layer in bots. Suppress this while we're fixing.
- // See https://bugs.chromium.org/p/dawn/issues/detail?id=139
- DAWN_SKIP_TEST_IF(IsMetal() && IsBackendValidationEnabled());
-
BasicTest({0, 0, 0, 2, 3, 4}, 3 * sizeof(uint32_t));
}
diff --git a/src/tests/end2end/MultisampledRenderingTests.cpp b/src/tests/end2end/MultisampledRenderingTests.cpp
index ff9f628..b291487 100644
--- a/src/tests/end2end/MultisampledRenderingTests.cpp
+++ b/src/tests/end2end/MultisampledRenderingTests.cpp
@@ -255,11 +255,6 @@
// Test multisampled rendering with depth test works correctly.
TEST_P(MultisampledRenderingTest, MultisampledRenderingWithDepthTest) {
- // TODO(hao.x.li@intel.com): Test failing on Metal with validation layer on, which blocks
- // end2end tests run with validation layer in bots. Suppress this while we're fixing.
- // See https://bugs.chromium.org/p/dawn/issues/detail?id=139
- DAWN_SKIP_TEST_IF(IsMetal() && IsBackendValidationEnabled());
-
constexpr bool kTestDepth = true;
dawn::CommandEncoder commandEncoder = device.CreateCommandEncoder();
dawn::RenderPipeline pipeline = CreateRenderPipelineWithOneOutputForTest(kTestDepth);
