f7e112a3c9
10 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Chris Russell
|
f7e112a3c9
|
Let query items borrow from query state to avoid needing to clone (#15396)
# Objective Improve the performance of `FilteredEntity(Ref|Mut)` and `Entity(Ref|Mut)Except`. `FilteredEntityRef` needs an `Access<ComponentId>` to determine what components it can access. There is one stored in the query state, but query items cannot borrow from the state, so it has to `clone()` the access for each row. Cloning the access involves memory allocations and can be expensive. ## Solution Let query items borrow from their query state. Add an `'s` lifetime to `WorldQuery::Item` and `WorldQuery::Fetch`, similar to the one in `SystemParam`, and provide `&'s Self::State` to the fetch so that it can borrow from the state. Unfortunately, there are a few cases where we currently return query items from temporary query states: the sorted iteration methods create a temporary state to query the sort keys, and the `EntityRef::components<Q>()` methods create a temporary state for their query. To allow these to continue to work with most `QueryData` implementations, introduce a new subtrait `ReleaseStateQueryData` that converts a `QueryItem<'w, 's>` to `QueryItem<'w, 'static>`, and is implemented for everything except `FilteredEntity(Ref|Mut)` and `Entity(Ref|Mut)Except`. `#[derive(QueryData)]` will generate `ReleaseStateQueryData` implementations that apply when all of the subqueries implement `ReleaseStateQueryData`. This PR does not actually change the implementation of `FilteredEntity(Ref|Mut)` or `Entity(Ref|Mut)Except`! That will be done as a follow-up PR so that the changes are easier to review. I have pushed the changes as chescock/bevy#5. ## Testing I ran performance traces of many_foxes, both against main and against chescock/bevy#5, both including #15282. These changes do appear to make generalized animation a bit faster: (Red is main, yellow is chescock/bevy#5)  ## Migration Guide The `WorldQuery::Item` and `WorldQuery::Fetch` associated types and the `QueryItem` and `ROQueryItem` type aliases now have an additional lifetime parameter corresponding to the `'s` lifetime in `Query`. Manual implementations of `WorldQuery` will need to update the method signatures to include the new lifetimes. Other uses of the types will need to be updated to include a lifetime parameter, although it can usually be passed as `'_`. In particular, `ROQueryItem` is used when implementing `RenderCommand`. Before: ```rust fn render<'w>( item: &P, view: ROQueryItem<'w, Self::ViewQuery>, entity: Option<ROQueryItem<'w, Self::ItemQuery>>, param: SystemParamItem<'w, '_, Self::Param>, pass: &mut TrackedRenderPass<'w>, ) -> RenderCommandResult; ``` After: ```rust fn render<'w>( item: &P, view: ROQueryItem<'w, '_, Self::ViewQuery>, entity: Option<ROQueryItem<'w, '_, Self::ItemQuery>>, param: SystemParamItem<'w, '_, Self::Param>, pass: &mut TrackedRenderPass<'w>, ) -> RenderCommandResult; ``` --- Methods on `QueryState` that take `&mut self` may now result in conflicting borrows if the query items capture the lifetime of the mutable reference. This affects `get()`, `iter()`, and others. To fix the errors, first call `QueryState::update_archetypes()`, and then replace a call `state.foo(world, param)` with `state.query_manual(world).foo_inner(param)`. Alternately, you may be able to restructure the code to call `state.query(world)` once and then make multiple calls using the `Query`. Before: ```rust let mut state: QueryState<_, _> = ...; let d1 = state.get(world, e1); let d2 = state.get(world, e2); // Error: cannot borrow `state` as mutable more than once at a time println!("{d1:?}"); println!("{d2:?}"); ``` After: ```rust let mut state: QueryState<_, _> = ...; state.update_archetypes(world); let d1 = state.get_manual(world, e1); let d2 = state.get_manual(world, e2); // OR state.update_archetypes(world); let d1 = state.query(world).get_inner(e1); let d2 = state.query(world).get_inner(e2); // OR let query = state.query(world); let d1 = query.get_inner(e1); let d1 = query.get_inner(e2); println!("{d1:?}"); println!("{d2:?}"); ``` |
||
|
Chris Russell
|
bea0a0a9bc
|
Let FilteredEntity(Ref|Mut) receive access when nested. (#18236)
# Objective Let `FilteredEntityRef` and `FilteredEntityMut` receive access when nested inside tuples or `#[derive(QueryData)]` types. Make sure to exclude any access that would conflict with other subqueries! Fixes #14349 ## Solution Replace `WorldQuery::set_access(state, access)` with a new method, `QueryData::provide_extra_access(state, access, available_access)`, that passes both the total available access and the currently used access. This is called after `WorldQuery::update_component_access()`, so any access used by ordinary subqueries will be known. `FilteredEntityRef` and `FilteredEntityMut` can use the combination to determine how much access they can safely take, while tuples can safely pass those parameters directly to their subqueries. This requires a new `Access::remove_conflicting_access()` method that can be used to remove any access that would conflict with existing access. Implementing this method was easier by first factoring some common set manipulation code out of `Access::extend`. I can extract that refactoring to a separate PR if desired. Have `FilteredEntity(Ref|Mut)` store `Access` instead of `FilteredAccess` because they do not need to keep track of the filter. This was necessary in an early draft but no longer is. I left it in because it's small and I'm touching that code anyway, but I can extract it to a separate PR if desired. |
||
|
Chris Russell
|
1f6642df4c
|
Fix unsound query transmutes on queries obtained from Query::as_readonly() (#17973)
# Objective
Fix unsound query transmutes on queries obtained from
`Query::as_readonly()`.
The following compiles, and the call to `transmute_lens()` should panic,
but does not:
```rust
fn bad_system(query: Query<&mut A>) {
let mut readonly = query.as_readonly();
let mut lens: QueryLens<&mut A> = readonly.transmute_lens();
let other_readonly: Query<&A> = query.as_readonly();
// `lens` and `other_readonly` alias, and are both alive here!
}
```
To make `Query::as_readonly()` zero-cost, we pointer-cast
`&QueryState<D, F>` to `&QueryState<D::ReadOnly, F>`. This means that
the `component_access` for a read-only query's state may include
accesses for the original mutable version, but the `Query` does not have
exclusive access to those components! `transmute` and `join` use that
access to ensure that a join is valid, and will incorrectly allow a
transmute that includes mutable access.
As a bonus, allow `Query::join`s that output `FilteredEntityRef` or
`FilteredEntityMut` to receive access from the `other` query. Currently
they only receive access from `self`.
## Solution
When transmuting or joining from a read-only query, remove any writes
before performing checking that the transmute is valid. For joins, be
sure to handle the case where one input query was the result of
`as_readonly()` but the other has valid mutable access.
This requires identifying read-only queries, so add a
`QueryData::IS_READ_ONLY` associated constant. Note that we only call
`QueryState::as_transmuted_state()` with `NewD: ReadOnlyQueryData`, so
checking for read-only queries is sufficient to check for
`as_transmuted_state()`.
Removing writes requires allocating a new `FilteredAccess`, so only do
so if the query is read-only and the state has writes. Otherwise, the
existing access is correct and we can continue using a reference to it.
Use the new read-only state to call `NewD::set_access`, so that
transmuting to a `FilteredAccessMut` results in a read-only
`FilteredAccessMut`. Otherwise, it would take the original write access,
and then the transmute would panic because it had too much access.
Note that `join` was previously passing `self.component_access` to
`NewD::set_access`. Switching it to `joined_component_access` also
allows a join that outputs `FilteredEntity(Ref|Mut)` to receive access
from `other`. The fact that it didn't do that before seems like an
oversight, so I didn't try to prevent that change.
## Testing
Added unit tests with the unsound transmute and join.
|
||
|
couyit
|
03af547c28
|
Move Item and fetch to QueryData from WorldQuery (#17679)
# Objective Fixes #17662 ## Solution Moved `Item` and `fetch` from `WorldQuery` to `QueryData`, and adjusted their implementations accordingly. Currently, documentation related to `fetch` is written under `WorldQuery`. It would be more appropriate to move it to the `QueryData` documentation for clarity. I am not very experienced with making contributions. If there are any mistakes or areas for improvement, I would appreciate any suggestions you may have. ## Migration Guide The `WorldQuery::Item` type and `WorldQuery::fetch` method have been moved to `QueryData`, as they were not useful for `QueryFilter` types. --------- Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com> |
||
|
Benjamin Brienen
|
1675d68366
|
Fix beta CI (#16984)
# Objective Fixes #16607 ## Solution Satisfy clippy ## Testing Ran clippy |
||
|
Gino Valente
|
8039f34b0d
|
bevy_ecs: Replace panics in QueryData derive compile errors (#15691)
# Objective
The current `QueryData` derive panics when it encounters an error.
Additionally, it doesn't provide the clearest error message:
```rust
#[derive(QueryData)]
#[query_data(mut)]
struct Foo {
// ...
}
```
```
error: proc-macro derive panicked
--> src/foo.rs:16:10
|
16 | #[derive(QueryData)]
| ^^^^^^^^^
|
= help: message: Invalid `query_data` attribute format
```
## Solution
Updated the derive logic to not panic and gave a bit more detail in the
error message.
This is makes the error message just a bit clearer and maintains the
correct span:
```
error: invalid attribute, expected `mutable` or `derive`
--> src/foo.rs:17:14
|
17 | #[query_data(mut)]
| ^^^
```
## Testing
You can test locally by running the following in
`crates/bevy_ecs/compile_fail`:
```
cargo test --target-dir ../../../target
```
|
||
|
Rich Churcher
|
fd329c0426
|
Allow to expect (adopted) (#15301)
# Objective > Rust 1.81 released the #[expect(...)] attribute, which works like #[allow(...)] but throws a warning if the lint isn't raised. This is preferred to #[allow(...)] because it tells us when it can be removed. - Adopts the parts of #15118 that are complete, and updates the branch so it can be merged. - There were a few conflicts, let me know if I misjudged any of 'em. Alice's [recommendation](https://github.com/bevyengine/bevy/issues/15059#issuecomment-2349263900) seems well-taken, let's do this crate by crate now that @BD103 has done the lion's share of this! (Relates to, but doesn't yet completely finish #15059.) Crates this _doesn't_ cover: - bevy_input - bevy_gilrs - bevy_window - bevy_winit - bevy_state - bevy_render - bevy_picking - bevy_core_pipeline - bevy_sprite - bevy_text - bevy_pbr - bevy_ui - bevy_gltf - bevy_gizmos - bevy_dev_tools - bevy_internal - bevy_dylib --------- Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com> Co-authored-by: Ben Frankel <ben.frankel7@gmail.com> Co-authored-by: Antony <antony.m.3012@gmail.com> |
||
|
no-materials
|
3a8d5598ad
|
Interpolate WorldQuery path in docs of generated types (#14985)
# Objective Fixes #14972 ## Solution Uses the `concat!` macro to interpolate the `path` variable. ## Testing * Run `cargo doc --workspace --open` * Check functionality of `WorldQuery` links within `NodeQueryItem`, `NodeQueryReadOnly`, `NodeQueryReadOnlyItem` docs |
||
|
EdJoPaTo
|
938d810766
|
Apply unused_qualifications lint (#14828)
# Objective Fixes #14782 ## Solution Enable the lint and fix all upcoming hints (`--fix`). Also tried to figure out the false-positive (see review comment). Maybe split this PR up into multiple parts where only the last one enables the lint, so some can already be merged resulting in less many files touched / less potential for merge conflicts? Currently, there are some cases where it might be easier to read the code with the qualifier, so perhaps remove the import of it and adapt its cases? In the current stage it's just a plain adoption of the suggestions in order to have a base to discuss. ## Testing `cargo clippy` and `cargo run -p ci` are happy. |
||
|
Mantas
|
5af2f022d8
|
Rename WorldQueryData & WorldQueryFilter to QueryData & QueryFilter (#10779)
# Rename `WorldQueryData` & `WorldQueryFilter` to `QueryData` & `QueryFilter` Fixes #10776 ## Solution Traits `WorldQueryData` & `WorldQueryFilter` were renamed to `QueryData` and `QueryFilter`, respectively. Related Trait types were also renamed. --- ## Changelog - Trait `WorldQueryData` has been renamed to `QueryData`. Derive macro's `QueryData` attribute `world_query_data` has been renamed to `query_data`. - Trait `WorldQueryFilter` has been renamed to `QueryFilter`. Derive macro's `QueryFilter` attribute `world_query_filter` has been renamed to `query_filter`. - Trait's `ExtractComponent` type `Query` has been renamed to `Data`. - Trait's `GetBatchData` types `Query` & `QueryFilter` has been renamed to `Data` & `Filter`, respectively. - Trait's `ExtractInstance` type `Query` has been renamed to `Data`. - Trait's `ViewNode` type `ViewQuery` has been renamed to `ViewData`. - Trait's `RenderCommand` types `ViewWorldQuery` & `ItemWorldQuery` has been renamed to `ViewData` & `ItemData`, respectively. ## Migration Guide Note: if merged before 0.13 is released, this should instead modify the migration guide of #10776 with the updated names. - Rename `WorldQueryData` & `WorldQueryFilter` trait usages to `QueryData` & `QueryFilter` and their respective derive macro attributes `world_query_data` & `world_query_filter` to `query_data` & `query_filter`. - Rename the following trait type usages: - Trait's `ExtractComponent` type `Query` to `Data`. - Trait's `GetBatchData` type `Query` to `Data`. - Trait's `ExtractInstance` type `Query` to `Data`. - Trait's `ViewNode` type `ViewQuery` to `ViewData`' - Trait's `RenderCommand` types `ViewWolrdQuery` & `ItemWorldQuery` to `ViewData` & `ItemData`, respectively. ```rust // Before #[derive(WorldQueryData)] #[world_query_data(derive(Debug))] struct EmptyQuery { empty: (), } // After #[derive(QueryData)] #[query_data(derive(Debug))] struct EmptyQuery { empty: (), } // Before #[derive(WorldQueryFilter)] struct CustomQueryFilter<T: Component, P: Component> { _c: With<ComponentC>, _d: With<ComponentD>, _or: Or<(Added<ComponentC>, Changed<ComponentD>, Without<ComponentZ>)>, _generic_tuple: (With<T>, With<P>), } // After #[derive(QueryFilter)] struct CustomQueryFilter<T: Component, P: Component> { _c: With<ComponentC>, _d: With<ComponentD>, _or: Or<(Added<ComponentC>, Changed<ComponentD>, Without<ComponentZ>)>, _generic_tuple: (With<T>, With<P>), } // Before impl ExtractComponent for ContrastAdaptiveSharpeningSettings { type Query = &'static Self; type Filter = With<Camera>; type Out = (DenoiseCAS, CASUniform); fn extract_component(item: QueryItem<Self::Query>) -> Option<Self::Out> { //... } } // After impl ExtractComponent for ContrastAdaptiveSharpeningSettings { type Data = &'static Self; type Filter = With<Camera>; type Out = (DenoiseCAS, CASUniform); fn extract_component(item: QueryItem<Self::Data>) -> Option<Self::Out> { //... } } // Before impl GetBatchData for MeshPipeline { type Param = SRes<RenderMeshInstances>; type Query = Entity; type QueryFilter = With<Mesh3d>; type CompareData = (MaterialBindGroupId, AssetId<Mesh>); type BufferData = MeshUniform; fn get_batch_data( mesh_instances: &SystemParamItem<Self::Param>, entity: &QueryItem<Self::Query>, ) -> (Self::BufferData, Option<Self::CompareData>) { // .... } } // After impl GetBatchData for MeshPipeline { type Param = SRes<RenderMeshInstances>; type Data = Entity; type Filter = With<Mesh3d>; type CompareData = (MaterialBindGroupId, AssetId<Mesh>); type BufferData = MeshUniform; fn get_batch_data( mesh_instances: &SystemParamItem<Self::Param>, entity: &QueryItem<Self::Data>, ) -> (Self::BufferData, Option<Self::CompareData>) { // .... } } // Before impl<A> ExtractInstance for AssetId<A> where A: Asset, { type Query = Read<Handle<A>>; type Filter = (); fn extract(item: QueryItem<'_, Self::Query>) -> Option<Self> { Some(item.id()) } } // After impl<A> ExtractInstance for AssetId<A> where A: Asset, { type Data = Read<Handle<A>>; type Filter = (); fn extract(item: QueryItem<'_, Self::Data>) -> Option<Self> { Some(item.id()) } } // Before impl ViewNode for PostProcessNode { type ViewQuery = ( &'static ViewTarget, &'static PostProcessSettings, ); fn run( &self, _graph: &mut RenderGraphContext, render_context: &mut RenderContext, (view_target, _post_process_settings): QueryItem<Self::ViewQuery>, world: &World, ) -> Result<(), NodeRunError> { // ... } } // After impl ViewNode for PostProcessNode { type ViewData = ( &'static ViewTarget, &'static PostProcessSettings, ); fn run( &self, _graph: &mut RenderGraphContext, render_context: &mut RenderContext, (view_target, _post_process_settings): QueryItem<Self::ViewData>, world: &World, ) -> Result<(), NodeRunError> { // ... } } // Before impl<P: CachedRenderPipelinePhaseItem> RenderCommand<P> for SetItemPipeline { type Param = SRes<PipelineCache>; type ViewWorldQuery = (); type ItemWorldQuery = (); #[inline] fn render<'w>( item: &P, _view: (), _entity: (), pipeline_cache: SystemParamItem<'w, '_, Self::Param>, pass: &mut TrackedRenderPass<'w>, ) -> RenderCommandResult { // ... } } // After impl<P: CachedRenderPipelinePhaseItem> RenderCommand<P> for SetItemPipeline { type Param = SRes<PipelineCache>; type ViewData = (); type ItemData = (); #[inline] fn render<'w>( item: &P, _view: (), _entity: (), pipeline_cache: SystemParamItem<'w, '_, Self::Param>, pass: &mut TrackedRenderPass<'w>, ) -> RenderCommandResult { // ... } } ``` |