Skip to content

Commit

Permalink
Command encoder goes now into locked state while compute pass is open
Browse files Browse the repository at this point in the history
  • Loading branch information
@Wumpf
Wumpf committed May 26, 2024
1 parent df461a6 commit 9d934f5
Show file tree
Hide file tree
Showing 5 changed files with 332 additions and 16 deletions.
230 changes: 229 additions & 1 deletion tests/tests/encoder.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use wgpu_test::{fail, gpu_test, FailureCase, GpuTestConfiguration, TestParameters};
use wgpu::util::DeviceExt;
use wgpu::CommandEncoder;
use wgpu_test::{
fail, gpu_test, FailureCase, GpuTestConfiguration, TestParameters, TestingContext,
};

#[gpu_test]
static DROP_ENCODER: GpuTestConfiguration = GpuTestConfiguration::new().run_sync(|ctx| {
Expand Down Expand Up @@ -72,3 +76,227 @@ static DROP_ENCODER_AFTER_ERROR: GpuTestConfiguration = GpuTestConfiguration::ne
// The encoder is still open!
drop(encoder);
});

// TODO: This should also apply to render passes once the lifetime bound is lifted.
#[gpu_test]
static ENCODER_OPERATIONS_FAIL_WHILE_COMPUTE_PASS_ALIVE: GpuTestConfiguration =
GpuTestConfiguration::new()
.parameters(TestParameters::default().features(
wgpu::Features::CLEAR_TEXTURE
| wgpu::Features::TIMESTAMP_QUERY
| wgpu::Features::TIMESTAMP_QUERY_INSIDE_ENCODERS,
))
.run_sync(encoder_operations_fail_while_compute_pass_alive);

fn encoder_operations_fail_while_compute_pass_alive(ctx: TestingContext) {
let buffer_source = ctx
.device
.create_buffer_init(&wgpu::util::BufferInitDescriptor {
label: None,
contents: &[0u8; 4],
usage: wgpu::BufferUsages::COPY_SRC,
});
let buffer_dest = ctx
.device
.create_buffer_init(&wgpu::util::BufferInitDescriptor {
label: None,
contents: &[0u8; 4],
usage: wgpu::BufferUsages::COPY_DST,
});

let texture_desc = wgpu::TextureDescriptor {
label: None,
size: wgpu::Extent3d {
width: 1,
height: 1,
depth_or_array_layers: 1,
},
mip_level_count: 1,
sample_count: 1,
dimension: wgpu::TextureDimension::D2,
format: wgpu::TextureFormat::Rgba8Unorm,
usage: wgpu::TextureUsages::COPY_DST,
view_formats: &[],
};
let texture_dst = ctx.device.create_texture(&texture_desc);
let texture_src = ctx.device.create_texture(&wgpu::TextureDescriptor {
usage: wgpu::TextureUsages::COPY_SRC,
..texture_desc
});
let query_set = ctx.device.create_query_set(&wgpu::QuerySetDescriptor {
count: 1,
ty: wgpu::QueryType::Timestamp,
label: None,
});

#[allow(clippy::type_complexity)]
let recording_ops: Vec<(_, Box<dyn Fn(&mut CommandEncoder)>)> = vec![
(
"begin_compute_pass",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default());
}),
),
(
"begin_render_pass",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.begin_render_pass(&wgpu::RenderPassDescriptor::default());
}),
),
(
"copy_buffer_to_buffer",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.copy_buffer_to_buffer(&buffer_source, 0, &buffer_dest, 0, 4);
}),
),
(
"copy_buffer_to_texture",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.copy_buffer_to_texture(
wgpu::ImageCopyBuffer {
buffer: &buffer_source,
layout: wgpu::ImageDataLayout {
offset: 0,
bytes_per_row: Some(4),
rows_per_image: None,
},
},
texture_dst.as_image_copy(),
texture_dst.size(),
);
}),
),
(
"copy_texture_to_buffer",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.copy_texture_to_buffer(
wgpu::ImageCopyTexture {
texture: &texture_src,
mip_level: 0,
origin: wgpu::Origin3d::ZERO,
aspect: wgpu::TextureAspect::All,
},
wgpu::ImageCopyBuffer {
buffer: &buffer_dest,
layout: wgpu::ImageDataLayout {
offset: 0,
bytes_per_row: Some(4),
rows_per_image: None,
},
},
texture_dst.size(),
);
}),
),
(
"copy_texture_to_texture",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.copy_texture_to_texture(
wgpu::ImageCopyTexture {
texture: &texture_src,
mip_level: 0,
origin: wgpu::Origin3d::ZERO,
aspect: wgpu::TextureAspect::All,
},
wgpu::ImageCopyTexture {
texture: &texture_dst,
mip_level: 0,
origin: wgpu::Origin3d::ZERO,
aspect: wgpu::TextureAspect::All,
},
texture_dst.size(),
);
}),
),
(
"clear_texture",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.clear_texture(&texture_dst, &wgpu::ImageSubresourceRange::default());
}),
),
(
"clear_buffer",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.clear_buffer(&buffer_dest, 0, None);
}),
),
(
"insert_debug_marker",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.insert_debug_marker("marker");
}),
),
(
"push_debug_group",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.push_debug_group("marker");
}),
),
(
"pop_debug_group",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.pop_debug_group();
}),
),
(
"resolve_query_set",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.resolve_query_set(&query_set, 0..1, &buffer_dest, 0);
}),
),
(
"write_timestamp",
Box::new(|encoder: &mut wgpu::CommandEncoder| {
encoder.write_timestamp(&query_set, 0);
}),
),
];

