[trackers] move common logic into TextureStateSet
#6689
Conversation
|
@teoxoy Is this preparation for replacing |
|
It was but since we decided last week not to focus on this I only put up this PR for the work that I already did. |
| } | ||
|
|
||
| /// SAFETY: `index` must be in bounds. | ||
| unsafe fn get(&self, index: usize) -> SingleOrManyStates<TextureUses, &ComplexTextureState> { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
My feeling is that unchecked would be helpful to readers.
|
|
||
| strict_assert!(index < self.set.simple.len()); | ||
|
|
||
| strict_assert!(if self.metadata.contains(index) |
There was a problem hiding this comment.
Doesn't this condition (self.metadata.contains(index)) get dropped if we replace all this with a call to TextureStateSet::tracker_assert_in_bounds? (Similarly in subsequent functions.)
There was a problem hiding this comment.
It does but it's already checked above in self.metadata.tracker_assert_in_bounds(index);.
There was a problem hiding this comment.
ResourceMetadata::tracker_assert_in_bounds does not actually assert that index is a member of the ResourceMetadata, only that the entries at index in owned and resources are consistent.
There was a problem hiding this comment.
ResourceMetadata::tracker_assert_in_boundsdoes not actually assert thatindexis a member of theResourceMetadata
Doesn't the strict_assert!(index < self.owned.len()); do that? ResourceMetadata::contains tries to lookup in self.owned.
No description provided.