From 36aaf87125abc64c4dab163e10f6bea6528c1a56 Mon Sep 17 00:00:00 2001 From: Trashtalk Date: Tue, 10 Dec 2024 00:52:02 +0100 Subject: [PATCH 1/4] need to quickly store these changes --- crates/bevy_ecs/Cargo.toml | 2 +- crates/bevy_ecs/src/query/access.rs | 178 +++++-------- crates/bevy_ecs/src/storage/mod.rs | 2 + .../bevy_ecs/src/storage/sorted_small_vec.rs | 235 ++++++++++++++++++ 4 files changed, 301 insertions(+), 116 deletions(-) create mode 100644 crates/bevy_ecs/src/storage/sorted_small_vec.rs diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 166c0178e463e..a4ba0c8e85fc9 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -41,7 +41,7 @@ derive_more = { version = "1", default-features = false, features = [ ] } nonmax = "0.5" arrayvec = { version = "0.7.4", optional = true } -smallvec = { version = "1", features = ["union"] } +smallvec = { version = "1.13", features = ["union", "const_generics", "const_new"] } indexmap = { version = "2.5.0", default-features = false, features = ["std"] } variadics_please = "1.0" diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index a0e311ab3dd5f..813bbed943517 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -1,49 +1,12 @@ use crate::component::ComponentId; +use crate::storage::SortedSmallVec; use crate::storage::SparseSetIndex; use crate::world::World; use core::{fmt, fmt::Debug, marker::PhantomData}; use derive_more::derive::From; use disqualified::ShortName; -use fixedbitset::FixedBitSet; -/// A wrapper struct to make Debug representations of [`FixedBitSet`] easier -/// to read, when used to store [`SparseSetIndex`]. -/// -/// Instead of the raw integer representation of the `FixedBitSet`, the list of -/// `T` valid for [`SparseSetIndex`] is shown. -/// -/// Normal `FixedBitSet` `Debug` output: -/// ```text -/// read_and_writes: FixedBitSet { data: [ 160 ], length: 8 } -/// ``` -/// -/// Which, unless you are a computer, doesn't help much understand what's in -/// the set. With `FormattedBitSet`, we convert the present set entries into -/// what they stand for, it is much clearer what is going on: -/// ```text -/// read_and_writes: [ ComponentId(5), ComponentId(7) ] -/// ``` -struct FormattedBitSet<'a, T: SparseSetIndex> { - bit_set: &'a FixedBitSet, - _marker: PhantomData, -} - -impl<'a, T: SparseSetIndex> FormattedBitSet<'a, T> { - fn new(bit_set: &'a FixedBitSet) -> Self { - Self { - bit_set, - _marker: PhantomData, - } - } -} - -impl<'a, T: SparseSetIndex + Debug> Debug for FormattedBitSet<'a, T> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_list() - .entries(self.bit_set.ones().map(T::get_sparse_set_index)) - .finish() - } -} +const ACCESS_SMALL_VEC_SIZE: usize = 8; /// Tracks read and write access to specific elements in a collection. /// @@ -53,14 +16,14 @@ impl<'a, T: SparseSetIndex + Debug> Debug for FormattedBitSet<'a, T> { pub struct Access { /// All accessed components, or forbidden components if /// `Self::component_read_and_writes_inverted` is set. - component_read_and_writes: FixedBitSet, + component_read_and_writes: SortedSmallVec, /// All exclusively-accessed components, or components that may not be /// exclusively accessed if `Self::component_writes_inverted` is set. - component_writes: FixedBitSet, + component_writes: SortedSmallVec, /// All accessed resources. - resource_read_and_writes: FixedBitSet, + resource_read_and_writes: SortedSmallVec, /// The exclusively-accessed resources. - resource_writes: FixedBitSet, + resource_writes: SortedSmallVec, /// Is `true` if this component can read all components *except* those /// present in `Self::component_read_and_writes`. component_read_and_writes_inverted: bool, @@ -74,7 +37,7 @@ pub struct Access { /// If this is true, then `reads_all` must also be true. writes_all_resources: bool, // Components that are not accessed, but whose presence in an archetype affect query results. - archetypal: FixedBitSet, + archetypal: SortedSmallVec, marker: PhantomData, } @@ -113,22 +76,10 @@ impl Clone for Access { impl Debug for Access { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Access") - .field( - "component_read_and_writes", - &FormattedBitSet::::new(&self.component_read_and_writes), - ) - .field( - "component_writes", - &FormattedBitSet::::new(&self.component_writes), - ) - .field( - "resource_read_and_writes", - &FormattedBitSet::::new(&self.resource_read_and_writes), - ) - .field( - "resource_writes", - &FormattedBitSet::::new(&self.resource_writes), - ) + .field("component_read_and_writes", &self.component_read_and_writes) + .field("component_writes", &self.component_writes) + .field("resource_read_and_writes", &self.resource_read_and_writes) + .field("resource_writes", &self.resource_writes) .field( "component_read_and_writes_inverted", &self.component_read_and_writes_inverted, @@ -136,7 +87,7 @@ impl Debug for Access { .field("component_writes_inverted", &self.component_writes_inverted) .field("reads_all_resources", &self.reads_all_resources) .field("writes_all_resources", &self.writes_all_resources) - .field("archetypal", &FormattedBitSet::::new(&self.archetypal)) + .field("archetypal", &self.archetypal) .finish() } } @@ -155,18 +106,18 @@ impl Access { writes_all_resources: false, component_read_and_writes_inverted: false, component_writes_inverted: false, - component_read_and_writes: FixedBitSet::new(), - component_writes: FixedBitSet::new(), - resource_read_and_writes: FixedBitSet::new(), - resource_writes: FixedBitSet::new(), - archetypal: FixedBitSet::new(), + component_read_and_writes: SortedSmallVec::new_const(), + component_writes: SortedSmallVec::new_const(), + resource_read_and_writes: SortedSmallVec::new_const(), + resource_writes: SortedSmallVec::new_const(), + archetypal: SortedSmallVec::new_const(), marker: PhantomData, } } fn add_component_sparse_set_index_read(&mut self, index: usize) { if !self.component_read_and_writes_inverted { - self.component_read_and_writes.grow_and_insert(index); + self.component_read_and_writes.insert(index); } else if index < self.component_read_and_writes.len() { self.component_read_and_writes.remove(index); } @@ -174,7 +125,7 @@ impl Access { fn add_component_sparse_set_index_write(&mut self, index: usize) { if !self.component_writes_inverted { - self.component_writes.grow_and_insert(index); + self.component_writes.insert(index); } else if index < self.component_writes.len() { self.component_writes.remove(index); } @@ -196,20 +147,19 @@ impl Access { /// Adds access to the resource given by `index`. pub fn add_resource_read(&mut self, index: T) { self.resource_read_and_writes - .grow_and_insert(index.sparse_set_index()); + .insert(index.sparse_set_index()); } /// Adds exclusive access to the resource given by `index`. pub fn add_resource_write(&mut self, index: T) { self.resource_read_and_writes - .grow_and_insert(index.sparse_set_index()); - self.resource_writes - .grow_and_insert(index.sparse_set_index()); + .insert(index.sparse_set_index()); + self.resource_writes.insert(index.sparse_set_index()); } fn remove_component_sparse_set_index_read(&mut self, index: usize) { if self.component_read_and_writes_inverted { - self.component_read_and_writes.grow_and_insert(index); + self.component_read_and_writes.insert(index); } else if index < self.component_read_and_writes.len() { self.component_read_and_writes.remove(index); } @@ -217,7 +167,7 @@ impl Access { fn remove_component_sparse_set_index_write(&mut self, index: usize) { if self.component_writes_inverted { - self.component_writes.grow_and_insert(index); + self.component_writes.insert(index); } else if index < self.component_writes.len() { self.component_writes.remove(index); } @@ -259,7 +209,7 @@ impl Access { /// /// [`Has`]: crate::query::Has pub fn add_archetypal(&mut self, index: T) { - self.archetypal.grow_and_insert(index.sparse_set_index()); + self.archetypal.insert(index.sparse_set_index()); } /// Returns `true` if this can access the component given by `index`. @@ -272,7 +222,7 @@ impl Access { /// Returns `true` if this can access any component. pub fn has_any_component_read(&self) -> bool { - self.component_read_and_writes_inverted || !self.component_read_and_writes.is_clear() + self.component_read_and_writes_inverted || !self.component_read_and_writes.is_empty() } /// Returns `true` if this can exclusively access the component given by `index`. @@ -282,7 +232,7 @@ impl Access { /// Returns `true` if this accesses any component mutably. pub fn has_any_component_write(&self) -> bool { - self.component_writes_inverted || !self.component_writes.is_clear() + self.component_writes_inverted || !self.component_writes.is_empty() } /// Returns `true` if this can access the resource given by `index`. @@ -295,7 +245,7 @@ impl Access { /// Returns `true` if this can access any resource. pub fn has_any_resource_read(&self) -> bool { - self.reads_all_resources || !self.resource_read_and_writes.is_clear() + self.reads_all_resources || !self.resource_read_and_writes.is_empty() } /// Returns `true` if this can exclusively access the resource given by `index`. @@ -305,7 +255,7 @@ impl Access { /// Returns `true` if this accesses any resource mutably. pub fn has_any_resource_write(&self) -> bool { - self.writes_all_resources || !self.resource_writes.is_clear() + self.writes_all_resources || !self.resource_writes.is_empty() } /// Returns true if this has an archetypal (indirect) access to the component given by `index`. @@ -365,13 +315,13 @@ impl Access { /// Returns `true` if this has access to all components (i.e. `EntityRef`). #[inline] pub fn has_read_all_components(&self) -> bool { - self.component_read_and_writes_inverted && self.component_read_and_writes.is_clear() + self.component_read_and_writes_inverted && self.component_read_and_writes.is_empty() } /// Returns `true` if this has write access to all components (i.e. `EntityMut`). #[inline] pub fn has_write_all_components(&self) -> bool { - self.component_writes_inverted && self.component_writes.is_clear() + self.component_writes_inverted && self.component_writes.is_empty() } /// Returns `true` if this has access to all resources (i.e. `EntityRef`). @@ -428,23 +378,22 @@ impl Access { other.component_read_and_writes_inverted, ) { (true, true) => { - self.component_read_and_writes - .intersect_with(&other.component_read_and_writes); + self.component_read_and_writes = self + .component_read_and_writes + .intersection(&other.component_read_and_writes) + .into(); } (true, false) => { - self.component_read_and_writes - .difference_with(&other.component_read_and_writes); + self.component_read_and_writes = self + .component_read_and_writes + .difference(&other.component_read_and_writes) + .into(); } (false, true) => { - // We have to grow here because the new bits are going to get flipped to 1. - self.component_read_and_writes.grow( - self.component_read_and_writes - .len() - .max(other.component_read_and_writes.len()), - ); - self.component_read_and_writes.toggle_range(..); - self.component_read_and_writes - .intersect_with(&other.component_read_and_writes); + self.component_read_and_writes = other + .component_read_and_writes + .difference(&self.component_read_and_writes) + .into(); } (false, false) => { self.component_read_and_writes @@ -457,23 +406,22 @@ impl Access { other.component_writes_inverted, ) { (true, true) => { - self.component_writes - .intersect_with(&other.component_writes); + self.component_writes = self + .component_writes + .intersection(&other.component_writes) + .into(); } (true, false) => { - self.component_writes - .difference_with(&other.component_writes); + self.component_writes = self + .component_writes + .difference(&other.component_writes) + .into(); } (false, true) => { - // We have to grow here because the new bits are going to get flipped to 1. - self.component_writes.grow( - self.component_writes - .len() - .max(other.component_writes.len()), - ); - self.component_writes.toggle_range(..); - self.component_writes - .intersect_with(&other.component_writes); + self.component_writes = other + .component_writes + .difference(&self.component_read_and_writes) + .into(); } (false, false) => { self.component_writes.union_with(&other.component_writes); @@ -654,7 +602,7 @@ impl Access { } fn get_component_conflicts(&self, other: &Access) -> AccessConflicts { - let mut conflicts = FixedBitSet::new(); + let mut conflicts = SortedSmallVec::new_const(); // We have a conflict if we write and they read or write, or if they // write and we read or write. @@ -679,12 +627,12 @@ impl Access { ] { // There's no way that I can see to do this without a temporary. // Neither CNF nor DNF allows us to avoid one. - let temp_conflicts: FixedBitSet = + let temp_conflicts = match (lhs_writes_inverted, rhs_reads_and_writes_inverted) { (true, true) => return AccessConflicts::All, - (false, true) => lhs_writes.difference(rhs_reads_and_writes).collect(), - (true, false) => rhs_reads_and_writes.difference(lhs_writes).collect(), - (false, false) => lhs_writes.intersection(rhs_reads_and_writes).collect(), + (false, true) => lhs_writes.difference(rhs_reads_and_writes).into(), + (true, false) => rhs_reads_and_writes.difference(lhs_writes).into(), + (false, false) => lhs_writes.intersection(rhs_reads_and_writes).into(), }; conflicts.union_with(&temp_conflicts); } @@ -820,7 +768,7 @@ impl Access { #[derive(Debug, Eq, PartialEq)] pub struct FilteredAccess { pub(crate) access: Access, - pub(crate) required: FixedBitSet, + pub(crate) required: SortedSmallVec, // An array of filter sets to express `With` or `Without` clauses in disjunctive normal form, for example: `Or<(With, With)>`. // Filters like `(With, Or<(With, Without)>` are expanded into `Or<((With, With), (With, Without))>`. pub(crate) filter_sets: Vec>, @@ -863,7 +811,7 @@ pub enum AccessConflicts { /// Conflict is for all indices All, /// There is a conflict for a subset of indices - Individual(FixedBitSet), + Individual(SortedSmallVec), } impl AccessConflicts { @@ -910,7 +858,7 @@ impl AccessConflicts { /// An [`AccessConflicts`] which represents the absence of any conflict pub(crate) fn empty() -> Self { - Self::Individual(FixedBitSet::new()) + Self::Individual(SortedSmallVec::const_new()) } } diff --git a/crates/bevy_ecs/src/storage/mod.rs b/crates/bevy_ecs/src/storage/mod.rs index eaeff97a8cccb..6c77de23e8f34 100644 --- a/crates/bevy_ecs/src/storage/mod.rs +++ b/crates/bevy_ecs/src/storage/mod.rs @@ -23,11 +23,13 @@ mod blob_array; mod blob_vec; mod resource; +mod sorted_small_vec; mod sparse_set; mod table; mod thin_array_ptr; pub use resource::*; +pub use sorted_small_vec::*; pub use sparse_set::*; pub use table::*; diff --git a/crates/bevy_ecs/src/storage/sorted_small_vec.rs b/crates/bevy_ecs/src/storage/sorted_small_vec.rs new file mode 100644 index 0000000000000..235e6cdc3a91c --- /dev/null +++ b/crates/bevy_ecs/src/storage/sorted_small_vec.rs @@ -0,0 +1,235 @@ +use smallvec::SmallVec; +use std::cmp::Ordering; + +/// Stores a sorted list of indices with quick implementation for union, difference, intersection. +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct SortedSmallVec(SmallVec<[usize; N]>); + +impl IntoIterator for SortedSmallVec { + type Item = usize; + type IntoIter = as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl Default for SortedSmallVec { + fn default() -> Self { + Self::new_const() + } +} + +impl SortedSmallVec { + pub fn new() -> Self { + Self(SmallVec::new()) + } + + pub const fn new_const() -> Self { + Self(SmallVec::new_const()) + } + + pub(crate) fn ones(&self) -> impl Iterator + '_ { + self.0.iter().copied() + } + + /// Insert the value if it's not already present in the vector. + /// Maintains a sorted order. + pub fn insert(&mut self, index: usize) { + match self.0.binary_search(&index) { + // element already present in the vector + Ok(_) => {} + Err(pos) => { + self.0.insert(pos, index); + } + } + } + + /// Returns true if the vector contains the value. + pub fn contains(&self, index: usize) -> bool { + self.0.binary_search(&index).is_ok() + } + + /// Returns true if the vector is empty. + pub(crate) fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Empties the contents of the vector + pub(crate) fn clear(&mut self) { + self.0.clear(); + } + + pub fn len(&self) -> usize { + self.0.len() + } + + /// Adds all the elements from `other` into this vector. (skipping duplicates) + pub fn union_with(&mut self, other: &Self) { + let mut i = 0; + let mut j = 0; + while i < self.len() && j < other.len() { + match self.0[i].cmp(&other.0[j]) { + Ordering::Less => i += 1, + Ordering::Greater => { + self.0.insert(i, other.0[j]); + j += 1; + } + Ordering::Equal => { + i += 1; + j += 1; + } + } + } + while j < other.len() { + self.0.push(other.0[j]); + j += 1; + } + } + + /// Returns the elements that are in both `self` and `other`. + pub fn intersection<'a>(&'a self, other: &'a Self) -> Intersection<'a, N> { + Intersection { + this: self, + other, + i: 0, + j: 0, + } + } + + /// Return the elements that are in `self` but not in `other`. + pub fn difference<'a>(&'a self, other: &'a Self) -> Difference<'a, N> { + Difference { + this: self, + other, + i: 0, + j: 0, + } + } + + /// Returns true if the two vectors have no common elements. + pub fn is_disjoint(&self, other: &Self) -> bool { + self.intersection(other).next().is_none() + } + + /// Returns true if all the elements in `self` are also in `other`. + pub fn is_subset(&self, other: &Self) -> bool { + self.difference(other).next().is_none() + } +} + +impl Extend for SortedSmallVec { + fn extend>(&mut self, other: T) { + let mut i = 0; + let mut other_iter = other.into_iter(); + let mut other_val = other_iter.next(); + while i < self.len() && other_val.is_some() { + let val_j = other_val.unwrap(); + match self.0[i].cmp(&val_j) { + Ordering::Less => { + i += 1; + } + Ordering::Greater => { + self.0.insert(i, val_j); + other_val = other_iter.next(); + } + Ordering::Equal => { + i += 1; + other_val = other_iter.next(); + } + } + } + while let Some(val_j) = other_val { + self.0.push(val_j); + other_val = other_iter.next(); + } + } +} + +/// Intersection between `this` and `other` sorted vectors. +struct Intersection<'a, const N: usize> { + this: &'a SortedSmallVec, + other: &'a SortedSmallVec, + i: usize, + j: usize, +} + +impl<'a, const N: usize> Iterator for Intersection<'a, N> { + type Item = usize; + + fn next(&mut self) -> Option { + let mut res = None; + while self.i < self.this.len() && self.j < self.other.len() { + if (self.i == 0 || self.this.0[self.i] != self.this.0[self.i - 1]) + && self.this.0[self.i] == self.other.0[self.j] + { + res = Some(self.this.0[self.i]); + self.i += 1; + self.j += 1; + return res; + } else if self.this.0[self.i] < self.other.0[self.j] { + self.i += 1; + } else { + self.j += 1; + } + } + res + } +} + +impl<'a, const N: usize> Into> for Intersection<'a, N> { + fn into(self) -> SortedSmallVec { + let mut sorted_vec = SortedSmallVec::new_const(); + for value in self.into_iter() { + sorted_vec.insert(value); + } + sorted_vec + } +} + +/// Difference between `this` and `other` sorted vectors. +struct Difference<'a, const N: usize> { + this: &'a SortedSmallVec, + other: &'a SortedSmallVec, + i: usize, + j: usize, +} + +impl<'a, const N: usize> Iterator for Difference<'a, N> { + type Item = usize; + + fn next(&mut self) -> Option { + let mut res = None; + while self.i < self.this.len() && self.j < self.other.len() { + if self.this.0[self.i] == self.other.0[self.j] { + self.i += 1; + self.j += 1; + } else if (self.i == 0 || self.this.0[self.i] != self.this.0[self.i - 1]) + && self.this.0[self.i] < self.other.0[self.j] + { + res = Some(self.this.0[self.i]); + self.i += 1; + return res; + } else { + self.j += 1; + } + } + if self.i < self.this.len() { + if self.i == 0 || self.this.0[self.i] != self.this.0[self.i - 1] { + res = Some(self.this.0[self.i]); + } + self.i += 1; + } + res + } +} + +impl<'a, const N: usize> Into> for Difference<'a, N> { + fn into(self) -> SortedSmallVec { + let mut sorted_vec = SortedSmallVec::new_const(); + for value in self.into_iter() { + sorted_vec.insert(value); + } + sorted_vec + } +} From c1765f4fd0597e8889bcdde54d680172fc61ceb9 Mon Sep 17 00:00:00 2001 From: Trashtalk Date: Thu, 12 Dec 2024 01:22:28 +0100 Subject: [PATCH 2/4] first working version --- crates/bevy_ecs/src/query/access.rs | 50 ++++++++++--------- .../bevy_ecs/src/storage/sorted_small_vec.rs | 23 ++++++++- 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/crates/bevy_ecs/src/query/access.rs b/crates/bevy_ecs/src/query/access.rs index 813bbed943517..51b16769ac5f9 100644 --- a/crates/bevy_ecs/src/query/access.rs +++ b/crates/bevy_ecs/src/query/access.rs @@ -858,13 +858,17 @@ impl AccessConflicts { /// An [`AccessConflicts`] which represents the absence of any conflict pub(crate) fn empty() -> Self { - Self::Individual(SortedSmallVec::const_new()) + Self::Individual(SortedSmallVec::new_const()) } } impl From> for AccessConflicts { fn from(value: Vec) -> Self { - Self::Individual(value.iter().map(T::sparse_set_index).collect()) + let mut conflicts = SortedSmallVec::new_const(); + for index in value.iter().map(|a| T::sparse_set_index(a)) { + conflicts.insert(index); + } + Self::Individual(conflicts) } } @@ -874,7 +878,7 @@ impl FilteredAccess { pub fn matches_everything() -> Self { Self { access: Access::default(), - required: FixedBitSet::default(), + required: SortedSmallVec::new_const(), filter_sets: vec![AccessFilters::default()], } } @@ -884,7 +888,7 @@ impl FilteredAccess { pub fn matches_nothing() -> Self { Self { access: Access::default(), - required: FixedBitSet::default(), + required: SortedSmallVec::new_const(), filter_sets: Vec::new(), } } @@ -926,7 +930,7 @@ impl FilteredAccess { } fn add_required(&mut self, index: T) { - self.required.grow_and_insert(index.sparse_set_index()); + self.required.insert(index.sparse_set_index()); } /// Adds a `With` filter: corresponds to a conjunction (AND) operation. @@ -935,7 +939,7 @@ impl FilteredAccess { /// Adding `AND With` via this method transforms it into the equivalent of `Or<((With, With), (With, With))>`. pub fn and_with(&mut self, index: T) { for filter in &mut self.filter_sets { - filter.with.grow_and_insert(index.sparse_set_index()); + filter.with.insert(index.sparse_set_index()); } } @@ -945,7 +949,7 @@ impl FilteredAccess { /// Adding `AND Without` via this method transforms it into the equivalent of `Or<((With, Without), (With, Without))>`. pub fn and_without(&mut self, index: T) { for filter in &mut self.filter_sets { - filter.without.grow_and_insert(index.sparse_set_index()); + filter.without.insert(index.sparse_set_index()); } } @@ -1068,8 +1072,8 @@ impl FilteredAccess { #[derive(Eq, PartialEq)] pub(crate) struct AccessFilters { - pub(crate) with: FixedBitSet, - pub(crate) without: FixedBitSet, + pub(crate) with: SortedSmallVec, + pub(crate) without: SortedSmallVec, _index_type: PhantomData, } @@ -1092,8 +1096,8 @@ impl Clone for AccessFilters { impl Debug for AccessFilters { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("AccessFilters") - .field("with", &FormattedBitSet::::new(&self.with)) - .field("without", &FormattedBitSet::::new(&self.without)) + .field("with", &self.with) + .field("without", &self.without) .finish() } } @@ -1101,8 +1105,8 @@ impl Debug for AccessFilters { impl Default for AccessFilters { fn default() -> Self { Self { - with: FixedBitSet::default(), - without: FixedBitSet::default(), + with: SortedSmallVec::new_const(), + without: SortedSmallVec::new_const(), _index_type: PhantomData, } } @@ -1276,9 +1280,9 @@ impl Default for FilteredAccessSet { #[cfg(test)] mod tests { - use crate::query::{ + use crate::{query::{ access::AccessFilters, Access, AccessConflicts, FilteredAccess, FilteredAccessSet, - }; + }, storage::SortedSmallVec}; use core::marker::PhantomData; use fixedbitset::FixedBitSet; @@ -1308,8 +1312,8 @@ mod tests { fn create_sample_access_filters() -> AccessFilters { let mut access_filters = AccessFilters::::default(); - access_filters.with.grow_and_insert(3); - access_filters.without.grow_and_insert(5); + access_filters.with.insert(3); + access_filters.without.insert(5); access_filters } @@ -1382,8 +1386,8 @@ mod tests { let original: AccessFilters = create_sample_access_filters(); let mut cloned = AccessFilters::::default(); - cloned.with.grow_and_insert(1); - cloned.without.grow_and_insert(2); + cloned.with.insert(1); + cloned.without.insert(2); cloned.clone_from(&original); @@ -1537,13 +1541,13 @@ mod tests { // The resulted access is expected to represent `Or<((With, With, With), (With, With, With, Without))>`. expected.filter_sets = vec![ AccessFilters { - with: FixedBitSet::with_capacity_and_blocks(3, [0b111]), - without: FixedBitSet::default(), + with: SortedSmallVec::from_vec(vec![0, 1, 2]), + without: SortedSmallVec::new_const(), _index_type: PhantomData, }, AccessFilters { - with: FixedBitSet::with_capacity_and_blocks(4, [0b1011]), - without: FixedBitSet::with_capacity_and_blocks(5, [0b10000]), + with: SortedSmallVec::from_vec(vec![0, 1, 3]), + without: SortedSmallVec::from_vec(vec![4]), _index_type: PhantomData, }, ]; diff --git a/crates/bevy_ecs/src/storage/sorted_small_vec.rs b/crates/bevy_ecs/src/storage/sorted_small_vec.rs index 235e6cdc3a91c..02a3d05ae3217 100644 --- a/crates/bevy_ecs/src/storage/sorted_small_vec.rs +++ b/crates/bevy_ecs/src/storage/sorted_small_vec.rs @@ -29,6 +29,14 @@ impl SortedSmallVec { Self(SmallVec::new_const()) } + pub fn from_vec(vec: Vec) -> Self { + let mut sorted_vec = Self(SmallVec::with_capacity(vec.len())); + for value in vec { + sorted_vec.insert(value); + } + sorted_vec + } + pub(crate) fn ones(&self) -> impl Iterator + '_ { self.0.iter().copied() } @@ -45,6 +53,17 @@ impl SortedSmallVec { } } + /// Removes a value if it's present in the vector + pub fn remove(&mut self, index: usize) { + match self.0.binary_search(&index) { + Ok(pos) => { + self.0.remove(pos); + } + // element is not there + Err(_) => {} + } + } + /// Returns true if the vector contains the value. pub fn contains(&self, index: usize) -> bool { self.0.binary_search(&index).is_ok() @@ -147,7 +166,7 @@ impl Extend for SortedSmallVec { } /// Intersection between `this` and `other` sorted vectors. -struct Intersection<'a, const N: usize> { +pub struct Intersection<'a, const N: usize> { this: &'a SortedSmallVec, other: &'a SortedSmallVec, i: usize, @@ -188,7 +207,7 @@ impl<'a, const N: usize> Into> for Intersection<'a, N> { } /// Difference between `this` and `other` sorted vectors. -struct Difference<'a, const N: usize> { +pub struct Difference<'a, const N: usize> { this: &'a SortedSmallVec, other: &'a SortedSmallVec, i: usize, From 0dacf05a521b92f29cfeda286f123c12f9b5f805 Mon Sep 17 00:00:00 2001 From: Trashtalk Date: Thu, 12 Dec 2024 13:00:23 +0100 Subject: [PATCH 3/4] added documentation --- crates/bevy_ecs/src/storage/sorted_small_vec.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/bevy_ecs/src/storage/sorted_small_vec.rs b/crates/bevy_ecs/src/storage/sorted_small_vec.rs index 02a3d05ae3217..151de9b6cff6e 100644 --- a/crates/bevy_ecs/src/storage/sorted_small_vec.rs +++ b/crates/bevy_ecs/src/storage/sorted_small_vec.rs @@ -21,14 +21,22 @@ impl Default for SortedSmallVec { } impl SortedSmallVec { + /// Construct an empty vector pub fn new() -> Self { Self(SmallVec::new()) } + /// Construct an empty vector + /// + /// This is a `const` version of [`SortedSmallVec::new()`] pub const fn new_const() -> Self { Self(SmallVec::new_const()) } + /// Construct a new `SortedSmallVec` from a `Vec`. + /// + /// Elements are copied and put in a sorted order if the original `Vec` isn't ordered. + /// Duplicates are removed. pub fn from_vec(vec: Vec) -> Self { let mut sorted_vec = Self(SmallVec::with_capacity(vec.len())); for value in vec { @@ -79,6 +87,7 @@ impl SortedSmallVec { self.0.clear(); } + /// Returns the number of elements in the vector. pub fn len(&self) -> usize { self.0.len() } From 1f2441f3061bccf96e394f6395e99c5fbdbfcc1f Mon Sep 17 00:00:00 2001 From: Trashtalk Date: Thu, 12 Dec 2024 13:15:30 +0100 Subject: [PATCH 4/4] made clippy suggestions --- .../bevy_ecs/src/storage/sorted_small_vec.rs | 48 +++++++++---------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/crates/bevy_ecs/src/storage/sorted_small_vec.rs b/crates/bevy_ecs/src/storage/sorted_small_vec.rs index 151de9b6cff6e..b5ac914e8fc66 100644 --- a/crates/bevy_ecs/src/storage/sorted_small_vec.rs +++ b/crates/bevy_ecs/src/storage/sorted_small_vec.rs @@ -1,5 +1,5 @@ use smallvec::SmallVec; -use std::cmp::Ordering; +use core::cmp::Ordering; /// Stores a sorted list of indices with quick implementation for union, difference, intersection. #[derive(Debug, Clone, Eq, PartialEq)] @@ -29,7 +29,7 @@ impl SortedSmallVec { /// Construct an empty vector /// /// This is a `const` version of [`SortedSmallVec::new()`] - pub const fn new_const() -> Self { + pub(crate) const fn new_const() -> Self { Self(SmallVec::new_const()) } @@ -51,7 +51,7 @@ impl SortedSmallVec { /// Insert the value if it's not already present in the vector. /// Maintains a sorted order. - pub fn insert(&mut self, index: usize) { + pub(crate) fn insert(&mut self, index: usize) { match self.0.binary_search(&index) { // element already present in the vector Ok(_) => {} @@ -62,18 +62,14 @@ impl SortedSmallVec { } /// Removes a value if it's present in the vector - pub fn remove(&mut self, index: usize) { - match self.0.binary_search(&index) { - Ok(pos) => { - self.0.remove(pos); - } - // element is not there - Err(_) => {} + pub(crate) fn remove(&mut self, index: usize) { + if let Ok(pos) = self.0.binary_search(&index) { + self.0.remove(pos); } - } + } /// Returns true if the vector contains the value. - pub fn contains(&self, index: usize) -> bool { + pub(crate) fn contains(&self, index: usize) -> bool { self.0.binary_search(&index).is_ok() } @@ -88,12 +84,12 @@ impl SortedSmallVec { } /// Returns the number of elements in the vector. - pub fn len(&self) -> usize { + pub(crate) fn len(&self) -> usize { self.0.len() } /// Adds all the elements from `other` into this vector. (skipping duplicates) - pub fn union_with(&mut self, other: &Self) { + pub(crate) fn union_with(&mut self, other: &Self) { let mut i = 0; let mut j = 0; while i < self.len() && j < other.len() { @@ -116,7 +112,7 @@ impl SortedSmallVec { } /// Returns the elements that are in both `self` and `other`. - pub fn intersection<'a>(&'a self, other: &'a Self) -> Intersection<'a, N> { + pub(crate) fn intersection<'a>(&'a self, other: &'a Self) -> Intersection<'a, N> { Intersection { this: self, other, @@ -126,7 +122,7 @@ impl SortedSmallVec { } /// Return the elements that are in `self` but not in `other`. - pub fn difference<'a>(&'a self, other: &'a Self) -> Difference<'a, N> { + pub(crate) fn difference<'a>(&'a self, other: &'a Self) -> Difference<'a, N> { Difference { this: self, other, @@ -136,12 +132,12 @@ impl SortedSmallVec { } /// Returns true if the two vectors have no common elements. - pub fn is_disjoint(&self, other: &Self) -> bool { + pub(crate) fn is_disjoint(&self, other: &Self) -> bool { self.intersection(other).next().is_none() } /// Returns true if all the elements in `self` are also in `other`. - pub fn is_subset(&self, other: &Self) -> bool { + pub(crate) fn is_subset(&self, other: &Self) -> bool { self.difference(other).next().is_none() } } @@ -175,7 +171,7 @@ impl Extend for SortedSmallVec { } /// Intersection between `this` and `other` sorted vectors. -pub struct Intersection<'a, const N: usize> { +pub(crate) struct Intersection<'a, const N: usize> { this: &'a SortedSmallVec, other: &'a SortedSmallVec, i: usize, @@ -205,10 +201,10 @@ impl<'a, const N: usize> Iterator for Intersection<'a, N> { } } -impl<'a, const N: usize> Into> for Intersection<'a, N> { - fn into(self) -> SortedSmallVec { +impl<'a, const N: usize> From> for SortedSmallVec { + fn from(intersection: Intersection<'a, N>) -> Self { let mut sorted_vec = SortedSmallVec::new_const(); - for value in self.into_iter() { + for value in intersection.into_iter() { sorted_vec.insert(value); } sorted_vec @@ -216,7 +212,7 @@ impl<'a, const N: usize> Into> for Intersection<'a, N> { } /// Difference between `this` and `other` sorted vectors. -pub struct Difference<'a, const N: usize> { +pub(crate) struct Difference<'a, const N: usize> { this: &'a SortedSmallVec, other: &'a SortedSmallVec, i: usize, @@ -252,10 +248,10 @@ impl<'a, const N: usize> Iterator for Difference<'a, N> { } } -impl<'a, const N: usize> Into> for Difference<'a, N> { - fn into(self) -> SortedSmallVec { +impl<'a, const N: usize> From> for SortedSmallVec { + fn from(difference: Difference<'a, N>) -> Self { let mut sorted_vec = SortedSmallVec::new_const(); - for value in self.into_iter() { + for value in difference.into_iter() { sorted_vec.insert(value); } sorted_vec