Add a size argument to Set[Vertex|Index]Buffer
Bug: dawn:22
Change-Id: I06a4fc2b651e5a4692893f710ca11785976c1fa4
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/20200
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
diff --git a/dawn.json b/dawn.json
index 5f49e04..ab7defd 100644
--- a/dawn.json
+++ b/dawn.json
@@ -1001,14 +1001,16 @@
"args": [
{"name": "slot", "type": "uint32_t"},
{"name": "buffer", "type": "buffer"},
- {"name": "offset", "type": "uint64_t", "default": "0"}
+ {"name": "offset", "type": "uint64_t", "default": "0"},
+ {"name": "size", "type": "uint64_t", "default": "0"}
]
},
{
"name": "set index buffer",
"args": [
{"name": "buffer", "type": "buffer"},
- {"name": "offset", "type": "uint64_t", "default": "0"}
+ {"name": "offset", "type": "uint64_t", "default": "0"},
+ {"name": "size", "type": "uint64_t", "default": "0"}
]
},
{
@@ -1186,14 +1188,16 @@
"args": [
{"name": "slot", "type": "uint32_t"},
{"name": "buffer", "type": "buffer"},
- {"name": "offset", "type": "uint64_t", "default": "0"}
+ {"name": "offset", "type": "uint64_t", "default": "0"},
+ {"name": "size", "type": "uint64_t", "default": "0"}
]
},
{
"name": "set index buffer",
"args": [
{"name": "buffer", "type": "buffer"},
- {"name": "offset", "type": "uint64_t", "default": "0"}
+ {"name": "offset", "type": "uint64_t", "default": "0"},
+ {"name": "size", "type": "uint64_t", "default": "0"}
]
},
{
diff --git a/src/dawn_native/Commands.h b/src/dawn_native/Commands.h
index 05632d7..8113b5d 100644
--- a/src/dawn_native/Commands.h
+++ b/src/dawn_native/Commands.h
@@ -218,12 +218,14 @@
struct SetIndexBufferCmd {
Ref<BufferBase> buffer;
uint64_t offset;
+ uint64_t size;
};
struct SetVertexBufferCmd {
uint32_t slot;
Ref<BufferBase> buffer;
uint64_t offset;
+ uint64_t size;
};
// This needs to be called before the CommandIterator is freed so that the Ref<> present in
diff --git a/src/dawn_native/RenderEncoderBase.cpp b/src/dawn_native/RenderEncoderBase.cpp
index aea8efe..7285fba 100644
--- a/src/dawn_native/RenderEncoderBase.cpp
+++ b/src/dawn_native/RenderEncoderBase.cpp
@@ -135,14 +135,29 @@
});
}
- void RenderEncoderBase::SetIndexBuffer(BufferBase* buffer, uint64_t offset) {
+ void RenderEncoderBase::SetIndexBuffer(BufferBase* buffer, uint64_t offset, uint64_t size) {
mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
DAWN_TRY(GetDevice()->ValidateObject(buffer));
+ uint64_t bufferSize = buffer->GetSize();
+ if (offset > bufferSize) {
+ return DAWN_VALIDATION_ERROR("Offset larger than the buffer size");
+ }
+ uint64_t remainingSize = bufferSize - offset;
+
+ if (size == 0) {
+ size = remainingSize;
+ } else {
+ if (size > remainingSize) {
+ return DAWN_VALIDATION_ERROR("Size + offset larger than the buffer size");
+ }
+ }
+
SetIndexBufferCmd* cmd =
allocator->Allocate<SetIndexBufferCmd>(Command::SetIndexBuffer);
cmd->buffer = buffer;
cmd->offset = offset;
+ cmd->size = size;
mUsageTracker.BufferUsedAs(buffer, wgpu::BufferUsage::Index);
@@ -150,7 +165,10 @@
});
}
- void RenderEncoderBase::SetVertexBuffer(uint32_t slot, BufferBase* buffer, uint64_t offset) {
+ void RenderEncoderBase::SetVertexBuffer(uint32_t slot,
+ BufferBase* buffer,
+ uint64_t offset,
+ uint64_t size) {
mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
DAWN_TRY(GetDevice()->ValidateObject(buffer));
@@ -158,11 +176,26 @@
return DAWN_VALIDATION_ERROR("Vertex buffer slot out of bounds");
}
+ uint64_t bufferSize = buffer->GetSize();
+ if (offset > bufferSize) {
+ return DAWN_VALIDATION_ERROR("Offset larger than the buffer size");
+ }
+ uint64_t remainingSize = bufferSize - offset;
+
+ if (size == 0) {
+ size = remainingSize;
+ } else {
+ if (size > remainingSize) {
+ return DAWN_VALIDATION_ERROR("Size + offset larger than the buffer size");
+ }
+ }
+
SetVertexBufferCmd* cmd =
allocator->Allocate<SetVertexBufferCmd>(Command::SetVertexBuffer);
cmd->slot = slot;
cmd->buffer = buffer;
cmd->offset = offset;
+ cmd->size = size;
mUsageTracker.BufferUsedAs(buffer, wgpu::BufferUsage::Vertex);
diff --git a/src/dawn_native/RenderEncoderBase.h b/src/dawn_native/RenderEncoderBase.h
index 33343ae..a4f3b9f 100644
--- a/src/dawn_native/RenderEncoderBase.h
+++ b/src/dawn_native/RenderEncoderBase.h
@@ -39,8 +39,8 @@
void SetPipeline(RenderPipelineBase* pipeline);
- void SetVertexBuffer(uint32_t slot, BufferBase* buffer, uint64_t offset);
- void SetIndexBuffer(BufferBase* buffer, uint64_t offset);
+ void SetVertexBuffer(uint32_t slot, BufferBase* buffer, uint64_t offset, uint64_t size);
+ void SetIndexBuffer(BufferBase* buffer, uint64_t offset, uint64_t size);
protected:
// Construct an "error" render encoder base.
diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
index 2726e5c..e195c03 100644
--- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp
+++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
@@ -298,13 +298,13 @@
namespace {
class VertexBufferTracker {
public:
- void OnSetVertexBuffer(uint32_t slot, Buffer* buffer, uint64_t offset) {
+ void OnSetVertexBuffer(uint32_t slot, Buffer* buffer, uint64_t offset, uint64_t size) {
mStartSlot = std::min(mStartSlot, slot);
mEndSlot = std::max(mEndSlot, slot + 1);
auto* d3d12BufferView = &mD3D12BufferViews[slot];
d3d12BufferView->BufferLocation = buffer->GetVA() + offset;
- d3d12BufferView->SizeInBytes = buffer->GetSize() - offset;
+ d3d12BufferView->SizeInBytes = size;
// The bufferView stride is set based on the vertex state before a draw.
}
@@ -360,9 +360,9 @@
class IndexBufferTracker {
public:
- void OnSetIndexBuffer(Buffer* buffer, uint64_t offset) {
+ void OnSetIndexBuffer(Buffer* buffer, uint64_t offset, uint64_t size) {
mD3D12BufferView.BufferLocation = buffer->GetVA() + offset;
- mD3D12BufferView.SizeInBytes = buffer->GetSize() - offset;
+ mD3D12BufferView.SizeInBytes = size;
// We don't need to dirty the state unless BufferLocation or SizeInBytes
// change, but most of the time this will always be the case.
@@ -1113,7 +1113,8 @@
case Command::SetIndexBuffer: {
SetIndexBufferCmd* cmd = iter->NextCommand<SetIndexBufferCmd>();
- indexBufferTracker.OnSetIndexBuffer(ToBackend(cmd->buffer.Get()), cmd->offset);
+ indexBufferTracker.OnSetIndexBuffer(ToBackend(cmd->buffer.Get()), cmd->offset,
+ cmd->size);
break;
}
@@ -1121,7 +1122,7 @@
SetVertexBufferCmd* cmd = iter->NextCommand<SetVertexBufferCmd>();
vertexBufferTracker.OnSetVertexBuffer(cmd->slot, ToBackend(cmd->buffer.Get()),
- cmd->offset);
+ cmd->offset, cmd->size);
break;
}
diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn
index aac3861..acae871 100644
--- a/src/tests/BUILD.gn
+++ b/src/tests/BUILD.gn
@@ -159,6 +159,7 @@
"unittests/validation/ErrorScopeValidationTests.cpp",
"unittests/validation/FenceValidationTests.cpp",
"unittests/validation/GetBindGroupLayoutValidationTests.cpp",
+ "unittests/validation/IndexBufferValidationTests.cpp",
"unittests/validation/QueueSubmitValidationTests.cpp",
"unittests/validation/RenderBundleValidationTests.cpp",
"unittests/validation/RenderPassDescriptorValidationTests.cpp",
diff --git a/src/tests/unittests/validation/IndexBufferValidationTests.cpp b/src/tests/unittests/validation/IndexBufferValidationTests.cpp
new file mode 100644
index 0000000..203e409
--- /dev/null
+++ b/src/tests/unittests/validation/IndexBufferValidationTests.cpp
@@ -0,0 +1,95 @@
+// Copyright 2020 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tests/unittests/validation/ValidationTest.h"
+
+#include "utils/ComboRenderBundleEncoderDescriptor.h"
+
+class IndexBufferValidationTest : public ValidationTest {};
+
+// Test that for OOB validation of index buffer offset and size.
+TEST_F(IndexBufferValidationTest, IndexBufferOffsetOOBValidation) {
+ wgpu::BufferDescriptor bufferDesc = {
+ .usage = wgpu::BufferUsage::Index,
+ .size = 256,
+ };
+ wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc);
+
+ DummyRenderPass renderPass(device);
+ // Control case, using the full buffer, with or without an explicit size is valid.
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ // Explicit size
+ pass.SetIndexBuffer(buffer, 0, 256);
+ // Implicit size
+ pass.SetIndexBuffer(buffer, 0, 0);
+ pass.SetIndexBuffer(buffer, 256 - 4, 0);
+ pass.SetIndexBuffer(buffer, 4, 0);
+ // Implicit size of zero
+ pass.SetIndexBuffer(buffer, 256, 0);
+ pass.EndPass();
+ encoder.Finish();
+ }
+
+ // Bad case, offset + size is larger than the buffer
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.SetIndexBuffer(buffer, 4, 256);
+ pass.EndPass();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ // Bad case, size is 0 but the offset is larger than the buffer
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.SetIndexBuffer(buffer, 256 + 4, 0);
+ pass.EndPass();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ utils::ComboRenderBundleEncoderDescriptor renderBundleDesc = {};
+ renderBundleDesc.colorFormatsCount = 1;
+ renderBundleDesc.cColorFormats[0] = wgpu::TextureFormat::RGBA8Unorm;
+
+ // Control case, using the full buffer, with or without an explicit size is valid.
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ // Explicit size
+ encoder.SetIndexBuffer(buffer, 0, 256);
+ // Implicit size
+ encoder.SetIndexBuffer(buffer, 0, 0);
+ encoder.SetIndexBuffer(buffer, 256 - 4, 0);
+ encoder.SetIndexBuffer(buffer, 4, 0);
+ // Implicit size of zero
+ encoder.SetIndexBuffer(buffer, 256, 0);
+ encoder.Finish();
+ }
+
+ // Bad case, offset + size is larger than the buffer
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ encoder.SetIndexBuffer(buffer, 4, 256);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ // Bad case, size is 0 but the offset is larger than the buffer
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ encoder.SetIndexBuffer(buffer, 256 + 4, 0);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+}
diff --git a/src/tests/unittests/validation/VertexBufferValidationTests.cpp b/src/tests/unittests/validation/VertexBufferValidationTests.cpp
index 65417df..9c67a0f 100644
--- a/src/tests/unittests/validation/VertexBufferValidationTests.cpp
+++ b/src/tests/unittests/validation/VertexBufferValidationTests.cpp
@@ -211,3 +211,75 @@
ASSERT_DEVICE_ERROR(encoder.Finish());
}
}
+
+// Test that for OOB validation of vertex buffer offset and size.
+TEST_F(VertexBufferValidationTest, VertexBufferOffsetOOBValidation) {
+ wgpu::Buffer buffer = MakeVertexBuffer();
+
+ DummyRenderPass renderPass(device);
+ // Control case, using the full buffer, with or without an explicit size is valid.
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ // Explicit size
+ pass.SetVertexBuffer(0, buffer, 0, 256);
+ // Implicit size
+ pass.SetVertexBuffer(0, buffer, 0, 0);
+ pass.SetVertexBuffer(0, buffer, 256 - 4, 0);
+ pass.SetVertexBuffer(0, buffer, 4, 0);
+ // Implicit size of zero
+ pass.SetVertexBuffer(0, buffer, 256, 0);
+ pass.EndPass();
+ encoder.Finish();
+ }
+
+ // Bad case, offset + size is larger than the buffer
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.SetVertexBuffer(0, buffer, 4, 256);
+ pass.EndPass();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ // Bad case, size is 0 but the offset is larger than the buffer
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.SetVertexBuffer(0, buffer, 256 + 4, 0);
+ pass.EndPass();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ utils::ComboRenderBundleEncoderDescriptor renderBundleDesc = {};
+ renderBundleDesc.colorFormatsCount = 1;
+ renderBundleDesc.cColorFormats[0] = wgpu::TextureFormat::RGBA8Unorm;
+
+ // Control case, using the full buffer, with or without an explicit size is valid.
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ // Explicit size
+ encoder.SetVertexBuffer(0, buffer, 0, 256);
+ // Implicit size
+ encoder.SetVertexBuffer(0, buffer, 0, 0);
+ encoder.SetVertexBuffer(0, buffer, 256 - 4, 0);
+ encoder.SetVertexBuffer(0, buffer, 4, 0);
+ // Implicit size of zero
+ encoder.SetVertexBuffer(0, buffer, 256, 0);
+ encoder.Finish();
+ }
+
+ // Bad case, offset + size is larger than the buffer
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ encoder.SetVertexBuffer(0, buffer, 4, 256);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ // Bad case, size is 0 but the offset is larger than the buffer
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ encoder.SetVertexBuffer(0, buffer, 256 + 4, 0);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+}
