Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[trackers] move common logic into TextureStateSet #6689

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
+105 −107
Open

[trackers] move common logic into TextureStateSet #6689

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
212 changes: 105 additions & 107 deletions wgpu-core/src/track/texture.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,75 @@ impl TextureStateSet {
fn set_size(&mut self, size: usize) {
self.simple.resize(size, TextureUses::UNINITIALIZED);
}

fn size(&self) -> usize {
self.simple.len()
}

/// SAFETY: `index` must be in bounds.
unsafe fn get(&self, index: usize) -> SingleOrManyStates<TextureUses, &ComplexTextureState> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these all have names containing unchecked? They are all marked unsafe anyway, but in the Rust standard library, methods named get and get_mut are checked.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? My counterargument would be that TextureStateSet is not broad/generic as the standard library is (we only have unsafe methods here but not their safe counterparts).

I don't mind adding _unchecked though - let me know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My feeling is that unchecked would be helpful to readers.

let simple = unsafe { *self.simple.get_unchecked(index) };
if simple == TextureUses::COMPLEX {
SingleOrManyStates::Many(unsafe { self.complex.get(&index).unwrap_unchecked() })
} else {
SingleOrManyStates::Single(simple)
}
}

/// # Safety
///
/// The `index` must be in bounds.
unsafe fn get_mut(
&mut self,
index: usize,
) -> SingleOrManyStates<&mut TextureUses, &mut ComplexTextureState> {
let simple = unsafe { self.simple.get_unchecked_mut(index) };
if *simple == TextureUses::COMPLEX {
SingleOrManyStates::Many(unsafe { self.complex.get_mut(&index).unwrap_unchecked() })
} else {
SingleOrManyStates::Single(simple)
}
}

/// # Safety
///
/// The `index` must be in bounds.
unsafe fn insert_simple(&mut self, index: usize, simple: TextureUses) {
unsafe { *self.simple.get_unchecked_mut(index) = simple };
}

/// # Safety
///
/// The `index` must be in bounds.
unsafe fn insert_complex(&mut self, index: usize, complex: ComplexTextureState) {
unsafe { *self.simple.get_unchecked_mut(index) = TextureUses::COMPLEX };
self.complex.insert(index, complex);
}

/// # Safety
///
/// The `index` must be in bounds.
unsafe fn make_simple(&mut self, index: usize, simple: TextureUses) {
unsafe { *self.simple.get_unchecked_mut(index) = simple };
unsafe { self.complex.remove(&index).unwrap_unchecked() };
}

/// # Safety
///
/// The `index` must be in bounds.
unsafe fn make_complex(&mut self, index: usize, complex: ComplexTextureState) {
unsafe { *self.simple.get_unchecked_mut(index) = TextureUses::COMPLEX };
self.complex.insert(index, complex);
}

fn tracker_assert_in_bounds(&self, index: usize) {
strict_assert!(index < self.size());
strict_assert!(if self.simple[index] == TextureUses::COMPLEX {
self.complex.contains_key(&index)
} else {
true
});
}
}

