Update tests and changelog for deferred error reporting

CHANGELOG.md

@@ -139,6 +139,12 @@ Bottom level categories:
 #### WebGPU
 
 - The type of the `size` parameter to `copy_buffer_to_buffer` has changed from `BufferAddress` to `impl Into<Option<BufferAddress>>`. This achieves the spec-defined behavior of the value being optional, while still accepting existing calls without changes. By @andyleiserson in [#7659](https://github.com/gfx-rs/wgpu/pull/7659).
+- To bring wgpu's error reporting into compliance with the WebGPU specification, the error type returned from some functions has changed, and some errors may be raised at a different time than they were previously.
+  - The error type returned by many methods on `CommandEncoder`, `RenderPassEncoder`, `ComputePassEncoder`, and `RenderBundleEncoder` has changed to `EncoderStateError` or `PassStateError`. These functions will return the `Ended` variant of these errors if called on an encoder that is no longer active. Reporting of all other errors is deferred until a call to `finish()`.
+  - Variants holding a `CommandEncoderError` in the error enums `ClearError`, `ComputePassErrorInner`, `QueryError`, and `RenderPassErrorInner` have been replaced with variants holding an `EncoderStateError`.
+  - The definition of `enum CommandEncoderError` has changed significantly, to reflect which errors can be raised by `CommandEncoder.finish()`. There are also some errors that no longer appear directly in `CommandEncoderError`, and instead appear nested within the `RenderPass` or `ComputePass` variants.
+  - `CopyError` has been removed. Errors that were previously a `CopyError` are now a `CommandEncoderError` returned by `finish()`. (The detailed reasons for copies to fail were and still are described by `TransferError`, which was previously a variant of `CopyError`, and is now a variant of `CommandEncoderError`).
+
 
 #### HAL
 

cts_runner/test.lst

@@ -1,14 +1,37 @@
 unittests:*
 webgpu:api,operation,command_buffer,basic:*
-webgpu:api,operation,command_buffer,copyBufferToBuffer:single:newSig=false;*
-// https://github.com/gfx-rs/wgpu/issues/7391
-//FAIL: webgpu:api,operation,command_buffer,copyBufferToBuffer:single:newSig=true;*
-webgpu:api,operation,command_buffer,copyBufferToBuffer:state_transitions:*
-webgpu:api,operation,command_buffer,copyBufferToBuffer:copy_order:*
+webgpu:api,operation,command_buffer,copyBufferToBuffer:*
 webgpu:api,operation,compute,basic:memcpy:*
 //FAIL: webgpu:api,operation,compute,basic:large_dispatch:*
 webgpu:api,operation,compute_pipeline,overrides:*
 webgpu:api,operation,device,lost:*
+webgpu:api,validation,encoding,cmds,clearBuffer:out_of_bounds,*
+webgpu:api,validation,encoding,cmds,compute_pass:set_pipeline:*
+webgpu:api,validation,encoding,cmds,compute_pass:dispatch_sizes:*
+webgpu:api,validation,encoding,cmds,copyTextureToTexture:copy_with_invalid_or_destroyed_texture:*
+webgpu:api,validation,encoding,cmds,copyTextureToTexture:texture,device_mismatch:*
+webgpu:api,validation,encoding,cmds,copyTextureToTexture:mipmap_level:*
+webgpu:api,validation,encoding,cmds,copyTextureToTexture:texture_usage:*
+webgpu:api,validation,encoding,cmds,copyTextureToTexture:sample_count:*
+//FAIL: webgpu:api,validation,encoding,cmds,copyTextureToTexture:multisampled_copy_restrictions:*
+// texture_format_compatibility fails on Windows and Mac in CI.
+// It passes locally on Mac. It is also a relatively long test.
+//FAIL: webgpu:api,validation,encoding,cmds,copyTextureToTexture:texture_format_compatibility:*
+//FAIL: webgpu:api,validation,encoding,cmds,copyTextureToTexture:depth_stencil_copy_restrictions:*
+//FAIL: webgpu:api,validation,encoding,cmds,copyTextureToTexture:copy_ranges:*
+//FAIL: webgpu:api,validation,encoding,cmds,copyTextureToTexture:copy_within_same_texture:*
+//FAIL: webgpu:api,validation,encoding,cmds,copyTextureToTexture:copy_aspects:*
+// Fails on Windows, https://github.com/gfx-rs/wgpu/issues/7844.
+//FAIL: webgpu:api,validation,encoding,cmds,copyTextureToTexture:copy_ranges_with_compressed_texture_formats:*
+webgpu:api,validation,encoding,cmds,index_access:*
+//FAIL: webgpu:api,validation,encoding,cmds,render,draw:*
+webgpu:api,validation,encoding,encoder_state:*
+webgpu:api,validation,encoding,encoder_open_state:non_pass_commands:*
+webgpu:api,validation,encoding,encoder_open_state:render_pass_commands:*
+//FAIL: webgpu:api,validation,encoding,encoder_open_state:render_bundle_commands:*
+webgpu:api,validation,encoding,encoder_open_state:compute_pass_commands:*
+webgpu:api,validation,encoding,beginComputePass:*
+webgpu:api,validation,encoding,beginRenderPass:*
 webgpu:api,validation,queue,submit:command_buffer,device_mismatch:*
 webgpu:api,validation,queue,submit:command_buffer,duplicate_buffers:*
 webgpu:api,validation,queue,submit:command_buffer,submit_invalidates:*

tests/tests/wgpu-gpu/bind_group_layout_dedup.rs

@@ -316,11 +316,11 @@ fn separate_pipelines_have_incompatible_derived_bgls(ctx: TestingContext) {
     pass.set_bind_group(0, &bg2, &[]);
     pass.dispatch_workgroups(1, 1, 1);
 
+    drop(pass);
+
     fail(
         &ctx.device,
-        || {
-            drop(pass);
-        },
+        || encoder.finish(),
         Some("label at index 0 is not compatible with the corresponding bindgrouplayout"),
     );
 }
@@ -388,13 +388,13 @@ fn derived_bgls_incompatible_with_regular_bgls(ctx: TestingContext) {
     pass.set_bind_group(0, &bg, &[]);
     pass.dispatch_workgroups(1, 1, 1);
 
+    drop(pass);
+
     fail(
         &ctx.device,
-        || {
-            drop(pass);
-        },
+        || encoder.finish(),
         Some("label at index 0 is not compatible with the corresponding bindgrouplayout"),
-    )
+    );
 }
 
 #[gpu_test]

