Split WorldQuery into WorldQueryData and WorldQueryFilter #8899

wainwrightmark · 2023-06-20T09:32:28Z

Objective

Fixes Use distinct types for query "data" generics and query "filter" generics #7680

Solution

The traits WorldQueryData : WorldQuery and WorldQueryFilter : WorldQuery have been added and some of the types and functions from WorldQuery has been moved into them.

ReadOnlyWorldQuery has been replaced with ReadOnlyWorldQueryData.

WorldQueryFilter is safe (as long as WorldQuery is implemented safely).

WorldQueryData is unsafe - safely implementing it requires that Self::ReadOnly is a readonly version of Self (this used to be a safety requirement of WorldQuery)

The type parameters Q and F of Query must now implement WorldQueryData and WorldQueryFilter respectively.

This makes it impossible to accidentally use a filter in the data position or vice versa which was something that could lead to bugs. ~~Compile failure tests have been added to check this.~~

It was previously sometimes useful to use Option<With<T>> in the data position. Use Has<T> instead in these cases.

The derive macro has been split into separate derive macros for WorldQueryData and WorldQueryFilter.

Previously it was possible to derive both WorldQuery for a struct that had a mixture of data and filter items. This would not work correctly in some cases but could be a useful pattern in others. This is no longer possible.

Notes

The changes outside of bevy_ecs are all changing type parameters to the new types, updating the macro use, or replacing Option<With<T>> with Has<T>.
All WorldQueryData types always returned true for IS_ARCHETYPAL so I moved it to WorldQueryFilter and
replaced all calls to it with true. That should be the only logic change outside of the macro generation code.
Changed<T> and Added<T> were being generated by a macro that I have expanded. Happy to revert that if desired.
The two derive macros share some functions for implementing WorldQuery but the tidiest way I could find to implement them was to give them a ton of arguments and ask clippy to ignore that.

Changelog

Changed

Split WorldQuery into WorldQueryData and WorldQueryFilter which now have separate derive macros. It is not possible to derive both for the same type.
Query now requires that the first type argument implements WorldQueryData and the second implements WorldQueryFilter

Migration Guide

Update derives

// old
#[derive(WorldQuery)]
#[world_query(mutable, derive(Debug))]
struct CustomQuery {
    entity: Entity,
    a: &'static mut ComponentA
}

#[derive(WorldQuery)]
struct QueryFilter {
    _c: With<ComponentC>
}

// new 
#[derive(WorldQueryData)]
#[world_query_data(mutable, derive(Debug))]
struct CustomQuery {
    entity: Entity,
    a: &'static mut ComponentA,
}

#[derive(WorldQueryFilter)]
struct QueryFilter {
    _c: With<ComponentC>
}

Replace Option<With<T>> with Has<T>

/// old
fn my_system(query: Query<(Entity, Option<With<ComponentA>>)>)
{
  for (entity, has_a_option) in query.iter(){
    let has_a:bool = has_a_option.is_some();
    //todo!()
  }
}

/// new
fn my_system(query: Query<(Entity, Has<ComponentA>)>)
{
  for (entity, has_a) in query.iter(){
    //todo!()
  }
}

Fix queries which had filters in the data position or vice versa.

// old
fn my_system(query: Query<(Entity, With<ComponentA>)>)
{
  for (entity, _) in query.iter(){
  //todo!()
  }
}

// new
fn my_system(query: Query<Entity, With<ComponentA>>)
{
  for entity in query.iter(){
  //todo!()
  }
}

