Small refactors related to bind group tracking for compute passes

* Don't remove resources from the usage scope, it is not necessary, and
  not useful, since we don't verify the scope is empty afterwards.
* Avoid some unnecessary `unsafe`.
This commit is contained in:
Andy Leiserson 2025-10-20 16:39:29 -07:00 committed by Teodor Tanasoaia
parent b3d94317d4
commit e8dfc72d04
5 changed files with 22 additions and 62 deletions

View File

@ -293,21 +293,14 @@ impl<'scope, 'snatch_guard, 'cmd_enc> State<'scope, 'snatch_guard, 'cmd_enc> {
} }
for bind_group in self.pass.binder.list_active() { for bind_group in self.pass.binder.list_active() {
unsafe { self.intermediate_trackers
self.intermediate_trackers .set_from_bind_group(&mut self.pass.scope, &bind_group.used);
.set_and_remove_from_usage_scope_sparse(&mut self.pass.scope, &bind_group.used)
}
} }
// Add the state of the indirect buffer if it hasn't been hit before. // Add the state of the indirect buffer if it hasn't been hit before.
unsafe { self.intermediate_trackers
self.intermediate_trackers .buffers
.buffers .set_multiple(&mut self.pass.scope.buffers, indirect_buffer);
.set_and_remove_from_usage_scope_sparse(
&mut self.pass.scope.buffers,
indirect_buffer,
);
}
CommandEncoder::drain_barriers( CommandEncoder::drain_barriers(
self.pass.base.raw_encoder, self.pass.base.raw_encoder,

View File

@ -442,12 +442,7 @@ impl BufferTracker {
/// over all elements in the usage scope. We use each the /// over all elements in the usage scope. We use each the
/// a given iterator of ids as a source of which IDs to look at. /// a given iterator of ids as a source of which IDs to look at.
/// All the IDs must have first been added to the usage scope. /// All the IDs must have first been added to the usage scope.
/// pub fn set_multiple(
/// # Safety
///
/// [`Self::set_size`] must be called with the maximum possible Buffer ID before this
/// method is called.
pub unsafe fn set_and_remove_from_usage_scope_sparse(
&mut self, &mut self,
scope: &mut BufferUsageScope, scope: &mut BufferUsageScope,
index_source: impl IntoIterator<Item = TrackerIndex>, index_source: impl IntoIterator<Item = TrackerIndex>,
@ -465,6 +460,9 @@ impl BufferTracker {
if unsafe { !scope.metadata.contains_unchecked(index) } { if unsafe { !scope.metadata.contains_unchecked(index) } {
continue; continue;
} }
// SAFETY: we checked that the index is in bounds for the scope, and
// called `set_size` to ensure it is valid for `self`.
unsafe { unsafe {
self.insert_or_barrier_update( self.insert_or_barrier_update(
index, index,
@ -477,8 +475,6 @@ impl BufferTracker {
}, },
) )
}; };
unsafe { scope.metadata.remove(index) };
} }
} }

View File

@ -130,14 +130,6 @@ impl<T: Clone> ResourceMetadata<T> {
}; };
iterate_bitvec_indices(&self.owned) iterate_bitvec_indices(&self.owned)
} }
/// Remove the resource with the given index from the set.
pub(super) unsafe fn remove(&mut self, index: usize) {
unsafe {
*self.resources.get_unchecked_mut(index) = None;
}
self.owned.set(index, false);
}
} }
/// A source of resource metadata. /// A source of resource metadata.

View File

@ -641,8 +641,7 @@ impl Tracker {
} }
/// Iterates through all resources in the given bind group and adopts /// Iterates through all resources in the given bind group and adopts
/// the state given for those resources in the UsageScope. It also /// the state given for those resources in the UsageScope.
/// removes all touched resources from the usage scope.
/// ///
/// If a transition is needed to get the resources into the needed /// If a transition is needed to get the resources into the needed
/// state, those transitions are stored within the tracker. A /// state, those transitions are stored within the tracker. A
@ -650,32 +649,17 @@ impl Tracker {
/// [`TextureTracker::drain_transitions`] is needed to get those transitions. /// [`TextureTracker::drain_transitions`] is needed to get those transitions.
/// ///
/// This is a really funky method used by Compute Passes to generate /// This is a really funky method used by Compute Passes to generate
/// barriers after a call to dispatch without needing to iterate /// barriers for each dispatch. We use the bind group as a source of which
/// over all elements in the usage scope. We use each the /// IDs to look at.
/// bind group as a source of which IDs to look at. The bind groups
/// must have first been added to the usage scope.
/// ///
/// Only stateful things are merged in here, all other resources are owned /// Only stateful things are merged in here, all other resources are owned
/// indirectly by the bind group. /// indirectly by the bind group.
/// pub fn set_from_bind_group(&mut self, scope: &mut UsageScope, bind_group: &BindGroupStates) {
/// # Safety self.buffers.set_multiple(
/// &mut scope.buffers,
/// The maximum ID given by each bind group resource must be less than the bind_group.buffers.used_tracker_indices(),
/// value given to `set_size` );
pub unsafe fn set_and_remove_from_usage_scope_sparse( self.textures
&mut self, .set_multiple(&mut scope.textures, &bind_group.views);
scope: &mut UsageScope,
bind_group: &BindGroupStates,
) {
unsafe {
self.buffers.set_and_remove_from_usage_scope_sparse(
&mut scope.buffers,
bind_group.buffers.used_tracker_indices(),
)
};
unsafe {
self.textures
.set_and_remove_from_usage_scope_sparse(&mut scope.textures, &bind_group.views)
};
} }
} }

View File

@ -611,12 +611,7 @@ impl TextureTracker {
/// over all elements in the usage scope. We use each the /// over all elements in the usage scope. We use each the
/// bind group as a source of which IDs to look at. The bind groups /// bind group as a source of which IDs to look at. The bind groups
/// must have first been added to the usage scope. /// must have first been added to the usage scope.
/// pub fn set_multiple(
/// # Safety
///
/// [`Self::set_size`] must be called with the maximum possible Buffer ID before this
/// method is called.
pub unsafe fn set_and_remove_from_usage_scope_sparse(
&mut self, &mut self,
scope: &mut TextureUsageScope, scope: &mut TextureUsageScope,
bind_group_state: &TextureViewBindGroupState, bind_group_state: &TextureViewBindGroupState,
@ -634,6 +629,8 @@ impl TextureTracker {
continue; continue;
} }
let texture_selector = &view.parent.full_range; let texture_selector = &view.parent.full_range;
// SAFETY: we checked that the index is in bounds for the scope, and
// called `set_size` to ensure it is valid for `self`.
unsafe { unsafe {
insert_or_barrier_update( insert_or_barrier_update(
texture_selector, texture_selector,
@ -649,8 +646,6 @@ impl TextureTracker {
&mut self.temp, &mut self.temp,
) )
}; };
unsafe { scope.metadata.remove(index) };
} }
} }
} }