tests/tests/wgpu-gpu/device.rs

@@ -347,34 +347,34 @@ static DEVICE_DESTROY_THEN_MORE: GpuTestConfiguration = GpuTestConfiguration::ne
         );
 
         // Creating a compute pass should fail.
+        let pass = encoder_for_compute_pass.begin_compute_pass(&wgpu::ComputePassDescriptor {
+            label: None,
+            timestamp_writes: None,
+        });
+        drop(pass);
         fail(
             &ctx.device,
-            || {
-                encoder_for_compute_pass.begin_compute_pass(&wgpu::ComputePassDescriptor {
-                    label: None,
-                    timestamp_writes: None,
-                });
-            },
+            || encoder_for_compute_pass.finish(),
             Some("device with '' label is invalid"),
         );
 
         // Creating a render pass should fail.
+        let pass = encoder_for_render_pass.begin_render_pass(&wgpu::RenderPassDescriptor {
+            label: None,
+            color_attachments: &[Some(wgpu::RenderPassColorAttachment {
+                ops: wgpu::Operations::default(),
+                resolve_target: None,
+                view: &target_view,
+                depth_slice: None,
+            })],
+            depth_stencil_attachment: None,
+            timestamp_writes: None,
+            occlusion_query_set: None,
+        });
+        drop(pass);
         fail(
             &ctx.device,
-            || {
-                encoder_for_render_pass.begin_render_pass(&wgpu::RenderPassDescriptor {
-                    label: None,
-                    color_attachments: &[Some(wgpu::RenderPassColorAttachment {
-                        ops: wgpu::Operations::default(),
-                        resolve_target: None,
-                        view: &target_view,
-                        depth_slice: None,
-                    })],
-                    depth_stencil_attachment: None,
-                    timestamp_writes: None,
-                    occlusion_query_set: None,
-                });
-            },
+            || encoder_for_render_pass.finish(),
             Some("device with '' label is invalid"),
         );
 

tests/tests/wgpu-gpu/encoder.rs

@@ -18,7 +18,8 @@ static DROP_QUEUE_BEFORE_CREATING_COMMAND_ENCODER: GpuTestConfiguration =
         .parameters(TestParameters::default().expect_fail(FailureCase::always()))
         .run_sync(|ctx| {
             // Use the device after the queue is dropped. Currently this panics
-            // but it probably shouldn't
+            // but it probably shouldn't.
+            // TODO(https://github.com/gfx-rs/wgpu/issues/7781) revisit this
             let TestingContext { device, queue, .. } = ctx;
             drop(queue);
             let _encoder =
@@ -62,33 +63,20 @@ static DROP_ENCODER_AFTER_ERROR: GpuTestConfiguration = GpuTestConfiguration::ne
             occlusion_query_set: None,
         });
 