The traits `WorldQueryData : WorldQuery` and `WorldQueryFilter : WorldQuery` have been added and some of the types and functions from `WorldQuery` has been moved into them. `ReadOnlyWorldQuery` has been replaced with `ReadOnlyWorldQueryData`. `WorldQueryFilter` is safe (as long as `WorldQuery` is implemented safely). `WorldQueryData` is unsafe - safely implementing it requires that `Self::ReadOnly` is a readonly version of `Self` (this used to be a safety requirement of `WorldQuery`. The type parameters `Q` and `F` of `Query` must now implement `WorldQueryData` and `WorldQueryFilter` respectively. This makes it impossible to accidentally use a filter in the data position or vice versa which was something that could lead to bugs. Compile failure tests have been added to check this. It was previously sometimes useful to use `Option<With<T>>` in the data position. Use `Has<T>` instead in these cases. The derive macro has been split into separate derive macros for `WorldQueryData` and `WorldQueryFilter`. Previously it was possible to derive both `WorldQuery` for a struct that had a mixture of data and filter items. This would not work correctly in some cases but could be a useful pattern in others. This is no longer possible.

crates/bevy_ecs/src/query/fetch.rs

alice-i-cecile · 2023-06-20T14:45:32Z

crates/bevy_ecs/src/query/filter.rs

-                }
-            };
+/// SAFETY: Updates component access correctly
+unsafe impl<T: Component> WorldQuery for Added<T> {


Why isn't this a WorldQueryFilter impl?

alice-i-cecile · 2023-06-20T14:48:49Z

Neat, this is impressive work. I like how much clearer (and error-resistant) the end user facing code becomes.

We should consider splitting out the Has<T> changes into a seperate PR and merging that ASAP: that's clearly a better way to write this pattern in general.

I was suspicious of the fact that WorldQuery itself still hangs around, but after investigating more the shared methods make a lot of sense and I think this is a sensible structure.

wainwrightmark · 2023-06-20T17:28:20Z

Just to explain the safety comments. The old WorldQuery has two requirements

/// # Safety
///
/// Component access of `Self::ReadOnly` must be a subset of `Self`
/// and `Self::ReadOnly` must match exactly the same archetypes/tables as `Self`
///
/// Implementor must ensure that
/// [`update_component_access`] and [`update_archetype_component_access`]
/// exactly reflects the results of the following methods:
///
/// - [`matches_component_set`]
/// - [`fetch`]

The first requirement has moved to WorkdQueryData, the second requirement remains on WorldQuery. The old safety comments generally only addressed the first requirement so I've moved them onto the WorldQueryData implementations. There don't seem to be any existing comments about the other safety requirements so I wrote "Updates component access correctly" which is not particularly detailed but I didn't feel super comfortable going into more detail as it's not an area I touched - I just kept the existing implementations. So if anyone has suggestions for what to put for these, that would be appreciated.

# Objective - Keep hashbrown dependency up to date ## Solution - Bump hashbrown to version `0.14` This supersedes bevyengine#8823

…ed (bevyengine#8903) # Objective Fixes bevyengine#8765 ## Solution When windows are created during plugin setup, the scale_factor of a WindowResolution struct will always be 1.0 (default). The correct scale factor is set later in flow. To get correct center calculations use the monitors scale factor directly instead. ## Results System: Windows 10 Pro (125% scaling) ### main ![scale_125_without_fix](https://github.com/bevyengine/bevy/assets/644930/df808013-adc9-4300-8930-08ac87cc62b8) ### This PR ![scale_125_with_fix](https://github.com/bevyengine/bevy/assets/644930/c3d73606-d9e3-4f65-b4cc-2a1c20dbb64d)

# Objective - Providing a "noob-friendly" example since not many people are proficient in 3D modeling / rendering concepts. ## Solution - Adding more information to the example, with an explanation. ~~~~ _Thanks to Nocta on discord for helping out when I didn't understand the subject well._ --------- Co-authored-by: François <[email protected]>

For those who wish to be able to `#[reflect]` stuff using the `Uuid` type I'm very unfamiliar with the codebase, so please tell me if I'm missing something

…tems (bevyengine#8907) # Objective The "bevy_text" feature attributes for the `PrimaryWindow`, `Window` and `TextureAtlas` imports in `bevy_ui::render` are used by non-text systems (`extract_uinode_borders` and `extract_atlas_uinodes`) and need to be removed.

# Objective - Use `AppTypeRegistry` on API defined in `bevy_ecs` (bevyengine#8895 (comment)) A lot of the API on `Reflect` depends on a registry. When it comes to the ECS. We should use `AppTypeRegistry` in the general case. This is however impossible in `bevy_ecs`, since `AppTypeRegistry` is defined in `bevy_app`. ## Solution - Move `AppTypeRegistry` resource definition from `bevy_app` to `bevy_ecs` - Still add the resource in the `App` plugin, since bevy_ecs itself doesn't know of plugins Note that `bevy_ecs` is a dependency of `bevy_app`, so nothing revolutionary happens. ## Alternative - Define the API as a trait in `bevy_app` over `bevy_ecs`. (though this prevents us from using bevy_ecs internals) - Do not rely on `AppTypeRegistry` for the API in question, requring users to extract themselves the resource and pass it to the API methods. --- ## Changelog - Moved `AppTypeRegistry` resource definition from `bevy_app` to `bevy_ecs` ## Migration Guide - If you were **not** using a `prelude::*` to import `AppTypeRegistry`, you should update your imports: ```diff - use bevy::app::AppTypeRegistry; + use bevy::ecs::reflect::AppTypeRegistry ```

# Objective - Fix broken normals when the NormalPrepass is enabled ## Solution - Don't use the normal prepass for the world_normal - Only loadthe normal prepass - when msaa is disabled - for opaque or alpha mask meshes and only for use it for N not world_normal

…in` (bevyengine#8097) # Objective - Better consistency with `add_systems`. - Deprecating `add_plugin` in favor of a more powerful `add_plugins`. - Allow passing `Plugin` to `add_plugins`. - Allow passing tuples to `add_plugins`. ## Solution - `App::add_plugins` now takes an `impl Plugins` parameter. - `App::add_plugin` is deprecated. - `Plugins` is a new sealed trait that is only implemented for `Plugin`, `PluginGroup` and tuples over `Plugins`. - All examples, benchmarks and tests are changed to use `add_plugins`, using tuples where appropriate. --- ## Changelog ### Changed - `App::add_plugins` now accepts all types that implement `Plugins`, which is implemented for: - Types that implement `Plugin`. - Types that implement `PluginGroup`. - Tuples (up to 16 elements) over types that implement `Plugins`. - Deprecated `App::add_plugin` in favor of `App::add_plugins`. ## Migration Guide - Replace `app.add_plugin(plugin)` calls with `app.add_plugins(plugin)`. --------- Co-authored-by: Carter Anderson <[email protected]>

# Objective - Fixes bevyengine#8645 ## Solution Cascaded shadow maps use a technique commonly called shadow pancaking to enhance shadow map resolution by restricting the orthographic projection used in creating the shadow maps to the frustum slice for the cascade. The implication of this restriction is that shadow casters can be closer than the near plane of the projection volume. Prior to this PR, we address clamp the depth of the prepass vertex output to ensure that these shadow casters do not get clipped, resulting in shadow loss. However, a flaw / bug of the prior approach is that the depth that gets written to the shadow map isn't quite correct - the depth was previously derived by interpolated the clamped clip position, resulting in depths that are further than they should be. This creates artifacts that are particularly noticeable when a very 'long' object intersects the near plane close to perpendicularly. The fix in this PR is to propagate the unclamped depth to the prepass fragment shader and use that depth value directly. A complementary solution would be to use [DEPTH_CLIP_CONTROL](https://docs.rs/wgpu/latest/wgpu/struct.Features.html#associatedconstant.DEPTH_CLIP_CONTROL) to request `unclipped_depth`. However due to the relatively low support of the feature on Vulkan (I believe it's ~38%), I went with this solution for now to get the broadest fix out first. --- ## Changelog - Fixed: Shadows from directional lights were sometimes incorrectly omitted when the shadow caster was partially out of view. --------- Co-authored-by: Carter Anderson <[email protected]>

# Objective `prepare_uinodes` creates a new `UiBatch` whenever the texture changes, when most often it's just queuing untextured quads. Instead of switching textures, we can reduce the number of batches generated significantly by adding a condition to the fragment shader so that it only multiplies by the `textureSample` value when drawing a textured quad. # Solution Add a `mode` field to `UiVertex`. In `prepare_uinodes` set `mode` to 0 if the quad is textured or 1 if untextured. Add a condition to the fragment shader that only multiplies by the `color` value from `textureSample` if `mode` is set to 1. --- ## Changelog * Added a `mode` field to `UiVertex`, and added an extra `u32` vertex attribute to the shader and vertex buffer layout. * In `prepare_uinodes` mode is set to 0 for the vertices of textured quads, and 1 if untextured. * Added a condition to the fragment shader in `ui.wgsl` that only multiplies by the `color` value from `textureSample` if the mode is equal to 0.

…evyengine#8868) # Objective - Fix the AsBindGroup texture attribute visibility flag parsing - This appears to have been caused by a syn crate update which then the visibility code got updated - Also I noticed that by default the vertex and fragment flags were on, so visibility(compute) would actually make the texture visible to vertex, fragment and compute shaders, I fixed this too ## Solution - Update flag parsing to use MetaList.parse_nested_meta function, which loads the flags into a Vec then loop through those flags - Change initial visibility flags to use VisibilityFlags::default() rather than VisibilityFlags::vertex_fragment()

# Objective - Fixes bevyengine#8918. - The app should not crash if only the `bevy_text` feature is enabled. ## Solution The `bevy_text` feature now depends on `bevy_asset` and `bevy_sprite`, because it uses resources from these crates.

# Objective Improve the documentation relating to windows, and update the parts that have not been updated since version 0.8. Version 0.9 introduced `Window` as a component, before that `WindowDescriptor` (which would become `Window` later) was used to store information about how a window will be created. Since version 0.9, from my understanding, this information will also be synchronised with the current state of the window, and can be used to modify this state. However, some of the documentation has not been updated to reflect that, here is an example: https://docs.rs/bevy/0.8.0/bevy/window/enum.WindowMode.html / https://docs.rs/bevy/latest/bevy/window/enum.WindowMode.html (notice that the verb "Creates" is still there). This PR aims at improving the documentation relating to windows. ## Solution - Change "will" for "should" when relevant, "should" implies that the information should in both direction (from the window state to the `Window` component and vice-versa) and can be used to get and set, will implies it is only used to set a state. - Remove references to "creation" or be more clear about it. - Reference back the `Window` component for most of its sub-structs. - Clarify what needs to be clarified - A lot of other minor changes, including fixing the link to W3schools in `bevy_winit` ## Warning Please note that my knowledge about how winit and bevy_winit work is limited and some of the informations I added in the doc may be inaccurate. A person who knows better how it works should review some of my claims, in particular: - How fullscreen works: bevyengine#8858 (comment) - How WindowResolution / sizes work: bevyengine#8858 (comment) - What happens when `WindowPosition` is set to `Centered` or `Automatic`. From my understanding of the code, it should always be set back to `At`, but is it really the case? For example [when creating the window](https://github.com/bevyengine/bevy/blob/main/crates/bevy_winit/src/winit_windows.rs#L74), or when [a `WindowEvent::Moved` is triggered](https://github.com/bevyengine/bevy/blob/main/crates/bevy_winit/src/lib.rs#L602) or when [Centered/Automatic by the code after the window is created](https://github.com/bevyengine/bevy/blob/main/crates/bevy_winit/src/system.rs#L243), am I missing some cases and do the codes I linked do that in all of them? - Are there any field in the `Window` component that can't be used to modify the state of the window, only at creation? --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Jerome Humbert <[email protected]>

# Objective Fix bevyengine#8908. ## Solution Assign the vertex buffers twice with a single item offset instead of setting the array_stride lower than the vertex layout's size for linestrips.

# Objective - Add morph targets to `bevy_pbr` (closes bevyengine#5756) & load them from glTF - Supersedes bevyengine#3722 - Fixes bevyengine#6814 [Morph targets][1] (also known as shape interpolation, shape keys, or blend shapes) allow animating individual vertices with fine grained controls. This is typically used for facial expressions. By specifying multiple poses as vertex offset, and providing a set of weight of each pose, it is possible to define surprisingly realistic transitions between poses. Blending between multiple poses also allow composition. Morph targets are part of the [gltf standard][2] and are a feature of Unity and Unreal, and babylone.js, it is only natural to implement them in bevy. ## Solution This implementation of morph targets uses a 3d texture where each pixel is a component of an animated attribute. Each layer is a different target. We use a 2d texture for each target, because the number of attribute×components×animated vertices is expected to always exceed the maximum pixel row size limit of webGL2. It copies fairly closely the way skinning is implemented on the CPU side, while on the GPU side, the shader morph target implementation is a relatively trivial detail. We add an optional `morph_texture` to the `Mesh` struct. The `morph_texture` is built through a method that accepts an iterator over attribute buffers. The `MorphWeights` component, user-accessible, controls the blend of poses used by mesh instances (so that multiple copy of the same mesh may have different weights), all the weights are uploaded to a uniform buffer of 256 `f32`. We limit to 16 poses per mesh, and a total of 256 poses. More literature: * Old babylone.js implementation (vertex attribute-based): https://www.eternalcoding.com/dev-log-1-morph-targets/ * Babylone.js implementation (similar to ours): https://www.youtube.com/watch?v=LBPRmGgU0PE * GPU gems 3: https://developer.nvidia.com/gpugems/gpugems3/part-i-geometry/chapter-3-directx-10-blend-shapes-breaking-limits * Development discord thread https://discord.com/channels/691052431525675048/1083325980615114772 https://user-images.githubusercontent.com/26321040/231181046-3bca2ab2-d4d9-472e-8098-639f1871ce2e.mp4 https://github.com/bevyengine/bevy/assets/26321040/d2a0c544-0ef8-45cf-9f99-8c3792f5a258 ## Acknowledgements * Thanks to `storytold` for sponsoring the feature * Thanks to `superdump` and `james7132` for guidance and help figuring out stuff ## Future work - Handling of less and more attributes (eg: animated uv, animated arbitrary attributes) - Dynamic pose allocation (so that zero-weighted poses aren't uploaded to GPU for example, enables much more total poses) - Better animation API, see bevyengine#8357 ---- ## Changelog - Add morph targets to bevy meshes - Support up to 64 poses per mesh of individually up to 116508 vertices, animation currently strictly limited to the position, normal and tangent attributes. - Load a morph target using `Mesh::set_morph_targets` - Add `VisitMorphTargets` and `VisitMorphAttributes` traits to `bevy_render`, this allows defining morph targets (a fairly complex and nested data structure) through iterators (ie: single copy instead of passing around buffers), see documentation of those traits for details - Add `MorphWeights` component exported by `bevy_render` - `MorphWeights` control mesh's morph target weights, blending between various poses defined as morph targets. - `MorphWeights` are directly inherited by direct children (single level of hierarchy) of an entity. This allows controlling several mesh primitives through a unique entity _as per GLTF spec_. - Add `MorphTargetNames` component, naming each indices of loaded morph targets. - Load morph targets weights and buffers in `bevy_gltf` - handle morph targets animations in `bevy_animation` (previously, it was a `warn!` log) - Add the `MorphStressTest.gltf` asset for morph targets testing, taken from the glTF samples repo, CC0. - Add morph target manipulation to `scene_viewer` - Separate the animation code in `scene_viewer` from the rest of the code, reducing `#[cfg(feature)]` noise - Add the `morph_targets.rs` example to show off how to manipulate morph targets, loading `MorpStressTest.gltf` ## Migration Guide - (very specialized, unlikely to be touched by 3rd parties) - `MeshPipeline` now has a single `mesh_layouts` field rather than separate `mesh_layout` and `skinned_mesh_layout` fields. You should handle all possible mesh bind group layouts in your implementation - You should also handle properly the new `MORPH_TARGETS` shader def and mesh pipeline key. A new function is exposed to make this easier: `setup_moprh_and_skinning_defs` - The `MeshBindGroup` is now `MeshBindGroups`, cached bind groups are now accessed through the `get` method. [1]: https://en.wikipedia.org/wiki/Morph_target_animation [2]: https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#morph-targets --------- Co-authored-by: François <[email protected]> Co-authored-by: Carter Anderson <[email protected]>

# Objective - Closes bevyengine#7323 - Reduce texture blurriness for TAA ## Solution - Add a `MipBias` component and view uniform. - Switch material `textureSample()` calls to `textureSampleBias()`. - Add a `-1.0` bias to TAA. --- ## Changelog - Added `MipBias` camera component, mostly for internal use. --------- Co-authored-by: François <[email protected]> Co-authored-by: Carter Anderson <[email protected]>

# Objective Partially address bevyengine#5504. Fix bevyengine#4278. Provide "whole entity" access in queries. This can be useful when you don't know at compile time what you're accessing (i.e. reflection via `ReflectComponent`). ## Solution Implement `WorldQuery` for `EntityRef`. - This provides read-only access to the entire entity, and supports anything that `EntityRef` can normally do. - It matches all archetypes and tables and will densely iterate when possible. - It marks all of the ArchetypeComponentIds of a matched archetype as read. - Adding it to a query will cause it to panic if used in conjunction with any other mutable access. - Expanded the docs on Query to advertise this feature. - Added tests to ensure the panics were working as intended. - Added `EntityRef` to the ECS prelude. To make this safe, `EntityRef::world` was removed as it gave potential `UnsafeCell`-like access to other parts of the `World` including aliased mutable access to the components it would otherwise read safely. ## Performance Not great beyond the additional parallelization opportunity over exclusive systems. The `EntityRef` is fetched from `Entities` like any other call to `World::entity`, which can be very random access heavy. This could be simplified if `ArchetypeRow` is available in `WorldQuery::fetch`'s arguments, but that's likely not something we should optimize for. ## Future work An equivalent API where it gives mutable access to all components on a entity can be done with a scoped version of `EntityMut` where it does not provide `&mut World` access nor allow for structural changes to the entity is feasible as well. This could be done as a safe alternative to exclusive system when structural mutation isn't required or the target set of entities is scoped. --- ## Changelog Added: `Access::has_any_write` Added: `EntityRef` now implements `WorldQuery`. Allows read-only access to the entire entity, incompatible with any other mutable access, can be mixed with `With`/`Without` filters for more targeted use. Added: `EntityRef` to `bevy::ecs::prelude`. Removed: `EntityRef::world` ## Migration Guide TODO --------- Co-authored-by: Carter Weinberg <[email protected]> Co-authored-by: Jakob Hellermann <[email protected]> Co-authored-by: Carter Anderson <[email protected]>

# Objective - Change despawn descendants to return self (bevyengine#8883). ## Solution - Change function signature `despawn_descendants` under trait `DespawnRecursiveExt`. - Add single extra test `spawn_children_after_despawn_descendants` (May be unnecessary) --------- Co-authored-by: Alice Cecile <[email protected]>

# Objective In Bevy main, the unconstrained size of an `ImageBundle` or `AtlasImageBundle` UI node is based solely on the size of its texture and doesn't change with window scale factor or `UiScale`. ## Solution * The size field of each `ImageMeasure` should be multiplied by the current combined scale factor. * Each `ImageMeasure` should be updated when the combined scale factor is changed. ## Example: ```rust use bevy::prelude::*; fn main() { App::new() .add_plugins(DefaultPlugins) .insert_resource(UiScale { scale: 1.5 }) .add_systems(Startup, setup) .run(); } fn setup(mut commands: Commands, asset_server: Res<AssetServer>) { commands.spawn(Camera2dBundle::default()); commands.spawn(NodeBundle { style: Style { // The size of the "bevy_logo_dark.png" texture is 520x130 pixels width: Val::Px(520.), height: Val::Px(130.), ..Default::default() }, background_color: Color::RED.into(), ..Default::default() }); commands .spawn(ImageBundle { style: Style { position_type: PositionType::Absolute, ..Default::default() }, image: UiImage::new(asset_server.load("bevy_logo_dark.png")), ..Default::default() }); } ``` The red node is given a size with the same dimensions as the texture. So we would expect the texture to fill the node exactly. * Result with Bevy main branch bb59509: <img width="400" alt="image-size-broke" src="https://github.com/bevyengine/bevy/assets/27962798/19fd927d-ecc5-49a7-be05-c121a8df163f"> * Result with this PR (and Bevy 0.10.1): <img width="400" alt="image-size-fixed" src="https://github.com/bevyengine/bevy/assets/27962798/40b47820-5f2d-408f-88ef-9e2beb9c92a0"> --- ## Changelog `bevy_ui::widget::image` * Update all `ImageMeasure`s on changes to the window scale factor or `UiScale`. * Multiply `ImageMeasure::size` by the window scale factor and `UiScale`. ## Migration Guide

# Objective `any_component_removed` condition is inversed. ## Solution Remove extra `!`. --- ## Changelog ### Fixed Fix `any_component_removed` condition.

…h (generate_custom_mesh) solve bevyengine#4922 (bevyengine#8909) # Objective - Fixes bevyengine#4922 ## Solution - Add an example that maps a custom texture on a 3D mesh. --- ## Changelog > Added the texture itself (confirmed with mod on discord before it should be ok) to the assets folder, added to the README and Cargo.toml. --------- Co-authored-by: Nicola Papale <[email protected]> Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Sélène Amanita <[email protected]>

`Style` flattened `size`, `min_size` and `max_size` to its root struct, causing compilation errors. I uncommented the code to avoid further silent error not caught by CI, but hid the view to keep the same behaviour.

…ute (bevyengine#8933) # Objective - Fix this error to be able to run UI examples in WebGPU ``` 1 error(s) generated while compiling the shader: :31:18 error: integral user-defined vertex outputs must have a flat interpolation attribute @location(3) mode: u32, ^^^^ :36:1 note: while analyzing entry point 'vertex' fn vertex( ^^ ``` It was introduce in bevyengine#8793 ## Solution - Add `@interpolate(flat)` to the `mode` field

Some code could be improved. ## Solution Improve the code

# Objective In Bevy 10.1 and before, the only way to enable text wrapping was to set a local `Val::Px` width constraint on the text node itself. `Val::Percent` constraints and constraints on the text node's ancestors did nothing. bevyengine#7779 fixed those problems. But perversely displaying unwrapped text is really difficult now, and requires users to nest each `TextBundle` in a `NodeBundle` and apply `min_width` and `max_width` constraints. Some constructions may even need more than one layer of nesting. I've seen several people already who have really struggled with this when porting their projects to main in advance of 0.11. ## Solution Add a `NoWrap` variant to the `BreakLineOn` enum. If `NoWrap` is set, ignore any constraints on the width for the text and call `TextPipeline::queue_text` with a width bound of `f32::INFINITY`. --- ## Changelog * Added a `NoWrap` variant to the `BreakLineOn` enum. * If `NoWrap` is set, any constraints on the width for the text are ignored and `TextPipeline::queue_text` is called with a width bound of `f32::INFINITY`. * Changed the `size` field of `FixedMeasure` to `pub`. This shouldn't have been private, it was always intended to have `pub` visibility. * Added a `with_no_wrap` method to `TextBundle`. ## Migration Guide `bevy_text::text::BreakLineOn` has a new variant `NoWrap` that disables text wrapping for the `Text`. Text wrapping can also be disabled using the `with_no_wrap` method of `TextBundle`.

# Objective `color_from_entity` uses the poor man's hash to get a fixed random color for an entity. While the poor man's hash is succinct, it has a tendency to clump. As a result, bevy_gizmos has a tendency to re-use very similar colors for different entities. This is bad, we would want non-similar colors that take the whole range of possible hues. This way, each bevy_gizmos aabb gizmo is easy to identify. ## Solution AHash is a nice and fast hash that just so happen to be available to use, so we use it.

…bevyengine#8951) # Objective `World::entity`, `World::entity_mut` and `Commands::entity` should be marked with `track_caller` to display where (in user code) the call with the invalid `Entity` was made. `Commands::entity` already has the attibute, but it does nothing due to the call to `unwrap_or_else`. ## Solution - Apply the `track_caller` attribute to the `World::entity_mut` and `World::entity`. - Remove the call to `unwrap_or_else` which makes the `track_caller` attribute useless (because `unwrap_or_else` is not `track_caller` itself). The avoid eager evaluation of the panicking branch it is never inlined. --------- Co-authored-by: Giacomo Stevanato <[email protected]>

# Objective - Labels are not correctly placed <img width="1392" alt="Screenshot 2023-04-22 at 00 12 54" src="https://user-images.githubusercontent.com/8672791/233742996-0189b3c2-ea6b-4f3f-b2e8-68fdbf74f52f.png"> ## Solution - Set a width in the UI so that text doesn't try to wrap <img width="1392" alt="Screenshot 2023-04-22 at 00 13 04" src="https://user-images.githubusercontent.com/8672791/233743064-8d6045e5-3936-4c22-be07-ac618399c093.png">

bevyengine#7112) # Objective Currently when `UntypedReflectDeserializerVisitor` deserializes a `Box<dyn Reflect>` it only considers the first entry of the map, silently ignoring any additional entries. For example the following RON data: ```json { "f32": 1.23, "u32": 1, } ``` is successfully deserialized as a `f32`, completly ignoring the `"u32": 1` part. ## Solution `UntypedReflectDeserializerVisitor` was changed to check if any other key could be deserialized, and in that case returns an error. --- ## Changelog `UntypedReflectDeserializer` now errors on malformed inputs instead of silently disgarding additional data. ## Migration Guide If you were deserializing `Box<dyn Reflect>` values with multiple entries (i.e. entries other than `"type": { /* fields */ }`) you should remove them or deserialization will fail.

# Objective operate on naga IR directly to improve handling of shader modules. - give codespan reporting into imported modules - allow glsl to be used from wgsl and vice-versa the ultimate objective is to make it possible to - provide user hooks for core shader functions (to modify light behaviour within the standard pbr pipeline, for example) - make automatic binding slot allocation possible but ... since this is already big, adds some value and (i think) is at feature parity with the existing code, i wanted to push this now. ## Solution i made a crate called naga_oil (https://github.com/robtfm/naga_oil - unpublished for now, could be part of bevy) which manages modules by - building each module independantly to naga IR - creating "header" files for each supported language, which are used to build dependent modules/shaders - make final shaders by combining the shader IR with the IR for imported modules then integrated this into bevy, replacing some of the existing shader processing stuff. also reworked examples to reflect this. ## Migration Guide shaders that don't use `#import` directives should work without changes. the most notable user-facing difference is that imported functions/variables/etc need to be qualified at point of use, and there's no "leakage" of visible stuff into your shader scope from the imports of your imports, so if you used things imported by your imports, you now need to import them directly and qualify them. the current strategy of including/'spreading' `mesh_vertex_output` directly into a struct doesn't work any more, so these need to be modified as per the examples (e.g. color_material.wgsl, or many others). mesh data is assumed to be in bindgroup 2 by default, if mesh data is bound into bindgroup 1 instead then the shader def `MESH_BINDGROUP_1` needs to be added to the pipeline shader_defs.

The traits `WorldQueryData : WorldQuery` and `WorldQueryFilter : WorldQuery` have been added and some of the types and functions from `WorldQuery` has been moved into them. `ReadOnlyWorldQuery` has been replaced with `ReadOnlyWorldQueryData`. `WorldQueryFilter` is safe (as long as `WorldQuery` is implemented safely). `WorldQueryData` is unsafe - safely implementing it requires that `Self::ReadOnly` is a readonly version of `Self` (this used to be a safety requirement of `WorldQuery`. The type parameters `Q` and `F` of `Query` must now implement `WorldQueryData` and `WorldQueryFilter` respectively. This makes it impossible to accidentally use a filter in the data position or vice versa which was something that could lead to bugs. Compile failure tests have been added to check this. It was previously sometimes useful to use `Option<With<T>>` in the data position. Use `Has<T>` instead in these cases. The derive macro has been split into separate derive macros for `WorldQueryData` and `WorldQueryFilter`. Previously it was possible to derive both `WorldQuery` for a struct that had a mixture of data and filter items. This would not work correctly in some cases but could be a useful pattern in others. This is no longer possible.

…b.com/wainwrightmark/bevy into split-worldquery-into-data-and-filter

wainwrightmark · 2023-06-27T08:39:57Z

I spent some time trying to understand the safety requirements of WorldQuery. I've codified them and updated all the safety comments on the implementations.

The general rules for a safe implementation are:

Implementor must ensure that
update_component_access, update_archetype_component_access, matches_component_set, and fetch
obey the following:

For each component mutably accessed by fetch, update_component_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 fetch, update_component_access should add read access unless write access has already been added, in which case it should panic.

For each component mutably accessed by fetch, update_archetype_component_access should add write access if that component belongs to the archetype.

For each component readonly accessed by fetch, update_archetype_component_access should add read access if that component belongs to the archetype.

If fetch mutably accesses a component and also accesses the same component again, update_component_access should panic.

update_component_access may not add a With filter for a component unless matches_component_set always returns false if the component set doesn't contain that component.

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 replace the filters with a disjunction where every member of the disjunction is a conjunction of the previous filters and the filters of a soundly implemented WorldQuery so long as matches_component_set is a disjunction of the implementations in those WorldQuerys.

And as an example, the safety comment for &mut T is:

fetch accesses a single component mutably.
This is sound because update_component_access and update_archetype_component_access add write access for that component and panic 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.

As far as I can tell these rules are correct and all the existing implementations follow them but I may have missed something. There is also probably scope to make them more succinct (especially the last one).

Co-authored-by: Alice Cecile <[email protected]>

Trashtalk217 · 2023-09-20T12:23:34Z

What is the status of this PR?

wainwrightmark · 2023-09-20T17:52:37Z

What is the status of this PR?

I think I was fairly happy with this last time I pushed but I imagine there have been some significant changes (e.g. new query types added) since then. I'm happy to rebase and get it back up to date if people want.

Trashtalk217 · 2023-09-20T18:10:39Z

Sounds great, I'd be willing to give a review.

Trashtalk217 · 2023-09-22T09:50:20Z

I've briefly looked through all the changes and I'll be honest, it's a bit of a mess. It looks like something went wrong with rebasing, since from the 7k code changes, only ~700 lines have anything to do with queries.

Can you clean up this PR? In all honesty, if I were you I'd close this PR, start a new one and lift over the relevant changes. But you may be better at git-fu than I am.

Also, if you don't have time to work on this PR, let me know. I could potentially adopt this PR if you want.

wainwrightmark · 2023-09-22T10:16:58Z

I've briefly looked through all the changes and I'll be honest, it's a bit of a mess. It looks like something went wrong with rebasing, since from the 7k code changes, only ~700 lines have anything to do with queries.

Can you clean up this PR? In all honesty, if I were you I'd close this PR, start a new one and lift over the relevant changes. But you may be better at git-fu than I am.

Also, if you don't have time to work on this PR, let me know. I could potentially adopt this PR if you want.

Yes, something definitely has gone wrong somewhere (possibly I merged instead of rebased at some point?).
I will do as you suggest and open a new PR.

If you did want to look at the old changes, the page here is much more helpful main...wainwrightmark:bevy:split-worldquery-into-data-and-filter#diff-b02ae2ab7ec1b2e91fc2f00da4d9483eea1f8f13336c8ec5445b507d608c67a8

Trashtalk217 · 2023-09-22T13:22:16Z

Thank you, let me know when the new PR is up.

wainwrightmark · 2023-09-24T21:35:18Z

Closing in favour of #9918

# Objective - Fixes #7680 - This is an updated for #8899 which had the same objective but fell a long way behind the latest changes ## Solution The traits `WorldQueryData : WorldQuery` and `WorldQueryFilter : WorldQuery` have been added and some of the types and functions from `WorldQuery` has been moved into them. `ReadOnlyWorldQuery` has been replaced with `ReadOnlyWorldQueryData`. `WorldQueryFilter` is safe (as long as `WorldQuery` is implemented safely). `WorldQueryData` is unsafe - safely implementing it requires that `Self::ReadOnly` is a readonly version of `Self` (this used to be a safety requirement of `WorldQuery`) The type parameters `Q` and `F` of `Query` must now implement `WorldQueryData` and `WorldQueryFilter` respectively. This makes it impossible to accidentally use a filter in the data position or vice versa which was something that could lead to bugs. ~~Compile failure tests have been added to check this.~~ It was previously sometimes useful to use `Option<With<T>>` in the data position. Use `Has<T>` instead in these cases. The `WorldQuery` derive macro has been split into separate derive macros for `WorldQueryData` and `WorldQueryFilter`. Previously it was possible to derive both `WorldQuery` for a struct that had a mixture of data and filter items. This would not work correctly in some cases but could be a useful pattern in others. *This is no longer possible.* --- ## Notes - The changes outside of `bevy_ecs` are all changing type parameters to the new types, updating the macro use, or replacing `Option<With<T>>` with `Has<T>`. - All `WorldQueryData` types always returned `true` for `IS_ARCHETYPAL` so I moved it to `WorldQueryFilter` and replaced all calls to it with `true`. That should be the only logic change outside of the macro generation code. - `Changed<T>` and `Added<T>` were being generated by a macro that I have expanded. Happy to revert that if desired. - The two derive macros share some functions for implementing `WorldQuery` but the tidiest way I could find to implement them was to give them a ton of arguments and ask clippy to ignore that. ## Changelog ### Changed - Split `WorldQuery` into `WorldQueryData` and `WorldQueryFilter` which now have separate derive macros. It is not possible to derive both for the same type. - `Query` now requires that the first type argument implements `WorldQueryData` and the second implements `WorldQueryFilter` ## Migration Guide - Update derives ```rust // old #[derive(WorldQuery)] #[world_query(mutable, derive(Debug))] struct CustomQuery { entity: Entity, a: &'static mut ComponentA } #[derive(WorldQuery)] struct QueryFilter { _c: With<ComponentC> } // new #[derive(WorldQueryData)] #[world_query_data(mutable, derive(Debug))] struct CustomQuery { entity: Entity, a: &'static mut ComponentA, } #[derive(WorldQueryFilter)] struct QueryFilter { _c: With<ComponentC> } ``` - Replace `Option<With<T>>` with `Has<T>` ```rust /// old fn my_system(query: Query<(Entity, Option<With<ComponentA>>)>) { for (entity, has_a_option) in query.iter(){ let has_a:bool = has_a_option.is_some(); //todo!() } } /// new fn my_system(query: Query<(Entity, Has<ComponentA>)>) { for (entity, has_a) in query.iter(){ //todo!() } } ``` - Fix queries which had filters in the data position or vice versa. ```rust // old fn my_system(query: Query<(Entity, With<ComponentA>)>) { for (entity, _) in query.iter(){ //todo!() } } // new fn my_system(query: Query<Entity, With<ComponentA>>) { for entity in query.iter(){ //todo!() } } // old fn my_system(query: Query<AnyOf<(&ComponentA, With<ComponentB>)>>) { for (entity, _) in query.iter(){ //todo!() } } // new fn my_system(query: Query<Option<&ComponentA>, Or<(With<ComponentA>, With<ComponentB>)>>) { for entity in query.iter(){ //todo!() } } ``` --------- Co-authored-by: Alice Cecile <[email protected]>

Selene-Amanita added A-ECS Entities, components, systems, and events C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Complex Quite challenging from either a design or technical perspective. Ask for help! labels Jun 20, 2023

removed unneeded compile fail tests

4dd9ea1

alice-i-cecile added this to the 0.12 milestone Jun 20, 2023