Refix validation for command encoder pass encoding
According to latest spec, beginRenderPass/beginComputePass
don't throw validation error. Error still defers to
CommandEncoder::Finish()
Bug: dawn:1602
Change-Id: I5fd76f2c8951273a8dd82b02e20f076079354b60
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/115120
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Shrek Shao <shrekshao@google.com>
diff --git a/src/dawn/native/CommandEncoder.cpp b/src/dawn/native/CommandEncoder.cpp
index 36603e7..0d9480e 100644
--- a/src/dawn/native/CommandEncoder.cpp
+++ b/src/dawn/native/CommandEncoder.cpp
@@ -796,14 +796,11 @@
Ref<ComputePassEncoder> CommandEncoder::BeginComputePass(const ComputePassDescriptor* descriptor) {
DeviceBase* device = GetDevice();
- // If descriptor is invalid, make pass invalid and stop immediately
- if (device->ConsumedError(ValidateComputePassDescriptor(device, descriptor))) {
- return ComputePassEncoder::MakeError(device, this, &mEncodingContext);
- }
-
bool success = mEncodingContext.TryEncode(
this,
[&](CommandAllocator* allocator) -> MaybeError {
+ DAWN_TRY(ValidateComputePassDescriptor(device, descriptor));
+
BeginComputePassCmd* cmd =
allocator->Allocate<BeginComputePassCmd>(Command::BeginComputePass);
@@ -867,18 +864,14 @@
bool stencilReadOnly = false;
Ref<AttachmentState> attachmentState;
- // If descriptor is invalid, make pass invalid and stop immediately
- if (device->ConsumedError(ValidateRenderPassDescriptor(device, descriptor, &width, &height,
- &sampleCount, mUsageValidationMode),
- "validating render pass descriptor.")) {
- return RenderPassEncoder::MakeError(device, this, &mEncodingContext);
- }
-
std::function<void()> passEndCallback = nullptr;
bool success = mEncodingContext.TryEncode(
this,
[&](CommandAllocator* allocator) -> MaybeError {
+ DAWN_TRY(ValidateRenderPassDescriptor(device, descriptor, &width, &height, &sampleCount,
+ mUsageValidationMode));
+
ASSERT(width > 0 && height > 0 && sampleCount > 0);
mEncodingContext.WillBeginRenderPass();
diff --git a/src/dawn/tests/end2end/SwapChainValidationTests.cpp b/src/dawn/tests/end2end/SwapChainValidationTests.cpp
index 20948ed..b02d760 100644
--- a/src/dawn/tests/end2end/SwapChainValidationTests.cpp
+++ b/src/dawn/tests/end2end/SwapChainValidationTests.cpp
@@ -78,16 +78,16 @@
void CheckTextureViewIsValid(wgpu::TextureView view) { CheckTextureView(view, false, false); }
private:
- void CheckTextureView(wgpu::TextureView view, bool errorAtBeginRenderPass, bool errorAtSubmit) {
+ void CheckTextureView(wgpu::TextureView view, bool errorAtFinish, bool errorAtSubmit) {
utils::ComboRenderPassDescriptor renderPassDesc({view});
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDesc);
+ pass.End();
- if (errorAtBeginRenderPass) {
- ASSERT_DEVICE_ERROR(encoder.BeginRenderPass(&renderPassDesc));
+ if (errorAtFinish) {
+ ASSERT_DEVICE_ERROR(encoder.Finish());
} else {
- wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPassDesc);
- pass.End();
wgpu::CommandBuffer commands = encoder.Finish();
if (errorAtSubmit) {
diff --git a/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp b/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp
index 152be1b..14ebe55 100644
--- a/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp
+++ b/src/dawn/tests/unittests/validation/DeprecatedAPITests.cpp
@@ -64,8 +64,9 @@
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- EXPECT_DEPRECATION_ERROR_OR_WARNING(
- pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ pass.End();
+ EXPECT_DEPRECATION_ERROR_ONLY(encoder.Finish(););
}
depthAttachment->depthLoadOp = wgpu::LoadOp::Undefined;
@@ -75,8 +76,9 @@
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- EXPECT_DEPRECATION_ERROR_OR_WARNING(
- pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ pass.End();
+ EXPECT_DEPRECATION_ERROR_ONLY(encoder.Finish(););
}
}
@@ -110,19 +112,22 @@
depthAttachment->clearStencil = 1;
- EXPECT_DEPRECATION_ERROR_OR_WARNING(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ pass.End();
depthAttachment->clearStencil = 0;
depthAttachment->depthClearValue = 0.0f;
depthAttachment->clearDepth = 1.0f;
- EXPECT_DEPRECATION_ERROR_OR_WARNING(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ pass.End();
renderPass.renderPassInfo.depthStencilAttachment = nullptr;
renderPass.renderPassInfo.cColorAttachments[0].clearColor = {1.0, 2.0, 3.0, 4.0};
renderPass.renderPassInfo.cColorAttachments[0].clearValue = {5.0, 4.0, 3.0, 2.0};
- EXPECT_DEPRECATION_ERROR_OR_WARNING(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ EXPECT_DEPRECATION_WARNING_ONLY(pass = encoder.BeginRenderPass(&renderPass.renderPassInfo));
+ pass.End();
}
// Test that endPass() is deprecated for both render and compute passes.
diff --git a/src/dawn/tests/unittests/validation/DeprecatedAPITests.h b/src/dawn/tests/unittests/validation/DeprecatedAPITests.h
index e00f217..dbbbe75 100644
--- a/src/dawn/tests/unittests/validation/DeprecatedAPITests.h
+++ b/src/dawn/tests/unittests/validation/DeprecatedAPITests.h
@@ -35,6 +35,24 @@
for (;;) \
break
+#define EXPECT_DEPRECATION_WARNING_ONLY(statement) \
+ if (!HasToggleEnabled(kDisallowDeprecatedAPIsToggleName)) { \
+ EXPECT_DEPRECATION_WARNING(statement); \
+ } else { \
+ statement; \
+ } \
+ for (;;) \
+ break
+
+#define EXPECT_DEPRECATION_ERROR_ONLY(statement) \
+ if (HasToggleEnabled(kDisallowDeprecatedAPIsToggleName)) { \
+ ASSERT_DEVICE_ERROR(statement); \
+ } else { \
+ statement; \
+ } \
+ for (;;) \
+ break
+
// Parameter is a single bool. When true, deprecated APIs are strictly disallowed (i.e. generate
// errors). Otherwise, deprecated APIs only generate a warning message.
class DeprecationTests : public ValidationTest, public testing::WithParamInterface<bool> {
diff --git a/src/dawn/tests/unittests/validation/QueryValidationTests.cpp b/src/dawn/tests/unittests/validation/QueryValidationTests.cpp
index 0712461..d0289e2 100644
--- a/src/dawn/tests/unittests/validation/QueryValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/QueryValidationTests.cpp
@@ -156,7 +156,9 @@
CreateQuerySet(otherDevice, wgpu::QueryType::Occlusion, 2);
renderPass.occlusionQuerySet = occlusionQuerySetOnOther;
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(encoder.BeginRenderPass(&renderPass));
+ wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+ pass.End();
+ ASSERT_DEVICE_ERROR(encoder.Finish());
// Clear this out so we don't hold a reference. The query set
// must be destroyed before the device local to this test case.
@@ -333,7 +335,8 @@
renderPass.occlusionQuerySet = querySet;
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(encoder.BeginRenderPass(&renderPass));
+ encoder.BeginRenderPass(&renderPass);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Test timestampWrites in compute pass descriptor
@@ -354,8 +357,9 @@
wgpu::QuerySet occlusionQuerySet = CreateQuerySet(device, wgpu::QueryType::Occlusion, 1);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeComputePassWithTimestampWrites(
- encoder, {{occlusionQuerySet, 0, wgpu::ComputePassTimestampLocation::Beginning}}));
+ EncodeComputePassWithTimestampWrites(
+ encoder, {{occlusionQuerySet, 0, wgpu::ComputePassTimestampLocation::Beginning}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps to a query set created from another device
@@ -365,16 +369,18 @@
CreateQuerySet(otherDevice, wgpu::QueryType::Timestamp, 2);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeComputePassWithTimestampWrites(
+ EncodeComputePassWithTimestampWrites(
encoder, {{querySet, 0, wgpu::ComputePassTimestampLocation::Beginning},
- {querySetFromOtherDevice, 1, wgpu::ComputePassTimestampLocation::End}}));
+ {querySetFromOtherDevice, 1, wgpu::ComputePassTimestampLocation::End}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps to the query index which exceeds the number of queries in query set
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeComputePassWithTimestampWrites(
- encoder, {{querySet, 2, wgpu::ComputePassTimestampLocation::Beginning}}));
+ EncodeComputePassWithTimestampWrites(
+ encoder, {{querySet, 2, wgpu::ComputePassTimestampLocation::Beginning}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Success to write timestamps to the same query index twice on same compute pass
@@ -399,16 +405,18 @@
// Fail to write timestamps at same location of a compute pass
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeComputePassWithTimestampWrites(
+ EncodeComputePassWithTimestampWrites(
encoder, {{querySet, 0, wgpu::ComputePassTimestampLocation::Beginning},
- {querySet, 1, wgpu::ComputePassTimestampLocation::Beginning}}));
+ {querySet, 1, wgpu::ComputePassTimestampLocation::Beginning}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps at invalid location of compute pass
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeComputePassWithTimestampWrites(
- encoder, {{querySet, 0, static_cast<wgpu::ComputePassTimestampLocation>(0xFFFFFFFF)}}));
+ EncodeComputePassWithTimestampWrites(
+ encoder, {{querySet, 0, static_cast<wgpu::ComputePassTimestampLocation>(0xFFFFFFFF)}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps to a destroyed query set
@@ -443,8 +451,9 @@
wgpu::QuerySet occlusionQuerySet = CreateQuerySet(device, wgpu::QueryType::Occlusion, 1);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeRenderPassWithTimestampWrites(
- encoder, {{occlusionQuerySet, 0, wgpu::RenderPassTimestampLocation::Beginning}}));
+ EncodeRenderPassWithTimestampWrites(
+ encoder, {{occlusionQuerySet, 0, wgpu::RenderPassTimestampLocation::Beginning}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps to a query set created from another device
@@ -454,16 +463,18 @@
CreateQuerySet(otherDevice, wgpu::QueryType::Timestamp, 2);
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeRenderPassWithTimestampWrites(
+ EncodeRenderPassWithTimestampWrites(
encoder, {{querySet, 0, wgpu::RenderPassTimestampLocation::Beginning},
- {querySetFromOtherDevice, 1, wgpu::RenderPassTimestampLocation::End}}));
+ {querySetFromOtherDevice, 1, wgpu::RenderPassTimestampLocation::End}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps to the query index which exceeds the number of queries in query set
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeRenderPassWithTimestampWrites(
- encoder, {{querySet, 2, wgpu::RenderPassTimestampLocation::Beginning}}));
+ EncodeRenderPassWithTimestampWrites(
+ encoder, {{querySet, 2, wgpu::RenderPassTimestampLocation::Beginning}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Success to write timestamps to the same query index and location twice on different render
@@ -483,24 +494,27 @@
// Fail to write timestamps to the same query index twice on same render pass
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeRenderPassWithTimestampWrites(
+ EncodeRenderPassWithTimestampWrites(
encoder, {{querySet, 0, wgpu::RenderPassTimestampLocation::Beginning},
- {querySet, 0, wgpu::RenderPassTimestampLocation::End}}));
+ {querySet, 0, wgpu::RenderPassTimestampLocation::End}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps at same location of a render pass
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeRenderPassWithTimestampWrites(
+ EncodeRenderPassWithTimestampWrites(
encoder, {{querySet, 0, wgpu::RenderPassTimestampLocation::Beginning},
- {querySet, 1, wgpu::RenderPassTimestampLocation::Beginning}}));
+ {querySet, 1, wgpu::RenderPassTimestampLocation::Beginning}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps at invalid location of render pass
{
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(EncodeRenderPassWithTimestampWrites(
- encoder, {{querySet, 0, static_cast<wgpu::RenderPassTimestampLocation>(0xFFFFFFFF)}}));
+ EncodeRenderPassWithTimestampWrites(
+ encoder, {{querySet, 0, static_cast<wgpu::RenderPassTimestampLocation>(0xFFFFFFFF)}});
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Fail to write timestamps to a destroyed query set
@@ -771,7 +785,8 @@
renderPass.occlusionQuerySet = querySet;
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(encoder.BeginRenderPass(&renderPass));
+ encoder.BeginRenderPass(&renderPass);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
class ResolveQuerySetValidationTest : public QuerySetValidationTest {
diff --git a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
index ef401da..57da7ce 100644
--- a/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp
@@ -29,14 +29,20 @@
class RenderPassDescriptorValidationTest : public ValidationTest {
public:
void AssertBeginRenderPassSuccess(const wgpu::RenderPassDescriptor* descriptor) {
- wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
- wgpu::RenderPassEncoder renderPassEncoder = commandEncoder.BeginRenderPass(descriptor);
- renderPassEncoder.End();
+ wgpu::CommandEncoder commandEncoder = TestBeginRenderPass(descriptor);
commandEncoder.Finish();
}
void AssertBeginRenderPassError(const wgpu::RenderPassDescriptor* descriptor) {
+ wgpu::CommandEncoder commandEncoder = TestBeginRenderPass(descriptor);
+ ASSERT_DEVICE_ERROR(commandEncoder.Finish());
+ }
+
+ private:
+ wgpu::CommandEncoder TestBeginRenderPass(const wgpu::RenderPassDescriptor* descriptor) {
wgpu::CommandEncoder commandEncoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(commandEncoder.BeginRenderPass(descriptor));
+ wgpu::RenderPassEncoder renderPassEncoder = commandEncoder.BeginRenderPass(descriptor);
+ renderPassEncoder.End();
+ return commandEncoder;
}
};
@@ -1464,7 +1470,7 @@
renderPassEncoder.End();
commandEncoder.Finish();
} else {
- EXPECT_DEPRECATION_ERROR_OR_WARNING(commandEncoder.BeginRenderPass(&descriptor));
+ EXPECT_DEPRECATION_WARNING_ONLY(commandEncoder.BeginRenderPass(&descriptor));
}
}
}
diff --git a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
index 66fe44a..9e081af 100644
--- a/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/RenderPipelineValidationTests.cpp
@@ -754,7 +754,11 @@
utils::ComboRenderPassDescriptor renderPassDescriptor;
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
- ASSERT_DEVICE_ERROR(encoder.BeginRenderPass(&renderPassDescriptor));
+ wgpu::RenderPassEncoder renderPass = encoder.BeginRenderPass(&renderPassDescriptor);
+ renderPass.SetPipeline(vertexOnlyPipeline);
+ renderPass.End();
+
+ ASSERT_DEVICE_ERROR(encoder.Finish());
}
// Vertex-only render pipeline can not work with color target
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index ca341aa..0ebab32 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -443,20 +443,6 @@
################################################################################
################################################################################
-# commandEncoder validation tests need updates in CTS
-# KEEP
-################################################################################
-crbug.com/dawn/1602 webgpu:api,validation,encoding,beginComputePass:* [ Skip ]
-crbug.com/dawn/1602 webgpu:api,validation,encoding,beginRenderPass:* [ Skip ]
-crbug.com/dawn/1602 webgpu:api,validation,encoding,encoder_open_state:non_pass_commands:* [ Skip ]
-crbug.com/dawn/1602 webgpu:api,validation,encoding,queries,general:occlusion_query,* [ Skip ]
-crbug.com/dawn/1602 webgpu:api,validation,render_pass,render_pass_descriptor:* [ Skip ]
-crbug.com/dawn/1602 webgpu:api,validation,render_pass,resolve:resolve_attachment:* [ Skip ]
-crbug.com/dawn/1602 webgpu:api,validation,render_pass,storeOp:* [ Skip ]
-crbug.com/dawn/1602 webgpu:api,validation,resource_usages,texture,in_render_misc:subresources,set_bind_group_on_same_index_depth_stencil_texture:* [ Skip ]
-################################################################################
-
-################################################################################
# Buffer.MapAsync pending map error tests need updates in CTS
################################################################################
crbug.com/dawn/1613 webgpu:api,validation,buffer,mapping:getMappedRange,state,mappingPending:* [ Failure ]
