commit 95aa7c5b295be93cd46c89a26178bc43d3406943
author Corentin Wallez <cwallez@chromium.org> Thu Jun 17 17:48:45 2021 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jun 17 17:48:45 2021 +0000
tree 58b0f820cb30c5febab0de11f3f35baa6ceb0d1b
parent 75f554d9739cb62a3fcdd0e8cafb9115cd853787

Add SetIndex/VertexBuffer offset argument alignment constraints

The vertex buffer alignment in particular is helps make the
implementation of programmable pulling much easier on Metal since the
vertex data will be able to be represented as array<u32>.

Bug: dawn:805
Change-Id: I2bf2742db3b8fa478be620c892925b8b75dc514c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/54659
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>

src/dawn_native/RenderEncoderBase.cpp

diff --git a/src/dawn_native/RenderEncoderBase.cpp b/src/dawn_native/RenderEncoderBase.cpp
index f1e7c42..2081a91 100644
--- a/src/dawn_native/RenderEncoderBase.cpp
+++ b/src/dawn_native/RenderEncoderBase.cpp
@@ -217,6 +217,11 @@
                     return DAWN_VALIDATION_ERROR("Index format must be specified");
                 }
 
+                if (offset % uint64_t(IndexFormatSize(format)) != 0) {
+                    return DAWN_VALIDATION_ERROR(
+                        "Offset must be a multiple of the index format size");
+                }
+
                 uint64_t bufferSize = buffer->GetSize();
                 if (offset > bufferSize) {
                     return DAWN_VALIDATION_ERROR("Offset larger than the buffer size");
@@ -264,6 +269,10 @@
                     return DAWN_VALIDATION_ERROR("Vertex buffer slot out of bounds");
                 }
 
+                if (offset % 4 != 0) {
+                    return DAWN_VALIDATION_ERROR("Offset must be a multiple of 4");
+                }
+
                 uint64_t bufferSize = buffer->GetSize();
                 if (offset > bufferSize) {
                     return DAWN_VALIDATION_ERROR("Offset larger than the buffer size");

src/tests/unittests/validation/IndexBufferValidationTests.cpp

diff --git a/src/tests/unittests/validation/IndexBufferValidationTests.cpp b/src/tests/unittests/validation/IndexBufferValidationTests.cpp
index 27f7b21..f2da91d 100644
--- a/src/tests/unittests/validation/IndexBufferValidationTests.cpp
+++ b/src/tests/unittests/validation/IndexBufferValidationTests.cpp
@@ -224,3 +224,39 @@
         ASSERT_DEVICE_ERROR(encoder.Finish());
     }
 }
+
+// Check the alignment constraint on the index buffer offset.
+TEST_F(IndexBufferValidationTest, OffsetAlignment) {
+    wgpu::Buffer indexBuffer =
+        utils::CreateBufferFromData<uint32_t>(device, wgpu::BufferUsage::Index, {0, 1, 2});
+
+    DummyRenderPass renderPass(device);
+    // Control cases: index buffer offset is a multiple of the index format size
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint32, 0);
+        pass.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint32, 4);
+        pass.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint16, 0);
+        pass.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint16, 2);
+        pass.EndPass();
+        encoder.Finish();
+    }
+
+    // Error case: index buffer offset isn't a multiple of 4 for IndexFormat::Uint32
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint32, 2);
+        pass.EndPass();
+        ASSERT_DEVICE_ERROR(encoder.Finish());
+    }
+    // Error case: index buffer offset isn't a multiple of 2 for IndexFormat::Uint16
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint16, 1);
+        pass.EndPass();
+        ASSERT_DEVICE_ERROR(encoder.Finish());
+    }
+}

src/tests/unittests/validation/VertexBufferValidationTests.cpp

diff --git a/src/tests/unittests/validation/VertexBufferValidationTests.cpp b/src/tests/unittests/validation/VertexBufferValidationTests.cpp
index bcc8825..31a6f65 100644
--- a/src/tests/unittests/validation/VertexBufferValidationTests.cpp
+++ b/src/tests/unittests/validation/VertexBufferValidationTests.cpp
@@ -328,3 +328,29 @@
         ASSERT_DEVICE_ERROR(encoder.Finish());
     }
 }
+
+// Check the alignment constraint on the index buffer offset.
+TEST_F(VertexBufferValidationTest, OffsetAlignment) {
+    wgpu::Buffer vertexBuffer = MakeVertexBuffer();
+
+    DummyRenderPass renderPass(device);
+    // Control cases: vertex buffer offset is a multiple of 4
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.SetVertexBuffer(0, vertexBuffer, 0);
+        pass.SetVertexBuffer(0, vertexBuffer, 4);
+        pass.SetVertexBuffer(0, vertexBuffer, 12);
+        pass.EndPass();
+        encoder.Finish();
+    }
+
+    // Error case: vertex buffer offset isn't a multiple of 4
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.SetVertexBuffer(0, vertexBuffer, 2);
+        pass.EndPass();
+        ASSERT_DEVICE_ERROR(encoder.Finish());
+    }
+}