encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Sort same-type bindings in the BGL by binding number 

This fixes a bug where dynamic offsets were applied to the wrong bindings.
Dynamic offsets are applied in increasing order of binding number.

Bug: dawn:408
Change-Id: I3de6ee1bfd6e00239ddc46f820c3f81ba82815cb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/21620
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
This commit is contained in:
Austin Eng 2020-05-12 20:32:45 +00:00 committed by Commit Bot service account
parent b6eeee0aa3
commit fce44b5fe1
3 changed files with 180 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/dawn_native/BindGroupLayout.cpp
View File

@ -214,6 +214,12 @@ namespace dawn_native {
// Buffers have smaller type enums. They should be placed first.
return a.type < b.type;
}
if (a.binding != b.binding) {
// Above we ensure that dynamic buffers are first. Now, ensure that bindings are in
// increasing order. This is because dynamic buffer offsets are applied in
// increasing order of binding number.
return a.binding < b.binding;
}
if (a.visibility != b.visibility) {
return a.visibility < b.visibility;
}

83
src/tests/end2end/BindGroupTests.cpp
View File

@ -745,6 +745,89 @@ TEST_P(BindGroupTests, DrawThenChangePipelineAndBindGroup) {
EXPECT_PIXEL_RGBA8_EQ(notFilled, renderPass.color, max, max);
}
// Regression test for crbug.com/dawn/408 where dynamic offsets were applied in the wrong order.
// Dynamic offsets should be applied in increasing order of binding number.
TEST_P(BindGroupTests, DynamicOffsetOrder) {
// We will put the following values and the respective offsets into a buffer.
// The test will ensure that the correct dynamic offset is applied to each buffer by reading the
// value from an offset binding.
std::array<uint32_t, 3> offsets = {3 * kMinDynamicBufferOffsetAlignment,
1 * kMinDynamicBufferOffsetAlignment,
2 * kMinDynamicBufferOffsetAlignment};
std::array<uint32_t, 3> values = {21, 67, 32};
// Create three buffers large enough to by offset by the largest offset.
wgpu::BufferDescriptor bufferDescriptor;
bufferDescriptor.size = 3 * kMinDynamicBufferOffsetAlignment + sizeof(uint32_t);
bufferDescriptor.usage = wgpu::BufferUsage::Storage | wgpu::BufferUsage::CopyDst;
wgpu::Buffer buffer0 = device.CreateBuffer(&bufferDescriptor);
wgpu::Buffer buffer2 = device.CreateBuffer(&bufferDescriptor);
wgpu::Buffer buffer3 = device.CreateBuffer(&bufferDescriptor);
// Populate the values
buffer0.SetSubData(offsets[0], sizeof(uint32_t), &values[0]);
buffer2.SetSubData(offsets[1], sizeof(uint32_t), &values[1]);
buffer3.SetSubData(offsets[2], sizeof(uint32_t), &values[2]);
wgpu::Buffer outputBuffer = utils::CreateBufferFromData(
device, wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::Storage, {0, 0, 0});
// Create the bind group and bind group layout.
// Note: The order of the binding numbers are intentionally different and not in increasing
// order.
wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
device, {
{3, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
{2, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
{4, wgpu::ShaderStage::Compute, wgpu::BindingType::StorageBuffer},
});
wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, bgl,
{
{0, buffer0, 0, sizeof(uint32_t)},
{3, buffer3, 0, sizeof(uint32_t)},
{2, buffer2, 0, sizeof(uint32_t)},
{4, outputBuffer, 0, 3 * sizeof(uint32_t)},
});
wgpu::ComputePipelineDescriptor pipelineDescriptor;
pipelineDescriptor.computeStage.module =
utils::CreateShaderModule(device, utils::SingleShaderStage::Compute, R"(
#version 450
layout(std430, set = 0, binding = 2) readonly buffer Buffer2 {
uint value2;
};
layout(std430, set = 0, binding = 3) readonly buffer Buffer3 {
uint value3;
};
layout(std430, set = 0, binding = 0) readonly buffer Buffer0 {
uint value0;
};
layout(std430, set = 0, binding = 4) buffer OutputBuffer {
uvec3 outputBuffer;
};
void main() {
outputBuffer = uvec3(value0, value2, value3);
}
)");
pipelineDescriptor.computeStage.entryPoint = "main";
pipelineDescriptor.layout = utils::MakeBasicPipelineLayout(device, &bgl);
wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&pipelineDescriptor);
wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder computePassEncoder = commandEncoder.BeginComputePass();
computePassEncoder.SetPipeline(pipeline);
computePassEncoder.SetBindGroup(0, bindGroup, offsets.size(), offsets.data());
computePassEncoder.Dispatch(1);
computePassEncoder.EndPass();
wgpu::CommandBuffer commands = commandEncoder.Finish();
queue.Submit(1, &commands);
EXPECT_BUFFER_U32_RANGE_EQ(values.data(), outputBuffer, 0, values.size());
}
// Test that visibility of bindings in BindGroupLayout can be none
// This test passes by not asserting or crashing.
TEST_P(BindGroupTests, BindGroupLayoutVisibilityCanBeNone) {

91
src/tests/unittests/validation/BindGroupValidationTests.cpp
View File

@ -1009,6 +1009,97 @@ TEST_F(SetBindGroupValidationTest, BindingSizeOutOfBoundDynamicStorageBuffer) {
TestComputePassBindGroup(bindGroup, offsets.data(), 3, false);
}
// Regression test for crbug.com/dawn/408 where dynamic offsets were applied in the wrong order.
// Dynamic offsets should be applied in increasing order of binding number.
TEST_F(SetBindGroupValidationTest, DynamicOffsetOrder) {
// Note: The order of the binding numbers of the bind group and bind group layout are
// intentionally different and not in increasing order.
wgpu::BindGroupLayout bgl = utils::MakeBindGroupLayout(
device, {
{3, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
{0, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
{2, wgpu::ShaderStage::Compute, wgpu::BindingType::ReadonlyStorageBuffer, true},
});
// Create buffers which are 3x, 2x, and 1x the size of the minimum buffer offset, plus 4 bytes
// to spare (to avoid zero-sized bindings). We will offset the bindings so they reach the very
// end of the buffer. Any mismatch applying too-large of an offset to a smaller buffer will hit the
// out-of-bounds condition during validation.
wgpu::Buffer buffer3x =
CreateBuffer(3 * kMinDynamicBufferOffsetAlignment + 4, wgpu::BufferUsage::Storage);
wgpu::Buffer buffer2x =
CreateBuffer(2 * kMinDynamicBufferOffsetAlignment + 4, wgpu::BufferUsage::Storage);
wgpu::Buffer buffer1x =
CreateBuffer(1 * kMinDynamicBufferOffsetAlignment + 4, wgpu::BufferUsage::Storage);
wgpu::BindGroup bindGroup = utils::MakeBindGroup(device, bgl,
{
{0, buffer3x, 0, 4},
{3, buffer2x, 0, 4},
{2, buffer1x, 0, 4},
});
std::array<uint32_t, 3> offsets;
{
// Base case works.
offsets = {/* binding 0 */ 0,
/* binding 2 */ 0,
/* binding 3 */ 0};
wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder computePassEncoder = commandEncoder.BeginComputePass();
computePassEncoder.SetBindGroup(0, bindGroup, offsets.size(), offsets.data());
computePassEncoder.EndPass();
commandEncoder.Finish();
}
{
// Offset the first binding to touch the end of the buffer. Should succeed.
// Will fail if the offset is applied to the first or second bindings since their buffers
// are too small.
offsets = {/* binding 0 */ 3 * kMinDynamicBufferOffsetAlignment,
/* binding 2 */ 0,
/* binding 3 */ 0};
wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder computePassEncoder = commandEncoder.BeginComputePass();
computePassEncoder.SetBindGroup(0, bindGroup, offsets.size(), offsets.data());
computePassEncoder.EndPass();
commandEncoder.Finish();
}
{
// Offset the second binding to touch the end of the buffer. Should succeed.
offsets = {/* binding 0 */ 0,
/* binding 2 */ 1 * kMinDynamicBufferOffsetAlignment,
/* binding 3 */ 0};
wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder computePassEncoder = commandEncoder.BeginComputePass();
computePassEncoder.SetBindGroup(0, bindGroup, offsets.size(), offsets.data());
computePassEncoder.EndPass();
commandEncoder.Finish();
}
{
// Offset the third binding to touch the end of the buffer. Should succeed.
// Will fail if the offset is applied to the second binding since its buffer
// is too small.
offsets = {/* binding 0 */ 0,
/* binding 2 */ 0,
/* binding 3 */ 2 * kMinDynamicBufferOffsetAlignment};
wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder computePassEncoder = commandEncoder.BeginComputePass();
computePassEncoder.SetBindGroup(0, bindGroup, offsets.size(), offsets.data());
computePassEncoder.EndPass();
commandEncoder.Finish();
}
{
// Offset each binding to touch the end of their buffer. Should succeed.
offsets = {/* binding 0 */ 3 * kMinDynamicBufferOffsetAlignment,
/* binding 2 */ 1 * kMinDynamicBufferOffsetAlignment,
/* binding 3 */ 2 * kMinDynamicBufferOffsetAlignment};
wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder computePassEncoder = commandEncoder.BeginComputePass();
computePassEncoder.SetBindGroup(0, bindGroup, offsets.size(), offsets.data());
computePassEncoder.EndPass();
commandEncoder.Finish();
}
}
// Test that an error is produced (and no ASSERTs fired) when using an error bindgroup in
// SetBindGroup
TEST_F(SetBindGroupValidationTest, ErrorBindGroup) {