We have an early-out to avoid updating `RenderVisibilityRanges` when a
`VisibilityRange` component is *modified*, but not when one is
*removed*. This means that removing `VisibilityRange` from an entity
might not update the rendering.
This PR fixes the issue by adding a check for removed
`VisibilityRange`s.
# Objective
- Fixes#16472.
## Solution
- Add flags to `SpritePlugin` and `UiPlugin` to disable their picking
backends.
## Testing
- The change is pretty trivial, so not much to test!
---
## Migration Guide
- `UiPlugin` now contains an extra `add_picking` field if
`bevy_ui_picking_backend` is enabled.
- `SpritePlugin` is no longer a unit struct, and has one field if
`bevy_sprite_picking_backend` is enabled (otherwise no fields).
# Objective
- Fixes#16469.
## Solution
- Make the picking backend features not enabled by default in each
sub-crate.
- Make features in `bevy_internal` to set the backend features
- Make the root `bevy` crate set the features by default.
## Testing
- The mesh and sprite picking examples still work correctly.
I'm not sure why, but somehow `#[derive(Reflect)]` on a tuple struct
with a boxed trait object can result in linker errors when dynamic
linking is used on Windows using `rust-lld`:
```
= note: rust-lld: error: <root>: undefined symbol: bevy_animation::_::_$LT$impl$u20$bevy_reflect..reflect..PartialReflect$u20$for$u20$bevy_animation..AnimationEventFn$GT$::reflect_partial_eq::hc4cce1dc55e42e0b␍
rust-lld: error: <root>: undefined symbol: bevy_animation::_::_$LT$impl$u20$bevy_reflect..from_reflect..FromReflect$u20$for$u20$bevy_animation..AnimationEventFn$GT$::from_reflect::hc2b1d575b8491092␍
rust-lld: error: <root>: undefined symbol: bevy_animation::_::_$LT$impl$u20$bevy_reflect..tuple_struct..TupleStruct$u20$for$u20$bevy_animation..AnimationEventFn$GT$::clone_dynamic::hab42a4edc8d6b5c2␍
rust-lld: error: <root>: undefined symbol: bevy_animation::_::_$LT$impl$u20$bevy_reflect..tuple_struct..TupleStruct$u20$for$u20$bevy_animation..AnimationEventFn$GT$::field::h729a3d6dd6a27a43␍
rust-lld: error: <root>: undefined symbol: bevy_animation::_::_$LT$impl$u20$bevy_reflect..tuple_struct..TupleStruct$u20$for$u20$bevy_animation..AnimationEventFn$GT$::field_mut::hde1c34846d77344b␍
rust-lld: error: <root>: undefined symbol: bevy_animation::_::_$LT$impl$u20$bevy_reflect..type_registry..GetTypeRegistration$u20$for$u20$bevy_animation..AnimationEventFn$GT$::get_type_registration::hb96eb543e403a132␍
rust-lld: error: <root>: undefined symbol: bevy_animation::_::_$LT$impl$u20$bevy_reflect..type_registry..GetTypeRegistration$u20$for$u20$bevy_animation..AnimationEventFn$GT$::register_type_dependencies::hcf1a4b69bcfea6ae␍
rust-lld: error: undefined symbol: bevy_animation::_::_$LT$impl$u20$bevy_reflect..reflect..PartialReflect$u20$for$u20$bevy_animation..AnimationEventFn$GT$::reflect_partial_eq::hc4cce1dc55e42e0b␍
```
etc.
Adding `#[reflect(opaque)]` to the `Reflect` derive fixes the problem,
and that's what this patch does. I think that adding
`#[reflect(opaque)]` is harmless, as there's little that reflection
allows with a boxed trait object anyhow.
# Objective
- Fixes#16406 even more. The previous implementation did not take into
account the depth of the requiree when setting the depth relative to the
required_by component.
## Solution
- Add the depth of the requiree!
## Testing
- Added a test.
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
We switch back and forwards between logical and physical coordinates all
over the place. Systems have to query for cameras and the UiScale when
they shouldn't need to. It's confusing and fragile and new scale factor
bugs get found constantly.
## Solution
* Use physical coordinates whereever possible in `bevy_ui`.
* Store physical coords in `ComputedNode` and tear out all the unneeded
scale factor calculations and queries.
* Add an `inverse_scale_factor` field to `ComputedNode` and set nodes
changed when their scale factor changes.
## Migration Guide
`ComputedNode`'s fields and methods now use physical coordinates.
`ComputedNode` has a new field `inverse_scale_factor`. Multiplying the
physical coordinates by the `inverse_scale_factor` will give the logical
values.
---------
Co-authored-by: atlv <email@atlasdostal.com>
# Objective
Needing to derive `AnimationEvent` for `Event` is unnecessary, and the
trigger logic coupled to it feels like we're coupling "event producer"
logic with the event itself, which feels wrong. It also comes with a
bunch of complexity, which is again unnecessary. We can have the
flexibility of "custom animation event trigger logic" without this
coupling and complexity.
The current `animation_events` example is also needlessly complicated,
due to it needing to work around system ordering issues. The docs
describing it are also slightly wrong. We can make this all a non-issue
by solving the underlying ordering problem.
Related to this, we use the `bevy_animation::Animation` system set to
solve PostUpdate animation order-of-operations issues. If we move this
to bevy_app as part of our "core schedule", we can cut out needless
`bevy_animation` crate dependencies in these instances.
## Solution
- Remove `AnimationEvent`, the derive, and all other infrastructure
associated with it (such as the `bevy_animation/derive` crate)
- Replace all instances of `AnimationEvent` traits with `Event + Clone`
- Store and use functions for custom animation trigger logic (ex:
`clip.add_event_fn()`). For "normal" cases users dont need to think
about this and should use the simpler `clip.add_event()`
- Run the `Animation` system set _before_ updating text
- Move `bevy_animation::Animation` to `bevy_app::Animation`. Remove
unnecessary `bevy_animation` dependency from `bevy_ui`
- Adjust `animation_events` example to use the simpler `clip.add_event`
API, as the workarounds are no longer necessary
This is polishing work that will land in 0.15, and I think it is simple
enough and valuable enough to land in 0.15 with it, in the interest of
making the feature as compelling as possible.
# Objective
A new user is likely to try `Query<Component>` instead of
`Query<&Component>`. The error message should guide them to the right
solution.
## Solution
Add a note to the on_unimplemented message for `QueryData` recommending
`&T` and `&mut T`.
The full error message now looks like:
```
error[E0277]: `A` is not valid to request as data in a `Query`
--> crates\bevy_ecs\src\query\world_query.rs:260:18
|
260 | fn system(query: Query<A>) {}
| ^^^^^^^^ invalid `Query` data
|
= help: the trait `fetch::QueryData` is not implemented for `A`
= note: if `A` is a component type, try using `&A` or `&mut A`
= help: the following other types implement trait `fetch::QueryData`:
&'__w mut T
&Archetype
&T
()
(F,)
(F0, F1)
(F0, F1, F2)
(F0, F1, F2, F3)
and 41 others
note: required by a bound in `system::query::Query`
--> crates\bevy_ecs\src\system\query.rs:362:37
|
362 | pub struct Query<'world, 'state, D: QueryData, F: QueryFilter = ()> {
| ^^^^^^^^^ required by this bound in `Query`
```
Alternative to #16450
# Objective
detailed_trace! in its current form does not work (and breaks CI)
## Solution
Fix detailed_trace by checking for the feature properly, adding it to
the correct crates, and removing it from the incorrect crates
# Objective
- Fixes#16406
- Fixes an issue where registering a "deeper" required component, then a
"shallower" required component, would result in the wrong required
constructor being used for the root component.
## Solution
- Make `register_required_components` add any "parent" of a component as
`required_by` to the new "child".
- Assign the depth of the `requiree` plus 1 as the depth of a new
runtime required component.
## Testing
- Added two new tests.
# Objective
- Add methods to facilitate `TextFont` component creation and insertion.
## Solution
- Added `from_font` and `from_font_size` which return a new `TextFont`
with said attributes provided as parameters.
- Added `with_font` and `with_font_size` which return an existing
`TextFont` modifying said attributes with the values provided as
parameters.
## Testing
- CI Checks.
- Tested methods locally by changing values and running the `text_debug`
example.
# Objective
Run this without this PR:
`cargo build -p bevy_hierarchy --no-default-features`
You'll get:
```
error[E0432]: unresolved import `bevy_reflect`
--> crates/bevy_hierarchy/src/events.rs:2:5
|
2 | use bevy_reflect::Reflect;
| ^^^^^^^^^^^^ use of undeclared crate or module `bevy_reflect`
For more information about this error, try `rustc --explain E0432`.
error: could not compile `bevy_hierarchy` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
```
Because of this line:
```rs
use bevy_reflect::Reflect;
#[derive(Event, Debug, Clone, PartialEq, Eq)]
#[cfg_attr(feature = "reflect", derive(Reflect), reflect(Debug, PartialEq))]
pub enum HierarchyEvent { .. }
```
## Solution
use FQN: `derive(bevy_reflect::Reflect)`
## Testing
`cargo build -p bevy_hierarchy --no-default-features`
# Objective
It looks like this file was created based on the `ui_texture_slice`
rendering code and some variable names weren't updated.
## Solution
Rename "texture slice" variable names to "box shadow".
# Objective
#16222 regressed the user experience of actually using gamepads:
```rust
// Before 16222
gamepad.just_pressed(GamepadButton::South)
// After 16222
gamepad.digital.just_pressed(GamepadButton::South)
// Before 16222
gamepad.get(GamepadButton::RightTrigger2)
// After 16222
gamepad.analog.get(GamepadButton::RighTrigger2)
```
Users shouldn't need to think about "digital vs analog" when checking if
a button is pressed. This abstraction was intentional and I strongly
believe it is in our users' best interest. Buttons and Axes are _both_
digital and analog, and this is largely an implementation detail. I
don't think reverting this will be controversial.
## Solution
- Revert most of #16222
- Add the `Into<T>` from #16222 to the internals
- Expose read/write `digital` and `analog` accessors on gamepad, in the
interest of enabling the mocking scenarios covered in #16222 (and
allowing the minority of users that care about the "digital" vs "analog"
distinction in this context to make that distinction)
---------
Co-authored-by: Hennadii Chernyshchyk <genaloner@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
# Objective
- bevy_render (poorly) implements gcd (which should be in bevy_math but
theres not enough justification to have it there either anyways cus its
just one usage)
## Solution
- hardcoded LUT replacement for the one usage
## Testing
- verified the alternative implementation of 4/gcd(4,x) agreed with
original for 0..200
# Objective
https://github.com/AccessKit/accesskit/pull/475 changed how text content
should be set for AccessKit nodes with a role of `Label`. This was
unfortunately missing from #16234.
## Solution
When building an `accesskit::Node` with `Role::Label`, calls `set_value`
instead of `set_label` on the node to set its content.
## Testing
I can't test this right now on my Windows machine due to a compilation
error with wgpu-hal I have no idea how to resolve.
**NOTE: This is based on, and should be merged alongside,
https://github.com/bevyengine/bevy/pull/15482.** I'll leave this in
draft until that PR is merged.
# Objective
Equivalent of https://github.com/bevyengine/bevy/pull/15482 but for
serialization. See that issue for the motivation.
Also part of this tracking issue:
https://github.com/bevyengine/bevy/issues/15518
This PR is non-breaking, just like the deserializer PR (because the new
type parameter `P` has a default `P = ()`).
## Solution
Identical solution to the deserializer PR.
## Testing
Added unit tests and a very comprehensive doc test outlining a clear
example and use case.
# Objective
Fixes#16406.
Currently, the `#[require(...)]` attribute internally registers
component requirements using `register_required_components_manual`. This
is done recursively in a way where every requirement in the "inheritance
tree" is added into a flat `RequiredComponents` hash map with component
constructors and inheritance depths stored.
However, this does not consider runtime requirements: if a plugins has
already registered `C` as required by `B`, and a component `A` requires
`B` through the macro attribute, spawning an entity with `A` won't add
`C`. The `required_by` hash set for `C` doesn't have `A`, and the
`RequiredComponents` of `A` don't have `C`.
Intuitively, I would've thought that the macro attribute's requirements
were always added *before* runtime requirements, and in that case I
believe this shouldn't have been an issue. But the macro requirements
are based on `Component::register_required_components`, which in a lot
of cases (I think) is only called *after* the first time a bundle with
the component is inserted. So if a runtime requirement is defined
*before* this (as is often the case, during `Plugin::build`), the macro
may not take it into account.
## Solution
Register requirements inherited from the `required` component in
`register_required_components_manual_unchecked`.
## Testing
I added a test, essentially the same as in #16406, and it now passes. I
also ran some of the tests in #16409, and they seem to work as expected.
All the existing tests for required components pass.
# Objective
Seemed to have missed the export of `DynamicComponentFetch` from #15593.
`TryFromFilteredError` which is returned by `impl
TryFrom<FiliteredEntityMut/Ref> for EntityRef/Mut` also seemed to have
been missing.
## Solution
Export both of them.
# Objective
MSRV in the standalone crates should be accurate
## Solution
Determine the msrv of each crate and set it
## Testing
Adding better msrv checks to the CI is a next-step.
# Objective
- Fix part of #15920
## Solution
- Keep track of the last written amount of bytes, and bind only that
much of the buffer.
## Testing
- Did you test these changes? If so, how? No
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
---
## Migration Guide
- Fixed a bug with StorageBuffer and DynamicStorageBuffer binding data
from the previous frame(s) due to caching GPU buffers between frames.
# Objective
When adding custom BRP methods one might need to:
- Run custom systems in the `RemoteLast` schedule.
- Order those systems before/after request processing and cleanup.
For example in `bevy_remote_inspector` we need a way to perform some
preparation _before_ request processing. And to perform cleanup
_between_ request processing and watcher cleanup.
## Solution
- Make `RemoteLast` public
- Add `RemoteSet` with `ProcessRequests` and `Cleanup` variants.
I didn't mean to make this item private, fixing it for the 0.15 release
to be consistent with 0.14.
(maintainers: please make sure this gets merged into the 0.15 release
branch as well as main)
# Objective
- Fixes#16152
## Solution
- Put `bevy_window` and `bevy_a11y` behind the `bevy_window` feature.
they were the only difference
- Add `ScheduleRunnerPlugin` to the `DefaultPlugins` when `bevy_window`
is disabled
- Remove `HeadlessPlugins`
- Update the `headless` example
# Objective
- Fixes#16285
- Inactive camera are keeping the component `ViewUniformOffset` from
when they were active, still matching some queries trying to render to
them
## Solution
- Remove component `ViewUniformOffset` from cameras that are inactive
## Testing
- Ran example `render_primitives` and switched camera
Tweaks picking docs slightly for formatting and to add additional
context about the ordering of `Over` and `Out` events. Also shifts `Out`
to trigger before `Over` in the global event ordering.
Because of how focus is tracked, we must send all `Over` and `Out`
events at the same time, in a block. Originally I had `Over` precede
`Out` in the global event order, because this seemed natural. However,
the effect of this, when a pointer moves between entities, is to have
the new entity receive `Over` before the old entity received `Out`,
which several users found confusing.
The new ordering (out before over globally, over before out locally per
entity) should make it much easier to write hover state cleanup code.
# Objective
- When picking sprites, the pointer is offset from the mouse, causing
you to pick sprites you're not mousing over!
## Solution
- Shift over the cursor by the minimum of the viewport.
## Testing
- I was already using the bevy_mod_picking PR for my project, so it
seems to work!
- I tested this on the sprite_example (making the camera only render to
part of the viewport), and it also works there.
## Notes
- This is just https://github.com/aevyrie/bevy_mod_picking/pull/365 but
in Bevy form.
- We don't need to renormalize the viewport in any way since the
viewport is specified in pixels, so all that matters is that the origin
is correct.
Co-authored-by: johanhelsing <johanhelsing@gmail.com>
# Objective
PCSS still has some fundamental issues (#16155). We should resolve them
before "releasing" the feature.
## Solution
1. Rename the already-optional `pbr_pcss` cargo feature to
`experimental_pbr_pcss` to better communicate its state to developers.
2. Adjust the description of the `experimental_pbr_pcss` cargo feature
to better communicate its state to developers.
3. Gate PCSS-related light component fields behind that cargo feature,
to prevent surfacing them to developers by default.
# Objective
UI Anti-aliasing is incorrectly implemented. It always uses an edge
radius of 0.25 logical pixels, and ignores the physical resolution. For
low dpi screens 0.25 is is too low and on higher dpi screens the
physical edge radius is much too large, resulting in visual artifacts.
## Solution
Multiply the distance by the scale factor in the `antialias` function so
that the edge radius stays constant in physical pixels.
## Testing
To see the problem really clearly run the button example with `UiScale`
set really high. With `UiScale(25.)` on main if you examine the button's
border you can see a thick gradient fading away from the edges:
<img width="127" alt="edgg"
src="https://github.com/user-attachments/assets/7c852030-c0e8-4aef-8d3e-768cb2464cab">
With this PR the edges are sharp and smooth at all scale factors:
<img width="127" alt="edge"
src="https://github.com/user-attachments/assets/b3231140-1bbc-4a4f-a1d3-dde21f287988">
# Objective
Text2d doesn't respond to changes to the window scalefactor.
Fixes#16223
## Solution
In `update_text2d_layout` store the previous scale factor in a `Local`
instead and check against the current scale factor to detect changes.
It seems like previously the text wasn't updated because of a bug with
the `WindowScaleFactorChanged` event and it isn't emitted after changes
to the scale factor. That needs to be looked into, but this will work
for now.
## Testing
Really simple app that draws a big message in the middle of the window:
```
use bevy::prelude::*;
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.add_systems(Startup, setup)
.run();
}
fn setup(mut commands: Commands) {
commands.spawn(Camera2d);
commands.spawn((
Text2d::new("Hello"),
TextFont {
font_size: 400.,
..Default::default()
},
));
}
```
Looks fine:
<img width="500" alt="hello1"
src="https://github.com/user-attachments/assets/5320746b-687e-4682-9e4c-bc43ab7ff9d3">
On main, after changing the monitor's scale factor:
<img width="500" alt="hello2"
src="https://github.com/user-attachments/assets/486cea16-fc44-4d66-9468-6f68905d4196">
With this PR the text maintains the same size and position after the
scale factor is changed.
# Objective
- Describe the objective or issue this PR addresses.
Use the fully qualified name for `Component` in the `require` attribute
- If you're fixing a specific issue, say "Fixes #X".
Fixes#16377
## Solution
- Describe the solution used to achieve the objective above.
Use the fully qualified name for `Component` in the `require` attribute,
i.e.,`<#ident as #bevy_ecs_path::component::Component>`
## Testing
- Did you test these changes? If so, how?
`cargo run -p ci -- lints`
`cargo run -p ci -- compile`
`cargo run -p ci -- test`
- Are there any parts that need more testing?
no
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
try to compile
```rust
#[derive(::bevy::ecs::component::Component, Default)]
pub struct A;
#[derive(::bevy::ecs::component::Component)]
#[require(A)]
pub struct B;
```
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Mac only
---
</details>
## Migration Guide
> This section is optional. If there are no breaking changes, you can
delete this section.
- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable
design choice is not a breaking change.
Co-authored-by: Volodymyr Enhelhardt <volodymyr.enhelhardt@ambr.net>
# Objective
We currently use special "floating" constructors for `EasingCurve`,
`FunctionCurve`, and `ConstantCurve` (ex: `easing_curve`). This erases
the type being created (and in general "what is happening"
structurally), for very minimal ergonomics improvements. With rare
exceptions, we prefer normal `X::new()` constructors over floating `x()`
constructors in Bevy. I don't think this use case merits special casing
here.
## Solution
Add `EasingCurve::new()`, use normal constructors everywhere, and remove
the floating constructors.
I think this should land in 0.15 in the interest of not breaking people
later.
# Objective
Fixes#16266
## Solution
Added an `UnregisterSystem` command struct and
`Commands::unregister_system`. Also renamed `World::remove_system` and
`World::remove_system_cached` to `World::unregister_*`
## Testing
It's a fairly simple change, but I tested locally to ensure it actually
works.
---------
Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
# Objective
Fixes#16316
## Solution
Tweaked a few crates cargo files until I was able to build and test
`bevy_ui` via `cargo test --package bevy_ui`
## Testing
- ran `cargo test --package bevy_ui` successfully
- CI should catch anything amiss (Hopefully?)
# Objective
`ButtonBundle` has an `ImageNode` component (renamed from `UiImage`)
which wasn't a problem in 0.14 but in 0.15 `requires` pulls in the
`ContentSize` and `NodeImageSize` which means that by default
`ButtonBundle` nodes are given a measure func based on the size of the
image belonging to `TRANSPARENT_IMAGE_HANDLE`, which is 1x1.
This doesn't make sense and the behaviour for default image nodes should
either be to go to zero size or not add a measure func.
## Solution
Check if an image has a `TRANSPARENT_IMAGE_HANDLE` and if it does remove
its measure func.
Possibly a zero-sized measure would make more sense, but that would
break existing code.
## Testing
Used `ButtonBundle` in the 0.15 `button` example and the border doesn't
render, after this change it does.
# Objective
- Fix bug where `UiSurface::set_camera_children` (and
`UiSurface::update_children` sometimes) will panic if you remove and add
a `Node` component in a single tick. This is more likely to happen now
because of `remove_with_requires`.
## Solution
- Filter out entities with `Node` when cleaning up entities from
`RemovedComponents<Node>`.
## Testing
- Not tested (rust compiler refused to cooperate when I tried to patch
this into my project), correct by inspection.
# Objective
- Allow to configure `on_thread_spawn` and `on_thread_destroy` when
using `TaskPoolPlugin` of bevy.
## Solution
- In `TaskPoolThreadAssignmentPolicy`, two options `on_thread_spawn` and
`on_thread_destroy` are added, which will be passed to two new methods
motioned above when creating corresponding task pool using builder.
- Due to lack of debug derive for these two options, manually implement
the debug for `TaskPoolThreadAssignmentPolicy`.
---
## Changelog
### Added
- `on_thread_spawn` option and `on_thread_destroy` option to the
`TaskPoolPlugin`, allow user to customize them as needed.
## Migration Guide
- `TaskPooolThreadAssignmentPolicy` now has two additional fields:
`on_thread_spawn` and `on_thread_destroy`. Please consider defaulting
them to `None`.
---------
Co-authored-by: François Mockers <mockersf@gmail.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
_If I understand it correctly_, we were checking mesh visibility, as
well as re-rendering point and spot light shadow maps for each view.
This makes it so that M views and N lights produce M x N complexity.
This PR aims to fix that, as well as introduce a stress test for this
specific scenario.
## Solution
- Keep track of what lights have already had mesh visibility calculated
and do not calculate it again;
- Reuse shadow depth textures and attachments across all views, and only
render shadow maps for the _first_ time a light is encountered on a
view;
- Directional lights remain unaltered, since their shadow map cascades
are view-dependent;
- Add a new `many_cameras_lights` stress test example to verify the
solution
## Showcase
110% speed up on the stress test
83% reduction of memory usage in stress test
### Before (5.35 FPS on stress test)
<img width="1392" alt="Screenshot 2024-09-11 at 12 25 57"
src="https://github.com/user-attachments/assets/136b0785-e9a4-44df-9a22-f99cc465e126">
### After (11.34 FPS on stress test)
<img width="1392" alt="Screenshot 2024-09-11 at 12 24 35"
src="https://github.com/user-attachments/assets/b8dd858f-5e19-467f-8344-2b46ca039630">
## Testing
- Did you test these changes? If so, how?
- On my game project where I have two cameras, and many shadow casting
lights I managed to get pretty much double the FPS.
- Also included a stress test, see the comparison above
- Are there any parts that need more testing?
- Yes, I would like help verifying that this fix is indeed correct, and
that we were really re-rendering the shadow maps by mistake and it's
indeed okay to not do that
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- Run the `many_cameras_lights` example
- On the `main` branch, cherry pick the commit with the example (`git
cherry-pick --no-commit 1ed4ace01`) and run it
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
- macOS
---------
Co-authored-by: François Mockers <francois.mockers@vleue.com>
# Objective
Should compile successfully with any combination of features
## Solution
Add the missing import on the right cfg.
## Testing
Tested building locally.
Fixes https://github.com/bevyengine/bevy/issues/16352
# Objective
Glam has some common and useful types and helpers that are not in the
prelude of `bevy_math`. This includes shorthand constructors like
`vec3`, or even `Vec3A`, the aligned version of `Vec3`.
```rust
// The "normal" way to create a 3D vector
let vec = Vec3::new(2.0, 1.0, -3.0);
// Shorthand version
let vec = vec3(2.0, 1.0, -3.0);
```
## Solution
Add the following types and methods to the prelude:
- `vec2`, `vec3`, `vec3a`, `vec4`
- `uvec2`, `uvec3`, `uvec4`
- `ivec2`, `ivec3`, `ivec4`
- `bvec2`, `bvec3`, `bvec3a`, `bvec4`, `bvec4a`
- `mat2`, `mat3`, `mat3a`, `mat4`
- `quat` (not sure if anyone uses this, but for consistency)
- `Vec3A`
- `BVec3A`, `BVec4A`
- `Mat3A`
I did not add the u16, i16, or f64 variants like `dvec2`, since there
are currently no existing types like those in the prelude.
The shorthand constructors are currently used a lot in some places in
Bevy, and not at all in others. In a follow-up, we might want to
consider if we have a preference for the shorthand, and make a PR to
change the codebase to use it more consistently.
# Objective
- Fixes: #15603
## Solution
- Add an unsafe `get_mut_by_id_unchecked` to `EntityMut` that borrows
&self instead of &mut self, thereby allowing access to multiple
components simultaneously.
## Testing
- a unit test function `get_mut_by_id_unchecked` was added.
---------
Co-authored-by: Mike <mike.hsu@gmail.com>
**NOTE: Also see https://github.com/bevyengine/bevy/pull/15548 for the
serializer equivalent**
# Objective
The current `ReflectDeserializer` and `TypedReflectDeserializer` use the
`TypeRegistration` and/or `ReflectDeserialize` of a given type in order
to determine how to deserialize a value of that type. However, there is
currently no way to statefully override deserialization of a given type
when using these two deserializers - that is, to have some local data in
the same scope as the `ReflectDeserializer`, and make use of that data
when deserializing.
The motivating use case for this came up when working on
[`bevy_animation_graph`](https://github.com/aecsocket/bevy_animation_graph/tree/feat/dynamic-nodes),
when loading an animation graph asset. The `AnimationGraph` stores
`Vec<Box<dyn NodeLike>>`s which we have to load in. Those `Box<dyn
NodeLike>`s may store `Handle`s to e.g. `Handle<AnimationClip>`. I want
to trigger a `load_context.load()` for that handle when it's loaded.
```rs
#[derive(Reflect)]
struct Animation {
clips: Vec<Handle<AnimationClip>>,
}
```
```rs
(
clips: [
"animation_clips/walk.animclip.ron",
"animation_clips/run.animclip.ron",
"animation_clips/jump.animclip.ron",
],
)
````
Currently, if this were deserialized from an asset loader, this would be
deserialized as a vec of `Handle::default()`s, which isn't useful since
we also need to `load_context.load()` those handles for them to be used.
With this processor field, a processor can detect when `Handle<T>`s are
being loaded, then actually load them in.
## Solution
```rs
trait ReflectDeserializerProcessor {
fn try_deserialize<'de, D>(
&mut self,
registration: &TypeRegistration,
deserializer: D,
) -> Result<Result<Box<dyn PartialReflect>, D>, D::Error>
where
D: serde::Deserializer<'de>;
}
```
```diff
- pub struct ReflectDeserializer<'a> {
+ pub struct ReflectDeserializer<'a, P = ()> { // also for ReflectTypedDeserializer
registry: &'a TypeRegistry,
+ processor: Option<&'a mut P>,
}
```
```rs
impl<'a, P: ReflectDeserializerProcessor> ReflectDeserializer<'a, P> { // also for ReflectTypedDeserializer
pub fn with_processor(registry: &'a TypeRegistry, processor: &'a mut P) -> Self {
Self {
registry,
processor: Some(processor),
}
}
}
```
This does not touch the existing `fn new`s.
This `processor` field is also added to all internal visitor structs.
When `TypedReflectDeserializer` runs, it will first try to deserialize a
value of this type by passing the `TypeRegistration` and deserializer to
the processor, and fallback to the default logic. This processor runs
the earliest, and takes priority over all other deserialization logic.
## Testing
Added unit tests to `bevy_reflect::serde::de`. Also using almost exactly
the same implementation in [my fork of
`bevy_animation_graph`](https://github.com/aecsocket/bevy_animation_graph/tree/feat/dynamic-nodes).
## Migration Guide
(Since I added `P = ()`, I don't think this is actually a breaking
change anymore, but I'll leave this in)
`bevy_reflect`'s `ReflectDeserializer` and `TypedReflectDeserializer`
now take a `ReflectDeserializerProcessor` as the type parameter `P`,
which allows you to customize deserialization for specific types when
they are found. However, the rest of the API surface (`new`) remains the
same.
<details>
<summary>Original implementation</summary>
Add `ReflectDeserializerProcessor`:
```rs
struct ReflectDeserializerProcessor {
pub can_deserialize: Box<dyn FnMut(&TypeRegistration) -> bool + 'p>,
pub deserialize: Box<
dyn FnMut(
&TypeRegistration,
&mut dyn erased_serde::Deserializer,
) -> Result<Box<dyn PartialReflect>, erased_serde::Error>
+ 'p,
}
```
Along with `ReflectDeserializer::new_with_processor` and
`TypedReflectDeserializer::new_with_processor`. This does not touch the
public API of the existing `new` fns.
This is stored as an `Option<&mut ReflectDeserializerProcessor>` on the
deserializer and any of the private `-Visitor` structs, and when we
attempt to deserialize a value, we first pass it through this processor.
Also added a very comprehensive doc test to
`ReflectDeserializerProcessor`, which is actually a scaled down version
of the code for the `bevy_animation_graph` loader. This should give
users a good motivating example for when and why to use this feature.
### Why `Box<dyn ..>`?
When I originally implemented this, I added a type parameter to
`ReflectDeserializer` to determine the processor used, with `()` being
"no processor". However when using this, I kept running into rustc
errors where it failed to validate certain type bounds and led to
overflows. I then switched to a dynamic dispatch approach.
The dynamic dispatch should not be that expensive, nor should it be a
performance regression, since it's only used if there is `Some`
processor. (Note: I have not benchmarked this, I am just speculating.)
Also, it means that we don't infect the rest of the code with an extra
type parameter, which is nicer to maintain.
### Why the `'p` on `ReflectDeserializerProcessor<'p>`?
Without a lifetime here, the `Box`es would automatically become `Box<dyn
FnMut(..) + 'static>`. This makes them practically useless, since any
local data you would want to pass in must then be `'static`. In the
motivating example, you couldn't pass in that `&mut LoadContext` to the
function.
This means that the `'p` infects the rest of the Visitor types, but this
is acceptable IMO. This PR also elides the lifetimes in the `impl<'de>
Visitor<'de> for -Visitor` blocks where possible.
### Future possibilities
I think it's technically possible to turn the processor into a trait,
and make the deserializers generic over that trait. This would also open
the door to an API like:
```rs
type Seed;
fn seed_deserialize(&mut self, r: &TypeRegistration) -> Option<Self::Seed>;
fn deserialize(&mut self, r: &TypeRegistration, d: &mut dyn erased_serde::Deserializer, s: Self::Seed) -> ...;
```
A similar processor system should also be added to the serialization
side, but that's for another PR. Ideally, both PRs will be in the same
release, since one isn't very useful without the other.
## Testing
Added unit tests to `bevy_reflect::serde::de`. Also using almost exactly
the same implementation in [my fork of
`bevy_animation_graph`](https://github.com/aecsocket/bevy_animation_graph/tree/feat/dynamic-nodes).
## Migration Guide
`bevy_reflect`'s `ReflectDeserializer` and `TypedReflectDeserializer`
now take a second lifetime parameter `'p` for storing the
`ReflectDeserializerProcessor` field lifetimes. However, the rest of the
API surface (`new`) remains the same, so if you are not storing these
deserializers or referring to them with lifetimes, you should not have
to make any changes.
</details>
# Objective
`glam` has opted to rename `Vec2::angle_between` to `Vec2::angle_to`
because of the difference in semantics compared to `Vec3::angle_between`
and others which return an unsigned angle `[0, PI]` where
`Vec2::angle_between` returns a signed angle `[-PI, PI]`.
We should follow suit for `Rot2` in 0.15 to avoid further confusion.
Links:
-
https://github.com/bitshifter/glam-rs/issues/514#issuecomment-2143202294
- https://github.com/bitshifter/glam-rs/pull/524
## Migration Guide
`Rot2::angle_between` has been deprecated, use `Rot2::angle_to` instead,
the semantics of `Rot2::angle_between` will change in the future.
---------
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
# Objective
Fixes#15940
## Solution
Remove the `pub use` and fix the compile errors.
Make `bevy_image` available as `bevy::image`.
## Testing
Feature Frenzy would be good here! Maybe I'll learn how to use it if I
have some time this weekend, or maybe a reviewer can use it.
## Migration Guide
Use `bevy_image` instead of `bevy_render::texture` items.
---------
Co-authored-by: chompaa <antony.m.3012@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
- Currently when you attempt to change the cursor_grab_mode it caches
the new value whether the cursor grab succeeded or failed. This change
handles the Result being returned by set_cursor_grab and changes the
cursor_grab_mode back to the cached version in case of an Error.
- Creates a way to handle #16237 and #16238
## Solution
- I changed the signature of winit_windows attempt_grab to return the
Result<(), ExternalError> that winit set_cursor_grab returns. The system
that calls attempt_grab now checks if there's an error returned, and if
there is it sets the grab_mode back to the cached version (similar to
what hit_test does a few lines down).
## Testing
- I tested using this system that previously would not correctly lock
the mouse on Ubuntu/x11
```
pub fn lock_mouse(mut primary_window: Query<&mut Window, With<PrimaryWindow>>) {
let window = &mut primary_window.single_mut();
if window.focused {
window.cursor_options.grab_mode = CursorGrabMode::Confined;
} else {
window.cursor_options.grab_mode = CursorGrabMode::None;
}
}
```
- I only tested on Ubuntu with x11
# Objective
Fixes#15928
## Solution
return Error instead of panic
## Testing
I don't know if we need to add a test for this. It is pretty
straightforward.
# Objective
- wgpu 0.20 made workgroup vars stop being zero-init by default. this
broke some applications (cough foresight cough) and now we workaround
it. wgpu exposes a compilation option that zero initializes workgroup
memory by default, but bevy does not expose it.
## Solution
- expose the compilation option wgpu gives us
## Testing
- ran examples: 3d_scene, compute_shader_game_of_life, gpu_readback,
lines, specialized_mesh_pipeline. they all work
- confirmed fix for our own problems
---
</details>
## Migration Guide
- add `zero_initialize_workgroup_memory: false,` to
`ComputePipelineDescriptor` or `RenderPipelineDescriptor` structs to
preserve 0.14 functionality, add `zero_initialize_workgroup_memory:
true,` to restore bevy 0.13 functionality.
# Objective
- Fixes#16235
## Solution
- Both Bevy and AccessKit export a `Node` struct, to reduce confusion
Bevy will no longer re-export `AccessKit` from `bevy_a11y`
## Testing
- Tested locally
## Migration Guide
```diff
# main.rs
-- use bevy_a11y::{
-- accesskit::{Node, Rect, Role},
-- AccessibilityNode,
-- };
++ use bevy_a11y::AccessibilityNode;
++ use accesskit::{Node, Rect, Role};
# Cargo.toml
++ accesskit = "0.17"
```
- Users will need to add `accesskit = "0.17"` to the dependencies
section of their `Cargo.toml` file and update their `accesskit` use
statements to come directly from the external crate instead of
`bevy_a11y`.
- Make sure to keep the versions of `accesskit` aligned with the
versions Bevy uses.
# Objective
- Fixed issue where `thiserror` `#[error(...)]` attributes were
improperly converted to `derive_more` `#[display(...)]` equivalents in
certain cases with a tuple struct/enum variant.
## Solution
- Used `re/#\[display\(.*\{[0-9]+\}.*\)\]/` to find occurences of using
`{0}` where `{_0}` was intended (checked for other field indexes too)and
updated accordingly.
## Testing
- `cargo check`
- CI
## Notes
This was discovered by @dtolnay in [this
comment](https://github.com/bevyengine/bevy/pull/15772#discussion_r1833730555).
# Objective
`AudioPlayer::<AudioSource>(assets.load("audio.mp3"))` is awkward and
complicated to type because the `AudioSource` generic type cannot be
elided. This is especially annoying because `AudioSource` is used in the
majority of cases. Most users don't need to think about it.
## Solution
Add an `AudioPlayer::new()` function that is hard-coded to
`AudioSource`, allowing `AudioPlayer::new(assets.load("audio.mp3"))`.
Prefer using that in the relevant places.
# Objective
In the existing implementation, additive blending effectively treats the
node with least index specially by basically forcing its weight to be
`1.0` regardless of what its computed weight would be (based on the
weights in the `AnimationGraph` and `AnimationPlayer`).
Arguably this makes some amount of sense, because the "base" animation
is often one which was not authored to be used additively, meaning that
its sampled values are interpreted absolutely rather than as deltas.
However, this also leads to strange behavior with respect to animation
masks: if the "base" animation is masked out on some target, then the
next node is treated as the "base" animation, despite the fact that it
would normally be interpreted additively, and the weight of that
animation is thrown away as a result.
This is all kind of weird and revolves around special treatment (if the
behavior is even really intentional in the first place). From a
mathematical standpoint, there is nothing special about how the "base"
animation must be treated other than having a weight of 1.0 under an
`Add` node, which is something that the user can do without relying on
some bizarre corner-case behavior of the animation system — this is the
only present situation under which weights are discarded.
This PR changes this behavior so that the weight of every node is
incorporated. In other words, for an animation graph that looks like
this:
```text
┌───────────────┐
│Base clip ┼──┐
│ 0.5 │ │
└───────────────┘ │
┌───────────────┐ │ ┌───────────────┐ ┌────┐
│Additive clip 1┼──┼─►┤Additive blend ┼────►│Root│
│ 0.1 │ │ │ 1.0 │ └────┘
└───────────────┘ │ └───────────────┘
┌───────────────┐ │
│Additive clip 2┼──┘
│ 0.2 │
└───────────────┘
```
Previously, the result would have been
```text
base_clip + 0.1 * additive_clip_1 + 0.2 * additive_clip_2
```
whereas now it would be
```text
0.5 * base_clip + 0.1 * additive_clip_1 + 0.2 * additive_clip_2
```
and in the scenario where `base_clip` is masked out:
```text
additive_clip_1 + 0.2 * additive_clip_2
```
vs.
```text
0.1 * additive_clip_1 + 0.2 * additive_clip_2
```
## Solution
For background, the way that the additive blending procedure works is
something like this:
- During graph traversal, the node values and weights of the children
are pushed onto the evaluator `stack`. The traversal order guarantees
that the item with least node index will be on top.
- Once we reach the `Add` node itself, we start popping off the `stack`
and into the evaluator's `blend_register`, which is an accumulator
holding up to one weight-value pair:
- If the `blend_register` is empty, it is filled using data from the top
of the `stack`.
- Otherwise, the `blend_register` is combined with data popped from the
`stack` and updated.
In the example above, the additive blending steps would look like this
(with the pre-existing implementation):
1. The `blend_register` is empty, so we pop `(base_clip, 0.5)` from the
top of the `stack` and put it in. Now the value of the `blend_register`
is `(base_clip, 0.5)`.
2. The `blend_register` is non-empty: we pop `(additive_clip_1, 0.1)`
from the top of the `stack` and combine it additively with the value in
the `blend_register`, forming `(base_clip + 0.1 * additive_clip_1, 0.6)`
in the `blend_register` (the carried weight value goes unused).
3. The `blend_register` is non-empty: we pop `(additive_clip_2, 0.2)`
from the top of the `stack` and combine it additively with the value in
the `blend_register`, forming `(base_clip + 0.1 * additive_clip_1 + 0.2
* additive_clip_2, 0.8)` in the `blend_register`.
The solution in this PR changes step 1: the `base_clip` is multiplied by
its weight as it is added to the `blend_register` in the first place,
yielding `0.5 * base_clip + 0.1 * additive_clip_1 + 0.2 *
additive_clip_2` as the final result.
### Note for reviewers
It might be tempting to look at the code, which contains a segment that
looks like this:
```rust
if additive {
current_value = A::blend(
[
BlendInput {
weight: 1.0, // <--
value: current_value,
additive: true,
},
BlendInput {
weight: weight_to_blend,
value: value_to_blend,
additive: true,
},
]
.into_iter(),
);
}
```
and conclude that the explicit value of `1.0` is responsible for
overwriting the weight of the base animation. This is incorrect.
Rather, this additive blend has to be written this way because it is
multiplying the *existing value in the blend register* by 1 (i.e. not
doing anything) before adding the next value to it. Changing this to
another quantity (e.g. the existing weight) would cause the value in the
blend register to be spuriously multiplied down.
## Testing
Tested on `animation_masks` example. Checked `morph_weights` example as
well.
## Migration Guide
I will write a migration guide later if this change is not included in
0.15.
# Objective
After #12929 we no longer have methods to get component or ticks for
previously obtained table column.
It's possible to use a lower level API by indexing the slice, but then
it won't be possible to construct `ComponentTicks`.
## Solution
Make `ComponentTicks` fields public. They don't hold any invariants and
you can't get a mutable reference to the struct in Bevy.
I also removed the getters since they are no longer needed.
## Testing
- I tested the compilation
---
## Migration Guide
- Instead of using `ComponentTicks::last_changed_tick` and
`ComponentTicks::added_tick` methods, access fields directly.
# Objective
Re-enable some tests in `entity_ref.rs` that are marked as `#[ignore]`,
but that pass after #14561.
## Solution
Remove `#[ignore]` from those tests.
# Objective
Automatic imaging sizing for image nodes isn't working because the the
`ContentSize` requirement for `UiImage` got lost in some merge again.
Fixes#16239Fixes#16240
Fixes the missing images seen in #16241
## Solution
Require `ContentSize` for `UiImage`.
# Objective
- Fixes#16254
- fix building in wasm without custom_cursor
## Solution
- Properly flag `CustomCursor::Url` which only exist in wasm, but also
only when `custom_cursor` is enabled
## Testing
- `cargo check --target wasm32-unknown-unknown -p bevy_winit`
# Objective
- Attempts to fix#16042
## Solution
- Added a new `RemoteSystem` `SystemSet` for the BRP systems.
- Changed the schedule on which these systems run from `Update` to
`Last`.
## Testing
- I did not test these changes and would appreciate a hand in doing so.
I assume it would be good to test that you can order against these
systems easily now.
---
## Migration Guide
- `process_remote_requests`, `process_ongoing_watching_requests` and
`remove_closed_watching_requests` now run in the `Last` schedule. Make
sure you use `RemoteSystem` `SystemSet` in case you need to order your
systems against them.
# Objective
Addressing a suggestion I made in Discord: store gamepad name as a
`Name` component.
Advantages:
- Will be nicely displayed in inspector / editor.
- Easier to spawn in tests, just `world.spawn(Gamepad::default())`.
## Solution
`Gamepad` component now stores only vendor and product IDs and `Name`
stores the gamepad name.
Since `GamepadInfo` is no longer necessary, I removed it and merged its
fields into the connection event.
## Testing
- Run unit tests.
---
## Migration Guide
- `GamepadInfo` no longer exists:
- Name now accesible via `Name` component.
- Other information available on `Gamepad` component directly.
- `GamepadConnection::Connected` now stores all info fields directly.
# Objective
Use same pattern when creating `TransparentUi` items where the
`sort_key` is the `UiNode` stack index + some offset.
## Solution
Refactored to follow same pattern.
## Testing
Ran few UI examples.
## Doubts
Maybe `stack_z_offsets::BACKGROUND_COLOR` should be renamed. This is
used for `ExtractedUiNode`, which is not only used for "background
color" it's also used to render borders, images and text (I think).
In `bevy_mod_picking` events are driven by several interlocking state
machines, which read and write events, and share state in a few common
resources. When I merged theses state machines into one to make event
ordering work properly, I combined this state and hid it in a `Local`.
This PR exposes the state in a resource again. Also adds a simple little
API for it. Useful for adding debug UI.
# Objective
Exposes a means to create an asset directory (and its parent
directories). Wasn't sure whether we also wanted the variant to create
directories without the parent (i.e. `mkdir` instead of `mkdir -p`)?
Fixes https://github.com/bevyengine/bevy_editor_prototypes/issues/144
# Objective
- Bumps accesskit and accesskit_winit dependencies
## Solution
- Fixes several breaking API changes introduced in accesskit 0.23.
## Testing
- Tested with the ui example and seems to work comparably
# Objective
Closes#16221.
## Solution
- Make `Gamepad` fields public and remove delegates / getters.
- Move `impl Into` to `Axis` methods (delegates for `Axis` used `impl
Into` to allow passing both `GamepadAxis` and `GamepadButton`).
- Improve docs.
## Testing
- I run tests.
Not sure if the migration guide is needed, since it's a feature from RC,
but I wrote it just in case.
---
## Migration Guide
- `Gamepad` fields are now public.
- Instead of using `Gamepad` delegates like `Gamepad::just_pressed`,
call these methods directly on the fields.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
GlobalTransform's current methods make it unintuitive, long and clunky
to access just the rotation or just the scale.
## Solution
Dedicated just_rotation() and scale() methods to access just these
properties.
I'm not sure about the naming, I chose just_rotation() to show that try
to indicate there is a waste since it also computes the other fields.
## Testing
- Did you test these changes? If so, how?
I tried logging the methods with a rotating and scaling cube and the
values were correct.
- Are there any parts that need more testing?
My methods are based on existing bevy/glam methods so should be correct
from the getgo.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
Probably the easiest is using the 3d_rotations example, adding scaling
to it and then logging the methods I added
---
## Showcase
```rust
fn log(gt_query: Query<&GlobalTransform>) {
for global_transform in gt_query().iter() {
println!("{} {}", global_transform.just_rotation(), global_transform.scale());
}
}
```
---------
Co-authored-by: Sigma-dev <antonin.programming@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- fix formatting issue in "mesh_view_binding.wgsl"
_note: As naga-oil preprocessor match the whole line when finding an
"#endif",
It's just for external formatting tool and consistency._
## Solution
Trivial change.
Add '//' before the closing comment of the "#endif"
# Objective
gpu based mesh uniform construction in the `GpuPreprocessNode` is
currently in `Core3d`. The node iterates all views and schedules the
uniform construction for each. so
- when there are multiple 3d cameras, it runs multiple times on each
view
- if a view wants to render meshes but doesn't use the `Core3d` graph,
the camera must run later than at least one `Core3d`-based camera (or
add the node to its own graph, duplicating the work)
- If views want to share mesh uniforms there is no way to avoid running
the preprocessing for every view
## Solution
- move the node to the top level of the rendergraph, before the camera
driver node
- make the `PreprocessBindGroup` `clone`able, and add a
`SkipGpuPreprocessing` component to allow opting out per view
# Objective
Currently, if we have two cameras with the same output texture, one with
`CameraOutputMode::Write` and one with `CameraOutputMode::Skip`, it is
possible for the `CameraOutputMode::Write` camera to be assigned alpha
blending (which is the fallback blending when multiple cameras write to
the same output texture), although it is the only camera writing to the
output texture. This may or may not happen every restart of the app,
because the camera iteration order in prepare_view_upscaling_pipelines
isn't consistent. Since this is random behaviour I consider this a bug
and didn't add a migration guide.
## Solution
In `prepare_view_upscaling_pipelines` make sure we don't consider
cameras with CameraOutputMode::Skip to be outputting something to the
output texture.
## Testing
I ran a few examples to make sure nothing obvious is broken. There is no
example using CameraOutputMode::Skip, so I only tested the change in my
own App where this was relevant, which however isn't public.
# Objective
- Choose LOD based on normal simplification error in addition to
position error
- Update meshoptimizer to 0.22, which has a bunch of simplifier
improvements
## Testing
- Did you test these changes? If so, how?
- Visualize normals, and compare LOD changes before and after. Normals
no longer visibly change as the LOD cut changes.
- Are there any parts that need more testing?
- No
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- Run the meshlet example in this PR and on main and move around to
change the LOD cut. Before running each example, in
meshlet_mesh_material.wgsl, replace `let color = vec3(rand_f(&rng),
rand_f(&rng), rand_f(&rng));` with `let color =
(vertex_output.world_normal + 1.0) / 2.0;`. Make sure to download the
appropriate bunny asset for each branch!
# Objective
1. UI texture slicing chops and scales an image to fit the size of a
node and isn't meant to place any constraints on the size of the node
itself, but because the required components changes required `ImageSize`
and `ContentSize` for nodes with `UiImage`, texture sliced nodes are
laid out using an `ImageMeasure`.
2. In 0.14 users could spawn a `(UiImage, NodeBundle)` which would
display an image stretched to fill the UI node's bounds ignoring the
image's instrinsic size. Now that `UiImage` requires `ContentSize`,
there's no option to display an image without its size placing
constrains on the UI layout (unless you force the `Node` to a fixed
size, but that's not a solution).
3. It's desirable that the `Sprite` and `UiImage` share similar APIs.
Fixes#16109
## Solution
* Remove the `Component` impl from `ImageScaleMode`.
* Add a `Stretch` variant to `ImageScaleMode`.
* Add a field `scale_mode: ImageScaleMode` to `Sprite`.
* Add a field `mode: UiImageMode` to `UiImage`.
* Add an enum `UiImageMode` similar to `ImageScaleMode` but with
additional UI specific variants.
* Remove the queries for `ImageScaleMode` from Sprite and UI extraction,
and refer to the new fields instead.
* Change `ui_layout_system` to update measure funcs on any change to
`ContentSize`s to enable manual clearing without removing the component.
* Don't add a measure unless `UiImageMode::Auto` is set in
`update_image_content_size_system`. Mutably deref the `Mut<ContentSize>`
if the `UiImage` is changed to force removal of any existing measure
func.
## Testing
Remove all the constraints from the ui_texture_slice example:
```rust
//! This example illustrates how to create buttons with their textures sliced
//! and kept in proportion instead of being stretched by the button dimensions
use bevy::{
color::palettes::css::{GOLD, ORANGE},
prelude::*,
winit::WinitSettings,
};
fn main() {
App::new()
.add_plugins(DefaultPlugins)
// Only run the app when there is user input. This will significantly reduce CPU/GPU use.
.insert_resource(WinitSettings::desktop_app())
.add_systems(Startup, setup)
.add_systems(Update, button_system)
.run();
}
fn button_system(
mut interaction_query: Query<
(&Interaction, &Children, &mut UiImage),
(Changed<Interaction>, With<Button>),
>,
mut text_query: Query<&mut Text>,
) {
for (interaction, children, mut image) in &mut interaction_query {
let mut text = text_query.get_mut(children[0]).unwrap();
match *interaction {
Interaction::Pressed => {
**text = "Press".to_string();
image.color = GOLD.into();
}
Interaction::Hovered => {
**text = "Hover".to_string();
image.color = ORANGE.into();
}
Interaction::None => {
**text = "Button".to_string();
image.color = Color::WHITE;
}
}
}
}
fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
let image = asset_server.load("textures/fantasy_ui_borders/panel-border-010.png");
let slicer = TextureSlicer {
border: BorderRect::square(22.0),
center_scale_mode: SliceScaleMode::Stretch,
sides_scale_mode: SliceScaleMode::Stretch,
max_corner_scale: 1.0,
};
// ui camera
commands.spawn(Camera2d);
commands
.spawn(Node {
width: Val::Percent(100.0),
height: Val::Percent(100.0),
align_items: AlignItems::Center,
justify_content: JustifyContent::Center,
..default()
})
.with_children(|parent| {
for [w, h] in [[150.0, 150.0], [300.0, 150.0], [150.0, 300.0]] {
parent
.spawn((
Button,
Node {
// width: Val::Px(w),
// height: Val::Px(h),
// horizontally center child text
justify_content: JustifyContent::Center,
// vertically center child text
align_items: AlignItems::Center,
margin: UiRect::all(Val::Px(20.0)),
..default()
},
UiImage::new(image.clone()),
ImageScaleMode::Sliced(slicer.clone()),
))
.with_children(|parent| {
// parent.spawn((
// Text::new("Button"),
// TextFont {
// font: asset_server.load("fonts/FiraSans-Bold.ttf"),
// font_size: 33.0,
// ..default()
// },
// TextColor(Color::srgb(0.9, 0.9, 0.9)),
// ));
});
}
});
}
```
This should result in a blank window, since without any constraints the
texture slice image nodes should be zero-sized. But in main the image
nodes are given the size of the underlying unsliced source image
`textures/fantasy_ui_borders/panel-border-010.png`:
<img width="321" alt="slicing"
src="https://github.com/user-attachments/assets/cbd74c9c-14cd-4b4d-93c6-7c0152bb05ee">
For this PR need to change the lines:
```
UiImage::new(image.clone()),
ImageScaleMode::Sliced(slicer.clone()),
```
to
```
UiImage::new(image.clone()).with_mode(UiImageMode::Sliced(slicer.clone()),
```
and then nothing should be rendered, as desired.
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
The schedule graph can easily confirm whether a set is contained or not.
This helps me in my personal project where I write an extension trait
for `Schedule` and I want to configure a specific set in its methods.
The set in question has a run condition though and I don't want to add
that condition to the same schedule as many times as the trait methods
are called. Since the non-pub set is unknown to the schedule until then,
a `contains_set` is sufficient.
It is probably trivial to add a method that returns an `Option<NodeId>`
as well but as I personally don't need it I did not add that. If it is
desired I can do so here though. It might be unneeded to have a
`contains_set` then because one could check `is_some` on the returned id
in that case.
An argument against that is that future changes may be easier if only a
`contains_set` needs to be ported.
## Solution
Added `ScheduleGraph::contains_set`.
## Testing
I put the below showcase code into a temporary unit test and it worked.
If wanted I add it as a test too but I did not see that other more
somewhat complicated methods have tests
---
## Showcase
```rs
#[derive(ScheduleLabel, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
struct MySchedule;
#[derive(SystemSet, Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
struct MySet;
let mut schedule = Schedule::new(MySchedule);
assert_eq!(schedule.graph().contains_set(MySet), false);
schedule.configure_sets(MySet);
assert_eq!(schedule.graph().contains_set(MySet), true);
```
# Objective
- `CircularSegment` and `CircularSector` are well defined 2D shapes with
both an area and a perimeter.
# Solution
- This PR implements `perimeter` for both and moves the existsing `area`
functions into the `Measured2d` implementations.
## Testing
- The `arc_tests` have been extended to also check for perimeters.
# Objective
Currently there's no way to change the window's cursor icon with the
`custom_cursor` feature **disabled**. You should still be able to set
system cursor icons.
Connections:
- https://github.com/bevyengine/bevy/pull/15649
## Solution
Move some `custom_cursor` feature gates around, as to expose the
`CursorIcon` type again.
Note this refactoring was mainly piloted by hunting after the compiler
warnings -- I shouldn't have missed anything, but FYI.
## Testing
Disabled the `custom_cursor` feature, ran the `window_settings` example.
# Objective
#14273 changed `Msaa` to be a component rather than a resource. However,
the documentation still says that it is a resource. This tripped me up
during migration to 0.15 until I looked at the type definition.
Additionally, the docs have some unnecessary repetition and some grammar
mistakes, and they don't link to camera documentation.
## Solution
Fix up the docs!
# Objective
When merging two meshes, we need to find the offset of indices for the
second mesh. Currently it is done by inserting empty positions if
positions is not set.
Although practically it is not an issue, this does not feel right:
- We did not have positions before, then why we have positions after
merge?
- Moreover, if positions are not set, but uvs are not empty, computed
offset will be zero, while it should be equal to the number of uvs.
## Solution
Use `Mesh::count_vertices` to find the number of vertices.
## Testing
Looking hard.
# Objective
There's integer overflow in `Mesh::merge` in branches like this:
405fa3e8ea/crates/bevy_mesh/src/mesh.rs (L857-L859)
we truncate `u32` to `u16` and ignore integer overflow on `u16`. This
may lead to unexpected results when the number of vertices exceeds
`u16::MAX`.
## Solution
Convert indices storage to `u32` when necessary.
## Testing
- Unit test added for `extend` function
- For changes in `Mesh`, I presume it is already tested elsewhere
# Objective
- Fixes#15757
## Solution
- Add the platform specific property `prefers_home_indicator_hidden` to
bevy's Window configuration, and applying it by invoking
`with_prefers_home_indicator_hidden` in `winit`.
## Testing
- I have tested the `bevy_mobile_example` on the iOS platform.
## Showcase
- Currently, the `prefers_home_indicator_hidden` is enabled in the
bevy_mobile_example demo. You can test it with an iOS device. The home
indicator will disappear after several seconds of inactivity in the
bottom areas.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fix#15841
## Solution
Added an update schedule as recommended in the issue.
## Testing
Doc test is run and passes.
Ran the example in a test app before and after adding the line.
## Showcase
Before:
```
PS C:\Users\BenjaminBrienen\source\bevy_experiment_test> cargo run
Blocking waiting for file lock on build directory
Compiling bevy_experiment_test v0.1.0 (C:\Users\BenjaminBrienen\source\bevy_experiment_test)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 1m 41s
Running `target\debug\bevy_experiment_test.exe`
```
(nothing happens)
After:
```
PS C:\Users\BenjaminBrienen\source\bevy_experiment_test> cargo run
Blocking waiting for file lock on build directory
Compiling bevy_experiment_test v0.1.0 (C:\Users\BenjaminBrienen\source\bevy_experiment_test)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 14.64s
Running `target\debug\bevy_experiment_test.exe`
system of subapp is executing and the Counter: 10
```
Fixes a small divergence between `bevy_mod_picking` and the up-streamed
`bevy_picking`: Both have a `Pointer<E>` constructor with the same
types, but in a different order.
This is part of work being done on `bevy_mod_picking` to simplify the
migration to `bevy_picking`.
# Objective
Make all the methods and associated functions belonging to
`ComputedNode` const.
## Solution
Constify (except for `inner_radius` which uses non-const `min` and
`max`).
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fixes#15676
## Solution
`remove` returns the removed item
Add `take`
## Testing
None yet
## Migration Guide
If you don't need the returned value from `remove`, discard it.
# Objective
The `ContentSize` requirement on `UiImage` got lost during merge
conflict fixes, causing some images such as the icons on the `game_menu`
example to disappear.
Fixes#16136
## Solution
Require `ContentSize` on `UiImage` again.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Taffy added layout rounding a while ago but it had a couple of bugs and
caused some problems with the fussy `ab_glyph` text implementation. So I
disabled Taffy's builtin rounding and added some hacks ad hoc that fixed
(some) of those issues. Since then though Taffy's rounding algorithm has
improved while we've changed layout a lot and migrated to `cosmic-text`
so those hacks don't help any more and in some cases cause significant
problems.
Also our rounding implementation only rounds to the nearest logical
pixel, whereas Taffy rounds to the nearest physical pixel meaning it's
much more accurate with high dpi displays.
fixes#15197
## Some examples of layout rounding errors visible in the UI examples
These errors are much more obvious at high scale factor, you might not
see any problems at a scale factor of 1.
`cargo run --example text_wrap_debug`
<img width="1000" alt="text_debug_gaps"
src="https://github.com/user-attachments/assets/5a584016-b8e2-487b-8842-f0f359077391">
The narrow horizontal and vertical lines are gaps in the layout caused
by errors in the coordinate rounding.
`cargo run --example text_debug`
<img width="1000" alt="text_debug"
src="https://github.com/user-attachments/assets/a4b37c02-a2fd-441c-a7bd-cd7a1a72e7dd">
The two text blocks here are aligned right to the same boundary but in
this screen shot you can see that the lower block is one pixel off to
the left. Because the size of this text node changes between frames with
the reported framerate the rounding errors cause it to jump left and
right.
## Solution
Remove all our custom rounding hacks and reenable Taffy's layout
rounding.
The gaps in the `text_wrap_debug` example are gone:
<img width="1000" alt="text_wrap_debug_fix"
src="https://github.com/user-attachments/assets/92d2dd97-30c6-4ac8-99f1-6d65358995a7">
This doesn't fix some of the gaps that occur between borders and content
but they seem appear to be a rendering problem as they disappear with
`UiAntiAlias::Off` set.
## Testing
Run the examples as described above in the `Objective` section. With
this PR the problems mentioned shouldn't appear.
Also added an example in a separate PR #16096 `layout_rounding_debug`
for identifying these issues.
## Migration Guide
`UiSurface::get_layout` now also returns the final sizes before
rounding. Call `.0` on the `Ok` result to get the previously returned
`taffy::Layout` value.
---------
Co-authored-by: Rob Parrett <robparrett@gmail.com>
# Objective
- Fixes#16122
When the wayland feature is not enabled, xwayland is used on wayland.
Nvidia drivers are somewhat bugged on linux and return outdated surfaces
on xwayland for seemingly no reason. Oftentimes at startup we get into
an infine loop where the surface is permanently outdated and nothing (or
sometimes only the first frame) is drawn on the screen.
## Solution
After experimenting I found that we can safely call configure again and
the issue seems to resolve itsef. After this change I couldn't reproduce
the original issue after many tries. More testing is probably needed
though.
The main issue is that `get_current_texture` fails sometimes because the
surface remains outdated even after configuring. It would be better to
just properly handle and never panic when `get_current_texture` fails.
This way we always call configure when outdated and bail when getting
the swapchain fails instead of crashing. The number of special cases is
also reduced.
## Testing
I tested the example "rotation" manually by trying to move around.
It works with X11 and Xwayland and the non panicing code paths didn't
change so other platforms aren't affected.
# Objective
- Display message for `AsBindGroupError::InvalidSamplerType` was not
correctly displaying the binding index
## Solution
- Simple typo fix
## Testing
- Tested locally
## Methodology
A good metric that correlates with compile time is the amount of code
generated by the compiler itself; even if the end binary is exactly the
same size, having more copies of the same code can really slow down
compile time, since it has to figure out whether it needs to include
them or not.
The measurement for this used was the [`cargo-llvm-lines`
crate](https://docs.rs/crate/cargo-llvm-lines) which can measure which
functions are generating the most lines of LLVM IR, which generally
means more code compiled. The example compiled was the `breakout` game,
to choose something that touches a decent portion of the engine.
## Solution
Based upon the measurements, `bevy_ptr::OwnedPtr::make` was taking up
4061 lines of LLVM IR in the example code. So, I separated part of this
function into a less-monomorphised version to reduce the amount of
generated code. This was by far the most lines emitted by any single
function.
## Results
After this change, only 2560 lines are emitted, accounting for a 36%
decrease. I tried timing the results and it seemed like it did decrease
compile times a bit, but honestly, the data is really noisy and I can't
be bothered to compile bevy for hours on end to get enough data points.
The tweak feels like an improvement, so, I'll offer it, however small.
# Objective
Remove `calculated_` from the name `ComputedNode::calculated_size` as
redundant, It's obvious from context that it's the resolved size value
and it's inconsistant since none of other fields of `ComputedNode` have
a `calculated_` prefix.
## Alternatives
Rename all the fields of `ComputedNode` to `calculated_*`, this seems
worse.
# Objective
Make the following functions `const` that will be useful to define
colors as constants.
- `Color::srgb_from_array`
- `Color::srgba_u8`
- `Color::srgb_u8`
The last two require Rust 1.82.0.
## Solution
- Make them `const`
- Change MSRV to 1.82.0
## Testing
I tested bevy_color only. My machine does not have enough RAM capacity
to test the whole bevy.
`cargo test -p bevy_color`
# Objective
Missed this in the required components PR review. `ContentSize` isn't
used by regular UI nodes, only those with intrinsically sized content
that needs a measure func.
## Solution
Remove `ContentSize` from `Node`'s required components and add it to the
required components of `Text` and `UiImage`.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
`UiImage` isn't just a general image component now, it's the defining
component for the image widget so it belongs in the image widget's
module.
# Objective
Order independent transparency can filter fragment writes based on the
alpha value and it is currently hard-coded to anything higher than 0.0.
By making that value configurable, users can optimize fragment writes,
potentially reducing the number of layers needed and improving
performance in favor of some transparency quality.
## Solution
This PR adds `alpha_threshold` to the
OrderIndependentTransparencySettings component and uses the struct to
configure a corresponding shader uniform. This uniform is then used
instead of the hard-coded value.
To configure OIT with a custom alpha threshold, use:
```rust
fn setup(mut commands: Commands) {
commands.spawn((
Camera3d::default(),
OrderIndependentTransparencySettings {
layer_count: 8,
alpha_threshold: 0.2,
},
));
}
```
## Testing
I tested this change using the included OIT example, as well as with two
additional projects.
## Migration Guide
If you previously explicitly initialized
OrderIndependentTransparencySettings with your own `layer_count`, you
will now have to add either a `..default()` statement or an explicit
`alpha_threshold` value:
```rust
fn setup(mut commands: Commands) {
commands.spawn((
Camera3d::default(),
OrderIndependentTransparencySettings {
layer_count: 16,
..default()
},
));
}
```
---------
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
# Objective
In `bevy_mod_picking` events are accessible through event listeners or
`EventReader`s. When I replaced event listeners with observers, I
removed the `EventReader` for simplicity. This adds it back.
## Solution
All picking events are now properly registered, and can be accessed
through `EventReader<Pointer<E>>`. `Pointer` now tracks the entity the
event targeted initially, and this can also be helpful in observers
(which don't currently do this).
## Testing
The picking examples run fine. This shouldn't really change anything.
---------
Co-authored-by: Aevyrie <aevyrie@gmail.com>
# Objective
- Mesh picking is noisy when a non triangle list is used
- Mesh picking runs even when users don't need it
- Resolve#16065
## Solution
- Don't add the mesh picking plugin by default
- Remove error spam
# Objective
- bevy_dev_tools 0.15.0-rc.1 failed to build docs
- it use bevy_text feature in bevy_ui but it's not enabled by default
- https://docs.rs/crate/bevy_dev_tools/0.15.0-rc.1
-
## Solution
- enable bevy_text feature of bevy_ui
# Objective
- `MeshPickingBackend` and `SpritePickingBackend` do not have the
`Plugin` suffix
- `DefaultPickingPlugins` is masquerading as a `Plugin` when in reality
it should be a `PluginGroup`
- Fixes#16081.
## Solution
- Rename some structures:
|Original Name|New Name|
|-|-|
|`MeshPickingBackend`|`MeshPickingPlugin`|
|`MeshPickingBackendSettings`|`MeshPickingSettings`|
|`SpritePickingBackend`|`SpritePickingPlugin`|
|`UiPickingBackendPlugin`|`UiPickingPlugin`|
- Make `DefaultPickingPlugins` a `PluginGroup`.
- Because `DefaultPickingPlugins` is within the `DefaultPlugins` plugin
group, I also added support for nested plugin groups to the
`plugin_group!` macro.
## Testing
- I used ripgrep to ensure all references were properly renamed.
- For the `plugin_group!` macro, I used `cargo expand` to manually
inspect the expansion of `DefaultPlugins`.
---
## Migration Guide
> [!NOTE]
>
> All 3 of the changed structures were added after 0.14, so this does
not need to be included in the 0.14 to 0.15 migration guide.
- `MeshPickingBackend` is now named `MeshPickingPlugin`.
- `MeshPickingBackendSettings` is now named `MeshPickingSettings`.
- `SpritePickingBackend` is now named `SpritePickingPlugin`.
- `UiPickingBackendPlugin` is now named `UiPickingPlugin`.
- `DefaultPickingPlugins` is now a a `PluginGroup` instead of a
`Plugin`.
# Objective
This PR introduces an `AsyncSeekForwardExt` trait, which I forgot in my
previous PR #14194.
This new trait is analogous to `AsyncSeekExt` and allows all
implementors of `AsyncSeekForward` to directly use the `seek_forward`
function in async contexts.
## Solution
- Implement a new `AsyncSeekForwardExt` trait
- Automatically implement this trait for all types that implement
`AsyncSeekForward`
## Showcase
This new trait allows a similar API to the previous Bevy version:
```rust
#[derive(Default)]
struct UniverseLoader;
#[derive(Asset, TypePath, Debug)]
struct JustALilAsteroid([u8; 128]);
impl AssetLoader for UniverseLoader {
type Asset = JustALilAsteroid;
type Settings = ();
type Error = std::io::Error;
async fn load<'a>(
&'a self,
reader: &'a mut Reader<'a>,
_settings: &'a Self::Settings,
_context: &'a mut LoadContext<'_>,
) -> Result<Self::Asset, Self::Error> {
// read the asteroids entry table
let entry_offset: u64 = /* ... */;
let current_offset: u64 = reader.seek_forward(0).await?;
// jump to the entry
reader.seek_forward(entry_offset - current_offset).await?;
let mut asteroid_buf = [0; 128];
reader.read_exact(&mut asteroid_buf).await?;
Ok(JustALilAsteroid(asteroid_buf))
}
fn extensions(&self) -> &[&str] {
&["celestial"]
}
}
```
The two additional linear texture samplers that PCSS added caused us to
blow past the limit on Apple Silicon macOS and WebGL. To fix the issue,
this commit adds a `--feature pbr_pcss` feature gate that disables PCSS
if not present.
Closes#15345.
Closes#15525.
Closes#15821.
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
The PCSS PR #13497 increased the size of clusterable objects from 64
bytes to 80 bytes but didn't decrease the UBO size to compensate, so we
blew past the 16kB limit on WebGL 2. This commit fixes the issue by
lowering the maximum number of clusterable objects to 204, which puts us
under the 16kB limit again.
Closes#15998.
# Objective
- Follow up on #16044
- `extract_uinode_borders` uses `bevy_hierarchy` directly instead of
going through the traversal utilities, meaning it won't handle
`GhostNode`s properly.
## Solution
- Replaced the use of `bevy_hierarchy::Parent` with
`UIChildren::get_parent`
## Testing
- Ran the `overflow` example, clipping looks ok.
---
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
1. Nodes with `Display::None` set are removed from the layout and have
no position or size. Outlines should not be drawn for a node with
`Display::None` set.
2. The outline and border colors are checked for transparency together.
If only one of the two is transparent, both will get queued.
3. The `node.is_empty()` check is insufficient to check if a border is
present since a non-zero sized node can have a zero width border.
## Solution
1. Add a check to `extract_uinode_borders` and ignore the node if
`Display::None` is set.
2. Filter the border and outline optional components by
`is_fully_transparent`.
3. Check if all the border widths are zero instead.
## Testing
I added dark cyan outlines around the left and right sections in the
`display_and_visibility` example. If you run the example and set the
outermost node to `Display::None` on the right, then you'll see the that
the outline on the left disappears.
# Objective
Fixes#16006
## Solution
We currently re-export `cosmic_text`, which is seemingly motivated by
the desire to use `cosmic_text::FontSystem` in `bevy_text` public APIs
instead of our `CosmicFontSystem` resource wrapper type.
This change makes `bevy_text` a "true" abstraction over `cosmic_text`
(it in fact, was already built to be that way generally and has this one
"leak").
This allows us to remove the `cosmic_text` re-export, which helps clean
up the Rust Analyzer imports and generally makes this a "cleaner" API.
# Objective
- Make the meshlet fill cluster buffers pass slightly faster
- Address https://github.com/bevyengine/bevy/issues/15920 for meshlets
- Added PreviousGlobalTransform as a required meshlet component to avoid
extra archetype moves, slightly alleviating
https://github.com/bevyengine/bevy/issues/14681 for meshlets
- Enforce that MeshletPlugin::cluster_buffer_slots is not greater than
2^25 (glitches will occur otherwise). Technically this field controls
post-lod/culling cluster count, and the issue is on pre-lod/culling
cluster count, but it's still valid now, and in the future this will be
more true.
Needs to be merged after https://github.com/bevyengine/bevy/pull/15846
and https://github.com/bevyengine/bevy/pull/15886
## Solution
- Old pass dispatched a thread per cluster, and did a binary search over
the instances to find which instance the cluster belongs to, and what
meshlet index within the instance it is.
- New pass dispatches a workgroup per instance, and has the workgroup
loop over all meshlets in the instance in order to write out the cluster
data.
- Use a push constant instead of arrayLength to fix the linked bug
- Remap 1d->2d dispatch for software raster only if actually needed to
save on spawning excess workgroups
## Testing
- Did you test these changes? If so, how?
- Ran the meshlet example, and an example with 1041 instances of 32217
meshlets per instance. Profiled the second scene with nsight, went from
0.55ms -> 0.40ms. Small savings. We're pretty much VRAM bandwidth bound
at this point.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- Run the meshlet example
## Changelog (non-meshlets)
- PreviousGlobalTransform now implements the Default trait
Take a bunch more improvements from @zeux's nanite.cpp code.
* Use position-only vertices (discard other attributes) to determine
meshlet connectivity for grouping
* Rather than using the lock borders flag when simplifying meshlet
groups, provide the locked vertices ourselves. The lock borders flag
locks the entire border of the meshlet group, but really we only want to
lock the edges between meshlet groups - outwards facing edges are fine
to unlock. This gives a really significant increase to the DAG quality.
* Add back stuck meshlets (group has only a single meshlet,
simplification failed) to the simplification queue to allow them to get
used later on and have another attempt at simplifying
* Target 8 meshlets per group instead of 4 (second biggest improvement
after manual locks)
* Provide a seed to metis for deterministic meshlet building
* Misc other improvements
We can remove the usage of unsafe after the next upstream meshopt
release, but for now we need to use the ffi function directly. I'll do
another round of improvements later, mainly attribute-aware
simplification and using spatial weights for meshlet grouping.
Need to merge https://github.com/bevyengine/bevy/pull/15846 first.
# Objective
- I made a mistake in #15902, specifically [this
diff](e2faedb99c)
-- the `point_light_count` variable is used for all point lights, not
just shadow mapped ones, so I cannot add `.min(max_texture_cubes)`
there. (Despite `spot_light_count` having `.min(..)`)
It may have broken code like this (where `index` is index of
`point_light` vec):
9930df83ed/crates/bevy_pbr/src/render/light.rs (L848-L850)
and also causes panic here:
9930df83ed/crates/bevy_pbr/src/render/light.rs (L1173-L1174)
## Solution
- Adds `.min(max_texture_cubes)` directly to the loop where texture
views for point lights are created.
## Testing
- `lighting` example (with the directional light removed; original
example doesn't crash as only 1 directional-or-spot light in total is
shadow-mapped on webgl) no longer crashes on webgl
# Objective
1. Prevent weird glitches with stray pixels scattered around the scene
2. Prevent weird glitchy full-screen triangles that pop-up and destroy
perf (SW rasterizing huge triangles is slow)
## Solution
1. Use floating point math in the SW rasterizer bounding box calculation
to handle negative verticss, and add backface culling
2. Force hardware raster for clusters that clip the near plane, and let
the hardware rasterizer handle the clipping
I also adjusted the SW rasterizer threshold to < 64 pixels (little bit
better perf in my test scene, but still need to do a more comprehensive
test), and enabled backface culling for the hardware raster pipeline.
## Testing
- Did you test these changes? If so, how?
- Yes, on an example scene. Issues no longer occur.
- Are there any parts that need more testing?
- No.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- Run the meshlet example.
# Objective
- bevy_remote Cargo.toml file references a readme that doesn't exist
- This is blocking releasing the rc
## Solution
- Remove the reference
# Objective
fixes#15502
Clipped borders and outlines aren't drawn correctly.
### Borders aren't clipped
Spawn two nodes with the same dimensions and border thickness, but clip
on of the nodes so that only its top left quarter is visible:
<img width="194" alt="clip"
src="https://github.com/user-attachments/assets/2d3f6d28-aa20-44df-967a-677725828294">
You can see that instead of clipping the border, instead the border is
scaled to fit inside of the unclipped section.
```rust
use bevy::color::palettes::css::BLUE;
use bevy::prelude::*;
use bevy::winit::WinitSettings;
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.insert_resource(WinitSettings::desktop_app())
.add_systems(Startup, setup)
.run();
}
fn setup(mut commands: Commands) {
commands.spawn(Camera2d);
commands
.spawn(Node {
width: Val::Percent(100.),
height: Val::Percent(100.),
justify_content: JustifyContent::Center,
align_items: AlignItems::Center,
..Default::default()
})
.with_children(|commands| {
commands
.spawn(Node {
column_gap: Val::Px(10.),
..Default::default()
})
.with_children(|commands| {
commands
.spawn(Node {
width: Val::Px(100.),
height: Val::Px(100.),
overflow: Overflow::clip(),
..Default::default()
})
.with_child((
Node {
position_type: PositionType::Absolute,
width: Val::Px(100.),
height: Val::Px(100.),
border: UiRect::all(Val::Px(10.)),
..Default::default()
},
BackgroundColor(Color::WHITE),
BorderColor(BLUE.into()),
));
commands
.spawn(Node {
width: Val::Px(50.),
height: Val::Px(50.),
overflow: Overflow::clip(),
..Default::default()
})
.with_child((
Node {
position_type: PositionType::Absolute,
width: Val::Px(100.),
height: Val::Px(100.),
border: UiRect::all(Val::Px(10.)),
..Default::default()
},
BackgroundColor(Color::WHITE),
BorderColor(BLUE.into()),
));
});
});
}
```
You can also see this problem in the `overflow` example. If you hover
over any of the clipped nodes you'll see that the outline only wraps the
visible section of the node
### Outlines are clipped incorrectly
A UI nodes Outline's are drawn outside of its bounds, so applying the
local clipping rect to the outline doesn't make any sense.
Instead an `Outline` should be clipped using its parent's clipping rect.
## Solution
* Pass the `point` value into the vertex shader instead of calculating
it in the shader.
* In `extract_uinode_borders` use the parents clipping rect when
clipping outlines.
The extra parameter isn't a great solution I think, but I wanted to fix
borders for the 0.15 release and this is the most minimal approach I
could think of without replacing the whole shader and prepare function.
## Showcase
<img width="149" alt="clipp"
src="https://github.com/user-attachments/assets/19fbd3cc-e7cd-42e1-a5e0-fd92aad04dcd">
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Less code
- Better iterator (implements `size_hint` for example)
## Solution
- Use `either`
- This change is free because `bevy_animation` depends on `bevy_asset`,
which already depends on `either`
## Testing
CI
# Objective
Bevy seems to want to standardize on "American English" spellings. Not
sure if this is laid out anywhere in writing, but see also #15947.
While perusing the docs for `typos`, I noticed that it has a `locale`
config option and tried it out.
## Solution
Switch to `en-us` locale in the `typos` config and run `typos -w`
## Migration Guide
The following methods or fields have been renamed from `*dependants*` to
`*dependents*`.
- `ProcessorAssetInfo::dependants`
- `ProcessorAssetInfos::add_dependant`
- `ProcessorAssetInfos::non_existent_dependants`
- `AssetInfo::dependants_waiting_on_load`
- `AssetInfo::dependants_waiting_on_recursive_dep_load`
- `AssetInfos::loader_dependants`
- `AssetInfos::remove_dependants_and_labels`
Use the new `disqualified` crate in `QueryEntityError` to make the error
message more readable.
---
## Showcase
Old:
QueryDoesNotMatch(0v1 with components my_game::main::foo::A,
my_game::main::foo::B, bevy_pbr::light::point_light::PointLight,
bevy_render::primitives::CubemapFrusta,
bevy_pbr::bundle::CubemapVisibleEntities,
bevy_transform::components::transform::Transform,
bevy_transform::components::global_transform::GlobalTransform,
bevy_render::view::visibility::Visibility,
bevy_render::view::visibility::InheritedVisibility,
bevy_render::view::visibility::ViewVisibility,
bevy_render::sync_world::SyncToRenderWorld)
New:
QueryDoesNotMatch(0v1 with components A, B, PointLight, CubemapFrusta,
CubemapVisibleEntities, Transform, GlobalTransform, Visibility,
InheritedVisibility, ViewVisibility, SyncToRenderWorld)
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Make compiler output more helpful when running `cargo check -p
bevy_mesh`. Currently it contains a lot of unreachable patterns/code
warnings due to features disabled by default.
## Solution
Mute the warnings.
## Testing
CI
# Objective
```
cargo check -p bevy_reflect
```
outputs a lot of warnings like:
```
warning: non-local `impl` definition, `impl` blocks should be written at the same level as their item
--> crates/bevy_reflect/src/impls/std.rs:223:13
|
223 | impl_type_path!($ty);
| ^-------------------
| |
| `TypePath` is not local
| move the `impl` block outside of this constant `_` and up 2 bodies
...
346 | / impl_reflect_for_atomic!(
347 | | ::core::sync::atomic::AtomicIsize,
| | --------------------------------- `AtomicIsize` is not local
348 | | ::core::sync::atomic::Ordering::SeqCst
349 | | );
| |_- in this macro invocation
|
= note: the macro `impl_type_path` defines the non-local `impl`, and may need to be changed
= note: the macro `impl_type_path` may come from an old version of the `bevy_reflect_derive` crate, try updating your dependency with `cargo update -p bevy_reflect_derive`
= note: an `impl` is never scoped, even when it is nested inside an item, as it may impact type checking outside of that item, which can be the case if neither the trait or the self type are at the same nesting level as the `impl`
= note: items in an anonymous const item (`const _: () = { ... }`) are treated as in the same scope as the anonymous const's declaration for the purpose of this lint
= note: `#[warn(non_local_definitions)]` on by default
= note: this warning originates in the macro `impl_type_path` which comes from the expansion of the macro `impl_reflect_for_atomic` (in Nightly builds, run with -Z macro-backtrace for more info)
```
## Solution
Move `impl_type_path!` into global scope. Warnings no longer pop up.
## Testing
CI
# Objective
Making work with `Indices` struct easier. Currently when building
indices in some quick-and-dirty code we need to do matches and handle
enum variants.
## Solution
`Indices::push` utility which works transparently with `U16` and `U32`
variants.
## Testing
Unit test added.
# Objective
Built-in observers & events should be `Reflect` so that components that
interact with them can be serialized in scenes. This is a similar pr to
#14259.
## Objective
Be able to depend on the crate for the types without bringing in
`smol-hyper` and other http dependencies.
## Solution
Create a new `HTTP` feature that is enabled by default.
# Objective
Make UI rendering optional.
Quite a few people have been experimenting with using Bevy UI for layout
and interaction but replacing the rendering with `bevy_prototype_lyon`
or whatever. It's awkward to do though as there is no way to disable the
existing UI rendering requiring users to create their own custom node
bundles and components. Also, you can't replace the UI's shader and a
number of other things.
This PR makes the setup and initialization of UI rendering for the
RenderApp optional. Then you can do whatever you want by replacing
`build_ui_render` with your own function. For instance, one that loads a
custom shader.
The UI layout and interaction components are still updated as normal.
## Solution
Add a field `enable_rendering` to `UiPlugin`.
Only call `build_ui_render` and initialize the `UiPipeline` if
`enable_rendering` is false.
I thought about implementing a "bevy_ui_render" feature but suspect
everything is too tightly coupled atm and it would be very fragile.
Similar to the struggles with the "bevy_text" feature but worse.
---
## Changelog
`UiPlugin`
* Added a bool field `enable_rendering`.
* Only calls `build_ui_render` and initializes the `UiPipeline` if
`enable_rendering` is true.
## Migration Guide
`UiPlugin` has a new field `enable_rendering`. If set to false, the UI's
rendering systems won't be added to the `RenderApp` and no UI elements
will be drawn. The layout and interaction components will still be
updated as normal.
# Objective
Adding a `WinitPlugin` to a `MinimalPlugins` setup is a bit tricky and
confusing due to having a terrible error message and no examples in the
repo.
## Solution
Document what you need to add.
# Objective
- Fixes https://github.com/bevyengine/bevy/issues/15871
(Camera is done in #15946)
## Solution
- Do the same as #15904 for other extraction systems
- Added missing `SyncComponentPlugin` for DOF, TAA, and SSAO
(According to the
[documentation](https://dev-docs.bevyengine.org/bevy/render/sync_component/struct.SyncComponentPlugin.html),
this plugin "needs to be added for manual extraction implementations."
We may need to check this is done.)
## Testing
Modified example locally to add toggles if not exist.
- [x] DOF - toggling DOF component and perspective in `depth_of_field`
example
- [x] TAA - toggling `Camera.is_active` and TAA component
- [x] clusters - not entirely sure, toggling `Camera.is_active` in
`many_lights` example (no crash/glitch even without this PR)
- [x] previous_view - toggling `Camera.is_active` in `skybox` (no
crash/glitch even without this PR)
- [x] lights - toggling `Visibility` of `DirectionalLight` in `lighting`
example
- [x] SSAO - toggling `Camera.is_active` and SSAO component in `ssao`
example
- [x] default UI camera view - toggling `Camera.is_active` (nop without
#15946 because UI defaults to some camera even if `DefaultCameraView` is
not there)
- [x] volumetric fog - toggling existence of volumetric light. Looks
like optimization, no change in behavior/visuals
# Objective
Switch to retained render world causes the extracted cameras in render
world to not be removed until camera in main world is despawned. When
extracting data from main world inactive cameras are skipped. Therefore
camera that was active and became inactive has a retained
`ExtractedCamera` component from previous frames (when it was active)
and is processed the same way as if it were active (there is no `active`
field on `ExtractedCamera`). This breakes switching between cameras in
`render_primitives` example.
Fixes#15822
## Solution
Fix it by removing `ExtractedCamera` and related components from
inactive cameras.
Note that despawning inactive camera seems to be bad option because they
are spawned using `SyncToRenderWorld` component.
## Testing
Switching camera in `render_primitives` example now works correctly.
---------
Co-authored-by: akimakinai <105044389+akimakinai@users.noreply.github.com>
# Objective
- The depth test was only using the final depth but it should be testing
each fragments
- Fully transparent fragments should not be added to the list
## Solution
- Test each fragment after sorting
## Testing
before:
TODO
after:
TODO
# Objective
- Fixes https://github.com/bevyengine/bevy/issues/13552
## Solution
- Thanks for the guidance from @DGriffin91, the current solution is to
transmit the light_map through the emissive channel to avoid increasing
the bandwidth of deferred shading.
- <del>Store lightmap sample result into G-Buffer and pass them into the
`Deferred Lighting Pipeline`, therefore we can get the correct indirect
lighting via the `apply_pbr_lighting` function.</del>
- <del>The original G-Buffer lacks storage for lightmap data, therefore
a new buffer is added. We can only use Rgba16Uint here due to the
32-byte limit on the render targets.</del>
## Testing
- Need to test all the examples that contains a prepass, with both the
forward and deferred rendering mode.
- I have tested the ones below.
- `lightmaps` (adjust the code based on the issue and check the
rendering result)
- `transmission` (it contains a prepass)
- `ssr` (it also uses the G-Bufffer)
- `meshlet` (forward and deferred)
- `pbr`
## Showcase
By updating the `lightmaps` example to use deferred rendering, this pull
request enables correct rendering result of the Cornell Box.
```
diff --git a/examples/3d/lightmaps.rs b/examples/3d/lightmaps.rs
index 564a3162b..11a748fba 100644
--- a/examples/3d/lightmaps.rs
+++ b/examples/3d/lightmaps.rs
@@ -1,12 +1,14 @@
//! Rendering a scene with baked lightmaps.
-use bevy::pbr::Lightmap;
+use bevy::core_pipeline::prepass::DeferredPrepass;
+use bevy::pbr::{DefaultOpaqueRendererMethod, Lightmap};
use bevy::prelude::*;
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.insert_resource(AmbientLight::NONE)
+ .insert_resource(DefaultOpaqueRendererMethod::deferred())
.add_systems(Startup, setup)
.add_systems(Update, add_lightmaps_to_meshes)
.run();
@@ -19,10 +21,12 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
..default()
});
- commands.spawn(Camera3dBundle {
- transform: Transform::from_xyz(-278.0, 273.0, 800.0),
- ..default()
- });
+ commands
+ .spawn(Camera3dBundle {
+ transform: Transform::from_xyz(-278.0, 273.0, 800.0),
+ ..default()
+ })
+ .insert(DeferredPrepass);
}
fn add_lightmaps_to_meshes(
```
<img width="1280" alt="image"
src="https://github.com/user-attachments/assets/17fd3367-61cc-4c23-b956-e7cfc751af3c">
## Emissive Issue
**The emissive light object appears incorrectly rendered because the
alpha channel of emission is set to 1 in deferred rendering and 0 in
forward rendering, leading to different emissive light result. Could
this be a bug?**
```wgsl
// pbr_deferred_functions.wgsl - pbr_input_from_deferred_gbuffer
let emissive = rgb9e5::rgb9e5_to_vec3_(gbuffer.g);
if ((pbr.material.flags & STANDARD_MATERIAL_FLAGS_UNLIT_BIT) != 0u) {
pbr.material.base_color = vec4(emissive, 1.0);
pbr.material.emissive = vec4(vec3(0.0), 1.0);
} else {
pbr.material.base_color = vec4(pow(base_rough.rgb, vec3(2.2)), 1.0);
pbr.material.emissive = vec4(emissive, 1.0);
}
// pbr_functions.wgsl - apply_pbr_lighting
emissive_light = emissive_light * mix(1.0, view_bindings::view.exposure, emissive.a);
```
---------
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
