Skip to content

Commit

Permalink
Add a function to clear bindgroups from passes and bundles, with stub…
Browse files Browse the repository at this point in the history
… backend implementations.

Since the spec allows null BindGroups to be passed to setBindGroup, we
need a way to clear the bindGroup associated with an index. This patch
implements that through a clear_bind_group function defined in various
necessary places, and implements as much of it as possible, without
getting into the wgpu-hal backend implementations, which are left as
TODOs.
  • Loading branch information
bradwerth committed Jun 5, 2024
1 parent 583cc6a commit 7dab9a9
Show file tree
Hide file tree
Showing 20 changed files with 322 additions and 1 deletion.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ TODO(wumpf): This is still work in progress. Should write a bit more about it. A

`wgpu::ComputePass` recording methods (e.g. `wgpu::ComputePass:set_render_pipeline`) no longer impose a lifetime constraint passed in resources.

Furthermore, you can now opt out of `wgpu::ComputePass`'s lifetime dependency on its parent `wgpu::CommandEncoder` using `wgpu::ComputePass::forget_lifetime`:
Furthermore, you can now opt out of `wgpu::ComputePass`'s lifetime dependency on its parent `wgpu::CommandEncoder` using `wgpu::ComputePass::forget_lifetime`:
```rust
fn independent_cpass<'enc>(encoder: &'enc mut wgpu::CommandEncoder) -> wgpu::ComputePass<'static> {
let cpass: wgpu::ComputePass<'enc> = encoder.begin_compute_pass(&wgpu::ComputePassDescriptor::default());
Expand Down Expand Up @@ -117,6 +117,7 @@ By @stefnotch in [#5410](https://github.com/gfx-rs/wgpu/pull/5410)

- Ensure render pipelines have at least 1 target. By @ErichDonGubler in [#5715](https://github.com/gfx-rs/wgpu/pull/5715)
- `wgpu::ComputePass` now internally takes ownership of `QuerySet` for both `wgpu::ComputePassTimestampWrites` as well as timestamp writes and statistics query, fixing crashes when destroying `QuerySet` before ending the pass. By @wumpf in [#5671](https://github.com/gfx-rs/wgpu/pull/5671)
- Stub support for passing null BindGroups to setBindGroup. By @bradwerth in [#5778](https://github.com/gfx-rs/wgpu/pull/5778)

#### Metal

Expand Down
15 changes: 15 additions & 0 deletions wgpu-core/src/command/bind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,11 @@ mod compat {
self.make_range(index)
}

pub fn unassign(&mut self, index: usize) -> Range<usize> {
self.entries[index].assigned = None;
self.make_range(index)
}

pub fn list_active(&self) -> impl Iterator<Item = usize> + '_ {
self.entries
.iter()
Expand Down Expand Up @@ -358,6 +363,16 @@ impl<A: HalApi> Binder<A> {
&self.payloads[bind_range]
}

pub(super) fn unassign_group<'a>(&'a mut self, index: usize) -> &'a [EntryPayload<A>] {
let payload = &mut self.payloads[index];

payload.group = None;
payload.dynamic_offsets.clear();

let bind_range = self.manager.unassign(index);
&self.payloads[bind_range]
}

pub(super) fn list_active<'a>(&'a self) -> impl Iterator<Item = &'a Arc<BindGroup<A>>> + '_ {
let payloads = &self.payloads;
self.manager
Expand Down
27 changes: 27 additions & 0 deletions wgpu-core/src/command/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,11 @@ impl RenderBundleEncoder {
//Note: stateless trackers are not merged: the lifetime reference
// is held to the bind group itself.
}
RenderCommand::ClearBindGroup {
index,
} => {
state.clear_bind_group(index);
}
RenderCommand::SetPipeline(pipeline_id) => {
let scope = PassErrorScope::SetPipelineRender(pipeline_id);

Expand Down Expand Up @@ -1351,6 +1356,15 @@ impl<A: HalApi> State<A> {
self.invalidate_bind_group_from(slot as usize + 1);
}

fn clear_bind_group(&mut self, slot: u32) {
// Record the index's new state.
self.bind[slot as usize] = None;

// Once we've changed the bind group at a particular index, all
// subsequent indices need to be rewritten.
self.invalidate_bind_group_from(slot as usize + 1);
}

/// Determine which bind group slots need to be re-set after a pipeline change.
///
/// Given that we are switching from the current pipeline state to `new`,
Expand Down Expand Up @@ -1568,6 +1582,19 @@ pub mod bundle_ffi {
});
}

#[no_mangle]
pub extern "C" fn wgpu_render_bundle_clear_bind_group(
bundle: &mut RenderBundleEncoder,
index: u32,
) {
bundle.current_bind_groups.clear_bind_group(index);

bundle
.base
.commands
.push(RenderCommand::ClearBindGroup { index });
}

#[no_mangle]
pub extern "C" fn wgpu_render_bundle_set_pipeline(
bundle: &mut RenderBundleEncoder,
Expand Down
32 changes: 32 additions & 0 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,18 @@ impl Global {
}
}
}
ArcComputeCommand::ClearBindGroup { index } => {
let pipeline_layout = state.binder.pipeline_layout.clone();
let entries = state.binder.unassign_group(index as usize);
if !entries.is_empty() && pipeline_layout.is_some() {
let pipeline_layout = pipeline_layout.as_ref().unwrap().raw();
for (i, ..) in entries.iter().enumerate() {
unsafe {
raw.clear_bind_group(pipeline_layout, index + i as u32);
}
}
}
}
ArcComputeCommand::SetPipeline(pipeline) => {
let pipeline_id = pipeline.as_info().id();
let scope = PassErrorScope::SetPipelineCompute(pipeline_id);
Expand Down Expand Up @@ -985,6 +997,26 @@ impl Global {
Ok(())
}

pub fn compute_pass_clear_bind_group<A: HalApi>(
&self,
pass: &mut ComputePass<A>,
index: u32,
) -> Result<(), ComputePassError> {
let scope = PassErrorScope::ClearBindGroup;
let base = pass
.base
.as_mut()
.ok_or(ComputePassErrorInner::PassEnded)
.map_pass_err(scope)?; // Can't use base_mut() utility here because of borrow checker.

pass.current_bind_groups.clear_bind_group(index);

base.commands
.push(ArcComputeCommand::ClearBindGroup { index });

Ok(())
}

pub fn compute_pass_set_pipeline<A: HalApi>(
&self,
pass: &mut ComputePass<A>,
Expand Down
16 changes: 16 additions & 0 deletions wgpu-core/src/command/compute_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ pub enum ComputeCommand {
bind_group_id: id::BindGroupId,
},

ClearBindGroup {
index: u32,
},

SetPipeline(id::ComputePipelineId),

/// Set a range of push constants to values stored in `push_constant_data`.
Expand Down Expand Up @@ -103,6 +107,10 @@ impl ComputeCommand {
})?,
},

ComputeCommand::ClearBindGroup { index } => {
ArcComputeCommand::ClearBindGroup { index }
}

ComputeCommand::SetPipeline(pipeline_id) => ArcComputeCommand::SetPipeline(
pipelines_guard
.get_owned(pipeline_id)
Expand Down Expand Up @@ -194,6 +202,10 @@ pub enum ArcComputeCommand<A: HalApi> {
bind_group: Arc<BindGroup<A>>,
},

ClearBindGroup {
index: u32,
},

SetPipeline(Arc<ComputePipeline<A>>),

/// Set a range of push constants to values stored in `push_constant_data`.
Expand Down Expand Up @@ -261,6 +273,10 @@ impl<A: HalApi> From<&ArcComputeCommand<A>> for ComputeCommand {
bind_group_id: bind_group.as_info().id(),
},

ArcComputeCommand::ClearBindGroup { index } => {
ComputeCommand::ClearBindGroup { index: *index }
}

ArcComputeCommand::SetPipeline(pipeline) => {
ComputeCommand::SetPipeline(pipeline.as_info().id())
}
Expand Down
3 changes: 3 additions & 0 deletions wgpu-core/src/command/draw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ pub enum RenderCommand {
num_dynamic_offsets: usize,
bind_group_id: id::BindGroupId,
},
ClearBindGroup {
index: u32,
},
SetPipeline(id::RenderPipelineId),
SetIndexBuffer {
buffer_id: id::BufferId,
Expand Down
13 changes: 13 additions & 0 deletions wgpu-core/src/command/dyn_compute_pass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ pub trait DynComputePass: std::fmt::Debug + WasmNotSendSync {
bind_group_id: id::BindGroupId,
offsets: &[wgt::DynamicOffset],
) -> Result<(), ComputePassError>;
fn clear_bind_group(
&mut self,
context: &global::Global,
index: u32,
) -> Result<(), ComputePassError>;
fn set_pipeline(
&mut self,
context: &global::Global,
Expand Down Expand Up @@ -85,6 +90,14 @@ impl<A: HalApi> DynComputePass for ComputePass<A> {
context.compute_pass_set_bind_group(self, index, bind_group_id, offsets)
}

fn clear_bind_group(
&mut self,
context: &global::Global,
index: u32,
) -> Result<(), ComputePassError> {
context.compute_pass_clear_bind_group(self, index)
}

fn set_pipeline(
&mut self,
context: &global::Global,
Expand Down
9 changes: 9 additions & 0 deletions wgpu-core/src/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,6 +861,13 @@ impl BindGroupStateChange {
}
false
}

fn clear_bind_group(&mut self, index: u32) {
if let Some(current_bind_group) = self.last_states.get_mut(index as usize) {
current_bind_group.reset();
}
}

fn reset(&mut self) {
self.last_states = [StateChange::new(); hal::MAX_BIND_GROUPS];
}
Expand Down Expand Up @@ -889,6 +896,8 @@ pub enum PassErrorScope {
Pass(Option<id::CommandBufferId>),
#[error("In a set_bind_group command")]
SetBindGroup(id::BindGroupId),
#[error("In a set_bind_group command with a null BindGroup")]
ClearBindGroup,
#[error("In a set_pipeline command")]
SetPipelineRender(id::RenderPipelineId),
#[error("In a set_pipeline command")]
Expand Down
20 changes: 20 additions & 0 deletions wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1532,6 +1532,18 @@ impl Global {
}
}
}
RenderCommand::ClearBindGroup { index } => {
let pipeline_layout = state.binder.pipeline_layout.clone();
let entries = state.binder.unassign_group(index as usize);
if !entries.is_empty() && pipeline_layout.is_some() {
let pipeline_layout = pipeline_layout.as_ref().unwrap().raw();
for (i, ..) in entries.iter().enumerate() {
unsafe {
raw.clear_bind_group(pipeline_layout, index + i as u32);
}
}
}
}
RenderCommand::SetPipeline(pipeline_id) => {
api_log!("RenderPass::set_pipeline {pipeline_id:?}");

Expand Down Expand Up @@ -2488,6 +2500,14 @@ pub mod render_commands {
});
}

pub fn wgpu_render_pass_clear_bind_group(pass: &mut RenderPass, index: u32) {
pass.current_bind_groups.clear_bind_group(index);

pass.base
.commands
.push(RenderCommand::ClearBindGroup { index });
}

pub fn wgpu_render_pass_set_pipeline(pass: &mut RenderPass, pipeline_id: id::RenderPipelineId) {
if pass.current_pipeline.set_and_check_redundant(pipeline_id) {
return;
Expand Down
3 changes: 3 additions & 0 deletions wgpu-hal/src/dx12/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,9 @@ impl crate::CommandEncoder for super::CommandEncoder {
self.reset_signature(&layout.shared);
};
}
unsafe fn clear_bind_group(&mut self, _layout: &super::PipelineLayout, _index: u32) {
// TODO.
}
unsafe fn set_push_constants(
&mut self,
layout: &super::PipelineLayout,
Expand Down
1 change: 1 addition & 0 deletions wgpu-hal/src/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ impl crate::CommandEncoder for Encoder {
dynamic_offsets: &[wgt::DynamicOffset],
) {
}
unsafe fn clear_bind_group(&mut self, layout: &Resource, index: u32) {}
unsafe fn set_push_constants(
&mut self,
layout: &Resource,
Expand Down
4 changes: 4 additions & 0 deletions wgpu-hal/src/gles/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,10 @@ impl crate::CommandEncoder for super::CommandEncoder {
self.rebind_sampler_states(dirty_textures, dirty_samplers);
}

unsafe fn clear_bind_group(&mut self, _layout: &super::PipelineLayout, _index: u32) {
// TODO.
}

unsafe fn set_push_constants(
&mut self,
_layout: &super::PipelineLayout,
Expand Down
3 changes: 3 additions & 0 deletions wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,6 +1093,9 @@ pub trait CommandEncoder: WasmNotSendSync + fmt::Debug {
dynamic_offsets: &[wgt::DynamicOffset],
);

/// Clears the bind group at `index`.
unsafe fn clear_bind_group(&mut self, layout: &<Self::A as Api>::PipelineLayout, index: u32);

/// Sets a range in push constant data.
///
/// IMPORTANT: while the data is passed as words, the offset is in bytes!
Expand Down
4 changes: 4 additions & 0 deletions wgpu-hal/src/metal/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,10 @@ impl crate::CommandEncoder for super::CommandEncoder {
}
}

unsafe fn clear_bind_group(&mut self, _layout: &super::PipelineLayout, _group_index: u32) {
// TODO.
}

unsafe fn set_push_constants(
&mut self,
layout: &super::PipelineLayout,
Expand Down
3 changes: 3 additions & 0 deletions wgpu-hal/src/vulkan/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,9 @@ impl crate::CommandEncoder for super::CommandEncoder {
)
};
}
unsafe fn clear_bind_group(&mut self, _layout: &super::PipelineLayout, _index: u32) {
// TODO.
}
unsafe fn set_push_constants(
&mut self,
layout: &super::PipelineLayout,
Expand Down
17 changes: 17 additions & 0 deletions wgpu/src/backend/webgpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3384,6 +3384,23 @@ impl crate::context::Context for ContextWebGpu {
}
}

fn render_pass_clear_bind_group(
&self,
_pass: &mut Self::RenderPassId,
pass_data: &mut Self::RenderPassData,
index: u32,
) {
pass_data
.0
.set_bind_group_with_u32_array_and_f64_and_dynamic_offsets_data_length(
index,
Some(&bind_group_data.0),
offsets,
0f64,
offsets.len() as u32,
);
}

fn render_pass_set_index_buffer(
&self,
_pass: &mut Self::RenderPassId,
Expand Down
Loading

0 comments on commit 7dab9a9

Please sign in to comment.