Validate stripIndexFormat at draw time.
Updates validation logic to match the recent changes in
https://github.com/gpuweb/gpuweb/pull/2385 that allows stripIndexFormat
to be undefined at pipeline creation time, even for strip topologies.
Non indexed draw calls are valid with such pipelines. Indexed draw calls
fail validation at draw time.
Bug: dawn:1224
Change-Id: I28ff78eac726d46f99a099ffb2338b5da81a4a88
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/72000
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Brandon Jones <bajones@chromium.org>
diff --git a/src/dawn_native/CommandBufferStateTracker.cpp b/src/dawn_native/CommandBufferStateTracker.cpp
index 9d64934..17a9b61 100644
--- a/src/dawn_native/CommandBufferStateTracker.cpp
+++ b/src/dawn_native/CommandBufferStateTracker.cpp
@@ -245,11 +245,20 @@
RenderPipelineBase* lastRenderPipeline = GetRenderPipeline();
wgpu::IndexFormat pipelineIndexFormat = lastRenderPipeline->GetStripIndexFormat();
- DAWN_INVALID_IF(
- IsStripPrimitiveTopology(lastRenderPipeline->GetPrimitiveTopology()) &&
+
+ if (IsStripPrimitiveTopology(lastRenderPipeline->GetPrimitiveTopology())) {
+ DAWN_INVALID_IF(
+ pipelineIndexFormat == wgpu::IndexFormat::Undefined,
+ "%s has a strip primitive topology (%s) but a strip index format of %s, which "
+ "prevents it for being used for indexed draw calls.",
+ lastRenderPipeline, lastRenderPipeline->GetPrimitiveTopology(),
+ pipelineIndexFormat);
+
+ DAWN_INVALID_IF(
mIndexFormat != pipelineIndexFormat,
- "Strip index format (%s) of %s does not match index buffer format (%s).",
- pipelineIndexFormat, lastRenderPipeline, mIndexFormat);
+ "Strip index format (%s) of %s does not match index buffer format (%s).",
+ pipelineIndexFormat, lastRenderPipeline, mIndexFormat);
+ }
// The chunk of code above should be similar to the one in |RecomputeLazyAspects|.
// It returns the first invalid state found. We shouldn't be able to reach this line
diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp
index 7b0884d..fb9ad467 100644
--- a/src/dawn_native/RenderPipeline.cpp
+++ b/src/dawn_native/RenderPipeline.cpp
@@ -248,14 +248,9 @@
DAWN_TRY(ValidateFrontFace(descriptor->frontFace));
DAWN_TRY(ValidateCullMode(descriptor->cullMode));
- // Pipeline descriptors must have stripIndexFormat != undefined IFF they are using strip
- // topologies.
- if (IsStripPrimitiveTopology(descriptor->topology)) {
- DAWN_INVALID_IF(
- descriptor->stripIndexFormat == wgpu::IndexFormat::Undefined,
- "StripIndexFormat is undefined when using a strip primitive topology (%s).",
- descriptor->topology);
- } else {
+ // Pipeline descriptors must have stripIndexFormat == undefined if they are using
+ // non-strip topologies.
+ if (!IsStripPrimitiveTopology(descriptor->topology)) {
DAWN_INVALID_IF(
descriptor->stripIndexFormat != wgpu::IndexFormat::Undefined,
"StripIndexFormat (%s) is not undefined when using a non-strip primitive "
diff --git a/src/tests/unittests/validation/IndexBufferValidationTests.cpp b/src/tests/unittests/validation/IndexBufferValidationTests.cpp
index d1d62b3..7c05e96 100644
--- a/src/tests/unittests/validation/IndexBufferValidationTests.cpp
+++ b/src/tests/unittests/validation/IndexBufferValidationTests.cpp
@@ -155,6 +155,8 @@
wgpu::PrimitiveTopology::TriangleStrip);
wgpu::RenderPipeline pipeline16 = MakeTestPipeline(wgpu::IndexFormat::Uint16,
wgpu::PrimitiveTopology::LineStrip);
+ wgpu::RenderPipeline pipelineUndef =
+ MakeTestPipeline(wgpu::IndexFormat::Undefined, wgpu::PrimitiveTopology::LineStrip);
wgpu::Buffer indexBuffer =
utils::CreateBufferFromData<uint32_t>(device, wgpu::BufferUsage::Index, {0, 1, 2});
@@ -196,6 +198,31 @@
encoder.DrawIndexed(3);
encoder.Finish();
}
+
+ // Expected to fail because pipeline doesn't specify an index format.
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ encoder.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint16);
+ encoder.SetPipeline(pipelineUndef);
+ encoder.DrawIndexed(3);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ encoder.SetIndexBuffer(indexBuffer, wgpu::IndexFormat::Uint32);
+ encoder.SetPipeline(pipelineUndef);
+ encoder.DrawIndexed(3);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+ }
+
+ // Expected to succeed because non-indexed draw calls don't require a pipeline index format.
+ {
+ wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&renderBundleDesc);
+ encoder.SetPipeline(pipelineUndef);
+ encoder.Draw(3);
+ encoder.Finish();
+ }
}
// Check that the index buffer must have the Index usage.
diff --git a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
index 2af96d2..009c187 100644
--- a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp
@@ -727,8 +727,8 @@
ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
}
-// Tests that strip primitive topologies require an index format
-TEST_F(RenderPipelineValidationTest, StripIndexFormatRequired) {
+// Tests that only strip primitive topologies allow an index format
+TEST_F(RenderPipelineValidationTest, StripIndexFormatAllowed) {
constexpr uint32_t kNumStripType = 2u;
constexpr uint32_t kNumListType = 3u;
constexpr uint32_t kNumIndexFormat = 3u;
@@ -751,14 +751,8 @@
descriptor.primitive.topology = primitiveTopology;
descriptor.primitive.stripIndexFormat = indexFormat;
- if (indexFormat == wgpu::IndexFormat::Undefined) {
- // Fail because the index format is undefined and the primitive
- // topology is a strip type.
- ASSERT_DEVICE_ERROR(device.CreateRenderPipeline(&descriptor));
- } else {
- // Succeeds because the index format is given.
- device.CreateRenderPipeline(&descriptor);
- }
+ // Always succeeds, regardless of if an index format is given.
+ device.CreateRenderPipeline(&descriptor);
}
}
