Skip to content

Commit e8dfc72

Browse files
andyleisersonteoxoy
authored andcommitted
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`.
1 parent b3d9431 commit e8dfc72

5 files changed

Lines changed: 22 additions & 62 deletions

File tree

wgpu-core/src/command/compute.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -293,21 +293,14 @@ impl<'scope, 'snatch_guard, 'cmd_enc> State<'scope, 'snatch_guard, 'cmd_enc> {
293293
}
294294

295295
for bind_group in self.pass.binder.list_active() {
296-
unsafe {
297-
self.intermediate_trackers
298-
.set_and_remove_from_usage_scope_sparse(&mut self.pass.scope, &bind_group.used)
299-
}
296+
self.intermediate_trackers
297+
.set_from_bind_group(&mut self.pass.scope, &bind_group.used);
300298
}
301299

302300
// Add the state of the indirect buffer if it hasn't been hit before.
303-
unsafe {
304-
self.intermediate_trackers
305-
.buffers
306-
.set_and_remove_from_usage_scope_sparse(
307-
&mut self.pass.scope.buffers,
308-
indirect_buffer,
309-
);
310-
}
301+
self.intermediate_trackers
302+
.buffers
303+
.set_multiple(&mut self.pass.scope.buffers, indirect_buffer);
311304

312305
CommandEncoder::drain_barriers(
313306
self.pass.base.raw_encoder,

wgpu-core/src/track/buffer.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -442,12 +442,7 @@ impl BufferTracker {
442442
/// over all elements in the usage scope. We use each the
443443
/// a given iterator of ids as a source of which IDs to look at.
444444
/// All the IDs must have first been added to the usage scope.
445-
///
446-
/// # Safety
447-
///
448-
/// [`Self::set_size`] must be called with the maximum possible Buffer ID before this
449-
/// method is called.
450-
pub unsafe fn set_and_remove_from_usage_scope_sparse(
445+
pub fn set_multiple(
451446
&mut self,
452447
scope: &mut BufferUsageScope,
453448
index_source: impl IntoIterator<Item = TrackerIndex>,
@@ -465,6 +460,9 @@ impl BufferTracker {
465460
if unsafe { !scope.metadata.contains_unchecked(index) } {
466461
continue;
467462
}
463+
464+
// SAFETY: we checked that the index is in bounds for the scope, and
465+
// called `set_size` to ensure it is valid for `self`.
468466
unsafe {
469467
self.insert_or_barrier_update(
470468
index,
@@ -477,8 +475,6 @@ impl BufferTracker {
477475
},
478476
)
479477
};
480-
481-
unsafe { scope.metadata.remove(index) };
482478
}
483479
}
484480

wgpu-core/src/track/metadata.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,6 @@ impl<T: Clone> ResourceMetadata<T> {
130130
};
131131
iterate_bitvec_indices(&self.owned)
132132
}
133-
134-
/// Remove the resource with the given index from the set.
135-
pub(super) unsafe fn remove(&mut self, index: usize) {
136-
unsafe {
137-
*self.resources.get_unchecked_mut(index) = None;
138-
}
139-
self.owned.set(index, false);
140-
}
141133
}
142134

143135
/// A source of resource metadata.

wgpu-core/src/track/mod.rs

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -641,41 +641,25 @@ impl Tracker {
641641
}
642642

643643
/// Iterates through all resources in the given bind group and adopts
644-
/// the state given for those resources in the UsageScope. It also
645-
/// removes all touched resources from the usage scope.
644+
/// the state given for those resources in the UsageScope.
646645
///
647646
/// If a transition is needed to get the resources into the needed
648647
/// state, those transitions are stored within the tracker. A
649648
/// subsequent call to [`BufferTracker::drain_transitions`] or
650649
/// [`TextureTracker::drain_transitions`] is needed to get those transitions.
651650
///
652651
/// This is a really funky method used by Compute Passes to generate
653-
/// barriers after a call to dispatch without needing to iterate
654-
/// over all elements in the usage scope. We use each the
655-
/// bind group as a source of which IDs to look at. The bind groups
656-
/// must have first been added to the usage scope.
652+
/// barriers for each dispatch. We use the bind group as a source of which
653+
/// IDs to look at.
657654
///
658655
/// Only stateful things are merged in here, all other resources are owned
659656
/// indirectly by the bind group.
660-
///
661-
/// # Safety
662-
///
663-
/// The maximum ID given by each bind group resource must be less than the
664-
/// value given to `set_size`
665-
pub unsafe fn set_and_remove_from_usage_scope_sparse(
666-
&mut self,
667-
scope: &mut UsageScope,
668-
bind_group: &BindGroupStates,
669-
) {
670-
unsafe {
671-
self.buffers.set_and_remove_from_usage_scope_sparse(
672-
&mut scope.buffers,
673-
bind_group.buffers.used_tracker_indices(),
674-
)
675-
};
676-
unsafe {
677-
self.textures
678-
.set_and_remove_from_usage_scope_sparse(&mut scope.textures, &bind_group.views)
679-
};
657+
pub fn set_from_bind_group(&mut self, scope: &mut UsageScope, bind_group: &BindGroupStates) {
658+
self.buffers.set_multiple(
659+
&mut scope.buffers,
660+
bind_group.buffers.used_tracker_indices(),
661+
);
662+
self.textures
663+
.set_multiple(&mut scope.textures, &bind_group.views);
680664
}
681665
}

wgpu-core/src/track/texture.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -611,12 +611,7 @@ impl TextureTracker {
611611
/// over all elements in the usage scope. We use each the
612612
/// bind group as a source of which IDs to look at. The bind groups
613613
/// must have first been added to the usage scope.
614-
///
615-
/// # Safety
616-
///
617-
/// [`Self::set_size`] must be called with the maximum possible Buffer ID before this
618-
/// method is called.
619-
pub unsafe fn set_and_remove_from_usage_scope_sparse(
614+
pub fn set_multiple(
620615
&mut self,
621616
scope: &mut TextureUsageScope,
622617
bind_group_state: &TextureViewBindGroupState,
@@ -634,6 +629,8 @@ impl TextureTracker {
634629
continue;
635630
}
636631
let texture_selector = &view.parent.full_range;
632+
// SAFETY: we checked that the index is in bounds for the scope, and
633+
// called `set_size` to ensure it is valid for `self`.
637634
unsafe {
638635
insert_or_barrier_update(
639636
texture_selector,
@@ -649,8 +646,6 @@ impl TextureTracker {
649646
&mut self.temp,
650647
)
651648
};
652-
653-
unsafe { scope.metadata.remove(index) };
654649
}
655650
}
656651
}

0 commit comments

Comments
 (0)