GraphicsCommandList validation (#3204)

* GraphicsCommandList validation * Add change to CHANGELOG.md * Close command list * Destroy command buffer * Find valid command list for reuse * Fix clippy
gfx-rs · Jan 4, 2023 · 71f5040 · 71f5040
1 parent 186a29c
commit 71f5040
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -202,6 +202,7 @@ Additionally `Surface::get_default_config` now returns an Option and returns Non
 #### DX12
 
 - Fix `depth16Unorm` formats by @teoxoy in [#3313](https://github.com/gfx-rs/wgpu/pull/3313)
+- Don't re-use `GraphicsCommandList` when `close` or `reset` fails. By @xiaopengli89 in [#3204](https://github.com/gfx-rs/wgpu/pull/3204)
 
 ### Examples
 

diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs
@@ -228,20 +228,34 @@ impl super::CommandEncoder {
 
 impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
     unsafe fn begin_encoding(&mut self, label: crate::Label) -> Result<(), crate::DeviceError> {
-        let list = match self.free_lists.pop() {
-            Some(list) => {
-                list.reset(self.allocator, native::PipelineState::null());
-                list
+        let list = loop {
+            if let Some(list) = self.free_lists.pop() {
+                let reset_result = list
+                    .reset(self.allocator, native::PipelineState::null())
+                    .into_result();
+                if reset_result.is_ok() {
+                    break Some(list);
+                } else {
+                    unsafe {
+                        list.destroy();
+                    }
+                }
+            } else {
+                break None;
             }
-            None => self
-                .device
+        };
+
+        let list = if let Some(list) = list {
+            list
+        } else {
+            self.device
                 .create_graphics_command_list(
                     native::CmdListType::Direct,
                     self.allocator,
                     native::PipelineState::null(),
                     0,
                 )
-                .into_device_result("Create command list")?,
+                .into_device_result("Create command list")?
         };
 
         if let Some(label) = label {
@@ -256,18 +270,29 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
     }
     unsafe fn discard_encoding(&mut self) {
         if let Some(list) = self.list.take() {
-            list.close();
-            self.free_lists.push(list);
+            if list.close().into_result().is_ok() {
+                self.free_lists.push(list);
+            } else {
+                unsafe {
+                    list.destroy();
+                }
+            }
         }
     }
     unsafe fn end_encoding(&mut self) -> Result<super::CommandBuffer, crate::DeviceError> {
         let raw = self.list.take().unwrap();
-        raw.close();
-        Ok(super::CommandBuffer { raw })
+        let closed = raw.close().into_result().is_ok();
+        Ok(super::CommandBuffer { raw, closed })
     }
     unsafe fn reset_all<I: Iterator<Item = super::CommandBuffer>>(&mut self, command_buffers: I) {
         for cmd_buf in command_buffers {
-            self.free_lists.push(cmd_buf.raw);
+            if cmd_buf.closed {
+                self.free_lists.push(cmd_buf.raw);
+            } else {
+                unsafe {
+                    cmd_buf.raw.destroy();
+                }
+            }
         }
         self.allocator.reset();
     }

diff --git a/wgpu-hal/src/dx12/mod.rs b/wgpu-hal/src/dx12/mod.rs
@@ -367,6 +367,7 @@ impl fmt::Debug for CommandEncoder {
 #[derive(Debug)]
 pub struct CommandBuffer {
     raw: native::GraphicsCommandList,
+    closed: bool,
 }
 
 unsafe impl Send for CommandBuffer {}