Record zero-attribute vertex buffer when creating render pipeline
Currently Dawn ignores all zero-attribute vertex buffer in the given
pipeline descriptor when creating RenderPipelineBase because
zero-attribute vertex buffer is treated as unused slot, however the spec
doesn't state that zero-attribute vertex buffer should be ignored.
To support zero-attribute vertex buffer, this commit has the following
changes.
1. Add VertexBufferNotUsed enum value to wgpu::VertexStepMode to
represent unused slots
2. Ignore VertexBufferNotUsed step mode buffers when creating
RenderPipelineBase and add tests to check it
3. Record zero-attribute vertex buffers when creating RenderPipelineBase
and add tests to check it
4. Fix VertexStateTest::LastAllowedVertexBuffer broken by the above
changes
Temporarily we set the enum value of
wgpu::VertexStepMode::VertexBufferNotUsed to 0 to pass the CTS tests
because currently empty vertex buffer slots step mode can be
zero-initialized. We will make a CL to Blink to explicitly set
wgpu::VertexStepMode::VertexBufferNotUsed for empty slots and change
the enum value to 2.
Bug: dawn:1000
Change-Id: Ibd4ab87f2c922e8e460f2311547f13d58f1d5611
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/89340
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Takahiro <hogehoge@gachapin.jp>
diff --git a/dawn.json b/dawn.json
index 2b417fb..f7fe018 100644
--- a/dawn.json
+++ b/dawn.json
@@ -1479,8 +1479,9 @@
"vertex step mode": {
"category": "enum",
"values": [
- {"value": 0, "name": "vertex"},
- {"value": 1, "name": "instance"}
+ {"value": 0, "name": "vertex buffer not used"},
+ {"value": 1, "name": "vertex"},
+ {"value": 2, "name": "instance"}
]
},
"load op": {
diff --git a/src/dawn/native/RenderPipeline.cpp b/src/dawn/native/RenderPipeline.cpp
index 75f6420..3ec6bdc 100644
--- a/src/dawn/native/RenderPipeline.cpp
+++ b/src/dawn/native/RenderPipeline.cpp
@@ -498,7 +498,8 @@
mVertexBufferCount = descriptor->vertex.bufferCount;
const VertexBufferLayout* buffers = descriptor->vertex.buffers;
for (uint8_t slot = 0; slot < mVertexBufferCount; ++slot) {
- if (buffers[slot].attributeCount == 0) {
+ // Skip unused slots
+ if (buffers[slot].stepMode == wgpu::VertexStepMode::VertexBufferNotUsed) {
continue;
}
diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp
index 43fb89e..21edca9 100644
--- a/src/dawn/native/ShaderModule.cpp
+++ b/src/dawn/native/ShaderModule.cpp
@@ -112,8 +112,9 @@
return tint::transform::VertexStepMode::kVertex;
case wgpu::VertexStepMode::Instance:
return tint::transform::VertexStepMode::kInstance;
+ case wgpu::VertexStepMode::VertexBufferNotUsed:
+ UNREACHABLE();
}
- UNREACHABLE();
}
ResultOrError<SingleShaderStage> TintPipelineStageToShaderStage(tint::ast::PipelineStage stage) {
diff --git a/src/dawn/native/d3d12/RenderPipelineD3D12.cpp b/src/dawn/native/d3d12/RenderPipelineD3D12.cpp
index 9f513cf..8980b30 100644
--- a/src/dawn/native/d3d12/RenderPipelineD3D12.cpp
+++ b/src/dawn/native/d3d12/RenderPipelineD3D12.cpp
@@ -106,6 +106,8 @@
return D3D12_INPUT_CLASSIFICATION_PER_VERTEX_DATA;
case wgpu::VertexStepMode::Instance:
return D3D12_INPUT_CLASSIFICATION_PER_INSTANCE_DATA;
+ case wgpu::VertexStepMode::VertexBufferNotUsed:
+ UNREACHABLE();
}
}
diff --git a/src/dawn/native/metal/RenderPipelineMTL.mm b/src/dawn/native/metal/RenderPipelineMTL.mm
index 89548ae..6e5afd5 100644
--- a/src/dawn/native/metal/RenderPipelineMTL.mm
+++ b/src/dawn/native/metal/RenderPipelineMTL.mm
@@ -98,6 +98,8 @@
return MTLVertexStepFunctionPerVertex;
case wgpu::VertexStepMode::Instance:
return MTLVertexStepFunctionPerInstance;
+ case wgpu::VertexStepMode::VertexBufferNotUsed:
+ UNREACHABLE();
}
}
diff --git a/src/dawn/native/opengl/RenderPipelineGL.cpp b/src/dawn/native/opengl/RenderPipelineGL.cpp
index 6f93260..54b6ffb 100644
--- a/src/dawn/native/opengl/RenderPipelineGL.cpp
+++ b/src/dawn/native/opengl/RenderPipelineGL.cpp
@@ -276,6 +276,8 @@
case wgpu::VertexStepMode::Instance:
gl.VertexAttribDivisor(glAttrib, 1);
break;
+ case wgpu::VertexStepMode::VertexBufferNotUsed:
+ UNREACHABLE();
}
}
}
diff --git a/src/dawn/native/vulkan/RenderPipelineVk.cpp b/src/dawn/native/vulkan/RenderPipelineVk.cpp
index 8a808a1..47c3c85 100644
--- a/src/dawn/native/vulkan/RenderPipelineVk.cpp
+++ b/src/dawn/native/vulkan/RenderPipelineVk.cpp
@@ -39,8 +39,9 @@
return VK_VERTEX_INPUT_RATE_VERTEX;
case wgpu::VertexStepMode::Instance:
return VK_VERTEX_INPUT_RATE_INSTANCE;
+ case wgpu::VertexStepMode::VertexBufferNotUsed:
+ UNREACHABLE();
}
- UNREACHABLE();
}
VkFormat VulkanVertexFormat(wgpu::VertexFormat format) {
diff --git a/src/dawn/tests/end2end/VertexStateTests.cpp b/src/dawn/tests/end2end/VertexStateTests.cpp
index 2089cd6..aea465d 100644
--- a/src/dawn/tests/end2end/VertexStateTests.cpp
+++ b/src/dawn/tests/end2end/VertexStateTests.cpp
@@ -550,6 +550,10 @@
vertexState.cAttributes[0].offset = 0;
vertexState.cAttributes[0].format = VertexFormat::Float32x4;
+ for (uint32_t i = 0; i < kBufferIndex; i++) {
+ vertexState.cVertexBuffers[i].stepMode = VertexStepMode::VertexBufferNotUsed;
+ }
+
wgpu::RenderPipeline pipeline =
MakeTestPipeline(vertexState, 1, {{0, VertexFormat::Float32x4, VertexStepMode::Vertex}});
diff --git a/src/dawn/tests/unittests/validation/DrawVertexAndIndexBufferOOBValidationTests.cpp b/src/dawn/tests/unittests/validation/DrawVertexAndIndexBufferOOBValidationTests.cpp
index fa66bba..4b91c27 100644
--- a/src/dawn/tests/unittests/validation/DrawVertexAndIndexBufferOOBValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/DrawVertexAndIndexBufferOOBValidationTests.cpp
@@ -440,6 +440,50 @@
}
}
+// Verify zero-attribute vertex buffer OOB for non-instanced Draw are caught in command encoder
+TEST_F(DrawVertexAndIndexBufferOOBValidationTests, ZeroAttribute) {
+ // Create a render pipeline with zero-attribute vertex buffer description
+ wgpu::RenderPipeline pipeline =
+ CreateRenderPipelineWithBufferDesc({{kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}}});
+
+ wgpu::Buffer vertexBuffer = CreateBuffer(3 * kFloat32x4Stride);
+
+ {
+ // Valid
+ VertexBufferList vertexBufferList = {{0, vertexBuffer, 0, wgpu::kWholeSize}};
+ TestRenderPassDraw(pipeline, vertexBufferList, 3, 1, 0, 0, true);
+ }
+
+ {
+ // OOB
+ VertexBufferList vertexBufferList = {{0, vertexBuffer, 0, 0}};
+ TestRenderPassDraw(pipeline, vertexBufferList, 3, 1, 0, 0, false);
+ }
+}
+
+// Verify vertex buffers don't need to be set to unused (hole) slots for Draw
+TEST_F(DrawVertexAndIndexBufferOOBValidationTests, UnusedSlots) {
+ // Set vertex buffer only to the second slot
+ wgpu::Buffer vertexBuffer = CreateBuffer(3 * kFloat32x4Stride);
+ VertexBufferList vertexBufferList = {{1, vertexBuffer, 0, wgpu::kWholeSize}};
+
+ {
+ // The first slot it unused so valid even if vertex buffer is not set to it.
+ wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+ {{0, wgpu::VertexStepMode::VertexBufferNotUsed, {}},
+ {kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}}});
+ TestRenderPassDraw(pipeline, vertexBufferList, 3, 1, 0, 0, true);
+ }
+
+ {
+ // The first slot it used so invalid if vertex buffer is not set to it.
+ wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+ {{0, wgpu::VertexStepMode::Vertex, {}},
+ {kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}}});
+ TestRenderPassDraw(pipeline, vertexBufferList, 3, 1, 0, 0, false);
+ }
+}
+
// Control case for DrawIndexed
TEST_F(DrawVertexAndIndexBufferOOBValidationTests, DrawIndexedBasic) {
wgpu::RenderPipeline pipeline = CreateBasicRenderPipeline();
@@ -556,6 +600,75 @@
}
}
+// Verify zero-attribute instance step mode vertex buffer OOB for instanced DrawIndex
+// are caught in command encoder
+TEST_F(DrawVertexAndIndexBufferOOBValidationTests, DrawIndexedZeroAttribute) {
+ // Create a render pipeline with a vertex step mode and a zero-attribute
+ // instance step mode vertex buffer description
+ wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+ {{kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {{0, wgpu::VertexFormat::Float32x4}}},
+ {kFloat32x2Stride, wgpu::VertexStepMode::Instance, {}}});
+
+ // Build index buffer for 12 indexes
+ wgpu::Buffer indexBuffer = CreateBuffer(12 * sizeof(uint32_t), wgpu::BufferUsage::Index);
+ IndexBufferDesc indexBufferDesc = {indexBuffer, wgpu::IndexFormat::Uint32};
+
+ // Build vertex buffer for 3 vertices
+ wgpu::Buffer vertexBuffer = CreateBuffer(3 * kFloat32x4Stride);
+
+ // Build vertex buffer for 3 instances
+ wgpu::Buffer instanceBuffer = CreateBuffer(3 * kFloat32x2Stride);
+
+ {
+ // Valid
+ VertexBufferList vertexBufferList = {{0, vertexBuffer, 0, wgpu::kWholeSize},
+ {1, vertexBuffer, 0, wgpu::kWholeSize}};
+ TestRenderPassDrawIndexed(pipeline, indexBufferDesc, vertexBufferList, 12, 3, 0, 0, 0,
+ true);
+ }
+
+ {
+ // OOB
+ VertexBufferList vertexBufferList = {{0, vertexBuffer, 0, wgpu::kWholeSize},
+ {1, vertexBuffer, 0, 0}};
+ TestRenderPassDrawIndexed(pipeline, indexBufferDesc, vertexBufferList, 12, 3, 0, 0, 0,
+ false);
+ }
+}
+
+// Verify vertex buffers don't need to be set to unused (hole) slots for DrawIndexed
+TEST_F(DrawVertexAndIndexBufferOOBValidationTests, DrawIndexedUnusedSlots) {
+ // Build index buffer
+ wgpu::Buffer indexBuffer = CreateBuffer(12 * sizeof(uint32_t), wgpu::BufferUsage::Index);
+ IndexBufferDesc indexBufferDesc = {indexBuffer, wgpu::IndexFormat::Uint32};
+
+ // Set vertex buffers only to the second and third slots
+ wgpu::Buffer vertexBuffer = CreateBuffer(3 * kFloat32x4Stride);
+ wgpu::Buffer instanceBuffer = CreateBuffer(3 * kFloat32x2Stride);
+ VertexBufferList vertexBufferList = {{1, vertexBuffer, 0, wgpu::kWholeSize},
+ {2, instanceBuffer, 0, wgpu::kWholeSize}};
+
+ {
+ // The first slot it unused so valid even if vertex buffer is not set to it.
+ wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+ {{0, wgpu::VertexStepMode::VertexBufferNotUsed, {}},
+ {kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}},
+ {kFloat32x2Stride, wgpu::VertexStepMode::Instance, {}}});
+ TestRenderPassDrawIndexed(pipeline, indexBufferDesc, vertexBufferList, 12, 3, 0, 0, 0,
+ true);
+ }
+
+ {
+ // The first slot it used so invalid if vertex buffer is not set to it.
+ wgpu::RenderPipeline pipeline = CreateRenderPipelineWithBufferDesc(
+ {{0, wgpu::VertexStepMode::Vertex, {}},
+ {kFloat32x4Stride, wgpu::VertexStepMode::Vertex, {}},
+ {kFloat32x2Stride, wgpu::VertexStepMode::Instance, {}}});
+ TestRenderPassDrawIndexed(pipeline, indexBufferDesc, vertexBufferList, 12, 3, 0, 0, 0,
+ false);
+ }
+}
+
// Verify zero array stride vertex buffer OOB for Draw and DrawIndexed are caught in command encoder
// This test only test cases that strideCount > 0. Cases of strideCount == 0 are tested in
// ZeroStrideCountVertexBufferNeverOOB.