From bf2c7705b9d89ab5afd5765de41148e1eb3ef95d Mon Sep 17 00:00:00 2001 From: xiaopengli89 Date: Sun, 13 Nov 2022 17:26:53 +0800 Subject: [PATCH 1/6] GraphicsCommandList validation --- wgpu-hal/src/dx12/command.rs | 32 +++++++++++++++++++++++++------- wgpu-hal/src/dx12/mod.rs | 1 + 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index 9f879e8b63..16a99d9eed 100644 --- a/wgpu-hal/src/dx12/command.rs +++ b/wgpu-hal/src/dx12/command.rs @@ -230,8 +230,23 @@ impl crate::CommandEncoder 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 + if list + .reset(self.allocator, native::PipelineState::null()) + .into_result() + .is_ok() + { + list + } else { + list.destroy(); + self.device + .create_graphics_command_list( + native::CmdListType::Direct, + self.allocator, + native::PipelineState::null(), + 0, + ) + .into_device_result("Create command list")? + } } None => self .device @@ -256,18 +271,21 @@ impl crate::CommandEncoder 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); + } } } unsafe fn end_encoding(&mut self) -> Result { 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>(&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); + } } self.allocator.reset(); } diff --git a/wgpu-hal/src/dx12/mod.rs b/wgpu-hal/src/dx12/mod.rs index b3be9e722c..162d0eb99a 100644 --- 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 {} From 67dcdcaa2710f3dc4f2c393b8c6c0f88521efbf1 Mon Sep 17 00:00:00 2001 From: xiaopengli89 Date: Sun, 13 Nov 2022 17:42:54 +0800 Subject: [PATCH 2/6] Add change to CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f80cfc5db4..9b83a53898 100644 --- 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 From 2af45a94db6f6763845c0d5bcdc12af72a8de449 Mon Sep 17 00:00:00 2001 From: xiaopengli89 Date: Sun, 13 Nov 2022 17:51:12 +0800 Subject: [PATCH 3/6] Close command list --- wgpu-hal/src/dx12/command.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index 16a99d9eed..fee9563ea6 100644 --- a/wgpu-hal/src/dx12/command.rs +++ b/wgpu-hal/src/dx12/command.rs @@ -273,6 +273,8 @@ impl crate::CommandEncoder for super::CommandEncoder { if let Some(list) = self.list.take() { if list.close().into_result().is_ok() { self.free_lists.push(list); + } else { + list.destroy(); } } } @@ -285,6 +287,8 @@ impl crate::CommandEncoder for super::CommandEncoder { for cmd_buf in command_buffers { if cmd_buf.closed { self.free_lists.push(cmd_buf.raw); + } else { + cmd_buf.raw.close(); } } self.allocator.reset(); From 0b3afa25e1d4afe26a0637c113c978672683c21a Mon Sep 17 00:00:00 2001 From: xiaopengli89 Date: Mon, 12 Dec 2022 20:56:11 +0800 Subject: [PATCH 4/6] Destroy command buffer --- wgpu-hal/src/dx12/command.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index fee9563ea6..484ee8c3ca 100644 --- a/wgpu-hal/src/dx12/command.rs +++ b/wgpu-hal/src/dx12/command.rs @@ -288,7 +288,7 @@ impl crate::CommandEncoder for super::CommandEncoder { if cmd_buf.closed { self.free_lists.push(cmd_buf.raw); } else { - cmd_buf.raw.close(); + cmd_buf.raw.destroy(); } } self.allocator.reset(); From af317455999a509e3e773aa5d07d31dbbda1a0fe Mon Sep 17 00:00:00 2001 From: xiaopengli89 Date: Tue, 13 Dec 2022 14:04:24 +0800 Subject: [PATCH 5/6] Find valid command list for reuse --- wgpu-hal/src/dx12/command.rs | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index 484ee8c3ca..ff75f0d611 100644 --- a/wgpu-hal/src/dx12/command.rs +++ b/wgpu-hal/src/dx12/command.rs @@ -228,35 +228,32 @@ impl super::CommandEncoder { impl crate::CommandEncoder for super::CommandEncoder { unsafe fn begin_encoding(&mut self, label: crate::Label) -> Result<(), crate::DeviceError> { - let list = match self.free_lists.pop() { - Some(list) => { - if list + let list = loop { + if let Some(list) = self.free_lists.pop() { + let reset_result = list .reset(self.allocator, native::PipelineState::null()) - .into_result() - .is_ok() - { - list + .into_result(); + if reset_result.is_ok() { + break Some(list); } else { list.destroy(); - self.device - .create_graphics_command_list( - native::CmdListType::Direct, - self.allocator, - native::PipelineState::null(), - 0, - ) - .into_device_result("Create command list")? } + } 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 { From 783ff3e1fbb31a62e82cd0b00653c217df573ce8 Mon Sep 17 00:00:00 2001 From: xiaopengli89 Date: Tue, 13 Dec 2022 14:25:27 +0800 Subject: [PATCH 6/6] Fix clippy --- wgpu-hal/src/dx12/command.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index ff75f0d611..d109531ace 100644 --- a/wgpu-hal/src/dx12/command.rs +++ b/wgpu-hal/src/dx12/command.rs @@ -236,7 +236,9 @@ impl crate::CommandEncoder for super::CommandEncoder { if reset_result.is_ok() { break Some(list); } else { - list.destroy(); + unsafe { + list.destroy(); + } } } else { break None; @@ -271,7 +273,9 @@ impl crate::CommandEncoder for super::CommandEncoder { if list.close().into_result().is_ok() { self.free_lists.push(list); } else { - list.destroy(); + unsafe { + list.destroy(); + } } } } @@ -285,7 +289,9 @@ impl crate::CommandEncoder for super::CommandEncoder { if cmd_buf.closed { self.free_lists.push(cmd_buf.raw); } else { - cmd_buf.raw.destroy(); + unsafe { + cmd_buf.raw.destroy(); + } } } self.allocator.reset();