for (op_name, op) in recording_ops.iter() {
let mut encoder = ctx
.device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());

let pass = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default());

ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);

log::info!("Testing operation {} on a locked command encoder", op_name);
fail(
&ctx.device,
|| op(&mut encoder),
Some("Command encoder is locked"),
);

// Drop the pass - this also fails now since the encoder is invalid:
fail(
&ctx.device,
|| drop(pass),
Some("Command encoder is invalid"),
);
// Also, it's not possible to create a new pass on the encoder:
fail(
&ctx.device,
|| encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default()),
Some("Command encoder is invalid"),
);
}

// Test encoder finishing separately since it consumes the encoder and doesn't fit above pattern.
{
let mut encoder = ctx
.device
.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
let pass = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default());
fail(
&ctx.device,
|| encoder.finish(),
Some("Command encoder is locked"),
);
fail(
&ctx.device,
|| drop(pass),
Some("Command encoder is invalid"),
);
}
}
10 changes: 4 additions & 6 deletions wgpu-core/src/command/clear.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ use wgt::{math::align_to, BufferAddress, BufferUsages, ImageSubresourceRange, Te
pub enum ClearError {
#[error("To use clear_texture the CLEAR_TEXTURE feature needs to be enabled")]
MissingClearTextureFeature,
#[error("Command encoder {0:?} is invalid")]
InvalidCommandEncoder(CommandEncoderId),
#[error("Device {0:?} is invalid")]
InvalidDevice(DeviceId),
#[error("Buffer {0:?} is invalid or destroyed")]
Expand Down Expand Up @@ -74,6 +72,8 @@ whereas subesource range specified start {subresource_base_array_layer} and coun
},
#[error(transparent)]
Device(#[from] DeviceError),
#[error(transparent)]
CommandEncoderError(#[from] super::CommandEncoderError),
}

impl Global {
Expand All @@ -89,8 +89,7 @@ impl Global {

let hub = A::hub(self);

let cmd_buf = CommandBuffer::get_encoder(hub, command_encoder_id)
.map_err(|_| ClearError::InvalidCommandEncoder(command_encoder_id))?;
let cmd_buf = CommandBuffer::get_encoder(hub, command_encoder_id)?;
let mut cmd_buf_data = cmd_buf.data.lock();
let cmd_buf_data = cmd_buf_data.as_mut().unwrap();

Expand Down Expand Up @@ -183,8 +182,7 @@ impl Global {

let hub = A::hub(self);

let cmd_buf = CommandBuffer::get_encoder(hub, command_encoder_id)
.map_err(|_| ClearError::InvalidCommandEncoder(command_encoder_id))?;
let cmd_buf = CommandBuffer::get_encoder(hub, command_encoder_id)?;
let mut cmd_buf_data = cmd_buf.data.lock();
let cmd_buf_data = cmd_buf_data.as_mut().unwrap();

Expand Down
6 changes: 5 additions & 1 deletion wgpu-core/src/command/compute.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,14 +305,16 @@ impl Global {
///
/// If creation fails, an invalid pass is returned.
/// Any operation on an invalid pass will return an error.
///
/// If successful, puts the encoder into the [`CommandEncoderStatus::Locked`] state.
pub fn command_encoder_create_compute_pass<A: HalApi>(
&self,
encoder_id: id::CommandEncoderId,
desc: &ComputePassDescriptor,
) -> (ComputePass<A>, Option<CommandEncoderError>) {
let hub = A::hub(self);

match CommandBuffer::get_encoder(hub, encoder_id) {
match CommandBuffer::lock_encoder(hub, encoder_id) {
Ok(cmd_buf) => (ComputePass::new(Some(cmd_buf), desc), None),
Err(err) => (ComputePass::new(None, desc), Some(err)),
}
Expand Down Expand Up @@ -340,6 +342,8 @@ impl Global {
return Err(ComputePassErrorInner::InvalidParentEncoder).map_pass_err(scope);
};

parent.unlock_encoder().map_pass_err(scope)?;

let base = pass
.base
.take()
Expand Down
Loading

0 comments on commit 9d934f5

Please sign in to comment.