encounter/dawn-cmake
1
0
Fork 0
mirror of https://github.com/encounter/dawn-cmake.git synced 2025-07-15 01:26:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

dawn_native: Move pass validation of buffer usages into the encoder.

Browse Source 
In a future CL the PassResourceUsage structure will become a
SyncScopeResourceUsage and will be used to validate at each
synchronization scope. For separation of concerns, the validation that
resource have the correct usage shouldn't be done at the sync scope
level but at each entrypoint that uses the resource.

The validation tests had no coverage of usage validation for pass usage
so validation tests are added for Indirct/Index/Vertex usages. (Uniform
and Storage are validated at bindgroup creation and already had
validation tests)

Bug: dawn:635
Change-Id: I5058ad30eb041809f0f60d9403f3cc2d5d7e7c96
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38380
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Corentin Wallez 2021-01-22 09:57:38 +00:00 committed by Commit Bot service account
parent db383498c5
commit 310d86f4a0
7 changed files with 123 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/dawn_native/CommandValidation.cpp
View File

@ -312,13 +312,7 @@ namespace dawn_native {
MaybeError ValidatePassResourceUsage(const PassResourceUsage& pass) {
// Buffers can only be used as single-write or multiple read.
for (size_t i = 0; i < pass.buffers.size(); ++i) {
const BufferBase* buffer = pass.buffers[i];
wgpu::BufferUsage usage = pass.bufferUsages[i];
if (usage & ~buffer->GetUsage()) {
return DAWN_VALIDATION_ERROR("Buffer missing usage for the pass");
}
bool readOnly = IsSubset(usage, kReadOnlyBufferUsages);
bool singleUse = wgpu::HasZeroOrOneBits(usage);

1
src/dawn_native/ComputePassEncoder.cpp
View File

@ -69,6 +69,7 @@ namespace dawn_native {
void ComputePassEncoder::DispatchIndirect(BufferBase* indirectBuffer, uint64_t indirectOffset) {
mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer));
DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect));
// Indexed dispatches need a compute-shader based validation to check that the dispatch
// sizes aren't too big. Disallow them as unsafe until the validation is implemented.

5
src/dawn_native/RenderEncoderBase.cpp
View File

@ -17,6 +17,7 @@
#include "common/Constants.h"
#include "dawn_native/Buffer.h"
#include "dawn_native/CommandEncoder.h"
#include "dawn_native/CommandValidation.h"
#include "dawn_native/Commands.h"
#include "dawn_native/Device.h"
#include "dawn_native/RenderPipeline.h"
@ -87,6 +88,7 @@ namespace dawn_native {
void RenderEncoderBase::DrawIndirect(BufferBase* indirectBuffer, uint64_t indirectOffset) {
mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer));
DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect));
if (indirectOffset % 4 != 0) {
return DAWN_VALIDATION_ERROR("Indirect offset must be a multiple of 4");
@ -111,6 +113,7 @@ namespace dawn_native {
uint64_t indirectOffset) {
mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer));
DAWN_TRY(ValidateCanUseAs(indirectBuffer, wgpu::BufferUsage::Indirect));
// Indexed indirect draws need a compute-shader based validation check that the range of
// indices is contained inside the index buffer on Metal. Disallow them as unsafe until
@ -167,6 +170,7 @@ namespace dawn_native {
uint64_t size) {
mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
DAWN_TRY(GetDevice()->ValidateObject(buffer));
DAWN_TRY(ValidateCanUseAs(buffer, wgpu::BufferUsage::Index));
DAWN_TRY(ValidateIndexFormat(format));
if (format == wgpu::IndexFormat::Undefined) {
@ -206,6 +210,7 @@ namespace dawn_native {
uint64_t size) {
mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
DAWN_TRY(GetDevice()->ValidateObject(buffer));
DAWN_TRY(ValidateCanUseAs(buffer, wgpu::BufferUsage::Vertex));
if (slot >= kMaxVertexBuffers) {
return DAWN_VALIDATION_ERROR("Vertex buffer slot out of bounds");

14
src/tests/unittests/validation/ComputeIndirectValidationTests.cpp
View File

@ -46,9 +46,10 @@ class ComputeIndirectValidationTest : public ValidationTest {
void TestIndirectOffset(utils::Expectation expectation,
std::initializer_list<uint32_t> bufferList,
uint64_t indirectOffset) {
uint64_t indirectOffset,
wgpu::BufferUsage usage = wgpu::BufferUsage::Indirect) {
wgpu::Buffer indirectBuffer =
utils::CreateBufferFromData<uint32_t>(device, wgpu::BufferUsage::Indirect, bufferList);
utils::CreateBufferFromData<uint32_t>(device, usage, bufferList);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
@ -85,3 +86,12 @@ TEST_F(ComputeIndirectValidationTest, IndirectOffsetBounds) {
uint64_t offset = std::numeric_limits<uint64_t>::max();
TestIndirectOffset(utils::Expectation::Failure, {1, 2, 3, 4, 5, 6}, offset);
}
// Check that the buffer must have the indirect usage
TEST_F(ComputeIndirectValidationTest, IndirectUsage) {
// Control case: using a buffer with the indirect usage is valid.
TestIndirectOffset(utils::Expectation::Success, {1, 2, 3}, 0, wgpu::BufferUsage::Indirect);
// Error case: using a buffer with the vertex usage is an error.
TestIndirectOffset(utils::Expectation::Failure, {1, 2, 3}, 0, wgpu::BufferUsage::Vertex);
}

20
src/tests/unittests/validation/DrawIndirectValidationTests.cpp
View File

@ -69,9 +69,10 @@ class DrawIndirectValidationTest : public ValidationTest {
void TestIndirectOffset(utils::Expectation expectation,
std::initializer_list<uint32_t> bufferList,
uint64_t indirectOffset,
bool indexed) {
bool indexed,
wgpu::BufferUsage usage = wgpu::BufferUsage::Indirect) {
wgpu::Buffer indirectBuffer =
utils::CreateBufferFromData<uint32_t>(device, wgpu::BufferUsage::Indirect, bufferList);
utils::CreateBufferFromData<uint32_t>(device, usage, bufferList);
DummyRenderPass renderPass(device);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
@ -146,3 +147,18 @@ TEST_F(DrawIndirectValidationTest, DrawIndexedIndirectOffsetBounds) {
TestIndirectOffsetDrawIndexed(utils::Expectation::Failure, {1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
offset);
}
// Check that the buffer must have the indirect usage
TEST_F(DrawIndirectValidationTest, IndirectUsage) {
// Control cases: using a buffer with the indirect usage is valid.
TestIndirectOffset(utils::Expectation::Success, {1, 2, 3, 4}, 0, false,
wgpu::BufferUsage::Indirect);
TestIndirectOffset(utils::Expectation::Success, {1, 2, 3, 4, 5}, 0, true,
wgpu::BufferUsage::Indirect);
// Error cases: using a buffer with the vertex usage is an error.
TestIndirectOffset(utils::Expectation::Failure, {1, 2, 3, 4}, 0, false,
wgpu::BufferUsage::Vertex);
TestIndirectOffset(utils::Expectation::Failure, {1, 2, 3, 4, 5}, 0, true,
wgpu::BufferUsage::Vertex);
}

42
src/tests/unittests/validation/IndexBufferValidationTests.cpp
View File

@ -184,3 +184,45 @@ TEST_F(IndexBufferValidationTest, IndexBufferFormatMatchesPipelineStripFormat) {
encoder.Finish();
}
}
// Check that the index buffer must have the Index usage.
TEST_F(IndexBufferValidationTest, InvalidUsage) {
wgpu::Buffer indexBuffer =
utils::CreateBufferFromData<uint32_t>(device, wgpu::BufferUsage::Index, {0, 1, 2});
wgpu::Buffer copyBuffer =
utils::CreateBufferFromData<uint32_t>(device, wgpu::BufferUsage::CopySrc, {0, 1, 2});
DummyRenderPass renderPass(device);
// Control case: using the index buffer is valid.
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
pass.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint32);
pass.EndPass();
encoder.Finish();
}
// Error case: using the copy buffer is an error.
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
pass.SetIndexBuffer(copyBuffer, wgpu::IndexFormat::Uint32);
pass.EndPass();
ASSERT_DEVICE_ERROR(encoder.Finish());
}
utils::ComboRenderBundleEncoderDescriptor renderBundleDesc = {};
renderBundleDesc.colorFormatsCount = 1;
renderBundleDesc.cColorFormats[0] = wgpu::TextureFormat::RGBA8Unorm;
// Control case: using the index buffer is valid.
{
wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
encoder.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint32);
encoder.Finish();
}
// Error case: using the copy buffer is an error.
{
wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
encoder.SetIndexBuffer(copyBuffer, wgpu::IndexFormat::Uint32);
ASSERT_DEVICE_ERROR(encoder.Finish());
}
}

46
src/tests/unittests/validation/VertexBufferValidationTests.cpp
View File

@ -83,6 +83,7 @@ class VertexBufferValidationTest : public ValidationTest {
wgpu::ShaderModule fsModule;
};
// Check that vertex buffers still count as bound if we switch the pipeline.
TEST_F(VertexBufferValidationTest, VertexBuffersInheritedBetweenPipelines) {
DummyRenderPass renderPass(device);
wgpu::ShaderModule vsModule2 = MakeVertexShader(2);
@ -119,7 +120,8 @@ TEST_F(VertexBufferValidationTest, VertexBuffersInheritedBetweenPipelines) {
encoder.Finish();
}
TEST_F(VertexBufferValidationTest, VertexBuffersNotInheritedBetweenRendePasses) {
// Check that vertex buffers that are set are reset between render passes.
TEST_F(VertexBufferValidationTest, VertexBuffersNotInheritedBetweenRenderPasses) {
DummyRenderPass renderPass(device);
wgpu::ShaderModule vsModule2 = MakeVertexShader(2);
wgpu::ShaderModule vsModule1 = MakeVertexShader(1);
@ -168,6 +170,7 @@ TEST_F(VertexBufferValidationTest, VertexBuffersNotInheritedBetweenRendePasses)
ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Check validation of the vertex buffer slot for OOB.
TEST_F(VertexBufferValidationTest, VertexBufferSlotValidation) {
wgpu::Buffer buffer = MakeVertexBuffer();
@ -281,3 +284,44 @@ TEST_F(VertexBufferValidationTest, VertexBufferOffsetOOBValidation) {
ASSERT_DEVICE_ERROR(encoder.Finish());
}
}
// Check that the vertex buffer must have the Vertex usage.
TEST_F(VertexBufferValidationTest, InvalidUsage) {
wgpu::Buffer vertexBuffer = MakeVertexBuffer();
wgpu::Buffer indexBuffer =
utils::CreateBufferFromData<uint32_t>(device, wgpu::BufferUsage::Index, {0, 0, 0});
DummyRenderPass renderPass(device);
// Control case: using the vertex buffer is valid.
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
pass.SetVertexBuffer(0, vertexBuffer);
pass.EndPass();
encoder.Finish();
}
// Error case: using the index buffer is an error.
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
pass.SetVertexBuffer(0, indexBuffer);
pass.EndPass();
ASSERT_DEVICE_ERROR(encoder.Finish());
}
utils::ComboRenderBundleEncoderDescriptor renderBundleDesc = {};
renderBundleDesc.colorFormatsCount = 1;
renderBundleDesc.cColorFormats[0] = wgpu::TextureFormat::RGBA8Unorm;
// Control case: using the vertex buffer is valid.
{
wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
encoder.SetVertexBuffer(0, vertexBuffer);
encoder.Finish();
}
// Error case: using the index buffer is an error.
{
wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
encoder.SetVertexBuffer(0, indexBuffer);
ASSERT_DEVICE_ERROR(encoder.Finish());
}
}