From d5ea5c9a647688306a09649a5d0edc5b5b3bca55 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sun, 22 Sep 2024 17:32:05 -0400 Subject: [PATCH 1/2] Borrow Access in FilteredEntity(Ref|Mut) to avoid cloning. --- crates/bevy_ecs/src/query/fetch.rs | 82 +++++------ crates/bevy_ecs/src/reflect/component.rs | 17 ++- crates/bevy_ecs/src/world/entity_ref.rs | 173 +++++++++++------------ 3 files changed, 127 insertions(+), 145 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 0e5aabf16890c..534b0f833bfa5 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -661,10 +661,10 @@ impl ReleaseStateQueryData for EntityMut<'_> { } /// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self` -unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { - type Fetch<'w, 's> = (UnsafeWorldCell<'w>, Access); - type Item<'w, 's> = FilteredEntityRef<'w>; - type State = FilteredAccess; +unsafe impl WorldQuery for FilteredEntityRef<'_, '_> { + type Fetch<'w, 's> = (UnsafeWorldCell<'w>, &'s Access); + type Item<'w, 's> = FilteredEntityRef<'w, 's>; + type State = Access; fn shrink<'wlong: 'wshort, 'wshort, 's>( item: Self::Item<'wlong, 's>, @@ -682,38 +682,34 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { unsafe fn init_fetch<'w, 's>( world: UnsafeWorldCell<'w>, - _state: &'s Self::State, + state: &'s Self::State, _last_run: Tick, _this_run: Tick, ) -> Self::Fetch<'w, 's> { - let mut access = Access::default(); - access.read_all_components(); - (world, access) + (world, state) } #[inline] unsafe fn set_archetype<'w, 's>( - fetch: &mut Self::Fetch<'w, 's>, - state: &'s Self::State, + _fetch: &mut Self::Fetch<'w, 's>, + _state: &'s Self::State, _: &'w Archetype, _table: &Table, ) { - fetch.1.clone_from(&state.access); } #[inline] unsafe fn set_table<'w, 's>( - fetch: &mut Self::Fetch<'w, 's>, - state: &'s Self::State, + _fetch: &mut Self::Fetch<'w, 's>, + _state: &'s Self::State, _: &'w Table, ) { - fetch.1.clone_from(&state.access); } #[inline] fn set_access<'w>(state: &mut Self::State, access: &FilteredAccess) { - state.clone_from(access); - state.access_mut().clear_writes(); + state.clone_from(&access.access); + state.clear_writes(); } #[inline(always)] @@ -725,7 +721,7 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { world.get_entity(entity).debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { FilteredEntityRef::new(cell, access.clone()) } + unsafe { FilteredEntityRef::new(cell, access) } } fn update_component_access( @@ -733,18 +729,18 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { filtered_access: &mut FilteredAccess, ) { assert!( - filtered_access.access().is_compatible(&state.access), + filtered_access.access().is_compatible(state), "FilteredEntityRef conflicts with a previous access in this query. Exclusive access cannot coincide with any other accesses.", ); - filtered_access.access.extend(&state.access); + filtered_access.access.extend(state); } fn init_state(_world: &mut World) -> Self::State { - FilteredAccess::default() + Access::default() } fn get_state(_components: &Components) -> Option { - Some(FilteredAccess::default()) + Some(Access::default()) } fn matches_component_set( @@ -756,18 +752,18 @@ unsafe impl<'a> WorldQuery for FilteredEntityRef<'a> { } /// SAFETY: `Self` is the same as `Self::ReadOnly` -unsafe impl<'a> QueryData for FilteredEntityRef<'a> { +unsafe impl QueryData for FilteredEntityRef<'_, '_> { type ReadOnly = Self; } /// SAFETY: Access is read-only. -unsafe impl ReadOnlyQueryData for FilteredEntityRef<'_> {} +unsafe impl ReadOnlyQueryData for FilteredEntityRef<'_, '_> {} /// SAFETY: The accesses of `Self::ReadOnly` are a subset of the accesses of `Self` -unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { - type Fetch<'w, 's> = (UnsafeWorldCell<'w>, Access); - type Item<'w, 's> = FilteredEntityMut<'w>; - type State = FilteredAccess; +unsafe impl WorldQuery for FilteredEntityMut<'_, '_> { + type Fetch<'w, 's> = (UnsafeWorldCell<'w>, &'s Access); + type Item<'w, 's> = FilteredEntityMut<'w, 's>; + type State = Access; fn shrink<'wlong: 'wshort, 'wshort, 's>( item: Self::Item<'wlong, 's>, @@ -785,37 +781,33 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { unsafe fn init_fetch<'w, 's>( world: UnsafeWorldCell<'w>, - _state: &'s Self::State, + state: &'s Self::State, _last_run: Tick, _this_run: Tick, ) -> Self::Fetch<'w, 's> { - let mut access = Access::default(); - access.write_all_components(); - (world, access) + (world, state) } #[inline] unsafe fn set_archetype<'w, 's>( - fetch: &mut Self::Fetch<'w, 's>, - state: &'s Self::State, + _fetch: &mut Self::Fetch<'w, 's>, + _state: &'s Self::State, _: &'w Archetype, _table: &Table, ) { - fetch.1.clone_from(&state.access); } #[inline] unsafe fn set_table<'w, 's>( - fetch: &mut Self::Fetch<'w, 's>, - state: &'s Self::State, + _fetch: &mut Self::Fetch<'w, 's>, + _state: &'s Self::State, _: &'w Table, ) { - fetch.1.clone_from(&state.access); } #[inline] fn set_access<'w>(state: &mut Self::State, access: &FilteredAccess) { - state.clone_from(access); + state.clone_from(&access.access); } #[inline(always)] @@ -827,7 +819,7 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { world.get_entity(entity).debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - unsafe { FilteredEntityMut::new(cell, access.clone()) } + unsafe { FilteredEntityMut::new(cell, access) } } fn update_component_access( @@ -835,18 +827,18 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { filtered_access: &mut FilteredAccess, ) { assert!( - filtered_access.access().is_compatible(&state.access), + filtered_access.access().is_compatible(state), "FilteredEntityMut conflicts with a previous access in this query. Exclusive access cannot coincide with any other accesses.", ); - filtered_access.access.extend(&state.access); + filtered_access.access.extend(state); } fn init_state(_world: &mut World) -> Self::State { - FilteredAccess::default() + Access::default() } fn get_state(_components: &Components) -> Option { - Some(FilteredAccess::default()) + Some(Access::default()) } fn matches_component_set( @@ -858,8 +850,8 @@ unsafe impl<'a> WorldQuery for FilteredEntityMut<'a> { } /// SAFETY: access of `FilteredEntityRef` is a subset of `FilteredEntityMut` -unsafe impl<'a> QueryData for FilteredEntityMut<'a> { - type ReadOnly = FilteredEntityRef<'a>; +unsafe impl<'w, 's> QueryData for FilteredEntityMut<'w, 's> { + type ReadOnly = FilteredEntityRef<'w, 's>; } /// SAFETY: `EntityRefExcept` guards access to all components in the bundle `B` diff --git a/crates/bevy_ecs/src/reflect/component.rs b/crates/bevy_ecs/src/reflect/component.rs index b3422208d1f4b..7bf546565a37f 100644 --- a/crates/bevy_ecs/src/reflect/component.rs +++ b/crates/bevy_ecs/src/reflect/component.rs @@ -109,9 +109,9 @@ pub struct ReflectComponentFns { /// Function pointer implementing [`ReflectComponent::contains()`]. pub contains: fn(FilteredEntityRef) -> bool, /// Function pointer implementing [`ReflectComponent::reflect()`]. - pub reflect: fn(FilteredEntityRef) -> Option<&dyn Reflect>, + pub reflect: for<'w> fn(FilteredEntityRef<'w, '_>) -> Option<&'w dyn Reflect>, /// Function pointer implementing [`ReflectComponent::reflect_mut()`]. - pub reflect_mut: fn(FilteredEntityMut) -> Option>, + pub reflect_mut: for<'w> fn(FilteredEntityMut<'w, '_>) -> Option>, /// Function pointer implementing [`ReflectComponent::reflect_unchecked_mut()`]. /// /// # Safety @@ -168,20 +168,23 @@ impl ReflectComponent { } /// Returns whether entity contains this [`Component`] - pub fn contains<'a>(&self, entity: impl Into>) -> bool { + pub fn contains<'w, 's>(&self, entity: impl Into>) -> bool { (self.0.contains)(entity.into()) } /// Gets the value of this [`Component`] type from the entity as a reflected reference. - pub fn reflect<'a>(&self, entity: impl Into>) -> Option<&'a dyn Reflect> { + pub fn reflect<'w, 's>( + &self, + entity: impl Into>, + ) -> Option<&'w dyn Reflect> { (self.0.reflect)(entity.into()) } /// Gets the value of this [`Component`] type from the entity as a mutable reflected reference. - pub fn reflect_mut<'a>( + pub fn reflect_mut<'w, 's>( &self, - entity: impl Into>, - ) -> Option> { + entity: impl Into>, + ) -> Option> { (self.0.reflect_mut)(entity.into()) } diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index a5329e50a5296..28d9c7691e465 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -13,7 +13,7 @@ use crate::{ world::{DeferredWorld, Mut, World}, }; use bevy_ptr::{OwningPtr, Ptr}; -use std::{any::TypeId, marker::PhantomData}; +use std::{any::TypeId, marker::PhantomData, sync::OnceLock}; use thiserror::Error; use super::{unsafe_world_cell::UnsafeEntityCell, Ref, ON_REMOVE, ON_REPLACE}; @@ -210,10 +210,10 @@ impl<'a> From<&'a EntityMut<'_>> for EntityRef<'a> { } } -impl<'a> TryFrom> for EntityRef<'a> { +impl<'a> TryFrom> for EntityRef<'a> { type Error = TryFromFilteredError; - fn try_from(value: FilteredEntityRef<'a>) -> Result { + fn try_from(value: FilteredEntityRef<'a, '_>) -> Result { if !value.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else { @@ -223,10 +223,10 @@ impl<'a> TryFrom> for EntityRef<'a> { } } -impl<'a> TryFrom<&'a FilteredEntityRef<'_>> for EntityRef<'a> { +impl<'a> TryFrom<&'a FilteredEntityRef<'_, '_>> for EntityRef<'a> { type Error = TryFromFilteredError; - fn try_from(value: &'a FilteredEntityRef<'_>) -> Result { + fn try_from(value: &'a FilteredEntityRef<'_, '_>) -> Result { if !value.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else { @@ -236,10 +236,10 @@ impl<'a> TryFrom<&'a FilteredEntityRef<'_>> for EntityRef<'a> { } } -impl<'a> TryFrom> for EntityRef<'a> { +impl<'a> TryFrom> for EntityRef<'a> { type Error = TryFromFilteredError; - fn try_from(value: FilteredEntityMut<'a>) -> Result { + fn try_from(value: FilteredEntityMut<'a, '_>) -> Result { if !value.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else { @@ -249,10 +249,10 @@ impl<'a> TryFrom> for EntityRef<'a> { } } -impl<'a> TryFrom<&'a FilteredEntityMut<'_>> for EntityRef<'a> { +impl<'a> TryFrom<&'a FilteredEntityMut<'_, '_>> for EntityRef<'a> { type Error = TryFromFilteredError; - fn try_from(value: &'a FilteredEntityMut<'_>) -> Result { + fn try_from(value: &'a FilteredEntityMut<'_, '_>) -> Result { if !value.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else { @@ -528,10 +528,10 @@ impl<'a> From<&'a mut EntityWorldMut<'_>> for EntityMut<'a> { } } -impl<'a> TryFrom> for EntityMut<'a> { +impl<'a> TryFrom> for EntityMut<'a> { type Error = TryFromFilteredError; - fn try_from(value: FilteredEntityMut<'a>) -> Result { + fn try_from(value: FilteredEntityMut<'a, '_>) -> Result { if !value.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else if !value.access.has_write_all() { @@ -543,10 +543,10 @@ impl<'a> TryFrom> for EntityMut<'a> { } } -impl<'a> TryFrom<&'a mut FilteredEntityMut<'_>> for EntityMut<'a> { +impl<'a> TryFrom<&'a mut FilteredEntityMut<'_, '_>> for EntityMut<'a> { type Error = TryFromFilteredError; - fn try_from(value: &'a mut FilteredEntityMut<'_>) -> Result { + fn try_from(value: &'a mut FilteredEntityMut<'_, '_>) -> Result { if !value.access.has_read_all() { Err(TryFromFilteredError::MissingReadAllAccess) } else if !value.access.has_write_all() { @@ -1900,19 +1900,22 @@ impl<'w, 'a, T: Component> VacantEntry<'w, 'a, T> { /// Provides read-only access to a single entity and some of its components defined by the contained [`Access`]. #[derive(Clone)] -pub struct FilteredEntityRef<'w> { +pub struct FilteredEntityRef<'w, 's> { entity: UnsafeEntityCell<'w>, - access: Access, + access: &'s Access, } -impl<'w> FilteredEntityRef<'w> { +impl<'w, 's> FilteredEntityRef<'w, 's> { /// # Safety /// - No `&mut World` can exist from the underlying `UnsafeWorldCell` /// - If `access` takes read access to a component no mutable reference to that /// component can exist at the same time as the returned [`FilteredEntityMut`] /// - If `access` takes any access for a component `entity` must have that component. #[inline] - pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>, access: Access) -> Self { + pub(crate) unsafe fn new( + entity: UnsafeEntityCell<'w>, + access: &'s Access, + ) -> Self { Self { entity, access } } @@ -1938,7 +1941,7 @@ impl<'w> FilteredEntityRef<'w> { /// Returns a reference to the underlying [`Access`]. #[inline] pub fn access(&self) -> &Access { - &self.access + self.access } /// Returns `true` if the current entity has a component of type `T`. @@ -2049,103 +2052,94 @@ impl<'w> FilteredEntityRef<'w> { } } -impl<'w> From> for FilteredEntityRef<'w> { +impl<'w, 's> From> for FilteredEntityRef<'w, 's> { #[inline] - fn from(entity_mut: FilteredEntityMut<'w>) -> Self { + fn from(entity_mut: FilteredEntityMut<'w, 's>) -> Self { // SAFETY: // - `FilteredEntityMut` guarantees exclusive access to all components in the new `FilteredEntityRef`. unsafe { FilteredEntityRef::new(entity_mut.entity, entity_mut.access) } } } -impl<'a> From<&'a FilteredEntityMut<'_>> for FilteredEntityRef<'a> { +impl<'a, 's> From<&'a FilteredEntityMut<'_, 's>> for FilteredEntityRef<'a, 's> { #[inline] - fn from(entity_mut: &'a FilteredEntityMut<'_>) -> Self { + fn from(entity_mut: &'a FilteredEntityMut<'_, 's>) -> Self { // SAFETY: // - `FilteredEntityMut` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { FilteredEntityRef::new(entity_mut.entity, entity_mut.access.clone()) } + unsafe { FilteredEntityRef::new(entity_mut.entity, entity_mut.access) } } } -impl<'a> From> for FilteredEntityRef<'a> { +fn read_all_components() -> &'static Access { + static READ_ALL_COMPONENTS: OnceLock> = OnceLock::new(); + + READ_ALL_COMPONENTS.get_or_init(|| { + let mut access = Access::new(); + access.read_all_components(); + access + }) +} + +impl<'a> From> for FilteredEntityRef<'a, 'static> { fn from(entity: EntityRef<'a>) -> Self { // SAFETY: // - `EntityRef` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.0, access) - } + unsafe { FilteredEntityRef::new(entity.0, read_all_components()) } } } -impl<'a> From<&'a EntityRef<'_>> for FilteredEntityRef<'a> { +impl<'a> From<&'a EntityRef<'_>> for FilteredEntityRef<'a, 'static> { fn from(entity: &'a EntityRef<'_>) -> Self { // SAFETY: // - `EntityRef` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.0, access) - } + unsafe { FilteredEntityRef::new(entity.0, read_all_components()) } } } -impl<'a> From> for FilteredEntityRef<'a> { +impl<'a> From> for FilteredEntityRef<'a, 'static> { fn from(entity: EntityMut<'a>) -> Self { // SAFETY: // - `EntityMut` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.0, access) - } + unsafe { FilteredEntityRef::new(entity.0, read_all_components()) } } } -impl<'a> From<&'a EntityMut<'_>> for FilteredEntityRef<'a> { +impl<'a> From<&'a EntityMut<'_>> for FilteredEntityRef<'a, 'static> { fn from(entity: &'a EntityMut<'_>) -> Self { // SAFETY: // - `EntityMut` guarantees exclusive access to all components in the new `FilteredEntityRef`. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.0, access) - } + unsafe { FilteredEntityRef::new(entity.0, read_all_components()) } } } -impl<'a> From> for FilteredEntityRef<'a> { +impl<'a> From> for FilteredEntityRef<'a, 'static> { fn from(entity: EntityWorldMut<'a>) -> Self { // SAFETY: // - `EntityWorldMut` guarantees exclusive access to the entire world. - unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.into_unsafe_entity_cell(), access) - } + unsafe { FilteredEntityRef::new(entity.into_unsafe_entity_cell(), read_all_components()) } } } -impl<'a> From<&'a EntityWorldMut<'_>> for FilteredEntityRef<'a> { +impl<'a> From<&'a EntityWorldMut<'_>> for FilteredEntityRef<'a, 'static> { fn from(entity: &'a EntityWorldMut<'_>) -> Self { // SAFETY: // - `EntityWorldMut` guarantees exclusive access to the entire world. unsafe { - let mut access = Access::default(); - access.read_all(); - FilteredEntityRef::new(entity.as_unsafe_entity_cell_readonly(), access) + FilteredEntityRef::new( + entity.as_unsafe_entity_cell_readonly(), + read_all_components(), + ) } } } /// Provides mutable access to a single entity and some of its components defined by the contained [`Access`]. -pub struct FilteredEntityMut<'w> { +pub struct FilteredEntityMut<'w, 's> { entity: UnsafeEntityCell<'w>, - access: Access, + access: &'s Access, } -impl<'w> FilteredEntityMut<'w> { +impl<'w, 's> FilteredEntityMut<'w, 's> { /// # Safety /// - No `&mut World` can exist from the underlying `UnsafeWorldCell` /// - If `access` takes read access to a component no mutable reference to that @@ -2154,20 +2148,23 @@ impl<'w> FilteredEntityMut<'w> { /// may exist at the same time as the returned [`FilteredEntityMut`] /// - If `access` takes any access for a component `entity` must have that component. #[inline] - pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>, access: Access) -> Self { + pub(crate) unsafe fn new( + entity: UnsafeEntityCell<'w>, + access: &'s Access, + ) -> Self { Self { entity, access } } /// Returns a new instance with a shorter lifetime. /// This is useful if you have `&mut FilteredEntityMut`, but you need `FilteredEntityMut`. - pub fn reborrow(&mut self) -> FilteredEntityMut<'_> { + pub fn reborrow(&mut self) -> FilteredEntityMut<'_, 's> { // SAFETY: We have exclusive access to the entire entity and its components. - unsafe { Self::new(self.entity, self.access.clone()) } + unsafe { Self::new(self.entity, self.access) } } /// Gets read-only access to all of the entity's components. #[inline] - pub fn as_readonly(&self) -> FilteredEntityRef<'_> { + pub fn as_readonly(&self) -> FilteredEntityRef<'_, 's> { FilteredEntityRef::from(self) } @@ -2193,7 +2190,7 @@ impl<'w> FilteredEntityMut<'w> { /// Returns a reference to the underlying [`Access`]. #[inline] pub fn access(&self) -> &Access { - &self.access + self.access } /// Returns `true` if the current entity has a component of type `T`. @@ -2323,55 +2320,45 @@ impl<'w> FilteredEntityMut<'w> { } } -impl<'a> From> for FilteredEntityMut<'a> { +fn write_all_components() -> &'static Access { + static WRITE_ALL_COMPONENTS: OnceLock> = OnceLock::new(); + + WRITE_ALL_COMPONENTS.get_or_init(|| { + let mut access = Access::new(); + access.write_all_components(); + access + }) +} + +impl<'a> From> for FilteredEntityMut<'a, 'static> { fn from(entity: EntityMut<'a>) -> Self { // SAFETY: // - `EntityMut` guarantees exclusive access to all components in the new `FilteredEntityMut`. - unsafe { - let mut access = Access::default(); - access.read_all(); - access.write_all(); - FilteredEntityMut::new(entity.0, access) - } + unsafe { FilteredEntityMut::new(entity.0, write_all_components()) } } } -impl<'a> From<&'a mut EntityMut<'_>> for FilteredEntityMut<'a> { +impl<'a> From<&'a mut EntityMut<'_>> for FilteredEntityMut<'a, 'static> { fn from(entity: &'a mut EntityMut<'_>) -> Self { // SAFETY: // - `EntityMut` guarantees exclusive access to all components in the new `FilteredEntityMut`. - unsafe { - let mut access = Access::default(); - access.read_all(); - access.write_all(); - FilteredEntityMut::new(entity.0, access) - } + unsafe { FilteredEntityMut::new(entity.0, write_all_components()) } } } -impl<'a> From> for FilteredEntityMut<'a> { +impl<'a> From> for FilteredEntityMut<'a, 'static> { fn from(entity: EntityWorldMut<'a>) -> Self { // SAFETY: // - `EntityWorldMut` guarantees exclusive access to the entire world. - unsafe { - let mut access = Access::default(); - access.read_all(); - access.write_all(); - FilteredEntityMut::new(entity.into_unsafe_entity_cell(), access) - } + unsafe { FilteredEntityMut::new(entity.into_unsafe_entity_cell(), write_all_components()) } } } -impl<'a> From<&'a mut EntityWorldMut<'_>> for FilteredEntityMut<'a> { +impl<'a> From<&'a mut EntityWorldMut<'_>> for FilteredEntityMut<'a, 'static> { fn from(entity: &'a mut EntityWorldMut<'_>) -> Self { // SAFETY: // - `EntityWorldMut` guarantees exclusive access to the entire world. - unsafe { - let mut access = Access::default(); - access.read_all(); - access.write_all(); - FilteredEntityMut::new(entity.as_unsafe_entity_cell(), access) - } + unsafe { FilteredEntityMut::new(entity.as_unsafe_entity_cell(), write_all_components()) } } } From 468d2248bec189d2c68a529e97c48aea9cc87b91 Mon Sep 17 00:00:00 2001 From: Chris Russell <8494645+chescock@users.noreply.github.com> Date: Sat, 21 Sep 2024 22:36:45 -0400 Subject: [PATCH 2/2] Have Entity(Ref|Mut)Except look up the component ids on initialization and store them in an Access that can be borrowed by each row. --- crates/bevy_animation/src/keyframes.rs | 24 +++--- crates/bevy_ecs/src/query/fetch.rs | 97 +++++++++++++------------ crates/bevy_ecs/src/world/entity_ref.rs | 77 +++++++++++++------- 3 files changed, 111 insertions(+), 87 deletions(-) diff --git a/crates/bevy_animation/src/keyframes.rs b/crates/bevy_animation/src/keyframes.rs index f2694930c0be7..9d37f273a526a 100644 --- a/crates/bevy_animation/src/keyframes.rs +++ b/crates/bevy_animation/src/keyframes.rs @@ -147,7 +147,7 @@ pub trait Keyframes: Reflect + Debug + Send + Sync { fn apply_single_keyframe<'a>( &self, transform: Option>, - entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + entity: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, weight: f32, ) -> Result<(), AnimationEvaluationError>; @@ -178,7 +178,7 @@ pub trait Keyframes: Reflect + Debug + Send + Sync { fn apply_tweened_keyframes<'a>( &self, transform: Option>, - entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + entity: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, interpolation: Interpolation, step_start: usize, time: f32, @@ -278,7 +278,7 @@ impl Keyframes for TranslationKeyframes { fn apply_single_keyframe<'a>( &self, transform: Option>, - _: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + _: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, weight: f32, ) -> Result<(), AnimationEvaluationError> { let mut component = transform.ok_or_else(|| { @@ -294,7 +294,7 @@ impl Keyframes for TranslationKeyframes { fn apply_tweened_keyframes<'a>( &self, transform: Option>, - _: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + _: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, interpolation: Interpolation, step_start: usize, time: f32, @@ -333,7 +333,7 @@ impl Keyframes for ScaleKeyframes { fn apply_single_keyframe<'a>( &self, transform: Option>, - _: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + _: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, weight: f32, ) -> Result<(), AnimationEvaluationError> { let mut component = transform.ok_or_else(|| { @@ -349,7 +349,7 @@ impl Keyframes for ScaleKeyframes { fn apply_tweened_keyframes<'a>( &self, transform: Option>, - _: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + _: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, interpolation: Interpolation, step_start: usize, time: f32, @@ -388,7 +388,7 @@ impl Keyframes for RotationKeyframes { fn apply_single_keyframe<'a>( &self, transform: Option>, - _: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + _: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, weight: f32, ) -> Result<(), AnimationEvaluationError> { let mut component = transform.ok_or_else(|| { @@ -404,7 +404,7 @@ impl Keyframes for RotationKeyframes { fn apply_tweened_keyframes<'a>( &self, transform: Option>, - _: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + _: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, interpolation: Interpolation, step_start: usize, time: f32, @@ -447,7 +447,7 @@ where fn apply_single_keyframe<'a>( &self, _: Option>, - mut entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + mut entity: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, weight: f32, ) -> Result<(), AnimationEvaluationError> { let mut component = entity.get_mut::().ok_or_else(|| { @@ -465,7 +465,7 @@ where fn apply_tweened_keyframes<'a>( &self, _: Option>, - mut entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + mut entity: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, interpolation: Interpolation, step_start: usize, time: f32, @@ -529,7 +529,7 @@ impl Keyframes for MorphWeightsKeyframes { fn apply_single_keyframe<'a>( &self, _: Option>, - mut entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + mut entity: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, weight: f32, ) -> Result<(), AnimationEvaluationError> { let mut dest = entity.get_mut::().ok_or_else(|| { @@ -548,7 +548,7 @@ impl Keyframes for MorphWeightsKeyframes { fn apply_tweened_keyframes<'a>( &self, _: Option>, - mut entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle)>, + mut entity: EntityMutExcept<'a, 'a, (Transform, AnimationPlayer, Handle)>, interpolation: Interpolation, step_start: usize, time: f32, diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 534b0f833bfa5..f441d10d7a7bb 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -13,7 +13,6 @@ use crate::{ }; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; use bevy_utils::all_tuples; -use smallvec::SmallVec; use std::{cell::UnsafeCell, marker::PhantomData}; /// Types that can be fetched from a [`World`] using a [`Query`]. @@ -857,13 +856,13 @@ unsafe impl<'w, 's> QueryData for FilteredEntityMut<'w, 's> { /// SAFETY: `EntityRefExcept` guards access to all components in the bundle `B` /// and populates `Access` values so that queries that conflict with this access /// are rejected. -unsafe impl<'a, B> WorldQuery for EntityRefExcept<'a, B> +unsafe impl WorldQuery for EntityRefExcept<'_, '_, B> where B: Bundle, { - type Fetch<'w, 's> = UnsafeWorldCell<'w>; - type Item<'w, 's> = EntityRefExcept<'w, B>; - type State = SmallVec<[ComponentId; 4]>; + type Fetch<'w, 's> = (UnsafeWorldCell<'w>, &'s Access); + type Item<'w, 's> = EntityRefExcept<'w, 's, B>; + type State = Access; fn shrink<'wlong: 'wshort, 'wshort, 's>( item: Self::Item<'wlong, 's>, @@ -879,11 +878,11 @@ where unsafe fn init_fetch<'w, 's>( world: UnsafeWorldCell<'w>, - _: &'s Self::State, + state: &'s Self::State, _: Tick, _: Tick, ) -> Self::Fetch<'w, 's> { - world + (world, state) } const IS_DENSE: bool = true; @@ -899,45 +898,47 @@ where unsafe fn set_table<'w, 's>(_: &mut Self::Fetch<'w, 's>, _: &'s Self::State, _: &'w Table) {} unsafe fn fetch<'w, 's>( - world: &mut Self::Fetch<'w, 's>, + (world, access): &mut Self::Fetch<'w, 's>, entity: Entity, _: TableRow, ) -> Self::Item<'w, 's> { let cell = world.get_entity(entity).unwrap(); - EntityRefExcept::new(cell) + EntityRefExcept::new(cell, access) } fn update_component_access( state: &Self::State, filtered_access: &mut FilteredAccess, ) { - let mut my_access = Access::new(); - my_access.read_all_components(); - for id in state { - my_access.remove_component_read(*id); - } - - let access = filtered_access.access_mut(); assert!( - access.is_compatible(&my_access), + filtered_access.access().is_compatible(state), "`EntityRefExcept<{}>` conflicts with a previous access in this query.", std::any::type_name::(), ); - access.extend(&my_access); + filtered_access.access.extend(state); } fn init_state(world: &mut World) -> Self::State { - Self::get_state(world.components()).unwrap() + let mut access = Access::new(); + access.read_all_components(); + B::component_ids(&mut world.components, &mut world.storages, &mut |id| { + access.remove_component_read(id); + }); + access } fn get_state(components: &Components) -> Option { - let mut ids = SmallVec::new(); + let mut access = Access::new(); + access.read_all_components(); + let mut all_initialized = true; B::get_component_ids(components, &mut |maybe_id| { if let Some(id) = maybe_id { - ids.push(id); + access.remove_component_read(id); + } else { + all_initialized = false; } }); - Some(ids) + all_initialized.then_some(access) } fn matches_component_set(_: &Self::State, _: &impl Fn(ComponentId) -> bool) -> bool { @@ -946,7 +947,7 @@ where } /// SAFETY: `Self` is the same as `Self::ReadOnly`. -unsafe impl<'a, B> QueryData for EntityRefExcept<'a, B> +unsafe impl QueryData for EntityRefExcept<'_, '_, B> where B: Bundle, { @@ -955,18 +956,18 @@ where /// SAFETY: `EntityRefExcept` enforces read-only access to its contained /// components. -unsafe impl<'a, B> ReadOnlyQueryData for EntityRefExcept<'a, B> where B: Bundle {} +unsafe impl ReadOnlyQueryData for EntityRefExcept<'_, '_, B> where B: Bundle {} /// SAFETY: `EntityMutExcept` guards access to all components in the bundle `B` /// and populates `Access` values so that queries that conflict with this access /// are rejected. -unsafe impl<'a, B> WorldQuery for EntityMutExcept<'a, B> +unsafe impl WorldQuery for EntityMutExcept<'_, '_, B> where B: Bundle, { - type Fetch<'w, 's> = UnsafeWorldCell<'w>; - type Item<'w, 's> = EntityMutExcept<'w, B>; - type State = SmallVec<[ComponentId; 4]>; + type Fetch<'w, 's> = (UnsafeWorldCell<'w>, &'s Access); + type Item<'w, 's> = EntityMutExcept<'w, 's, B>; + type State = Access; fn shrink<'wlong: 'wshort, 'wshort, 's>( item: Self::Item<'wlong, 's>, @@ -982,11 +983,11 @@ where unsafe fn init_fetch<'w, 's>( world: UnsafeWorldCell<'w>, - _: &'s Self::State, + state: &'s Self::State, _: Tick, _: Tick, ) -> Self::Fetch<'w, 's> { - world + (world, state) } const IS_DENSE: bool = true; @@ -1002,45 +1003,47 @@ where unsafe fn set_table<'w, 's>(_: &mut Self::Fetch<'w, 's>, _: &'s Self::State, _: &'w Table) {} unsafe fn fetch<'w, 's>( - world: &mut Self::Fetch<'w, 's>, + (world, access): &mut Self::Fetch<'w, 's>, entity: Entity, _: TableRow, ) -> Self::Item<'w, 's> { let cell = world.get_entity(entity).unwrap(); - EntityMutExcept::new(cell) + EntityMutExcept::new(cell, access) } fn update_component_access( state: &Self::State, filtered_access: &mut FilteredAccess, ) { - let mut my_access = Access::new(); - my_access.write_all_components(); - for id in state { - my_access.remove_component_read(*id); - } - - let access = filtered_access.access_mut(); assert!( - access.is_compatible(&my_access), + filtered_access.access().is_compatible(state), "`EntityMutExcept<{}>` conflicts with a previous access in this query.", std::any::type_name::() ); - access.extend(&my_access); + filtered_access.access.extend(state); } fn init_state(world: &mut World) -> Self::State { - Self::get_state(world.components()).unwrap() + let mut access = Access::new(); + access.write_all_components(); + B::component_ids(&mut world.components, &mut world.storages, &mut |id| { + access.remove_component_read(id); + }); + access } fn get_state(components: &Components) -> Option { - let mut ids = SmallVec::new(); + let mut access = Access::new(); + access.write_all_components(); + let mut all_initialized = true; B::get_component_ids(components, &mut |maybe_id| { if let Some(id) = maybe_id { - ids.push(id); + access.remove_component_read(id); + } else { + all_initialized = false; } }); - Some(ids) + all_initialized.then_some(access) } fn matches_component_set(_: &Self::State, _: &impl Fn(ComponentId) -> bool) -> bool { @@ -1050,11 +1053,11 @@ where /// SAFETY: All accesses that `EntityRefExcept` provides are also accesses that /// `EntityMutExcept` provides. -unsafe impl<'a, B> QueryData for EntityMutExcept<'a, B> +unsafe impl<'w, 's, B> QueryData for EntityMutExcept<'w, 's, B> where B: Bundle, { - type ReadOnly = EntityRefExcept<'a, B>; + type ReadOnly = EntityRefExcept<'w, 's, B>; } /// SAFETY: diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 28d9c7691e465..afcf6274717e8 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -2070,6 +2070,22 @@ impl<'a, 's> From<&'a FilteredEntityMut<'_, 's>> for FilteredEntityRef<'a, 's> { } } +impl<'w, 's, B: Bundle> From> for FilteredEntityRef<'w, 's> { + fn from(entity_ref: EntityRefExcept<'w, 's, B>) -> Self { + // SAFETY: + // - `EntityRefExcept` guarantees shared access to all components in the new `FilteredEntityRef`. + unsafe { FilteredEntityRef::new(entity_ref.entity, entity_ref.access) } + } +} + +impl<'w, 's, B: Bundle> From> for FilteredEntityRef<'w, 's> { + fn from(entity_ref: EntityMutExcept<'w, 's, B>) -> Self { + // SAFETY: + // - `EntityMutExcept` guarantees exclusive access to all components in the new `FilteredEntityRef`. + unsafe { FilteredEntityRef::new(entity_ref.entity, entity_ref.access) } + } +} + fn read_all_components() -> &'static Access { static READ_ALL_COMPONENTS: OnceLock> = OnceLock::new(); @@ -2320,6 +2336,14 @@ impl<'w, 's> FilteredEntityMut<'w, 's> { } } +impl<'w, 's, B: Bundle> From> for FilteredEntityMut<'w, 's> { + fn from(entity_ref: EntityMutExcept<'w, 's, B>) -> Self { + // SAFETY: + // - `EntityMutExcept` guarantees exclusive access to all components in the new `FilteredEntityMut`. + unsafe { FilteredEntityMut::new(entity_ref.entity, entity_ref.access) } + } +} + fn write_all_components() -> &'static Access { static WRITE_ALL_COMPONENTS: OnceLock> = OnceLock::new(); @@ -2374,23 +2398,28 @@ pub enum TryFromFilteredError { /// Provides read-only access to a single entity and all its components, save /// for an explicitly-enumerated set. #[derive(Clone)] -pub struct EntityRefExcept<'w, B> +pub struct EntityRefExcept<'w, 's, B> where B: Bundle, { entity: UnsafeEntityCell<'w>, + access: &'s Access, phantom: PhantomData, } -impl<'w, B> EntityRefExcept<'w, B> +impl<'w, 's, B> EntityRefExcept<'w, 's, B> where B: Bundle, { /// # Safety /// Other users of `UnsafeEntityCell` must only have mutable access to the components in `B`. - pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>) -> Self { + pub(crate) unsafe fn new( + entity: UnsafeEntityCell<'w>, + access: &'s Access, + ) -> Self { Self { entity, + access, phantom: PhantomData, } } @@ -2412,7 +2441,7 @@ where { let components = self.entity.world().components(); let id = components.component_id::()?; - if bundle_contains_component::(components, id) { + if !self.access.has_component_read(id) { None } else { // SAFETY: We have read access for all components that weren't @@ -2432,7 +2461,7 @@ where { let components = self.entity.world().components(); let id = components.component_id::()?; - if bundle_contains_component::(components, id) { + if !self.access.has_component_read(id) { None } else { // SAFETY: We have read access for all components that weren't @@ -2442,14 +2471,14 @@ where } } -impl<'a, B> From<&'a EntityMutExcept<'_, B>> for EntityRefExcept<'a, B> +impl<'a, 's, B> From<&'a EntityMutExcept<'_, 's, B>> for EntityRefExcept<'a, 's, B> where B: Bundle, { - fn from(entity_mut: &'a EntityMutExcept<'_, B>) -> Self { + fn from(entity_mut: &'a EntityMutExcept<'_, 's, B>) -> Self { // SAFETY: All accesses that `EntityRefExcept` provides are also // accesses that `EntityMutExcept` provides. - unsafe { EntityRefExcept::new(entity_mut.entity) } + unsafe { EntityRefExcept::new(entity_mut.entity, entity_mut.access) } } } @@ -2462,23 +2491,28 @@ where /// need access to all components, prefer a standard query with a /// [`crate::query::Without`] filter. #[derive(Clone)] -pub struct EntityMutExcept<'w, B> +pub struct EntityMutExcept<'w, 's, B> where B: Bundle, { entity: UnsafeEntityCell<'w>, + access: &'s Access, phantom: PhantomData, } -impl<'w, B> EntityMutExcept<'w, B> +impl<'w, 's, B> EntityMutExcept<'w, 's, B> where B: Bundle, { /// # Safety /// Other users of `UnsafeEntityCell` must not have access to any components not in `B`. - pub(crate) unsafe fn new(entity: UnsafeEntityCell<'w>) -> Self { + pub(crate) unsafe fn new( + entity: UnsafeEntityCell<'w>, + access: &'s Access, + ) -> Self { Self { entity, + access, phantom: PhantomData, } } @@ -2494,16 +2528,16 @@ where /// /// This is useful if you have `&mut EntityMutExcept`, but you need /// `EntityMutExcept`. - pub fn reborrow(&mut self) -> EntityMutExcept<'_, B> { + pub fn reborrow(&mut self) -> EntityMutExcept<'_, 's, B> { // SAFETY: We have exclusive access to the entire entity and the // applicable components. - unsafe { Self::new(self.entity) } + unsafe { Self::new(self.entity, self.access) } } /// Gets read-only access to all of the entity's components, except for the /// ones in `CL`. #[inline] - pub fn as_readonly(&self) -> EntityRefExcept<'_, B> { + pub fn as_readonly(&self) -> EntityRefExcept<'_, 's, B> { EntityRefExcept::from(self) } @@ -2540,7 +2574,7 @@ where { let components = self.entity.world().components(); let id = components.component_id::()?; - if bundle_contains_component::(components, id) { + if !self.access.has_component_write(id) { None } else { // SAFETY: We have write access for all components that weren't @@ -2550,19 +2584,6 @@ where } } -fn bundle_contains_component(components: &Components, query_id: ComponentId) -> bool -where - B: Bundle, -{ - let mut found = false; - B::get_component_ids(components, &mut |maybe_id| { - if let Some(id) = maybe_id { - found = found || id == query_id; - } - }); - found -} - /// Inserts a dynamic [`Bundle`] into the entity. /// /// # Safety