Skip to content

Commit

Permalink
avoid creating the bind group for indirect validation if buffer size …
Browse files Browse the repository at this point in the history
…is 0
  • Loading branch information
teoxoy committed Oct 17, 2024
1 parent 74ef445 commit 38a991e
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 14 deletions.
58 changes: 47 additions & 11 deletions tests/tests/dispatch_workgroups_indirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ static NUM_WORKGROUPS_BUILTIN: GpuTestConfiguration = GpuTestConfiguration::new(
)
.run_async(|ctx| async move {
let num_workgroups = [1, 2, 3];
let res = run_test(&ctx, &num_workgroups, false).await;
let res = run_test(&ctx, &num_workgroups, false, false).await;
assert_eq!(res, num_workgroups);
});

Expand All @@ -38,16 +38,16 @@ static DISCARD_DISPATCH: GpuTestConfiguration = GpuTestConfiguration::new()
.run_async(|ctx| async move {
let max = ctx.device.limits().max_compute_workgroups_per_dimension;

let res = run_test(&ctx, &[max, max, max], false).await;
let res = run_test(&ctx, &[max, max, max], false, false).await;
assert_eq!(res, [max; 3]);

let res = run_test(&ctx, &[max + 1, 1, 1], false).await;
let res = run_test(&ctx, &[max + 1, 1, 1], false, false).await;
assert_eq!(res, [0; 3]);

let res = run_test(&ctx, &[1, max + 1, 1], false).await;
let res = run_test(&ctx, &[1, max + 1, 1], false, false).await;
assert_eq!(res, [0; 3]);

let res = run_test(&ctx, &[1, 1, max + 1], false).await;
let res = run_test(&ctx, &[1, 1, max + 1], false, false).await;
assert_eq!(res, [0; 3]);
});

Expand All @@ -68,18 +68,46 @@ static RESET_BIND_GROUPS: GpuTestConfiguration = GpuTestConfiguration::new()
.run_async(|ctx| async move {
ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);

let _ = run_test(&ctx, &[0, 0, 0], true).await;
let _ = run_test(&ctx, &[0, 0, 0], true,false).await;

let error = pollster::block_on(ctx.device.pop_error_scope());
assert!(error.map_or(false, |error| {
format!("{error}").contains("The current set ComputePipeline with '' label expects a BindGroup to be set at index 0")
}));
});

/// Make sure that zero sized buffer validation is raised.
#[gpu_test]
static ZERO_SIZED_BUFFER: GpuTestConfiguration = GpuTestConfiguration::new()
.parameters(
TestParameters::default()
.features(wgpu::Features::PUSH_CONSTANTS)
.downlevel_flags(
wgpu::DownlevelFlags::COMPUTE_SHADERS | wgpu::DownlevelFlags::INDIRECT_EXECUTION,
)
.limits(wgpu::Limits {
max_push_constant_size: 4,
..wgpu::Limits::downlevel_defaults()
}),
)
.run_async(|ctx| async move {
ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);

let _ = run_test(&ctx, &[0, 0, 0], false, true).await;

let error = pollster::block_on(ctx.device.pop_error_scope());
assert!(error.map_or(false, |error| {
format!("{error}").contains(
"Indirect buffer uses bytes 0..12 which overruns indirect buffer of size 0",
)
}));
});

async fn run_test(
ctx: &TestingContext,
num_workgroups: &[u32; 3],
forget_to_set_bind_group: bool,
set_buffer_size_to_0: bool,
) -> [u32; 3] {
const SHADER_SRC: &str = "
struct TestOffsetPc {
Expand Down Expand Up @@ -186,18 +214,26 @@ async fn run_test(
(256 * 2 * 2, 256 * 2 * 2 + 32),
(256 + 12, 256 * 2 * 2 + 64),
] {
let indirect_buffer_size = if set_buffer_size_to_0 {
0
} else {
indirect_buffer_size
};

let indirect_buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
label: None,
size: indirect_buffer_size,
usage: wgpu::BufferUsages::COPY_DST | wgpu::BufferUsages::INDIRECT,
mapped_at_creation: false,
});

ctx.queue.write_buffer(
&indirect_buffer,
indirect_offset,
bytemuck::bytes_of(num_workgroups),
);
if !set_buffer_size_to_0 {
ctx.queue.write_buffer(
&indirect_buffer,
indirect_offset,
bytemuck::bytes_of(num_workgroups),
);
}

let mut encoder = ctx.device.create_command_encoder(&Default::default());
{
Expand Down
5 changes: 4 additions & 1 deletion wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,10 @@ impl Device {
let bind_group = indirect_validation
.create_src_bind_group(self.raw(), &self.limits, buffer_size, raw_buffer)
.map_err(resource::CreateBufferError::IndirectValidationBindGroup)?;
Ok(Snatchable::new(bind_group))
match bind_group {
Some(bind_group) => Ok(Snatchable::new(bind_group)),
None => Ok(Snatchable::empty()),
}
} else {
Ok(Snatchable::empty())
}
Expand Down
10 changes: 8 additions & 2 deletions wgpu-core/src/indirect_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,14 +266,19 @@ impl IndirectValidation {
})
}

/// `Ok(None)` will only be returned if `buffer_size` is `0`.
pub fn create_src_bind_group(
&self,
device: &dyn hal::DynDevice,
limits: &wgt::Limits,
buffer_size: u64,
buffer: &dyn hal::DynBuffer,
) -> Result<Box<dyn hal::DynBindGroup>, DeviceError> {
) -> Result<Option<Box<dyn hal::DynBindGroup>>, DeviceError> {
let binding_size = calculate_src_buffer_binding_size(buffer_size, limits);
let binding_size = match NonZeroU64::new(binding_size) {
Some(binding_size) => binding_size,
None => return Ok(None),
};
let hal_desc = hal::BindGroupDescriptor {
label: None,
layout: self.src_bind_group_layout.as_ref(),
Expand All @@ -285,7 +290,7 @@ impl IndirectValidation {
buffers: &[hal::BufferBinding {
buffer,
offset: 0,
size: Some(NonZeroU64::new(binding_size).unwrap()),
size: Some(binding_size),
}],
samplers: &[],
textures: &[],
Expand All @@ -294,6 +299,7 @@ impl IndirectValidation {
unsafe {
device
.create_bind_group(&hal_desc)
.map(Some)
.map_err(DeviceError::from_hal)
}
}
Expand Down

0 comments on commit 38a991e

Please sign in to comment.