Skip to content

Commit

Permalink
fix(dx12): discard cmd. enc. buf. on drop
Browse files Browse the repository at this point in the history
  • Loading branch information
ErichDonGubler authored and cwfitzgerald committed Mar 1, 2024
1 parent 61d779d commit 45ef175
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 33 deletions.
23 changes: 3 additions & 20 deletions tests/tests/bind_group_layout_dedup.rs
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
22 changes: 14 additions & 8 deletions tests/tests/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions wgpu-hal/src/dx12/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 1 addition & 5 deletions wgpu-hal/src/dx12/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 45ef175

Please sign in to comment.