forked from bevyengine/bevy
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[pull] main from bevyengine:main #6
Open
pull
wants to merge
4,466
commits into
octoape:main
Choose a base branch
from
bevyengine:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Objective - Fix issue identified on the [Discord server](https://discord.com/channels/691052431525675048/691052431974465548/1328922812530036839) ## Solution - Implement `Clone` for `QueryIter` using the existing `QueryIter::remaining` method ## Testing - CI --- ## Showcase Users can now explicitly clone a read-only `QueryIter`: ```rust fn combinations(query: Query<&ComponentA>) { let mut iter = query.iter(); while let Some(a) = iter.next() { // Can now clone rather than use remaining for b in iter.clone() { // Check every combination (a, b) } } } ``` ## Notes This doesn't add any new functionality outside the context of generic code (e.g., `T: Iterator<...> + Clone`), it's mostly for discoverability. Users are more likely to be familiar with `Clone::clone` than they are with the methods on `QueryIter`.
# Objective While `add_looping_edges` is a helpful method for manually defining directional navigation maps, we don't always want to loop around! ## Solution Add a non-looping variant. These commits are cherrypicked from the more complex #17247. ## Testing I've updated the `directional_navigation` example to use these changes, and verified that it works. --------- Co-authored-by: Rob Parrett <[email protected]> Co-authored-by: Benjamin Brienen <[email protected]>
# Objective The safety documentation for `Ptr::assert_unique` is incomplete. Currently it only mentions the existence of other `Ptr` instances, but it should also mention that the underlying data must be mutable and that there cannot be active references to it.
# Objective While working on more complex directional navigation work, I noticed a few small things. ## Solution Rather than stick them in a bigger PR, split them out now. - Include more useful information when responding to `DirectionalNavigationError`. - Use the less controversial `Click` events (rather than `Pressed`) in the example - Implement add_looping_edges in terms of `add_edges`. Thanks @rparrett for the idea. ## Testing Ran the `directional_navigation` example and things still work.
# Objective Occasionally bevy users will want to store systems or observer systems in a component or resource, but they first try to store `IntoSystem` instead of `System`, which leads to some headaches having to deal with the `M` marker type parameter. We should recommend they use the `X` trait instead of the `IntoX` trait in that case, as well for returning from a function. ## Solution Add usage notes to the `IntoX` traits about using `X` instead.
This allows this: ```rust #[derive(Component)] #[require(StateScoped<MyState>(StateScoped(MyState)))] struct ComponentA; ``` To be shortened to this: ```rust #[derive(Component)] #[require(StateScoped<MyState>)] struct ComponentA; ``` When `MyState` implements `Default`. Signed-off-by: Jean Mertz <[email protected]>
Added an external assets section to .gitignore. This prevents contributors from accidentally adding or committing them. I believe currently the only externel asset is the meshlet bunny.
This adds support for one-to-many non-fragmenting relationships (with planned paths for fragmenting and non-fragmenting many-to-many relationships). "Non-fragmenting" means that entities with the same relationship type, but different relationship targets, are not forced into separate tables (which would cause "table fragmentation"). Functionally, this fills a similar niche as the current Parent/Children system. The biggest differences are: 1. Relationships have simpler internals and significantly improved performance and UX. Commands and specialized APIs are no longer necessary to keep everything in sync. Just spawn entities with the relationship components you want and everything "just works". 2. Relationships are generalized. Bevy can provide additional built in relationships, and users can define their own. **REQUEST TO REVIEWERS**: _please don't leave top level comments and instead comment on specific lines of code. That way we can take advantage of threaded discussions. Also dont leave comments simply pointing out CI failures as I can read those just fine._ ## Built on top of what we have Relationships are implemented on top of the Bevy ECS features we already have: components, immutability, and hooks. This makes them immediately compatible with all of our existing (and future) APIs for querying, spawning, removing, scenes, reflection, etc. The fewer specialized APIs we need to build, maintain, and teach, the better. ## Why focus on one-to-many non-fragmenting first? 1. This allows us to improve Parent/Children relationships immediately, in a way that is reasonably uncontroversial. Switching our hierarchy to fragmenting relationships would have significant performance implications. ~~Flecs is heavily considering a switch to non-fragmenting relations after careful considerations of the performance tradeoffs.~~ _(Correction from @SanderMertens: Flecs is implementing non-fragmenting storage specialized for asset hierarchies, where asset hierarchies are many instances of small trees that have a well defined structure)_ 2. Adding generalized one-to-many relationships is currently a priority for the [Next Generation Scene / UI effort](#14437). Specifically, we're interested in building reactions and observers on top. ## The changes This PR does the following: 1. Adds a generic one-to-many Relationship system 3. Ports the existing Parent/Children system to Relationships, which now lives in `bevy_ecs::hierarchy`. The old `bevy_hierarchy` crate has been removed. 4. Adds on_despawn component hooks 5. Relationships can opt-in to "despawn descendants" behavior, meaning that the entire relationship hierarchy is despawned when `entity.despawn()` is called. The built in Parent/Children hierarchies enable this behavior, and `entity.despawn_recursive()` has been removed. 6. `world.spawn` now applies commands after spawning. This ensures that relationship bookkeeping happens immediately and removes the need to manually flush. This is in line with the equivalent behaviors recently added to the other APIs (ex: insert). 7. Removes the ValidParentCheckPlugin (system-driven / poll based) in favor of a `validate_parent_has_component` hook. ## Using Relationships The `Relationship` trait looks like this: ```rust pub trait Relationship: Component + Sized { type RelationshipSources: RelationshipSources<Relationship = Self>; fn get(&self) -> Entity; fn from(entity: Entity) -> Self; } ``` A relationship is a component that: 1. Is a simple wrapper over a "target" Entity. 2. Has a corresponding `RelationshipSources` component, which is a simple wrapper over a collection of entities. Every "target entity" targeted by a "source entity" with a `Relationship` has a `RelationshipSources` component, which contains every "source entity" that targets it. For example, the `Parent` component (as it currently exists in Bevy) is the `Relationship` component and the entity containing the Parent is the "source entity". The entity _inside_ the `Parent(Entity)` component is the "target entity". And that target entity has a `Children` component (which implements `RelationshipSources`). In practice, the Parent/Children relationship looks like this: ```rust #[derive(Relationship)] #[relationship(relationship_sources = Children)] pub struct Parent(pub Entity); #[derive(RelationshipSources)] #[relationship_sources(relationship = Parent)] pub struct Children(Vec<Entity>); ``` The Relationship and RelationshipSources derives automatically implement Component with the relevant configuration (namely, the hooks necessary to keep everything in sync). The most direct way to add relationships is to spawn entities with relationship components: ```rust let a = world.spawn_empty().id(); let b = world.spawn(Parent(a)).id(); assert_eq!(world.entity(a).get::<Children>().unwrap(), &[b]); ``` There are also convenience APIs for spawning more than one entity with the same relationship: ```rust world.spawn_empty().with_related::<Children>(|s| { s.spawn_empty(); s.spawn_empty(); }) ``` The existing `with_children` API is now a simpler wrapper over `with_related`. This makes this change largely non-breaking for existing spawn patterns. ```rust world.spawn_empty().with_children(|s| { s.spawn_empty(); s.spawn_empty(); }) ``` There are also other relationship APIs, such as `add_related` and `despawn_related`. ## Automatic recursive despawn via the new on_despawn hook `RelationshipSources` can opt-in to "despawn descendants" behavior, which will despawn all related entities in the relationship hierarchy: ```rust #[derive(RelationshipSources)] #[relationship_sources(relationship = Parent, despawn_descendants)] pub struct Children(Vec<Entity>); ``` This means that `entity.despawn_recursive()` is no longer required. Instead, just use `entity.despawn()` and the relevant related entities will also be despawned. To despawn an entity _without_ despawning its parent/child descendants, you should remove the `Children` component first, which will also remove the related `Parent` components: ```rust entity .remove::<Children>() .despawn() ``` This builds on the on_despawn hook introduced in this PR, which is fired when an entity is despawned (before other hooks). ## Relationships are the source of truth `Relationship` is the _single_ source of truth component. `RelationshipSources` is merely a reflection of what all the `Relationship` components say. By embracing this, we are able to significantly improve the performance of the system as a whole. We can rely on component lifecycles to protect us against duplicates, rather than needing to scan at runtime to ensure entities don't already exist (which results in quadratic runtime). A single source of truth gives us constant-time inserts. This does mean that we cannot directly spawn populated `Children` components (or directly add or remove entities from those components). I personally think this is a worthwhile tradeoff, both because it makes the performance much better _and_ because it means theres exactly one way to do things (which is a philosophy we try to employ for Bevy APIs). As an aside: treating both sides of the relationship as "equivalent source of truth relations" does enable building simple and flexible many-to-many relationships. But this introduces an _inherent_ need to scan (or hash) to protect against duplicates. [`evergreen_relations`](https://github.com/EvergreenNest/evergreen_relations) has a very nice implementation of the "symmetrical many-to-many" approach. Unfortunately I think the performance issues inherent to that approach make it a poor choice for Bevy's default relationship system. ## Followup Work * Discuss renaming `Parent` to `ChildOf`. I refrained from doing that in this PR to keep the diff reasonable, but I'm personally biased toward this change (and using that naming pattern generally for relationships). * [Improved spawning ergonomics](#16920) * Consider adding relationship observers/triggers for "relationship targets" whenever a source is added or removed. This would replace the current "hierarchy events" system, which is unused upstream but may have existing users downstream. I think triggers are the better fit for this than a buffered event queue, and would prefer not to add that back. * Fragmenting relations: My current idea hinges on the introduction of "value components" (aka: components whose type _and_ value determines their ComponentId, via something like Hashing / PartialEq). By labeling a Relationship component such as `ChildOf(Entity)` as a "value component", `ChildOf(e1)` and `ChildOf(e2)` would be considered "different components". This makes the transition between fragmenting and non-fragmenting a single flag, and everything else continues to work as expected. * Many-to-many support * Non-fragmenting: We can expand Relationship to be a list of entities instead of a single entity. I have largely already written the code for this. * Fragmenting: With the "value component" impl mentioned above, we get many-to-many support "for free", as it would allow inserting multiple copies of a Relationship component with different target entities. Fixes #3742 (If this PR is merged, I think we should open more targeted followup issues for the work above, with a fresh tracking issue free of the large amount of less-directed historical context) Fixes #17301 Fixes #12235 Fixes #15299 Fixes #15308 ## Migration Guide * Replace `ChildBuilder` with `ChildSpawnerCommands`. * Replace calls to `.set_parent(parent_id)` with `.insert(Parent(parent_id))`. * Replace calls to `.replace_children()` with `.remove::<Children>()` followed by `.add_children()`. Note that you'll need to manually despawn any children that are not carried over. * Replace calls to `.despawn_recursive()` with `.despawn()`. * Replace calls to `.despawn_descendants()` with `.despawn_related::<Children>()`. * If you have any calls to `.despawn()` which depend on the children being preserved, you'll need to remove the `Children` component first. --------- Co-authored-by: Alice Cecile <[email protected]>
# Objective Segment2d and Segment3d are currently hard to work with because unlike many other primary shapes, they are bound to the origin. The objective of this PR is to allow these segments to exist anywhere in cartesian space, making them much more useful in a variety of contexts. ## Solution Reworking the existing segment type's internal fields and methods to allow them to exist anywhere in cartesian space. I have done both reworks for 2d and 3d segments but I was unsure if I should just have it all here or not so feel free to tell me how I should proceed, for now I have only pushed Segment2d changes. As I am not a very seasoned contributor, this first implementation is very likely sloppy and will need some additional work from my end, I am open to all criticisms and willing to work to get this to bevy's standards. ## Testing I am not very familiar with the standards of testing. Of course my changes had to pass the thorough existing tests for primitive shapes. I also checked the gizmo 2d shapes intersection example and everything looked fine. I did add a few utility methods to the types that have no tests yet. I am willing to implement some if it is deemed necessary ## Migration Guide The segment type constructors changed so if someone previously created a Segment2d with a direction and length they would now need to use the `from_direction` constructor --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Joona Aalto <[email protected]>
# Objective It's not immediately obvious that `TargetCamera` only works with UI node entities. It's natural to assume from looking at something like the `multiple_windows` example that it will work with everything. ## Solution Rename `TargetCamera` to `UiTargetCamera`. ## Migration Guide `TargetCamera` has been renamed to `UiTargetCamera`.
…th (#17434) This commit overhauls the documentation in the Bevy scene loading example. It adds thorough explanatory comments to guide new Rust and Bevy developers. The rewritten docs clarify how to: - Register types for reflection, enabling serialization and dynamic property access - Skip serializing certain fields with `#[reflect(skip_serializing)]` - Use `FromWorld` for components that require runtime initialization - Store and serialize `Resources` in scene files - Load scenes using a `DynamicSceneRoot` and handle updates in a system - Serialize a brand-new scene to a separate file asynchronously using `IoTaskPool` These additions aim to provide a clear, step-by-step reference that demonstrates how to implement a scene-based workflow, making it easier for beginners and experienced developers alike to use Bevy’s scene system effectively. --------- Co-authored-by: Alice Cecile <[email protected]>
# Objective Update out of date comment. ## Solution ## Testing N/A
Fixes #17397. Also renamed all variants into present-tense. ## Migration Guide - `PointerAction::Pressed` has been seperated into two variants, `PointerAction::Press` and `PointerAction::Release`. - `PointerAction::Moved` has been renamed to `PointerAction::Move`. - `PointerAction::Canceled` has been renamed to `PointerAction::Cancel`.
Makes use of `std` explicit, simplifying a possible `no_std` port. # Objective - Contributes to #15460 - Simplify future `no_std` work on `bevy_asset` ## Solution - Add `#![no_std]` to switch to `core::prelude` instead of `std::prelude` ## Testing - CI --- ## Notes This is entirely a change around the names of imports and has no impact on functionality. This just reduces the quantity of changes involved in the (likely more controversial) `no_std`-ification of `bevy_asset`.
# Objective Fixes #17164 ## Solution - Switch to `ubuntu-latest` - Remove `libegl1-mesa`. It seems that whatever this was providing is now covered by other deps or their deps? ## Testing Tested [over here](rparrett#21) by commenting out workflow `if` condition.
# Objective - Contributes to #16877 ## Solution - Initial creation of `bevy_platform_support` crate. - Moved `bevy_utils::Instant` into new `bevy_platform_support` crate. - Moved `portable-atomic`, `portable-atomic-util`, and `critical-section` into new `bevy_platform_support` crate. ## Testing - CI --- ## Showcase Instead of needing code like this to import an `Arc`: ```rust #[cfg(feature = "portable-atomic")] use portable_atomic_util::Arc; #[cfg(not(feature = "portable-atomic"))] use alloc::sync::Arc; ``` We can now use: ```rust use bevy_platform_support::sync::Arc; ``` This applies to many other types, but the goal is overall the same: allowing crates to use `std`-like types without the boilerplate of conditional compilation and platform-dependencies. ## Migration Guide - Replace imports of `bevy_utils::Instant` with `bevy_platform_support::time::Instant` - Replace imports of `bevy::utils::Instant` with `bevy::platform_support::time::Instant` ## Notes - `bevy_platform_support` hasn't been reserved on `crates.io` - ~~`bevy_platform_support` is not re-exported from `bevy` at this time. It may be worthwhile exporting this crate, but I am unsure of a reasonable name to export it under (`platform_support` may be a bit wordy for user-facing).~~ - I've included an implementation of `Instant` which is suitable for `no_std` platforms that are not Wasm for the sake of eliminating feature gates around its use. It may be a controversial inclusion, so I'm happy to remove it if required. - There are many other items (`spin`, `bevy_utils::Sync(Unsafe)Cell`, etc.) which should be added to this crate. I have kept the initial scope small to demonstrate utility without making this too unwieldy. --------- Co-authored-by: TimJentzsch <[email protected]> Co-authored-by: Chris Russell <[email protected]> Co-authored-by: François Mockers <[email protected]>
simple derive macro for `FromWorld`. Going to be needed for composable pipeline specializers but probably a nice thing to have regardless ## Testing simple manual testing, nothing seemed to blow up. I'm no proc macro pro though, so there's a chance I've mishandled spans somewhere or something.
# Objective `Text2d` ignores `TextBounds` when calculating the offset for text aligment. On main a text entity positioned in the center of the window with center justification and 600px horizontal text bounds isn't centered like it should be but shifted off to the right: <img width="305" alt="hellox" src="https://github.com/user-attachments/assets/8896c6f0-1b9f-4633-9c12-1de6eff5f3e1" /> (second example in the testing section below) Fixes #14266 I already had a PR in review for this (#14270) but it used post layout adjustment (which we want to avoid) and ignored `TextBounds`. ## Solution * If `TextBounds` are present for an axis, use them instead of the size of the computed text layout size to calculate the offset. * Adjust the vertical offset of text so it's top is aligned with the top of the texts bounding rect (when present). ## Testing ``` use bevy::prelude::*; use bevy::color::palettes; use bevy::sprite::Anchor; use bevy::text::TextBounds; fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, setup) .run(); } fn example(commands: &mut Commands, dest: Vec3, justify: JustifyText) { commands.spawn(( Sprite { color: palettes::css::YELLOW.into(), custom_size: Some(10. * Vec2::ONE), anchor: Anchor::Center, ..Default::default() }, Transform::from_translation(dest), )); for a in [ Anchor::TopLeft, Anchor::TopRight, Anchor::BottomRight, Anchor::BottomLeft, ] { commands.spawn(( Text2d(format!("L R\n{:?}\n{:?}", a, justify)), TextFont { font_size: 14.0, ..default() }, TextLayout { justify, ..Default::default() }, TextBounds::new(300., 75.), Transform::from_translation(dest + Vec3::Z), a, )); } } fn setup(mut commands: Commands) { commands.spawn(Camera2d::default()); for (i, j) in [ JustifyText::Left, JustifyText::Right, JustifyText::Center, JustifyText::Justified, ] .into_iter() .enumerate() { example(&mut commands, (300. - 150. * i as f32) * Vec3::Y, j); } commands.spawn(Sprite { color: palettes::css::YELLOW.into(), custom_size: Some(10. * Vec2::ONE), anchor: Anchor::Center, ..Default::default() }); } ``` <img width="566" alt="cap" src="https://github.com/user-attachments/assets/e6a98fa5-80b2-4380-a9b7-155bb49635b8" /> This probably looks really confusing but it should make sense if you imagine each block of text surrounded by a 300x75 rectangle that is anchored to the center of the yellow square. # ``` use bevy::prelude::*; use bevy::sprite::Anchor; use bevy::text::TextBounds; fn main() { App::new() .add_plugins(DefaultPlugins) .add_systems(Startup, setup) .run(); } fn setup(mut commands: Commands) { commands.spawn(Camera2d::default()); commands.spawn(( Text2d::new("hello"), TextFont { font_size: 60.0, ..default() }, TextLayout::new_with_justify(JustifyText::Center), TextBounds::new(600., 200.), Anchor::Center, )); } ``` <img width="338" alt="hello" src="https://github.com/user-attachments/assets/e5e89364-afda-4baa-aca8-df4cdacbb4ed" /> The text being above the center is intended. When `TextBounds` are present, the text block's offset is calculated using its `TextBounds` not the layout size returned by cosmic-text. # Probably we should add a vertical alignment setting for Text2d. Didn't do it here as this is intended for a 0.15.2 release.
# Objective Make the `animated_mesh` example more intuitive and easier for the user to extend. # Solution The `animated_mesh` example shows how to spawn a single mesh and play a single animation. The original code is roughly: 1. In `setup_mesh_and_animation`, spawn an entity with a SceneRoot that will load and spawn the mesh. Also record the animation to play as a resource. 2. Use `play_animation_once_loaded` to detect when any animation players are spawned, then play the animation from the resource. When I used this example as a starting point for my own app, I hit a wall when trying to spawn multiple meshes with different animations. `play_animation_once_loaded` tells me an animation player spawned somewhere, but how do I get from there to the right animation? The entity it runs on is spawned by the scene so I can't attach any data to it? The new code takes a different approach. Instead of a global resource, the animation is recorded as a component on the entity with the SceneRoot. Instead of detecting animation players spawning wherever, an observer is attached to that specific entity. This feels more intuitive and localised, and I think most users will work out how to get from there to different animations and meshes. The downside is more lines of code, and the "find the animation players" part still feels a bit magical and inefficient. # Side Notes - The solution was mostly stolen from #14852 (comment). - The example still feels too complicated. - "Why do I have to make this graph to play one animation?" - "Why can't I choose and play the animation in one step and avoid this temporary component?" - I think this requires engine changes. - I originally started on a separate example of multiple meshes ([branch](main...greeble-dev:bevy:animated-mesh-multiple)). - I decided that the user could probably work this out themselves from the single animation example. - But maybe still worth following through. # Testing `cargo run --example animated_mesh` --------- Co-authored-by: Rob Parrett <[email protected]>
# Objective The gltf-json crate seems like it strips/adds an `_` when doing the name comparison for custom vertex attributes. * gltf-json [add](https://github.com/gltf-rs/gltf/blob/88e719d5de9268318aa922e38fd54f4844bea18c/gltf-json/src/mesh.rs#L341) * gltf-json [strip](https://github.com/gltf-rs/gltf/blob/88e719d5de9268318aa922e38fd54f4844bea18c/gltf-json/src/mesh.rs#L298C12-L298C42) * [bevy's handling](https://github.com/bevyengine/bevy/blob/b66c3ceb0ee39374ff1759ffb1b5bee2e4b93e99/crates/bevy_gltf/src/vertex_attributes.rs#L273-L276) seems like it uses the non-underscore'd version. The bevy example gltf: [barycentric.gltf](https://github.com/bevyengine/bevy/blob/b66c3ceb0ee39374ff1759ffb1b5bee2e4b93e99/assets/models/barycentric/barycentric.gltf), includes two underscores: `__BARYCENTRIC` in the gltf file, resulting in needing `_BARYCENTRIC` (one underscore) as the attribute name in Bevy. This extra underscore is redundant and does not appear if exporting from blender, which only requires a single underscore to trigger the attribute export. I'm not sure if we want to change the example itself (maybe there's a reason it has two underscores, I couldn't find a reason), but a docs comment would help. ## Solution add docs detailing the behavior
# Objective The existing `RelationshipSourceCollection` uses `Vec` as the only possible backing for our relationships. While a reasonable choice, benchmarking use cases might reveal that a different data type is better or faster. For example: - Not all relationships require a stable ordering between the relationship sources (i.e. children). In cases where we a) have many such relations and b) don't care about the ordering between them, a hash set is likely a better datastructure than a `Vec`. - The number of children-like entities may be small on average, and a `smallvec` may be faster ## Solution - Implement `RelationshipSourceCollection` for `EntityHashSet`, our custom entity-optimized `HashSet`. -~~Implement `DoubleEndedIterator` for `EntityHashSet` to make things compile.~~ - This implementation was cursed and very surprising. - Instead, by moving the iterator type on `RelationshipSourceCollection` from an erased RPTIT to an explicit associated type we can add a trait bound on the offending methods! - Implement `RelationshipSourceCollection` for `SmallVec` ## Testing I've added a pair of new tests to make sure this pattern compiles successfully in practice! ## Migration Guide `EntityHashSet` and `EntityHashMap` are no longer re-exported in `bevy_ecs::entity` directly. If you were not using `bevy_ecs` / `bevy`'s `prelude`, you can access them through their now-public modules, `hash_set` and `hash_map` instead. ## Notes to reviewers The `EntityHashSet::Iter` type needs to be public for this impl to be allowed. I initially renamed it to something that wasn't ambiguous and re-exported it, but as @Victoronz pointed out, that was somewhat unidiomatic. In 1a85648, I instead made the `entity_hash_set` public (and its `entity_hash_set`) sister public, and removed the re-export. I prefer this design (give me module docs please), but it leads to a lot of churn in this PR. Let me know which you'd prefer, and if you'd like me to split that change out into its own micro PR.
Some primitives seems to have their export being missed, we can see in the documentation that types are not followable: : [CuboidMeshBuilder](https://docs.rs/bevy/latest/bevy/math/prelude/struct.Cuboid.html#impl-Meshable-for-Cuboid) is not exposed. This Pr addresses this, + its surrounding modules which were around.
# Objective Fixes #14970 ## Solution It seems the clamp call in `ui.wgsl` had the parameters order incorrect. ## Testing Tested using examples/ui in native and my current project in wasm - both in linux. Could use some help with testing in other platforms. ---
…17370) docs: enhance documentation in `query.rs` to clarify borrowing rules. Please, let me know if you don't agree with the wording.. There is always room for improvement. Tested locally and it looks like this:  --------- Co-authored-by: Alice Cecile <[email protected]>
# Objective Diagnostics for labels don't suggest how to best implement them. ``` error[E0277]: the trait bound `Label: ScheduleLabel` is not satisfied --> src/main.rs:15:35 | 15 | let mut sched = Schedule::new(Label); | ------------- ^^^^^ the trait `ScheduleLabel` is not implemented for `Label` | | | required by a bound introduced by this call | = help: the trait `ScheduleLabel` is implemented for `Interned<(dyn ScheduleLabel + 'static)>` note: required by a bound in `bevy_ecs::schedule::Schedule::new` --> /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/schedule/schedule.rs:297:28 | 297 | pub fn new(label: impl ScheduleLabel) -> Self { | ^^^^^^^^^^^^^ required by this bound in `Schedule::new` ``` ## Solution `diagnostics::on_unimplemented` and `diagnostics::do_not_recommend` ## Showcase New error message: ``` error[E0277]: the trait bound `Label: ScheduleLabel` is not satisfied --> src/main.rs:15:35 | 15 | let mut sched = Schedule::new(Label); | ------------- ^^^^^ the trait `ScheduleLabel` is not implemented for `Label` | | | required by a bound introduced by this call | = note: consider annotating `Label` with `#[derive(ScheduleLabel)]` note: required by a bound in `bevy_ecs::schedule::Schedule::new` --> /home/vj/workspace/rust/bevy/crates/bevy_ecs/src/schedule/schedule.rs:297:28 | 297 | pub fn new(label: impl ScheduleLabel) -> Self { | ^^^^^^^^^^^^^ required by this bound in `Schedule::new` ```
# Objective Some usecases in the ecosystems are blocked by the inability to stop bevy internals and third party plugins from touching their entities. However the specifics of a general purpose entity disabling system are controversial and further complicated by hierarchies. We can partially unblock these usecases with an opt-in approach: default query filters. ## Solution - Introduce DefaultQueryFilters, these filters are automatically applied to queries that don't otherwise mention the filtered component. - End users and third party plugins can register default filters and are responsible for handling entities they have hidden this way. - Extra features can be left for after user feedback - The default value could later include official ways to hide entities --- ## Changelog - Add DefaultQueryFilters
# Objective Fixes #17416 ## Solution I just included ReflectFromReflect in all macros and implementations. I think this should be ok, at least it compiles properly and does fix the errors in my test code. ## Testing I generated a DynamicMap and tried to convert it into a concrete `HashMap` as a `Box<dyn Reflect>`. Without my fix, it doesn't work, because this line panics: ```rust let rfr = ty.data::<ReflectFromReflect>().unwrap(); ``` where `ty` is the `TypeRegistration` for the (matching) `HashMap`. I don't know why `ReflectFromReflect` wasn't included everywhere, I assume that it was an oversight and not an architecture decision I'm not aware of. # Migration Guide The hasher in reflected `HashMap`s and `HashSet`s now have to implement `Default`. This is the case for the ones provided by Bevy already, and is generally a sensible thing to do.
# Objective Update typos, fix new typos. 1.29.6 was just released to fix an [issue](crate-ci/typos#1228) where January's corrections were not included in the binaries for the last release. Reminder: typos can be tossed in the monthly [non-critical corrections issue](crate-ci/typos#1221). ## Solution I chose to allow `implementors`, because a good argument seems to be being made [here](crate-ci/typos#1226) and there is now a PR to address that. ## Discussion Should I exclude `bevy_mikktspace`? At one point I think we had an informal policy of "don't mess with mikktspace until #9050 is merged" but it doesn't seem like that is likely to be merged any time soon. I think these particular corrections in mikktspace are fine because - The same typo mistake seems to have been fixed in that PR - The entire file containing these corrections was deleted in that PR ## Typo of the Month correspindong -> corresponding
# Objective Add `NodeImageMode` to `bevy_ui::prelude`.
# Objective While surveying the state of documentation for bevy_assets, I noticed a few minor issues. ## Solution Revise the docs to focus on clear explanations of core ideas and cross-linking related objects.
# Objective Continuation of #16547. We do not yet have parallel versions of `par_iter_many` and `par_iter_many_unique`. It is currently very painful to try and use parallel iteration over entity lists. Even if a list is not long, each operation might still be very expensive, and worth parallelizing. Plus, it has been requested several times! ## Solution Once again, we implement what we lack! These parallel iterators collect their input entity list into a `Vec`/`UniqueEntityVec`, then chunk that over the available threads, inspired by the original `par_iter`. Since no order guarantee is given to the caller, we could sort the input list according to `EntityLocation`, but that would likely only be worth it for very large entity lists. There is some duplication which could likely be improved, but I'd like to leave that for a follow-up. ## Testing The doc tests on `for_each_init` of `QueryParManyIter` and `QueryParManyUniqueIter`.
… in `PreprocessWorkItem`. (#17853) The `output_index` field is only used in direct mode, and the `indirect_parameters_index` field is only used in indirect mode. Consequently, we can combine them into a single field, reducing the size of `PreprocessWorkItem`, which `batch_and_prepare_{binned,sorted}_render_phase` must construct every frame for every mesh instance, from 96 bits to 64 bits.
# Objective Fixes #17851 ## Solution Align the `slider` uniform to 16 bytes by making it a `vec4`. ## Testing Run the example using: ``` cargo run -p build-wasm-example -- --api webgl2 ui_material basic-http-server examples/wasm/ ```
# Objective `bevy_assets` has long been unapproachable for contributors and users. More and better documentation would help that. We're gradually moving towards globally denying missing docs (#3492)! However, writing all of the hundreds of missing doc strings in a single go will be miserable to review. ## Solution Remove the allow for missing docs temporarily, and then pick some easy missing doc warnings largely at random to tackle. Stop when the change set is starting to feel intimidating.
…17849) There was nonsense code in `batch_and_prepare_sorted_render_phase` that created temporary buffers to add objects to instead of using the correct ones. I think this was debug code. This commit removes that code in favor of writing to the actual buffers. Closes #17846. Co-authored-by: Alice Cecile <[email protected]>
# Objective It is impossible to register a type with `TypeRegistry::register` if the type is unnameable (in the current scope). ## Solution Add `TypeRegistry::register_by_val` which mirrors std's `size_of_val` and friends. ## Testing There's a doc test (unrelated but there seem to be some pre-existing broken doc links in `bevy_reflect`).
# Objective Add reference to reported position space in picking backend docs. Fixes #17844 ## Solution Add explanatory docs to the implementation notes of each picking backend. ## Testing `cargo r -p ci -- doc-check` & `cargo r -p ci -- lints`
# Objective - Fix the tracy debugger working with the latest version of bevy. - Broken experience for users of the latest version of the tracy profiler.  ## Solution - Bump the dependencies of tracy and recompile , works well with the latest tracy version. - Used the matrix available at https://github.com/nagisa/rust_tracy_client ## Testing - Tested changes using the tracy GUI client and running a few examples with `--features "trace_tracy"` --- ## Showcase Screenshot shows tracy profiler v0.11.1 <img width="1651" alt="Screenshot 2025-02-14 at 6 57 10 PM" src="https://github.com/user-attachments/assets/835575a6-db44-4abb-ac68-18a23e937c19" />
# Objective Support accessing resources using reflection when using `FilteredResources` in a dynamic system. This is similar to how components can be queried using reflection when using `FilteredEntityRef|Mut`. ## Solution Change `ReflectResource` from taking `&World` and `&mut World` to taking `impl Into<FilteredResources>` and `impl Into<FilteredResourcesMut>`, similar to how `ReflectComponent` takes `impl Into<FilteredEntityRef>` and `impl Into<FilteredEntityMut>`. There are `From` impls that ensure code passing `&World` and `&mut World` continues to work as before. ## Migration Guide If you are manually creating a `ReflectComponentFns` struct, the `reflect` function now takes `FilteredResources` instead `&World`, and there is a new `reflect_mut` function that takes `FilteredResourcesMut`.
…#17822) # Objective Simplify the API surface by removing duplicated functionality between `Query` and `QueryState`. Reduce the amount of `unsafe` code required in `QueryState`. This is a follow-up to #15858. ## Solution Move implementations of `Query` methods from `QueryState` to `Query`. Instead of the original methods being on `QueryState`, with `Query` methods calling them by passing the individual parameters, the original methods are now on `Query`, with `QueryState` methods calling them by constructing a `Query`. This also adds two `_inner` methods that were missed in #15858: `iter_many_unique_inner` and `single_inner`. One goal here is to be able to deprecate and eventually remove many of the methods on `QueryState`, reducing the overall API surface. (I expected to do that in this PR, but this change was large enough on its own!) Now that the `QueryState` methods each consist of a simple expression like `self.query(world).get_inner(entity)`, a future PR can deprecate some or all of them with simple migration instructions. The other goal is to reduce the amount of `unsafe` code. The current implementation of a read-only method like `QueryState::get` directly calls the `unsafe fn get_unchecked_manual` and needs to repeat the proof that `&World` has enough access. With this change, `QueryState::get` is entirely safe code, with the proof that `&World` has enough access done by the `query()` method and shared across all read-only operations. ## Future Work The next step will be to mark the `QueryState` methods as `#[deprecated]` and migrate callers to the methods on `Query`.
…#17841) The `collect_buffers_for_phase` system tries to reuse these buffers, but its efforts are stymied by the fact that `clear_batched_gpu_instance_buffers` clears the containing hash table and therefore frees the buffers. This patch makes `clear_batched_gpu_instance_buffers` stop doing that so that the allocations can be reused.
…WorkItem>`. (#17862) Appending to these vectors is performance-critical in `batch_and_prepare_binned_render_phase`, so `RawBufferVec`, which doesn't have the overhead of `encase`, is more appropriate.
# Objective Fix panic in `custom_render_phase`. This example was broken by #17764, but that breakage evolved into a panic after #17849. This new panic seems to illustrate the problem in a pretty straightforward way. ``` 2025-02-15T00:44:11.833622Z INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "macOS 15.3 Sequoia", kernel: "24.3.0", cpu: "Apple M4 Max", core_count: "16", memory: "64.0 GiB" } 2025-02-15T00:44:11.908328Z INFO bevy_render::renderer: AdapterInfo { name: "Apple M4 Max", vendor: 0, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Metal } 2025-02-15T00:44:12.314930Z INFO bevy_winit::system: Creating new window App (0v1) thread 'Compute Task Pool (1)' panicked at /Users/me/src/bevy/crates/bevy_ecs/src/system/function_system.rs:216:28: bevy_render::batching::gpu_preprocessing::batch_and_prepare_sorted_render_phase<custom_render_phase::Stencil3d, custom_render_phase::StencilPipeline> could not access system parameter ResMut<PhaseBatchedInstanceBuffers<Stencil3d, MeshUniform>> ``` ## Solution Add a `SortedRenderPhasePlugin` for the custom phase. ## Testing `cargo run --example custom_render_phase`
… do some easy Resultification (#17855) ## Objective There's no general error for when an entity doesn't exist, and some methods are going to need one when they get Resultified. The closest thing is `EntityFetchError`, but that error has a slightly more specific purpose. ## Solution - Added `EntityDoesNotExistError`. - Contains `Entity` and `EntityDoesNotExistDetails`. - Changed `EntityFetchError` and `QueryEntityError`: - Changed `NoSuchEntity` variant to wrap `EntityDoesNotExistError` and renamed the variant to `EntityDoesNotExist`. - Renamed `EntityFetchError` to `EntityMutableFetchError` to make its purpose clearer. - Renamed `TryDespawnError` to `EntityDespawnError` to make it more general. - Changed `World::inspect_entity` to return `Result<[ok], EntityDoesNotExistError>` instead of panicking. - Changed `World::get_entity` and `WorldEntityFetch::fetch_ref` to return `Result<[ok], EntityDoesNotExistError>` instead of `Result<[ok], Entity>`. - Changed `UnsafeWorldCell::get_entity` to return `Result<UnsafeEntityCell, EntityDoesNotExistError>` instead of `Option<UnsafeEntityCell>`. ## Migration Guide - `World::inspect_entity` now returns `Result<impl Iterator<Item = &ComponentInfo>, EntityDoesNotExistError>` instead of `impl Iterator<Item = &ComponentInfo>`. - `World::get_entity` now returns `EntityDoesNotExistError` as an error instead of `Entity`. You can still access the entity's ID through the error's `entity` field. - `UnsafeWorldCell::get_entity` now returns `Result<UnsafeEntityCell, EntityDoesNotExistError>` instead of `Option<UnsafeEntityCell>`.
# Context Renaming `Parent` to `ChildOf` in #17247 has been contentious. While those users concerns are valid (especially around legibility of code IMO!), @cart [has decided](https://discord.com/channels/691052431525675048/749335865876021248/1340434322833932430) to stick with the new name. > In general this conversation is unsurprising to me, as it played out essentially the same way when I asked for opinions in my PR. There are strong opinions on both sides. Everyone is right in their own way. > > I chose ChildOf for the following reasons: > > 1. I think it derives naturally from the system we have built, the concepts we have chosen, and how we generally name the types that implement a trait in Rust. This is the name of the type implementing Relationship. We are adding that Relationship component to a given entity (whether it "is" the relationship or "has" the relationship is kind of immaterial ... we are naming the relationship that it "is" or "has"). What is the name of the relationship that a child has to its parent? It is a "child" of the parent of course! > 2. In general the non-parent/child relationships I've seen in the wild generally benefit from (or need to) use the naming convention in (1) (aka calling the Relationship the name of the relationship the entity has). Many relationships don't have an equivalent to the Parent/Child name concept. > 3. I do think we could get away with using (1) for pretty much everything else and special casing Parent/Children. But by embracing the naming convention, we help establish that this is in fact a pattern, and we help prime people to think about these things in a consistent way. Consistency and predictability is a generally desirable property. And for something as divisive and polarizing as relationship naming, I think drawing a hard line in the sand is to the benefit of the community as a whole. > 4. I believe the fact that we dont see as much of the XOf naming style elsewhere is to our benefit. When people see things in that style, they are primed to think of them as relationships (after some exposure to Bevy and the ecosystem). I consider this a useful hint. > 5. Most of the practical confusion from using ChildOf seems to be from calling the value of the target field we read from the relationship child_of. The name of the target field should be parent (we could even consider renaming child_of.0 to child_of.parent for clarity). I suspect that existing Bevy users renaming their existing code will feel the most friction here, as this requires a reframing. Imo it is natural and expected to receive pushback from these users hitting this case. ## Objective The new documentation doesn't do a particularly good job at quickly explaining the meaning of each component or how to work with them; making a tricky migration more painful and slowing down new users as they learn about some of the most fundamental types in Bevy. ## Solution 1. Clearly explain what each component does in the very first line, assuming no background knowledge. This is the first relationships that 99% of users will encounter, so explaining that they are relationships is unhelpful as an introduction. 2. Add doc aliases for the rejected `IsParent`/`IsChild`/`Parent` names, to improve autocomplete and doc searching. 3. Do some assorted docs cleanup while we're here. --------- Co-authored-by: Eagster <[email protected]>
# Objective Fixes #17022 ## Solution Only enable `bevy_gltf/dds` if `bevy_gltf` is already enabled. ## Testing Tested with empty project ```toml [dependencies] bevy = { version = "0.16.0-dev", path = "../bevy", default-features = false, features = [ "dds", ] } ``` ### Before ``` cargo tree --depth 1 -i bevy_gltf bevy_gltf v0.16.0-dev (/Users/robparrett/src/bevy/crates/bevy_gltf) └── bevy_internal v0.16.0-dev (/Users/robparrett/src/bevy/crates/bevy_internal) ``` ### After ``` cargo tree --depth 1 -i bevy_gltf warning: nothing to print. To find dependencies that require specific target platforms, try to use option `--target all` first, and then narrow your search scope accordingly. ```
…res. (#17887) Fixes #17290. <details> <summary>Compilation errors before fix</summary> `cargo clippy --tests --all-features --package bevy_image`: ```rust error[E0061]: this function takes 7 arguments but 6 arguments were supplied --> crates/bevy_core_pipeline/src/tonemapping/mod.rs:451:5 | 451 | Image::from_buffer( | ^^^^^^^^^^^^^^^^^^ ... 454 | bytes, | ----- argument #1 of type `std::string::String` is missing | note: associated function defined here --> /Users/josiahnelson/Desktop/Programming/Rust/bevy/crates/bevy_image/src/image.rs:930:12 | 930 | pub fn from_buffer( | ^^^^^^^^^^^ help: provide the argument | 451 | Image::from_buffer(/* std::string::String */, bytes, image_type, CompressedImageFormats::NONE, false, image_sampler, RenderAssetUsages::RENDER_WORLD) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` `cargo clippy --tests --all-features --package bevy_gltf`: ```rust error[E0560]: struct `bevy_pbr::StandardMaterial` has no field named `specular_channel` --> crates/bevy_gltf/src/loader.rs:1343:13 | 1343 | specular_channel: specular.specular_channel, | ^^^^^^^^^^^^^^^^ `bevy_pbr::StandardMaterial` does not have this field | = note: available fields are: `emissive_exposure_weight`, `diffuse_transmission`, `diffuse_transmission_channel`, `diffuse_transmission_texture`, `flip_normal_map_y` ... and 9 others error[E0560]: struct `bevy_pbr::StandardMaterial` has no field named `specular_texture` --> crates/bevy_gltf/src/loader.rs:1345:13 | 1345 | specular_texture: specular.specular_texture, | ^^^^^^^^^^^^^^^^ `bevy_pbr::StandardMaterial` does not have this field | = note: available fields are: `emissive_exposure_weight`, `diffuse_transmission`, `diffuse_transmission_channel`, `diffuse_transmission_texture`, `flip_normal_map_y` ... and 9 others error[E0560]: struct `bevy_pbr::StandardMaterial` has no field named `specular_tint_channel` --> crates/bevy_gltf/src/loader.rs:1351:13 | 1351 | specular_tint_channel: specular.specular_color_channel, | ^^^^^^^^^^^^^^^^^^^^^ `bevy_pbr::StandardMaterial` does not have this field | = note: available fields are: `emissive_exposure_weight`, `diffuse_transmission`, `diffuse_transmission_channel`, `diffuse_transmission_texture`, `flip_normal_map_y` ... and 9 others error[E0560]: struct `bevy_pbr::StandardMaterial` has no field named `specular_tint_texture` --> crates/bevy_gltf/src/loader.rs:1353:13 | 1353 | specular_tint_texture: specular.specular_color_texture, | ^^^^^^^^^^^^^^^^^^^^^ `bevy_pbr::StandardMaterial` does not have this field | = note: available fields are: `emissive_exposure_weight`, `diffuse_transmission`, `diffuse_transmission_channel`, `diffuse_transmission_texture`, `flip_normal_map_y` ... and 9 others ``` </details>
Let's show Kenney some ❤️
# Objective - #17787 removed sweeping of binned render phases from 2D by accident due to them not using the `BinnedRenderPhasePlugin`. - Fixes #17885 ## Solution - Schedule `sweep_old_entities` in `QueueSweep` like `BinnedRenderPhasePlugin` does, but for 2D where that plugin is not used. ## Testing Tested with the modified `shader_defs` example in #17885 .
…-populated buffers. (#17863) The GPU can fill out many of the fields in `IndirectParametersMetadata` using information it already has: * `early_instance_count` and `late_instance_count` are always initialized to zero. * `mesh_index` is already present in the work item buffer as the `input_index` of the first work item in each batch. This patch moves these fields to a separate buffer, the *GPU indirect parameters metadata* buffer. That way, it avoids having to write them on CPU during `batch_and_prepare_binned_render_phase`. This effectively reduces the number of bits that that function must write per mesh from 160 to 64 (in addition to the 64 bits per mesh *instance*). Additionally, this PR refactors `UntypedPhaseIndirectParametersBuffers` to add another layer, `MeshClassIndirectParametersBuffers`, which allows abstracting over the buffers corresponding indexed and non-indexed meshes. This patch doesn't make much use of this abstraction, but forthcoming patches will, and it's overall a cleaner approach. This didn't seem to have much of an effect by itself on `batch_and_prepare_binned_render_phase` time, but subsequent PRs dependent on this PR yield roughly a 2× speedup.
Currently, Bevy rebuilds the buffer containing all the transforms for joints every frame, during the extraction phase. This is inefficient in cases in which many skins are present in the scene and their joints don't move, such as the Caldera test scene. To address this problem, this commit switches skin extraction to use a set of retained GPU buffers with allocations managed by the offset allocator. I use fine-grained change detection in order to determine which skins need updating. Note that the granularity is on the level of an entire skin, not individual joints. Using the change detection at that level would yield poor performance in common cases in which an entire skin is animated at once. Also, this patch yields additional performance from the fact that changing joint transforms no longer requires the skinned mesh to be re-extracted. Note that this optimization can be a double-edged sword. In `many_foxes`, fine-grained change detection regressed the performance of `extract_skins` by 3.4x. This is because every joint is updated every frame in that example, so change detection is pointless and is pure overhead. Because the `many_foxes` workload is actually representative of animated scenes, this patch includes a heuristic that disables fine-grained change detection if the number of transformed entities in the frame exceeds a certain fraction of the total number of joints. Currently, this threshold is set to 25%. Note that this is a crude heuristic, because it doesn't distinguish between the number of transformed *joints* and the number of transformed *entities*; however, it should be good enough to yield the optimum code path most of the time. Finally, this patch fixes a bug whereby skinned meshes are actually being incorrectly retained if the buffer offsets of the joints of those skinned meshes changes from frame to frame. To fix this without retaining skins, we would have to re-extract every skinned mesh every frame. Doing this was a significant regression on Caldera. With this PR, by contrast, mesh joints stay at the same buffer offset, so we don't have to update the `MeshInputUniform` containing the buffer offset every frame. This also makes PR #17717 easier to implement, because that PR uses the buffer offset from the previous frame, and the logic for calculating that is simplified if the previous frame's buffer offset is guaranteed to be identical to that of the current frame. On Caldera, this patch reduces the time spent in `extract_skins` from 1.79 ms to near zero. On `many_foxes`, this patch regresses the performance of `extract_skins` by approximately 10%-25%, depending on the number of foxes. This has only a small impact on frame rate.
Currently, the specialized pipeline cache maps a (view entity, mesh entity) tuple to the retained pipeline for that entity. This causes two problems: 1. Using the view entity is incorrect, because the view entity isn't stable from frame to frame. 2. Switching the view entity to a `RetainedViewEntity`, which is necessary for correctness, significantly regresses performance of `specialize_material_meshes` and `specialize_shadows` because of the loss of the fast `EntityHash`. This patch fixes both problems by switching to a *two-level* hash table. The outer level of the table maps each `RetainedViewEntity` to an inner table, which maps each `MainEntity` to its pipeline ID and change tick. Because we loop over views first and, within that loop, loop over entities visible from that view, we hoist the slow lookup of the view entity out of the inner entity loop. Additionally, this patch fixes a bug whereby pipeline IDs were leaked when removing the view. We still have a problem with leaking pipeline IDs for deleted entities, but that won't be fixed until the specialized pipeline cache is retained. This patch improves performance of the [Caldera benchmark] from 7.8× faster than 0.14 to 9.0× faster than 0.14, when applied on top of the global binding arrays PR, #17898. [Caldera benchmark]: https://github.com/DGriffin91/bevy_caldera_scene
PR #17688 broke motion vector computation, and therefore motion blur, because it enabled retention of `MeshInputUniform`s, and `MeshInputUniform`s contain the indices of the previous frame's transform and the previous frame's skinned mesh joint matrices. On frame N, if a `MeshInputUniform` is retained on GPU from the previous frame, the `previous_input_index` and `previous_skin_index` would refer to the indices for frame N - 2, not the index for frame N - 1. This patch fixes the problems. It solves these issues in two different ways, one for transforms and one for skins: 1. To fix transforms, this patch supplies the *frame index* to the shader as part of the view uniforms, and specifies which frame index each mesh's previous transform refers to. So, in the situation described above, the frame index would be N, the previous frame index would be N - 1, and the `previous_input_frame_number` would be N - 2. The shader can now detect this situation and infer that the mesh has been retained, and can therefore conclude that the mesh's transform hasn't changed. 2. To fix skins, this patch replaces the explicit `previous_skin_index` with an invariant that the index of the joints for the current frame and the index of the joints for the previous frame are the same. This means that the `MeshInputUniform` never has to be updated even if the skin is animated. The downside is that we have to copy joint matrices from the previous frame's buffer to the current frame's buffer in `extract_skins`. The rationale behind (2) is that we currently have no mechanism to detect when joints that affect a skin have been updated, short of comparing all the transforms and setting a flag for `extract_meshes_for_gpu_building` to consume, which would regress performance as we want `extract_skins` and `extract_meshes_for_gpu_building` to be able to run in parallel. To test this change, use `cargo run --example motion_blur`.
…ning systems run. (#17922) The `check_visibility` system currently follows this algorithm: 1. Store all meshes that were visible last frame in the `PreviousVisibleMeshes` set. 2. Determine which meshes are visible. For each such visible mesh, remove it from `PreviousVisibleMeshes`. 3. Mark all meshes that remain in `PreviousVisibleMeshes` as invisible. This algorithm would be correct if the `check_visibility` were the only system that marked meshes visible. However, it's not: the shadow-related systems `check_dir_light_mesh_visibility` and `check_point_light_mesh_visibility` can as well. This results in the following sequence of events for meshes that are in a shadow map but *not* visible from a camera: A. `check_visibility` runs, finds that no camera contains these meshes, and marks them hidden, which sets the changed flag. B. `check_dir_light_mesh_visibility` and/or `check_point_light_mesh_visibility` run, discover that these meshes are visible in the shadow map, and marks them as visible, again setting the `ViewVisibility` changed flag. C. During the extraction phase, the mesh extraction system sees that `ViewVisibility` is changed and re-extracts the mesh. This is inefficient and results in needless work during rendering. This patch fixes the issue in two ways: * The `check_dir_light_mesh_visibility` and `check_point_light_mesh_visibility` systems now remove meshes that they discover from `PreviousVisibleMeshes`. * Step (3) above has been moved from `check_visibility` to a separate system, `mark_newly_hidden_entities_invisible`. This system runs after all visibility-determining systems, ensuring that `PreviousVisibleMeshes` contains only those meshes that truly became invisible on this frame. This fix dramatically improves the performance of [the Caldera benchmark], when combined with several other patches I've submitted. [the Caldera benchmark]: https://github.com/DGriffin91/bevy_caldera_scene
This adds an option to animate the materials in the `many_cubes` stress test. Each material instance `base_color` is varied each frame. This has been tested in conjunction with the `--vary-material-data-per-instance` and `--material-texture-count` options. If `--vary-material-data-per-instance` is not used it will just update the single material, otherwise it will update all of them. If `--material-texture-count` is used the `base_color` is multiplied with the texture so the effect is still visible. Because this test is focused on the performance of updating material data and not the performance of bevy's color system it uses its own function (`fast_hue_to_rgb`) to quickly set the hue. This appeared to be around 8x faster than using `base_color.set_hue(hue)` in the tight loop.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )