Report error immediately if pass ends twice
The previous code reports error till command encoder finish.
This change also updates webgpu-cts expectations.txt accordingly.
Bug: dawn:1578
Change-Id: I1fe758a02d9cfdb1440e3cabf10aea0bd60b9c59
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/116842
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Yunchao He <yunchao.he@intel.com>
diff --git a/src/dawn/native/ComputePassEncoder.cpp b/src/dawn/native/ComputePassEncoder.cpp
index 0480753..d3c6a5c 100644
--- a/src/dawn/native/ComputePassEncoder.cpp
+++ b/src/dawn/native/ComputePassEncoder.cpp
@@ -150,6 +150,13 @@
}
void ComputePassEncoder::APIEnd() {
+ if (mEnded && IsValidationEnabled()) {
+ GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("%s was already ended.", this));
+ return;
+ }
+
+ mEnded = true;
+
if (mEncodingContext->TryEncode(
this,
[&](CommandAllocator* allocator) -> MaybeError {
diff --git a/src/dawn/native/EncodingContext.h b/src/dawn/native/EncodingContext.h
index 3614a09..4a44eec 100644
--- a/src/dawn/native/EncodingContext.h
+++ b/src/dawn/native/EncodingContext.h
@@ -86,11 +86,11 @@
if (DAWN_UNLIKELY(encoder != mCurrentEncoder)) {
if (mCurrentEncoder != mTopLevelEncoder) {
// The top level encoder was used when a pass encoder was current.
- HandleError(DAWN_VALIDATION_ERROR("Command cannot be recorded while %s is active.",
- mCurrentEncoder));
+ HandleError(DAWN_VALIDATION_ERROR(
+ "Command cannot be recorded while %s is locked and %s is currently open.",
+ mTopLevelEncoder, mCurrentEncoder));
} else {
- HandleError(
- DAWN_VALIDATION_ERROR("Recording in an error or already ended %s.", encoder));
+ HandleError(DAWN_VALIDATION_ERROR("Recording in an error %s.", encoder));
}
return false;
}
diff --git a/src/dawn/native/ProgrammableEncoder.h b/src/dawn/native/ProgrammableEncoder.h
index 0aa53d1..25b730f 100644
--- a/src/dawn/native/ProgrammableEncoder.h
+++ b/src/dawn/native/ProgrammableEncoder.h
@@ -59,6 +59,8 @@
uint64_t mDebugGroupStackSize = 0;
+ bool mEnded = false;
+
private:
const bool mValidationEnabled;
};
diff --git a/src/dawn/native/RenderPassEncoder.cpp b/src/dawn/native/RenderPassEncoder.cpp
index cc3de8d..3b5a062 100644
--- a/src/dawn/native/RenderPassEncoder.cpp
+++ b/src/dawn/native/RenderPassEncoder.cpp
@@ -136,6 +136,13 @@
}
void RenderPassEncoder::APIEnd() {
+ if (mEnded && IsValidationEnabled()) {
+ GetDevice()->ConsumedError(DAWN_VALIDATION_ERROR("%s was already ended.", this));
+ return;
+ }
+
+ mEnded = true;
+
mEncodingContext->TryEncode(
this,
[&](CommandAllocator* allocator) -> MaybeError {
diff --git a/src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp b/src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp
index dc45bc0..cc8ca34 100644
--- a/src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp
@@ -58,8 +58,7 @@
ASSERT_DEVICE_ERROR(
encoder.Finish(),
HasSubstr("Command buffer recording ended before [RenderPassEncoder] was ended."));
- ASSERT_DEVICE_ERROR(
- pass.End(), HasSubstr("Recording in an error or already ended [RenderPassEncoder]."));
+ ASSERT_DEVICE_ERROR(pass.End(), HasSubstr("Recording in an error [RenderPassEncoder]."));
}
}
@@ -90,8 +89,7 @@
ASSERT_DEVICE_ERROR(
encoder.Finish(),
HasSubstr("Command buffer recording ended before [ComputePassEncoder] was ended."));
- ASSERT_DEVICE_ERROR(
- pass.End(), HasSubstr("Recording in an error or already ended [ComputePassEncoder]."));
+ ASSERT_DEVICE_ERROR(pass.End(), HasSubstr("Recording in an error [ComputePassEncoder]."));
}
}
@@ -112,10 +110,8 @@
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&placeholderRenderPass);
pass.End();
- pass.End();
- ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Recording in an error or already ended [RenderPassEncoder]."));
+ ASSERT_DEVICE_ERROR(pass.End(), HasSubstr("[RenderPassEncoder] was already ended."));
+ encoder.Finish();
}
}
@@ -134,10 +130,35 @@
wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
pass.End();
- pass.End();
- ASSERT_DEVICE_ERROR(
- encoder.Finish(),
- HasSubstr("Recording in an error or already ended [ComputePassEncoder]."));
+ ASSERT_DEVICE_ERROR(pass.End(), HasSubstr("[ComputePassEncoder] was already ended."));
+ encoder.Finish();
+ }
+}
+
+// Test that a pass cannot be ended again in another pass
+TEST_F(CommandBufferValidationTest, PassEndedAgainMidAnotherPass) {
+ // Error case, render pass ended again in another render pass
+ {
+ PlaceholderRenderPass placeholderRenderPass(device);
+
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::RenderPassEncoder pass0 = encoder.BeginRenderPass(&placeholderRenderPass);
+ pass0.End();
+ wgpu::RenderPassEncoder pass1 = encoder.BeginRenderPass(&placeholderRenderPass);
+ ASSERT_DEVICE_ERROR(pass0.End(), HasSubstr("[RenderPassEncoder] was already ended."));
+ pass1.End();
+ encoder.Finish();
+ }
+
+ // Error case, compute pass ended again in another compute pass
+ {
+ wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+ wgpu::ComputePassEncoder pass0 = encoder.BeginComputePass();
+ pass0.End();
+ wgpu::ComputePassEncoder pass1 = encoder.BeginComputePass();
+ ASSERT_DEVICE_ERROR(pass0.End(), HasSubstr("[ComputePassEncoder] was already ended."));
+ pass1.End();
+ encoder.Finish();
}
}
diff --git a/webgpu-cts/expectations.txt b/webgpu-cts/expectations.txt
index c383204..1f33c92 100644
--- a/webgpu-cts/expectations.txt
+++ b/webgpu-cts/expectations.txt
@@ -406,7 +406,7 @@
crbug.com/dawn/0000 webgpu:api,validation,buffer,mapping:mapAsync,state,mappingPending: [ Failure ]
crbug.com/dawn/0000 webgpu:api,validation,createBindGroupLayout:multisampled_validation:viewDimension="2d" [ Failure ]
crbug.com/dawn/0000 webgpu:api,validation,createBindGroupLayout:multisampled_validation:viewDimension="_undef_" [ Failure ]
-crbug.com/dawn/0000 webgpu:api,validation,encoding,encoder_state:pass_end_twice: [ Failure ]
+crbug.com/dawn/0000 webgpu:api,validation,encoding,encoder_state:pass_end_invalid_order:* [ Failure ]
crbug.com/dawn/0000 [ nvidia-0x2184 target-cpu-64 ubuntu ] webgpu:shader,execution,expression,binary,i32_arithmetic:remainder:inputSource="storage_r";vectorize="_undef_" [ Failure ]
crbug.com/dawn/0000 [ nvidia-0x2184 target-cpu-64 ubuntu ] webgpu:shader,execution,expression,binary,i32_arithmetic:remainder:inputSource="storage_r";vectorize=2 [ Failure ]
crbug.com/dawn/0000 [ nvidia-0x2184 target-cpu-64 ubuntu ] webgpu:shader,execution,expression,binary,i32_arithmetic:remainder:inputSource="storage_r";vectorize=3 [ Failure ]