-        // Set a bad viewport on renderpass, triggering an error.
-        fail(
-            &ctx.device,
-            || {
-                renderpass.set_viewport(0.0, 0.0, -1.0, -1.0, 0.0, 1.0);
-                drop(renderpass);
-            },
-            Some("less than zero"),
-        );
+        // This viewport is invalid because it has negative size.
+        renderpass.set_viewport(0.0, 0.0, -1.0, -1.0, 0.0, 1.0);
+        drop(renderpass);
 
-        // This is the actual interesting error condition. We've created
-        // a CommandEncoder which errored out when processing a command.
-        // The encoder is still open!
-        drop(encoder);
+        fail(&ctx.device, || encoder.finish(), Some("less than zero"));
     });
 
 #[gpu_test]
 static ENCODER_OPERATIONS_FAIL_WHILE_PASS_ALIVE: GpuTestConfiguration = GpuTestConfiguration::new()
-    .parameters(
-        TestParameters::default()
-            .features(
-                wgpu::Features::CLEAR_TEXTURE
-                    | wgpu::Features::TIMESTAMP_QUERY
-                    | wgpu::Features::TIMESTAMP_QUERY_INSIDE_ENCODERS,
-            )
-            .expect_fail(FailureCase::always()), // temporary, until https://github.com/gfx-rs/wgpu/issues/7391 is completed
-    )
+    .parameters(TestParameters::default().features(
+        wgpu::Features::CLEAR_TEXTURE
+            | wgpu::Features::TIMESTAMP_QUERY
+            | wgpu::Features::TIMESTAMP_QUERY_INSIDE_ENCODERS,
+    ))
     .run_sync(encoder_operations_fail_while_pass_alive);
 
 fn encoder_operations_fail_while_pass_alive(ctx: TestingContext) {
@@ -303,6 +291,7 @@ fn encoder_operations_fail_while_pass_alive(ctx: TestingContext) {
 
     for &pass_type in [PassType::Compute, PassType::Render].iter() {
         for (op_name, op) in recording_ops.iter() {
+            // Test the case where the pass is not ended before calling finish()
             let mut encoder = ctx
                 .device
                 .create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
@@ -312,26 +301,25 @@ fn encoder_operations_fail_while_pass_alive(ctx: TestingContext) {
             ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
 
             log::info!("Testing operation {op_name:?} on a locked command encoder while a {pass_type:?} pass is active");
-            fail(&ctx.device, || op(&mut encoder), Some("encoder is locked"));
+            op(&mut encoder);
 
-            // Drop the pass - this also fails now since the encoder is invalid:
-            fail(&ctx.device, || drop(pass), Some("encoder is invalid"));
-            // Also, it's not possible to create a new pass on the encoder:
-            fail(
-                &ctx.device,
-                || encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default()),
-                Some("encoder is invalid"),
-            );
-        }
+            fail(&ctx.device, || encoder.finish(), Some("encoder is locked"));
 
-        // Test encoder finishing separately since it consumes the encoder and doesn't fit above pattern.
-        {
+            drop(pass);
+
+            // ...and the case where the pass is ended before calling finish()
             let mut encoder = ctx
                 .device
                 .create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
+
             let pass = create_pass(&mut encoder, pass_type);
+
+            log::info!("Testing operation {op_name:?} on a locked command encoder while a {pass_type:?} pass is active");
+            op(&mut encoder);
+
+            drop(pass);
+
             fail(&ctx.device, || encoder.finish(), Some("encoder is locked"));
-            fail(&ctx.device, || drop(pass), Some("encoder is invalid"));
         }
     }
 }

xtask/src/cts.rs

@@ -34,7 +34,7 @@ pub fn run_cts(shell: Shell, mut args: Arguments) -> anyhow::Result<()> {
         tests.extend(shell.read_file(file)?.lines().filter_map(|line| {
             let trimmed = line.trim();
             let is_comment = trimmed.starts_with("//") || trimmed.starts_with("#");
-            (!is_comment).then(|| OsString::from(trimmed))
+            (!trimmed.is_empty() && !is_comment).then(|| OsString::from(trimmed))
         }))
     }