/// Stores all texture state within a single usage scope.
Expand All @@ -218,16 +287,7 @@ impl Default for TextureUsageScope {
impl TextureUsageScope {
fn tracker_assert_in_bounds(&self, index: usize) {
self.metadata.tracker_assert_in_bounds(index);

strict_assert!(index < self.set.simple.len());

strict_assert!(if self.metadata.contains(index)
jimblandy marked this conversation as resolved.
Show resolved Hide resolved
&& self.set.simple[index] == TextureUses::COMPLEX
{
self.set.complex.contains_key(&index)
} else {
true
});
self.set.tracker_assert_in_bounds(index);
}

pub fn clear(&mut self) {
Expand Down Expand Up @@ -262,8 +322,8 @@ impl TextureUsageScope {
&mut self,
scope: &Self,
) -> Result<(), ResourceUsageCompatibilityError> {
let incoming_size = scope.set.simple.len();
if incoming_size > self.set.simple.len() {
let incoming_size = scope.set.size();
if incoming_size > self.set.size() {
self.set_size(incoming_size);
}

Expand Down Expand Up @@ -385,24 +445,8 @@ impl TextureTracker {

fn tracker_assert_in_bounds(&self, index: usize) {
self.metadata.tracker_assert_in_bounds(index);

strict_assert!(index < self.start_set.simple.len());
strict_assert!(index < self.end_set.simple.len());

strict_assert!(if self.metadata.contains(index)
&& self.start_set.simple[index] == TextureUses::COMPLEX
{
self.start_set.complex.contains_key(&index)
} else {
true
});
strict_assert!(if self.metadata.contains(index)
&& self.end_set.simple[index] == TextureUses::COMPLEX
{
self.end_set.complex.contains_key(&index)
} else {
true
});
self.start_set.tracker_assert_in_bounds(index);
self.end_set.tracker_assert_in_bounds(index);
}

/// Sets the size of all the vectors inside the tracker.
Expand All @@ -418,7 +462,7 @@ impl TextureTracker {

/// Extend the vectors to let the given index be valid.
fn allow_index(&mut self, index: usize) {
if index >= self.start_set.simple.len() {
if index >= self.start_set.size() {
self.set_size(index + 1);
}
}
Expand Down Expand Up @@ -498,8 +542,8 @@ impl TextureTracker {
/// If the ID is higher than the length of internal vectors,
/// the vectors will be extended. A call to set_size is not needed.
pub fn set_from_tracker(&mut self, tracker: &Self) {
let incoming_size = tracker.start_set.simple.len();
if incoming_size > self.start_set.simple.len() {
let incoming_size = tracker.start_set.size();
if incoming_size > self.start_set.size() {
self.set_size(incoming_size);
}

Expand Down Expand Up @@ -538,8 +582,8 @@ impl TextureTracker {
/// If the ID is higher than the length of internal vectors,
/// the vectors will be extended. A call to set_size is not needed.
pub fn set_from_usage_scope(&mut self, scope: &TextureUsageScope) {
let incoming_size = scope.set.simple.len();
if incoming_size > self.start_set.simple.len() {
let incoming_size = scope.set.size();
if incoming_size > self.start_set.size() {
self.set_size(incoming_size);
}

Expand Down Expand Up @@ -588,8 +632,8 @@ impl TextureTracker {
scope: &mut TextureUsageScope,
bind_group_state: &TextureViewBindGroupState,
) {
let incoming_size = scope.set.simple.len();
if incoming_size > self.start_set.simple.len() {
let incoming_size = scope.set.size();
if incoming_size > self.start_set.size() {
self.set_size(incoming_size);
}

Expand Down Expand Up @@ -651,21 +695,12 @@ impl DeviceTextureTracker {

fn tracker_assert_in_bounds(&self, index: usize) {
self.metadata.tracker_assert_in_bounds(index);

strict_assert!(index < self.current_state_set.simple.len());

strict_assert!(if self.metadata.contains(index)
&& self.current_state_set.simple[index] == TextureUses::COMPLEX
{
self.current_state_set.complex.contains_key(&index)
} else {
true
});
self.current_state_set.tracker_assert_in_bounds(index);
}

/// Extend the vectors to let the given index be valid.
fn allow_index(&mut self, index: usize) {
if index >= self.current_state_set.simple.len() {
if index >= self.current_state_set.size() {
self.current_state_set.set_size(index + 1);
self.metadata.set_size(index + 1);
}
Expand Down Expand Up @@ -923,19 +958,12 @@ impl<'a> TextureStateProvider<'a> {
SingleOrManyStates::Many(EitherIter::Left(iter::once((selector, state))))
}
}
TextureStateProvider::TextureSet { set } => {
let new_state = *unsafe { set.simple.get_unchecked(index) };

if new_state == TextureUses::COMPLEX {
let new_complex = unsafe { set.complex.get(&index).unwrap_unchecked() };

SingleOrManyStates::Many(EitherIter::Right(
new_complex.to_selector_state_iter(),
))
} else {
SingleOrManyStates::Single(new_state)
TextureStateProvider::TextureSet { set } => match unsafe { set.get(index) } {
SingleOrManyStates::Single(single) => SingleOrManyStates::Single(single),
SingleOrManyStates::Many(complex) => {
SingleOrManyStates::Many(EitherIter::Right(complex.to_selector_state_iter()))
}
}
},
}
}
}
Expand Down Expand Up @@ -1075,12 +1103,12 @@ unsafe fn insert<T: Clone>(
strict_assert_eq!(invalid_resource_state(state), false);

if let Some(start_state) = start_state {
unsafe { *start_state.simple.get_unchecked_mut(index) = state };
unsafe { start_state.insert_simple(index, state) };
}

// We only need to insert ourselves the end state if there is no end state provider.
if end_state_provider.is_none() {
unsafe { *end_state.simple.get_unchecked_mut(index) = state };
unsafe { end_state.insert_simple(index, state) };
}
}
SingleOrManyStates::Many(state_iter) => {
Expand All @@ -1090,14 +1118,12 @@ unsafe fn insert<T: Clone>(
unsafe { ComplexTextureState::from_selector_state_iter(full_range, state_iter) };

if let Some(start_state) = start_state {
unsafe { *start_state.simple.get_unchecked_mut(index) = TextureUses::COMPLEX };
start_state.complex.insert(index, complex.clone());
unsafe { start_state.insert_complex(index, complex.clone()) };
}

// We only need to insert ourselves the end state if there is no end state provider.
if end_state_provider.is_none() {
unsafe { *end_state.simple.get_unchecked_mut(index) = TextureUses::COMPLEX };
end_state.complex.insert(index, complex);
unsafe { end_state.insert_complex(index, complex) };
}
}
}
Expand All @@ -1111,7 +1137,7 @@ unsafe fn insert<T: Clone>(

// We only need to insert into the end, as there is guaranteed to be
// a start state provider.
unsafe { *end_state.simple.get_unchecked_mut(index) = state };
unsafe { end_state.insert_simple(index, state) };
}
SingleOrManyStates::Many(state_iter) => {
let full_range = texture_selector.unwrap().clone();
Expand All @@ -1122,8 +1148,7 @@ unsafe fn insert<T: Clone>(

// We only need to insert into the end, as there is guaranteed to be
// a start state provider.
unsafe { *end_state.simple.get_unchecked_mut(index) = TextureUses::COMPLEX };
end_state.complex.insert(index, complex);
unsafe { end_state.insert_complex(index, complex) };
}
}
}
Expand All @@ -1142,14 +1167,7 @@ unsafe fn merge(
state_provider: TextureStateProvider<'_>,
metadata_provider: ResourceMetadataProvider<'_, Arc<Texture>>,
) -> Result<(), ResourceUsageCompatibilityError> {
let current_simple = unsafe { current_state_set.simple.get_unchecked_mut(index) };
let current_state = if *current_simple == TextureUses::COMPLEX {
SingleOrManyStates::Many(unsafe {
current_state_set.complex.get_mut(&index).unwrap_unchecked()
})
} else {
SingleOrManyStates::Single(current_simple)
};
let current_state = unsafe { current_state_set.get_mut(index) };

let new_state = unsafe { state_provider.get_state(Some(texture_selector), index) };

Expand Down Expand Up @@ -1204,8 +1222,7 @@ unsafe fn merge(
}
}

*current_simple = TextureUses::COMPLEX;
current_state_set.complex.insert(index, new_complex);
unsafe { current_state_set.make_complex(index, new_complex) };
}
(SingleOrManyStates::Many(current_complex), SingleOrManyStates::Single(new_simple)) => {
for (mip_id, mip) in current_complex.mips.iter_mut().enumerate() {
Expand Down Expand Up @@ -1284,14 +1301,7 @@ unsafe fn barrier(
state_provider: TextureStateProvider<'_>,
barriers: &mut Vec<PendingTransition<TextureUses>>,
) {
let current_simple = unsafe { *current_state_set.simple.get_unchecked(index) };
let current_state = if current_simple == TextureUses::COMPLEX {
SingleOrManyStates::Many(unsafe {
current_state_set.complex.get(&index).unwrap_unchecked()
})
} else {
SingleOrManyStates::Single(current_simple)
};
let current_state = unsafe { current_state_set.get(index) };

let new_state = unsafe { state_provider.get_state(Some(texture_selector), index) };

Expand Down Expand Up @@ -1381,7 +1391,6 @@ unsafe fn barrier(
}
}

#[allow(clippy::needless_option_as_deref)] // we use this for reborrowing Option<&mut T>
#[inline(always)]
unsafe fn update(
texture_selector: &TextureSelector,
Expand All @@ -1393,23 +1402,14 @@ unsafe fn update(
// We only ever need to update the start state here if the state is complex.
//
// If the state is simple, the first insert to the tracker would cover it.
let mut start_complex = None;
if let Some(start_state_set) = start_state_set {
let start_simple = unsafe { *start_state_set.simple.get_unchecked(index) };
if start_simple == TextureUses::COMPLEX {
start_complex =
Some(unsafe { start_state_set.complex.get_mut(&index).unwrap_unchecked() });
let mut start_complex = start_state_set.and_then(|start_state_set| {
match unsafe { start_state_set.get_mut(index) } {
SingleOrManyStates::Single(_) => None,
SingleOrManyStates::Many(complex) => Some(complex),
}
}
});

let current_simple = unsafe { current_state_set.simple.get_unchecked_mut(index) };
let current_state = if *current_simple == TextureUses::COMPLEX {
SingleOrManyStates::Many(unsafe {
current_state_set.complex.get_mut(&index).unwrap_unchecked()
})
} else {
SingleOrManyStates::Single(current_simple)
};
let current_state = unsafe { current_state_set.get_mut(index) };

let new_state = unsafe { state_provider.get_state(Some(texture_selector), index) };

Expand Down Expand Up @@ -1445,8 +1445,7 @@ unsafe fn update(
}
}

*current_simple = TextureUses::COMPLEX;
current_state_set.complex.insert(index, new_complex);
unsafe { current_state_set.make_complex(index, new_complex) };
}
(SingleOrManyStates::Many(current_complex), SingleOrManyStates::Single(new_single)) => {
for (mip_id, mip) in current_complex.mips.iter().enumerate() {
Expand All @@ -1471,8 +1470,7 @@ unsafe fn update(
}
}

unsafe { *current_state_set.simple.get_unchecked_mut(index) = new_single };
unsafe { current_state_set.complex.remove(&index).unwrap_unchecked() };
unsafe { current_state_set.make_simple(index, new_single) };
}
(SingleOrManyStates::Many(current_complex), SingleOrManyStates::Many(new_many)) => {
for (selector, new_state) in new_many {
Expand Down
Loading