Skip to content

Commit

Permalink
Separate Query filter access from fetch access during initial evaluat…
Browse files Browse the repository at this point in the history
…ion (bevyengine#1977)

Fixes bevyengine#1955 

See this comment for implementation details / motivation: bevyengine#1955 (comment)
  • Loading branch information
cart authored and ostwilkens committed Jul 27, 2021
1 parent 0d34b0e commit b3cb81b
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 6 deletions.
28 changes: 26 additions & 2 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ pub mod prelude {
mod tests {
use crate::{
bundle::Bundle,
component::{Component, ComponentDescriptor, StorageType, TypeInfo},
component::{Component, ComponentDescriptor, ComponentId, StorageType, TypeInfo},
entity::Entity,
query::{Added, ChangeTrackers, Changed, FilterFetch, With, Without, WorldQuery},
query::{
Added, ChangeTrackers, Changed, FilterFetch, FilteredAccess, With, Without, WorldQuery,
},
world::{Mut, World},
};
use bevy_tasks::TaskPool;
Expand Down Expand Up @@ -1118,6 +1120,28 @@ mod tests {
query.iter(&world_b);
}

#[test]
fn query_filters_dont_collide_with_fetches() {
let mut world = World::new();
world.query_filtered::<&mut i32, Changed<i32>>();
}

#[test]
fn filtered_query_access() {
let mut world = World::new();
let query = world.query_filtered::<&mut i32, Changed<f64>>();

let mut expected = FilteredAccess::<ComponentId>::default();
let i32_id = world.components.get_id(TypeId::of::<i32>()).unwrap();
let f64_id = world.components.get_id(TypeId::of::<f64>()).unwrap();
expected.add_write(i32_id);
expected.add_read(f64_id);
assert!(
query.component_access.eq(&expected),
"ComponentId access from query fetch and query filter should be combined"
);
}

#[test]
#[should_panic]
fn multiple_worlds_same_query_get() {
Expand Down
34 changes: 32 additions & 2 deletions crates/bevy_ecs/src/query/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl<T: SparseSetIndex> Access<T> {
}
}

#[derive(Clone)]
#[derive(Clone, Eq, PartialEq)]
pub struct FilteredAccess<T: SparseSetIndex> {
access: Access<T>,
with: FixedBitSet,
Expand Down Expand Up @@ -152,6 +152,12 @@ impl<T: SparseSetIndex> FilteredAccess<T> {
|| self.without.intersection(&other.with).next().is_some()
}
}

pub fn extend(&mut self, access: &FilteredAccess<T>) {
self.access.extend(&access.access);
self.with.union_with(&access.with);
self.without.union_with(&access.without);
}
}

pub struct FilteredAccessSet<T: SparseSetIndex> {
Expand Down Expand Up @@ -202,7 +208,7 @@ impl<T: SparseSetIndex> Default for FilteredAccessSet<T> {

#[cfg(test)]
mod tests {
use crate::query::Access;
use crate::query::{Access, FilteredAccess};

#[test]
fn access_get_conflicts() {
Expand Down Expand Up @@ -230,4 +236,28 @@ mod tests {
assert_eq!(access_d.get_conflicts(&access_b), vec![]);
assert_eq!(access_d.get_conflicts(&access_c), vec![0]);
}

#[test]
fn filtered_access_extend() {
let mut access_a = FilteredAccess::<usize>::default();
access_a.add_read(0);
access_a.add_read(1);
access_a.add_with(2);

let mut access_b = FilteredAccess::<usize>::default();
access_b.add_read(0);
access_b.add_write(3);
access_b.add_without(4);

access_a.extend(&access_b);

let mut expected = FilteredAccess::<usize>::default();
expected.add_read(0);
expected.add_read(1);
expected.add_with(2);
expected.add_write(3);
expected.add_without(4);

assert!(access_a.eq(&expected));
}
}
15 changes: 13 additions & 2 deletions crates/bevy_ecs/src/query/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,20 @@ where
pub fn new(world: &mut World) -> Self {
let fetch_state = <Q::State as FetchState>::init(world);
let filter_state = <F::State as FetchState>::init(world);
let mut component_access = Default::default();

let mut component_access = FilteredAccess::default();
fetch_state.update_component_access(&mut component_access);
filter_state.update_component_access(&mut component_access);

// Use a temporary empty FilteredAccess for filters. This prevents them from conflicting with the
// main Query's `fetch_state` access. Filters are allowed to conflict with the main query fetch
// because they are evaluated *before* a specific reference is constructed.
let mut filter_component_access = FilteredAccess::default();
filter_state.update_component_access(&mut filter_component_access);

// Merge the temporary filter access with the main access. This ensures that filter access is
// properly considered in a global "cross-query" context (both within systems and across systems).
component_access.extend(&filter_component_access);

let mut state = Self {
world_id: world.id(),
archetype_generation: ArchetypeGeneration::new(usize::MAX),
Expand Down

0 comments on commit b3cb81b

Please sign in to comment.