CommandEncoder: prevent calls even after a failed finish
Finish validation acquires the command allocator but didn't set the
finished state so further top-level would still try to record in the
allocator, causing an ASSERT to fire.
BUG=chromium:939969
Change-Id: I334878098e6b824c2c4cef4fccb75472d3b63bbe
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6041
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
diff --git a/src/dawn_native/CommandEncoder.cpp b/src/dawn_native/CommandEncoder.cpp
index 1ac0790..21b9992 100644
--- a/src/dawn_native/CommandEncoder.cpp
+++ b/src/dawn_native/CommandEncoder.cpp
@@ -808,6 +808,9 @@
CommandBufferBase* CommandEncoderBase::Finish() {
if (GetDevice()->ConsumedError(ValidateFinish())) {
+ // Even if finish validation fails, it is now invalid to call any encoding commands on
+ // this object, so we set its state to finished.
+ mEncodingState = EncodingState::Finished;
return CommandBufferBase::MakeError(GetDevice());
}
ASSERT(!IsError());
@@ -833,6 +836,11 @@
}
void CommandEncoderBase::PassEnded() {
+ // This function may still be called when the command encoder is finished, just do nothing.
+ if (mEncodingState == EncodingState::Finished) {
+ return;
+ }
+
if (mEncodingState == EncodingState::ComputePass) {
mAllocator.Allocate<EndComputePassCmd>(Command::EndComputePass);
} else {
diff --git a/src/tests/unittests/validation/CommandBufferValidationTests.cpp b/src/tests/unittests/validation/CommandBufferValidationTests.cpp
index da2a246..3353390 100644
--- a/src/tests/unittests/validation/CommandBufferValidationTests.cpp
+++ b/src/tests/unittests/validation/CommandBufferValidationTests.cpp
@@ -155,6 +155,22 @@
}
}
+// Test that beginning a compute pass before ending the previous pass causes an error.
+TEST_F(CommandBufferValidationTest, CallsAfterAFailedFinish) {
+ // A buffer that can't be used in CopyBufferToBuffer
+ dawn::BufferDescriptor bufferDesc;
+ bufferDesc.size = 16;
+ bufferDesc.usage = dawn::BufferUsageBit::Uniform;
+ dawn::Buffer buffer = device.CreateBuffer(&bufferDesc);
+
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+ encoder.CopyBufferToBuffer(buffer, 0, buffer, 0, 0);
+ ASSERT_DEVICE_ERROR(encoder.Finish());
+
+ // TODO(cwallez@chromium.org): Currently this does nothing, should it be a device error?
+ encoder.CopyBufferToBuffer(buffer, 0, buffer, 0, 0);
+}
+
// Test that using a single buffer in multiple read usages in the same pass is allowed.
TEST_F(CommandBufferValidationTest, BufferWithMultipleReadUsage) {
// Create a buffer used as both vertex and index buffer.
