# Objective
Based on the discussion in #14864, I wanted to experiment with the core
`GenericTypeCell` type, whose `get_or_insert` method accounted for 2% of
the final binary size of the `3d_scene` example. The reason for this
large percentage is likely because the type is fundamental to the rest
of Bevy while having 3 generic parameters (the type stored `T`, the type
to retrieve `G`, and the function used to insert a new value `F`).
- Acts on #14864
## Solution
- Split `get_or_insert` into smaller functions with minimised
parameterisation. These new functions are private as to preserve the
public facing API, but could be exposed if desired.
## Testing
- Ran CI locally.
- Used `cargo bloat --release --example 3d_scene -n 100000
--message-format json > out.json` and @cart's [bloat
analyzer](https://gist.github.com/cart/722756ba3da0e983d207633e0a48a8ab)
to measure a 428KiB reduction in binary size when compiling on Windows
10.
- ~I have _not_ benchmarked to determine if this improves/hurts
performance.~ See
[below](https://github.com/bevyengine/bevy/pull/14865#issuecomment-2306083606).
## Notes
In my opinion this seems like a good test-case for the concept of
debloating generics within the Bevy codebase. I believe the performance
impact here is negligible in either direction (at runtime and compile
time), but the binary reduction is measurable and quite significant for
a relatively minor change in code.
---------
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
# Objective
Citing @mweatherley
> As mentioned before, a multi-sampling function in the API which takes
an iterator is probably something we want (e.g. `sample_iter(iter: impl
IntoIterator<Item = f32>) -> impl IntoIterator<Item = T> { //... }`, but
there are some design choices to be made on the details (e.g. does this
filter out points that aren't in the domain? does it do sorting? etc.)
## Solution
I think the most flexible solution for end users is to expose all the
`sample_...` functions with an `iter` equivalent, so we'll have
- `sample_iter`
- `sample_iter_unchecked`
- `sample_iter_clamped`
Answering some questions from the original idea:
> does this filter out points that aren't in the domain?
With the methods the user has the choice to just sample or if they want
to filter out invalid types us `sample_iter` and then apply `filter_map`
to the iterator returned themselves.
> does it do sorting?
I think it's the same thing. If the user wants it, they need to do it
themselves by either collecting and sorting a `Vec` or using
`itertools`. I think there is a legit use case for "please sample me
this collection of points that are unordered" and we would destroy it if
we take away to much agency from users by sorting for them
## Testing
- Added a test which covers all three methods
# Objective
Add `bevy_picking` sprite backend as part of the `bevy_mod_picking`
upstreamening (#12365).
## Solution
More or less a copy/paste from `bevy_mod_picking`, with the changes
[here](https://github.com/aevyrie/bevy_mod_picking/pull/354). I'm
putting that link here since those changes haven't yet made it through
review, so should probably be reviewed on their own.
## Testing
I couldn't find any sprite-backend-specific tests in `bevy_mod_picking`
and unfortunately I'm not familiar enough with Bevy's testing patterns
to write tests for code that relies on windowing and input. I'm willing
to break the pointer hit system into testable blocks and add some more
modular tests if that's deemed important enough to block, otherwise I
can open an issue for adding tests as follow-up.
## Follow-up work
- More docs/tests
- Ignore pick events on transparent sprite pixels with potential opt-out
---------
Co-authored-by: Aevyrie <aevyrie@gmail.com>
# Objective
`arc_2d` wasn't actually doing what the docs were saying. The arc wasn't
offset by what was previously `direction_angle` but by `direction_angle
- arc_angle / 2.0`. This meant that the arcs center was laying on the
`Vec2::Y` axis and then it was offset. This was probably done to fit the
behavior of the `Arc2D` primitive. I would argue that this isn't
desirable for the plain `arc_2d` gizmo method since
- a) the docs get longer to explain the weird centering
- b) the mental model the user has to know gets bigger with more
implicit assumptions
given the code
```rust
my_gizmos.arc_2d(Vec2::ZERO, 0.0, FRAC_PI_2, 75.0, ORANGE_RED);
```
we get
where after the fix with
```rust
my_gizmos.arc_2d(Isometry2d::IDENTITY, FRAC_PI_2, 75.0, ORANGE_RED);
```
we get
To get the same result with the previous implementation you would have
to randomly add `arc_angle / 2.0` to the `direction_angle`.
```rust
my_gizmos.arc_2d(Vec2::ZERO, FRAC_PI_4, FRAC_PI_2, 75.0, ORANGE_RED);
```
This makes constructing similar helping functions as they already exist
in 3D like
- `long_arc_2d_between`
- `short_arc_2d_between`
much harder.
## Solution
- Make the arc really start at `Vec2::Y * radius` in counter-clockwise
direction + offset by an angle as the docs state it
- Use `Isometry2d` instead of `position : Vec2` and `direction_angle :
f32` to reduce the chance of messing up rotation/translation
- Adjust the docs for the changes above
- Adjust the gizmo rendering of some primitives
## Testing
- check `2d_gizmos.rs` and `render_primitives.rs` examples
## Migration Guide
- users have to adjust their usages of `arc_2d`:
- before:
```rust
arc_2d(
pos,
angle,
arc_angle,
radius,
color
)
```
- after:
```rust
arc_2d(
// this `+ arc_angle * 0.5` quirk is only if you want to preserve the
previous behavior
// with the new API.
// feel free to try to fix this though since your current calls to this
function most likely
// involve some computations to counter-act that quirk in the first
place
Isometry2d::new(pos, Rot2::radians(angle + arc_angle * 0.5),
arc_angle,
radius,
color
)
```
# Objective
- Faster meshlet rasterization path for small triangles
- Avoid having to allocate and write out a triangle buffer
- Refactor gpu_scene.rs
## Solution
- Replace the 32bit visbuffer texture with a 64bit visbuffer buffer,
where the left 32 bits encode depth, and the right 32 bits encode the
existing cluster + triangle IDs. Can't use 64bit textures, wgpu/naga
doesn't support atomic ops on textures yet.
- Instead of writing out a buffer of packed cluster + triangle IDs (per
triangle) to raster, the culling pass now writes out a buffer of just
cluster IDs (per cluster, so less memory allocated, cheaper to write
out).
- Clusters for software raster are allocated from the left side
- Clusters for hardware raster are allocated in the same buffer, from
the right side
- The buffer size is fixed at MeshletPlugin build time, and should be
set to a reasonable value for your scene (no warning on overflow, and no
good way to determine what value you need outside of renderdoc - I plan
to fix this in a future PR adding a meshlet stats overlay)
- Currently I don't have a heuristic for software vs hardware raster
selection for each cluster. The existing code is just a placeholder. I
need to profile on a release scene and come up with a heuristic,
probably in a future PR.
- The culling shader is getting pretty hard to follow at this point, but
I don't want to spend time improving it as the entire shader/pass is
getting rewritten/replaced in the near future.
- Software raster is a compute workgroup per-cluster. Each workgroup
loads and transforms the <=64 vertices of the cluster, and then
rasterizes the <=64 triangles of the cluster.
- Two variants are implemented: Scanline for clusters with any larger
triangles (still smaller than hardware is good at), and brute-force for
very very tiny triangles
- Once the shader determines that a pixel should be filled in, it does
an atomicMax() on the visbuffer to store the results, copying how Nanite
works
- On devices with a low max workgroups per dispatch limit, an extra
compute pass is inserted before software raster to convert from a 1d to
2d dispatch (I don't think 3d would ever be necessary).
- I haven't implemented the top-left rule or subpixel precision yet, I'm
leaving that for a future PR since I get usable results without it for
now
- Resources used:
https://kristoffer-dyrkorn.github.io/triangle-rasterizer and chapters
6-8 of
https://fgiesen.wordpress.com/2013/02/17/optimizing-sw-occlusion-culling-index
- Hardware raster now spawns 64*3 vertex invocations per meshlet,
instead of the actual meshlet vertex count. Extra invocations just
early-exit.
- While this is slower than the existing system, hardware draws should
be rare now that software raster is usable, and it saves a ton of memory
using the unified cluster ID buffer. This would be fixed if wgpu had
support for mesh shaders.
- Instead of writing to a color+depth attachment, the hardware raster
pass also does the same atomic visbuffer writes that software raster
uses.
- We have to bind a dummy render target anyways, as wgpu doesn't
currently support render passes without any attachments
- Material IDs are no longer written out during the main rasterization
passes.
- If we had async compute queues, we could overlap the software and
hardware raster passes.
- New material and depth resolve passes run at the end of the visbuffer
node, and write out view depth and material ID depth textures
### Misc changes
- Fixed cluster culling importing, but never actually using the previous
view uniforms when doing occlusion culling
- Fixed incorrectly adding the LOD error twice when building the meshlet
mesh
- Splitup gpu_scene module into meshlet_mesh_manager, instance_manager,
and resource_manager
- resource_manager is still too complex and inefficient (extract and
prepare are way too expensive). I plan on improving this in a future PR,
but for now ResourceManager is mostly a 1:1 port of the leftover
MeshletGpuScene bits.
- Material draw passes have been renamed to the more accurate material
shade pass, as well as some other misc renaming (in the future, these
will be compute shaders even, and not actual draw calls)
---
## Migration Guide
- TBD (ask me at the end of the release for meshlet changes as a whole)
---------
Co-authored-by: vero <email@atlasdostal.com>
# Objective
When using instancing, 2 `VertexBufferLayout`s are needed, one for
per-vertex and one for per-instance data. Shader locations of all
attributes must not overlap, so one of the layouts needs to start its
locations at an offset. However,
`VertexBufferLayout::from_vertex_formats` will always start locations at
0, requiring manual adjustment, which is currently pretty verbose.
## Solution
Add `VertexBufferLayout::offset_locations`, which adds an offset to all
attribute locations.
Code using this method looks like this:
```rust
VertexState {
shader: BACKBUFFER_SHADER_HANDLE.typed(),
shader_defs: Vec::new(),
entry_point: "vertex".into(),
buffers: vec![
VertexBufferLayout::from_vertex_formats(
VertexStepMode::Vertex,
[VertexFormat::Float32x2],
),
VertexBufferLayout::from_vertex_formats(
VertexStepMode::Instance,
[VertexFormat::Float32x2, VertexFormat::Float32x3],
)
.offset_locations(1),
],
}
```
Alternative solutions include:
- Pass the starting location to `from_vertex_formats` – this is a bit
simpler than my solution here, but most calls don't need an offset, so
they'd always pass 0 there.
- Do nothing and make the user hand-write this.
---
## Changelog
- Add `VertexBufferLayout::offset_locations` to simplify buffer layout
construction when using instancing.
---------
Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Adding more features to `AsBindGroup` proc macro means making the trait
arguments uglier. Downstream implementors of the trait without the proc
macro might want to do different things than our default arguments.
## Solution
Make `AsBindGroup` take an associated `Param` type.
## Migration Guide
`AsBindGroup` now allows the user to specify a `SystemParam` to be used
for creating bind groups.
# Objective
Closes#7622.
I was working on adding support for reflecting generic functions and
found that I wanted to use an argument's `TypeId` for hashing and
comparison, but its `TypePath` for debugging and error messaging.
While I could just keep them separate, place them in a tuple or a local
struct or something, I think I see an opportunity to make a dedicate
type for this.
Additionally, we can use this type to clean up some duplication amongst
the type info structs in a manner similar to #7622.
## Solution
Added the `Type` type. This should be seen as the most basic
representation of a type apart from `TypeId`. It stores both the
`TypeId` of the type as well as its `TypePathTable`.
The `Hash` and `PartialEq` implementations rely on the `TypeId`, while
the `Debug` implementation relies on the `TypePath`.
This makes it especially useful as a key in a `HashMap` since we get the
speed of the `TypeId` hashing/comparisons with the readability of
`TypePath`.
With this type, we're able to reduce the duplication across the type
info structs by removing individual fields for `TypeId` and
`TypePathTable`, replacing them with a single `Type` field. Similarly,
we can remove many duplicate methods and replace it with a macro that
delegates to the stored `Type`.
### Caveats
It should be noted that this type is currently 3x larger than `TypeId`.
On my machine, it's 48 bytes compared to `TypeId`'s 16. While this
doesn't matter for `TypeInfo` since it would contain that data
regardless, it is something to keep in mind when using elsewhere.
## Testing
All tests should pass as normal:
```
cargo test --package bevy_reflect
```
---
## Showcase
`bevy_reflect` now exports a `Type` struct. This type contains both the
`TypeId` and the `TypePathTable` of the given type, allowing it to be
used like `TypeId` but have the debuggability of `TypePath`.
```rust
// We can create this for any type implementing `TypePath`:
let ty = Type::of::<String>();
// It has `Hash` and `Eq` impls powered by `TypeId`, making it useful for maps:
let mut map = HashMap::<Type, i32>::new();
map.insert(ty, 25);
// And it has a human-readable `Debug` representation:
let debug = format!("{:?}", map);
assert_eq!(debug, "{alloc::string::String: 25}");
```
## Migration Guide
Certain type info structs now only return their item types as `Type`
instead of exposing direct methods on them.
The following methods have been removed:
- `ArrayInfo::item_type_path_table`
- `ArrayInfo::item_type_id`
- `ArrayInfo::item_is`
- `ListInfo::item_type_path_table`
- `ListInfo::item_type_id`
- `ListInfo::item_is`
- `SetInfo::value_type_path_table`
- `SetInfo::value_type_id`
- `SetInfo::value_is`
- `MapInfo::key_type_path_table`
- `MapInfo::key_type_id`
- `MapInfo::key_is`
- `MapInfo::value_type_path_table`
- `MapInfo::value_type_id`
- `MapInfo::value_is`
Instead, access the `Type` directly using one of the new methods:
- `ArrayInfo::item_ty`
- `ListInfo::item_ty`
- `SetInfo::value_ty`
- `MapInfo::key_ty`
- `MapInfo::value_ty`
For example:
```rust
// BEFORE
let type_id = array_info.item_type_id();
// AFTER
let type_id = array_info.item_ty().id();
```
# Objective
I tried writing something like this in my project
```rust
.observe(|e: Trigger<OnAdd, Skeleton>| {
panic!("Skeletoned! {e:?}");
});
```
and it didn't compile.
Having `Debug` trait defined on `Trigger` event will ease debugging the
observers a little bit.
## Solution
Add a bespoke `Debug` implementation when both the bundle and the event
have `Debug` implemented for them.
## Testing
I've added `println!("{trigger:#?}");` to the [observers
example](938d810766/examples/ecs/observers.rs (L124))
and it compiled!
Caveats with this PR are:
- removing this implementation if for any reason we will need it, will
be a breaking change
- the implementation is manually generated, which adds potential toil
when changing the `Trigger` structure
## Showcase
Log output:
```rust
on_add_mine: Trigger {
event: OnAdd,
propagate: false,
trigger: ObserverTrigger {
observer: 2v1#4294967298,
event_type: ComponentId(
0,
),
entity: 454v1#4294967750,
},
_marker: PhantomData<observers::Mine>,
}
```
Thank you for maintaining this engine! 🧡
# Objective
- Gizmo rendering on WebGPU has been fixed by #14653, but gizmo joints
still cause error
(https://github.com/bevyengine/bevy/issues/14696#issuecomment-2283689669)
when enabled.
## Solution
- Applies the same fix as #14653 to Gizmo joints.
I'm noob and just copied their solution, please correct me if I did
something wrong.
## Testing
- Tested 2d-gizmos and 3d-gizmos examples in WebGPU on Chrome. No
rendering errors, and the gizmo joints are apparently rendered ok.
# Objective
Support building systems with parameters whose types can be determined
at runtime.
## Solution
Create a `DynSystemParam` type that can be built using a
`SystemParamBuilder` of any type and then downcast to the appropriate
type dynamically.
## Example
```rust
let system = (
DynParamBuilder::new(LocalBuilder(3_usize)),
DynParamBuilder:🆕:<Query<()>>(QueryParamBuilder::new(|builder| {
builder.with::<A>();
})),
DynParamBuilder:🆕:<&Entities>(ParamBuilder),
)
.build_state(&mut world)
.build_system(
|mut p0: DynSystemParam, mut p1: DynSystemParam, mut p2: DynSystemParam| {
let local = p0.downcast_mut::<Local<usize>>().unwrap();
let query_count = p1.downcast_mut::<Query<()>>().unwrap();
let entities = p2.downcast_mut::<&Entities>().unwrap();
},
);
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Periwink <charlesbour@gmail.com>
# Objective
- Remove the `wgpu_trace` feature while still making it easy/possible to
record wgpu traces for debugging.
- Close#14725.
- Get a taste of the bevy codebase. :P
## Solution
This PR performs the above objective by removing the `wgpu_trace`
feature from all `Cargo.toml` files.
However, wgpu traces are still useful for debugging - but to record
them, you need to pass in a directory path to store the traces in. To
avoid forcing users into manually creating the renderer,
`bevy_render::settings::WgpuSettings` now has a `trace_path` field, so
that all of Bevy's automatic initialization can happen while still
allowing for tracing.
## Testing
- Did you test these changes? If so, how?
- I have tested these changes, but only via running `cargo run -p ci`. I
am hoping the Github Actions workflows will catch anything I missed.
- Are there any parts that need more testing?
- I do not believe so.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If you want to test these changes, I have updated the debugging guide
(`docs/debugging.md`) section on WGPU Tracing.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
- I ran the above command on a Windows 10 64-bit (x64) machine, using
the `stable-x86_64-pc-windows-msvc` toolchain. I do not have anything
set up for other platforms or targets (though I can't imagine this needs
testing on other platforms).
---
## Migration Guide
1. The `bevy/wgpu_trace`, `bevy_render/wgpu_trace`, and
`bevy_internal/wgpu_trace` features no longer exist. Remove them from
your `Cargo.toml`, CI, tooling, and what-not.
2. Follow the instructions in the updated `docs/debugging.md` file in
the repository, under the WGPU Tracing section.
Because of the changes made, you can now generate traces to any path,
rather than the hardcoded `%WorkspaceRoot%/wgpu_trace` (where
`%WorkspaceRoot%` is... the root of your crate's workspace) folder.
(If WGPU hasn't restored tracing functionality...) Do note that WGPU has
not yet restored tracing functionality. However, once it does, the above
should be sufficient to generate new traces.
---------
Co-authored-by: TrialDragon <31419708+TrialDragon@users.noreply.github.com>
# Objective
Add a convenience constructor to make simple animation graphs easier to
build.
I've had some notes about attempting this since #11989 that I just
remembered after seeing #14852.
This partially addresses #14852, but I don't really know animation well
enough to write all of the documentation it's asking for.
## Solution
Add `AnimationGraph::from_clips` and use it to simplify `animated_fox`.
Do some other little bits of incidental cleanup and documentation .
## Testing
I ran `cargo run --example animated_fox`.
# Objective
- Fixes#14658.
## Solution
- Added `on_unimplemented` Diagnostic for `IntoObserverSystem` calling
out argument ordering in a `note`
- Added an example to the documentation on `App::observe` to provide
some explanation to users.
## Testing
- Ran CI locally
- Deliberately introduced a parameter order error in the
`ecs/observers.rs` example as a test.
---
## Showcase
<details>
<summary>Error Before</summary>
```
error[E0277]: the trait bound `{closure@examples/ecs/observers.rs:19:13: 22:37}: IntoObserverSystem<_, _, _>` is not satisfied
--> examples/ecs/observers.rs:19:13
|
18 | .observe(
| ------- required by a bound introduced by this call
19 | / |mines: Query<&Mine>,
20 | | trigger: Trigger<ExplodeMines>,
21 | | index: Res<SpatialIndex>,
22 | | mut commands: Commands| {
... |
34 | | }
35 | | },
| |_____________^ the trait `bevy::prelude::IntoSystem<bevy::prelude::Trigger<'static, _, _>, (), _>` is not implemented for closure `{closure@examples/ecs/observers.rs:19:13: 22:37}`, which is required by `{closure@examples/ecs/observers.rs:19:13: 22:37}: IntoObserverSystem<_, _, _>`
|
= note: required for `{closure@examples/ecs/observers.rs:19:13: 22:37}` to implement `IntoObserverSystem<_, _, _>`
note: required by a bound in `bevy::prelude::App::observe`
--> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:995:24
|
993 | pub fn observe<E: Event, B: Bundle, M>(
| ------- required by a bound in this associated function
994 | &mut self,
995 | observer: impl IntoObserverSystem<E, B, M>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `App::observe`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `bevy` (example "observers") due to 1 previous error
```
</details>
<details>
<summary>Error After</summary>
```
error[E0277]: `{closure@examples/ecs/observers.rs:19:13: 22:37}` cannot become an `ObserverSystem`
--> examples/ecs/observers.rs:19:13
|
18 | .observe(
| ------- required by a bound introduced by this call
19 | / |mines: Query<&Mine>,
20 | | trigger: Trigger<ExplodeMines>,
21 | | index: Res<SpatialIndex>,
22 | | mut commands: Commands| {
... |
34 | | }
35 | | },
| |_____________^ the trait `IntoObserverSystem` is not implemented
|
= help: the trait `bevy::prelude::IntoSystem<bevy::prelude::Trigger<'static, _, _>, (), _>` is not implemented for closure `{closure@examples/ecs/observers.rs:19:13: 22:37}`, which is required by `{closure@examples/ecs/observers.rs:19:13: 22:37}: IntoObserverSystem<_, _, _>`
= note: for function `ObserverSystem`s, ensure the first argument is a `Trigger<T>` and any subsequent ones are `SystemParam`
= note: required for `{closure@examples/ecs/observers.rs:19:13: 22:37}` to implement `IntoObserverSystem<_, _, _>`
note: required by a bound in `bevy::prelude::App::observe`
--> C:\Users\Zac\Documents\GitHub\bevy\crates\bevy_app\src\app.rs:1025:24
|
1023 | pub fn observe<E: Event, B: Bundle, M>(
| ------- required by a bound in this associated function
1024 | &mut self,
1025 | observer: impl IntoObserverSystem<E, B, M>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `App::observe`
For more information about this error, try `rustc --explain E0277`.
error: could not compile `bevy` (example "observers") due to 1 previous error
```
</details>
# Objective
- The examples use a more verbose than necessary way to initialize the
image
- The order of the camera doesn't need to be specified. At least I
didn't see a difference in my testing
## Solution
- Use `Image::new_fill()` to fill the image instead of abusing
`resize()`
- Remove the camera ordering
# Objective
Rewrite screenshotting to be able to accept any `RenderTarget`.
Closes#12478
## Solution
Previously, screenshotting relied on setting a variety of state on the
requested window. When extracted, the window's `swap_chain_texture_view`
property would be swapped out with a texture_view created that frame for
the screenshot pipeline to write back to the cpu.
Besides being tightly coupled to window in a way that prevented
screenshotting other render targets, this approach had the drawback of
relying on the implicit state of `swap_chain_texture_view` being
returned from a `NormalizedRenderTarget` when view targets were
prepared. Because property is set every frame for windows, that wasn't a
problem, but poses a problem for render target images. Namely, to do the
equivalent trick, we'd have to replace the `GpuImage`'s texture view,
and somehow restore it later.
As such, this PR creates a new `prepare_view_textures` system which runs
before `prepare_view_targets` that allows a new `prepare_screenshots`
system to be sandwiched between and overwrite the render targets texture
view if a screenshot has been requested that frame for the given target.
Additionally, screenshotting itself has been changed to use a component
+ observer pattern. We now spawn a `Screenshot` component into the
world, whose lifetime is tracked with a series of marker components.
When the screenshot is read back to the CPU, we send the image over a
channel back to the main world where an observer fires on the screenshot
entity before being despawned the next frame. This allows the user to
access resources in their save callback that might be useful (e.g.
uploading the screenshot over the network, etc.).
## Testing
TODO:
- [x] Web
- [ ] Manual texture view
---
## Showcase
render to texture example:
<img
src="https://github.com/user-attachments/assets/612ac47b-8a24-4287-a745-3051837963b0"
width=200/>
web saving still works:
<img
src="https://github.com/user-attachments/assets/e2a15b17-1ff5-4006-ab2a-e5cc74888b9c"
width=200/>
## Migration Guide
`ScreenshotManager` has been removed. To take a screenshot, spawn a
`Screenshot` entity with the specified render target and provide an
observer targeting the `ScreenshotCaptured` event. See the
`window/screenshot` example to see an example.
---------
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
# Objective
`ParamSetBuilder` is supposed to be used as a tuple constructor, but the
field was not marked `pub` so it's not actually usable outside of its
module.
## Solution
Mark the field `pub`.
Realize one advantage of doc tests over unit tests is that they test the
public API.
Add a doc test example that uses the field so that this would have been
caught.
# Objective
There is a tiny seam at the top of the annulus caused by normal
floating-point error in calculating the coordinates. When generating the
last pair of triangles, given `n == i` then `(TAU / n) * i` does not
equal `TAU` exactly.
Fixes https://github.com/komadori/bevy_mod_outline/issues/42
## Solution
This can be fixed by changing the calculation so that `(TAU / n) * (i %
n) == 0.0`, which is equivalent for trigonometric purposes.
## Testing
Added the unit test
`bevy_render::mesh::primitives::dim2::tests::test_annulus`.
Fixes#14825
Edit: After feedback, these are the updated methods:
- `toggle_inherited_visible(&mut self)`
- Toggles between `Visibility::Inherited` and `Visibility::Visible`. If
the value is `Visibility::Hidden`, it remains unaffected.
- `toggle_inherited_hidden(&mut self)`
- Toggles between `Visibility::Inherited` and `Visibility::Hidden`. If
the value is `Visibility::Visible`, it remains unaffected.
- `toggle_visible_hidden(&mut self)`
- Toggles between `Visibility::Visible` and `Visibility::Hidden`. If the
value is `Visibility::Inherited`, it remains unaffected.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
- There is a flaw in the implementation of `FogVolume`'s
`density_texture_offset` from #14868. Because of the way I am wrapping
the UVW coordinates in the volumetric fog shader, a seam is visible when
the 3d texture is wrapping around from one side to the other:
## Solution
- This PR fixes the issue by removing the wrapping from the shader and
instead leaving it to the user to configure the 3d noise texture to use
`ImageAddressMode::Repeat` if they want it to repeat. Using
`ImageAddressMode::Repeat` is the proper solution to avoid the obvious
seam:
- The sampler cannot be implicitly configured to use
`ImageAddressMode::Repeat` because that's not always desirable. For
example, the `fog_volumes` example wouldn't work properly because the
texture from the edges of the volume would overflow to the other sides,
which would be bad in this instance (but it's good in the case of the
`scrolling_fog` example). So leaving it to the user to decide on their
own whether they want the density texture to repeat seems to be the best
solution.
## Testing
- The `scrolling_fog` example still looks the same, it was just changed
to explicitly declare that the density texture should be repeating when
loading the asset. The `fog_volumes` example is unaffected.
<details>
<summary>Minimal reproduction example on current main</summary>
<pre>
use bevy::core_pipeline::experimental::taa::{TemporalAntiAliasBundle,
TemporalAntiAliasPlugin};
use bevy::pbr::{FogVolume, VolumetricFogSettings, VolumetricLight};
use bevy::prelude::*;<br>
fn main() {
App::new()
.add_plugins((DefaultPlugins, TemporalAntiAliasPlugin))
.add_systems(Startup, setup)
.run();
}<br>
fn setup(mut commands: Commands, assets: Res<AssetServer>) {
commands.spawn((
Camera3dBundle {
transform: Transform::from_xyz(3.5, -1.0, 0.4)
.looking_at(Vec3::new(0.0, 0.0, 0.4), Vec3::Y),
msaa: Msaa::Off,
..default()
},
TemporalAntiAliasBundle::default(),
VolumetricFogSettings {
ambient_intensity: 0.0,
jitter: 0.5,
..default()
},
));<br>
commands.spawn((
DirectionalLightBundle {
transform: Transform::from_xyz(-6.0, 5.0, -9.0)
.looking_at(Vec3::new(0.0, 0.0, 0.0), Vec3::Y),
directional_light: DirectionalLight {
illuminance: 32_000.0,
shadows_enabled: true,
..default()
},
..default()
},
VolumetricLight,
));<br>
commands.spawn((
SpatialBundle {
visibility: Visibility::Visible,
transform: Transform::from_xyz(0.0, 0.0,
0.0).with_scale(Vec3::splat(3.0)),
..default()
},
FogVolume {
density_texture: Some(assets.load("volumes/fog_noise.ktx2")),
density_texture_offset: Vec3::new(0.0, 0.0, 0.4),
scattering: 1.0,
..default()
},
));
}
</pre>
</details>
# Objective
This example previously had kind of a needlessly complex state machine
that tracked moves between its previous orientation and the new one that
was randomly generated. Using `smooth_nudge` simplifies the example in
addition to making good use of the new API.
## Solution
Use `smooth_nudge` to transition between the current transform and the
new one. This does away with the need to keep track of the move's
starting position and progress. It also just sort of looks nicer.
## Testing
Run the `align` example:
`cargo run --example align`
# Objective
- Fixes#14873, see that issue for a whole lot of context
## Solution
- Add a blessed system set for this stuff. See [this Discord
discussion](https://discord.com/channels/691052431525675048/749335865876021248/1276262931327094908).
Note that the gizmo systems,
[LWIM](https://github.com/Leafwing-Studios/leafwing-input-manager/pull/522/files#diff-9b59ee4899ad0a5d008889ea89a124a7291316532e42f9f3d6ae842b906fb095R154)
and now a new plugin I'm working on are all already ordering against
`run_fixed_main_schedule`, so having a dedicated system set should be
more robust and hopefully also more discoverable.
---
## ~~Showcase~~
~~I can add a little video of a smooth camera later if this gets merged
:)~~
Apparently a release note is not needed, so I'll leave it out. See the
changes in the fixed timestep example for usage showcase and the video
in #14873 for a more or less accurate video of the effect (it does not
use the same solution though, so it is not quite the same)
## Migration Guide
[run_fixed_main_schedule](https://docs.rs/bevy/latest/bevy/time/fn.run_fixed_main_schedule.html)
is no longer public. If you used to order against it, use the new
dedicated `RunFixedMainLoopSystem` system set instead. You can replace
your usage of `run_fixed_main_schedule` one for one by
`RunFixedMainLoopSystem::FixedMainLoop`, but it is now more idiomatic to
place your systems in either
`RunFixedMainLoopSystem::BeforeFixedMainLoop` or
`RunFixedMainLoopSystem::AfterFixedMainLoop`
Old:
```rust
app.add_systems(
RunFixedMainLoop,
some_system.before(run_fixed_main_schedule)
);
```
New:
```rust
app.add_systems(
RunFixedMainLoop,
some_system.in_set(RunFixedMainLoopSystem::BeforeFixedMainLoop)
);
```
---------
Co-authored-by: Tau Gärtli <git@tau.garden>
# Objective
When trying to test a gizmos change I ran `cargo test -p bevy_gizmos`
and the output had a lot of noise from warnings and failed doc errors.
This was because I didn't have all of the features enabled.
## Solution
I admit this might be pedantic, and am happy if the concensus is to
reject it. Although it does reduce the lines of code, testing noise, and
the amount of code compiled. I don't think it affects the complexity of
public code, and it doesn't change much to the complexity of internal
code.
I've removed un-needed `bevy_render` imports in all of the gizmos docs
examples, there's probably other unnecessary ones there too, but I
haven't looked exhaustively. It isn't needed for those docs, and isn't
available except in a subset of `cfg` combinations.
I've also made several of the `use` statements slightly more specific. I
shouldn't have changed the public interfaces, except that
`GizmoMeshConfig` requires either `bevy_sprite` or `bevy_pbr`, as it
does nothing without them.
I've also avoided adding some systems and plugins in situations where
they can't work. An example of this is where the `light` module depends
on `all(feature = "bevy_pbr", feature = "bevy_render")`, but it has
`use` statements that only require `bevy_render`.
## Testing
During development I ran:
```
cargo check -p bevy_gizmos && cargo check -p bevy_gizmos --features=bevy_pbr && cargo check -p bevy_gizmos --features=bevy_sprite && cargo check -p bevy_gizmos --features=bevy_render
```
Afterwards I ran this just to be sure:
```
cargo check && cargo check --features=bevy_pbr && cargo check --features=bevy_sprite && cargo check --features=bevy_render
```
Finally I ran:
```
cargo test -p bevy_gizmos && cargo test -p bevy_gizmos --features=bevy_pbr && test check -p bevy_gizmos --features=bevy_sprite && cargo test -p bevy_gizmos --features=bevy_render
```
## Migration Guide
There shouldn't be any reason to migrate, although if for some reason
you use `GizmoMeshConfig` and `bevy_render` but not `bevy_pbr` or
`bevy_sprite` (such that it does nothing), then you will get an error
that it no longer exists.
# Objective
It looks like `Gizmos::grid*` was missed in the colour migration.
## Solution
This updates the `grid` methods and implementation to use `Color`
instead of `LinearRgba`.
It looks like `ExtractedPointLight` and `ExtractedDirectionalLight` also
use `LinearRgba`, although I think in extracted structures it's probably
fine to make more assumptions about what you want?
## Testing
I ran `cargo test --all -- bevy_gizmos` and it passed.
## Migration Guide
This shouldn't be adding anything that isn't already in a migration
guide? I assume as it uses `impl Into<...>` in the public interfaces
that any users of these APIs shouldn't have to make any code changes.
# Objective
Running `cargo ci` on Fedora 40 causes a system crash due to many `ld`
processes consuming all available memory when performing doc tests.
## Solution
This PR extends #13553.
- Add reference to #12207 in the CI sections of `CONTRIBUTING.md` and
`config_fast_builds.toml`.
- Add sample `rustdocflags` configurations for `lld` and `mold` to
`config_fast_builds.toml` for Linux.
## Testing
Crashing configuration
- config.toml
```
[target.x86_64-unknown-linux-gnu]
linker = "clang"
rustflags = ["-Clink-arg=-fuse-ld=lld"]
[alias]
ci = "run --package ci --"
```
- Test command
`cargo ci`
Working configuration
- config.toml
```
[target.x86_64-unknown-linux-gnu]
linker = "clang"
rustflags = ["-Clink-arg=-fuse-ld=lld"]
rustdocflags = ["-Clink-arg=-fuse-ld=lld"]
[alias]
ci = "run --package ci --"
```
- Test command
`cargo ci`
# Objective
- Fixes#14844
## Solution
- implement reflect using the `impl_reflect_value` macro
## Testing
- I wrote a test locally to understand and learn how reflection worked
on a basic level and to confirm that yes indeed the bound struct could
use the reflection traits that have been implemented for it.
note: I did remove a line that asked for bound to not have reflect
implemented in a test, since that's the point of this PR and the test
worked without the line so I am not sure what that was about, not sure
if that uncovers a deeper issue or not.
# Objective
- The goal of this PR is to make it possible to move the density texture
of a `FogVolume` over time in order to create dynamic effects like fog
moving in the wind.
- You could theoretically move the `FogVolume` itself, but this is not
ideal, because the `FogVolume` AABB would eventually leave the area. If
you want an area to remain foggy while also creating the impression that
the fog is moving in the wind, a scrolling density texture is a better
solution.
## Solution
- The PR adds a `density_texture_offset` field to the `FogVolume`
component. This offset is in the UVW coordinates of the density texture,
meaning that a value of `(0.5, 0.0, 0.0)` moves the 3d texture by half
along the x-axis.
- Values above 1.0 are wrapped, a 1.5 offset is the same as a 0.5
offset. This makes it so that the density texture wraps around on the
other side, meaning that a repeating 3d noise texture can seamlessly
scroll forever. It also makes it easy to move the density texture over
time by simply increasing the offset every frame.
## Testing
- A `scrolling_fog` example has been added to demonstrate the feature.
It uses the offset to scroll a repeating 3d noise density texture to
create the impression of fog moving in the wind.
- The camera is looking at a pillar with the sun peaking behind it. This
highlights the effect the changing density has on the volumetric
lighting interactions.
- Temporal anti-aliasing combined with the `jitter` option of
`VolumetricFogSettings` is used to improve the quality of the effect.
---
## Showcase
https://github.com/user-attachments/assets/3aa50ebd-771c-4c99-ab5d-255c0c3be1a8
Closes#14836.
`filter_map_unchanged` optionally maps to an inner value by applying a
function to the contained reference. This is useful in a situation where
you need to convert a `Mut<T>` to a `Mut<U>`, but only if `T` contains
`U`.
---------
Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>
# Objective
- Fixes#14796
## Solution
- Copy docs for wrapper methods, make sure they are consistent with the
original docs except for the section on precision.
# Objective
It looks like running `compile_fail_utils::test` on an invalid path just
skips the test. I'm not sure why `ui_test` doesn't fail the test, but it
seems pretty easy to accidentally cause a test to be skipped (I
experienced
[this](https://github.com/bevyengine/bevy/pull/14813#discussion_r1721880973)
while doing some refactoring on `bevy_reflect`).
## Solution
Check to make sure the given path exists before continuing on with the
tests.
Alternatively, we could look into seeing why this doesn't work properly
upstream. But I figured this solution was simple enough just to
implement directly without having to worry about updating `ui_test`.
## Testing
To verify that this works as expected `cd` into
`crates/bevy_reflect/compile_fail`. Then run the following:
```
cargo test --target-dir ../../../target
```
All compile fail tests should pass. Now edit the path used in
`crates/bevy_reflect/compile_fail/tests/derive.rs`. For example:
```diff
fn main() -> compile_fail_utils::ui_test::Result<()> {
- compile_fail_utils::test("reflect_derive", "tests/reflect_derive")
+ compile_fail_utils::test("reflect_derive", "tests/does_not_exist")
}
```
Run the tests again:
```
cargo test --target-dir ../../../target
```
Verify the test fails with an error like:
```
Error: path does not exist: "tests/does_not_exist"
```
# Objective
Make the naming of a parameter more consistent.
## Solution
- Changing the name of a parameter.
## Testing
These changes can't be tested as they are documentation based.
---
I apologize if something is wrong here, this is my first PR to bevy.
# Objective
`RenderLayers` was marketed as being unlimited in the Bevy 0.14 release
notes, but the most obvious constructor doesn't actually support
unlimited layers.
We should explicitly document this.
## Solution
Add some docs mentioning the limit and pointing the user to `with` or
`from_layers` if they need an arbitrary number of layers.
# Objective
Fixes#14782
## Solution
Enable the lint and fix all upcoming hints (`--fix`). Also tried to
figure out the false-positive (see review comment). Maybe split this PR
up into multiple parts where only the last one enables the lint, so some
can already be merged resulting in less many files touched / less
potential for merge conflicts?
Currently, there are some cases where it might be easier to read the
code with the qualifier, so perhaps remove the import of it and adapt
its cases? In the current stage it's just a plain adoption of the
suggestions in order to have a base to discuss.
## Testing
`cargo clippy` and `cargo run -p ci` are happy.
# Objective
add a quick shorthand for creating a sprite with an custom size. This is
often desired when working with custom units or scaled sprites and
allows for a cleaner syntax in those cases/
## Solution
Implemented a `sized` function on the Sprite struct which creates a
Sprite, sets the custom size and leaves the rest at their default values
---
## Changelog
- Added `Sprite::sized(custom_size: Vec2)`
# Objective
currently if we use an image with the wrong sampler type in a material,
wgpu panics with an invalid texture format. turn this into a warning and
fail more gracefully.
## Solution
the expected sampler type is specified in the AsBindGroup derive, so we
can just check the image sampler is what it should be.
i am not totally sure about the mapping of image sampler type to
#[sampler(type)], i assumed:
```
"filtering" => [ TextureSampleType::Float { filterable: true } ],
"non_filtering" => [
TextureSampleType::Float { filterable: false },
TextureSampleType::Sint,
TextureSampleType::Uint,
],
"comparison" => [ TextureSampleType::Depth ],
```
# Objective
- Fixes#14684
## Solution
- Added documentation to `all_tuples_with_size` based on existing
`all_tuples` documentation.
- Updated `all_tuples` documentation to match formatting of and link
back to `all_tuples_with_size`
## Testing
- Doctests ran locally.
## Notes
Formatting changes I have proposed make the documentation a little
cleaner in my opinion, but I am open to reverting them and amending
`all_tuples_with_size` to match if there are any reasonable objections.
This reverts commit e37bf18e2b, added in
#14784.
# Objective
The PR was fine, but the work was very poorly motivated and the
resulting API is not actually very nice. The actual user need is likely
better addressed by #14825.
## Solution
Revert the offending PR.
Fix `mesh2d_manual` example from changes in #13069.
```
wgpu error: Validation Error
Caused by:
In RenderPass::end
In a set_pipeline command
Render pipeline targets are incompatible with render pass
Incompatible depth-stencil attachment format: the RenderPass uses a texture with format Some(Depth32Float) but the RenderPipeline with 'colored_mesh2d_pipeline' label uses an attachment with format None
```
# Objective
- Fixes#10478
## Solution
Generalised `From/Into` implementations over `&str` and `Option<&str>`
for `AssetSourceId` and `AssetPath` across all lifetimes, not just
static. To maintain access to the `'static`-only specialisation, these
types (and `CowArc`) now include an `as_static` method which will apply
the specialisation.
```rust
// Snipped from `AssetApp`
fn register_asset_source(
&mut self,
id: impl Into<AssetSourceId<'static>>,
// ^^^^^^^
// | as_static is only available for 'static lifetimes
source: AssetSourceBuilder,
) -> &mut Self {
let id = id.into().as_static();
// ^^^^^^ ^^^^^^^^^
// | | Specialized (internally storing CowArc::Static)
// | Generic Into (internally storing CowArc::Borrowed)
// ...
}
```
This post-fix specialisation is available here because the actual
specialisation performed is only a marker for if/when modification or
ownership is required, making the transform a very cheap operation. For
cleanliness, I've also added `from_static`, which wraps this behaviour
in a clean shorthand designed to replace `from` calls.
---
## Changelog
- Generalised the following implementations over a generic lifetime:
- `From<&'static str> for AssetSourceId<'static>`
- `From<Option<&'static str>> for AssetSourceId<'static>`
- `From<&'static str> for AssetPath<'static>`
- `From<&'static Path> for AssetPath<'static>`
- Added `as_static` specialisation to:
- `CowArc`
- `AssetSourceId`
- `AssetPath`
- Added `from_static` specialised constructor to:
- `AssetSourceId`
- `AssetPath`
## Migration Guide
In areas where these implementations where being used, you can now add
`from_static` in order to get the original specialised implementation
which avoids creating an `Arc` internally.
```rust
// Before
let asset_path = AssetPath::from("my/path/to/an/asset.ext");
// After
let asset_path = AssetPath::from_static("my/path/to/an/asset.ext");
```
To be clear, this is only required if you wish to maintain the
performance benefit that came with the specialisation. Existing code is
_not_ broken by this change.
# Objective
The `target` directory in `/tools/compile_fail_utils` was not being
ignored by Git.
## Solution
Added `/tools/compile_fail_utils/target` to the root `.gitignore` in the
same spirit as the other top-level, workspace-excluded crate: `benches`.
# Objective
One of the changes in #14704 made `DynamicFunction` effectively the same
as `DynamicClosure<'static>`. This change meant that the de facto
function type would likely be `DynamicClosure<'static>` instead of the
intended `DynamicFunction`, since the former is much more flexible.
We _could_ explore ways of making `DynamicFunction` implement `Copy`
using some unsafe code, but it likely wouldn't be worth it. And users
would likely still reach for the convenience of
`DynamicClosure<'static>` over the copy-ability of `DynamicFunction`.
The goal of this PR is to fix this confusion between the two types.
## Solution
Firstly, the `DynamicFunction` type was removed. Again, it was no
different than `DynamicClosure<'static>` so it wasn't a huge deal to
remove.
Secondly, `DynamicClosure<'env>` and `DynamicClosureMut<'env>` were
renamed to `DynamicFunction<'env>` and `DynamicFunctionMut<'env>`,
respectively.
Yes, we still ultimately kept the naming of `DynamicFunction`, but
changed its behavior to that of `DynamicClosure<'env>`. We need a term
to refer to both functions and closures, and "function" was the best
option.
[Originally](https://discord.com/channels/691052431525675048/1002362493634629796/1274091992162242710),
I was going to go with "callable" as the replacement term to encompass
both functions and closures (e.g. `DynamciCallable<'env>`). However, it
was
[suggested](https://discord.com/channels/691052431525675048/1002362493634629796/1274653581777047625)
by @SkiFire13 that the simpler "function" term could be used instead.
While "callable" is perhaps the better umbrella term—being truly
ambiguous over functions and closures— "function" is more familiar, used
more often, easier to discover, and is subjectively just
"better-sounding".
## Testing
Most changes are purely swapping type names or updating documentation,
but you can verify everything still works by running the following
command:
```
cargo test --package bevy_reflect
```
# Objective
currently if an asset loader panics, the asset is left in a perpetual
`Loading` state. this can occur with external crates (eg the image crate
panics on bad data). catch this panic and resolve the asset to `Failed`
## Solution
`AssertUnwindSafe(loader.load).catch_unwind()` and map the panic to an
`AssetLoadError`
separated out from #13170
# Objective
- Fix error when closing window in 2d_viewport_to_world example.
Before
```
2024-08-17T22:51:47.690252Z INFO bevy_winit::system: Creating new window "App" (0v1#4294967296)
2024-08-17T22:52:22.062959Z INFO bevy_window::system: No windows are open, exiting
2024-08-17T22:52:22.064045Z INFO bevy_winit::system: Closing window 0v1#4294967296
thread 'Compute Task Pool (5)' panicked at examples/2d/2d_viewport_to_world.rs:20:41:
called `Result::unwrap()` on an `Err` value: NoEntities("bevy_ecs::query::state::QueryState<&bevy_window:🪟:Window>")
```
After
```
2024-08-17T22:57:31.623499Z INFO bevy_winit::system: Creating new window "App" (0v1#4294967296)
2024-08-17T22:57:32.990058Z INFO bevy_window::system: No windows are open, exiting
2024-08-17T22:57:32.991152Z INFO bevy_winit::system: Closing window 0v1#4294967296
2024-08-17T22:57:32.994426Z INFO bevy_window::system: No windows are open, exiting
* Terminal will be reused by tasks, press any key to close it.
```
## Solution
- Check if the window still exists before drawing the cursor
