From efd1deccfc862f296f494685b4209c7907f5af4d Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Sat, 3 Jan 2026 14:09:31 -0800 Subject: [PATCH 01/27] add lookahead methods to WorldQuery --- crates/bevy_ecs/src/query/world_query.rs | 173 ++++++++++++++++++++++- crates/bevy_ecs/src/storage/table/mod.rs | 4 +- 2 files changed, 174 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index fef5b0257f167..7088565721899 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -1,11 +1,15 @@ +use core::ops::Range; + use crate::{ archetype::Archetype, change_detection::Tick, component::{ComponentId, Components}, + entity::Entity, query::FilteredAccess, - storage::Table, + storage::{Table, TableRow}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; +use nonmax::NonMaxU32; use variadics_please::all_tuples; /// Types that can be used as parameters in a [`Query`]. @@ -32,6 +36,11 @@ use variadics_please::all_tuples; /// /// When implementing [`update_component_access`], note that `add_read` and `add_write` both also add a `With` filter, whereas `extend_access` does not change the filters. /// +/// When implementing [`find_table_chunk`] and [`find_archetype_chunk`], their return values must be +/// subsets of `rows` and `indices` respectively, even if returning an empty range. +/// +/// [`find_table_chunk`], [`find_archetype_chunk`], and [`matches`] must not mutably access any world data. +/// /// [`QueryData::provide_extra_access`]: crate::query::QueryData::provide_extra_access /// [`QueryData::fetch`]: crate::query::QueryData::fetch /// [`QueryFilter::filter_fetch`]: crate::query::QueryFilter::filter_fetch @@ -41,6 +50,9 @@ use variadics_please::all_tuples; /// [`update_component_access`]: Self::update_component_access /// [`QueryData`]: crate::query::QueryData /// [`QueryFilter`]: crate::query::QueryFilter +/// [`find_table_chunk`]: crate::query::WorldQuery::find_table_chunk +/// [`find_archetype_chunk`]: crate::query::WorldQuery::find_archetype_chunk +/// [`matches`]: crate::query::WorldQuery::matches pub unsafe trait WorldQuery { /// Per archetype/table state retrieved by this [`WorldQuery`] to compute [`Self::Item`](crate::query::QueryData::Item) for each entity. type Fetch<'w>: Clone; @@ -131,6 +143,114 @@ pub unsafe trait WorldQuery { state: &Self::State, set_contains_id: &impl Fn(ComponentId) -> bool, ) -> bool; + + /// TODO: docs + /// + /// # Safety + /// + /// Must always be called _after_ [`WorldQuery::set_table`]. + /// `rows` must be in the range of the current table. + #[inline] + unsafe fn find_table_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + table: &Table, + indices: Range, + ) -> Range { + let entities = table.entities(); + indices.find_chunk(|index| { + let table_row = TableRow::new(index); + // SAFETY: caller guarantees `indices` is in range of `table` + let entity = unsafe { *entities.get_unchecked(index.get() as usize) }; + // SAFETY: invariants upheld by caller + unsafe { Self::matches(state, fetch, entity, table_row) } + }) + } + + /// TODO: docs + /// + /// # Safety + /// + /// Must always be called _after_ [`WorldQuery::set_archetype`]. + /// `indices` must be in the range of the current archetype. + #[inline] + unsafe fn find_archetype_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + archetype: &Archetype, + indices: Range, + ) -> Range { + let archetype_entities = archetype.entities(); + indices.find_chunk(|index| { + // SAFETY: caller guarantees `indices` is in range of `archetype` + let archetype_entity = + unsafe { archetype_entities.get_unchecked(index.get() as usize) }; + + // SAFETY: invariants upheld by caller + unsafe { + Self::matches( + state, + fetch, + archetype_entity.id(), + archetype_entity.table_row(), + ) + } + }) + } + + /// Returns true if the provided [`Entity`] and [`TableRow`] should be included in the query results. + /// If false, the entity will be skipped. + /// + /// Note that this is called after already restricting the matched [`Table`]s and [`Archetype`]s to the + /// ones that are compatible with the Filter's access. + /// + /// Implementors of this method will generally either have a trivial `true` body (required for archetypal filters), + /// or access the necessary data within this function to make the final decision on filter inclusion. + /// + /// # Safety + /// + /// Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and + /// `table_row` must be in the range of the current table and archetype. + unsafe fn matches( + state: &Self::State, + fetch: &Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool; +} + +pub trait RangeExt { + fn find_chunk bool>(self, func: F) -> Self; +} + +impl RangeExt for Range { + #[inline] + fn find_chunk bool>(mut self, mut func: F) -> Self { + let mut index = self.start; + while index < self.end { + // index is taken from an exclusive range, so it can't be max + let nonmax_index = unsafe { NonMaxU32::new_unchecked(index) }; + let matches = func(nonmax_index); + if matches { + break; + } + index = index.wrapping_add(1); + } + self.start = index; + + while index < self.end { + // index is taken from an exclusive range, so it can't be max + let nonmax_index = unsafe { NonMaxU32::new_unchecked(index) }; + index = index.wrapping_add(1); + let matches = func(nonmax_index); + if !matches { + break; + } + } + self.end = index; + + self + } } macro_rules! impl_tuple_world_query { @@ -152,12 +272,18 @@ macro_rules! impl_tuple_world_query { clippy::unused_unit, reason = "Zero-length tuples will generate some function bodies equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." )] + #[allow( + unused_mut, + reason = "Zero-length tuples will generate some function bodies that don't mutate their parameters; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." + )] $(#[$meta])* /// SAFETY: /// `fetch` accesses are the conjunction of the subqueries' accesses /// This is sound because `update_component_access` adds accesses according to the implementations of all the subqueries. /// `update_component_access` adds all `With` and `Without` filters from the subqueries. /// This is sound because `matches_component_set` always returns `false` if any the subqueries' implementations return `false`. + /// `lookahead_table` and `lookahead_archetype` always return a subset of `rows`/`indices` + /// This is sound because each `lookahead_table` and `lookahead_archetype` each unsafe impl<$($name: WorldQuery),*> WorldQuery for ($($name,)*) { type Fetch<'w> = ($($name::Fetch<'w>,)*); type State = ($($name::State,)*); @@ -216,6 +342,51 @@ macro_rules! impl_tuple_world_query { let ($($name,)*) = state; true $(&& $name::matches_component_set($name, set_contains_id))* } + + #[inline] + unsafe fn find_table_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + table: &Table, + mut rows: Range, + ) -> Range { + let ($($name,)*) = fetch; + let ($($state,)*) = state; + // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's + // always valid to pass to the next term. Other invariants are upheld by + // the caller. + $(rows = unsafe { $name::find_table_chunk($state, $name, table, rows) };)* + rows + } + + #[inline] + unsafe fn find_archetype_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + table: &Archetype, + mut indices: Range, + ) -> Range { + let ($($name,)*) = fetch; + let ($($state,)*) = state; + // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's + // always valid to pass to the next term. Other invariants are upheld by + // the caller. + $(indices = unsafe { $name::find_archetype_chunk($state, $name, table, indices) };)* + indices + } + + #[inline] + unsafe fn matches( + state: &Self::State, + fetch: &Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool { + let ($($name,)*) = fetch; + let ($($state,)*) = state; + // SAFETY: invariants are upheld by the caller. + true $(&& unsafe { $name::matches($state, $name, entity, table_row) })* + } } }; } diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index 846fba5ad57e3..bd9a182ad9e4c 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -12,7 +12,7 @@ pub use column::*; use core::{ cell::UnsafeCell, num::NonZeroUsize, - ops::{Index, IndexMut}, + ops::{Index, IndexMut, Range}, panic::Location, }; use nonmax::NonMaxU32; @@ -97,7 +97,7 @@ impl TableId { /// [`Archetype`]: crate::archetype::Archetype /// [`Archetype::entity_table_row`]: crate::archetype::Archetype::entity_table_row /// [`Archetype::table_id`]: crate::archetype::Archetype::table_id -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] #[repr(transparent)] pub struct TableRow(NonMaxU32); From 36d923290414f0002f8ee8fc6bed56caa40c69ed Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Tue, 6 Jan 2026 23:20:25 -0800 Subject: [PATCH 02/27] fix QueryData derive --- crates/bevy_ecs/macros/src/world_query.rs | 39 +++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/crates/bevy_ecs/macros/src/world_query.rs b/crates/bevy_ecs/macros/src/world_query.rs index 39feecf3798b8..66d9d27aa7a06 100644 --- a/crates/bevy_ecs/macros/src/world_query.rs +++ b/crates/bevy_ecs/macros/src/world_query.rs @@ -171,6 +171,45 @@ pub(crate) fn world_query_impl( fn matches_component_set(state: &Self::State, _set_contains_id: &impl Fn(#path::component::ComponentId) -> bool) -> bool { true #(&& <#field_types>::matches_component_set(&state.#field_aliases, _set_contains_id))* } + + #[inline] + unsafe fn find_table_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + table: &#path::storage::Table, + mut rows: core::ops::Range, + ) -> core::ops::Range { + // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's + // always valid to pass to the next term. Other invariants are upheld by + // the caller. + #(rows = unsafe { <#field_types>::find_table_chunk(&state.#field_aliases, &fetch.#field_aliases, table, rows) };)* + rows + } + + #[inline] + unsafe fn find_archetype_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + archetype: &#path::archetype::Archetype, + mut indices: core::ops::Range, + ) -> core::ops::Range { + // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's + // always valid to pass to the next term. Other invariants are upheld by + // the caller. + #(indices = unsafe { <#field_types>::find_archetype_chunk(&state.#field_aliases, &fetch.#field_aliases, archetype, indices) };)* + indices + } + + #[inline] + unsafe fn matches( + state: &Self::State, + fetch: &Self::Fetch<'_>, + entity: #path::entity::Entity, + table_row: #path::storage::TableRow, + ) -> bool { + // SAFETY: invariants are upheld by the caller. + true #(&& unsafe { <#field_types>::matches(&state.#field_aliases, &fetch.#field_aliases, entity, table_row) })* + } } } } From 50f4c0d5263be08f0704ad49af061935871ec977 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Tue, 6 Jan 2026 23:42:39 -0800 Subject: [PATCH 03/27] fix query/mod.rs test --- crates/bevy_ecs/src/query/mod.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 065eadd24db88..25aaaaa3559c0 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -881,6 +881,16 @@ mod tests { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` From 7ae3236368e01d876d05a81e3b7ebd9f75dc3e1c Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Wed, 7 Jan 2026 16:52:42 -0800 Subject: [PATCH 04/27] move IS_ARCHETYPAL to WorldQuery --- crates/bevy_ecs/macros/src/query_data.rs | 2 - crates/bevy_ecs/macros/src/query_filter.rs | 2 - crates/bevy_ecs/macros/src/world_query.rs | 37 ++++--- crates/bevy_ecs/src/query/fetch.rs | 55 ++++------ crates/bevy_ecs/src/query/filter.rs | 31 ++---- crates/bevy_ecs/src/query/mod.rs | 2 +- crates/bevy_ecs/src/query/world_query.rs | 116 +++++++++++++-------- 7 files changed, 129 insertions(+), 116 deletions(-) diff --git a/crates/bevy_ecs/macros/src/query_data.rs b/crates/bevy_ecs/macros/src/query_data.rs index a91dd193e51c1..70adaf5a9de33 100644 --- a/crates/bevy_ecs/macros/src/query_data.rs +++ b/crates/bevy_ecs/macros/src/query_data.rs @@ -233,7 +233,6 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { unsafe impl #user_impl_generics #path::query::QueryData for #read_only_struct_name #user_ty_generics #user_where_clauses { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true #(&& <#read_only_field_types as #path::query::QueryData>::IS_ARCHETYPAL)*; type ReadOnly = #read_only_struct_name #user_ty_generics; type Item<'__w, '__s> = #read_only_item_struct_name #user_ty_generics_with_world_and_state; @@ -304,7 +303,6 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { unsafe impl #user_impl_generics #path::query::QueryData for #struct_name #user_ty_generics #user_where_clauses { const IS_READ_ONLY: bool = #is_read_only; - const IS_ARCHETYPAL: bool = true #(&& <#field_types as #path::query::QueryData>::IS_ARCHETYPAL)*; type ReadOnly = #read_only_struct_name #user_ty_generics; type Item<'__w, '__s> = #item_struct_name #user_ty_generics_with_world_and_state; diff --git a/crates/bevy_ecs/macros/src/query_filter.rs b/crates/bevy_ecs/macros/src/query_filter.rs index ed4f55c6af02f..f688cb0380744 100644 --- a/crates/bevy_ecs/macros/src/query_filter.rs +++ b/crates/bevy_ecs/macros/src/query_filter.rs @@ -74,8 +74,6 @@ pub fn derive_query_filter_impl(input: TokenStream) -> TokenStream { // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. unsafe impl #user_impl_generics #path::query::QueryFilter for #struct_name #user_ty_generics #user_where_clauses { - const IS_ARCHETYPAL: bool = true #(&& <#field_types as #path::query::QueryFilter>::IS_ARCHETYPAL)*; - #[allow(unused_variables)] #[inline(always)] unsafe fn filter_fetch<'__w>( diff --git a/crates/bevy_ecs/macros/src/world_query.rs b/crates/bevy_ecs/macros/src/world_query.rs index 66d9d27aa7a06..1896a72234278 100644 --- a/crates/bevy_ecs/macros/src/world_query.rs +++ b/crates/bevy_ecs/macros/src/world_query.rs @@ -130,6 +130,7 @@ pub(crate) fn world_query_impl( } const IS_DENSE: bool = true #(&& <#field_types>::IS_DENSE)*; + const IS_ARCHETYPAL: bool = true #(&& <#field_types>::IS_ARCHETYPAL)*; /// SAFETY: we call `set_archetype` for each member that implements `Fetch` #[inline] @@ -179,11 +180,15 @@ pub(crate) fn world_query_impl( table: &#path::storage::Table, mut rows: core::ops::Range, ) -> core::ops::Range { - // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's - // always valid to pass to the next term. Other invariants are upheld by - // the caller. - #(rows = unsafe { <#field_types>::find_table_chunk(&state.#field_aliases, &fetch.#field_aliases, table, rows) };)* - rows + if Self::IS_ARCHETYPAL { + rows + } else { + // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's + // always valid to pass to the next term. Other invariants are upheld by + // the caller. + #(rows = unsafe { <#field_types>::find_table_chunk(&state.#field_aliases, &fetch.#field_aliases, table, rows) };)* + rows + } } #[inline] @@ -193,11 +198,15 @@ pub(crate) fn world_query_impl( archetype: &#path::archetype::Archetype, mut indices: core::ops::Range, ) -> core::ops::Range { - // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's - // always valid to pass to the next term. Other invariants are upheld by - // the caller. - #(indices = unsafe { <#field_types>::find_archetype_chunk(&state.#field_aliases, &fetch.#field_aliases, archetype, indices) };)* - indices + if Self::IS_ARCHETYPAL { + indices + } else { + // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's + // always valid to pass to the next term. Other invariants are upheld by + // the caller. + #(indices = unsafe { <#field_types>::find_archetype_chunk(&state.#field_aliases, &fetch.#field_aliases, archetype, indices) };)* + indices + } } #[inline] @@ -207,8 +216,12 @@ pub(crate) fn world_query_impl( entity: #path::entity::Entity, table_row: #path::storage::TableRow, ) -> bool { - // SAFETY: invariants are upheld by the caller. - true #(&& unsafe { <#field_types>::matches(&state.#field_aliases, &fetch.#field_aliases, entity, table_row) })* + if Self::IS_ARCHETYPAL { + true + } else { + // SAFETY: invariants are upheld by the caller. + true #(&& unsafe { <#field_types>::matches(&state.#field_aliases, &fetch.#field_aliases, entity, table_row) })* + } } } } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index d0b191513e8ed..8ae4747e9fbd0 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -287,15 +287,6 @@ pub unsafe trait QueryData: WorldQuery { /// True if this query is read-only and may not perform mutable access. const IS_READ_ONLY: bool; - /// Returns true if (and only if) this query data relies strictly on archetypes to limit which - /// entities are accessed by the Query. - /// - /// This enables optimizations for [`QueryIter`](`crate::query::QueryIter`) that rely on knowing exactly how - /// many elements are being iterated (such as `Iterator::collect()`). - /// - /// If this is `true`, then [`QueryData::fetch`] must always return `Some`. - const IS_ARCHETYPAL: bool; - /// The read-only variant of this [`QueryData`], which satisfies the [`ReadOnlyQueryData`] trait. type ReadOnly: ReadOnlyQueryData::State>; @@ -401,6 +392,7 @@ unsafe impl WorldQuery for Entity { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -438,7 +430,6 @@ unsafe impl WorldQuery for Entity { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for Entity { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = Entity; @@ -498,6 +489,7 @@ unsafe impl WorldQuery for EntityLocation { // This is set to true to avoid forcing archetypal iteration in compound queries, is likely to be slower // in most practical use case. const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -535,7 +527,6 @@ unsafe impl WorldQuery for EntityLocation { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for EntityLocation { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = EntityLocation; @@ -666,6 +657,7 @@ unsafe impl WorldQuery for SpawnDetails { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -705,7 +697,6 @@ unsafe impl WorldQuery for SpawnDetails { // Is its own ReadOnlyQueryData. unsafe impl QueryData for SpawnDetails { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = Self; @@ -788,6 +779,7 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -831,7 +823,6 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl<'a> QueryData for EntityRef<'a> { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = EntityRef<'w>; @@ -898,6 +889,7 @@ unsafe impl<'a> WorldQuery for EntityMut<'a> { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -941,7 +933,6 @@ unsafe impl<'a> WorldQuery for EntityMut<'a> { /// SAFETY: access of `EntityRef` is a subset of `EntityMut` unsafe impl<'a> QueryData for EntityMut<'a> { const IS_READ_ONLY: bool = false; - const IS_ARCHETYPAL: bool = true; type ReadOnly = EntityRef<'a>; type Item<'w, 's> = EntityMut<'w>; @@ -992,6 +983,7 @@ unsafe impl WorldQuery for FilteredEntityRef<'_, '_> { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; unsafe fn init_fetch<'w, 's>( world: UnsafeWorldCell<'w>, @@ -1050,7 +1042,6 @@ unsafe impl WorldQuery for FilteredEntityRef<'_, '_> { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl<'a, 'b> QueryData for FilteredEntityRef<'a, 'b> { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = FilteredEntityRef<'w, 's>; @@ -1117,6 +1108,7 @@ unsafe impl WorldQuery for FilteredEntityMut<'_, '_> { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; unsafe fn init_fetch<'w, 's>( world: UnsafeWorldCell<'w>, @@ -1175,7 +1167,6 @@ unsafe impl WorldQuery for FilteredEntityMut<'_, '_> { /// SAFETY: access of `FilteredEntityRef` is a subset of `FilteredEntityMut` unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { const IS_READ_ONLY: bool = false; - const IS_ARCHETYPAL: bool = true; type ReadOnly = FilteredEntityRef<'a, 'b>; type Item<'w, 's> = FilteredEntityMut<'w, 's>; @@ -1255,6 +1246,7 @@ where } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; unsafe fn set_archetype<'w, 's>( _: &mut Self::Fetch<'w>, @@ -1311,7 +1303,6 @@ where B: Bundle, { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = EntityRefExcept<'w, 's, B>; @@ -1373,6 +1364,7 @@ where } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; unsafe fn set_archetype<'w, 's>( _: &mut Self::Fetch<'w>, @@ -1430,7 +1422,6 @@ where B: Bundle, { const IS_READ_ONLY: bool = false; - const IS_ARCHETYPAL: bool = true; type ReadOnly = EntityRefExcept<'a, 'b, B>; type Item<'w, 's> = EntityMutExcept<'w, 's, B>; @@ -1483,6 +1474,7 @@ unsafe impl WorldQuery for &Archetype { // This could probably be a non-dense query and just set a Option<&Archetype> fetch value in // set_archetypes, but forcing archetypal iteration is likely to be slower in any compound query. const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -1520,7 +1512,6 @@ unsafe impl WorldQuery for &Archetype { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for &Archetype { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = &'w Archetype; @@ -1620,6 +1611,8 @@ unsafe impl WorldQuery for &T { } }; + const IS_ARCHETYPAL: bool = true; + #[inline] unsafe fn set_archetype<'w>( fetch: &mut ReadFetch<'w, T>, @@ -1679,7 +1672,6 @@ unsafe impl WorldQuery for &T { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for &T { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = &'w T; @@ -1802,6 +1794,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { StorageType::SparseSet => false, } }; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w>( @@ -1869,7 +1862,6 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = Ref<'w, T>; @@ -2019,6 +2011,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { StorageType::SparseSet => false, } }; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w>( @@ -2086,7 +2079,6 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { /// SAFETY: access of `&T` is a subset of `&mut T` unsafe impl<'__w, T: Component> QueryData for &'__w mut T { const IS_READ_ONLY: bool = false; - const IS_ARCHETYPAL: bool = true; type ReadOnly = &'__w T; type Item<'w, 's> = Mut<'w, T>; @@ -2194,6 +2186,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Mut<'__w, T> { // Forwarded to `&mut T` const IS_DENSE: bool = <&mut T as WorldQuery>::IS_DENSE; + const IS_ARCHETYPAL: bool = true; #[inline] // Forwarded to `&mut T` @@ -2246,7 +2239,6 @@ unsafe impl<'__w, T: Component> WorldQuery for Mut<'__w, T> { // SAFETY: access of `Ref` is a subset of `Mut` unsafe impl<'__w, T: Component> QueryData for Mut<'__w, T> { const IS_READ_ONLY: bool = false; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Ref<'__w, T>; type Item<'w, 's> = Mut<'w, T>; @@ -2328,6 +2320,9 @@ unsafe impl WorldQuery for Option { } const IS_DENSE: bool = T::IS_DENSE; + // `Option` matches all entities, even if `T` does not, + // so it's always an `ArchetypeQueryData`, even for non-archetypal `T`. + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -2394,9 +2389,6 @@ unsafe impl WorldQuery for Option { // SAFETY: defers to soundness of `T: WorldQuery` impl unsafe impl QueryData for Option { const IS_READ_ONLY: bool = T::IS_READ_ONLY; - // `Option` matches all entities, even if `T` does not, - // so it's always an `ArchetypeQueryData`, even for non-archetypal `T`. - const IS_ARCHETYPAL: bool = true; type ReadOnly = Option; type Item<'w, 's> = Option>; @@ -2538,6 +2530,7 @@ unsafe impl WorldQuery for Has { StorageType::SparseSet => false, } }; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -2582,7 +2575,6 @@ unsafe impl WorldQuery for Has { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for Has { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = bool; @@ -2647,7 +2639,6 @@ macro_rules! impl_tuple_query_data { // SAFETY: defers to soundness `$name: WorldQuery` impl unsafe impl<$($name: QueryData),*> QueryData for ($($name,)*) { const IS_READ_ONLY: bool = true $(&& $name::IS_READ_ONLY)*; - const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; type ReadOnly = ($($name::ReadOnly,)*); type Item<'w, 's> = ($($name::Item<'w, 's>,)*); @@ -2754,6 +2745,7 @@ macro_rules! impl_anytuple_fetch { } const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; + const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; #[inline] unsafe fn set_archetype<'w, 's>( @@ -2845,7 +2837,6 @@ macro_rules! impl_anytuple_fetch { // SAFETY: defers to soundness of `$name: WorldQuery` impl unsafe impl<$($name: QueryData),*> QueryData for AnyOf<($($name,)*)> { const IS_READ_ONLY: bool = true $(&& $name::IS_READ_ONLY)*; - const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; type ReadOnly = AnyOf<($($name::ReadOnly,)*)>; type Item<'w, 's> = ($(Option<$name::Item<'w, 's>>,)*); @@ -2956,6 +2947,7 @@ unsafe impl WorldQuery for NopWorldQuery { } const IS_DENSE: bool = D::IS_DENSE; + const IS_ARCHETYPAL: bool = true; #[inline(always)] unsafe fn set_archetype( @@ -2990,7 +2982,6 @@ unsafe impl WorldQuery for NopWorldQuery { /// SAFETY: `Self::ReadOnly` is `Self` unsafe impl QueryData for NopWorldQuery { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = (); @@ -3045,6 +3036,7 @@ unsafe impl WorldQuery for PhantomData { // `PhantomData` does not match any components, so all components it matches // are stored in a Table (vacuous truth). const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; unsafe fn set_archetype<'w, 's>( _fetch: &mut Self::Fetch<'w>, @@ -3080,7 +3072,6 @@ unsafe impl WorldQuery for PhantomData { /// SAFETY: `Self::ReadOnly` is `Self` unsafe impl QueryData for PhantomData { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = (); @@ -3253,6 +3244,7 @@ mod tests { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -3291,7 +3283,6 @@ mod tests { unsafe impl QueryData for NonReleaseQueryData { type ReadOnly = Self; const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type Item<'w, 's> = (); diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index f8578d5d21705..4cfed81e4c551 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -82,15 +82,6 @@ use variadics_please::all_tuples; note = "a `QueryFilter` typically uses a combination of `With` and `Without` statements" )] pub unsafe trait QueryFilter: WorldQuery { - /// Returns true if (and only if) this Filter relies strictly on archetypes to limit which - /// components are accessed by the Query. - /// - /// This enables optimizations for [`QueryIter`](`crate::query::QueryIter`) that rely on knowing exactly how - /// many elements are being iterated (such as `Iterator::collect()`). - /// - /// If this is `true`, then [`QueryFilter::filter_fetch`] must always return true. - const IS_ARCHETYPAL: bool; - /// Returns true if the provided [`Entity`] and [`TableRow`] should be included in the query results. /// If false, the entity will be skipped. /// @@ -167,6 +158,7 @@ unsafe impl WorldQuery for With { StorageType::SparseSet => false, } }; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype( @@ -203,8 +195,6 @@ unsafe impl WorldQuery for With { // SAFETY: WorldQuery impl performs no access at all unsafe impl QueryFilter for With { - const IS_ARCHETYPAL: bool = true; - #[inline(always)] unsafe fn filter_fetch( _state: &Self::State, @@ -268,6 +258,7 @@ unsafe impl WorldQuery for Without { StorageType::SparseSet => false, } }; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype( @@ -304,8 +295,6 @@ unsafe impl WorldQuery for Without { // SAFETY: WorldQuery impl performs no access at all unsafe impl QueryFilter for Without { - const IS_ARCHETYPAL: bool = true; - #[inline(always)] unsafe fn filter_fetch( _state: &Self::State, @@ -403,6 +392,7 @@ macro_rules! impl_or_query_filter { } const IS_DENSE: bool = true $(&& $filter::IS_DENSE)*; + const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*; #[inline] unsafe fn init_fetch<'w, 's>(world: UnsafeWorldCell<'w>, state: &'s Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> { @@ -506,8 +496,6 @@ macro_rules! impl_or_query_filter { $(#[$meta])* // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. unsafe impl<$($filter: QueryFilter),*> QueryFilter for Or<($($filter,)*)> { - const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*; - #[inline(always)] unsafe fn filter_fetch( state: &Self::State, @@ -548,8 +536,6 @@ macro_rules! impl_tuple_query_filter { $(#[$meta])* // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. unsafe impl<$($name: QueryFilter),*> QueryFilter for ($($name,)*) { - const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; - #[inline(always)] unsafe fn filter_fetch( state: &Self::State, @@ -605,6 +591,7 @@ unsafe impl WorldQuery for Allow { // Even if the component is sparse, this implementation doesn't do anything with it const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype(_: &mut (), _: &ComponentId, _: &Archetype, _: &Table) {} @@ -633,8 +620,6 @@ unsafe impl WorldQuery for Allow { // SAFETY: WorldQuery impl performs no access at all unsafe impl QueryFilter for Allow { - const IS_ARCHETYPAL: bool = true; - #[inline(always)] unsafe fn filter_fetch( _: &Self::State, @@ -778,6 +763,7 @@ unsafe impl WorldQuery for Added { StorageType::SparseSet => false, } }; + const IS_ARCHETYPAL: bool = false; #[inline] unsafe fn set_archetype<'w, 's>( @@ -836,7 +822,6 @@ unsafe impl WorldQuery for Added { // SAFETY: WorldQuery impl performs only read access on ticks unsafe impl QueryFilter for Added { - const IS_ARCHETYPAL: bool = false; #[inline(always)] unsafe fn filter_fetch( _state: &Self::State, @@ -1005,6 +990,7 @@ unsafe impl WorldQuery for Changed { StorageType::SparseSet => false, } }; + const IS_ARCHETYPAL: bool = false; #[inline] unsafe fn set_archetype<'w, 's>( @@ -1063,8 +1049,6 @@ unsafe impl WorldQuery for Changed { // SAFETY: WorldQuery impl performs only read access on ticks unsafe impl QueryFilter for Changed { - const IS_ARCHETYPAL: bool = false; - #[inline(always)] unsafe fn filter_fetch( _state: &Self::State, @@ -1189,6 +1173,7 @@ unsafe impl WorldQuery for Spawned { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = false; #[inline] unsafe fn set_archetype<'w, 's>( @@ -1218,8 +1203,6 @@ unsafe impl WorldQuery for Spawned { // SAFETY: WorldQuery impl accesses no components or component ticks unsafe impl QueryFilter for Spawned { - const IS_ARCHETYPAL: bool = false; - #[inline(always)] unsafe fn filter_fetch( _state: &Self::State, diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 25aaaaa3559c0..4548b075bc2ed 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -841,6 +841,7 @@ mod tests { } const IS_DENSE: bool = true; + const IS_ARCHETYPAL: bool = true; #[inline] unsafe fn set_archetype<'w, 's>( @@ -896,7 +897,6 @@ mod tests { /// SAFETY: `Self` is the same as `Self::ReadOnly` unsafe impl QueryData for ReadsRData { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = true; type ReadOnly = Self; type Item<'w, 's> = (); diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 7088565721899..9b2bfb2292003 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -92,6 +92,15 @@ pub unsafe trait WorldQuery { /// iterators. const IS_DENSE: bool; + /// Returns true if (and only if) this query term relies strictly on archetypes to limit which + /// entities are accessed by the Query. + /// + /// This enables optimizations for [`QueryIter`](`crate::query::QueryIter`) that rely on knowing exactly how + /// many elements are being iterated (such as `Iterator::collect()`). + /// + /// If this is `true`, then [`WorldQuery::matches`] must always return `true`. + const IS_ARCHETYPAL: bool; + /// Adjusts internal state to account for the next [`Archetype`]. This will always be called on /// archetypes that match this [`WorldQuery`]. /// @@ -155,16 +164,20 @@ pub unsafe trait WorldQuery { state: &Self::State, fetch: &Self::Fetch<'_>, table: &Table, - indices: Range, + rows: Range, ) -> Range { - let entities = table.entities(); - indices.find_chunk(|index| { - let table_row = TableRow::new(index); - // SAFETY: caller guarantees `indices` is in range of `table` - let entity = unsafe { *entities.get_unchecked(index.get() as usize) }; - // SAFETY: invariants upheld by caller - unsafe { Self::matches(state, fetch, entity, table_row) } - }) + if Self::IS_ARCHETYPAL { + rows + } else { + let entities = table.entities(); + rows.find_chunk(|index| { + let table_row = TableRow::new(index); + // SAFETY: caller guarantees `indices` is in range of `table` + let entity = unsafe { *entities.get_unchecked(index.get() as usize) }; + // SAFETY: invariants upheld by caller + unsafe { Self::matches(state, fetch, entity, table_row) } + }) + } } /// TODO: docs @@ -180,22 +193,26 @@ pub unsafe trait WorldQuery { archetype: &Archetype, indices: Range, ) -> Range { - let archetype_entities = archetype.entities(); - indices.find_chunk(|index| { - // SAFETY: caller guarantees `indices` is in range of `archetype` - let archetype_entity = - unsafe { archetype_entities.get_unchecked(index.get() as usize) }; - - // SAFETY: invariants upheld by caller - unsafe { - Self::matches( - state, - fetch, - archetype_entity.id(), - archetype_entity.table_row(), - ) - } - }) + if Self::IS_ARCHETYPAL { + indices + } else { + let archetype_entities = archetype.entities(); + indices.find_chunk(|index| { + // SAFETY: caller guarantees `indices` is in range of `archetype` + let archetype_entity = + unsafe { archetype_entities.get_unchecked(index.get() as usize) }; + + // SAFETY: invariants upheld by caller + unsafe { + Self::matches( + state, + fetch, + archetype_entity.id(), + archetype_entity.table_row(), + ) + } + }) + } } /// Returns true if the provided [`Entity`] and [`TableRow`] should be included in the query results. @@ -304,6 +321,7 @@ macro_rules! impl_tuple_world_query { } const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; + const IS_ARCHETYPAL: bool = true $(&& $name::IS_ARCHETYPAL)*; #[inline] unsafe fn set_archetype<'w, 's>( @@ -350,13 +368,17 @@ macro_rules! impl_tuple_world_query { table: &Table, mut rows: Range, ) -> Range { - let ($($name,)*) = fetch; - let ($($state,)*) = state; - // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's - // always valid to pass to the next term. Other invariants are upheld by - // the caller. - $(rows = unsafe { $name::find_table_chunk($state, $name, table, rows) };)* - rows + if Self::IS_ARCHETYPAL { + rows + } else { + let ($($name,)*) = fetch; + let ($($state,)*) = state; + // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's + // always valid to pass to the next term. Other invariants are upheld by + // the caller. + $(rows = unsafe { $name::find_table_chunk($state, $name, table, rows) };)* + rows + } } #[inline] @@ -366,13 +388,17 @@ macro_rules! impl_tuple_world_query { table: &Archetype, mut indices: Range, ) -> Range { - let ($($name,)*) = fetch; - let ($($state,)*) = state; - // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's - // always valid to pass to the next term. Other invariants are upheld by - // the caller. - $(indices = unsafe { $name::find_archetype_chunk($state, $name, table, indices) };)* - indices + if Self::IS_ARCHETYPAL { + indices + } else { + let ($($name,)*) = fetch; + let ($($state,)*) = state; + // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's + // always valid to pass to the next term. Other invariants are upheld by + // the caller. + $(indices = unsafe { $name::find_archetype_chunk($state, $name, table, indices) };)* + indices + } } #[inline] @@ -382,10 +408,14 @@ macro_rules! impl_tuple_world_query { entity: Entity, table_row: TableRow, ) -> bool { - let ($($name,)*) = fetch; - let ($($state,)*) = state; - // SAFETY: invariants are upheld by the caller. - true $(&& unsafe { $name::matches($state, $name, entity, table_row) })* + if Self::IS_ARCHETYPAL { + true + } else { + let ($($name,)*) = fetch; + let ($($state,)*) = state; + // SAFETY: invariants are upheld by the caller. + true $(&& unsafe { $name::matches($state, $name, entity, table_row) })* + } } } }; From 5eebfff9a632db8da0d882fbcd738cdb69b92c2d Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Wed, 7 Jan 2026 18:50:42 -0800 Subject: [PATCH 05/27] add missing trait methods --- crates/bevy_ecs/src/query/fetch.rs | 247 ++++++++++++++++++++++- crates/bevy_ecs/src/query/filter.rs | 169 +++++++++++++++- crates/bevy_ecs/src/query/world_query.rs | 23 ++- crates/bevy_ecs/src/storage/table/mod.rs | 2 +- 4 files changed, 430 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 8ae4747e9fbd0..ccc07aaf7a95b 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -6,7 +6,7 @@ use crate::{ entity::{Entities, Entity, EntityLocation}, query::{ access_iter::{EcsAccessLevel, EcsAccessType}, - Access, DebugCheckedUnwrap, FilteredAccess, WorldQuery, + Access, DebugCheckedUnwrap, FilteredAccess, RangeExt, WorldQuery, }, storage::{ComponentSparseSet, Table, TableRow}, world::{ @@ -16,7 +16,7 @@ use crate::{ }; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; use bevy_utils::prelude::DebugName; -use core::{cell::UnsafeCell, iter, marker::PhantomData, panic::Location}; +use core::{cell::UnsafeCell, iter, marker::PhantomData, ops::Range, panic::Location}; use variadics_please::all_tuples; /// Types that can be fetched from a [`World`] using a [`Query`]. @@ -425,6 +425,16 @@ unsafe impl WorldQuery for Entity { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` @@ -522,6 +532,16 @@ unsafe impl WorldQuery for EntityLocation { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` @@ -690,6 +710,16 @@ unsafe impl WorldQuery for SpawnDetails { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } // SAFETY: @@ -818,6 +848,16 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` @@ -928,6 +968,16 @@ unsafe impl<'a> WorldQuery for EntityMut<'a> { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: access of `EntityRef` is a subset of `EntityMut` @@ -1037,6 +1087,16 @@ unsafe impl WorldQuery for FilteredEntityRef<'_, '_> { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` @@ -1162,6 +1222,16 @@ unsafe impl WorldQuery for FilteredEntityMut<'_, '_> { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: access of `FilteredEntityRef` is a subset of `FilteredEntityMut` @@ -1295,6 +1365,16 @@ where fn matches_component_set(_: &Self::State, _: &impl Fn(ComponentId) -> bool) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly`. @@ -1413,6 +1493,16 @@ where fn matches_component_set(_: &Self::State, _: &impl Fn(ComponentId) -> bool) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: All accesses that `EntityRefExcept` provides are also accesses that @@ -1507,6 +1597,16 @@ unsafe impl WorldQuery for &Archetype { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` @@ -1667,6 +1767,16 @@ unsafe impl WorldQuery for &T { ) -> bool { set_contains_id(state) } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` @@ -1857,6 +1967,16 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { ) -> bool { set_contains_id(state) } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` @@ -2074,6 +2194,16 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { ) -> bool { set_contains_id(state) } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: access of `&T` is a subset of `&mut T` @@ -2234,6 +2364,16 @@ unsafe impl<'__w, T: Component> WorldQuery for Mut<'__w, T> { ) -> bool { <&mut T as WorldQuery>::matches_component_set(state, set_contains_id) } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } // SAFETY: access of `Ref` is a subset of `Mut` @@ -2384,6 +2524,16 @@ unsafe impl WorldQuery for Option { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } // SAFETY: defers to soundness of `T: WorldQuery` impl @@ -2570,6 +2720,16 @@ unsafe impl WorldQuery for Has { // `Has` always matches true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` @@ -2721,6 +2881,10 @@ macro_rules! impl_anytuple_fetch { clippy::unused_unit, reason = "Zero-length tuples will generate some function bodies equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." )] + #[allow( + unused_mut, + reason = "Zero-length tuples won't mutate any of the parameters." + )] /// SAFETY: /// `fetch` accesses are a subset of the subqueries' accesses /// This is sound because `update_component_access` adds accesses according to the implementations of all the subqueries. @@ -2815,6 +2979,55 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = _state; false $(|| $name::matches_component_set($name, _set_contains_id))* } + + #[inline] + unsafe fn find_table_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + table: &Table, + rows: Range, + ) -> Range { + if Self::IS_ARCHETYPAL { + rows + } else { + let ($($name,)*) = fetch; + let ($($state,)*) = state; + let mut new_rows = rows.end..rows.end; + // SAFETY: invariants are upheld by the caller. + $(new_rows = new_rows.union_with(unsafe { $name::find_table_chunk($state, &$name.0, table, rows.clone()) });)* + new_rows + } + } + + #[inline] + unsafe fn find_archetype_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + archetype: &Archetype, + mut indices: Range, + ) -> Range { + if Self::IS_ARCHETYPAL { + indices + } else { + let ($($name,)*) = fetch; + let ($($state,)*) = state; + let mut new_indices = indices.end..indices.end; + // SAFETY: invariants are upheld by the caller. + $(new_indices = new_indices.union_with(unsafe { $name::find_archetype_chunk($state, &$name.0, archetype, indices.clone()) });)* + new_indices + } + } + + #[inline] + unsafe fn matches( + state: &Self::State, + fetch: &Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool { + todo!() // TODO: weirdness with `fetch` below. Make sure find_table_chunk and + // find_archetype_chunk are fine too. + } } #[expect( @@ -2977,6 +3190,16 @@ unsafe impl WorldQuery for NopWorldQuery { ) -> bool { D::matches_component_set(state, set_contains_id) } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self::ReadOnly` is `Self` @@ -3067,6 +3290,16 @@ unsafe impl WorldQuery for PhantomData { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self::ReadOnly` is `Self` @@ -3277,6 +3510,16 @@ mod tests { ) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } /// SAFETY: `Self` is the same as `Self::ReadOnly` diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 4cfed81e4c551..b4969eb837142 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -3,13 +3,13 @@ use crate::{ change_detection::Tick, component::{Component, ComponentId, Components, StorageType}, entity::{Entities, Entity}, - query::{DebugCheckedUnwrap, FilteredAccess, StorageSwitch, WorldQuery}, + query::{DebugCheckedUnwrap, FilteredAccess, RangeExt, StorageSwitch, WorldQuery}, storage::{ComponentSparseSet, Table, TableRow}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; use bevy_ptr::{ThinSlicePtr, UnsafeCellDeref}; use bevy_utils::prelude::DebugName; -use core::{cell::UnsafeCell, marker::PhantomData}; +use core::{cell::UnsafeCell, marker::PhantomData, ops::Range}; use variadics_please::all_tuples; /// Types that filter the results of a [`Query`]. @@ -191,6 +191,16 @@ unsafe impl WorldQuery for With { ) -> bool { set_contains_id(id) } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } // SAFETY: WorldQuery impl performs no access at all @@ -291,8 +301,17 @@ unsafe impl WorldQuery for Without { ) -> bool { !set_contains_id(id) } -} + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } +} // SAFETY: WorldQuery impl performs no access at all unsafe impl QueryFilter for Without { #[inline(always)] @@ -372,6 +391,10 @@ macro_rules! impl_or_query_filter { clippy::unused_unit, reason = "Zero-length tuples will generate some function bodies equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." )] + #[allow( + unused_mut, + reason = "Zero-length tuples won't mutate any of the parameters." + )] /// SAFETY: /// [`QueryFilter::filter_fetch`] accesses are a subset of the subqueries' accesses /// This is sound because `update_component_access` adds accesses according to the implementations of all the subqueries. @@ -479,6 +502,55 @@ macro_rules! impl_or_query_filter { let ($($filter,)*) = state; false $(|| $filter::matches_component_set($filter, set_contains_id))* } + + #[inline] + unsafe fn find_table_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + table: &Table, + rows: Range, + ) -> Range { + if Self::IS_ARCHETYPAL { + rows + } else { + let ($($filter,)*) = fetch; + let ($($state,)*) = state; + let mut new_rows = rows.end..rows.end; + // SAFETY: invariants are upheld by the caller. + $(new_rows = new_rows.union_with(unsafe { $filter::find_table_chunk($state, &$filter.fetch, table, rows.clone()) });)* + new_rows + } + } + + #[inline] + unsafe fn find_archetype_chunk( + state: &Self::State, + fetch: &Self::Fetch<'_>, + archetype: &Archetype, + mut indices: Range, + ) -> Range { + if Self::IS_ARCHETYPAL { + indices + } else { + let ($($filter,)*) = fetch; + let ($($state,)*) = state; + let mut new_indices = indices.end..indices.end; + // SAFETY: invariants are upheld by the caller. + $(new_indices = new_indices.union_with(unsafe { $filter::find_archetype_chunk($state, &$filter.fetch, archetype, indices.clone()) });)* + new_indices + } + } + + #[inline] + unsafe fn matches( + state: &Self::State, + fetch: &Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool { + todo!() // TODO: weirdness with `fetch` below. Make sure find_table_chunk and + // find_archetype_chunk are fine too. + } } #[expect( @@ -616,6 +688,16 @@ unsafe impl WorldQuery for Allow { // Allow always matches true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + _fetch: &Self::Fetch<'_>, + _entity: Entity, + _table_row: TableRow, + ) -> bool { + true + } } // SAFETY: WorldQuery impl performs no access at all @@ -818,6 +900,37 @@ unsafe impl WorldQuery for Added { ) -> bool { set_contains_id(id) } + + #[inline] + unsafe fn matches( + _state: &Self::State, + fetch: &Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool { + // SAFETY: The invariants are upheld by the caller. + fetch.ticks.extract( + |table| { + // SAFETY: set_table was previously called + let table = unsafe { table.debug_checked_unwrap() }; + // SAFETY: The caller ensures `table_row` is in range. + let tick = unsafe { table.get_unchecked(table_row.index()) }; + + tick.deref().is_newer_than(fetch.last_run, fetch.this_run) + }, + |sparse_set| { + // SAFETY: The caller ensures `entity` is in range. + let tick = unsafe { + sparse_set + .debug_checked_unwrap() + .get_added_tick(entity) + .debug_checked_unwrap() + }; + + tick.deref().is_newer_than(fetch.last_run, fetch.this_run) + }, + ) + } } // SAFETY: WorldQuery impl performs only read access on ticks @@ -1045,6 +1158,37 @@ unsafe impl WorldQuery for Changed { ) -> bool { set_contains_id(id) } + + #[inline] + unsafe fn matches( + _state: &Self::State, + fetch: &Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool { + // SAFETY: The invariants are upheld by the caller. + fetch.ticks.extract( + |table| { + // SAFETY: set_table was previously called + let table = unsafe { table.debug_checked_unwrap() }; + // SAFETY: The caller ensures `table_row` is in range. + let tick = unsafe { table.get_unchecked(table_row.index()) }; + + tick.deref().is_newer_than(fetch.last_run, fetch.this_run) + }, + |sparse_set| { + // SAFETY: The caller ensures `entity` is in range. + let tick = unsafe { + sparse_set + .debug_checked_unwrap() + .get_changed_tick(entity) + .debug_checked_unwrap() + }; + + tick.deref().is_newer_than(fetch.last_run, fetch.this_run) + }, + ) + } } // SAFETY: WorldQuery impl performs only read access on ticks @@ -1110,7 +1254,7 @@ unsafe impl QueryFilter for Changed { /// # use bevy_ecs::query::SpawnDetails; /// /// fn system1(query: Query) { -/// for entity in &query { /* entity spawned */ } +/// for entity in &query { /* entity spawned*/ } /// } /// /// fn system2(query: Query<(Entity, SpawnDetails)>) { @@ -1199,6 +1343,23 @@ unsafe impl WorldQuery for Spawned { fn matches_component_set(_state: &(), _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { true } + + #[inline] + unsafe fn matches( + _state: &Self::State, + fetch: &Self::Fetch<'_>, + entity: Entity, + _table_row: TableRow, + ) -> bool { + // SAFETY: only living entities are queried + let spawned = unsafe { + fetch + .entities + .entity_get_spawned_or_despawned_unchecked(entity) + .1 + }; + spawned.is_newer_than(fetch.last_run, fetch.this_run) + } } // SAFETY: WorldQuery impl accesses no components or component ticks diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 9b2bfb2292003..e475d0b75b717 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -236,16 +236,31 @@ pub unsafe trait WorldQuery { ) -> bool; } +/// Extension trait for `Range`, adding convenience methods for use with [`WorldQuery::find_table_chunk`] +/// and [`WorldQuery::find_archetype_chunk`]. pub trait RangeExt { + /// Returns the union of `self` and `other`, i.e. the smallest range which fully + /// covers each of them. + fn union_with(self, other: Self) -> Self; + /// Returns the next contiguous segment for which `func` returns true for all + /// indices in the segment. fn find_chunk bool>(self, func: F) -> Self; } impl RangeExt for Range { + #[inline] + fn union_with(self, other: Self) -> Self { + Range { + start: self.start.min(other.start), + end: self.end.min(other.end), + } + } + #[inline] fn find_chunk bool>(mut self, mut func: F) -> Self { let mut index = self.start; while index < self.end { - // index is taken from an exclusive range, so it can't be max + // SAFETY: index is taken from an exclusive range, so it can't be max let nonmax_index = unsafe { NonMaxU32::new_unchecked(index) }; let matches = func(nonmax_index); if matches { @@ -256,7 +271,7 @@ impl RangeExt for Range { self.start = index; while index < self.end { - // index is taken from an exclusive range, so it can't be max + // SAFETY: index is taken from an exclusive range, so it can't be max let nonmax_index = unsafe { NonMaxU32::new_unchecked(index) }; index = index.wrapping_add(1); let matches = func(nonmax_index); @@ -385,7 +400,7 @@ macro_rules! impl_tuple_world_query { unsafe fn find_archetype_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - table: &Archetype, + archetype: &Archetype, mut indices: Range, ) -> Range { if Self::IS_ARCHETYPAL { @@ -396,7 +411,7 @@ macro_rules! impl_tuple_world_query { // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's // always valid to pass to the next term. Other invariants are upheld by // the caller. - $(indices = unsafe { $name::find_archetype_chunk($state, $name, table, indices) };)* + $(indices = unsafe { $name::find_archetype_chunk($state, $name, archetype, indices) };)* indices } } diff --git a/crates/bevy_ecs/src/storage/table/mod.rs b/crates/bevy_ecs/src/storage/table/mod.rs index bd9a182ad9e4c..9f3418e766836 100644 --- a/crates/bevy_ecs/src/storage/table/mod.rs +++ b/crates/bevy_ecs/src/storage/table/mod.rs @@ -12,7 +12,7 @@ pub use column::*; use core::{ cell::UnsafeCell, num::NonZeroUsize, - ops::{Index, IndexMut, Range}, + ops::{Index, IndexMut}, panic::Location, }; use nonmax::NonMaxU32; From 5d8db809bdfcc49453f9e4b04c66224c555404fa Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Wed, 7 Jan 2026 18:56:07 -0800 Subject: [PATCH 06/27] remove `QueryFilter::filter_fetch` --- crates/bevy_ecs/macros/src/query_filter.rs | 15 +- crates/bevy_ecs/src/query/filter.rs | 230 ++------------------- 2 files changed, 17 insertions(+), 228 deletions(-) diff --git a/crates/bevy_ecs/macros/src/query_filter.rs b/crates/bevy_ecs/macros/src/query_filter.rs index f688cb0380744..a7f44ebc53592 100644 --- a/crates/bevy_ecs/macros/src/query_filter.rs +++ b/crates/bevy_ecs/macros/src/query_filter.rs @@ -71,20 +71,7 @@ pub fn derive_query_filter_impl(input: TokenStream) -> TokenStream { ); let filter_impl = quote! { - // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. - unsafe impl #user_impl_generics #path::query::QueryFilter - for #struct_name #user_ty_generics #user_where_clauses { - #[allow(unused_variables)] - #[inline(always)] - unsafe fn filter_fetch<'__w>( - _state: &Self::State, - _fetch: &mut ::Fetch<'__w>, - _entity: #path::entity::Entity, - _table_row: #path::storage::TableRow, - ) -> bool { - true #(&& <#field_types>::filter_fetch(&_state.#field_aliases, &mut _fetch.#field_aliases, _entity, _table_row))* - } - } + impl #user_impl_generics #path::query::QueryFilter for #struct_name #user_ty_generics #user_where_clauses {} }; let filter_asserts = quote! { diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index b4969eb837142..ec3d110f1225e 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -71,37 +71,12 @@ use variadics_please::all_tuples; /// ``` /// /// [`Query`]: crate::system::Query -/// -/// # Safety -/// -/// The [`WorldQuery`] implementation must not take any mutable access. -/// This is the same safety requirement as [`ReadOnlyQueryData`](crate::query::ReadOnlyQueryData). #[diagnostic::on_unimplemented( message = "`{Self}` is not a valid `Query` filter", label = "invalid `Query` filter", note = "a `QueryFilter` typically uses a combination of `With` and `Without` statements" )] -pub unsafe trait QueryFilter: WorldQuery { - /// Returns true if the provided [`Entity`] and [`TableRow`] should be included in the query results. - /// If false, the entity will be skipped. - /// - /// Note that this is called after already restricting the matched [`Table`]s and [`Archetype`]s to the - /// ones that are compatible with the Filter's access. - /// - /// Implementors of this method will generally either have a trivial `true` body (required for archetypal filters), - /// or access the necessary data within this function to make the final decision on filter inclusion. - /// - /// # Safety - /// - /// Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and - /// `table_row` must be in the range of the current table and archetype. - unsafe fn filter_fetch( - state: &Self::State, - fetch: &mut Self::Fetch<'_>, - entity: Entity, - table_row: TableRow, - ) -> bool; -} +pub trait QueryFilter: WorldQuery {} /// Filter that selects entities with a component `T`. /// @@ -203,18 +178,7 @@ unsafe impl WorldQuery for With { } } -// SAFETY: WorldQuery impl performs no access at all -unsafe impl QueryFilter for With { - #[inline(always)] - unsafe fn filter_fetch( - _state: &Self::State, - _fetch: &mut Self::Fetch<'_>, - _entity: Entity, - _table_row: TableRow, - ) -> bool { - true - } -} +impl QueryFilter for With {} /// Filter that selects entities without a component `T`. /// @@ -312,18 +276,8 @@ unsafe impl WorldQuery for Without { true } } -// SAFETY: WorldQuery impl performs no access at all -unsafe impl QueryFilter for Without { - #[inline(always)] - unsafe fn filter_fetch( - _state: &Self::State, - _fetch: &mut Self::Fetch<'_>, - _entity: Entity, - _table_row: TableRow, - ) -> bool { - true - } -} + +impl QueryFilter for Without {} /// A filter that tests if any of the given filters apply. /// @@ -396,6 +350,8 @@ macro_rules! impl_or_query_filter { reason = "Zero-length tuples won't mutate any of the parameters." )] /// SAFETY: + /// [`WorldQuery::matches`], [`WorldQuery::find_table_chunk`], and [`WorldQuery::find_archetype_chunk`] only + /// call their respective methods on subqueries, so they can't perform mutable access unless subqueries are invalid. /// [`QueryFilter::filter_fetch`] accesses are a subset of the subqueries' accesses /// This is sound because `update_component_access` adds accesses according to the implementations of all the subqueries. /// `update_component_access` replace the filters with a disjunction where every element is a conjunction of the previous filters and the filters of one of the subqueries. @@ -553,75 +509,15 @@ macro_rules! impl_or_query_filter { } } - #[expect( - clippy::allow_attributes, - reason = "This is a tuple-related macro; as such the lints below may not always apply." - )] - #[allow( - non_snake_case, - reason = "The names of some variables are provided by the macro's caller, not by us." - )] - #[allow( - unused_variables, - reason = "Zero-length tuples won't use any of the parameters." - )] $(#[$meta])* - // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. - unsafe impl<$($filter: QueryFilter),*> QueryFilter for Or<($($filter,)*)> { - #[inline(always)] - unsafe fn filter_fetch( - state: &Self::State, - fetch: &mut Self::Fetch<'_>, - entity: Entity, - table_row: TableRow - ) -> bool { - let ($($state,)*) = state; - let ($($filter,)*) = fetch; - // If this is an archetypal query, then it is guaranteed to return true, - // and we can help the compiler remove branches by checking the const `IS_ARCHETYPAL` first. - (Self::IS_ARCHETYPAL - // SAFETY: The invariants are upheld by the caller. - $(|| ($filter.matches && unsafe { $filter::filter_fetch($state, &mut $filter.fetch, entity, table_row) }))* - // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. - // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, - // so we treat them as matching for non-archetypal queries, as well. - || !(false $(|| $filter.matches)*)) - } - } + impl<$($filter: QueryFilter),*> QueryFilter for Or<($($filter,)*)> {} }; } macro_rules! impl_tuple_query_filter { - ($(#[$meta:meta])* $(($name: ident, $state: ident)),*) => { - #[expect( - clippy::allow_attributes, - reason = "This is a tuple-related macro; as such the lints below may not always apply." - )] - #[allow( - non_snake_case, - reason = "The names of some variables are provided by the macro's caller, not by us." - )] - #[allow( - unused_variables, - reason = "Zero-length tuples won't use any of the parameters." - )] + ($(#[$meta:meta])* $($name: ident),*) => { $(#[$meta])* - // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. - unsafe impl<$($name: QueryFilter),*> QueryFilter for ($($name,)*) { - #[inline(always)] - unsafe fn filter_fetch( - state: &Self::State, - fetch: &mut Self::Fetch<'_>, - entity: Entity, - table_row: TableRow - ) -> bool { - let ($($state,)*) = state; - let ($($name,)*) = fetch; - // SAFETY: The invariants are upheld by the caller. - true $(&& unsafe { $name::filter_fetch($state, $name, entity, table_row) })* - } - } - + impl<$($name: QueryFilter),*> QueryFilter for ($($name,)*) {} }; } @@ -630,8 +526,7 @@ all_tuples!( impl_tuple_query_filter, 0, 15, - F, - S + F ); all_tuples!( #[doc(fake_variadic)] @@ -700,18 +595,7 @@ unsafe impl WorldQuery for Allow { } } -// SAFETY: WorldQuery impl performs no access at all -unsafe impl QueryFilter for Allow { - #[inline(always)] - unsafe fn filter_fetch( - _: &Self::State, - _: &mut Self::Fetch<'_>, - _: Entity, - _: TableRow, - ) -> bool { - true - } -} +impl QueryFilter for Allow {} /// A filter on a component that only retains results the first time after they have been added. /// @@ -804,7 +688,7 @@ impl Clone for AddedFetch<'_, T> { } /// SAFETY: -/// [`QueryFilter::filter_fetch`] accesses a single component in a readonly way. +/// [`WorldQuery::matches`] accesses a single component in a readonly way. /// This is sound because `update_component_access` adds read access for that component and panics when appropriate. /// `update_component_access` adds a `With` filter for a component. /// This is sound because `matches_component_set` returns whether the set contains that component. @@ -933,39 +817,7 @@ unsafe impl WorldQuery for Added { } } -// SAFETY: WorldQuery impl performs only read access on ticks -unsafe impl QueryFilter for Added { - #[inline(always)] - unsafe fn filter_fetch( - _state: &Self::State, - fetch: &mut Self::Fetch<'_>, - entity: Entity, - table_row: TableRow, - ) -> bool { - // SAFETY: The invariants are upheld by the caller. - fetch.ticks.extract( - |table| { - // SAFETY: set_table was previously called - let table = unsafe { table.debug_checked_unwrap() }; - // SAFETY: The caller ensures `table_row` is in range. - let tick = unsafe { table.get_unchecked(table_row.index()) }; - - tick.deref().is_newer_than(fetch.last_run, fetch.this_run) - }, - |sparse_set| { - // SAFETY: The caller ensures `entity` is in range. - let tick = unsafe { - sparse_set - .debug_checked_unwrap() - .get_added_tick(entity) - .debug_checked_unwrap() - }; - - tick.deref().is_newer_than(fetch.last_run, fetch.this_run) - }, - ) - } -} +impl QueryFilter for Added {} /// A filter on a component that only retains results the first time after they have been added or mutably dereferenced. /// @@ -1062,7 +914,7 @@ impl Clone for ChangedFetch<'_, T> { } /// SAFETY: -/// `fetch` accesses a single component in a readonly way. +/// `matches` accesses a single component in a readonly way. /// This is sound because `update_component_access` add read access for that component and panics when appropriate. /// `update_component_access` adds a `With` filter for a component. /// This is sound because `matches_component_set` returns whether the set contains that component. @@ -1191,39 +1043,7 @@ unsafe impl WorldQuery for Changed { } } -// SAFETY: WorldQuery impl performs only read access on ticks -unsafe impl QueryFilter for Changed { - #[inline(always)] - unsafe fn filter_fetch( - _state: &Self::State, - fetch: &mut Self::Fetch<'_>, - entity: Entity, - table_row: TableRow, - ) -> bool { - // SAFETY: The invariants are upheld by the caller. - fetch.ticks.extract( - |table| { - // SAFETY: set_table was previously called - let table = unsafe { table.debug_checked_unwrap() }; - // SAFETY: The caller ensures `table_row` is in range. - let tick = unsafe { table.get_unchecked(table_row.index()) }; - - tick.deref().is_newer_than(fetch.last_run, fetch.this_run) - }, - |sparse_set| { - // SAFETY: The caller ensures `entity` is in range. - let tick = unsafe { - sparse_set - .debug_checked_unwrap() - .get_changed_tick(entity) - .debug_checked_unwrap() - }; - - tick.deref().is_newer_than(fetch.last_run, fetch.this_run) - }, - ) - } -} +impl QueryFilter for Changed {} /// A filter that only retains results the first time after the entity has been spawned. /// @@ -1362,25 +1182,7 @@ unsafe impl WorldQuery for Spawned { } } -// SAFETY: WorldQuery impl accesses no components or component ticks -unsafe impl QueryFilter for Spawned { - #[inline(always)] - unsafe fn filter_fetch( - _state: &Self::State, - fetch: &mut Self::Fetch<'_>, - entity: Entity, - _table_row: TableRow, - ) -> bool { - // SAFETY: only living entities are queried - let spawned = unsafe { - fetch - .entities - .entity_get_spawned_or_despawned_unchecked(entity) - .1 - }; - spawned.is_newer_than(fetch.last_run, fetch.this_run) - } -} +impl QueryFilter for Spawned {} /// A marker trait to indicate that the filter works at an archetype level. /// From de006fd9de94a90932de165cf32245b64e138ad8 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Wed, 7 Jan 2026 19:18:36 -0800 Subject: [PATCH 07/27] remove Option wrapper from `QueryData::fetch` return type --- crates/bevy_ecs/macros/src/query_data.rs | 16 ++-- crates/bevy_ecs/src/query/fetch.rs | 100 +++++++++++------------ crates/bevy_ecs/src/query/mod.rs | 3 +- 3 files changed, 57 insertions(+), 62 deletions(-) diff --git a/crates/bevy_ecs/macros/src/query_data.rs b/crates/bevy_ecs/macros/src/query_data.rs index 70adaf5a9de33..1b66cdde2c7f2 100644 --- a/crates/bevy_ecs/macros/src/query_data.rs +++ b/crates/bevy_ecs/macros/src/query_data.rs @@ -261,10 +261,10 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { _fetch: &mut ::Fetch<'__w>, _entity: #path::entity::Entity, _table_row: #path::storage::TableRow, - ) -> Option> { - Some(Self::Item { - #(#field_members: <#read_only_field_types>::fetch(&_state.#field_aliases, &mut _fetch.#field_aliases, _entity, _table_row)?,)* - }) + ) -> Self::Item<'__w, '__s> { + Self::Item { + #(#field_members: <#read_only_field_types>::fetch(&_state.#field_aliases, &mut _fetch.#field_aliases, _entity, _table_row),)* + } } fn iter_access( @@ -331,10 +331,10 @@ pub fn derive_query_data_impl(input: TokenStream) -> TokenStream { _fetch: &mut ::Fetch<'__w>, _entity: #path::entity::Entity, _table_row: #path::storage::TableRow, - ) -> Option> { - Some(Self::Item { - #(#field_members: <#field_types>::fetch(&_state.#field_aliases, &mut _fetch.#field_aliases, _entity, _table_row)?,)* - }) + ) -> Self::Item<'__w, '__s> { + Self::Item { + #(#field_members: <#field_types>::fetch(&_state.#field_aliases, &mut _fetch.#field_aliases, _entity, _table_row),)* + } } fn iter_access( diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index ccc07aaf7a95b..d6624e9151e82 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -329,13 +329,15 @@ pub unsafe trait QueryData: WorldQuery { /// /// - Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and /// `table_row` must be in the range of the current table and archetype. + /// - Must always be called _after_ [`WorldQuery::matches`], or after one of [`WorldQuery::find_table_chunk`] + /// or [`WorldQuery::find_archetype_chunk`] and the provided table row/index is within the range returned. /// - There must not be simultaneous conflicting component access registered in `update_component_access`. unsafe fn fetch<'w, 's>( state: &'s Self::State, fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Option>; + ) -> Self::Item<'w, 's>; /// Returns an iterator over the access needed by [`QueryData::fetch`]. Access conflicts are usually /// checked in [`WorldQuery::update_component_access`], but in certain cases this method can be useful to implement @@ -456,8 +458,8 @@ unsafe impl QueryData for Entity { _fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Option> { - Some(entity) + ) -> Self::Item<'w, 's> { + entity } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -562,9 +564,9 @@ unsafe impl QueryData for EntityLocation { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { // SAFETY: `fetch` must be called with an entity that exists in the world - Some(unsafe { fetch.get_spawned(entity).debug_checked_unwrap() }) + unsafe { fetch.get_spawned(entity).debug_checked_unwrap() } } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -742,19 +744,19 @@ unsafe impl QueryData for SpawnDetails { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { // SAFETY: only living entities are queried let (spawned_by, spawn_tick) = unsafe { fetch .entities .entity_get_spawned_or_despawned_unchecked(entity) }; - Some(Self { + Self { spawned_by, spawn_tick, last_run: fetch.last_run, this_run: fetch.this_run, - }) + } } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -878,7 +880,7 @@ unsafe impl<'a> QueryData for EntityRef<'a> { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { fetch @@ -887,7 +889,7 @@ unsafe impl<'a> QueryData for EntityRef<'a> { .debug_checked_unwrap() }; // SAFETY: Read-only access to every component has been registered. - Some(unsafe { EntityRef::new(cell) }) + unsafe { EntityRef::new(cell) } } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -998,7 +1000,7 @@ unsafe impl<'a> QueryData for EntityMut<'a> { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { fetch @@ -1007,7 +1009,7 @@ unsafe impl<'a> QueryData for EntityMut<'a> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - Some(unsafe { EntityMut::new(cell) }) + unsafe { EntityMut::new(cell) } } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -1136,7 +1138,7 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityRef<'a, 'b> { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { fetch @@ -1145,7 +1147,7 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityRef<'a, 'b> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - Some(unsafe { FilteredEntityRef::new(cell, access) }) + unsafe { FilteredEntityRef::new(cell, access) } } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -1269,7 +1271,7 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { // SAFETY: `fetch` must be called with an entity that exists in the world let cell = unsafe { fetch @@ -1278,7 +1280,7 @@ unsafe impl<'a, 'b> QueryData for FilteredEntityMut<'a, 'b> { .debug_checked_unwrap() }; // SAFETY: mutable access to every component has been registered. - Some(unsafe { FilteredEntityMut::new(cell, access) }) + unsafe { FilteredEntityMut::new(cell, access) } } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -1397,12 +1399,12 @@ where fetch: &mut Self::Fetch<'w>, entity: Entity, _: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { let cell = fetch .world .get_entity_with_ticks(entity, fetch.last_run, fetch.this_run) .unwrap(); - Some(EntityRefExcept::new(cell, access)) + EntityRefExcept::new(cell, access) } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -1526,12 +1528,12 @@ where fetch: &mut Self::Fetch<'w>, entity: Entity, _: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { let cell = fetch .world .get_entity_with_ticks(entity, fetch.last_run, fetch.this_run) .unwrap(); - Some(EntityMutExcept::new(cell, access)) + EntityMutExcept::new(cell, access) } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -1627,12 +1629,12 @@ unsafe impl QueryData for &Archetype { fetch: &mut Self::Fetch<'w>, entity: Entity, _table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { let (entities, archetypes) = *fetch; // SAFETY: `fetch` must be called with an entity that exists in the world let location = unsafe { entities.get_spawned(entity).debug_checked_unwrap() }; // SAFETY: The assigned archetype for a living entity must always be valid. - Some(unsafe { archetypes.get(location.archetype_id).debug_checked_unwrap() }) + unsafe { archetypes.get(location.archetype_id).debug_checked_unwrap() } } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -1797,8 +1799,8 @@ unsafe impl QueryData for &T { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Option> { - Some(fetch.components.extract( + ) -> Self::Item<'w, 's> { + fetch.components.extract( |table| { // SAFETY: set_table was previously called let table = unsafe { table.debug_checked_unwrap() }; @@ -1816,7 +1818,7 @@ unsafe impl QueryData for &T { }; item.deref() }, - )) + ) } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -1997,8 +1999,8 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Option> { - Some(fetch.components.extract( + ) -> Self::Item<'w, 's> { + fetch.components.extract( |table| { // SAFETY: set_table was previously called let (table_components, added_ticks, changed_ticks, callers) = @@ -2043,7 +2045,7 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> { ), } }, - )) + ) } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -2224,8 +2226,8 @@ unsafe impl<'__w, T: Component> QueryData for &'__w mut T fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Option> { - Some(fetch.components.extract( + ) -> Self::Item<'w, 's> { + fetch.components.extract( |table| { // SAFETY: set_table was previously called let (table_components, added_ticks, changed_ticks, callers) = @@ -2270,7 +2272,7 @@ unsafe impl<'__w, T: Component> QueryData for &'__w mut T ), } }, - )) + ) } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -2398,7 +2400,7 @@ unsafe impl<'__w, T: Component> QueryData for Mut<'__w, T> fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { <&mut T as QueryData>::fetch(state, fetch, entity, table_row) } @@ -2554,14 +2556,11 @@ unsafe impl QueryData for Option { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Option> { - Some( - fetch - .matches - // SAFETY: The invariants are upheld by the caller. - .then(|| unsafe { T::fetch(state, &mut fetch.fetch, entity, table_row) }) - .flatten(), - ) + ) -> Self::Item<'w, 's> { + fetch + .matches + // SAFETY: The invariants are upheld by the caller. + .then(|| unsafe { T::fetch(state, &mut fetch.fetch, entity, table_row) }) } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -2750,8 +2749,8 @@ unsafe impl QueryData for Has { fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Option> { - Some(*fetch) + ) -> Self::Item<'w, 's> { + *fetch } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -2825,11 +2824,11 @@ macro_rules! impl_tuple_query_data { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow - ) -> Option> { + ) -> Self::Item<'w, 's> { let ($($state,)*) = state; let ($($name,)*) = fetch; // SAFETY: The invariants are upheld by the caller. - Some(($(unsafe { $name::fetch($state, $name, entity, table_row) }?,)*)) + ($(unsafe { $name::fetch($state, $name, entity, table_row) },)*) } fn iter_access(state: &Self::State) -> impl Iterator> { @@ -3066,10 +3065,10 @@ macro_rules! impl_anytuple_fetch { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow - ) -> Option> { + ) -> Self::Item<'w, 's> { let ($($name,)*) = _fetch; let ($($state,)*) = _state; - let result = ($( + ($( // SAFETY: The invariants are required to be upheld by the caller. $name.1.then(|| unsafe { $name::fetch($state, &mut $name.0, _entity, _table_row) }).flatten(), )*); @@ -3219,8 +3218,7 @@ unsafe impl QueryData for NopWorldQuery { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Option> { - Some(()) + ) -> Self::Item<'w, 's> { } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -3318,8 +3316,7 @@ unsafe impl QueryData for PhantomData { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Option> { - Some(()) + ) -> Self::Item<'w, 's> { } fn iter_access(_state: &Self::State) -> impl Iterator> { @@ -3540,8 +3537,7 @@ mod tests { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Option> { - Some(()) + ) -> Self::Item<'w, 's> { } fn iter_access(_state: &Self::State) -> impl Iterator> { diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 4548b075bc2ed..5ba94af5e51ec 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -911,8 +911,7 @@ mod tests { _fetch: &mut Self::Fetch<'w>, _entity: Entity, _table_row: TableRow, - ) -> Option> { - Some(()) + ) -> Self::Item<'w, 's> { } fn iter_access( From 227d82f8f5ae6286be8fc5a5a1a1f594b8eb28d1 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Thu, 8 Jan 2026 16:17:05 -0800 Subject: [PATCH 08/27] fix union impl --- crates/bevy_ecs/src/query/fetch.rs | 117 +++++++++++++++-------- crates/bevy_ecs/src/query/filter.rs | 90 ++++++++++------- crates/bevy_ecs/src/query/world_query.rs | 21 ++-- 3 files changed, 144 insertions(+), 84 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index d6624e9151e82..4c629f17c2f56 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -2776,6 +2776,13 @@ impl ArchetypeQueryData for Has {} /// Entities are guaranteed to have at least one of the components in `T`. pub struct AnyOf(PhantomData); +#[doc(hidden)] +#[derive(Clone)] +pub struct AnyOfFetch { + fetch: T, + matches: bool, +} + macro_rules! impl_tuple_query_data { ($(#[$meta:meta])* $(($name: ident, $item: ident, $state: ident)),*) => { #[expect( @@ -2890,21 +2897,33 @@ macro_rules! impl_anytuple_fetch { /// `update_component_access` replaces the filters with a disjunction where every element is a conjunction of the previous filters and the filters of one of the subqueries. /// This is sound because `matches_component_set` returns a disjunction of the results of the subqueries' implementations. unsafe impl<$($name: WorldQuery),*> WorldQuery for AnyOf<($($name,)*)> { - type Fetch<'w> = ($(($name::Fetch<'w>, bool),)*); + type Fetch<'w> = AnyOfFetch<($(AnyOfFetch<$name::Fetch<'w>>,)*)>; type State = ($($name::State,)*); fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { - let ($($name,)*) = fetch; - ($( - ($name::shrink_fetch($name.0), $name.1), - )*) + let ($($name,)*) = fetch.fetch; + AnyOfFetch { + fetch: + ($(AnyOfFetch { + fetch: $name::shrink_fetch($name.fetch), + matches: $name.matches, + },)*), + matches: fetch.matches, + } } #[inline] unsafe fn init_fetch<'w, 's>(_world: UnsafeWorldCell<'w>, state: &'s Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; - // SAFETY: The invariants are upheld by the caller. - ($(( unsafe { $name::init_fetch(_world, $name, _last_run, _this_run) }, false),)*) + AnyOfFetch { + fetch: + ($(AnyOfFetch { + // SAFETY: The invariants are upheld by the caller. + fetch: unsafe { $name::init_fetch(_world, $name, _last_run, _this_run) }, + matches: false, + },)*), + matches: false, + } } const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; @@ -2917,26 +2936,30 @@ macro_rules! impl_anytuple_fetch { _archetype: &'w Archetype, _table: &'w Table ) { - let ($($name,)*) = _fetch; + let ($($name,)*) = &mut _fetch.fetch; let ($($state,)*) = _state; + _fetch.matches = false; $( - $name.1 = $name::matches_component_set($state, &|id| _archetype.contains(id)); - if $name.1 { + $name.matches = $name::matches_component_set($state, &|id| _archetype.contains(id)); + _fetch.matches = _fetch.matches || $name.matches; + if $name.matches { // SAFETY: The invariants are upheld by the caller. - unsafe { $name::set_archetype(&mut $name.0, $state, _archetype, _table); } + unsafe { $name::set_archetype(&mut $name.fetch, $state, _archetype, _table); } } )* } #[inline] unsafe fn set_table<'w, 's>(_fetch: &mut Self::Fetch<'w>, _state: &'s Self::State, _table: &'w Table) { - let ($($name,)*) = _fetch; + let ($($name,)*) = &mut _fetch.fetch; let ($($state,)*) = _state; + _fetch.matches = false; $( - $name.1 = $name::matches_component_set($state, &|id| _table.has_column(id)); - if $name.1 { + $name.matches = $name::matches_component_set($state, &|id| _table.has_column(id)); + _fetch.matches = _fetch.matches || $name.matches; + if $name.matches { // SAFETY: The invariants are required to be upheld by the caller. - unsafe { $name::set_table(&mut $name.0, $state, _table); } + unsafe { $name::set_table(&mut $name.fetch, $state, _table); } } )* } @@ -2986,15 +3009,19 @@ macro_rules! impl_anytuple_fetch { table: &Table, rows: Range, ) -> Range { - if Self::IS_ARCHETYPAL { + // If this is an archetypal query, it must match all entities. + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + if Self::IS_ARCHETYPAL || !fetch.matches { rows } else { - let ($($name,)*) = fetch; + let ($($name,)*) = &fetch.fetch; let ($($state,)*) = state; - let mut new_rows = rows.end..rows.end; + let mut next_chunk = rows.end..rows.end; // SAFETY: invariants are upheld by the caller. - $(new_rows = new_rows.union_with(unsafe { $name::find_table_chunk($state, &$name.0, table, rows.clone()) });)* - new_rows + $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_table_chunk($state, &$name.fetch, table, rows.clone()) });)* + next_chunk } } @@ -3005,15 +3032,19 @@ macro_rules! impl_anytuple_fetch { archetype: &Archetype, mut indices: Range, ) -> Range { - if Self::IS_ARCHETYPAL { + // If this is an archetypal query, it must match all entities. + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + if Self::IS_ARCHETYPAL || !fetch.matches { indices } else { - let ($($name,)*) = fetch; + let ($($name,)*) = &fetch.fetch; let ($($state,)*) = state; - let mut new_indices = indices.end..indices.end; + let mut next_chunk = indices.end..indices.end; // SAFETY: invariants are upheld by the caller. - $(new_indices = new_indices.union_with(unsafe { $name::find_archetype_chunk($state, &$name.0, archetype, indices.clone()) });)* - new_indices + $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_archetype_chunk($state, &$name.fetch, archetype, indices.clone()) });)* + next_chunk } } @@ -3024,8 +3055,18 @@ macro_rules! impl_anytuple_fetch { entity: Entity, table_row: TableRow, ) -> bool { - todo!() // TODO: weirdness with `fetch` below. Make sure find_table_chunk and - // find_archetype_chunk are fine too. + // If this is an archetypal query, it must match all entities. + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + if Self::IS_ARCHETYPAL || !fetch.matches { + true + } else { + let ($($name,)*) = &fetch.fetch; + let ($($state,)*) = state; + // SAFETY: invariants are upheld by the caller. + false $(|| unsafe { $name::matches(&$state, &$name.fetch, entity, table_row) })* + } } } @@ -3066,24 +3107,16 @@ macro_rules! impl_anytuple_fetch { _entity: Entity, _table_row: TableRow ) -> Self::Item<'w, 's> { - let ($($name,)*) = _fetch; + let ($($name,)*) = &mut _fetch.fetch; let ($($state,)*) = _state; ($( // SAFETY: The invariants are required to be upheld by the caller. - $name.1.then(|| unsafe { $name::fetch($state, &mut $name.0, _entity, _table_row) }).flatten(), - )*); - // If this is an archetypal query, then it is guaranteed to return `Some`, - // and we can help the compiler remove branches by checking the const `IS_ARCHETYPAL` first. - (Self::IS_ARCHETYPAL - // We want to return `Some` if the query matches this entity, - // which happens if at least one subquery returns `Some`. - // So, fetch everything as usual, but if all the subqueries return `None` then return `None` instead. - || !matches!(result, ($(Option::>::None,)*)) - // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. - // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, - // so we treat them as matching for non-archetypal queries, as well. - || !(false $(|| $name.1)*)) - .then_some(result) + unsafe { $name::matches(&$state, &$name.fetch, _entity, _table_row) }.then(|| + // SAFETY: the above guard verifies this subquery matches the given entity. + // other invariants are upheld by the caller. + unsafe { $name::fetch($state, &mut $name.fetch, _entity, _table_row) } + ), + )*) } fn iter_access(state: &Self::State) -> impl Iterator> { diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index ec3d110f1225e..d0f053c8a0b5a 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -312,20 +312,12 @@ impl QueryFilter for Without {} pub struct Or(PhantomData); #[doc(hidden)] -pub struct OrFetch<'w, T: WorldQuery> { - fetch: T::Fetch<'w>, +#[derive(Clone)] +pub struct OrFetch { + fetch: T, matches: bool, } -impl Clone for OrFetch<'_, T> { - fn clone(&self) -> Self { - Self { - fetch: self.fetch.clone(), - matches: self.matches, - } - } -} - macro_rules! impl_or_query_filter { ($(#[$meta:meta])* $(($filter: ident, $state: ident)),*) => { $(#[$meta])* @@ -357,17 +349,21 @@ macro_rules! impl_or_query_filter { /// `update_component_access` replace the filters with a disjunction where every element is a conjunction of the previous filters and the filters of one of the subqueries. /// This is sound because `matches_component_set` returns a disjunction of the results of the subqueries' implementations. unsafe impl<$($filter: QueryFilter),*> WorldQuery for Or<($($filter,)*)> { - type Fetch<'w> = ($(OrFetch<'w, $filter>,)*); + type Fetch<'w> = OrFetch<($(OrFetch<$filter::Fetch<'w>>,)*)>; type State = ($($filter::State,)*); fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { - let ($($filter,)*) = fetch; - ($( - OrFetch { - fetch: $filter::shrink_fetch($filter.fetch), - matches: $filter.matches - }, - )*) + let ($($filter,)*) = fetch.fetch; + OrFetch { + fetch: + ($( + OrFetch { + fetch: $filter::shrink_fetch($filter.fetch), + matches: $filter.matches + }, + )*), + matches: fetch.matches, + } } const IS_DENSE: bool = true $(&& $filter::IS_DENSE)*; @@ -376,11 +372,15 @@ macro_rules! impl_or_query_filter { #[inline] unsafe fn init_fetch<'w, 's>(world: UnsafeWorldCell<'w>, state: &'s Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> { let ($($filter,)*) = state; - ($(OrFetch { - // SAFETY: The invariants are upheld by the caller. - fetch: unsafe { $filter::init_fetch(world, $filter, last_run, this_run) }, - matches: false, - },)*) + OrFetch { + fetch: + ($(OrFetch { + // SAFETY: The invariants are upheld by the caller. + fetch: unsafe { $filter::init_fetch(world, $filter, last_run, this_run) }, + matches: false, + },)*), + matches: false + } } #[inline] @@ -390,10 +390,12 @@ macro_rules! impl_or_query_filter { if Self::IS_ARCHETYPAL { return; } - let ($($filter,)*) = fetch; + let ($($filter,)*) = &mut fetch.fetch; let ($($state,)*) = state; + fetch.matches = false; $( $filter.matches = $filter::matches_component_set($state, &|id| table.has_column(id)); + fetch.matches = fetch.matches || $filter.matches; if $filter.matches { // SAFETY: The invariants are upheld by the caller. unsafe { $filter::set_table(&mut $filter.fetch, $state, table); } @@ -413,10 +415,12 @@ macro_rules! impl_or_query_filter { if Self::IS_ARCHETYPAL { return; } - let ($($filter,)*) = fetch; + let ($($filter,)*) = &mut fetch.fetch; let ($($state,)*) = &state; + fetch.matches = false; $( $filter.matches = $filter::matches_component_set($state, &|id| archetype.contains(id)); + fetch.matches = fetch.matches || $filter.matches; if $filter.matches { // SAFETY: The invariants are upheld by the caller. unsafe { $filter::set_archetype(&mut $filter.fetch, $state, archetype, table); } @@ -466,14 +470,18 @@ macro_rules! impl_or_query_filter { table: &Table, rows: Range, ) -> Range { - if Self::IS_ARCHETYPAL { + // If this is an archetypal query, it must match all entities. + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + if Self::IS_ARCHETYPAL || !fetch.matches { rows } else { - let ($($filter,)*) = fetch; + let ($($filter,)*) = &fetch.fetch; let ($($state,)*) = state; let mut new_rows = rows.end..rows.end; // SAFETY: invariants are upheld by the caller. - $(new_rows = new_rows.union_with(unsafe { $filter::find_table_chunk($state, &$filter.fetch, table, rows.clone()) });)* + $(new_rows = new_rows.union_or_first(unsafe { $filter::find_table_chunk($state, &$filter.fetch, table, rows.clone()) });)* new_rows } } @@ -485,14 +493,18 @@ macro_rules! impl_or_query_filter { archetype: &Archetype, mut indices: Range, ) -> Range { - if Self::IS_ARCHETYPAL { + // If this is an archetypal query, it must match all entities. + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + if Self::IS_ARCHETYPAL || !fetch.matches { indices } else { - let ($($filter,)*) = fetch; + let ($($filter,)*) = &fetch.fetch; let ($($state,)*) = state; let mut new_indices = indices.end..indices.end; // SAFETY: invariants are upheld by the caller. - $(new_indices = new_indices.union_with(unsafe { $filter::find_archetype_chunk($state, &$filter.fetch, archetype, indices.clone()) });)* + $(new_indices = new_indices.union_or_first(unsafe { $filter::find_archetype_chunk($state, &$filter.fetch, archetype, indices.clone()) });)* new_indices } } @@ -504,8 +516,18 @@ macro_rules! impl_or_query_filter { entity: Entity, table_row: TableRow, ) -> bool { - todo!() // TODO: weirdness with `fetch` below. Make sure find_table_chunk and - // find_archetype_chunk are fine too. + // If this is an archetypal query, it must match all entities. + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + if Self::IS_ARCHETYPAL || !fetch.matches { + true + } else { + let ($($filter,)*) = &fetch.fetch; + let ($($state,)*) = state; + // SAFETY: invariants are upheld by the caller. + false $(|| unsafe { $filter::matches(&$state, &$filter.fetch, entity, table_row) })* + } } } diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index e475d0b75b717..62a0f16a34c59 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -1,4 +1,4 @@ -use core::ops::Range; +use core::{mem, ops::Range}; use crate::{ archetype::Archetype, @@ -239,9 +239,9 @@ pub unsafe trait WorldQuery { /// Extension trait for `Range`, adding convenience methods for use with [`WorldQuery::find_table_chunk`] /// and [`WorldQuery::find_archetype_chunk`]. pub trait RangeExt { - /// Returns the union of `self` and `other`, i.e. the smallest range which fully - /// covers each of them. - fn union_with(self, other: Self) -> Self; + /// Returns the union of `self` and `other` if they overlap, otherwise returns + /// whichever is first. + fn union_or_first(self, other: Self) -> Self; /// Returns the next contiguous segment for which `func` returns true for all /// indices in the segment. fn find_chunk bool>(self, func: F) -> Self; @@ -249,11 +249,16 @@ pub trait RangeExt { impl RangeExt for Range { #[inline] - fn union_with(self, other: Self) -> Self { - Range { - start: self.start.min(other.start), - end: self.end.min(other.end), + fn union_or_first(mut self, mut other: Self) -> Self { + if self.start > other.start { + mem::swap(&mut self, &mut other); } + + if self.end >= other.start && self.end < other.end { + self.end = other.end + } + + self } #[inline] From 795ce1cb68baa46b0f86615ab0235c962915a0a9 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Fri, 9 Jan 2026 13:57:38 -0800 Subject: [PATCH 09/27] pass entity slices directly --- crates/bevy_ecs/macros/src/world_query.rs | 8 ++++---- crates/bevy_ecs/src/query/fetch.rs | 10 +++++----- crates/bevy_ecs/src/query/filter.rs | 10 +++++----- crates/bevy_ecs/src/query/world_query.rs | 18 ++++++++---------- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/crates/bevy_ecs/macros/src/world_query.rs b/crates/bevy_ecs/macros/src/world_query.rs index 1896a72234278..41258f4bcaa89 100644 --- a/crates/bevy_ecs/macros/src/world_query.rs +++ b/crates/bevy_ecs/macros/src/world_query.rs @@ -177,7 +177,7 @@ pub(crate) fn world_query_impl( unsafe fn find_table_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - table: &#path::storage::Table, + table_entities: &[#path::entity::Entity], mut rows: core::ops::Range, ) -> core::ops::Range { if Self::IS_ARCHETYPAL { @@ -186,7 +186,7 @@ pub(crate) fn world_query_impl( // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's // always valid to pass to the next term. Other invariants are upheld by // the caller. - #(rows = unsafe { <#field_types>::find_table_chunk(&state.#field_aliases, &fetch.#field_aliases, table, rows) };)* + #(rows = unsafe { <#field_types>::find_table_chunk(&state.#field_aliases, &fetch.#field_aliases, table_entities, rows) };)* rows } } @@ -195,7 +195,7 @@ pub(crate) fn world_query_impl( unsafe fn find_archetype_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - archetype: &#path::archetype::Archetype, + archetype_entities: &[#path::archetype::ArchetypeEntity], mut indices: core::ops::Range, ) -> core::ops::Range { if Self::IS_ARCHETYPAL { @@ -204,7 +204,7 @@ pub(crate) fn world_query_impl( // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's // always valid to pass to the next term. Other invariants are upheld by // the caller. - #(indices = unsafe { <#field_types>::find_archetype_chunk(&state.#field_aliases, &fetch.#field_aliases, archetype, indices) };)* + #(indices = unsafe { <#field_types>::find_archetype_chunk(&state.#field_aliases, &fetch.#field_aliases, archetype_entities, indices) };)* indices } } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 4c629f17c2f56..ce83cbe68ce16 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1,5 +1,5 @@ use crate::{ - archetype::{Archetype, Archetypes}, + archetype::{Archetype, ArchetypeEntity, Archetypes}, bundle::Bundle, change_detection::{ComponentTicksMut, ComponentTicksRef, MaybeLocation, Tick}, component::{Component, ComponentId, Components, Mutable, StorageType}, @@ -3006,7 +3006,7 @@ macro_rules! impl_anytuple_fetch { unsafe fn find_table_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - table: &Table, + table_entities: &[Entity], rows: Range, ) -> Range { // If this is an archetypal query, it must match all entities. @@ -3020,7 +3020,7 @@ macro_rules! impl_anytuple_fetch { let ($($state,)*) = state; let mut next_chunk = rows.end..rows.end; // SAFETY: invariants are upheld by the caller. - $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_table_chunk($state, &$name.fetch, table, rows.clone()) });)* + $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_table_chunk($state, &$name.fetch, table_entities, rows.clone()) });)* next_chunk } } @@ -3029,7 +3029,7 @@ macro_rules! impl_anytuple_fetch { unsafe fn find_archetype_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - archetype: &Archetype, + archetype_entities: &[ArchetypeEntity], mut indices: Range, ) -> Range { // If this is an archetypal query, it must match all entities. @@ -3043,7 +3043,7 @@ macro_rules! impl_anytuple_fetch { let ($($state,)*) = state; let mut next_chunk = indices.end..indices.end; // SAFETY: invariants are upheld by the caller. - $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_archetype_chunk($state, &$name.fetch, archetype, indices.clone()) });)* + $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_archetype_chunk($state, &$name.fetch, archetype_entities, indices.clone()) });)* next_chunk } } diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index d0f053c8a0b5a..3911743860ef8 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -1,5 +1,5 @@ use crate::{ - archetype::Archetype, + archetype::{Archetype, ArchetypeEntity}, change_detection::Tick, component::{Component, ComponentId, Components, StorageType}, entity::{Entities, Entity}, @@ -467,7 +467,7 @@ macro_rules! impl_or_query_filter { unsafe fn find_table_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - table: &Table, + table_entities: &[Entity], rows: Range, ) -> Range { // If this is an archetypal query, it must match all entities. @@ -481,7 +481,7 @@ macro_rules! impl_or_query_filter { let ($($state,)*) = state; let mut new_rows = rows.end..rows.end; // SAFETY: invariants are upheld by the caller. - $(new_rows = new_rows.union_or_first(unsafe { $filter::find_table_chunk($state, &$filter.fetch, table, rows.clone()) });)* + $(new_rows = new_rows.union_or_first(unsafe { $filter::find_table_chunk($state, &$filter.fetch, table_entities, rows.clone()) });)* new_rows } } @@ -490,7 +490,7 @@ macro_rules! impl_or_query_filter { unsafe fn find_archetype_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - archetype: &Archetype, + archetype_entities: &[ArchetypeEntity], mut indices: Range, ) -> Range { // If this is an archetypal query, it must match all entities. @@ -504,7 +504,7 @@ macro_rules! impl_or_query_filter { let ($($state,)*) = state; let mut new_indices = indices.end..indices.end; // SAFETY: invariants are upheld by the caller. - $(new_indices = new_indices.union_or_first(unsafe { $filter::find_archetype_chunk($state, &$filter.fetch, archetype, indices.clone()) });)* + $(new_indices = new_indices.union_or_first(unsafe { $filter::find_archetype_chunk($state, &$filter.fetch, archetype_entities, indices.clone()) });)* new_indices } } diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 62a0f16a34c59..34be815d36d77 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -1,7 +1,7 @@ use core::{mem, ops::Range}; use crate::{ - archetype::Archetype, + archetype::{Archetype, ArchetypeEntity}, change_detection::Tick, component::{ComponentId, Components}, entity::Entity, @@ -163,17 +163,16 @@ pub unsafe trait WorldQuery { unsafe fn find_table_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - table: &Table, + table_entities: &[Entity], rows: Range, ) -> Range { if Self::IS_ARCHETYPAL { rows } else { - let entities = table.entities(); rows.find_chunk(|index| { let table_row = TableRow::new(index); // SAFETY: caller guarantees `indices` is in range of `table` - let entity = unsafe { *entities.get_unchecked(index.get() as usize) }; + let entity = unsafe { *table_entities.get_unchecked(index.get() as usize) }; // SAFETY: invariants upheld by caller unsafe { Self::matches(state, fetch, entity, table_row) } }) @@ -190,13 +189,12 @@ pub unsafe trait WorldQuery { unsafe fn find_archetype_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - archetype: &Archetype, + archetype_entities: &[ArchetypeEntity], indices: Range, ) -> Range { if Self::IS_ARCHETYPAL { indices } else { - let archetype_entities = archetype.entities(); indices.find_chunk(|index| { // SAFETY: caller guarantees `indices` is in range of `archetype` let archetype_entity = @@ -385,7 +383,7 @@ macro_rules! impl_tuple_world_query { unsafe fn find_table_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - table: &Table, + table_entities: &[Entity], mut rows: Range, ) -> Range { if Self::IS_ARCHETYPAL { @@ -396,7 +394,7 @@ macro_rules! impl_tuple_world_query { // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's // always valid to pass to the next term. Other invariants are upheld by // the caller. - $(rows = unsafe { $name::find_table_chunk($state, $name, table, rows) };)* + $(rows = unsafe { $name::find_table_chunk($state, $name, table_entities, rows) };)* rows } } @@ -405,7 +403,7 @@ macro_rules! impl_tuple_world_query { unsafe fn find_archetype_chunk( state: &Self::State, fetch: &Self::Fetch<'_>, - archetype: &Archetype, + archetype_entities: &[ArchetypeEntity], mut indices: Range, ) -> Range { if Self::IS_ARCHETYPAL { @@ -416,7 +414,7 @@ macro_rules! impl_tuple_world_query { // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's // always valid to pass to the next term. Other invariants are upheld by // the caller. - $(indices = unsafe { $name::find_archetype_chunk($state, $name, archetype, indices) };)* + $(indices = unsafe { $name::find_archetype_chunk($state, $name, archetype_entities, indices) };)* indices } } From 06b7956de7683a07b439d164df9e185584a9c0a8 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Thu, 8 Jan 2026 21:11:41 -0800 Subject: [PATCH 10/27] fix query/iter.rs --- crates/bevy_ecs/src/query/iter.rs | 399 ++++++++++++------ crates/bevy_ecs/src/query/world_query.rs | 2 +- crates/bevy_ecs/src/system/query.rs | 22 +- .../bevy_ecs/src/world/unsafe_world_cell.rs | 14 +- 4 files changed, 293 insertions(+), 144 deletions(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 0bd178ba03727..63f8cfda12192 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -216,38 +216,56 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { table, ); - let entities = table.entities(); - for row in rows { - // SAFETY: Caller assures `row` in range of the current archetype. - let entity = unsafe { entities.get_unchecked(row as usize) }; - // SAFETY: This is from an exclusive range, so it can't be max. - let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(row)) }; - - // SAFETY: set_table was called prior. - // Caller assures `row` in range of the current archetype. - let fetched = unsafe { - !F::filter_fetch( + let table_entities = table.entities(); + + let mut next_chunk = rows.clone(); + while next_chunk.start < rows.end { + // SAFETY: + // - `set_table` was called prior + // - since `find_table_chunk` can only narrow ranges, and the caller + // assures `rows` is in range of the table, `next_chunk` must be as well. + next_chunk = unsafe { + D::find_table_chunk( + &self.query_state.fetch_state, + &self.cursor.fetch, + table_entities, + next_chunk, + ) + }; + // SAFETY: + // - `set_table` was called prior + // - since `find_table_chunk` can only narrow ranges, and the caller + // assures `rows` is in range of the table, `next_chunk` must be as well. + next_chunk = unsafe { + F::find_table_chunk( &self.query_state.filter_state, - &mut self.cursor.filter, - *entity, - row, + &self.cursor.filter, + table_entities, + next_chunk, ) }; - if fetched { - continue; - } - // SAFETY: set_table was called prior. - // Caller assures `row` in range of the current archetype. - if let Some(item) = D::fetch( - &self.query_state.fetch_state, - &mut self.cursor.fetch, - *entity, - row, - ) { + for row in next_chunk.clone() { + // SAFETY: Caller assures `row` in range of the current archetype. + let entity = unsafe { table_entities.get_unchecked(row as usize) }; + // SAFETY: This is from an exclusive range, so it can't be max. + let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(row)) }; + + // SAFETY: + // - set_table was called prior. + // - find_table_chunk was called prior, and this row was in the returned range. + let item = D::fetch( + &self.query_state.fetch_state, + &mut self.cursor.fetch, + *entity, + row, + ); accum = func(accum, item); } + + next_chunk = next_chunk.end..rows.end; } + accum } @@ -286,37 +304,55 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { table, ); - let entities = archetype.entities(); - for index in indices { - // SAFETY: Caller assures `index` in range of the current archetype. - let archetype_entity = unsafe { entities.get_unchecked(index as usize) }; + let archetype_entities = archetype.entities(); - // SAFETY: set_archetype was called prior. - // Caller assures `index` in range of the current archetype. - let fetched = unsafe { - !F::filter_fetch( + let mut next_chunk = indices.clone(); + while next_chunk.start < indices.end { + // SAFETY: + // - `set_archetype` was called prior + // - since `find_archetype_chunk` can only narrow ranges, and the caller + // assures `indices` is in range of the archetype, `next_chunk` must be as well. + next_chunk = unsafe { + D::find_archetype_chunk( + &self.query_state.fetch_state, + &self.cursor.fetch, + archetype_entities, + next_chunk, + ) + }; + // SAFETY: + // - `set_archetype` was called prior + // - since `find_archetype_chunk` can only narrow ranges, and the caller + // assures `indices` is in range of the archetype, `next_chunk` must be as well. + next_chunk = unsafe { + F::find_archetype_chunk( &self.query_state.filter_state, - &mut self.cursor.filter, - archetype_entity.id(), - archetype_entity.table_row(), + &self.cursor.filter, + archetype_entities, + next_chunk, ) }; - if fetched { - continue; - } - // SAFETY: set_archetype was called prior, `index` is an archetype index in range of the current archetype - // Caller assures `index` in range of the current archetype. - if let Some(item) = unsafe { - D::fetch( - &self.query_state.fetch_state, - &mut self.cursor.fetch, - archetype_entity.id(), - archetype_entity.table_row(), - ) - } { + for index in next_chunk.clone() { + // SAFETY: by induction (see above) index must be in range of the archetype. + let archetype_entity = unsafe { archetype_entities.get_unchecked(index as usize) }; + + // SAFETY: + // - `set_archetype` was called prior. + // - `find_archetype_chunk` was called prior, and `index` is in the returned range. + let item = unsafe { + D::fetch( + &self.query_state.fetch_state, + &mut self.cursor.fetch, + archetype_entity.id(), + archetype_entity.table_row(), + ) + }; + accum = func(accum, item); } + + next_chunk = next_chunk.end..indices.end; } accum } @@ -361,37 +397,58 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { archetype, table, ); - let entities = table.entities(); - for row in rows { - // SAFETY: Caller assures `row` in range of the current archetype. - let entity = unsafe { *entities.get_unchecked(row as usize) }; - // SAFETY: This is from an exclusive range, so it can't be max. - let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(row)) }; - - // SAFETY: set_table was called prior. - // Caller assures `row` in range of the current archetype. - let filter_matched = unsafe { - F::filter_fetch( + + let table_entities = table.entities(); + + let mut next_chunk = rows.clone(); + while next_chunk.start < rows.end { + // SAFETY: + // - `set_table` was called prior + // - since `find_table_chunk` can only narrow ranges, and the caller + // assures `rows` is in range of the archetype, `next_chunk` must be as well. + next_chunk = unsafe { + D::find_table_chunk( + &self.query_state.fetch_state, + &self.cursor.fetch, + table_entities, + next_chunk, + ) + }; + // SAFETY: + // - `set_table` was called prior + // - since `find_table_chunk` can only narrow ranges, and the caller + // assures `rows` is in range of the archetype, `next_chunk` must be as well. + next_chunk = unsafe { + F::find_table_chunk( &self.query_state.filter_state, - &mut self.cursor.filter, - entity, - row, + &self.cursor.filter, + table_entities, + next_chunk, ) }; - if !filter_matched { - continue; - } - // SAFETY: set_table was called prior. - // Caller assures `row` in range of the current archetype. - if let Some(item) = D::fetch( - &self.query_state.fetch_state, - &mut self.cursor.fetch, - entity, - row, - ) { + for row in next_chunk.clone() { + // SAFETY: by induction (see above) `row` must be in range of the table. + let entity = unsafe { *table_entities.get_unchecked(row as usize) }; + // SAFETY: This is from an exclusive range, so it can't be max. + let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(row)) }; + + // SAFETY: + // - `set_archetype` was called prior. + // - `find_archetype_chunk` was called prior, and `row` is in the returned range. + let item = unsafe { + D::fetch( + &self.query_state.fetch_state, + &mut self.cursor.fetch, + entity, + row, + ) + }; + accum = func(accum, item); } + + next_chunk = next_chunk.end..rows.end; } accum } @@ -1065,18 +1122,33 @@ where ); } - // The entity list has already been filtered by the query lens, so we forego filtering here. // SAFETY: - // - set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype - // - fetch is only called once for each entity. - unsafe { - D::fetch( + // - set_archetype was called prior. + // - `location.table_row` is in range of the current archetype. + let matches = unsafe { + D::matches( &self.query_state.fetch_state, - &mut self.fetch, + &self.fetch, entity, location.table_row, ) - } + }; + + matches.then(|| { + // The entity list has already been filtered by the query lens, so we forego filtering here. + // SAFETY: + // - set_archetype was called prior, `location.table_row` is an archetype index in range of the current archetype + // - fetch is only called once for each entity. + // - the surrounding guard verifies that this query matches the given row. + unsafe { + D::fetch( + &self.query_state.fetch_state, + &mut self.fetch, + entity, + location.table_row, + ) + } + }) } } @@ -1246,23 +1318,32 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> // SAFETY: set_archetype was called prior. // `location.archetype_row` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d - if unsafe { - F::filter_fetch( + let matches_fetch = + unsafe { D::matches(&query_state.fetch_state, fetch, entity, location.table_row) }; + // SAFETY: set_archetype was called prior. + // `location.archetype_row` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d + let matches_filter = unsafe { + F::matches( &query_state.filter_state, filter, entity, location.table_row, ) - } { + }; + + if matches_fetch && matches_filter { // SAFETY: // - set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype // - fetch is only called once for each entity. - let item = unsafe { - D::fetch(&query_state.fetch_state, fetch, entity, location.table_row) + // - this only runs if the fetch and filter both match + return unsafe { + Some(D::fetch( + &query_state.fetch_state, + fetch, + entity, + location.table_row, + )) }; - if let Some(item) = item { - return Some(item); - } } } None @@ -2042,12 +2123,12 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> // - set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype // - fetch is only called once for each entity. unsafe { - D::fetch( + Some(D::fetch( &self.query_state.fetch_state, &mut self.fetch, entity, location.table_row, - ) + )) } } @@ -2480,34 +2561,66 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let index = self.current_row - 1; if self.is_dense { // SAFETY: This must have been called previously in `next` as `current_row > 0` - let entity = unsafe { self.table_entities.get_unchecked(index as usize) }; - // SAFETY: - // - `set_table` must have been called previously either in `next` or before it. - // - `*entity` and `index` are in the current table. - unsafe { - D::fetch( - &query_state.fetch_state, - &mut self.fetch, - *entity, - // SAFETY: This is from an exclusive range, so it can't be max. - TableRow::new(NonMaxU32::new_unchecked(index)), - ) - } + let entity = unsafe { *self.table_entities.get_unchecked(index as usize) }; + // SAFETY: This is from an exclusive range, so it can't be max. + let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(index)) }; + + // SAFETY: set_table was called prior. + // `row` is a table row in range of the current table, + let matches_fetch = + unsafe { D::matches(&query_state.fetch_state, &self.fetch, entity, row) }; + // SAFETY: set_table was called prior. + // `row` is a table row in range of the current table, + let matches_filter = + unsafe { F::matches(&query_state.filter_state, &self.filter, entity, row) }; + + (matches_fetch && matches_filter).then(|| + // SAFETY: + // - `set_table` must have been called previously either in `next` or before it. + // - `*entity` and `index` are in the current table. + // - `row` matches both the fetch and filter + unsafe { D::fetch(&query_state.fetch_state, &mut self.fetch, entity, row) } + ) } else { // SAFETY: This must have been called previously in `next` as `current_row > 0` let archetype_entity = unsafe { self.archetype_entities.get_unchecked(index as usize) }; - // SAFETY: - // - `set_archetype` must have been called previously either in `next` or before it. - // - `archetype_entity.id()` and `archetype_entity.table_row()` are in the current archetype. - unsafe { - D::fetch( + + // SAFETY: set_archetype was called prior. + // `archetype_entity.table_row()` is an archetype index row in range of the current archetype, + let matches_fetch = unsafe { + D::matches( &query_state.fetch_state, - &mut self.fetch, + &self.fetch, archetype_entity.id(), archetype_entity.table_row(), ) - } + }; + // SAFETY: set_archetype was called prior. + // `archetype_entity.table_row()` is an archetype index row in range of the current archetype, + let matches_filter = unsafe { + F::matches( + &query_state.filter_state, + &self.filter, + archetype_entity.id(), + archetype_entity.table_row(), + ) + }; + + (matches_fetch && matches_filter).then(|| + // SAFETY: + // - `set_archetype` must have been called previously either in `next` or before it. + // - `archetype_entity.id()` and `archetype_entity.table_row()` are in the current archetype. + // - the current archetype entity matches both the fetch and filter + unsafe { + D::fetch( + &query_state.fetch_state, + &mut self.fetch, + archetype_entity.id(), + archetype_entity.table_row(), + ) + } + ) } } else { None @@ -2568,12 +2681,21 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // SAFETY: set_table was called prior. // `current_row` is a table row in range of the current table, because if it was not, then the above would have been executed. let entity = - unsafe { self.table_entities.get_unchecked(self.current_row as usize) }; + unsafe { *self.table_entities.get_unchecked(self.current_row as usize) }; // SAFETY: The row is less than the u32 len, so it must not be max. let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(self.current_row)) }; self.current_row += 1; - if !F::filter_fetch(&query_state.filter_state, &mut self.filter, *entity, row) { + // SAFETY: set_table was called prior. + // `row` is a table row in range of the current table, + let matches_fetch = + unsafe { D::matches(&query_state.fetch_state, &self.fetch, entity, row) }; + // SAFETY: set_table was called prior. + // `row` is a table row in range of the current table, + let matches_filter = + unsafe { F::matches(&query_state.filter_state, &self.filter, entity, row) }; + + if !(matches_fetch && matches_filter) { continue; } @@ -2582,11 +2704,14 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `current_row` must be a table row in range of the current table, // because if it was not, then the above would have been executed. // - fetch is only called once for each `entity`. - let item = - unsafe { D::fetch(&query_state.fetch_state, &mut self.fetch, *entity, row) }; - if let Some(item) = item { - return Some(item); - } + break unsafe { + Some(D::fetch( + &query_state.fetch_state, + &mut self.fetch, + entity, + row, + )) + }; } } else { loop { @@ -2626,12 +2751,28 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { }; self.current_row += 1; - if !F::filter_fetch( - &query_state.filter_state, - &mut self.filter, - archetype_entity.id(), - archetype_entity.table_row(), - ) { + // SAFETY: set_archetype was called prior. + // `archetype_entity.table_row()` is an archetype index row in range of the current archetype, + let matches_fetch = unsafe { + D::matches( + &query_state.fetch_state, + &self.fetch, + archetype_entity.id(), + archetype_entity.table_row(), + ) + }; + // SAFETY: set_archetype was called prior. + // `archetype_entity.table_row()` is an archetype index row in range of the current archetype, + let matches_filter = unsafe { + F::matches( + &query_state.filter_state, + &self.filter, + archetype_entity.id(), + archetype_entity.table_row(), + ) + }; + + if !(matches_fetch && matches_filter) { continue; } @@ -2640,17 +2781,15 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `current_row` must be an archetype index row in range of the current archetype, // because if it was not, then the if above would have been executed. // - fetch is only called once for each `archetype_entity`. - let item = unsafe { - D::fetch( + // - `archetype_entity` matches both the fetch and filter + break unsafe { + Some(D::fetch( &query_state.fetch_state, &mut self.fetch, archetype_entity.id(), archetype_entity.table_row(), - ) + )) }; - if let Some(item) = item { - return Some(item); - } } } } diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 34be815d36d77..df2f89cf90a55 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -253,7 +253,7 @@ impl RangeExt for Range { } if self.end >= other.start && self.end < other.end { - self.end = other.end + self.end = other.end; } self diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index b8c29d448adbf..48e94ca4bf52b 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1581,18 +1581,22 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { D::set_archetype(&mut fetch, &self.state.fetch_state, archetype, table); F::set_archetype(&mut filter, &self.state.filter_state, archetype, table); - if F::filter_fetch( + let matches_fetch = + D::matches(&self.state.fetch_state, &fetch, entity, location.table_row); + let matches_filter = F::matches( &self.state.filter_state, - &mut filter, + &filter, entity, location.table_row, - ) && let Some(item) = D::fetch( - &self.state.fetch_state, - &mut fetch, - entity, - location.table_row, - ) { - Ok(item) + ); + + if matches_fetch && matches_filter { + Ok(D::fetch( + &self.state.fetch_state, + &mut fetch, + entity, + location.table_row, + )) } else { Err(QueryEntityError::QueryDoesNotMatch( entity, diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index cae3f5d333db7..5803a288b2f8b 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -1012,10 +1012,16 @@ impl<'w> UnsafeEntityCell<'w> { }; // SAFETY: Archetype and table are from the same world used to initialize state and fetch. // Table corresponds to archetype. State is the same state used to init fetch above. - unsafe { Q::set_archetype(&mut fetch, &state, archetype, table) } - // SAFETY: Called after set_archetype above. Entity and location are guaranteed to exist. - let item = unsafe { Q::fetch(&state, &mut fetch, self.id(), location.table_row) }; - item.map(Q::release_state) + unsafe { Q::set_archetype(&mut fetch, &state, archetype, table) }; + // SAFETY: + // - set_archetype was called previously + // - table_row is in range of the current archetype + unsafe { Q::matches(&state, &fetch, self.id(), location.table_row) } + .then(|| + // SAFETY: Called after set_archetype and above. Entity and location are + // guaranteed to exist, and this is only called if `matches` returns `true`. + unsafe { Q::fetch(&state, &mut fetch, self.id(), location.table_row) }) + .map(Q::release_state) .ok_or(QueryAccessError::EntityDoesNotMatch) } else { Err(QueryAccessError::EntityDoesNotMatch) From 5ddaeb95d230dab568f1dfca7213d3c7a359495a Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Fri, 9 Jan 2026 17:45:27 -0800 Subject: [PATCH 11/27] use mutable access for Fetch --- crates/bevy_ecs/macros/src/world_query.rs | 12 ++-- crates/bevy_ecs/src/query/fetch.rs | 58 +++++++++---------- crates/bevy_ecs/src/query/filter.rs | 30 +++++----- crates/bevy_ecs/src/query/iter.rs | 32 +++++----- crates/bevy_ecs/src/query/mod.rs | 2 +- crates/bevy_ecs/src/query/world_query.rs | 12 ++-- crates/bevy_ecs/src/system/query.rs | 10 +++- .../bevy_ecs/src/world/unsafe_world_cell.rs | 2 +- 8 files changed, 82 insertions(+), 76 deletions(-) diff --git a/crates/bevy_ecs/macros/src/world_query.rs b/crates/bevy_ecs/macros/src/world_query.rs index 41258f4bcaa89..874238c84d5eb 100644 --- a/crates/bevy_ecs/macros/src/world_query.rs +++ b/crates/bevy_ecs/macros/src/world_query.rs @@ -176,7 +176,7 @@ pub(crate) fn world_query_impl( #[inline] unsafe fn find_table_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, table_entities: &[#path::entity::Entity], mut rows: core::ops::Range, ) -> core::ops::Range { @@ -186,7 +186,7 @@ pub(crate) fn world_query_impl( // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's // always valid to pass to the next term. Other invariants are upheld by // the caller. - #(rows = unsafe { <#field_types>::find_table_chunk(&state.#field_aliases, &fetch.#field_aliases, table_entities, rows) };)* + #(rows = unsafe { <#field_types>::find_table_chunk(&state.#field_aliases, &mut fetch.#field_aliases, table_entities, rows) };)* rows } } @@ -194,7 +194,7 @@ pub(crate) fn world_query_impl( #[inline] unsafe fn find_archetype_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, archetype_entities: &[#path::archetype::ArchetypeEntity], mut indices: core::ops::Range, ) -> core::ops::Range { @@ -204,7 +204,7 @@ pub(crate) fn world_query_impl( // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's // always valid to pass to the next term. Other invariants are upheld by // the caller. - #(indices = unsafe { <#field_types>::find_archetype_chunk(&state.#field_aliases, &fetch.#field_aliases, archetype_entities, indices) };)* + #(indices = unsafe { <#field_types>::find_archetype_chunk(&state.#field_aliases, &mut fetch.#field_aliases, archetype_entities, indices) };)* indices } } @@ -212,7 +212,7 @@ pub(crate) fn world_query_impl( #[inline] unsafe fn matches( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, entity: #path::entity::Entity, table_row: #path::storage::TableRow, ) -> bool { @@ -220,7 +220,7 @@ pub(crate) fn world_query_impl( true } else { // SAFETY: invariants are upheld by the caller. - true #(&& unsafe { <#field_types>::matches(&state.#field_aliases, &fetch.#field_aliases, entity, table_row) })* + true #(&& unsafe { <#field_types>::matches(&state.#field_aliases, &mut fetch.#field_aliases, entity, table_row) })* } } } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index ce83cbe68ce16..7091d6c9067f9 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -431,7 +431,7 @@ unsafe impl WorldQuery for Entity { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -538,7 +538,7 @@ unsafe impl WorldQuery for EntityLocation { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -716,7 +716,7 @@ unsafe impl WorldQuery for SpawnDetails { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -854,7 +854,7 @@ unsafe impl<'a> WorldQuery for EntityRef<'a> { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -974,7 +974,7 @@ unsafe impl<'a> WorldQuery for EntityMut<'a> { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -1093,7 +1093,7 @@ unsafe impl WorldQuery for FilteredEntityRef<'_, '_> { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -1228,7 +1228,7 @@ unsafe impl WorldQuery for FilteredEntityMut<'_, '_> { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -1371,7 +1371,7 @@ where #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -1499,7 +1499,7 @@ where #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -1603,7 +1603,7 @@ unsafe impl WorldQuery for &Archetype { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -1773,7 +1773,7 @@ unsafe impl WorldQuery for &T { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -1973,7 +1973,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Ref<'__w, T> { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -2200,7 +2200,7 @@ unsafe impl<'__w, T: Component> WorldQuery for &'__w mut T { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -2370,7 +2370,7 @@ unsafe impl<'__w, T: Component> WorldQuery for Mut<'__w, T> { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -2530,7 +2530,7 @@ unsafe impl WorldQuery for Option { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -2723,7 +2723,7 @@ unsafe impl WorldQuery for Has { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -3005,7 +3005,7 @@ macro_rules! impl_anytuple_fetch { #[inline] unsafe fn find_table_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, table_entities: &[Entity], rows: Range, ) -> Range { @@ -3016,11 +3016,11 @@ macro_rules! impl_anytuple_fetch { if Self::IS_ARCHETYPAL || !fetch.matches { rows } else { - let ($($name,)*) = &fetch.fetch; + let ($($name,)*) = &mut fetch.fetch; let ($($state,)*) = state; let mut next_chunk = rows.end..rows.end; // SAFETY: invariants are upheld by the caller. - $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_table_chunk($state, &$name.fetch, table_entities, rows.clone()) });)* + $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_table_chunk($state, &mut $name.fetch, table_entities, rows.clone()) });)* next_chunk } } @@ -3028,7 +3028,7 @@ macro_rules! impl_anytuple_fetch { #[inline] unsafe fn find_archetype_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, archetype_entities: &[ArchetypeEntity], mut indices: Range, ) -> Range { @@ -3039,11 +3039,11 @@ macro_rules! impl_anytuple_fetch { if Self::IS_ARCHETYPAL || !fetch.matches { indices } else { - let ($($name,)*) = &fetch.fetch; + let ($($name,)*) = &mut fetch.fetch; let ($($state,)*) = state; let mut next_chunk = indices.end..indices.end; // SAFETY: invariants are upheld by the caller. - $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_archetype_chunk($state, &$name.fetch, archetype_entities, indices.clone()) });)* + $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_archetype_chunk($state, &mut $name.fetch, archetype_entities, indices.clone()) });)* next_chunk } } @@ -3051,7 +3051,7 @@ macro_rules! impl_anytuple_fetch { #[inline] unsafe fn matches( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, entity: Entity, table_row: TableRow, ) -> bool { @@ -3062,10 +3062,10 @@ macro_rules! impl_anytuple_fetch { if Self::IS_ARCHETYPAL || !fetch.matches { true } else { - let ($($name,)*) = &fetch.fetch; + let ($($name,)*) = &mut fetch.fetch; let ($($state,)*) = state; // SAFETY: invariants are upheld by the caller. - false $(|| unsafe { $name::matches(&$state, &$name.fetch, entity, table_row) })* + false $(|| unsafe { $name::matches(&$state, &mut $name.fetch, entity, table_row) })* } } } @@ -3111,7 +3111,7 @@ macro_rules! impl_anytuple_fetch { let ($($state,)*) = _state; ($( // SAFETY: The invariants are required to be upheld by the caller. - unsafe { $name::matches(&$state, &$name.fetch, _entity, _table_row) }.then(|| + unsafe { $name::matches(&$state, &mut $name.fetch, _entity, _table_row) }.then(|| // SAFETY: the above guard verifies this subquery matches the given entity. // other invariants are upheld by the caller. unsafe { $name::fetch($state, &mut $name.fetch, _entity, _table_row) } @@ -3226,7 +3226,7 @@ unsafe impl WorldQuery for NopWorldQuery { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -3325,7 +3325,7 @@ unsafe impl WorldQuery for PhantomData { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -3544,7 +3544,7 @@ mod tests { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 3911743860ef8..4be3f361fa195 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -170,7 +170,7 @@ unsafe impl WorldQuery for With { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -269,7 +269,7 @@ unsafe impl WorldQuery for Without { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -466,7 +466,7 @@ macro_rules! impl_or_query_filter { #[inline] unsafe fn find_table_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, table_entities: &[Entity], rows: Range, ) -> Range { @@ -477,11 +477,11 @@ macro_rules! impl_or_query_filter { if Self::IS_ARCHETYPAL || !fetch.matches { rows } else { - let ($($filter,)*) = &fetch.fetch; + let ($($filter,)*) = &mut fetch.fetch; let ($($state,)*) = state; let mut new_rows = rows.end..rows.end; // SAFETY: invariants are upheld by the caller. - $(new_rows = new_rows.union_or_first(unsafe { $filter::find_table_chunk($state, &$filter.fetch, table_entities, rows.clone()) });)* + $(new_rows = new_rows.union_or_first(unsafe { $filter::find_table_chunk($state, &mut $filter.fetch, table_entities, rows.clone()) });)* new_rows } } @@ -489,7 +489,7 @@ macro_rules! impl_or_query_filter { #[inline] unsafe fn find_archetype_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, archetype_entities: &[ArchetypeEntity], mut indices: Range, ) -> Range { @@ -500,11 +500,11 @@ macro_rules! impl_or_query_filter { if Self::IS_ARCHETYPAL || !fetch.matches { indices } else { - let ($($filter,)*) = &fetch.fetch; + let ($($filter,)*) = &mut fetch.fetch; let ($($state,)*) = state; let mut new_indices = indices.end..indices.end; // SAFETY: invariants are upheld by the caller. - $(new_indices = new_indices.union_or_first(unsafe { $filter::find_archetype_chunk($state, &$filter.fetch, archetype_entities, indices.clone()) });)* + $(new_indices = new_indices.union_or_first(unsafe { $filter::find_archetype_chunk($state, &mut $filter.fetch, archetype_entities, indices.clone()) });)* new_indices } } @@ -512,7 +512,7 @@ macro_rules! impl_or_query_filter { #[inline] unsafe fn matches( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, entity: Entity, table_row: TableRow, ) -> bool { @@ -523,10 +523,10 @@ macro_rules! impl_or_query_filter { if Self::IS_ARCHETYPAL || !fetch.matches { true } else { - let ($($filter,)*) = &fetch.fetch; + let ($($filter,)*) = &mut fetch.fetch; let ($($state,)*) = state; // SAFETY: invariants are upheld by the caller. - false $(|| unsafe { $filter::matches(&$state, &$filter.fetch, entity, table_row) })* + false $(|| unsafe { $filter::matches(&$state, &mut $filter.fetch, entity, table_row) })* } } } @@ -609,7 +609,7 @@ unsafe impl WorldQuery for Allow { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { @@ -810,7 +810,7 @@ unsafe impl WorldQuery for Added { #[inline] unsafe fn matches( _state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, entity: Entity, table_row: TableRow, ) -> bool { @@ -1036,7 +1036,7 @@ unsafe impl WorldQuery for Changed { #[inline] unsafe fn matches( _state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, entity: Entity, table_row: TableRow, ) -> bool { @@ -1189,7 +1189,7 @@ unsafe impl WorldQuery for Spawned { #[inline] unsafe fn matches( _state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, entity: Entity, _table_row: TableRow, ) -> bool { diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 63f8cfda12192..4064591155f4e 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -227,7 +227,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { next_chunk = unsafe { D::find_table_chunk( &self.query_state.fetch_state, - &self.cursor.fetch, + &mut self.cursor.fetch, table_entities, next_chunk, ) @@ -239,7 +239,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { next_chunk = unsafe { F::find_table_chunk( &self.query_state.filter_state, - &self.cursor.filter, + &mut self.cursor.filter, table_entities, next_chunk, ) @@ -315,7 +315,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { next_chunk = unsafe { D::find_archetype_chunk( &self.query_state.fetch_state, - &self.cursor.fetch, + &mut self.cursor.fetch, archetype_entities, next_chunk, ) @@ -327,7 +327,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { next_chunk = unsafe { F::find_archetype_chunk( &self.query_state.filter_state, - &self.cursor.filter, + &mut self.cursor.filter, archetype_entities, next_chunk, ) @@ -409,7 +409,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { next_chunk = unsafe { D::find_table_chunk( &self.query_state.fetch_state, - &self.cursor.fetch, + &mut self.cursor.fetch, table_entities, next_chunk, ) @@ -421,7 +421,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { next_chunk = unsafe { F::find_table_chunk( &self.query_state.filter_state, - &self.cursor.filter, + &mut self.cursor.filter, table_entities, next_chunk, ) @@ -1128,7 +1128,7 @@ where let matches = unsafe { D::matches( &self.query_state.fetch_state, - &self.fetch, + &mut self.fetch, entity, location.table_row, ) @@ -2568,11 +2568,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // SAFETY: set_table was called prior. // `row` is a table row in range of the current table, let matches_fetch = - unsafe { D::matches(&query_state.fetch_state, &self.fetch, entity, row) }; + unsafe { D::matches(&query_state.fetch_state, &mut self.fetch, entity, row) }; // SAFETY: set_table was called prior. // `row` is a table row in range of the current table, let matches_filter = - unsafe { F::matches(&query_state.filter_state, &self.filter, entity, row) }; + unsafe { F::matches(&query_state.filter_state, &mut self.filter, entity, row) }; (matches_fetch && matches_filter).then(|| // SAFETY: @@ -2591,7 +2591,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let matches_fetch = unsafe { D::matches( &query_state.fetch_state, - &self.fetch, + &mut self.fetch, archetype_entity.id(), archetype_entity.table_row(), ) @@ -2601,7 +2601,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let matches_filter = unsafe { F::matches( &query_state.filter_state, - &self.filter, + &mut self.filter, archetype_entity.id(), archetype_entity.table_row(), ) @@ -2689,11 +2689,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // SAFETY: set_table was called prior. // `row` is a table row in range of the current table, let matches_fetch = - unsafe { D::matches(&query_state.fetch_state, &self.fetch, entity, row) }; + unsafe { D::matches(&query_state.fetch_state, &mut self.fetch, entity, row) }; // SAFETY: set_table was called prior. // `row` is a table row in range of the current table, let matches_filter = - unsafe { F::matches(&query_state.filter_state, &self.filter, entity, row) }; + unsafe { F::matches(&query_state.filter_state, &mut self.filter, entity, row) }; if !(matches_fetch && matches_filter) { continue; @@ -2756,9 +2756,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let matches_fetch = unsafe { D::matches( &query_state.fetch_state, - &self.fetch, + &mut self.fetch, archetype_entity.id(), archetype_entity.table_row(), + ) }; // SAFETY: set_archetype was called prior. @@ -2766,9 +2767,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let matches_filter = unsafe { F::matches( &query_state.filter_state, - &self.filter, + &mut self.filter, archetype_entity.id(), archetype_entity.table_row(), + ) }; diff --git a/crates/bevy_ecs/src/query/mod.rs b/crates/bevy_ecs/src/query/mod.rs index 5ba94af5e51ec..b55a91db9d1c2 100644 --- a/crates/bevy_ecs/src/query/mod.rs +++ b/crates/bevy_ecs/src/query/mod.rs @@ -886,7 +886,7 @@ mod tests { #[inline] unsafe fn matches( _state: &Self::State, - _fetch: &Self::Fetch<'_>, + _fetch: &mut Self::Fetch<'_>, _entity: Entity, _table_row: TableRow, ) -> bool { diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index df2f89cf90a55..9245eecc13334 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -162,7 +162,7 @@ pub unsafe trait WorldQuery { #[inline] unsafe fn find_table_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, table_entities: &[Entity], rows: Range, ) -> Range { @@ -188,7 +188,7 @@ pub unsafe trait WorldQuery { #[inline] unsafe fn find_archetype_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, archetype_entities: &[ArchetypeEntity], indices: Range, ) -> Range { @@ -228,7 +228,7 @@ pub unsafe trait WorldQuery { /// `table_row` must be in the range of the current table and archetype. unsafe fn matches( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, entity: Entity, table_row: TableRow, ) -> bool; @@ -382,7 +382,7 @@ macro_rules! impl_tuple_world_query { #[inline] unsafe fn find_table_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, table_entities: &[Entity], mut rows: Range, ) -> Range { @@ -402,7 +402,7 @@ macro_rules! impl_tuple_world_query { #[inline] unsafe fn find_archetype_chunk( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, archetype_entities: &[ArchetypeEntity], mut indices: Range, ) -> Range { @@ -422,7 +422,7 @@ macro_rules! impl_tuple_world_query { #[inline] unsafe fn matches( state: &Self::State, - fetch: &Self::Fetch<'_>, + fetch: &mut Self::Fetch<'_>, entity: Entity, table_row: TableRow, ) -> bool { diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 48e94ca4bf52b..5521481409e3e 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -1581,11 +1581,15 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { D::set_archetype(&mut fetch, &self.state.fetch_state, archetype, table); F::set_archetype(&mut filter, &self.state.filter_state, archetype, table); - let matches_fetch = - D::matches(&self.state.fetch_state, &fetch, entity, location.table_row); + let matches_fetch = D::matches( + &self.state.fetch_state, + &mut fetch, + entity, + location.table_row, + ); let matches_filter = F::matches( &self.state.filter_state, - &filter, + &mut filter, entity, location.table_row, ); diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index 5803a288b2f8b..f0d704771355b 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -1016,7 +1016,7 @@ impl<'w> UnsafeEntityCell<'w> { // SAFETY: // - set_archetype was called previously // - table_row is in range of the current archetype - unsafe { Q::matches(&state, &fetch, self.id(), location.table_row) } + unsafe { Q::matches(&state, &mut fetch, self.id(), location.table_row) } .then(|| // SAFETY: Called after set_archetype and above. Entity and location are // guaranteed to exist, and this is only called if `matches` returns `true`. From e2ffd2e7475a723361cd6b379ee6123e2aec9bd1 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Fri, 9 Jan 2026 17:33:21 -0800 Subject: [PATCH 12/27] fix AssetChanged --- crates/bevy_asset/src/asset_changed.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/bevy_asset/src/asset_changed.rs b/crates/bevy_asset/src/asset_changed.rs index 555861dd9809f..165a86b4e2315 100644 --- a/crates/bevy_asset/src/asset_changed.rs +++ b/crates/bevy_asset/src/asset_changed.rs @@ -201,6 +201,7 @@ unsafe impl WorldQuery for AssetChanged { } const IS_DENSE: bool = <&A>::IS_DENSE; + const IS_ARCHETYPAL: bool = false; unsafe fn set_archetype<'w, 's>( fetch: &mut Self::Fetch<'w>, @@ -261,30 +262,27 @@ unsafe impl WorldQuery for AssetChanged { ) -> bool { set_contains_id(state.asset_id) } -} - -#[expect(unsafe_code, reason = "QueryFilter is an unsafe trait.")] -/// SAFETY: read-only access -unsafe impl QueryFilter for AssetChanged { - const IS_ARCHETYPAL: bool = false; #[inline] - unsafe fn filter_fetch( + unsafe fn matches( state: &Self::State, fetch: &mut Self::Fetch<'_>, entity: Entity, table_row: TableRow, ) -> bool { fetch.inner.as_mut().is_some_and(|inner| { - // SAFETY: We delegate to the inner `fetch` for `A` + // SAFETY: We delegate to the inner `fetch` for `A`. + // Component accesses are archetypal, so we don't need to check if it `matches`. unsafe { let handle = <&A>::fetch(&state.asset_id, inner, entity, table_row); - handle.is_some_and(|handle| fetch.check.has_changed(handle)) + fetch.check.has_changed(handle) } }) } } +impl QueryFilter for AssetChanged {} + #[cfg(test)] #[expect(clippy::print_stdout, reason = "Allowed in tests.")] mod tests { From 6a2a1bcec885fe2d47187f3acff2beefcaeaae60 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Fri, 9 Jan 2026 18:02:43 -0800 Subject: [PATCH 13/27] fix MainEntity and RenderEntity --- crates/bevy_render/src/sync_world.rs | 38 ++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/crates/bevy_render/src/sync_world.rs b/crates/bevy_render/src/sync_world.rs index 6a1a1fa813a21..d43e26cd498e2 100644 --- a/crates/bevy_render/src/sync_world.rs +++ b/crates/bevy_render/src/sync_world.rs @@ -315,6 +315,7 @@ mod render_entities_world_query_impls { } const IS_DENSE: bool = <&'static RenderEntity as WorldQuery>::IS_DENSE; + const IS_ARCHETYPAL: bool = <&'static RenderEntity as WorldQuery>::IS_ARCHETYPAL; #[inline] unsafe fn set_archetype<'w, 's>( @@ -357,13 +358,23 @@ mod render_entities_world_query_impls { ) -> bool { <&RenderEntity as WorldQuery>::matches_component_set(&state, set_contains_id) } + + #[inline] + unsafe fn matches( + state: &Self::State, + fetch: &mut Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool { + // SAFETY: invariants are upheld by caller + unsafe { <&RenderEntity as WorldQuery>::matches(state, fetch, entity, table_row) } + } } // SAFETY: Component access of Self::ReadOnly is a subset of Self. // Self::ReadOnly matches exactly the same archetypes/tables as Self. unsafe impl QueryData for RenderEntity { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = <&MainEntity as QueryData>::IS_ARCHETYPAL; type ReadOnly = RenderEntity; type Item<'w, 's> = Entity; @@ -379,11 +390,9 @@ mod render_entities_world_query_impls { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { // SAFETY: defers to the `&T` implementation, with T set to `RenderEntity`. - let component = - unsafe { <&RenderEntity as QueryData>::fetch(state, fetch, entity, table_row) }; - component.map(RenderEntity::id) + unsafe { <&RenderEntity as QueryData>::fetch(state, fetch, entity, table_row).id() } } fn iter_access( @@ -430,6 +439,7 @@ mod render_entities_world_query_impls { } const IS_DENSE: bool = <&'static MainEntity as WorldQuery>::IS_DENSE; + const IS_ARCHETYPAL: bool = <&'static MainEntity as WorldQuery>::IS_ARCHETYPAL; #[inline] unsafe fn set_archetype<'w, 's>( @@ -472,13 +482,23 @@ mod render_entities_world_query_impls { ) -> bool { <&MainEntity as WorldQuery>::matches_component_set(&state, set_contains_id) } + + #[inline] + unsafe fn matches( + state: &Self::State, + fetch: &mut Self::Fetch<'_>, + entity: Entity, + table_row: TableRow, + ) -> bool { + // SAFETY: invariants are upheld by caller + unsafe { <&MainEntity as WorldQuery>::matches(state, fetch, entity, table_row) } + } } // SAFETY: Component access of Self::ReadOnly is a subset of Self. // Self::ReadOnly matches exactly the same archetypes/tables as Self. unsafe impl QueryData for MainEntity { const IS_READ_ONLY: bool = true; - const IS_ARCHETYPAL: bool = <&MainEntity as QueryData>::IS_ARCHETYPAL; type ReadOnly = MainEntity; type Item<'w, 's> = Entity; @@ -494,11 +514,9 @@ mod render_entities_world_query_impls { fetch: &mut Self::Fetch<'w>, entity: Entity, table_row: TableRow, - ) -> Option> { + ) -> Self::Item<'w, 's> { // SAFETY: defers to the `&T` implementation, with T set to `MainEntity`. - let component = - unsafe { <&MainEntity as QueryData>::fetch(state, fetch, entity, table_row) }; - component.map(MainEntity::id) + unsafe { <&MainEntity as QueryData>::fetch(state, fetch, entity, table_row).id() } } fn iter_access( From 96aab9cbfa227ec6ec98e600290eb93fc5772a79 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Fri, 9 Jan 2026 20:10:14 -0800 Subject: [PATCH 14/27] fixes --- crates/bevy_ecs/src/query/iter.rs | 14 +++++++++++++- crates/bevy_ecs/src/query/world_query.rs | 2 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 4064591155f4e..6a4a1a9c054e7 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -245,6 +245,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { ) }; + if next_chunk.is_empty() { + break; + } + for row in next_chunk.clone() { // SAFETY: Caller assures `row` in range of the current archetype. let entity = unsafe { table_entities.get_unchecked(row as usize) }; @@ -333,6 +337,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { ) }; + if next_chunk.is_empty() { + break; + } + for index in next_chunk.clone() { // SAFETY: by induction (see above) index must be in range of the archetype. let archetype_entity = unsafe { archetype_entities.get_unchecked(index as usize) }; @@ -427,6 +435,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { ) }; + if next_chunk.is_empty() { + break; + } + for row in next_chunk.clone() { // SAFETY: by induction (see above) `row` must be in range of the table. let entity = unsafe { *table_entities.get_unchecked(row as usize) }; @@ -435,7 +447,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> { // SAFETY: // - `set_archetype` was called prior. - // - `find_archetype_chunk` was called prior, and `row` is in the returned range. + // - `find_table_chunk` was called prior, and `row` is in the returned range. let item = unsafe { D::fetch( &self.query_state.fetch_state, diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 9245eecc13334..66b9be5c34e9d 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -276,11 +276,11 @@ impl RangeExt for Range { while index < self.end { // SAFETY: index is taken from an exclusive range, so it can't be max let nonmax_index = unsafe { NonMaxU32::new_unchecked(index) }; - index = index.wrapping_add(1); let matches = func(nonmax_index); if !matches { break; } + index = index.wrapping_add(1); } self.end = index; From bd48b3c25e1268004819c2ccc17005628663c910 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Sat, 10 Jan 2026 20:39:50 -0800 Subject: [PATCH 15/27] fix AnyOf and Or --- crates/bevy_ecs/src/query/fetch.rs | 42 ++++++++++++++++++----------- crates/bevy_ecs/src/query/filter.rs | 18 +++++++++---- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 7091d6c9067f9..c75f02c9d3c5e 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -3019,8 +3019,12 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = &mut fetch.fetch; let ($($state,)*) = state; let mut next_chunk = rows.end..rows.end; - // SAFETY: invariants are upheld by the caller. - $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_table_chunk($state, &mut $name.fetch, table_entities, rows.clone()) });)* + $( + if $name.matches { + // SAFETY: invariants are upheld by the caller. + next_chunk = next_chunk.union_or_first(unsafe { $name::find_table_chunk($state, &mut $name.fetch, table_entities, rows.clone()) }); + } + )* next_chunk } } @@ -3042,8 +3046,12 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = &mut fetch.fetch; let ($($state,)*) = state; let mut next_chunk = indices.end..indices.end; - // SAFETY: invariants are upheld by the caller. - $(next_chunk = next_chunk.union_or_first(unsafe { $name::find_archetype_chunk($state, &mut $name.fetch, archetype_entities, indices.clone()) });)* + $( + if $name.matches { + // SAFETY: invariants are upheld by the caller. + next_chunk = next_chunk.union_or_first(unsafe { $name::find_archetype_chunk($state, &mut $name.fetch, archetype_entities, indices.clone()) }); + } + )* next_chunk } } @@ -3065,7 +3073,7 @@ macro_rules! impl_anytuple_fetch { let ($($name,)*) = &mut fetch.fetch; let ($($state,)*) = state; // SAFETY: invariants are upheld by the caller. - false $(|| unsafe { $name::matches(&$state, &mut $name.fetch, entity, table_row) })* + false $(|| ($name.matches && unsafe { $name::matches(&$state, &mut $name.fetch, entity, table_row) }))* } } } @@ -3107,16 +3115,20 @@ macro_rules! impl_anytuple_fetch { _entity: Entity, _table_row: TableRow ) -> Self::Item<'w, 's> { - let ($($name,)*) = &mut _fetch.fetch; - let ($($state,)*) = _state; - ($( - // SAFETY: The invariants are required to be upheld by the caller. - unsafe { $name::matches(&$state, &mut $name.fetch, _entity, _table_row) }.then(|| - // SAFETY: the above guard verifies this subquery matches the given entity. - // other invariants are upheld by the caller. - unsafe { $name::fetch($state, &mut $name.fetch, _entity, _table_row) } - ), - )*) + if !_fetch.matches { + ($(Option::<$name::Item<'w, 's>>::None,)*) + } else { + let ($($name,)*) = &mut _fetch.fetch; + let ($($state,)*) = _state; + ($( + // SAFETY: The invariants are required to be upheld by the caller. + ( $name.matches && unsafe { $name::matches(&$state, &mut $name.fetch, _entity, _table_row) }).then(|| + // SAFETY: the above guard verifies this subquery matches the given entity. + // other invariants are upheld by the caller. + unsafe { $name::fetch($state, &mut $name.fetch, _entity, _table_row) } + ), + )*) + } } fn iter_access(state: &Self::State) -> impl Iterator> { diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 4be3f361fa195..642ab97a87c06 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -480,8 +480,12 @@ macro_rules! impl_or_query_filter { let ($($filter,)*) = &mut fetch.fetch; let ($($state,)*) = state; let mut new_rows = rows.end..rows.end; - // SAFETY: invariants are upheld by the caller. - $(new_rows = new_rows.union_or_first(unsafe { $filter::find_table_chunk($state, &mut $filter.fetch, table_entities, rows.clone()) });)* + $( + if $filter.matches { + // SAFETY: invariants are upheld by the caller. + new_rows = new_rows.union_or_first(unsafe { $filter::find_table_chunk($state, &mut $filter.fetch, table_entities, rows.clone()) }); + } + )* new_rows } } @@ -503,8 +507,12 @@ macro_rules! impl_or_query_filter { let ($($filter,)*) = &mut fetch.fetch; let ($($state,)*) = state; let mut new_indices = indices.end..indices.end; - // SAFETY: invariants are upheld by the caller. - $(new_indices = new_indices.union_or_first(unsafe { $filter::find_archetype_chunk($state, &mut $filter.fetch, archetype_entities, indices.clone()) });)* + $( + if $filter.matches { + // SAFETY: invariants are upheld by the caller. + new_indices = new_indices.union_or_first(unsafe { $filter::find_archetype_chunk($state, &mut $filter.fetch, archetype_entities, indices.clone()) }); + } + )* new_indices } } @@ -526,7 +534,7 @@ macro_rules! impl_or_query_filter { let ($($filter,)*) = &mut fetch.fetch; let ($($state,)*) = state; // SAFETY: invariants are upheld by the caller. - false $(|| unsafe { $filter::matches(&$state, &mut $filter.fetch, entity, table_row) })* + false $(|| ($filter.matches && unsafe { $filter::matches(&$state, &mut $filter.fetch, entity, table_row) }))* } } } From f50eb3273505f2d5995fc40b7c86f6a0e049ede8 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Tue, 13 Jan 2026 19:02:27 -0800 Subject: [PATCH 16/27] use `return` instead of `break` --- crates/bevy_ecs/src/query/iter.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 6a4a1a9c054e7..845a55bb99dc2 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -2716,7 +2716,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `current_row` must be a table row in range of the current table, // because if it was not, then the above would have been executed. // - fetch is only called once for each `entity`. - break unsafe { + return unsafe { Some(D::fetch( &query_state.fetch_state, &mut self.fetch, @@ -2796,7 +2796,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // because if it was not, then the if above would have been executed. // - fetch is only called once for each `archetype_entity`. // - `archetype_entity` matches both the fetch and filter - break unsafe { + return unsafe { Some(D::fetch( &query_state.fetch_state, &mut self.fetch, From 531374373d61d2ae5e7bd567d02ad8aa604e04cb Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Tue, 13 Jan 2026 20:18:07 -0800 Subject: [PATCH 17/27] docs --- crates/bevy_ecs/src/query/fetch.rs | 7 ++-- crates/bevy_ecs/src/query/world_query.rs | 41 +++++++++++++++---- .../migration-guides/query_lookahead.md | 15 +++++++ 3 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 release-content/migration-guides/query_lookahead.md diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index c75f02c9d3c5e..df4de36495561 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -329,8 +329,9 @@ pub unsafe trait QueryData: WorldQuery { /// /// - Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and /// `table_row` must be in the range of the current table and archetype. - /// - Must always be called _after_ [`WorldQuery::matches`], or after one of [`WorldQuery::find_table_chunk`] - /// or [`WorldQuery::find_archetype_chunk`] and the provided table row/index is within the range returned. + /// - Must always be called _after_ [`WorldQuery::matches`] if and only if it returns `true`, + /// or after one of [`WorldQuery::find_table_chunk`] or [`WorldQuery::find_archetype_chunk`] + /// and the provided table row/index is within the range returned. /// - There must not be simultaneous conflicting component access registered in `update_component_access`. unsafe fn fetch<'w, 's>( state: &'s Self::State, @@ -373,7 +374,7 @@ pub trait ReleaseStateQueryData: QueryData { /// This is needed to implement [`ExactSizeIterator`] for /// [`QueryIter`](crate::query::QueryIter) that contains archetype-level filters. /// -/// The trait must only be implemented for query data where its corresponding [`QueryData::IS_ARCHETYPAL`] is [`prim@true`]. +/// The trait must only be implemented for query data where its corresponding [`WorldQuery::IS_ARCHETYPAL`] is [`prim@true`]. pub trait ArchetypeQueryData: QueryData {} /// SAFETY: diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 66b9be5c34e9d..271ccca1577de 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -36,8 +36,12 @@ use variadics_please::all_tuples; /// /// When implementing [`update_component_access`], note that `add_read` and `add_write` both also add a `With` filter, whereas `extend_access` does not change the filters. /// +/// If [`IS_ARCHETYPAL`] is `true`, [`matches`] must return `true` for all inputs. +/// /// When implementing [`find_table_chunk`] and [`find_archetype_chunk`], their return values must be -/// subsets of `rows` and `indices` respectively, even if returning an empty range. +/// subsets of `rows` and `indices` respectively, even if returning an empty range. Additionally, +/// they must match the behavior of [`matches`], i.e. calling that method on any +/// row/index in the returned range should return `true`. /// /// [`find_table_chunk`], [`find_archetype_chunk`], and [`matches`] must not mutably access any world data. /// @@ -53,6 +57,7 @@ use variadics_please::all_tuples; /// [`find_table_chunk`]: crate::query::WorldQuery::find_table_chunk /// [`find_archetype_chunk`]: crate::query::WorldQuery::find_archetype_chunk /// [`matches`]: crate::query::WorldQuery::matches +/// [`IS_ARCHETYPAL`]: crate::query::WorldQuery::IS_ARCHETYPAL pub unsafe trait WorldQuery { /// Per archetype/table state retrieved by this [`WorldQuery`] to compute [`Self::Item`](crate::query::QueryData::Item) for each entity. type Fetch<'w>: Clone; @@ -153,12 +158,21 @@ pub unsafe trait WorldQuery { set_contains_id: &impl Fn(ComponentId) -> bool, ) -> bool; - /// TODO: docs + /// Searches `rows` for the next contiguous range of indices + /// this `WorldQuery` matches. /// - /// # Safety + /// This can be used to speed up query iteration for custom [`WorldQuery`] + /// implementations, for example by using an acceleration structure + /// to seek ahead in the query for the next valid entity. /// - /// Must always be called _after_ [`WorldQuery::set_table`]. - /// `rows` must be in the range of the current table. + /// The default implementation will simply defer to [`WorldQuery::matches`], + /// calling the method for each row until it finds the next whole chunk. + /// Custom implementations should match this behavior, i.e. calling [`WorldQuery::matches`] + /// on any row in the returned range should return `true`. + /// + /// # Safety + /// - Must always be called _after_ [`WorldQuery::set_table`]. + /// - `rows` must be in the range of the current table. #[inline] unsafe fn find_table_chunk( state: &Self::State, @@ -179,12 +193,21 @@ pub unsafe trait WorldQuery { } } - /// TODO: docs + /// Searches `indices` for the next contiguous range of indices + /// this `WorldQuery` matches. /// - /// # Safety + /// This can be used to speed up query iteration for custom [`WorldQuery`] + /// implementations, for example by using an acceleration structure + /// to seek ahead in the query for the next valid entity. + /// + /// The default implementation will simply defer to [`WorldQuery::matches`], + /// calling the method for each row until it finds the next whole chunk. + /// Custom implementations should match this behavior, i.e. calling [`WorldQuery::matches`] + /// on any index in the returned range should return `true`. /// - /// Must always be called _after_ [`WorldQuery::set_archetype`]. - /// `indices` must be in the range of the current archetype. + /// # Safety + /// - Must always be called _after_ [`WorldQuery::set_archetype`]. + /// - `indices` must be in the range of the current archetype. #[inline] unsafe fn find_archetype_chunk( state: &Self::State, diff --git a/release-content/migration-guides/query_lookahead.md b/release-content/migration-guides/query_lookahead.md new file mode 100644 index 0000000000000..03a0b314cf687 --- /dev/null +++ b/release-content/migration-guides/query_lookahead.md @@ -0,0 +1,15 @@ +--- +title: "WorldQuery, QueryData, and QueryFilter trait methods rearranged" +pull_requests: [todo!()] +--- + +- `QueryData::IS_ARCHETYPAL`, `QueryFilter::IS_ARCHETYPAL` have been moved to `WorldQuery`. +- `QueryFilter::filter_fetch` has been moved to `WorldQuery::matches`. +- `QueryData::fetch` now returns `QueryData::Item` directly, without an `Option` + wrapper. Instead, implementations must provide `WorldQuery::matches` to + decide whether or not a row matches the query. `QueryData::fetch` can use + the return value of `matches` as a safety guarantee to avoid double-checking + a condition, for example with `Option::unwrap_unchecked`. +- `WorldQuery` has a few new methods: `find_table_chunk` and `find_archetype_chunk`. + These are unnecessary for most implementations, but add some optimization + opportunities. See module docs for more info. From b1858e7a4f955f2ad725b673e943167234e9fe01 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Tue, 13 Jan 2026 23:00:51 -0800 Subject: [PATCH 18/27] add pr num to migration guide --- release-content/migration-guides/query_lookahead.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release-content/migration-guides/query_lookahead.md b/release-content/migration-guides/query_lookahead.md index 03a0b314cf687..7dcc8c04e7c3a 100644 --- a/release-content/migration-guides/query_lookahead.md +++ b/release-content/migration-guides/query_lookahead.md @@ -1,6 +1,6 @@ --- title: "WorldQuery, QueryData, and QueryFilter trait methods rearranged" -pull_requests: [todo!()] +pull_requests: [22500] --- - `QueryData::IS_ARCHETYPAL`, `QueryFilter::IS_ARCHETYPAL` have been moved to `WorldQuery`. From 955ace733a375bae88de45db8195995303045014 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Tue, 13 Jan 2026 23:14:40 -0800 Subject: [PATCH 19/27] fmt --- crates/bevy_ecs/src/query/iter.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 845a55bb99dc2..8f4076c6a865c 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -2586,13 +2586,12 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let matches_filter = unsafe { F::matches(&query_state.filter_state, &mut self.filter, entity, row) }; - (matches_fetch && matches_filter).then(|| + (matches_fetch && matches_filter).then(|| // SAFETY: // - `set_table` must have been called previously either in `next` or before it. // - `*entity` and `index` are in the current table. // - `row` matches both the fetch and filter - unsafe { D::fetch(&query_state.fetch_state, &mut self.fetch, entity, row) } - ) + unsafe { D::fetch(&query_state.fetch_state, &mut self.fetch, entity, row) }) } else { // SAFETY: This must have been called previously in `next` as `current_row > 0` let archetype_entity = @@ -2619,7 +2618,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { ) }; - (matches_fetch && matches_filter).then(|| + (matches_fetch && matches_filter).then(|| // SAFETY: // - `set_archetype` must have been called previously either in `next` or before it. // - `archetype_entity.id()` and `archetype_entity.table_row()` are in the current archetype. @@ -2631,8 +2630,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { archetype_entity.id(), archetype_entity.table_row(), ) - } - ) + }) } } else { None @@ -2771,7 +2769,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { &mut self.fetch, archetype_entity.id(), archetype_entity.table_row(), - ) }; // SAFETY: set_archetype was called prior. @@ -2782,7 +2779,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { &mut self.filter, archetype_entity.id(), archetype_entity.table_row(), - ) }; From 726de8029a6ba99ba48d8ee4b56accd745c13df7 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Wed, 14 Jan 2026 16:14:49 -0800 Subject: [PATCH 20/27] fix docs --- crates/bevy_ecs/src/query/filter.rs | 10 +++++----- crates/bevy_ecs/src/query/world_query.rs | 5 ++--- crates/bevy_ecs/src/system/query.rs | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 642ab97a87c06..e08ce19beefd1 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -109,7 +109,7 @@ pub struct With(PhantomData); /// SAFETY: /// `update_component_access` does not add any accesses. -/// This is sound because [`QueryFilter::filter_fetch`] does not access any components. +/// This is sound because [`WorldQuery::matches`] does not access any components. /// `update_component_access` adds a `With` filter for `T`. /// This is sound because `matches_component_set` returns whether the set contains the component. unsafe impl WorldQuery for With { @@ -208,7 +208,7 @@ pub struct Without(PhantomData); /// SAFETY: /// `update_component_access` does not add any accesses. -/// This is sound because [`QueryFilter::filter_fetch`] does not access any components. +/// This is sound because [`WorldQuery::matches`] does not access any components. /// `update_component_access` adds a `Without` filter for `T`. /// This is sound because `matches_component_set` returns whether the set does not contain the component. unsafe impl WorldQuery for Without { @@ -344,7 +344,7 @@ macro_rules! impl_or_query_filter { /// SAFETY: /// [`WorldQuery::matches`], [`WorldQuery::find_table_chunk`], and [`WorldQuery::find_archetype_chunk`] only /// call their respective methods on subqueries, so they can't perform mutable access unless subqueries are invalid. - /// [`QueryFilter::filter_fetch`] accesses are a subset of the subqueries' accesses + /// [`WorldQuery::matches`] accesses are a subset of the subqueries' accesses /// This is sound because `update_component_access` adds accesses according to the implementations of all the subqueries. /// `update_component_access` replace the filters with a disjunction where every element is a conjunction of the previous filters and the filters of one of the subqueries. /// This is sound because `matches_component_set` returns a disjunction of the results of the subqueries' implementations. @@ -574,7 +574,7 @@ pub struct Allow(PhantomData); /// SAFETY: /// `update_component_access` does not add any accesses. -/// This is sound because [`QueryFilter::filter_fetch`] does not access any components. +/// This is sound because [`WorldQuery::matches`] does not access any components. /// `update_component_access` adds an archetypal filter for `T`. /// This is sound because it doesn't affect the query unsafe impl WorldQuery for Allow { @@ -1219,7 +1219,7 @@ impl QueryFilter for Spawned {} /// This is needed to implement [`ExactSizeIterator`] for /// [`QueryIter`](crate::query::QueryIter) that contains archetype-level filters. /// -/// The trait must only be implemented for filters where its corresponding [`QueryFilter::IS_ARCHETYPAL`] +/// The trait must only be implemented for filters where its corresponding [`WorldQuery::IS_ARCHETYPAL`] /// is [`prim@true`]. As such, only the [`With`] and [`Without`] filters can implement the trait. /// [Tuples](prim@tuple) and [`Or`] filters are automatically implemented with the trait only if its containing types /// also implement the same trait. diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 271ccca1577de..9939c556cd0b7 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -18,11 +18,11 @@ use variadics_please::all_tuples; /// # Safety /// /// Implementor must ensure that -/// [`update_component_access`], [`QueryData::provide_extra_access`], [`matches_component_set`], [`QueryData::fetch`], [`QueryFilter::filter_fetch`] and [`init_fetch`] +/// [`update_component_access`], [`QueryData::provide_extra_access`], [`matches_component_set`], [`QueryData::fetch`], [`matches`] and [`init_fetch`] /// obey the following: /// /// - For each component mutably accessed by [`QueryData::fetch`], [`update_component_access`] or [`QueryData::provide_extra_access`] should add write access unless read or write access has already been added, in which case it should panic. -/// - For each component readonly accessed by [`QueryData::fetch`] or [`QueryFilter::filter_fetch`], [`update_component_access`] or [`QueryData::provide_extra_access`] should add read access unless write access has already been added, in which case it should panic. +/// - For each component readonly accessed by [`QueryData::fetch`] or [`matches`], [`update_component_access`] or [`QueryData::provide_extra_access`] should add read access unless write access has already been added, in which case it should panic. /// - If `fetch` mutably accesses the same component twice, [`update_component_access`] should panic. /// - [`update_component_access`] may not add a `Without` filter for a component unless [`matches_component_set`] always returns `false` when the component set contains that component. /// - [`update_component_access`] may not add a `With` filter for a component unless [`matches_component_set`] always returns `false` when the component set doesn't contain that component. @@ -47,7 +47,6 @@ use variadics_please::all_tuples; /// /// [`QueryData::provide_extra_access`]: crate::query::QueryData::provide_extra_access /// [`QueryData::fetch`]: crate::query::QueryData::fetch -/// [`QueryFilter::filter_fetch`]: crate::query::QueryFilter::filter_fetch /// [`init_fetch`]: Self::init_fetch /// [`matches_component_set`]: Self::matches_component_set /// [`Query`]: crate::system::Query diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 5521481409e3e..70486253bd3ab 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -2053,7 +2053,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// This is equivalent to `self.iter().count()` but may be more efficient in some cases. /// - /// If [`D::IS_ARCHETYPAL`](QueryData::IS_ARCHETYPAL) && [`F::IS_ARCHETYPAL`](QueryFilter::IS_ARCHETYPAL) is `true`, + /// If [`D::IS_ARCHETYPAL`](WorldQuery::IS_ARCHETYPAL) && [`F::IS_ARCHETYPAL`](WorldQuery::IS_ARCHETYPAL) is `true`, /// this will do work proportional to the number of matched archetypes or tables, but will not iterate each entity. /// If it is `false`, it will have to do work for each entity. /// From 4e0b59f90da77a0e397b5dabf9041f92485eecbc Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Wed, 14 Jan 2026 17:20:14 -0800 Subject: [PATCH 21/27] fix docs more --- crates/bevy_ecs/src/system/query.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/query.rs b/crates/bevy_ecs/src/system/query.rs index 70486253bd3ab..519f4bd4993bb 100644 --- a/crates/bevy_ecs/src/system/query.rs +++ b/crates/bevy_ecs/src/system/query.rs @@ -60,7 +60,7 @@ use core::{ /// ## Component access /// /// You can fetch an entity's component by specifying a reference to that component in the query's data parameter: -/// +//query.rs/ /// ``` /// # use bevy_ecs::prelude::*; /// # @@ -2053,7 +2053,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Query<'w, 's, D, F> { /// /// This is equivalent to `self.iter().count()` but may be more efficient in some cases. /// - /// If [`D::IS_ARCHETYPAL`](WorldQuery::IS_ARCHETYPAL) && [`F::IS_ARCHETYPAL`](WorldQuery::IS_ARCHETYPAL) is `true`, + /// If [`D::IS_ARCHETYPAL`](crate::query::WorldQuery::IS_ARCHETYPAL) && [`F::IS_ARCHETYPAL`](crate::query::WorldQuery::IS_ARCHETYPAL) is `true`, /// this will do work proportional to the number of matched archetypes or tables, but will not iterate each entity. /// If it is `false`, it will have to do work for each entity. /// From b248ca5439a8684a388dbd22e84b85108142f1ba Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Thu, 22 Jan 2026 10:55:26 -0800 Subject: [PATCH 22/27] add QueryData::try_fetch --- crates/bevy_ecs/src/query/fetch.rs | 72 ++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index df4de36495561..26e17a74a3eac 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -322,9 +322,6 @@ pub unsafe trait QueryData: WorldQuery { /// [`WorldQuery::set_archetype`] with an `entity` in the current archetype. /// Accesses components registered in [`WorldQuery::update_component_access`]. /// - /// This method returns `None` if the entity does not match the query. - /// If `Self` implements [`ArchetypeQueryData`], this must always return `Some`. - /// /// # Safety /// /// - Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and @@ -340,6 +337,36 @@ pub unsafe trait QueryData: WorldQuery { table_row: TableRow, ) -> Self::Item<'w, 's>; + /// Attempt to fetch [`Self::Item`](`QueryData::Item`) for either the given `entity` in the current [`Table`], + /// or for the given `entity` in the current [`Archetype`]. This must always be called after + /// [`WorldQuery::set_table`] with a `table_row` in the range of the current [`Table`] or after + /// [`WorldQuery::set_archetype`] with an `entity` in the current archetype. + /// Accesses components registered in [`WorldQuery::update_component_access`]. + /// + /// Unlike [`Self::fetch`](`QueryData::fetch`), this method is fallible, so it is not a requirement + /// to check that the passed row is valid for this query beforehand. This method returns `None` if + /// the entity does not match the query. This method must return `Some()` if and only if + /// [`Self::matches`](`WorldQuery::matches`) would return `true` for the given row, and in that + /// case must return the same item that [`Self::fetch`](`QueryData::fetch`) would return. + /// + /// # Safety + /// + /// - Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and + /// `table_row` must be in the range of the current table and archetype. + /// - There must not be simultaneous conflicting component access registered in `update_component_access`. + #[inline(always)] + unsafe fn try_fetch<'w, 's>( + state: &'s Self::State, + fetch: &mut Self::Fetch<'w>, + entity: Entity, + table_row: TableRow, + ) -> Option> { + Self::matches(state, fetch, entity, table_row).then(|| { + // SAFETY: this is only called if the given row is valid for this query + unsafe { Self::fetch(state, fetch, entity, table_row) } + }) + } + /// Returns an iterator over the access needed by [`QueryData::fetch`]. Access conflicts are usually /// checked in [`WorldQuery::update_component_access`], but in certain cases this method can be useful to implement /// a way of checking for access conflicts in a non-allocating way. @@ -2839,6 +2866,19 @@ macro_rules! impl_tuple_query_data { ($(unsafe { $name::fetch($state, $name, entity, table_row) },)*) } + #[inline(always)] + unsafe fn try_fetch<'w, 's>( + state: &'s Self::State, + fetch: &mut Self::Fetch<'w>, + entity: Entity, + table_row: TableRow + ) -> Option> { + let ($($state,)*) = state; + let ($($name,)*) = fetch; + // SAFETY: The invariants are upheld by the caller. + Some(($(unsafe { $name::try_fetch($state, $name, entity, table_row) }?,)*)) + } + fn iter_access(state: &Self::State) -> impl Iterator> { let ($($name,)*) = state; iter::empty()$(.chain($name::iter_access($name)))* @@ -3132,6 +3172,32 @@ macro_rules! impl_anytuple_fetch { } } + #[inline(always)] + unsafe fn try_fetch<'w, 's>( + _state: &'s Self::State, + _fetch: &mut Self::Fetch<'w>, + _entity: Entity, + _table_row: TableRow + ) -> Option> { + let ($($name,)*) = &mut _fetch.fetch; + let ($($state,)*) = _state; + let result = ($( + // SAFETY: The invariants are required to be upheld by the caller. + $name.matches.then(|| unsafe { $name::try_fetch($state, &mut $name.fetch, _entity, _table_row) }).flatten(), + )*); + // If this is an archetypal query, then it is guaranteed to return `Some`, + // and we can help the compiler remove branches by checking the const `IS_ARCHETYPAL` first. + // If *none* of the subqueries matched the archetype, then this archetype was added in a transmute. + // We must treat those as matching in order to be consistent with `size_hint` for archetypal queries, + // so we treat them as matching for non-archetypal queries, as well. + (Self::IS_ARCHETYPAL || !_fetch.matches + // We want to return `Some` if the query matches this entity, + // which happens if at least one subquery returns `Some`. + // So, fetch everything as usual, but if all the subqueries return `None` then return `None` instead. + || !matches!(result, ($(Option::>::None,)*))) + .then_some(result) + } + fn iter_access(state: &Self::State) -> impl Iterator> { let ($($name,)*) = state; iter::empty()$(.chain($name::iter_access($name)))* From a103033dae2547852f9d5077f5ada47c9b1b61dd Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Thu, 22 Jan 2026 14:07:12 -0800 Subject: [PATCH 23/27] use try_fetch in iteration code --- crates/bevy_ecs/src/query/iter.rs | 136 ++++++++++-------------------- 1 file changed, 43 insertions(+), 93 deletions(-) diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 8f4076c6a865c..2f49db570cce3 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -1134,33 +1134,16 @@ where ); } + // The entity list has already been filtered by the query lens, so we forego filtering here. // SAFETY: - // - set_archetype was called prior. - // - `location.table_row` is in range of the current archetype. - let matches = unsafe { - D::matches( - &self.query_state.fetch_state, - &mut self.fetch, - entity, - location.table_row, - ) - }; - - matches.then(|| { - // The entity list has already been filtered by the query lens, so we forego filtering here. - // SAFETY: - // - set_archetype was called prior, `location.table_row` is an archetype index in range of the current archetype - // - fetch is only called once for each entity. - // - the surrounding guard verifies that this query matches the given row. - unsafe { - D::fetch( - &self.query_state.fetch_state, - &mut self.fetch, - entity, - location.table_row, - ) - } - }) + // - set_archetype was called prior, `location.table_row` is an archetype index in range of the current archetype + // - fetch is only called once for each entity. + D::try_fetch( + &self.query_state.fetch_state, + &mut self.fetch, + entity, + location.table_row, + ) } } @@ -1328,10 +1311,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> F::set_archetype(filter, &query_state.filter_state, archetype, table); } - // SAFETY: set_archetype was called prior. - // `location.archetype_row` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d - let matches_fetch = - unsafe { D::matches(&query_state.fetch_state, fetch, entity, location.table_row) }; // SAFETY: set_archetype was called prior. // `location.archetype_row` is an archetype index row in range of the current archetype, because if it was not, the match above would have `continue`d let matches_filter = unsafe { @@ -1343,20 +1322,16 @@ impl<'w, 's, D: QueryData, F: QueryFilter, I: Iterator> ) }; - if matches_fetch && matches_filter { - // SAFETY: - // - set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype - // - fetch is only called once for each entity. - // - this only runs if the fetch and filter both match - return unsafe { - Some(D::fetch( - &query_state.fetch_state, - fetch, - entity, - location.table_row, - )) - }; - } + return matches_filter + .then(|| { + // SAFETY: + // - set_archetype was called prior, `location.archetype_row` is an archetype index in range of the current archetype + // - fetch is only called once for each entity. + unsafe { + D::try_fetch(&query_state.fetch_state, fetch, entity, location.table_row) + } + }) + .flatten(); } None } @@ -2577,36 +2552,24 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // SAFETY: This is from an exclusive range, so it can't be max. let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(index)) }; - // SAFETY: set_table was called prior. - // `row` is a table row in range of the current table, - let matches_fetch = - unsafe { D::matches(&query_state.fetch_state, &mut self.fetch, entity, row) }; // SAFETY: set_table was called prior. // `row` is a table row in range of the current table, let matches_filter = unsafe { F::matches(&query_state.filter_state, &mut self.filter, entity, row) }; - (matches_fetch && matches_filter).then(|| + matches_filter // SAFETY: // - `set_table` must have been called previously either in `next` or before it. // - `*entity` and `index` are in the current table. - // - `row` matches both the fetch and filter - unsafe { D::fetch(&query_state.fetch_state, &mut self.fetch, entity, row) }) + .then(|| unsafe { + D::try_fetch(&query_state.fetch_state, &mut self.fetch, entity, row) + }) + .flatten() } else { // SAFETY: This must have been called previously in `next` as `current_row > 0` let archetype_entity = unsafe { self.archetype_entities.get_unchecked(index as usize) }; - // SAFETY: set_archetype was called prior. - // `archetype_entity.table_row()` is an archetype index row in range of the current archetype, - let matches_fetch = unsafe { - D::matches( - &query_state.fetch_state, - &mut self.fetch, - archetype_entity.id(), - archetype_entity.table_row(), - ) - }; // SAFETY: set_archetype was called prior. // `archetype_entity.table_row()` is an archetype index row in range of the current archetype, let matches_filter = unsafe { @@ -2618,19 +2581,19 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { ) }; - (matches_fetch && matches_filter).then(|| + matches_filter // SAFETY: // - `set_archetype` must have been called previously either in `next` or before it. // - `archetype_entity.id()` and `archetype_entity.table_row()` are in the current archetype. - // - the current archetype entity matches both the fetch and filter - unsafe { - D::fetch( + .then(|| unsafe { + D::try_fetch( &query_state.fetch_state, &mut self.fetch, archetype_entity.id(), archetype_entity.table_row(), ) }) + .flatten() } } else { None @@ -2696,16 +2659,12 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { let row = unsafe { TableRow::new(NonMaxU32::new_unchecked(self.current_row)) }; self.current_row += 1; - // SAFETY: set_table was called prior. - // `row` is a table row in range of the current table, - let matches_fetch = - unsafe { D::matches(&query_state.fetch_state, &mut self.fetch, entity, row) }; // SAFETY: set_table was called prior. // `row` is a table row in range of the current table, let matches_filter = unsafe { F::matches(&query_state.filter_state, &mut self.filter, entity, row) }; - if !(matches_fetch && matches_filter) { + if !matches_filter { continue; } @@ -2714,14 +2673,12 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `current_row` must be a table row in range of the current table, // because if it was not, then the above would have been executed. // - fetch is only called once for each `entity`. - return unsafe { - Some(D::fetch( - &query_state.fetch_state, - &mut self.fetch, - entity, - row, - )) - }; + let item = + unsafe { D::try_fetch(&query_state.fetch_state, &mut self.fetch, entity, row) }; + + if let Some(item) = item { + return Some(item); + } } } else { loop { @@ -2761,16 +2718,6 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { }; self.current_row += 1; - // SAFETY: set_archetype was called prior. - // `archetype_entity.table_row()` is an archetype index row in range of the current archetype, - let matches_fetch = unsafe { - D::matches( - &query_state.fetch_state, - &mut self.fetch, - archetype_entity.id(), - archetype_entity.table_row(), - ) - }; // SAFETY: set_archetype was called prior. // `archetype_entity.table_row()` is an archetype index row in range of the current archetype, let matches_filter = unsafe { @@ -2782,7 +2729,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { ) }; - if !(matches_fetch && matches_filter) { + if !matches_filter { continue; } @@ -2791,15 +2738,18 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIterationCursor<'w, 's, D, F> { // - `current_row` must be an archetype index row in range of the current archetype, // because if it was not, then the if above would have been executed. // - fetch is only called once for each `archetype_entity`. - // - `archetype_entity` matches both the fetch and filter - return unsafe { - Some(D::fetch( + let item = unsafe { + D::try_fetch( &query_state.fetch_state, &mut self.fetch, archetype_entity.id(), archetype_entity.table_row(), - )) + ) }; + + if let Some(item) = item { + return Some(item); + } } } } From 812a3641d207bf1371dec19b5979c8531ca74548 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Thu, 22 Jan 2026 14:18:43 -0800 Subject: [PATCH 24/27] tuple lookahead caching --- crates/bevy_ecs/src/query/fetch.rs | 4 +- crates/bevy_ecs/src/query/world_query.rs | 82 +++++++++++++++++++----- 2 files changed, 67 insertions(+), 19 deletions(-) diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 26e17a74a3eac..83e391c2e0d64 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -2863,7 +2863,7 @@ macro_rules! impl_tuple_query_data { let ($($state,)*) = state; let ($($name,)*) = fetch; // SAFETY: The invariants are upheld by the caller. - ($(unsafe { $name::fetch($state, $name, entity, table_row) },)*) + ($(unsafe { $name::fetch($state, &mut $name.fetch, entity, table_row) },)*) } #[inline(always)] @@ -2876,7 +2876,7 @@ macro_rules! impl_tuple_query_data { let ($($state,)*) = state; let ($($name,)*) = fetch; // SAFETY: The invariants are upheld by the caller. - Some(($(unsafe { $name::try_fetch($state, $name, entity, table_row) }?,)*)) + Some(($(unsafe { $name::try_fetch($state, &mut $name.fetch, entity, table_row) }?,)*)) } fn iter_access(state: &Self::State) -> impl Iterator> { diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 9939c556cd0b7..d58bf7a8438b2 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -310,6 +310,24 @@ impl RangeExt for Range { } } +/// A wrapper `Fetch` type for tuple query terms that caches the last seen valid chunk. +#[doc(hidden)] +pub struct TupleFetch<'w, T: WorldQuery> { + /// the `Fetch` type of the query term + pub(crate) fetch: T::Fetch<'w>, + // the cached chunk last found in `find_table_chunk`/`find_archetype_chunk` + chunk: Range, +} + +impl<'w, T: WorldQuery> Clone for TupleFetch<'w, T> { + fn clone(&self) -> Self { + Self { + fetch: self.fetch.clone(), + chunk: self.chunk.clone(), + } + } +} + macro_rules! impl_tuple_world_query { ($(#[$meta:meta])* $(($name: ident, $state: ident)),*) => { @@ -342,22 +360,30 @@ macro_rules! impl_tuple_world_query { /// `lookahead_table` and `lookahead_archetype` always return a subset of `rows`/`indices` /// This is sound because each `lookahead_table` and `lookahead_archetype` each unsafe impl<$($name: WorldQuery),*> WorldQuery for ($($name,)*) { - type Fetch<'w> = ($($name::Fetch<'w>,)*); + type Fetch<'w> = ($(TupleFetch<'w, $name>,)*); type State = ($($name::State,)*); fn shrink_fetch<'wlong: 'wshort, 'wshort>(fetch: Self::Fetch<'wlong>) -> Self::Fetch<'wshort> { let ($($name,)*) = fetch; ($( - $name::shrink_fetch($name), + TupleFetch { + fetch: $name::shrink_fetch($name.fetch), + chunk: $name.chunk + }, )*) } #[inline] unsafe fn init_fetch<'w, 's>(world: UnsafeWorldCell<'w>, state: &'s Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; - // SAFETY: The invariants are upheld by the caller. - ($(unsafe { $name::init_fetch(world, $name, last_run, this_run) },)*) + ($( + TupleFetch { + // SAFETY: The invariants are upheld by the caller. + fetch: unsafe { $name::init_fetch(world, $name, last_run, this_run) }, + chunk: 0..0, + }, + )*) } const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; @@ -372,16 +398,22 @@ macro_rules! impl_tuple_world_query { ) { let ($($name,)*) = fetch; let ($($state,)*) = state; - // SAFETY: The invariants are upheld by the caller. - $(unsafe { $name::set_archetype($name, $state, archetype, table); })* + $({ + // SAFETY: The invariants are upheld by the caller. + unsafe { $name::set_archetype(&mut $name.fetch, $state, archetype, table); }; + $name.chunk = 0..0; + })* } #[inline] unsafe fn set_table<'w, 's>(fetch: &mut Self::Fetch<'w>, state: &'s Self::State, table: &'w Table) { let ($($name,)*) = fetch; let ($($state,)*) = state; - // SAFETY: The invariants are upheld by the caller. - $(unsafe { $name::set_table($name, $state, table); })* + $({ + // SAFETY: The invariants are upheld by the caller. + unsafe { $name::set_table(&mut $name.fetch, $state, table); } + $name.chunk = 0..0 + })* } @@ -408,16 +440,31 @@ macro_rules! impl_tuple_world_query { table_entities: &[Entity], mut rows: Range, ) -> Range { - if Self::IS_ARCHETYPAL { + if Self::IS_ARCHETYPAL || rows.is_empty() { rows } else { + let mut chunk = rows.clone(); let ($($name,)*) = fetch; let ($($state,)*) = state; - // SAFETY: `rows` is only ever narrowed as we iterate subqueries, so it's - // always valid to pass to the next term. Other invariants are upheld by - // the caller. - $(rows = unsafe { $name::find_table_chunk($state, $name, table_entities, rows) };)* - rows + loop { + let mut any_valid_terms = false; + $({ + let mut term_chunk = $name.chunk.start.max(chunk.start)..$name.chunk.end; + if term_chunk.is_empty() { + // SAFETY: chunk.start..rows.end is always a subset of `rows` + term_chunk = unsafe { $name::find_table_chunk($state, &mut $name.fetch, table_entities, chunk.start..rows.end) }; + } + any_valid_terms |= !term_chunk.is_empty(); + chunk.start = chunk.start.max(term_chunk.start); + chunk.end = chunk.end.min(term_chunk.end).max(chunk.start); + $name.chunk = term_chunk; + })* + + if !chunk.is_empty() || !any_valid_terms { + break; + } + } + chunk } } @@ -428,15 +475,16 @@ macro_rules! impl_tuple_world_query { archetype_entities: &[ArchetypeEntity], mut indices: Range, ) -> Range { - if Self::IS_ARCHETYPAL { + if Self::IS_ARCHETYPAL || indices.is_empty() { indices } else { + let mut chunk = indices.clone(); let ($($name,)*) = fetch; let ($($state,)*) = state; // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's // always valid to pass to the next term. Other invariants are upheld by // the caller. - $(indices = unsafe { $name::find_archetype_chunk($state, $name, archetype_entities, indices) };)* + $(indices = unsafe { $name::find_archetype_chunk($state, &mut $name.fetch, archetype_entities, indices) };)* indices } } @@ -454,7 +502,7 @@ macro_rules! impl_tuple_world_query { let ($($name,)*) = fetch; let ($($state,)*) = state; // SAFETY: invariants are upheld by the caller. - true $(&& unsafe { $name::matches($state, $name, entity, table_row) })* + true $(&& unsafe { $name::matches($state, &mut $name.fetch, entity, table_row) })* } } } From 857666bace2a24eeb6b2ef1d03679e1f3dff84b6 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Fri, 30 Jan 2026 17:07:28 -0800 Subject: [PATCH 25/27] fix tuple caching and WorldQuery docs --- crates/bevy_ecs/src/query/world_query.rs | 29 ++++++++++++------------ 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index d58bf7a8438b2..43caa8f0bb9b2 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -39,9 +39,10 @@ use variadics_please::all_tuples; /// If [`IS_ARCHETYPAL`] is `true`, [`matches`] must return `true` for all inputs. /// /// When implementing [`find_table_chunk`] and [`find_archetype_chunk`], their return values must be -/// subsets of `rows` and `indices` respectively, even if returning an empty range. Additionally, -/// they must match the behavior of [`matches`], i.e. calling that method on any -/// row/index in the returned range should return `true`. +/// _well formed_ subsets of `rows` and `indices` respectively, even if returning an empty range. +/// What this means is that given a range `(start..end)`, the returned range `(chunk_start..chunk_end)` +/// must satisfy `start <= chunk_start <= chunk_end <= end`. Additionally, they must match the behavior +/// of [`matches`], i.e. calling that method on any row/index in the returned range must return `true`. /// /// [`find_table_chunk`], [`find_archetype_chunk`], and [`matches`] must not mutably access any world data. /// @@ -315,15 +316,15 @@ impl RangeExt for Range { pub struct TupleFetch<'w, T: WorldQuery> { /// the `Fetch` type of the query term pub(crate) fetch: T::Fetch<'w>, - // the cached chunk last found in `find_table_chunk`/`find_archetype_chunk` - chunk: Range, + // the `end` value of the chunk last found in `find_table_chunk`/`find_archetype_chunk` + chunk_end: u32, } impl<'w, T: WorldQuery> Clone for TupleFetch<'w, T> { fn clone(&self) -> Self { Self { fetch: self.fetch.clone(), - chunk: self.chunk.clone(), + chunk_end: self.chunk_end, } } } @@ -369,7 +370,7 @@ macro_rules! impl_tuple_world_query { ($( TupleFetch { fetch: $name::shrink_fetch($name.fetch), - chunk: $name.chunk + chunk_end: $name.chunk_end }, )*) } @@ -381,7 +382,7 @@ macro_rules! impl_tuple_world_query { TupleFetch { // SAFETY: The invariants are upheld by the caller. fetch: unsafe { $name::init_fetch(world, $name, last_run, this_run) }, - chunk: 0..0, + chunk_end: 0, }, )*) } @@ -401,7 +402,7 @@ macro_rules! impl_tuple_world_query { $({ // SAFETY: The invariants are upheld by the caller. unsafe { $name::set_archetype(&mut $name.fetch, $state, archetype, table); }; - $name.chunk = 0..0; + $name.chunk_end = 0; })* } @@ -412,7 +413,7 @@ macro_rules! impl_tuple_world_query { $({ // SAFETY: The invariants are upheld by the caller. unsafe { $name::set_table(&mut $name.fetch, $state, table); } - $name.chunk = 0..0 + $name.chunk_end = 0; })* } @@ -449,15 +450,15 @@ macro_rules! impl_tuple_world_query { loop { let mut any_valid_terms = false; $({ - let mut term_chunk = $name.chunk.start.max(chunk.start)..$name.chunk.end; + let mut term_chunk = chunk.start..$name.chunk_end; if term_chunk.is_empty() { // SAFETY: chunk.start..rows.end is always a subset of `rows` term_chunk = unsafe { $name::find_table_chunk($state, &mut $name.fetch, table_entities, chunk.start..rows.end) }; } any_valid_terms |= !term_chunk.is_empty(); - chunk.start = chunk.start.max(term_chunk.start); - chunk.end = chunk.end.min(term_chunk.end).max(chunk.start); - $name.chunk = term_chunk; + chunk.start = term_chunk.start; + chunk.end = chunk.end.min(term_chunk.end); + $name.chunk_end = term_chunk.end; })* if !chunk.is_empty() || !any_valid_terms { From 57b2bcb4cc76b850d4a0d07395a2a21f8c9513bc Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Fri, 30 Jan 2026 17:09:48 -0800 Subject: [PATCH 26/27] safer AssetChanged WorldQuery impl --- crates/bevy_asset/src/asset_changed.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/bevy_asset/src/asset_changed.rs b/crates/bevy_asset/src/asset_changed.rs index 165a86b4e2315..9721a3d1d6b2f 100644 --- a/crates/bevy_asset/src/asset_changed.rs +++ b/crates/bevy_asset/src/asset_changed.rs @@ -272,10 +272,9 @@ unsafe impl WorldQuery for AssetChanged { ) -> bool { fetch.inner.as_mut().is_some_and(|inner| { // SAFETY: We delegate to the inner `fetch` for `A`. - // Component accesses are archetypal, so we don't need to check if it `matches`. unsafe { - let handle = <&A>::fetch(&state.asset_id, inner, entity, table_row); - fetch.check.has_changed(handle) + let handle = <&A>::try_fetch(&state.asset_id, inner, entity, table_row); + handle.is_some_and(|handle| fetch.check.has_changed(handle)) } }) } From b2f5e96b4ce2f5104c4d266c4dc8f710261aa338 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Sat, 31 Jan 2026 10:29:42 -0800 Subject: [PATCH 27/27] fix tuple impl again --- crates/bevy_ecs/src/query/world_query.rs | 56 ++++++++++++++++++------ 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 43caa8f0bb9b2..0bd3f2e2bf687 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -448,24 +448,32 @@ macro_rules! impl_tuple_world_query { let ($($name,)*) = fetch; let ($($state,)*) = state; loop { - let mut any_valid_terms = false; - $({ + $(if !$name::IS_ARCHETYPAL { // help the compiler out to remove unnecessary terms let mut term_chunk = chunk.start..$name.chunk_end; + // if the term chunk is empty, we've gone past what what previously + // cached and need to refresh. if term_chunk.is_empty() { // SAFETY: chunk.start..rows.end is always a subset of `rows` term_chunk = unsafe { $name::find_table_chunk($state, &mut $name.fetch, table_entities, chunk.start..rows.end) }; + $name.chunk_end = term_chunk.end; + } + // if the term chunk is empty even after refreshing the cache, there's + // no more matches in this table. + if term_chunk.is_empty() { + return term_chunk; } - any_valid_terms |= !term_chunk.is_empty(); chunk.start = term_chunk.start; chunk.end = chunk.end.min(term_chunk.end); - $name.chunk_end = term_chunk.end; + if chunk.is_empty() { + // the current chunk can't continue matching, so we + // have to advance its end to the next feasible chunk + chunk.end = term_chunk.end; + continue; + } })* - if !chunk.is_empty() || !any_valid_terms { - break; - } + return chunk; } - chunk } } @@ -482,11 +490,33 @@ macro_rules! impl_tuple_world_query { let mut chunk = indices.clone(); let ($($name,)*) = fetch; let ($($state,)*) = state; - // SAFETY: `indices` is only ever narrowed as we iterate subqueries, so it's - // always valid to pass to the next term. Other invariants are upheld by - // the caller. - $(indices = unsafe { $name::find_archetype_chunk($state, &mut $name.fetch, archetype_entities, indices) };)* - indices + loop { + $(if !$name::IS_ARCHETYPAL { // help the compiler out to remove unnecessary terms + let mut term_chunk = chunk.start..$name.chunk_end; + // if the term chunk is empty, we've gone past what what previously + // cached and need to refresh. + if term_chunk.is_empty() { + // SAFETY: chunk.start..indices.end is always a subset of `indices` + term_chunk = unsafe { $name::find_archetype_chunk($state, &mut $name.fetch, archetype_entities, chunk.start..indices.end) }; + $name.chunk_end = term_chunk.end; + } + // if the term chunk is empty even after refreshing the cache, there's + // no more matches in this table. + if term_chunk.is_empty() { + return term_chunk; + } + chunk.start = term_chunk.start; + chunk.end = chunk.end.min(term_chunk.end); + if chunk.is_empty() { + // the current chunk can't continue matching, so we + // have to advance its end to the next feasible chunk + chunk.end = term_chunk.end; + continue; + } + })* + + return chunk; + } } }