# Objective
The definition of several `QueryState` methods use unnecessary explicit
lifetimes, which adds to visual noise.
## Solution
Elide the lifetimes.
# Objective
- bevy_sprite crate is missing docs for important types. `Sprite` being
undocumented was especially confusing for me even though it is one of
the first types I need to learn.
## Solution
- Improves the situation a little by adding some documentations.
I'm unsure about my understanding of functionality and writing. I'm
happy to be pointed out any mistakes.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Federico Rinaldi <gisquerin@gmail.com>
# Objective
`Instant` and `Duration` from the `instant` crate are exposed in
`bevy_utils` to have a single abstraction for native/wasm.
It would be useful to have the same thing for
[`SystemTime`](https://doc.rust-lang.org/std/time/struct.SystemTime.html).
---
## Changelog
### Added
- `bevy_utils` now re-exposes the `instant::SystemTime` struct
Co-authored-by: Charles Bournhonesque <cbournhonesque@snapchat.com>
# Objective
- Users are often confused when their command effects are not visible in
the next system. This PR auto inserts sync points if there are deferred
buffers on a system and there are dependents on that system (systems
with after relationships).
- Manual sync points can lead to users adding more than needed and it's
hard for the user to have a global understanding of their system graph
to know which sync points can be merged. However we can easily calculate
which sync points can be merged automatically.
## Solution
1. Add new edge types to allow opting out of new behavior
2. Insert an sync point for each edge whose initial node has deferred
system params.
3. Reuse nodes if they're at the number of sync points away.
* add opt outs for specific edges with `after_ignore_deferred`,
`before_ignore_deferred` and `chain_ignore_deferred`. The
`auto_insert_apply_deferred` boolean on `ScheduleBuildSettings` can be
set to false to opt out for the whole schedule.
## Perf
This has a small negative effect on schedule build times.
```text
group auto-sync main-for-auto-sync
----- ----------- ------------------
build_schedule/1000_schedule 1.06 2.8±0.15s ? ?/sec 1.00 2.7±0.06s ? ?/sec
build_schedule/1000_schedule_noconstraints 1.01 26.2±0.88ms ? ?/sec 1.00 25.8±0.36ms ? ?/sec
build_schedule/100_schedule 1.02 13.1±0.33ms ? ?/sec 1.00 12.9±0.28ms ? ?/sec
build_schedule/100_schedule_noconstraints 1.08 505.3±29.30µs ? ?/sec 1.00 469.4±12.48µs ? ?/sec
build_schedule/500_schedule 1.00 485.5±6.29ms ? ?/sec 1.00 485.5±9.80ms ? ?/sec
build_schedule/500_schedule_noconstraints 1.00 6.8±0.10ms ? ?/sec 1.02 6.9±0.16ms ? ?/sec
```
---
## Changelog
- Auto insert sync points and added `after_ignore_deferred`,
`before_ignore_deferred`, `chain_no_deferred` and
`auto_insert_apply_deferred` APIs to opt out of this behavior
## Migration Guide
- `apply_deferred` points are added automatically when there is ordering
relationship with a system that has deferred parameters like `Commands`.
If you want to opt out of this you can switch from `after`, `before`,
and `chain` to the corresponding `ignore_deferred` API,
`after_ignore_deferred`, `before_ignore_deferred` or
`chain_ignore_deferred` for your system/set ordering.
- You can also set `ScheduleBuildSettings::auto_insert_sync_points` to
`false` if you want to do it for the whole schedule. Note that in this
mode you can still add `apply_deferred` points manually.
- For most manual insertions of `apply_deferred` you should remove them
as they cannot be merged with the automatically inserted points and
might reduce parallelizability of the system graph.
## TODO
- [x] remove any apply_deferred used in the engine
- [x] ~~decide if we should deprecate manually using apply_deferred.~~
We'll still allow inserting manual sync points for now for whatever edge
cases users might have.
- [x] Update migration guide
- [x] rerun schedule build benchmarks
---------
Co-authored-by: Joseph <21144246+JoJoJet@users.noreply.github.com>
# Objective
- Finish the work done in #8942 .
## Solution
- Rebase the changes made in #8942 and fix the issues stopping it from
being merged earlier
---------
Co-authored-by: Thomas <1234328+thmsgntz@users.noreply.github.com>
# Objective
Make direction construction a bit more ergonomic.
## Solution
Add `Direction2d::from_xy` and `Direction3d::from_xyz`, similar to
`Transform::from_xyz`:
```rust
let dir2 = Direction2d::from_xy(0.5, 0.5).unwrap();
let dir3 = Direction3d::from_xyz(0.5, 0.5, 0.5).unwrap();
```
This can be a bit cleaner than using `new`:
```rust
let dir2 = Direction2d::new(Vec2::new(0.5, 0.5)).unwrap();
let dir3 = Direction3d::new(Vec3::new(0.5, 0.5, 0.5)).unwrap();
```
Fixes https://github.com/bevyengine/bevy/issues/10974
# Objective
Duplicate the ordering logic of the `Main` schedule into the `FixedMain`
schedule.
---
## Changelog
- `FixedUpdate` is no longer the main schedule ran in
`RunFixedUpdateLoop`, `FixedMain` has replaced this and has a similar
structure to `Main`.
## Migration Guide
- Usage of `RunFixedUpdateLoop` should be renamed to `RunFixedMainLoop`.
# Objective
Fixes#10968
## Solution
Pull startup schedules from a list of `ScheduleLabel`s in the same way
the update schedules are handled.
---
## Changelog
- Added `MainScheduleOrder::startup_labels` to allow the editing of the
startup schedule order.
## Migration Guide
- Added a new field to `MainScheduleOrder`, `startup_labels`, for
editing the startup schedule order.
# Objective
Keep up to date with wgpu.
## Solution
Update the wgpu version.
Currently blocked on naga_oil updating to naga 0.14 and releasing a new
version.
3d scenes (or maybe any scene with lighting?) currently don't render
anything due to
```
error: naga_oil bug, please file a report: composer failed to build a valid header: Type [2] '' is invalid
= Capability Capabilities(CUBE_ARRAY_TEXTURES) is required
```
I'm not sure what should be passed in for `wgpu::InstanceFlags`, or if we want to make the gles3minorversion configurable (might be useful for debugging?)
Currently blocked on https://github.com/bevyengine/naga_oil/pull/63, and https://github.com/gfx-rs/wgpu/issues/4569 to be fixed upstream in wgpu first.
## Known issues
Amd+windows+vulkan has issues with texture_binding_arrays (see the image [here](https://github.com/bevyengine/bevy/pull/10266#issuecomment-1819946278)), but that'll be fixed in the next wgpu/naga version, and you can just use dx12 as a workaround for now (Amd+linux mesa+vulkan texture_binding_arrays are fixed though).
---
## Changelog
Updated wgpu to 0.18, naga to 0.14.2, and naga_oil to 0.11.
- Windows desktop GL should now be less painful as it no longer requires Angle.
- You can now toggle shader validation and debug information for debug and release builds using `WgpuSettings.instance_flags` and [InstanceFlags](https://docs.rs/wgpu/0.18.0/wgpu/struct.InstanceFlags.html)
## Migration Guide
- `RenderPassDescriptor` `color_attachments` (as well as `RenderPassColorAttachment`, and `RenderPassDepthStencilAttachment`) now use `StoreOp::Store` or `StoreOp::Discard` instead of a `boolean` to declare whether or not they should be stored.
- `RenderPassDescriptor` now have `timestamp_writes` and `occlusion_query_set` fields. These can safely be set to `None`.
- `ComputePassDescriptor` now have a `timestamp_writes` field. This can be set to `None` for now.
- See the [wgpu changelog](https://github.com/gfx-rs/wgpu/blob/trunk/CHANGELOG.md#v0180-2023-10-25) for additional details
# Objective
- Make the implementation order consistent between all sources to fit
the order in the trait.
## Solution
- Change the implementation order.
# Objective
A workaround for a webgl issue was introduced in #9383 but one function
for mesh2d was missed.
## Solution
Applied the migration guide from #9383 in
`mesh2d_normal_local_to_world()
Note: I'm not using normals so I have not tested the bug & fix
# Objective
The documentation for `AnimationPlayer::play` mentions a non-existent
`transition_duration` argument from an old iteration of the API. It's
confusing.
## Solution
Remove the offending sentence.
# Objective
Since #10776 split `WorldQuery` to `WorldQueryData` and
`WorldQueryFilter`, it should be clear that the query is actually
composed of two parts. It is not factually correct to call "query" only
the data part. Therefore I suggest to rename the `Q` parameter to `D` in
`Query` and related items.
As far as I know, there shouldn't be breaking changes from renaming
generic type parameters.
## Solution
I used a combination of rust-analyzer go to reference and `Ctrl-F`ing
various patterns to catch as many cases as possible. Hopefully I got
them all. Feel free to check if you're concerned of me having missed
some.
## Notes
This and #10779 have many lines in common, so merging one will cause a
lot of merge conflicts to the other.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- The example in the docs is unsound.
Demo:
```rust
#[derive(Resource)]
struct MyRes(u32);
fn main() {
let mut w = World::new();
w.insert_resource(MyRes(0));
let (mut res, comp) = split_world_access(&mut w);
let mut r1 = res.get_resource_mut::<MyRes>().unwrap();
let mut r2 = res.get_resource_mut::<MyRes>().unwrap();
*r1 = MyRes(1);
*r2 = MyRes(2);
}
```
The API in the example allows aliasing mutable references to the same
resource. Miri also complains when running this.
## Solution
- Change the example API to make the returned `Mut` borrow from the
`OnlyResourceAccessWorld` instead of borrowing from the world via `'w`.
This prevents obtaining more than one `Mut` at the same time from it.
I didn't notice minus where vertices are generated, so could not
understand the order there.
Adding a comment to help the next person who is going to understand Bevy
by reading its code.
# Objective
- Reduce nesting in `process_handle_drop_internal()`.
- Related to #10896.
## Solution
- Use early returns when possible.
- Reduced from 9 levels of indents to 4.
# Objective
add `RenderLayers` awareness to lights. lights default to
`RenderLayers::layer(0)`, and must intersect the camera entity's
`RenderLayers` in order to affect the camera's output.
note that lights already use renderlayers to filter meshes for shadow
casting. this adds filtering lights per view based on intersection of
camera layers and light layers.
fixes#3462
## Solution
PointLights and SpotLights are assigned to individual views in
`assign_lights_to_clusters`, so we simply cull the lights which don't
match the view layers in that function.
DirectionalLights are global, so we
- add the light layers to the `DirectionalLight` struct
- add the view layers to the `ViewUniform` struct
- check for intersection before processing the light in
`apply_pbr_lighting`
potential issue: when mesh/light layers are smaller than the view layers
weird results can occur. e.g:
camera = layers 1+2
light = layers 1
mesh = layers 2
the mesh does not cast shadows wrt the light as (1 & 2) == 0.
the light affects the view as (1+2 & 1) != 0.
the view renders the mesh as (1+2 & 2) != 0.
so the mesh is rendered and lit, but does not cast a shadow.
this could be fixed (so that the light would not affect the mesh in that
view) by adding the light layers to the point and spot light structs,
but i think the setup is pretty unusual, and space is at a premium in
those structs (adding 4 bytes more would reduce the webgl point+spot
light max count to 240 from 256).
I think typical usage is for cameras to have a single layer, and
meshes/lights to maybe have multiple layers to render to e.g. minimaps
as well as primary views.
if there is a good use case for the above setup and we should support
it, please let me know.
---
## Migration Guide
Lights no longer affect all `RenderLayers` by default, now like cameras
and meshes they default to `RenderLayers::layer(0)`. To recover the
previous behaviour and have all lights affect all views, add a
`RenderLayers::all()` component to the light entity.
# Objective
Printing `DynamicStruct` with a debug format does not show the contained
type anymore. For instance, in `examples/reflection/reflection.rs`,
adding `dbg!(&reflect_value);` to line 96 will print:
```rust
[examples/reflection/reflection.rs:96] &reflect_value = DynamicStruct(bevy_reflect::DynamicStruct {
a: 4,
nested: DynamicStruct(bevy_reflect::DynamicStruct {
b: 8,
}),
})
```
## Solution
Show the represented type instead (`reflection::Foo` and
`reflection::Bar` in this case):
```rust
[examples/reflection/reflection.rs:96] &reflect_value = DynamicStruct(reflection::Foo {
a: 4,
nested: DynamicStruct(reflection::Bar {
b: 8,
}),
})
```
---------
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
# Objective
The `Despawn` command breaks the hierarchy whenever you use it if the
despawned entity has a parent or any children. This is a serious footgun
because the `Despawn` command has the shortest name, the behavior is
unexpected and not likely to be what you want, and the crash that it
causes can be very difficult to track down.
## Solution
Until this can be fixed by relations, add a note mentioning the footgun
in the documentation.
## Solution
`Commands.remove` and `.retain` (because I copied `remove`s doc)
referenced `EntityWorldMut.remove` and `retain` for more detail but the
`Commands` docs are much more detailed (which makes sense because it is
the most common api), so I have instead inverted this so that
`EntityWorldMut` docs link to `Commands`.
I also made `EntityWorldMut.despawn` reference `World.despawn` for more
details, like `Commands.despawn` does.
# Objective
Fixes#5891.
For mikktspace normal maps, normals must be renormalized in vertex
shaders to match the way mikktspace bakes vertex tangents and normal
maps so that the exact inverse process is applied when shading.
However, for invalid normals like `vec3<f32>(0.0, 0.0, 0.0)`, this
normalization causes NaN values, and because it's in the vertex shader,
it affects the entire triangle and causes it to be shaded as black:
*A cone with a tip that has a vertex normal of [0, 0, 0], causing the
mesh to be shaded as black.*
In some cases, normals of zero are actually *useful*. For example, a
smoothly shaded cone without creases requires the apex vertex normal to
be zero, because there is no singular normal that works correctly, so
the apex shouldn't contribute to the overall shading. Duplicate vertices
for the apex fix some shading issues, but it causes visible creases and
is more expensive. See #5891 and #10298 for more details.
For correctly shaded cones and other similar low-density shapes with
sharp tips, vertex normals of zero can not be normalized in the vertex
shader.
## Solution
Only normalize the vertex normals and tangents in the vertex shader if
the normal isn't [0, 0, 0]. This way, mikktspace normal maps should
still work for everything except the zero normals, and the zero normals
will only be normalized in the fragment shader.
This allows us to render cones correctly:
Notice how there is still a weird shadow banding effect in one area. I
noticed that it can be fixed by normalizing
[here](d2614f2d80/crates/bevy_pbr/src/render/pbr_functions.wgsl (L51)),
which produces a perfectly smooth cone without duplicate vertices:
I didn't add this change yet, because it seems a bit arbitrary. I can
add it here if that'd be useful or make another PR though.
# Objective
`update_accessibility_nodes` is one of the most nested functions in the
entire Bevy repository, with a maximum of 9 levels of indentations. This
PR refactors it down to 3 levels of indentations, while improving
readability on other fronts. The result is a function that is actually
understandable at a first glance.
- This is a proof of concept to demonstrate that it is possible to
gradually lower the nesting limit proposed by #10896.
PS: I read AccessKit's documentation, but I don't have experience with
it. Therefore, naming of variables and subroutines may be a bit off.
PS2: I don't know if the test suite covers the functionality of this
system, but since I've spent quite some time on it and the changes were
simple, I'm pretty confident the refactor is functionally equivalent to
the original.
## Solution
I documented each change with a commit, but as a summary I did the
following to reduce nesting:
- I moved from `if-let` blocks to `let-else` statements where
appropriate to reduce rightward shift
- I extracted the closure body to a new function `update_adapter`
- I factored out parts of `update_adapter` into new functions
`queue_node_for_update` and `add_children_nodes`
**Note for reviewers:** GitHub's diff viewer is not the greatest in
showing horizontal code shifts, therefore you may want to use a
different tool like VSCode to review some commits, especially the second
one (anyway, that commit is very straightforward, despite changing many
lines).
# Objective
A nodes outline should be clipped using its own clipping rect, not its
parents.
fixes#10921
## Solution
Clip outlines by the node's own clipping rect, not the parent's.
If you compare the `overflow` ui example in main with this PR, you'll
see that the outlines that appear when you hover above the images are
now clipped along with the images.
---
## Changelog
* Outlines are now clipped using the node's own clipping rect, not the
parent's.
# Objective
- Improve readability.
- Somewhat relates to #10896.
## Solution
- Use early returns to minimize nesting.
- Change `emitter_translation` to use `if let` instead of `map`.
# Objective
The `update_emitter_positions`, and `update_listener_positions` systems
are added for every call to `add_audio_source`.
Instead, add them once in the `AudioPlugin` directly.
Also merged the calls to `add_systems`.
Caught while working on my schedule visualizer c:
# Objective
Test more complex function signatures for exclusive systems, and test
that `StaticSystemParam` is indeed a `SystemParam`.
I mean, it currently works, but might as well add a test for it.
# Objective
- Resolves#10853
## Solution
- ~~Changed the name of `Input` struct to `PressableInput`.~~
- Changed the name of `Input` struct to `ButtonInput`.
## Migration Guide
- Breaking Change: Users need to rename `Input` to `ButtonInput` in
their projects.
# Objective
A better alternative version of #10843.
Currently, Bevy has a single `Ray` struct for 3D. To allow better
interoperability with Bevy's primitive shapes (#10572) and some third
party crates (that handle e.g. spatial queries), it would be very useful
to have separate versions for 2D and 3D respectively.
## Solution
Separate `Ray` into `Ray2d` and `Ray3d`. These new structs also take
advantage of the new primitives by using `Direction2d`/`Direction3d` for
the direction:
```rust
pub struct Ray2d {
pub origin: Vec2,
pub direction: Direction2d,
}
pub struct Ray3d {
pub origin: Vec3,
pub direction: Direction3d,
}
```
and by using `Plane2d`/`Plane3d` in `intersect_plane`:
```rust
impl Ray2d {
// ...
pub fn intersect_plane(&self, plane_origin: Vec2, plane: Plane2d) -> Option<f32> {
// ...
}
}
```
---
## Changelog
### Added
- `Ray2d` and `Ray3d`
- `Ray2d::new` and `Ray3d::new` constructors
- `Plane2d::new` and `Plane3d::new` constructors
### Removed
- Removed `Ray` in favor of `Ray3d`
### Changed
- `direction` is now a `Direction2d`/`Direction3d` instead of a vector,
which provides guaranteed normalization
- `intersect_plane` now takes a `Plane2d`/`Plane3d` instead of just a
vector for the plane normal
- `Direction2d` and `Direction3d` now derive `Serialize` and
`Deserialize` to preserve ray (de)serialization
## Migration Guide
`Ray` has been renamed to `Ray3d`.
### Ray creation
Before:
```rust
Ray {
origin: Vec3::ZERO,
direction: Vec3::new(0.5, 0.6, 0.2).normalize(),
}
```
After:
```rust
// Option 1:
Ray3d {
origin: Vec3::ZERO,
direction: Direction3d::new(Vec3::new(0.5, 0.6, 0.2)).unwrap(),
}
// Option 2:
Ray3d::new(Vec3::ZERO, Vec3::new(0.5, 0.6, 0.2))
```
### Plane intersections
Before:
```rust
let result = ray.intersect_plane(Vec2::X, Vec2::Y);
```
After:
```rust
let result = ray.intersect_plane(Vec2::X, Plane2d::new(Vec2::Y));
```
# Objective
Implement `TryFrom<Vec2>`/`TryFrom<Vec3>` for direction primitives as
considered in #10857.
## Solution
Implement `TryFrom` for the direction primitives.
These are all equivalent:
```rust
let dir2d = Direction2d::try_from(Vec2::new(0.5, 0.5)).unwrap();
let dir2d = Vec2::new(0.5, 0.5).try_into().unwrap(); // (assumes that the type is inferred)
let dir2d = Direction2d::new(Vec2::new(0.5, 0.5)).unwrap();
```
For error cases, an `Err(InvalidDirectionError)` is returned. It
contains the type of failure:
```rust
/// An error indicating that a direction is invalid.
#[derive(Debug, PartialEq)]
pub enum InvalidDirectionError {
/// The length of the direction vector is zero or very close to zero.
Zero,
/// The length of the direction vector is `std::f32::INFINITY`.
Infinite,
/// The length of the direction vector is `NaN`.
NaN,
}
```
# Objective
- Resolves#10784
## Solution
- As @ickshonpe mentioned in #10784, this is intended behavior but could
benefit from mentioning it in docs.
- I'm also thinking about adding a helper function to disable os scaling
such as `disable_os_scaling()`, but not sure if it's needed.
---
## Changelog
> Add a comment about scaling behavior that happens, and point user to
how he can avoid that behavior.
# Objective
Adds `EntityCommands.retain` and `EntityWorldMut.retain` to remove all
components except the given bundle from the entity.
Fixes#10865.
## Solution
I added a private unsafe function in `EntityWorldMut` called
`remove_bundle_info` which performs the shared behaviour of `remove` and
`retain`, namely taking a `BundleInfo` of components to remove, and
removing them from the given entity. Then `retain` simply gets all the
components on the entity and filters them by whether they are in the
bundle it was passed, before passing this `BundleInfo` into
`remove_bundle_info`.
`EntityCommands.retain` just creates a new type `Retain` which runs
`EntityWorldMut.retain` when run.
---
## Changelog
Added `EntityCommands.retain` and `EntityWorldMut.retain`, which remove
all components except the given bundle from the entity, they can also be
used to remove all components by passing `()` as the bundle.
# Objective
The name `TextAlignment` is really deceptive and almost every new user
gets confused about the differences between aligning text with
`TextAlignment`, aligning text with `Style` and aligning text with
anchor (when using `Text2d`).
## Solution
* Rename `TextAlignment` to `JustifyText`. The associated helper methods
are also renamed.
* Improve the doc comments for text explaining explicitly how the
`JustifyText` component affects the arrangement of text.
* Add some extra cases to the `text_debug` example that demonstate the
differences between alignment using `JustifyText` and alignment using
`Style`.
<img width="757" alt="text_debug_2"
src="https://github.com/bevyengine/bevy/assets/27962798/9d53e647-93f9-4bc7-8a20-0d9f783304d2">
---
## Changelog
* `TextAlignment` has been renamed to `JustifyText`
* `TextBundle::with_text_alignment` has been renamed to
`TextBundle::with_text_justify`
* `Text::with_alignment` has been renamed to `Text::with_justify`
* The `text_alignment` field of `TextMeasureInfo` has been renamed to
`justification`
## Migration Guide
* `TextAlignment` has been renamed to `JustifyText`
* `TextBundle::with_text_alignment` has been renamed to
`TextBundle::with_text_justify`
* `Text::with_alignment` has been renamed to `Text::with_justify`
* The `text_alignment` field of `TextMeasureInfo` has been renamed to
`justification`
# Objective
- Fixes#10806
## Solution
Replaced `new` and `index` methods for both `TableRow` and `TableId`
with `from_*` and `as_*` methods. These remove the need to perform
casting at call sites, reducing the total number of casts in the Bevy
codebase. Within these methods, an appropriate `debug_assertion` ensures
the cast will behave in an expected manner (no wrapping, etc.). I am
using a `debug_assertion` instead of an `assert` to reduce any possible
runtime overhead, however minimal. This choice is something I am open to
changing (or leaving up to another PR) if anyone has any strong
arguments for it.
---
## Changelog
- `ComponentSparseSet::sparse` stores a `TableRow` instead of a `u32`
(private change)
- Replaced `TableRow::new` and `TableRow::index` methods with
`TableRow::from_*` and `TableRow::as_*`, with `debug_assertions`
protecting any internal casting.
- Replaced `TableId::new` and `TableId::index` methods with
`TableId::from_*` and `TableId::as_*`, with `debug_assertions`
protecting any internal casting.
- All `TableId` methods are now `const`
## Migration Guide
- `TableRow::new` -> `TableRow::from_usize`
- `TableRow::index` -> `TableRow::as_usize`
- `TableId::new` -> `TableId::from_usize`
- `TableId::index` -> `TableId::as_usize`
---
## Notes
I have chosen to remove the `index` and `new` methods for the following
chain of reasoning:
- Across the codebase, `new` was called with a mixture of `u32` and
`usize` values. Likewise for `index`.
- Choosing `new` to either be `usize` or `u32` would break half of these
call-sites, requiring `as` casting at the site.
- Adding a second method `new_u32` or `new_usize` avoids the above, bu
looks visually inconsistent.
- Therefore, they should be replaced with `from_*` and `as_*` methods
instead.
Worth noting is that by updating `ComponentSparseSet`, there are now
zero instances of interacting with the inner value of `TableRow` as a
`u32`, it is exclusively used as a `usize` value (due to interactions
with methods like `len` and slice indexing). I have left the `as_u32`
and `from_u32` methods as the "proper" constructors/getters.
This removes the `From<Vec2/3>` implementations for the direction types.
It doesn't seem right to have when it only works if the vector is
nonzero and finite and produces NaN otherwise.
Added `Direction2d/3d::new` which uses `Vec2/3::try_normalize` to
guarantee it returns either a valid direction or `None`.
This should make it impossible to create an invalid direction, which I
think was the intention with these types.
# Objective
Fixes#10291
This adds a way to easily log messages once within system which are
called every frame.
## Solution
Opted for a macro-based approach. The fact that the 'once' call is
tracked per call site makes the `log_once!()` macro very versatile and
easy-to-use. I suspect it will be very handy for all of us, but
especially beginners, to get some initial feedback from systems without
spamming up the place!
I've made the macro's return its internal `has_fired` state, for
situations in which that might be useful to know (trigger something else
alongside the log, for example).
Please let me know if I placed the macro's in the right location, and if
you would like me to do something more clever with the macro's
themselves, since its looking quite copy-pastey at the moment. I've
tried ways to replace 5 with 1 macro's, but no success yet.
One downside of this approach is: Say you wish to warn the user if a
resource is invalid. In this situation, the
`resource.is_valid()` check would still be performed every frame:
```rust
fn my_system(my_res: Res<MyResource>) {
if !my_res.is_valid() {
warn_once!("resource is invalid!");
}
}
```
If you want to prevent that, you would still need to introduce a local
boolean. I don't think this is a very big deal, as expensive checks
shouldn't be called every frame in any case.
## Changelog
Added: `trace_once!()`, `debug_once!()`, `info_once!()`, `warn_once!()`,
and `error_once!()` log macros which fire only once per call site.
# Objective
Resolves Issue #10772.
## Solution
Added the deprecated warning for QueryState::for_each_unchecked, as
noted in the comments of PR #6773.
Followed the wording in the deprecation messages for `for_each` and
`for_each_mut`
# Objective
After #6547, `Query::for_each` has been capable of automatic
vectorization on certain queries, which is seeing a notable (>50% CPU
time improvements) for iteration. However, `Query::for_each` isn't
idiomatic Rust, and lacks the flexibility of iterator combinators.
Ideally, `Query::iter` and friends should be able to achieve the same
results. However, this does seem to blocked upstream
(rust-lang/rust#104914) by Rust's loop optimizations.
## Solution
This is an intermediate solution and refactor. This moves the
`Query::for_each` implementation onto the `Iterator::fold`
implementation for `QueryIter` instead. This should result in the same
automatic vectorization optimization on all `Iterator` functions that
internally use fold, including `Iterator::for_each`, `Iterator::count`,
etc.
With this, it should close the gap between the two completely.
Internally, this PR changes `Query::for_each` to use
`query.iter().for_each(..)` instead of the duplicated implementation.
Separately, the duplicate implementations of internal iteration (i.e.
`Query::par_for_each`) now use portions of the current `Query::for_each`
implementation factored out into their own functions.
This also massively cleans up our internal fragmentation of internal
iteration options, deduplicating the iteration code used in `for_each`
and `par_iter().for_each()`.
---
## Changelog
Changed: `Query::for_each`, `Query::for_each_mut`, `Query::for_each`,
and `Query::for_each_mut` have been moved to `QueryIter`'s
`Iterator::for_each` implementation, and still retains their performance
improvements over normal iteration. These APIs are deprecated in 0.13
and will be removed in 0.14.
---------
Co-authored-by: JoJoJet <21144246+JoJoJet@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
when loading gltfs we may want to filter the results. in particular, i
need to be able to exclude cameras.
i can do this already by modifying the gltf after load and before
spawning, but it seems like a useful general option.
## Solution
add `GltfLoaderSettings` struct with bool members:
- `load_cameras` : checked before processing camera nodes.
- `load_lights` : checked before processing light nodes
- `load_meshes` : checked before loading meshes, materials and morph
weights
Existing code will work as before. Now you also have the option to
restrict what parts of the gltf are loaded. For example, to load a gltf
but exclude the cameras, replace a call to
`asset_server.load("my.gltf")` with:
```rust
asset_server.load_with_settings(
"my.gltf",
|s: &mut GltfLoaderSettings| {
s.load_cameras = false;
}
);
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fixes#10444
Currently manually removing an asset prevents it from being reloaded
while there are still active handles. Doing so will result in a panic,
because the storage entry has been marked as "empty / None" but the ID
is still assumed to be active by the asset server.
Patterns like `images.remove() -> asset_server.reload()` and
`images.remove() -> images.insert()` would fail if the handle was still
alive.
## Solution
Most of the groundwork for this was already laid in Bevy Asset V2. This
is largely just a matter of splitting out `remove` into two separate
operations:
* `remove_dropped`: remove the stored asset, invalidate the internal
Assets entry (preventing future insertions with the old id), and recycle
the id
* `remove_still_alive`: remove the stored asset, but leave the entry
otherwise untouched (and dont recycle the id).
`remove_still_alive` and `insert` can be called any number of times (in
any order) for an id until `remove_dropped` has been called, which will
invalidate the id.
From a user-facing perspective, there are no API changes and this is non
breaking. The public `Assets::remove` will internally call
`remove_still_alive`. `remove_dropped` can only be called by the
internal "handle management" system.
---
## Changelog
- Fix a bug preventing `Assets::remove` from blocking future inserts for
a specific `AssetIndex`.
# Objective
Fixes https://github.com/bevyengine/bevy/issues/10786
## Solution
The bind_group_layout entries for the prepass were wrong when not all 4
prepass textures were used, as it just zipped [17, 18, 19, 20] with the
smallvec of prepass `bind_group_layout` entries that potentially didn't
contain 4 entries. (eg. if you had a depth and motion vector prepass but
no normal prepass, then depth would be correct but the entry for the
motion vector prepass would be 18 (normal prepass' spot) instead of 19).
Change the prepass `get_bind_group_layout_entries` function to return an
array of `[Option<BindGroupLayoutEntryBuilder>; 4]` and only add the
layout entry if it exists.
# Objective
avoid panics from `calculate_bounds` systems if entities are despawned
in PostUpdate.
there's a running general discussion (#10166) about command panicking.
in the meantime we may as well fix up some cases where it's clear a
failure to insert is safe.
## Solution
change `.insert(aabb)` to `.try_insert(aabb)`
# Objective
> Issue raised on
[Discord](https://discord.com/channels/691052431525675048/1002362493634629796/1179182488787103776)
Currently the following code fails due to a missing `TypePath` bound:
```rust
#[derive(Reflect)] struct Foo<T>(T);
#[derive(Reflect)] struct Bar<T>(Foo<T>);
#[derive(Reflect)] struct Baz<T>(Bar<Foo<T>>);
```
## Solution
Add `TypePath` to the per-field bounds instead of _just_ the generic
type parameter bounds.
### Related Work
It should be noted that #9046 would help make these kinds of issues
easier to work around and/or avoid entirely.
---
## Changelog
- Fixes missing `TypePath` requirement when deriving `Reflect` on nested
generics
# Objective
Fixes#10401
## Solution
* Allow sources to register specific processed/unprocessed watch
warnings.
* Specify per-platform watch warnings. This removes the need to cover
all platform cases in one warning message.
* Only register watch warnings for the _processed_ embedded source, as
warning about watching unprocessed embedded isn't helpful.
---
## Changelog
- Asset sources can now register specific watch warnings.
# Objective
- Shorten paths by removing unnecessary prefixes
## Solution
- Remove the prefixes from many paths which do not need them. Finding
the paths was done automatically using built-in refactoring tools in
Jetbrains RustRover.
# Objective
These type are unavailable to editors and scripting interfaces making
use of reflection.
## Solution
`#[derive(Reflect)]` and call `.register_type` during plugin
initialization.
---
## Changelog
### Added
- Implement `Reflect` for audio-related types, and register them.
# Objective
- Materials should be a more frequent rebind then meshes (due to being
able to use a single vertex buffer, such as in #10164) and therefore
should be in a higher bind group.
---
## Changelog
- For 2d and 3d mesh/material setups (but not UI materials, or other
rendering setups such as gizmos, sprites, or text), mesh data is now in
bind group 1, and material data is now in bind group 2, which is swapped
from how they were before.
## Migration Guide
- Custom 2d and 3d mesh/material shaders should now use bind group 2
`@group(2) @binding(x)` for their bound resources, instead of bind group
1.
- Many internal pieces of rendering code have changed so that mesh data
is now in bind group 1, and material data is now in bind group 2.
Semi-custom rendering setups (that don't use the Material or Material2d
APIs) should adapt to these changes.
# Objective
Keep essentially the same structure of `EntityHasher` from #9903, but
rephrase the multiplication slightly to save an instruction.
cc @superdump
Discord thread:
https://discord.com/channels/691052431525675048/1172033156845674507/1174969772522356756
## Solution
Today, the hash is
```rust
self.hash = i | (i.wrapping_mul(FRAC_U64MAX_PI) << 32);
```
with `i` being `(generation << 32) | index`.
Expanding things out, we get
```rust
i | ( (i * CONST) << 32 )
= (generation << 32) | index | ((((generation << 32) | index) * CONST) << 32)
= (generation << 32) | index | ((index * CONST) << 32) // because the generation overflowed
= (index * CONST | generation) << 32 | index
```
What if we do the same thing, but with `+` instead of `|`? That's almost
the same thing, except that it has carries, which are actually often
better in a hash function anyway, since it doesn't saturate. (`|` can be
dangerous, since once something becomes `-1` it'll stay that, and
there's no mixing available.)
```rust
(index * CONST + generation) << 32 + index
= (CONST << 32 + 1) * index + generation << 32
= (CONST << 32 + 1) * index + (WHATEVER << 32 + generation) << 32 // because the extra overflows and thus can be anything
= (CONST << 32 + 1) * index + ((CONST * generation) << 32 + generation) << 32 // pick "whatever" to be something convenient
= (CONST << 32 + 1) * index + ((CONST << 32 + 1) * generation) << 32
= (CONST << 32 + 1) * index +((CONST << 32 + 1) * (generation << 32)
= (CONST << 32 + 1) * (index + generation << 32)
= (CONST << 32 + 1) * (generation << 32 | index)
= (CONST << 32 + 1) * i
```
So we can do essentially the same thing using a single multiplication
instead of doing multiply-shift-or.
LLVM was already smart enough to merge the shifting into a
multiplication, but this saves the extra `or`:
<https://rust.godbolt.org/z/MEvbz4eo4>
It's a very small change, and often will disappear in load latency
anyway, but it's a couple percent faster in lookups:
(There was more of an improvement here before #10558, but with `to_bits`
being a single `qword` load now, keeping things mostly as it is turned
out to be better than the bigger changes I'd tried in #10605.)
---
## Changelog
(Probably skip it)
## Migration Guide
(none needed)
# Objective
the pbr prepass vertex shader currently only sets
`VertexOutput::world_position` when deferred or motion prepasses are
enabled.
the field is always in the vertex output so is otherwise undetermined,
and the calculation is very cheap.
## Solution
always set the world position in the pbr prepass vert shader.
# Objective
`GlobalsUniform` provides the current time to shaders, which is useful
for animations. `UiMaterial` is an abstraction that makes it easier to
write custom shaders for UI elements.
This PR makes it possible to use the `GlobalsUniform` in `UiMaterial`
shaders.
## Solution
The `GlobalsUniform` is bound to `@group(0) @binding(1)`. It is
accessible in shaders with:
```wgsl
#import bevy_render::globals::Globals
@group(0) @binding(1)
var<uniform> globals: Globals;
```
---
## Changelog
Added `GlobalsUniform` in `UiMaterial` shaders
## Discussion
Should I modify the existing ui_material example to showcase this?
# Objective
Related to #10612.
Enable the
[`clippy::manual_let_else`](https://rust-lang.github.io/rust-clippy/master/#manual_let_else)
lint as a warning. The `let else` form seems more idiomatic to me than a
`match`/`if else` that either match a pattern or diverge, and from the
clippy doc, the lint doesn't seem to have any possible false positive.
## Solution
Add the lint as warning in `Cargo.toml`, refactor places where the lint
triggers.
# Objective
- Follow up to #9694
## Solution
- Same api as #9694 but adapted for `BindGroupLayoutEntry`
- Use the same `ShaderStages` visibilty for all entries by default
- Add `BindingType` helper function that mirror the wgsl equivalent and
that make writing layouts much simpler.
Before:
```rust
let layout = render_device.create_bind_group_layout(&BindGroupLayoutDescriptor {
label: Some("post_process_bind_group_layout"),
entries: &[
BindGroupLayoutEntry {
binding: 0,
visibility: ShaderStages::FRAGMENT,
ty: BindingType::Texture {
sample_type: TextureSampleType::Float { filterable: true },
view_dimension: TextureViewDimension::D2,
multisampled: false,
},
count: None,
},
BindGroupLayoutEntry {
binding: 1,
visibility: ShaderStages::FRAGMENT,
ty: BindingType::Sampler(SamplerBindingType::Filtering),
count: None,
},
BindGroupLayoutEntry {
binding: 2,
visibility: ShaderStages::FRAGMENT,
ty: BindingType::Buffer {
ty: bevy::render::render_resource::BufferBindingType::Uniform,
has_dynamic_offset: false,
min_binding_size: Some(PostProcessSettings::min_size()),
},
count: None,
},
],
});
```
After:
```rust
let layout = render_device.create_bind_group_layout(
"post_process_bind_group_layout"),
&BindGroupLayoutEntries::sequential(
ShaderStages::FRAGMENT,
(
texture_2d_f32(),
sampler(SamplerBindingType::Filtering),
uniform_buffer(false, Some(PostProcessSettings::min_size())),
),
),
);
```
Here's a more extreme example in bevy_solari:
86dab7f5da
---
## Changelog
- Added `BindGroupLayoutEntries` and all `BindingType` helper functions.
## Migration Guide
`RenderDevice::create_bind_group_layout()` doesn't take a
`BindGroupLayoutDescriptor` anymore. You need to provide the parameters
separately
```rust
// 0.12
let layout = render_device.create_bind_group_layout(&BindGroupLayoutDescriptor {
label: Some("post_process_bind_group_layout"),
entries: &[
BindGroupLayoutEntry {
// ...
},
],
});
// 0.13
let layout = render_device.create_bind_group_layout(
"post_process_bind_group_layout",
&[
BindGroupLayoutEntry {
// ...
},
],
);
```
## TODO
- [x] implement a `Dynamic` variant
- [x] update the `RenderDevice::create_bind_group_layout()` api to match
the one from `RenderDevice::creat_bind_group()`
- [x] docs
# Objective
- Fixes#7680
- This is an updated for https://github.com/bevyengine/bevy/pull/8899
which had the same objective but fell a long way behind the latest
changes
## Solution
The traits `WorldQueryData : WorldQuery` and `WorldQueryFilter :
WorldQuery` have been added and some of the types and functions from
`WorldQuery` has been moved into them.
`ReadOnlyWorldQuery` has been replaced with `ReadOnlyWorldQueryData`.
`WorldQueryFilter` is safe (as long as `WorldQuery` is implemented
safely).
`WorldQueryData` is unsafe - safely implementing it requires that
`Self::ReadOnly` is a readonly version of `Self` (this used to be a
safety requirement of `WorldQuery`)
The type parameters `Q` and `F` of `Query` must now implement
`WorldQueryData` and `WorldQueryFilter` respectively.
This makes it impossible to accidentally use a filter in the data
position or vice versa which was something that could lead to bugs.
~~Compile failure tests have been added to check this.~~
It was previously sometimes useful to use `Option<With<T>>` in the data
position. Use `Has<T>` instead in these cases.
The `WorldQuery` derive macro has been split into separate derive macros
for `WorldQueryData` and `WorldQueryFilter`.
Previously it was possible to derive both `WorldQuery` for a struct that
had a mixture of data and filter items. This would not work correctly in
some cases but could be a useful pattern in others. *This is no longer
possible.*
---
## Notes
- The changes outside of `bevy_ecs` are all changing type parameters to
the new types, updating the macro use, or replacing `Option<With<T>>`
with `Has<T>`.
- All `WorldQueryData` types always returned `true` for `IS_ARCHETYPAL`
so I moved it to `WorldQueryFilter` and
replaced all calls to it with `true`. That should be the only logic
change outside of the macro generation code.
- `Changed<T>` and `Added<T>` were being generated by a macro that I
have expanded. Happy to revert that if desired.
- The two derive macros share some functions for implementing
`WorldQuery` but the tidiest way I could find to implement them was to
give them a ton of arguments and ask clippy to ignore that.
## Changelog
### Changed
- Split `WorldQuery` into `WorldQueryData` and `WorldQueryFilter` which
now have separate derive macros. It is not possible to derive both for
the same type.
- `Query` now requires that the first type argument implements
`WorldQueryData` and the second implements `WorldQueryFilter`
## Migration Guide
- Update derives
```rust
// old
#[derive(WorldQuery)]
#[world_query(mutable, derive(Debug))]
struct CustomQuery {
entity: Entity,
a: &'static mut ComponentA
}
#[derive(WorldQuery)]
struct QueryFilter {
_c: With<ComponentC>
}
// new
#[derive(WorldQueryData)]
#[world_query_data(mutable, derive(Debug))]
struct CustomQuery {
entity: Entity,
a: &'static mut ComponentA,
}
#[derive(WorldQueryFilter)]
struct QueryFilter {
_c: With<ComponentC>
}
```
- Replace `Option<With<T>>` with `Has<T>`
```rust
/// old
fn my_system(query: Query<(Entity, Option<With<ComponentA>>)>)
{
for (entity, has_a_option) in query.iter(){
let has_a:bool = has_a_option.is_some();
//todo!()
}
}
/// new
fn my_system(query: Query<(Entity, Has<ComponentA>)>)
{
for (entity, has_a) in query.iter(){
//todo!()
}
}
```
- Fix queries which had filters in the data position or vice versa.
```rust
// old
fn my_system(query: Query<(Entity, With<ComponentA>)>)
{
for (entity, _) in query.iter(){
//todo!()
}
}
// new
fn my_system(query: Query<Entity, With<ComponentA>>)
{
for entity in query.iter(){
//todo!()
}
}
// old
fn my_system(query: Query<AnyOf<(&ComponentA, With<ComponentB>)>>)
{
for (entity, _) in query.iter(){
//todo!()
}
}
// new
fn my_system(query: Query<Option<&ComponentA>, Or<(With<ComponentA>, With<ComponentB>)>>)
{
for entity in query.iter(){
//todo!()
}
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Kind of helps #10509
## Solution
Add a line to `prepass.wgsl` that ensure the `instance_index` push
constant is always used on WebGL 2. This is not a full fix, as the
_second_ a custom shader is used that doesn't use the push constant, the
breakage will resurface. We have satisfying medium term and long term
solutions. This is just a short term hack for 0.12.1 that will make more
cases work. See #10509 for more details.
# Objective
- `insert_reflect` relies on `reflect_type_path`, which doesn't gives
the actual type path for object created by `clone_value`, leading to an
unexpected panic. This is a workaround for it.
- Fix#10590
## Solution
- Tries to get type path from `get_represented_type_info` if get failed
from `reflect_type_path`.
---
## Defect remaining
- `get_represented_type_info` implies a shortage on performance than
using `TypeRegistry`.
# Objective
Fixes#10688
There were a number of issues at play:
1. The GLTF loader was not registering Scene dependencies properly. They
were being registered at the root instead of on the scene assets. This
made `LoadedWithDependencies` fire immediately on load.
2. Recursive labeled assets _inside_ of labeled assets were not being
loaded. This only became relevant for scenes after fixing (1) because we
now add labeled assets to the nested scene `LoadContext` instead of the
root load context. I'm surprised nobody has hit this yet. I'm glad I
caught it before somebody hit it.
3. Accessing "loaded with dependencies" state on the Asset Server is
boilerplatey + error prone (because you need to manually query two
states).
## Solution
1. In GltfLoader, use a nested LoadContext for scenes and load
dependencies through that context.
2. In the `AssetServer`, load labeled assets recursively.
3. Added a simple `asset_server.is_loaded_with_dependencies(id)`
I also added some docs to `LoadContext` to help prevent this problem in
the future.
---
## Changelog
- Added `AssetServer::is_loaded_with_dependencies`
- Fixed GLTF Scene dependencies
- Fixed nested labeled assets not being loaded
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Fixes#10676, preventing a possible memory leak for commands which
owned resources.
## Solution
Implemented `Drop` for `CommandQueue`. This has been done entirely in
the private API of `CommandQueue`, ensuring no breaking changes. Also
added a unit test, `test_command_queue_inner_drop_early`, based on the
reproduction steps as outlined in #10676.
## Notes
I believe this can be applied to `0.12.1` as well, but I am uncertain of
the process to make that kind of change. Please let me know if there's
anything I can do to help with the back-porting of this change.
---------
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Fixes#10157
## Solution
Add `AssetMetaCheck` resource which can configure when/if asset meta
files will be read:
```rust
app
// Never attempts to look up meta files. The default meta configuration will be used for each asset.
.insert_resource(AssetMetaCheck::Never)
.add_plugins(DefaultPlugins)
```
This serves as a band-aid fix for the issue with wasm's
`HttpWasmAssetReader` creating a bunch of requests for non-existent
meta, which can slow down asset loading (by waiting for the 404
response) and creates a bunch of noise in the logs. This also provides a
band-aid fix for the more serious issue of itch.io deployments returning
403 responses, which results in full failure of asset loads.
If users don't want to include meta files for all deployed assets for
web builds, and they aren't using meta files at all, they should set
this to `AssetMetaCheck::Never`.
If users do want to include meta files for specific assets, they can use
`AssetMetaCheck::Paths`, which will only look up meta for those paths.
Currently, this defaults to `AssetMetaCheck::Always`, which makes this
fully non-breaking for the `0.12.1` release. _**However it _is_ worth
discussing making this `AssetMetaCheck::Never` by default**_, given that
I doubt most people are using meta files without the Asset Processor
enabled. This would be a breaking change, but it would make WASM / Itch
deployments work by default, which is a pretty big win imo. The downside
is that people using meta files _without_ processing would need to
manually enable `AssetMetaCheck::Always`, which is also suboptimal.
When in `AssetMetaCheck::Processed`, the meta check resource is ignored,
as processing requires asset meta files to function.
In general, I don't love adding this knob as things should ideally "just
work" in all cases. But this is the reality of the current situation.
---
## Changelog
- Added `AssetMetaCheck` resource, which can configure when/if asset
meta files will be read
# Objective
Resolves#10743.
## Solution
Copied over the documentation written by @stepancheng from PR #10718.
I left out the lines from the doctest where `<()>` is removed, as that
seemed to be the part people couldn't decide on whether to keep or not.
## Objective
Currently, events are dropped after two frames. This cadence wasn't
*chosen* for a specific reason, double buffering just lets events
persist for at least two frames. Events only need to be dropped at a
predictable point so that the event queues don't grow forever (i.e.
events should never cause a memory leak).
Events (and especially input events) need to be observable by systems in
`FixedUpdate`, but as-is events are dropped before those systems even
get a chance to see them.
## Solution
Instead of unconditionally dropping events in `First`, require
`FixedUpdate` to first queue the buffer swap (if the `TimePlugin` has
been installed). This way, events are only dropped after a frame that
runs `FixedUpdate`.
## Future Work
In the same way we have independent copies of `Time` for tracking time
in `Main` and `FixedUpdate`, we will need independent copies of `Input`
for tracking press/release status correctly in `Main` and `FixedUpdate`.
--
Every run of `FixedUpdate` covers a specific timespan. For example, if
the fixed timestep `Δt` is 10ms, the first three `FixedUpdate` runs
cover `[0ms, 10ms)`, `[10ms, 20ms)`, and `[20ms, 30ms)`.
`FixedUpdate` can run many times in one frame. For truly
framerate-independent behavior, each `FixedUpdate` should only see the
events that occurred in its covered timespan, but what happens right now
is the first step in the frame reads all pending events.
Fixing that will require timestamped events.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
forward for bevy user to consume
# Objective
- since winit 0.27 an event WindowOccluded will be produced:
https://docs.rs/winit/latest/winit/event/enum.WindowEvent.html#variant.Occluded
- on ios this is currently the only way to know if an app comes back
from the background which is an important time to to things (like
resetting the badge)
## Solution
- pick up the winit event and forward it to a new `EventWriter`
---
## Changelog
### Added
- new Event `WindowOccluded` added allowing to hook into
`WindowEvent::Occluded` of winit
# Objective
I tried setting `ClearColorConfig` in my app via `Color::FOO.into()`
expecting it to work, but the impl was missing.
## Solution
- Add `impl From<Color> for ClearColorConfig`
- Change examples to use this impl
## Changelog
### Added
- `ClearColorConfig` can be constructed via `.into()` on a `Color`
---------
Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
# Objective
`wgpu` has a helper method `texture.as_image_copy()` for a common
pattern when making a default-like `ImageCopyTexture` from a texture.
This is used in various places in Bevy for texture copy operations, but
it was not used where `write_texture` is called.
## Solution
- Replace struct `ImageCopyTexture` initialization with
`texture.as_image_copy()` where appropriate
Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
# Objective
- Window size, scale and position are not correct on the first execution
of the systems
- Fixes#10407, fixes#10642
## Solution
- Don't run `update` before we get a chance to create the window in
winit
- Finish reverting #9826 after #10389
# Objective
The `generate_composite_uuid` utility function hidden in
`bevy_reflect::__macro_exports` could be generally useful to users.
For example, I previously relied on `Hash` to generate a `u64` to create
a deterministic `HandleId`. In v0.12, `HandleId` has been replaced by
`AssetId` which now requires a `Uuid`, which I could generate with this
function.
## Solution
Relocate `generate_composite_uuid` from `bevy_reflect::__macro_exports`
to `bevy_utils::uuid`.
It is still re-exported under `bevy_reflect::__macro_exports` so there
should not be any breaking changes (although, users should generally not
rely on pseudo-private/hidden modules like `__macro_exports`).
I chose to keep it in `bevy_reflect::__macro_exports` so as to not
clutter up our public API and to reduce the number of changes in this
PR. We could have also marked the export as `#[doc(hidden)]`, but
personally I like that we have a dedicated module for this (makes it
clear what is public and what isn't when just looking at the macro
code).
---
## Changelog
- Moved `generate_composite_uuid` to `bevy_utils::uuid` and made it
public
- Note: it was technically already public, just hidden
# Objective
It is currently impossible to control the relative ordering of two 2D
materials at the same depth. This is required to implement wireframes
for 2D meshes correctly
(https://github.com/bevyengine/bevy/issues/5881).
## Solution
Add a `Material2d::depth_bias` function that mirrors the existing 3D
`Material::depth_bias` function.
(this is pulled out of https://github.com/bevyengine/bevy/pull/10489)
---
## Changelog
### Added
- Added `Material2d::depth_bias`
## Migration Guide
`PreparedMaterial2d` has a new `depth_bias` field. A value of 0.0 can be
used to get the previous behavior.
# Objective
Resolves #10727.
`outline.width` was being assigned to `node.outline_offset` instead of
`outline.offset`.
## Solution
Changed `.width` to `.offset` in line 413.
# Objective
Bevy_hierarchy is very useful for ECS only usages of bevy_ecs, but it
currently pulls in bevy_reflect, bevy_app and bevy_core with no way to
opt out.
## Solution
This PR provides features `bevy_app` and `reflect` that are enabled by
default. If disabled, they should remove these dependencies from
bevy_hierarchy.
---
## Changelog
Added features `bevy_app` and `reflect` to bevy_hierarchy.
# Objective
Updates [`futures-lite`](https://github.com/smol-rs/futures-lite) in
bevy_tasks to the next major version (1 -> 2).
Also removes the duplication of `futures-lite`, as `async-fs` requires v
2, so there are currently 2 copies of futures-lite in the dependency
tree.
Futures-lite has received [a number of
updates](https://github.com/smol-rs/futures-lite/blob/master/CHANGELOG.md)
since bevy's current version `1.4`.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Mike <mike.hsu@gmail.com>
# Objective
- Fix#10499
## Solution
- Use `.get_represented_type_info()` module path and type ident instead
of `.reflect_*` module path and type ident when serializing the `Option`
enum
---
## Changelog
- Fix serialization bug
- Add simple test
- Add `serde_json` dev dependency
- Add `serde` with `derive` feature dev dependency (wouldn't compile for
me without it)
---------
Co-authored-by: hank <hank@hank.co.in>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Explain https://github.com/bevyengine/bevy/issues/10625.
This might be obvious to those familiar with Bevy internals, but it
surprised me.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
I incorrectly assumed that moving a system from `Update` to
`FixedUpdate` would simplify logic without hurting performance.
But this is not the case: if there's single-threaded long computation in
the `FixedUpdate`, the machine won't do anything else in parallel with
it. Which might be not what users expect.
So this PR adds a note. But maybe it is obvious, I don't know.
# Objective
fix webgpu+chrome(119) textureSample in non-uniform control flow error
## Solution
modify view transmission texture sampling to use textureSampleLevel.
there are no mips for the view transmission texture, so this doesn't
change the result, but it removes the need for the samples to be in
uniform control flow.
note: in future we may add a mipchain to the transmission texture to
improve the blur effect. if uniformity analysis hasn't improved, this
would require switching to manual derivative calculations (which is
something we plan to do anyway).
## Objective
- Split different types of gizmos into their own files
## Solution
- Move `arc_2d` and `Arc2dBuilder` into `arcs.rs`
- turns out there's no 3d arc function! I'll add one Soon(TM) in another
MR
## Changelog
- Changed
- moved `gizmos::Arc2dBuilder` to `gizmos::arcs::Arc2dBuilder`
## Migration Guide
- `gizmos::Arc2dBuilder` -> `gizmos::arcs::Arc2dBuilder`
# Objective
Problems:
* The clipped, non-visible regions of UI nodes are interactive.
* `RelativeCursorPostion` is set relative to the visible part of the
node. It should be relative to the whole node.
* The `RelativeCursorPostion::mouse_over` method returns `true` when the
mouse is over a clipped part of a node.
fixes#10470
## Solution
Intersect a node's bounding rect with its clipping rect before checking
if it contains the cursor.
Added the field `normalized_visible_node_rect` to
`RelativeCursorPosition`. This is set to the bounds of the unclipped
area of the node rect by `ui_focus_system` expressed in normalized
coordinates relative to the entire node.
Instead of checking if the normalized cursor position lies within a unit
square, it instead checks if it is contained by
`normalized_visible_node_rect`.
Added outlines to the `overflow` example that appear when the cursor is
over the visible part of the images, but not the clipped area.
---
## Changelog
* `ui_focus_system` intersects a node's bounding rect with its clipping
rect before checking if mouse over.
* Added the field `normalized_visible_node_rect` to
`RelativeCursorPosition`. This is set to the bounds of the unclipped
area of the node rect by `ui_focus_system` expressed in normalized
coordinates relative to the entire node.
* `RelativeCursorPostion` is calculated relative to the whole node's
position and size, not only the visible part.
* `RelativeCursorPosition::mouse_over` only returns true when the mouse
is over an unclipped region of the UI node.
* Removed the `Deref` and `DerefMut` derives from
`RelativeCursorPosition` as it is no longer a single field struct.
* Added some outlines to the `overflow` example that respond to
`Interaction` changes.
## Migration Guide
The clipped areas of UI nodes are no longer interactive.
`RelativeCursorPostion` is now calculated relative to the whole node's
position and size, not only the visible part. Its `mouse_over` method
only returns true when the cursor is over an unclipped part of the node.
`RelativeCursorPosition` no longer implements `Deref` and `DerefMut`.
# Objective
- Fixes#10695
## Solution
Fixed obvious blunder in `PartialEq` implementation for
`UntypedAssetId`'s where the `TypeId` was not included in the
comparison. This was not picked up in the unit tests I added because
they only tested over a single asset type.
# Objective
- I've been experimenting with different patterns to try and make async
tasks more convenient. One of the better ones I've found is to return a
command queue to allow for deferred &mut World access. It can be
convenient to check for task completion in a normal system, but it is
hard to do something with the command queue after getting it back. This
pr adds a `append` to Commands. This allows appending the returned
command queue onto the system's commands.
## Solution
- I edited the async compute example to use the new `append`, but not
sure if I should keep the example changed as this might be too
opinionated.
## Future Work
- It would be very easy to pull the pattern used in the example out into
a plugin or a external crate, so users wouldn't have to add the checking
system.
---
## Changelog
- add `append` to `Commands` and `CommandQueue`
# Objective
Enables warning on `clippy::undocumented_unsafe_blocks` across the
workspace rather than only in `bevy_ecs`, `bevy_transform` and
`bevy_utils`. This adds a little awkwardness in a few areas of code that
have trivial safety or explain safety for multiple unsafe blocks with
one comment however automatically prevents these comments from being
missed.
## Solution
This adds `undocumented_unsafe_blocks = "warn"` to the workspace
`Cargo.toml` and fixes / adds a few missed safety comments. I also added
`#[allow(clippy::undocumented_unsafe_blocks)]` where the safety is
explained somewhere above.
There are a couple of safety comments I added I'm not 100% sure about in
`bevy_animation` and `bevy_render/src/view` and I'm not sure about the
use of `#[allow(clippy::undocumented_unsafe_blocks)]` compared to adding
comments like `// SAFETY: See above`.
# Objective
First, some terminology:
- **Minor radius**: The radius of the tube of a torus, i.e. the
"half-thickness"
- **Major radius**: The distance from the center of the tube to the
center of the torus
- **Inner radius**: The radius of the hole (if it exists), `major_radius
- minor_radius`
- **Outer radius**: The radius of the overall shape, `major_radius +
minor_radius`
- **Ring torus**: The familiar donut shape with a hole in the center,
`major_radius > minor_radius`
- **Horn torus**: A torus that doesn't have a hole but also isn't
self-intersecting, `major_radius == minor_radius`
- **Spindle torus**: A self-intersecting torus, `major_radius <
minor_radius`
Different tori from [Wikipedia](https://en.wikipedia.org/wiki/Torus),
where *R* is the major radius and *r* is the minor radius:
Currently, Bevy's torus is represented by a `radius` and `ring_radius`.
I believe these correspond to the outer radius and minor radius, but
they are rather confusing and inconsistent names, and they make the
assumption that the torus always has a ring.
I also couldn't find any other big engines using this representation;
[Godot](https://docs.godotengine.org/en/stable/classes/class_torusmesh.html)
and [Unity
ProBuilder](https://docs.unity3d.com/Packages/com.unity.probuilder@4.0/manual/Torus.html)
use the inner and outer radii, while
[Unreal](https://docs.unrealengine.com/5.3/en-US/BlueprintAPI/GeometryScript/Primitives/AppendTorus/)
uses the minor and major radii.
[Blender](https://docs.blender.org/manual/en/latest/modeling/meshes/primitives.html#torus)
supports both, but defaults to minor/major.
Bevy's `Torus` primitive should have an efficient, consistent, clear and
flexible representation, and the current `radius` and `ring_radius`
properties are not ideal for that.
## Solution
Change `Torus` to be represented by a `minor_radius` and `major_radius`.
- Mathematically correct and consistent
- Flexible, not restricted to ring tori
- Computations and conversions are efficient
- `inner_radius = major_radius - minor_radius`
- `outer_radius = major_radius + minor_radius`
- Mathematical formulae for things like area and volume rely on the
minor and major radii, no conversion needed
Perhaps the primary issue with this representation is that "minor
radius" and "major radius" are rather mathematical, and an inner/outer
radius can be more intuitive in some cases. However, this can be
mitigated with constructors and helpers.
# Objective
Make the impl block for RemovedSystem generic so that the methods can be
called for systems that have inputs or outputs.
## Solution
Simply adding generics to the impl block.
# Objective
Adds `.entry` to `EntityWorldMut` with `Entry`, `OccupiedEntry` and
`VacantEntry` for easier in-situ modification, based on `HashMap.entry`.
Fixes#10635
## Solution
This adds the `entry` method to `EntityWorldMut` which returns an
`Entry`. This is an enum of `OccupiedEntry` and `VacantEntry` and has
the methods `and_modify`, `insert_entry`, `or_insert`, `or_insert_with`
and `or_default`. The only difference between `OccupiedEntry` and
`VacantEntry` is the type, they are both a mutable reference to the
`EntityWorldMut` and a marker for the component type, `HashMap` also
stores things to make it quicker to access the data in `OccupiedEntry`
but I wasn't sure if we had anything it would be logical to store to
make accessing/modifying the component faster? As such, the differences
are that `OccupiedEntry` assumes the entity has the component (because
nothing else can have an `EntityWorldMut` so it can't be changed outside
the entry api) and has different methods.
All the methods are based very closely off `hashbrown::HashMap` (because
its easier to read the source of) with a couple of quirks like
`OccupiedEntry.insert` doesn't return the old value because we don't
appear to have an api for mem::replacing components.
---
## Changelog
- Added a new function `EntityWorldMut.entry` which returns an `Entry`,
allowing easier in-situ modification of a component.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Pascal Hertleif <killercup@gmail.com>
# Objective
- Fixes#10629
- `UntypedAssetId` and `AssetId` (along with `UntypedHandle` and
`Handle`) do not hash to the same values when pointing to the same
`Asset`. Additionally, comparison and conversion between these types
does not follow idiomatic Rust standards.
## Solution
- Added new unit tests to validate/document expected behaviour
- Added trait implementations to make working with Un/Typed values more
ergonomic
- Ensured hashing and comparison between Un/Typed values is consistent
- Removed `From` trait implementations that panic, and replaced them
with `TryFrom`
---
## Changelog
- Ensured `Handle::<A>::hash` and `UntypedHandle::hash` will produce the
same value for the same `Asset`
- Added non-panicing `Handle::<A>::try_typed`
- Added `PartialOrd` to `UntypedHandle` to match `Handle<A>` (this will
return `None` for `UntypedHandles` for differing `Asset` types)
- Added `TryFrom<UntypedHandle>` for `Handle<A>`
- Added `From<Handle<A>>` for `UntypedHandle`
- Removed panicing `From<Untyped...>` implementations. These are
currently unused within the Bevy codebase, and shouldn't be used
externally, hence removal.
- Added cross-implementations of `PartialEq` and `PartialOrd` for
`UntypedHandle` and `Handle<A>` allowing direct comparison when
`TypeId`'s match.
- Near-identical changes to `AssetId` and `UntypedAssetId`
## Migration Guide
If you relied on any of the panicing `From<Untyped...>` implementations,
simply call the existing `typed` methods instead. Alternatively, use the
new `TryFrom` implementation instead to directly expose possible
mistakes.
## Notes
I've made these changes since `Handle` is such a fundamental type to the
entire `Asset` ecosystem within Bevy, and yet it had pretty unclear
behaviour with no direct testing. The fact that hashing untyped vs typed
versions of the same handle would produce different values is something
I expect to cause a very subtle bug for someone else one day.
I haven't included it in this PR to avoid any controversy, but I also
believe the `typed_unchecked` methods should be removed from these
types, or marked as `unsafe`. The `texture_atlas` example currently uses
it, and I believe it is a bad choice. The performance gained by not
type-checking before conversion would be entirely dwarfed by the act of
actually loading an asset and creating its handle anyway. If an end user
is in a tight loop repeatedly calling `typed_unchecked` on an
`UntypedHandle` for the entire runtime of their application, I think the
small performance drop caused by that safety check is ~~a form of cosmic
justice~~ reasonable.
# Objective
- Standardize fmt for toml files
## Solution
- Add [taplo](https://taplo.tamasfe.dev/) to CI (check for fmt and diff
for toml files), for context taplo is used by the most popular extension
in VScode [Even Better
TOML](https://marketplace.visualstudio.com/items?itemName=tamasfe.even-better-toml
- Add contribution section to explain toml fmt with taplo.
Now to pass CI you need to run `taplo fmt --option indent_string=" "` or
if you use vscode have the `Even Better TOML` extension with 4 spaces
for indent
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
## Objective
- Give all the intuitive groups of gizmos their own file
- don't be a breaking change
- don't change Gizmos interface
- eventually do arcs too
- future types of gizmos should be in their own files
- see also https://github.com/bevyengine/bevy/issues/9400
## Solution
- Moved `gizmos.circle`, `gizmos.2d_circle`, and assorted helpers into
`circles.rs`
- Can also do arcs in this MR if y'all want; just figured I should do
one thing at a time.
## Changelog
- Changed
- `gizmos::CircleBuilder` moved to `gizmos::circles::Circle2dBuilder`
- `gizmos::Circle2dBuilder` moved to `gizmos::circles::Circle2dBuilder`
## Migration Guide
- change `gizmos::CircleBuilder` to `gizmos::circles::Circle2dBuilder`
- change `gizmos::Circle2dBuilder` to `gizmos::circles::Circle2dBuilder`
---------
Co-authored-by: François <mockersf@gmail.com>
# Objective
This PR adds some helpers for `Triangle2d` to work with its winding
order. This could also be extended to polygons (and `Triangle3d` once
it's added).
## Solution
- Add `WindingOrder` enum with `Clockwise`, `Counterclockwise` and
`Invalid` variants
- `Invalid` is for cases where the winding order can not be reliably
computed, i.e. the points lie on a single line and the area is zero
- Add `Triangle2d::winding_order` method that uses a signed surface area
to determine the winding order
- Add `Triangle2d::reverse` method that reverses the winding order by
swapping the second and third vertices
The API looks like this:
```rust
let mut triangle = Triangle2d::new(
Vec2::new(0.0, 2.0),
Vec2::new(-0.5, -1.2),
Vec2::new(-1.0, -1.0),
);
assert_eq!(triangle.winding_order(), WindingOrder::Clockwise);
// Reverse winding order
triangle.reverse();
assert_eq!(triangle.winding_order(), WindingOrder::Counterclockwise);
```
I also added tests to make sure the methods work correctly. For now,
they live in the same file as the primitives.
## Open questions
- Should it be `Counterclockwise` or `CounterClockwise`? The first one
is more correct but perhaps a bit less readable. Counter-clockwise is
also a valid spelling, but it seems to be a lot less common than
counterclockwise.
- Is `WindingOrder::Invalid` a good name? Parry uses
`TriangleOrientation::Degenerate`, but I'm not a huge fan, at least as a
non-native English speaker. Any better suggestions?
- Is `WindingOrder` fine in `bevy_math::primitives`? It's not specific
to a dimension, so I put it there for now.
# Objective
Fix the `bevy_asset/file_watcher` feature in practice depending on
multithreading, while not informing the user of it.
**As I understand it** (I didn't check it), the file watcher feature
depends on spawning a concurrent thread to receive file update events
from the `notify-debouncer-full` crate. But if multithreading is
disabled, that thread will never have time to read the events and
consume them.
- Fixes#10573
## Solution
Add a `compile_error!` causing compilation failure if `file_watcher` is
enabled while `multi-threaded` is disabled.
This is considered better than adding a dependency on `multi-threaded`
on the `file_watcher`, as (according to @mockersf) toggling on/off
`multi-threaded` has a non-zero chance of changing behavior. And we
shouldn't implicitly change behavior. A compilation failure prevents
compilation of code that is invalid, while informing the user of the
steps needed to fix it.
# Objective
- Sometimes it's very useful to know if a `Transform` contains any `NaN`
or infinite values. It's a bit boiler-plate heavy to check translation,
rotation and scale individually.
## Solution
- Add a new method `is_finite` that returns true if, and only if
translation, rotation and scale all are finite.
- It's a natural extension of `Quat::is_finite`, and `Vec3::is_finite`,
which return true if, and only if all their components' `is_finite()`
returns true.
---
## Changelog
- Added `Transform::is_finite`
# Objective
The `map_async` method involves a type `BufferAsyncError`:
https://docs.rs/bevy/latest/bevy/render/render_resource/struct.BufferSlice.html#method.map_async
This type is not re-exported in Bevy, so if a user wants to store a
struct involving this type they have to add wgpu manually to their
manifest.
## Solution
- Re-export wgpu::BufferAsyncError
---
## Changelog
### Added
- Re-export wgpu::BufferAsyncError
Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
# Objective
- Fix adding `#![allow(clippy::type_complexity)]` everywhere. like #9796
## Solution
- Use the new [lints] table that will land in 1.74
(https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints)
- inherit lint to the workspace, crates and examples.
```
[lints]
workspace = true
```
## Changelog
- Bump rust version to 1.74
- Enable lints table for the workspace
```toml
[workspace.lints.clippy]
type_complexity = "allow"
```
- Allow type complexity for all crates and examples
```toml
[lints]
workspace = true
```
---------
Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
# Objective
- Follow up on https://github.com/bevyengine/bevy/pull/10519, diving
deeper into optimising `Entity` due to the `derive`d `PartialOrd`
`partial_cmp` not being optimal with codegen:
https://github.com/rust-lang/rust/issues/106107
- Fixes#2346.
## Solution
Given the previous PR's solution and the other existing LLVM codegen
bug, there seemed to be a potential further optimisation possible with
`Entity`. In exploring providing manual `PartialOrd` impl, it turned out
initially that the resulting codegen was not immediately better than the
derived version. However, once `Entity` was given `#[repr(align(8)]`,
the codegen improved remarkably, even more once the fields in `Entity`
were rearranged to correspond to a `u64` layout (Rust doesn't
automatically reorder fields correctly it seems). The field order and
`align(8)` additions also improved `to_bits` codegen to be a single
`mov` op. In turn, this led me to replace the previous
"non-shortcircuiting" impl of `PartialEq::eq` to use direct `to_bits`
comparison.
The result was remarkably better codegen across the board, even for
hastable lookups.
The current baseline codegen is as follows:
https://godbolt.org/z/zTW1h8PnY
Assuming the following example struct that mirrors with the existing
`Entity` definition:
```rust
#[derive(Clone, Copy, Eq, PartialEq, PartialOrd, Ord)]
pub struct FakeU64 {
high: u32,
low: u32,
}
```
the output for `to_bits` is as follows:
```
example::FakeU64::to_bits:
shl rdi, 32
mov eax, esi
or rax, rdi
ret
```
Changing the struct to:
```rust
#[derive(Clone, Copy, Eq)]
#[repr(align(8))]
pub struct FakeU64 {
low: u32,
high: u32,
}
```
and providing manual implementations for `PartialEq`/`PartialOrd`/`Ord`,
`to_bits` now optimises to:
```
example::FakeU64::to_bits:
mov rax, rdi
ret
```
The full codegen example for this PR is here for reference:
https://godbolt.org/z/n4Mjx165a
To highlight, `gt` comparison goes from
```
example::greater_than:
cmp edi, edx
jae .LBB3_2
xor eax, eax
ret
.LBB3_2:
setne dl
cmp esi, ecx
seta al
or al, dl
ret
```
to
```
example::greater_than:
cmp rdi, rsi
seta al
ret
```
As explained on Discord by @scottmcm :
>The root issue here, as far as I understand it, is that LLVM's
middle-end is inexplicably unwilling to merge loads if that would make
them under-aligned. It leaves that entirely up to its target-specific
back-end, and thus a bunch of the things that you'd expect it to do that
would fix this just don't happen.
## Benchmarks
Before discussing benchmarks, everything was tested on the following
specs:
AMD Ryzen 7950X 16C/32T CPU
64GB 5200 RAM
AMD RX7900XT 20GB Gfx card
Manjaro KDE on Wayland
I made use of the new entity hashing benchmarks to see how this PR would
improve things there. With the changes in place, I first did an
implementation keeping the existing "non shortcircuit" `PartialEq`
implementation in place, but with the alignment and field ordering
changes, which in the benchmark is the `ord_shortcircuit` column. The
`to_bits` `PartialEq` implementation is the `ord_to_bits` column. The
main_ord column is the current existing baseline from `main` branch.
My machine is not super set-up for benchmarking, so some results are
within noise, but there's not just a clear improvement between the
non-shortcircuiting implementation, but even further optimisation taking
place with the `to_bits` implementation.
On my machine, a fair number of the stress tests were not showing any
difference (indicating other bottlenecks), but I was able to get a clear
difference with `many_foxes` with a fox count of 10,000:
Test with `cargo run --example many_foxes --features
bevy/trace_tracy,wayland --release -- --count 10000`:
On avg, a framerate of about 28-29FPS was improved to 30-32FPS. "This
trace" represents the current PR's perf, while "External trace"
represents the `main` branch baseline.
## Changelog
Changed: micro-optimized Entity align and field ordering as well as
providing manual `PartialOrd`/`Ord` impls to help LLVM optimise further.
## Migration Guide
Any `unsafe` code relying on field ordering of `Entity` or sufficiently
cursed shenanigans should change to reflect the different internal
representation and alignment requirements of `Entity`.
Co-authored-by: james7132 <contact@jamessliu.com>
Co-authored-by: NathanW <nathansward@comcast.net>
Bevy introduced unintentional breaking behaviour along with the v0.12.0
release regarding the `App::set_runner` API. See: #10385, #10389 for
details. We weren't able to catch this before release because this API
is only used internally in one or two places (the very places which
motivated the break).
This commit adds a regression test to help guarantee some expected
behaviour for custom runners, namely that `app::update` won't be called
before the runner has a chance to initialise state.
# Objective
There is no easy way to discard some amount for `Time<Fixed>`'s
overstep. This can be useful for online games when the client receives
information about a tick (which happens when you get a FPS drop or the
ping changes for example) it has not yet processed, it can discard
overstep equal to the number of ticks it can jump ahead.
Currently the workaround would be to create a new `Time<Fixed>` copy the
old timestep, advance it by the overstep amount that would remain after
subtracting the discarded amount, and using `.context_mut()` to
overwrite the old context with the new one. If you overwrite the whole
`Time<Fixed>` or forget to copy over the timestep you can introduce
undesirable side effects.
## Solution
Introduce a `discard_overstep` method, which discards the provided
amount of overstep. It uses satuarting_sub to avoid errors (negative
`Duration`s do not exist).
---
## Changelog
- Added `discard_overstep` function to `Time<Fixed>`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Allow bevy applications that does not have any assets folder to start
from a read-only directory. (typically installed to a systems folder)
Fixes#10613
## Solution
- warn instead of panic when assets folder creation fails.
# Objective
- Currently, in 0.12 there is an issue that it is not possible to build
bevy for Wasm with feature "file_watcher" enabled. It still would not
compile, but now with proper explanation.
- Fixes https://github.com/bevyengine/bevy/issues/10507
## Solution
- Remove `notify-debouncer-full` dependency on WASM platform entirely.
- Compile with "file_watcher" feature now on platform `wasm32` gives
meaningful compile error.
---
## Changelog
### Fixed
- Compile with "file_watcher" feature now on platform `wasm32` gives
meaningful compile error.
# Add and implement constructors for Primitives
- Adds more Primitive types and adds a constructor for almost all of
them
- Works towards finishing #10572
## Solution
- Created new primitives
- Torus
- Conical Frustum
- Cone
- Ellipse
- Implemented constructors (`Primitive::new`) for almost every single
other primitive.
---------
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Handy to have a constant instead of `VolumeLevel::new(0.0)`
- `VolumeLevel::new` is not `const`
## Solution
- Adds a `VolumeLevel::ZERO` constant, which we have for most of our
other types where it makes sense.
---
## Changelog
- Add `VolumeLevel::ZERO`
# Objective
The way `bevy_app` works was changed unnecessarily in #9826 whose
changes should have been specific to `bevy_winit`.
I'm somewhat disappointed that happened and we can see in
https://github.com/bevyengine/bevy/pull/10195 that it made things more
complicated.
Even worse, in #10385 it's clear that this breaks the clean abstraction
over another engine someone built with Bevy!
Fixes#10385.
## Solution
- Move the changes made to `bevy_app` in #9826 to `bevy_winit`
- Revert the changes to `ScheduleRunnerPlugin` and the `run_once` runner
in #10195 as they're no longer necessary.
While this code is breaking relative to `0.12.0`, it reverts the
behavior of `bevy_app` back to how it was in `0.11`.
Due to the nature of the breakage relative to `0.11` I hope this will be
considered for `0.12.1`.
# Objective
- Fix the panic on using Images in UiMaterials due to assets not being
loaded.
- Fixes#10513
## Solution
- add `let else` statement that `return`s or `continue`s instead of
unwrapping, causing a panic.
# Objective
- Fixes#10518
## Solution
I've added a method to `LoadContext`, `load_direct_with_reader`, which
mirrors the behaviour of `load_direct` with a single key difference: it
is provided with the `Reader` by the caller, rather than getting it from
the contained `AssetServer`. This allows for an `AssetLoader` to process
its `Reader` stream, and then directly hand the results off to the
`LoadContext` to handle further loading. The outer `AssetLoader` can
control how the `Reader` is interpreted by providing a relevant
`AssetPath`.
For example, a Gzip decompression loader could process the asset
`images/my_image.png.gz` by decompressing the bytes, then handing the
decompressed result to the `LoadContext` with the new path
`images/my_image.png.gz/my_image.png`. This intuitively reflects the
nature of contained assets, whilst avoiding unintended behaviour, since
the generated path cannot be a real file path (a file and folder of the
same name cannot coexist in most file-systems).
```rust
#[derive(Asset, TypePath)]
pub struct GzAsset {
pub uncompressed: ErasedLoadedAsset,
}
#[derive(Default)]
pub struct GzAssetLoader;
impl AssetLoader for GzAssetLoader {
type Asset = GzAsset;
type Settings = ();
type Error = GzAssetLoaderError;
fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a (),
load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Self::Asset, Self::Error>> {
Box::pin(async move {
let compressed_path = load_context.path();
let file_name = compressed_path
.file_name()
.ok_or(GzAssetLoaderError::IndeterminateFilePath)?
.to_string_lossy();
let uncompressed_file_name = file_name
.strip_suffix(".gz")
.ok_or(GzAssetLoaderError::IndeterminateFilePath)?;
let contained_path = compressed_path.join(uncompressed_file_name);
let mut bytes_compressed = Vec::new();
reader.read_to_end(&mut bytes_compressed).await?;
let mut decoder = GzDecoder::new(bytes_compressed.as_slice());
let mut bytes_uncompressed = Vec::new();
decoder.read_to_end(&mut bytes_uncompressed)?;
// Now that we have decompressed the asset, let's pass it back to the
// context to continue loading
let mut reader = VecReader::new(bytes_uncompressed);
let uncompressed = load_context
.load_direct_with_reader(&mut reader, contained_path)
.await?;
Ok(GzAsset { uncompressed })
})
}
fn extensions(&self) -> &[&str] {
&["gz"]
}
}
```
Because this example is so prudent, I've included an
`asset_decompression` example which implements this exact behaviour:
```rust
fn main() {
App::new()
.add_plugins(DefaultPlugins)
.init_asset::<GzAsset>()
.init_asset_loader::<GzAssetLoader>()
.add_systems(Startup, setup)
.add_systems(Update, decompress::<Image>)
.run();
}
fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
commands.spawn(Camera2dBundle::default());
commands.spawn((
Compressed::<Image> {
compressed: asset_server.load("data/compressed_image.png.gz"),
..default()
},
Sprite::default(),
TransformBundle::default(),
VisibilityBundle::default(),
));
}
fn decompress<A: Asset>(
mut commands: Commands,
asset_server: Res<AssetServer>,
mut compressed_assets: ResMut<Assets<GzAsset>>,
query: Query<(Entity, &Compressed<A>)>,
) {
for (entity, Compressed { compressed, .. }) in query.iter() {
let Some(GzAsset { uncompressed }) = compressed_assets.remove(compressed) else {
continue;
};
let uncompressed = uncompressed.take::<A>().unwrap();
commands
.entity(entity)
.remove::<Compressed<A>>()
.insert(asset_server.add(uncompressed));
}
}
```
A key limitation to this design is how to type the internally loaded
asset, since the example `GzAssetLoader` is unaware of the internal
asset type `A`. As such, in this example I store the contained asset as
an `ErasedLoadedAsset`, and leave it up to the consumer of the `GzAsset`
to handle typing the final result, which is the purpose of the
`decompress` system. This limitation can be worked around by providing
type information to the `GzAssetLoader`, such as `GzAssetLoader<Image,
ImageAssetLoader>`, but this would require registering the asset loader
for every possible decompression target.
Aside from this limitation, nested asset containerisation works as an
end user would expect; if the user registers a `TarAssetLoader`, and a
`GzAssetLoader`, then they can load assets with compound
containerisation, such as `images.tar.gz`.
---
## Changelog
- Added `LoadContext::load_direct_with_reader`
- Added `asset_decompression` example
## Notes
- While I believe my implementation of a Gzip asset loader is
reasonable, I haven't included it as a public feature of `bevy_asset` to
keep the scope of this PR as focussed as possible.
- I have included `flate2` as a `dev-dependency` for the example; it is
not included in the main dependency graph.
# Objective
Addresses #[10438](https://github.com/bevyengine/bevy/issues/10438)
The objective was to include the failing path in the error for the user
to see.
## Solution
Add a `path` field to the `ReadAssetBytesError::Io` variant to expose
the failing path in the error message.
## Migration Guide
- The `ReadAssetBytesError::Io` variant now contains two named fields
instead of converting from `std::io::Error`.
1. `path`: The requested (failing) path (`PathBuf`)
2. `source`: The source `std::io::Error`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Fixes#10532
## Solution
I've updated the various `Event` send methods to return the sent
`EventId`(s). Since these methods previously returned nothing, and this
information is cheap to copy, there should be minimal negative
consequences to providing this additional information. In the case of
`send_batch`, an iterator is returned built from `Range` and `Map`,
which only consumes 16 bytes on the stack with no heap allocations for
all batch sizes. As such, the cost of this information is negligible.
These changes are reflected for `EventWriter` and `World`. For `World`,
the return types are optional to account for the possible lack of an
`Events` resource. Again, these methods previously returned no
information, so its inclusion should only be a benefit.
## Usage
Now when sending events, the IDs of those events is available for
immediate use:
```rust
// Example of a request-response system where the requester can track handled requests.
/// A system which can make and track requests
fn requester(
mut requests: EventWriter<Request>,
mut handled: EventReader<Handled>,
mut pending: Local<HashSet<EventId<Request>>>,
) {
// Check status of previous requests
for Handled(id) in handled.read() {
pending.remove(&id);
}
if !pending.is_empty() {
error!("Not all my requests were handled on the previous frame!");
pending.clear();
}
// Send a new request and remember its ID for later
let request_id = requests.send(Request::MyRequest { /* ... */ });
pending.insert(request_id);
}
/// A system which handles requests
fn responder(
mut requests: EventReader<Request>,
mut handled: EventWriter<Handled>,
) {
for (request, id) in requests.read_with_id() {
if handle(request).is_ok() {
handled.send(Handled(id));
}
}
}
```
In the above example, a `requester` system can send request events, and
keep track of which ones are currently pending by `EventId`. Then, a
`responder` system can act on that event, providing the ID as a
reference that the `requester` can use. Before this PR, it was not
trivial for a system sending events to keep track of events by ID. This
is unfortunate, since for a system reading events, it is trivial to
access the ID of a event.
---
## Changelog
- Updated `Events`:
- Added `send_batch`
- Modified `send` to return the sent `EventId`
- Modified `send_default` to return the sent `EventId`
- Updated `EventWriter`
- Modified `send_batch` to return all sent `EventId`s
- Modified `send` to return the sent `EventId`
- Modified `send_default` to return the sent `EventId`
- Updated `World`
- Modified `send_event` to return the sent `EventId` if sent, otherwise
`None`.
- Modified `send_event_default` to return the sent `EventId` if sent,
otherwise `None`.
- Modified `send_event_batch` to return all sent `EventId`s if sent,
otherwise `None`.
- Added unit test `test_send_events_ids` to ensure returned `EventId`s
match the sent `Event`s
- Updated uses of modified methods.
## Migration Guide
### `send` / `send_default` / `send_batch`
For the following methods:
- `Events::send`
- `Events::send_default`
- `Events::send_batch`
- `EventWriter::send`
- `EventWriter::send_default`
- `EventWriter::send_batch`
- `World::send_event`
- `World::send_event_default`
- `World::send_event_batch`
Ensure calls to these methods either handle the returned value, or
suppress the result with `;`.
```rust
// Now fails to compile due to mismatched return type
fn send_my_event(mut events: EventWriter<MyEvent>) {
events.send_default()
}
// Fix
fn send_my_event(mut events: EventWriter<MyEvent>) {
events.send_default();
}
```
This will most likely be noticed within `match` statements:
```rust
// Before
match is_pressed {
true => events.send(PlayerAction::Fire),
// ^--^ No longer returns ()
false => {}
}
// After
match is_pressed {
true => {
events.send(PlayerAction::Fire);
},
false => {}
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
# Objective
Give us the ability to load untyped assets in AssetLoaders.
## Solution
Basically just copied the code from `load`, but used
`asset_server.load_untyped` instead internally.
## Changelog
Added `load_untyped` method to `LoadContext`
# Objective
- Implement a subset of
https://github.com/bevyengine/rfcs/blob/main/rfcs/12-primitive-shapes.md#feature-name-primitive-shapes
## Solution
- Define a very basic set of primitives in bevy_math
- Assume a 0,0,0 origin for most shapes
- Use radius and half extents to avoid unnecessary computational
overhead wherever they get used
- Provide both Boxed and const generics variants for shapes with
variable sizes
- Boxed is useful if a 3rd party crate wants to use something like
enum-dispatch for all supported primitives
- Const generics is useful when just working on a single primitive, as
it causes no allocs
#### Some discrepancies from the RFC:
- Box was changed to Cuboid, because Box is already used for an alloc
type
- Skipped Cone because it's unclear where the origin should be for
different uses
- Skipped Wedge because it's too niche for an initial PR (we also don't
implement Torus, Pyramid or a Death Star (there's an SDF for that!))
- Skipped Frustum because while it would be a useful math type, it's not
really a common primitive
- Skipped Triangle3d and Quad3d because those are just rotated 2D shapes
## Future steps
- Add more primitives
- Add helper methods to make primitives easier to construct (especially
when half extents are involved)
- Add methods to calculate AABBs for primitives (useful for physics, BVH
construction, for the mesh AABBs, etc)
- Add wrappers for common and cheap operations, like extruding 2D shapes
and translating them
- Use the primitives to generate meshes
- Provide signed distance functions and gradients for primitives (maybe)
---
## Changelog
- Added a collection of primitives to the bevy_math crate
---------
Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
# Objective
Closes#10319
## Changelog
* Added a new `Color::rgba_from_array([f32; 4]) -> Color` method.
* Added a new `Color::rgb_from_array([f32; 3]) -> Color` method.
* Added a new `Color::rgba_linear_from_array([f32; 4]) -> Color` method.
* Added a new `Color::rgb_linear_from_array([f32; 3]) -> Color` method.
* Added a new `Color::hsla_from_array([f32; 4]) -> Color` method.
* Added a new `Color::hsl_from_array([f32; 3]) -> Color` method.
* Added a new `Color::lcha_from_array([f32; 4]) -> Color` method.
* Added a new `Color::lch_from_array([f32; 3]) -> Color` method.
* Added a new `Color::rgba_to_vec4(&self) -> Vec4` method.
* Added a new `Color::rgba_to_array(&self) -> [f32; 4]` method.
* Added a new `Color::rgb_to_vec3(&self) -> Vec3` method.
* Added a new `Color::rgb_to_array(&self) -> [f32; 3]` method.
* Added a new `Color::rgba_linear_to_vec4(&self) -> Vec4` method.
* Added a new `Color::rgba_linear_to_array(&self) -> [f32; 4]` method.
* Added a new `Color::rgb_linear_to_vec3(&self) -> Vec3` method.
* Added a new `Color::rgb_linear_to_array(&self) -> [f32; 3]` method.
* Added a new `Color::hsla_to_vec4(&self) -> Vec4` method.
* Added a new `Color::hsla_to_array(&self) -> [f32; 4]` method.
* Added a new `Color::hsl_to_vec3(&self) -> Vec3` method.
* Added a new `Color::hsl_to_array(&self) -> [f32; 3]` method.
* Added a new `Color::lcha_to_vec4(&self) -> Vec4` method.
* Added a new `Color::lcha_to_array(&self) -> [f32; 4]` method.
* Added a new `Color::lch_to_vec3(&self) -> Vec3` method.
* Added a new `Color::lch_to_array(&self) -> [f32; 3]` method.
## Migration Guide
`Color::from(Vec4)` is now `Color::rgba_from_array(impl Into<[f32; 4]>)`
`Vec4::from(Color)` is now `Color::rgba_to_vec4(&self)`
Before:
```rust
let color_vec4 = Vec4::new(0.5, 0.5, 0.5);
let color_from_vec4 = Color::from(color_vec4);
let color_array = [0.5, 0.5, 0.5];
let color_from_array = Color::from(color_array);
```
After:
```rust
let color_vec4 = Vec4::new(0.5, 0.5, 0.5);
let color_from_vec4 = Color::rgba_from_array(color_vec4);
let color_array = [0.5, 0.5, 0.5];
let color_from_array = Color::rgba_from_array(color_array);
```
# Objective
Close#10504. Improve the development experience for working with scenes
by not requiring the user to specify a matching version of `ron` in
their `Cargo.toml`
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- When compiling bevy for both singlethreaded and multithreaded contexts
and using `Task` directly, you can run into errors where you expect a
`Task` to be returned but `FakeTask` is instead. Due to `FakeTask` being
private the only solution is to ignore the return at all however because
it *is* returned that isn't totally clear. The error is confusing and
doesn't provide a solution or help figuring it out.
## Solution
- Made `FakeTask` public and added brief documentation providing a use
(none) that helps guide usage (no usage) of FakeTask.
In gamepad.rs, `ButtonSettings` `is_pressed` and `is_released` are both
private, but their implementations use publicly available values.
Keeping them private forces consumers to unnecessarily re-implement this
logic, so just make them public.
## Objective
- Add an arrow gizmo as suggested by #9400
## Solution
(excuse my Protomen music)
https://github.com/bevyengine/bevy/assets/14184826/192adf24-079f-4a4b-a17b-091e892974ec
Wasn't horribly hard when i remembered i can change coordinate systems
whenever I want. Gave them four tips (as suggested by @alice-i-cecile in
discord) instead of trying to decide what direction the tips should
point.
Made the tip length default to 1/10 of the arrow's length, which looked
good enough to me. Hard-coded the angle from the body to the tips to 45
degrees.
## Still TODO
- [x] actual doc comments
- [x] doctests
- [x] `ArrowBuilder.with_tip_length()`
---
## Changelog
- Added `gizmos.arrow()` and `gizmos.arrow_2d()`
- Added arrows to `2d_gizmos` and `3d_gizmos` examples
## Migration Guide
N/A
---------
Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
# Objective
Allows chained systems taking an `In<_>` input parameter to be run as
one-shot systems. This API was mentioned in #8963.
In addition, `run_system(_with_input)` returns the system output, for
any `'static` output type.
## Solution
A new function, `World::run_system_with_input` allows a `SystemId<I, O>`
to be run by providing an `I` value as input and producing `O` as an
output.
`SystemId<I, O>` is now generic over the input type `I` and output type
`O`, along with the related functions and types `RegisteredSystem`,
`RemovedSystem`, `register_system`, `remove_system`, and
`RegisteredSystemError`. These default to `()`, preserving the existing
API, for all of the public types.
---
## Changelog
- Added `World::run_system_with_input` function to allow one-shot
systems that take `In<_>` input parameters
- Changed `World::run_system` and `World::register_system` to support
systems with return types beyond `()`
- Added `Commands::run_system_with_input` command that schedules a
one-shot system with an `In<_>` input parameter
# Objective
- Ensure ExtendedMaterial can be referenced in bevy_egui_inspector
correctly
## Solution
Add a more manual `TypePath` implementation to work around bugs in the
derive macro.
# Objective
Make sure a camera which has had its render target changed recomputes
its info.
On main, the following is possible:
- System A has an inactive camera with render target set to the default
`Image` (i.e. white 1x1 rgba texture)
Later:
- System B sets the same camera active and sets the `camera.target` to a
newly created `Image`
**Bug**: Since `camera_system` only checks `Modified` and not `Added`
events, the size of the render target is not recomputed, which means the
camera will render with 1x1 size even though the new target is an
entirely different size.
## Solution
- Ensure `camera_system` checks `Added` image assets events
## Changelog
### Fixed
- Cameras which have their render targets changed to a newly created
target with a different size than the previous target will now render
properly
---------
Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Afonso Lage <lage.afonso@gmail.com>
Fix a precision issue with in the manual near-clipping function.
This only affected lines that span large distances (starting at 100_000~
units) in my testing.
Fixes#10403
# Objective
- Reduce work from inactive cameras
Tracing was done on the `3d_shapes` example on PR
https://github.com/bevyengine/bevy/pull/10543 .
Doing tracing on a "real" application showed more instances of
unnecessary work.
## Solution
- Skip work on inactive cameras
Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
# Objective
The quality of Bevy's text rendering can vary wildly depending on the
font, font size, pixel alignment and scale factor.
But this situation can be improved dramatically with some small
adjustments.
## Solution
* Text node positions are rounded to the nearest physical pixel before
rendering.
* Each glyph texture has a 1-pixel wide transparent border added along
its edges.
This means font atlases will use more memory because of the extra pixel
of padding for each glyph but it's more than worth it I think (although
glyph size is increased by 2 pixels on both axes, the net increase is 1
pixel as the font texture atlas's padding has been removed).
## Results
Screenshots are from the 'ui' example with a scale factor of 1.5.
Things can get much uglier with the right font and worst scale
factor<sup>tm</sup>.
### before
<img width="300" alt="list-bad-text"
src="https://github.com/bevyengine/bevy/assets/27962798/482b384d-8743-4bae-9a65-468ff1b4c301">
### after
<img width="300" alt="good_list_text"
src="https://github.com/bevyengine/bevy/assets/27962798/34323b0a-f714-47ba-9728-a59804987bc8">
---
## Changelog
* Font texture atlases are no longer padded.
* Each glyph texture has a 1-pixel wide padding added along its edges.
* Text node positions are rounded to the nearest physical pixel before
rendering.
# Objective
Fixes#10436
Alternative to #10465
## Solution
`load_untyped_async` / `load_internal` currently has a bug. In
`load_untyped_async`, we pass None into `load_internal` for the
`UntypedHandle` of the labeled asset path. This results in a call to
`get_or_create_path_handle_untyped` with `loader.asset_type_id()`
This is a new code path that wasn't hit prior to the newly added
`load_untyped` because `load_untyped_async` was a private method only
used in the context of the `load_folder` impl (which doesn't have
labels)
The fix required some refactoring to catch that case and defer handle
retrieval. I have also made `load_untyped_async` public as it is now
"ready for public use" and unlocks new scenarios.
# Objective
Currently, if a large amount of inactive cameras are spawned, they will
immensely slow down performance.
This can be reproduced by adding
```rust
let default_image = images.add(default());
for _ in 0..10000 {
commands.spawn(Camera3dBundle {
camera: Camera {
is_active: false,
target: RenderTarget::Image(default_image.clone()),
..default()
},
..default()
});
}
```
to for example `3d_shapes`.
Using `tracy`, it's clear that preparing view bind groups for all
cameras is still happening.
Also, visibility checks on the extracted views from inactive cameras
also take place.
## Performance gains
The following `tracy` comparisons show the effect of skipping this
unneeded work.
Yellow is Bevy main, red is with the fix.
### Visibility checks
### Bind group preparation
## Solution
- Check if the cameras are inactive in the appropriate places, and if so
skip them
## Changelog
### Changed
- Do not extract views from inactive cameras or check visiblity from
their extracted views
Signed-off-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
Co-authored-by: Torstein Grindvik <torstein.grindvik@muybridge.com>
(This is my first PR here, so I've probably missed some things. Please
let me know what else I should do to help you as a reviewer!)
# Objective
Due to https://github.com/rust-lang/rust/issues/117800, the `derive`'d
`PartialEq::eq` on `Entity` isn't as good as it could be. Since that's
used in hashtable lookup, let's improve it.
## Solution
The derived `PartialEq::eq` short-circuits if the generation doesn't
match. However, having a branch there is sub-optimal, especially on
64-bit systems like x64 that could just load the whole `Entity` in one
load anyway.
Due to complications around `poison` in LLVM and the exact details of
what unsafe code is allowed to do with reference in Rust
(https://github.com/rust-lang/unsafe-code-guidelines/issues/346), LLVM
isn't allowed to completely remove the short-circuiting. `&Entity` is
marked `dereferencable(8)` so LLVM knows it's allowed to *load* all 8
bytes -- and does so -- but it has to assume that the `index` might be
undef/poison if the `generation` doesn't match, and thus while it finds
a way to do it without needing a branch, it has to do something slightly
more complicated than optimal to combine the results. (LLVM is allowed
to change non-short-circuiting code to use branches, but not the other
way around.)
Here's a link showing the codegen today:
<https://rust.godbolt.org/z/9WzjxrY7c>
```rust
#[no_mangle]
pub fn demo_eq_ref(a: &Entity, b: &Entity) -> bool {
a == b
}
```
ends up generating the following assembly:
```asm
demo_eq_ref:
movq xmm0, qword ptr [rdi]
movq xmm1, qword ptr [rsi]
pcmpeqd xmm1, xmm0
pshufd xmm0, xmm1, 80
movmskpd eax, xmm0
cmp eax, 3
sete al
ret
```
(It's usually not this bad in real uses after inlining and LTO, but it
makes a strong demo.)
This PR manually implements `PartialEq::eq` *without* short-circuiting,
and because that tells LLVM that neither the generations nor the index
can be poison, it doesn't need to be so careful and can generate the
"just compare the two 64-bit values" code you'd have probably already
expected:
```asm
demo_eq_ref:
mov rax, qword ptr [rsi]
cmp qword ptr [rdi], rax
sete al
ret
```
Since this doesn't change the representation of `Entity`, if it's
instead passed by *value*, then each `Entity` is two `u32` registers,
and the old and the new code do exactly the same thing. (Other
approaches, like changing `Entity` to be `[u32; 2]` or `u64`, affect
this case.)
This should hopefully merge easily with changes like
https://github.com/bevyengine/bevy/pull/9907 that also want to change
`Entity`.
## Benchmarks
I'm not super-confident that I got my machine fully consistent for
benchmarking, but whether I run the old or the new one first I get
reasonably consistent results.
Here's a fairly typical example of the benchmarks I added in this PR:
Building the sets seems to be basically the same. It's usually reported
as noise, but sometimes I see a few percent slower or faster.
But lookup hits in particular -- since a hit checks that the key is
equal -- consistently shows around 10% improvement.
`cargo run --example many_cubes --features bevy/trace_tracy --release --
--benchmark` showed as slightly faster with this change, though if I had
to bet I'd probably say it's more noise than meaningful (but at least
it's not worse either):
This is my first PR here -- and my first time running Tracy -- so please
let me know what else I should run, or run things on your own more
reliable machines to double-check.
---
## Changelog
(probably not worth including)
Changed: micro-optimized `Entity::eq` to help LLVM slightly.
## Migration Guide
(I really hope nobody was using this on uninitialized entities where
sufficiently tortured `unsafe` could could technically notice that this
has changed.)
# Objective
After #9002, it seems that "single shot" animations were broken. When
completing, they would reset to their initial value. Which is generally
not what you want.
- Fixes#10480
## Solution
Avoid `%`-ing the animation after the number of completions exceeds the
specified one. Instead, we early-return. This is also true when the
player is playing in reverse.
---
## Changelog
- Avoid resetting animations after `Repeat::Never` animation completion.
