diff --git a/CHANGELOG.md b/CHANGELOG.md index 999e221f1a..034f597bda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,7 @@ Bottom level categories: - Fix panic when creating a surface while no backend is available. By @wumpf [#5166](https://github.com/gfx-rs/wgpu/pull/5166) - Correctly compute minimum buffer size for array-typed `storage` and `uniform` vars. By @jimblandy [#5222](https://github.com/gfx-rs/wgpu/pull/5222) - Fix timeout when presenting a surface where no work has been done. By @waywardmonkeys in [#5200](https://github.com/gfx-rs/wgpu/pull/5200) +- Fix an issue where command encoders weren't properly freed if an error occurred during command encoding. By @ErichDonGubler in [#5251](https://github.com/gfx-rs/wgpu/pull/5251). #### WGL 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 4e0c61a896..3858e3d070 100644 --- a/tests/tests/encoder.rs +++ b/tests/tests/encoder.rs @@ -21,16 +21,9 @@ static DROP_QUEUE_BEFORE_CREATING_COMMAND_ENCODER: GpuTestConfiguration = device.create_command_encoder(&wgpu::CommandEncoderDescriptor::default()); }); -// 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_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-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index 2d5fca200a..416c0b057e 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -75,7 +75,7 @@ impl CommandEncoder { Ok(()) } - fn discard(&mut self) { + pub(crate) fn discard(&mut self) { if self.is_open { self.is_open = false; unsafe { self.raw.discard_encoding() }; @@ -112,7 +112,7 @@ pub(crate) struct DestroyedBufferError(pub id::BufferId); pub(crate) struct DestroyedTextureError(pub id::TextureId); pub struct CommandBufferMutable { - encoder: CommandEncoder, + pub(crate) encoder: CommandEncoder, status: CommandEncoderStatus, pub(crate) trackers: Tracker, buffer_memory_init_actions: Vec>, diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index 39408d2d40..fe2a5de041 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -1377,6 +1377,7 @@ impl Global { .command_buffers .unregister(command_encoder_id.transmute()) { + cmd_buf.data.lock().as_mut().unwrap().encoder.discard(); cmd_buf .device .untrack(&cmd_buf.data.lock().as_ref().unwrap().trackers); diff --git a/wgpu-hal/src/dx12/command.rs b/wgpu-hal/src/dx12/command.rs index f527898d90..9d96d29cae 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 2507c125f8..3603b033b8 100644 --- a/wgpu-hal/src/dx12/device.rs +++ b/wgpu-hal/src/dx12/device.rs @@ -663,11 +663,7 @@ impl crate::Device 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, diff --git a/wgpu-hal/src/gles/command.rs b/wgpu-hal/src/gles/command.rs index 926122e4ad..4385e2a31e 100644 --- a/wgpu-hal/src/gles/command.rs +++ b/wgpu-hal/src/gles/command.rs @@ -93,6 +93,13 @@ impl super::CommandBuffer { } } +impl Drop for super::CommandEncoder { + fn drop(&mut self) { + use crate::CommandEncoder; + unsafe { self.discard_encoding() } + } +} + impl super::CommandEncoder { fn rebind_stencil_func(&mut self) { fn make(s: &super::StencilSide, face: u32) -> C {