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>

src/dawn/native/ComputePassEncoder.cpp

@@ -150,6 +150,13 @@ ObjectType ComputePassEncoder::GetType() const {
 }
 
 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 {

src/dawn/native/EncodingContext.h

@@ -86,11 +86,11 @@ class EncodingContext {
         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;
         }

src/dawn/native/ProgrammableEncoder.h

@@ -59,6 +59,8 @@ class ProgrammableEncoder : public ApiObjectBase {
 
     uint64_t mDebugGroupStackSize = 0;
 
+    bool mEnded = false;
+
   private:
     const bool mValidationEnabled;
 };

src/dawn/native/RenderPassEncoder.cpp

@@ -136,6 +136,13 @@ void RenderPassEncoder::TrackQueryAvailability(QuerySetBase* querySet, uint32_t
 }
 
 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 {

src/dawn/tests/unittests/validation/CommandBufferValidationTests.cpp

@@ -58,8 +58,7 @@ TEST_F(CommandBufferValidationTest, EndedMidRenderPass) {
         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 @@ TEST_F(CommandBufferValidationTest, EndedMidComputePass) {
         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 @@ TEST_F(CommandBufferValidationTest, RenderPassEndedTwice) {
         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 @@ TEST_F(CommandBufferValidationTest, ComputePassEndedTwice) {
         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();
     }
 }
 

webgpu-cts/expectations.txt

@@ -406,7 +406,7 @@ crbug.com/dawn/0000 webgpu:api,validation,buffer,mapping:mapAsync,earlyRejection
 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 ]