From 45ef1757cb1b43f76f6c87e346948635c51a7c1f Mon Sep 17 00:00:00 2001
From: Erich Gubler <erichdongubler@gmail.com>
Date: Tue, 13 Feb 2024 15:47:56 -0500
Subject: [PATCH] fix(dx12): discard cmd. enc. buf. on drop

---
 tests/tests/bind_group_layout_dedup.rs | 23 +++--------------------
 tests/tests/encoder.rs                 | 22 ++++++++++++++--------
 wgpu-hal/src/dx12/command.rs           |  7 +++++++
 wgpu-hal/src/dx12/device.rs            |  6 +-----
 4 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/tests/tests/bind_group_layout_dedup.rs b/tests/tests/bind_group_layout_dedup.rs
index 8da284b41b..a0f6dad25d 100644
--- a/tests/tests/bind_group_layout_dedup.rs
+++ b/tests/tests/bind_group_layout_dedup.rs
@@ -1,9 +1,6 @@
 use std::num::NonZeroU64;
 
-use wgpu_test::{
-    fail, gpu_test, FailureCase, GpuTestConfiguration, TestParameters, TestingContext,
-};
-use wgt::Backends;
+use wgpu_test::{fail, gpu_test, GpuTestConfiguration, TestParameters, TestingContext};
 
 const SHADER_SRC: &str = "
 @group(0) @binding(0)
@@ -307,18 +304,10 @@ fn bgl_dedupe_derived(ctx: TestingContext) {
     ctx.queue.submit(Some(encoder.finish()));
 }
 
-const DX12_VALIDATION_ERROR: &str = "The command allocator cannot be reset because a command list is currently being recorded with the allocator.";
-
 #[gpu_test]
 static SEPARATE_PROGRAMS_HAVE_INCOMPATIBLE_DERIVED_BGLS: GpuTestConfiguration =
     GpuTestConfiguration::new()
-        .parameters(
-            TestParameters::default()
-                .test_features_limits()
-                .expect_fail(
-                    FailureCase::backend(Backends::DX12).validation_error(DX12_VALIDATION_ERROR),
-                ),
-        )
+        .parameters(TestParameters::default().test_features_limits())
         .run_sync(separate_programs_have_incompatible_derived_bgls);
 
 fn separate_programs_have_incompatible_derived_bgls(ctx: TestingContext) {
@@ -376,13 +365,7 @@ fn separate_programs_have_incompatible_derived_bgls(ctx: TestingContext) {
 #[gpu_test]
 static DERIVED_BGLS_INCOMPATIBLE_WITH_REGULAR_BGLS: GpuTestConfiguration =
     GpuTestConfiguration::new()
-        .parameters(
-            TestParameters::default()
-                .test_features_limits()
-                .expect_fail(
-                    FailureCase::backend(Backends::DX12).validation_error(DX12_VALIDATION_ERROR),
-                ),
-        )
+        .parameters(TestParameters::default().test_features_limits())
         .run_sync(derived_bgls_incompatible_with_regular_bgls);
 
 fn derived_bgls_incompatible_with_regular_bgls(ctx: TestingContext) {
diff --git a/tests/tests/encoder.rs b/tests/tests/encoder.rs
index 3487eb05d1..3858e3d070 100644
--- a/tests/tests/encoder.rs
+++ b/tests/tests/encoder.rs
@@ -8,16 +8,22 @@ static DROP_ENCODER: GpuTestConfiguration = GpuTestConfiguration::new().run_sync
     drop(encoder);
 });
 
-// This test crashes on DX12 with the exception:
-//
-// ID3D12CommandAllocator::Reset: The command allocator cannot be reset because a
-// command list is currently being recorded with the allocator. [ EXECUTION ERROR
-// #543: COMMAND_ALLOCATOR_CANNOT_RESET]
-//
-// For now, we mark the test as failing on DX12.
+#[gpu_test]
+static DROP_QUEUE_BEFORE_CREATING_COMMAND_ENCODER: GpuTestConfiguration =
+    GpuTestConfiguration::new()
+        .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
+            let device = ctx.device.clone();
+            drop(ctx);
+            let _encoder =
+                device.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
+        });
+
 #[gpu_test]
 static DROP_ENCODER_AFTER_ERROR: GpuTestConfiguration = GpuTestConfiguration::new()
-    .parameters(TestParameters::default().expect_fail(FailureCase::backend(wgpu::Backends::DX12)))
+    .parameters(TestParameters::default())
     .run_sync(|ctx| {
         let mut encoder = ctx
             .device
diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs
index 3d05813ed7..f8f4484097 100644
--- a/wgpu-hal/src/dx12/command.rs
+++ b/wgpu-hal/src/dx12/command.rs
@@ -56,6 +56,13 @@ impl super::Temp {
     }
 }
 
+impl Drop for super::CommandEncoder {
+    fn drop(&mut self) {
+        use crate::CommandEncoder;
+        unsafe { self.discard_encoding() }
+    }
+}
+
 impl super::CommandEncoder {
     unsafe fn begin_pass(&mut self, kind: super::PassKind, label: crate::Label) {
         let list = self.list.as_ref().unwrap();
diff --git a/wgpu-hal/src/dx12/device.rs b/wgpu-hal/src/dx12/device.rs
index bb128b2a6d..f6281d2b8a 100644
--- a/wgpu-hal/src/dx12/device.rs
+++ b/wgpu-hal/src/dx12/device.rs
@@ -663,11 +663,7 @@ impl crate::Device<super::Api> for super::Device {
             end_of_pass_timer_query: None,
         })
     }
-    unsafe fn destroy_command_encoder(&self, encoder: super::CommandEncoder) {
-        if let Some(list) = encoder.list {
-            list.close();
-        }
-    }
+    unsafe fn destroy_command_encoder(&self, _encoder: super::CommandEncoder) {}
 
     unsafe fn create_bind_group_layout(
         &self,