# Objective
- Progress towards #19887.
- I am avoiding dealing with the `occlusion_culling` example since it is
kinda annoying to resolve nicely (I will do so in another PR).
## Solution
- Rewrite these examples to replace FromWorld implementations with
systems and other resource changes with systems as well.
## Testing
- All the changed examples have been tested and still work.
# Objective
> I think we should axe the shared `Event` trait entirely
It doesn't serve any functional purpose, and I don't think it's useful
pedagogically
@alice-i-cecile on discord
## Solution
- Remove `Event` as a supertrait of `BufferedEvent`
- Remove any `Event` derives that were made unnecessary
- Update release notes
---------
Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
[Explanation](https://bevyengine.org/learn/contribute/helping-out/explaining-examples/)
for the 3d shapes example.
This shares a lot of detail with the [2d
shapes](https://github.com/bevyengine/bevy/pull/19211) example, so it's
similar in structure. The explanation for why asset handles are not
components has been copied over for now with minor adjustment, I'll do
another editing pass on this to make it match the surrounding context
and focus before taking it out of drafts.
---------
Co-authored-by: theotherphil <phil.j.ellison@gmail.com>
Co-authored-by: Carter Weinberg <weinbergcarter@gmail.com>
# Objective
- make lights usable without bevy_render
## Solution
- make a new crate for lights to live in
## Testing
- 3d_scene, lighting, volumetric_fog, ssr, transmission, pcss,
light_textures
Note: no breaking changes because of re-exports, except for light
textures, which were introduced this cycle so it doesn't matter anyways
# Objective
- This unblocks some work I am doing for #19887.
## Solution
- Rename `RenderGraphApp` to `RenderGraphExt`.
- Implement `RenderGraphExt` for `World`.
- Change `SubApp` and `App` to call the `World` impl.
# Objective
- Example `light_textures` exit if feature `pbr_light_textures` is not
enabled. this is checked in code instead of using `required-features`
- Same for `clustered_decals` and `par_clustered_decals`
- Those examples are also using `eprintln`
- Those examples are using `process:exit` to exit
## Solution
- Use `required-features`
- Use logs
- Use `AppExit`
# Objective
I was lurking and noticed that some links to the Bevy website were not
updated in newer code (`bevyengine.org` -> `bevy.org`).
## Solution
- Look for `bevyengine.org` occurrences in the current code, replace
them with `bevy.org`.
## Testing
- Did you test these changes? If so, how? I visited the Bevy website!
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
## Longer term
- Maybe add a lint to flag references to the old website but I don't
know how to do that. But not sure it's needed as the more time will pass
the less it will be relevant.
# Objective
add support for light textures (also known as light cookies, light
functions, and light projectors)
## Solution
- add components:
```rs
/// Add to a [`PointLight`] to add a light texture effect.
/// A texture mask is applied to the light source to modulate its intensity,
/// simulating patterns like window shadows, gobo/cookie effects, or soft falloffs.
pub struct PointLightTexture {
/// The texture image. Only the R channel is read.
pub image: Handle<Image>,
/// The cubemap layout. The image should be a packed cubemap in one of the formats described by the [`CubemapLayout`] enum.
pub cubemap_layout: CubemapLayout,
}
/// Add to a [`SpotLight`] to add a light texture effect.
/// A texture mask is applied to the light source to modulate its intensity,
/// simulating patterns like window shadows, gobo/cookie effects, or soft falloffs.
pub struct SpotLightTexture {
/// The texture image. Only the R channel is read.
/// Note the border of the image should be entirely black to avoid leaking light.
pub image: Handle<Image>,
}
/// Add to a [`DirectionalLight`] to add a light texture effect.
/// A texture mask is applied to the light source to modulate its intensity,
/// simulating patterns like window shadows, gobo/cookie effects, or soft falloffs.
pub struct DirectionalLightTexture {
/// The texture image. Only the R channel is read.
pub image: Handle<Image>,
/// Whether to tile the image infinitely, or use only a single tile centered at the light's translation
pub tiled: bool,
}
```
- store images to the `RenderClusteredDecals` buffer
- read the image and modulate the lights
- add `light_textures` example to showcase the new features
## Testing
see light_textures example
# Objective
Closes#18075
In order to enable a number of patterns for dynamic materials in the
engine, it's necessary to decouple the renderer from the `Material`
trait.
This opens the possibility for:
- Materials that aren't coupled to `AsBindGroup`.
- 2d using the underlying 3d bindless infrastructure.
- Dynamic materials that can change their layout at runtime.
- Materials that aren't even backed by a Rust struct at all.
## Solution
In short, remove all trait bounds from render world material systems and
resources. This means moving a bunch of stuff onto `MaterialProperties`
and engaging in some hacks to make specialization work. Rather than
storing the bind group data in `MaterialBindGroupAllocator`, right now
we're storing it in a closure on `MaterialProperties`. TBD if this has
bad performance characteristics.
## Benchmarks
- `many_cubes`:
`cargo run --example many_cubes --release --features=bevy/trace_tracy --
--vary-material-data-per-instance`:
- @DGriffin91's Caldera
`cargo run --release --features=bevy/trace_tracy -- --random-materials`
- @DGriffin91's Caldera with 20 unique material types (i.e.
`MaterialPlugin<M>`) and random materials per mesh
`cargo run --release --features=bevy/trace_tracy -- --random-materials`
### TODO
- We almost certainly lost some parallelization from removing the type
params that could be gained back from smarter iteration.
- Test all the things that could have broken.
- ~Fix meshlets~
## Showcase
See [the
example](https://github.com/bevyengine/bevy/pull/19667/files#diff-9d768cfe1c3aa81eff365d250d3cbe5a63e8df63e81dd85f64c3c3cd993f6d94)
for a custom material implemented without the use of the `Material`
trait and thus `AsBindGroup`.
---------
Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
Co-authored-by: IceSentry <c.giguere42@gmail.com>
This is a bit of a test case in writing the
[explanation](https://bevyengine.org/learn/contribute/helping-out/explaining-examples/)
for an example whose subject (`DistanceFog` as a component on cameras)
is the focus, but isn't that complicated either. Not certain if this
could be an exception or something more common.
Putting the controls below the explanation, as they're more of a
fall-back for the on-screen info (also that's where they were before).
# Objective
- Start the realtime direct lighting work for bevy solari
## Solution
- Setup all the CPU-side code for the realtime lighting path (minus some
parts for the temporal reuse I haven't written yet)
- Implement RIS with 32 samples to choose a good random light
- Don't sample a disk for the directional light, just treat it as a
single point. This is faster and not much worse quality.
## Future
- Spatiotemporal reuse (ReSTIR DI)
- Denoiser (DLSS-RR)
- Light tile optimization for faster light selection
- Indirect lighting (ReSTIR GI)
## Testing
- Run the solari example to see realtime
- Run the solari example with `-- --pathtracer` to see the existing
pathtracer
---
## Showcase
1 frame direct lighting:
Accumulated pathtracer output:
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fix https://github.com/bevyengine/bevy/issues/19617
# Solution
Add newlines before all impl blocks.
I suspect that at least some of these will be objectionable! If there's
a desired Bevy style for this then I'll update the PR. If not then we
can just close it - it's the work of a single find and replace.
# Objective
As someone who is currently learning Bevy, I found the implementation of
the ambient light in the 3d/lighting.rs example unsatisfactory.
## Solution
- I adjusted the brightness of the ambient light in the scene to 200
(where the default is 80). It was previously 0.02, a value so low it has
no noticeable effect.
- I added a keybind (space bar) to toggle the ambient light, allowing
users to see the difference it makes. I also added text showing the
state of the ambient light (on, off) and text showing the keybind.
I'm very new to Bevy and Rust, so apologies if any of this code is not
up to scratch.
## Testing
I checked all the text still updates correctly and all keybinds still
work. In my testing, it looks to work okay.
I'd appreciate others testing too, just to make sure.
---
## Showcase
<details>
<summary>Click to view showcase</summary>
<img width="960" alt="Screenshot (11)"
src="https://github.com/user-attachments/assets/916e569e-cd49-43fd-b81d-aae600890cd3"
/>
<img width="959" alt="Screenshot (12)"
src="https://github.com/user-attachments/assets/0e16bb3a-c38a-4a8d-8248-edf3b820d238"
/>
</details>
# Objective
Closes#19564.
The current `Event` trait looks like this:
```rust
pub trait Event: Send + Sync + 'static {
type Traversal: Traversal<Self>;
const AUTO_PROPAGATE: bool = false;
fn register_component_id(world: &mut World) -> ComponentId { ... }
fn component_id(world: &World) -> Option<ComponentId> { ... }
}
```
The `Event` trait is used by both buffered events
(`EventReader`/`EventWriter`) and observer events. If they are observer
events, they can optionally be targeted at specific `Entity`s or
`ComponentId`s, and can even be propagated to other entities.
However, there has long been a desire to split the trait semantically
for a variety of reasons, see #14843, #14272, and #16031 for discussion.
Some reasons include:
- It's very uncommon to use a single event type as both a buffered event
and targeted observer event. They are used differently and tend to have
distinct semantics.
- A common footgun is using buffered events with observers or event
readers with observer events, as there is no type-level error that
prevents this kind of misuse.
- #19440 made `Trigger::target` return an `Option<Entity>`. This
*seriously* hurts ergonomics for the general case of entity observers,
as you need to `.unwrap()` each time. If we could statically determine
whether the event is expected to have an entity target, this would be
unnecessary.
There's really two main ways that we can categorize events: push vs.
pull (i.e. "observer event" vs. "buffered event") and global vs.
targeted:
| | Push | Pull |
| ------------ | --------------- | --------------------------- |
| **Global** | Global observer | `EventReader`/`EventWriter` |
| **Targeted** | Entity observer | - |
There are many ways to approach this, each with their tradeoffs.
Ultimately, we kind of want to split events both ways:
- A type-level distinction between observer events and buffered events,
to prevent people from using the wrong kind of event in APIs
- A statically designated entity target for observer events to avoid
accidentally using untargeted events for targeted APIs
This PR achieves these goals by splitting event traits into `Event`,
`EntityEvent`, and `BufferedEvent`, with `Event` being the shared trait
implemented by all events.
## `Event`, `EntityEvent`, and `BufferedEvent`
`Event` is now a very simple trait shared by all events.
```rust
pub trait Event: Send + Sync + 'static {
// Required for observer APIs
fn register_component_id(world: &mut World) -> ComponentId { ... }
fn component_id(world: &World) -> Option<ComponentId> { ... }
}
```
You can call `trigger` for *any* event, and use a global observer for
listening to the event.
```rust
#[derive(Event)]
struct Speak {
message: String,
}
// ...
app.add_observer(|trigger: On<Speak>| {
println!("{}", trigger.message);
});
// ...
commands.trigger(Speak {
message: "Y'all like these reworked events?".to_string(),
});
```
To allow an event to be targeted at entities and even propagated
further, you can additionally implement the `EntityEvent` trait:
```rust
pub trait EntityEvent: Event {
type Traversal: Traversal<Self>;
const AUTO_PROPAGATE: bool = false;
}
```
This lets you call `trigger_targets`, and to use targeted observer APIs
like `EntityCommands::observe`:
```rust
#[derive(Event, EntityEvent)]
#[entity_event(traversal = &'static ChildOf, auto_propagate)]
struct Damage {
amount: f32,
}
// ...
let enemy = commands.spawn((Enemy, Health(100.0))).id();
// Spawn some armor as a child of the enemy entity.
// When the armor takes damage, it will bubble the event up to the enemy.
let armor_piece = commands
.spawn((ArmorPiece, Health(25.0), ChildOf(enemy)))
.observe(|trigger: On<Damage>, mut query: Query<&mut Health>| {
// Note: `On::target` only exists because this is an `EntityEvent`.
let mut health = query.get(trigger.target()).unwrap();
health.0 -= trigger.amount();
});
commands.trigger_targets(Damage { amount: 10.0 }, armor_piece);
```
> [!NOTE]
> You *can* still also trigger an `EntityEvent` without targets using
`trigger`. We probably *could* make this an either-or thing, but I'm not
sure that's actually desirable.
To allow an event to be used with the buffered API, you can implement
`BufferedEvent`:
```rust
pub trait BufferedEvent: Event {}
```
The event can then be used with `EventReader`/`EventWriter`:
```rust
#[derive(Event, BufferedEvent)]
struct Message(String);
fn write_hello(mut writer: EventWriter<Message>) {
writer.write(Message("I hope these examples are alright".to_string()));
}
fn read_messages(mut reader: EventReader<Message>) {
// Process all buffered events of type `Message`.
for Message(message) in reader.read() {
println!("{message}");
}
}
```
In summary:
- Need a basic event you can trigger and observe? Derive `Event`!
- Need the event to be targeted at an entity? Derive `EntityEvent`!
- Need the event to be buffered and support the
`EventReader`/`EventWriter` API? Derive `BufferedEvent`!
## Alternatives
I'll now cover some of the alternative approaches I have considered and
briefly explored. I made this section collapsible since it ended up
being quite long :P
<details>
<summary>Expand this to see alternatives</summary>
### 1. Unified `Event` Trait
One option is not to have *three* separate traits (`Event`,
`EntityEvent`, `BufferedEvent`), and to instead just use associated
constants on `Event` to determine whether an event supports targeting
and buffering or not:
```rust
pub trait Event: Send + Sync + 'static {
type Traversal: Traversal<Self>;
const AUTO_PROPAGATE: bool = false;
const TARGETED: bool = false;
const BUFFERED: bool = false;
fn register_component_id(world: &mut World) -> ComponentId { ... }
fn component_id(world: &World) -> Option<ComponentId> { ... }
}
```
Methods can then use bounds like `where E: Event<TARGETED = true>` or
`where E: Event<BUFFERED = true>` to limit APIs to specific kinds of
events.
This would keep everything under one `Event` trait, but I don't think
it's necessarily a good idea. It makes APIs harder to read, and docs
can't easily refer to specific types of events. You can also create
weird invariants: what if you specify `TARGETED = false`, but have
`Traversal` and/or `AUTO_PROPAGATE` enabled?
### 2. `Event` and `Trigger`
Another option is to only split the traits between buffered events and
observer events, since that is the main thing people have been asking
for, and they have the largest API difference.
If we did this, I think we would need to make the terms *clearly*
separate. We can't really use `Event` and `BufferedEvent` as the names,
since it would be strange that `BufferedEvent` doesn't implement
`Event`. Something like `ObserverEvent` and `BufferedEvent` could work,
but it'd be more verbose.
For this approach, I would instead keep `Event` for the current
`EventReader`/`EventWriter` API, and call the observer event a
`Trigger`, since the "trigger" terminology is already used in the
observer context within Bevy (both as a noun and a verb). This is also
what a long [bikeshed on
Discord](https://discord.com/channels/691052431525675048/749335865876021248/1298057661878898791)
seemed to land on at the end of last year.
```rust
// For `EventReader`/`EventWriter`
pub trait Event: Send + Sync + 'static {}
// For observers
pub trait Trigger: Send + Sync + 'static {
type Traversal: Traversal<Self>;
const AUTO_PROPAGATE: bool = false;
const TARGETED: bool = false;
fn register_component_id(world: &mut World) -> ComponentId { ... }
fn component_id(world: &World) -> Option<ComponentId> { ... }
}
```
The problem is that "event" is just a really good term for something
that "happens". Observers are rapidly becoming the more prominent API,
so it'd be weird to give them the `Trigger` name and leave the good
`Event` name for the less common API.
So, even though a split like this seems neat on the surface, I think it
ultimately wouldn't really work. We want to keep the `Event` name for
observer events, and there is no good alternative for the buffered
variant. (`Message` was suggested, but saying stuff like "sends a
collision message" is weird.)
### 3. `GlobalEvent` + `TargetedEvent`
What if instead of focusing on the buffered vs. observed split, we
*only* make a distinction between global and targeted events?
```rust
// A shared event trait to allow global observers to work
pub trait Event: Send + Sync + 'static {
fn register_component_id(world: &mut World) -> ComponentId { ... }
fn component_id(world: &World) -> Option<ComponentId> { ... }
}
// For buffered events and non-targeted observer events
pub trait GlobalEvent: Event {}
// For targeted observer events
pub trait TargetedEvent: Event {
type Traversal: Traversal<Self>;
const AUTO_PROPAGATE: bool = false;
}
```
This is actually the first approach I implemented, and it has the neat
characteristic that you can only use non-targeted APIs like `trigger`
with a `GlobalEvent` and targeted APIs like `trigger_targets` with a
`TargetedEvent`. You have full control over whether the entity should or
should not have a target, as they are fully distinct at the type-level.
However, there's a few problems:
- There is no type-level indication of whether a `GlobalEvent` supports
buffered events or just non-targeted observer events
- An `Event` on its own does literally nothing, it's just a shared trait
required to make global observers accept both non-targeted and targeted
events
- If an event is both a `GlobalEvent` and `TargetedEvent`, global
observers again have ambiguity on whether an event has a target or not,
undermining some of the benefits
- The names are not ideal
### 4. `Event` and `EntityEvent`
We can fix some of the problems of Alternative 3 by accepting that
targeted events can also be used in non-targeted contexts, and simply
having the `Event` and `EntityEvent` traits:
```rust
// For buffered events and non-targeted observer events
pub trait Event: Send + Sync + 'static {
fn register_component_id(world: &mut World) -> ComponentId { ... }
fn component_id(world: &World) -> Option<ComponentId> { ... }
}
// For targeted observer events
pub trait EntityEvent: Event {
type Traversal: Traversal<Self>;
const AUTO_PROPAGATE: bool = false;
}
```
This is essentially identical to this PR, just without a dedicated
`BufferedEvent`. The remaining major "problem" is that there is still
zero type-level indication of whether an `Event` event *actually*
supports the buffered API. This leads us to the solution proposed in
this PR, using `Event`, `EntityEvent`, and `BufferedEvent`.
</details>
## Conclusion
The `Event` + `EntityEvent` + `BufferedEvent` split proposed in this PR
aims to solve all the common problems with Bevy's current event model
while keeping the "weirdness" factor minimal. It splits in terms of both
the push vs. pull *and* global vs. targeted aspects, while maintaining a
shared concept for an "event".
### Why I Like This
- The term "event" remains as a single concept for all the different
kinds of events in Bevy.
- Despite all event types being "events", they use fundamentally
different APIs. Instead of assuming that you can use an event type with
any pattern (when only one is typically supported), you explicitly opt
in to each one with dedicated traits.
- Using separate traits for each type of event helps with documentation
and clearer function signatures.
- I can safely make assumptions on expected usage.
- If I see that an event is an `EntityEvent`, I can assume that I can
use `observe` on it and get targeted events.
- If I see that an event is a `BufferedEvent`, I can assume that I can
use `EventReader` to read events.
- If I see both `EntityEvent` and `BufferedEvent`, I can assume that
both APIs are supported.
In summary: This allows for a unified concept for events, while limiting
the different ways to use them with opt-in traits. No more guess-work
involved when using APIs.
### Problems?
- Because `BufferedEvent` implements `Event` (for more consistent
semantics etc.), you can still use all buffered events for non-targeted
observers. I think this is fine/good. The important part is that if you
see that an event implements `BufferedEvent`, you know that the
`EventReader`/`EventWriter` API should be supported. Whether it *also*
supports other APIs is secondary.
- I currently only support `trigger_targets` for an `EntityEvent`.
However, you can technically target components too, without targeting
any entities. I consider that such a niche and advanced use case that
it's not a huge problem to only support it for `EntityEvent`s, but we
could also split `trigger_targets` into `trigger_entities` and
`trigger_components` if we wanted to (or implement components as
entities :P).
- You can still trigger an `EntityEvent` *without* targets. I consider
this correct, since `Event` implements the non-targeted behavior, and
it'd be weird if implementing another trait *removed* behavior. However,
it does mean that global observers for entity events can technically
return `Entity::PLACEHOLDER` again (since I got rid of the
`Option<Entity>` added in #19440 for ergonomics). I think that's enough
of an edge case that it's not a huge problem, but it is worth keeping in
mind.
- ~~Deriving both `EntityEvent` and `BufferedEvent` for the same type
currently duplicates the `Event` implementation, so you instead need to
manually implement one of them.~~ Changed to always requiring `Event` to
be derived.
## Related Work
There are plans to implement multi-event support for observers,
especially for UI contexts. [Cart's
example](https://github.com/bevyengine/bevy/issues/14649#issuecomment-2960402508)
API looked like this:
```rust
// Truncated for brevity
trigger: Trigger<(
OnAdd<Pressed>,
OnRemove<Pressed>,
OnAdd<InteractionDisabled>,
OnRemove<InteractionDisabled>,
OnInsert<Hovered>,
)>,
```
I believe this shouldn't be in conflict with this PR. If anything, this
PR might *help* achieve the multi-event pattern for entity observers
with fewer footguns: by statically enforcing that all of these events
are `EntityEvent`s in the context of `EntityCommands::observe`, we can
avoid misuse or weird cases where *some* events inside the trigger are
targeted while others are not.
## Objective
Make it easier to use `IrradianceVolume` with fewer ways to silently
fail. Fix#19614.
## Solution
* Add `#[require(LightProbe)]` to `struct IrradianceVolume`.
* Document this fact.
* Also document the volume being centered on the origin by default (this
was the other thing that was unclear when getting started).
I also looked at the other implementor of `LightProbeComponent`,
`EnvironmentMapLight`, but it has a use which is *not* as a light probe,
so it should not require `LightProbe`.
## Testing
* Confirmed that `examples/3d/irradiance_volumes.rs` still works after
removing `LightProbe`.
* Reviewed generated documentation.
# Bevy Solari
<img
src="https://github.com/user-attachments/assets/94061fc8-01cf-4208-b72a-8eecad610d76"
width="100" />
## Preface
- See release notes.
- Please talk to me in #rendering-dev on discord or open a github
discussion if you have questions about the long term plan, and keep
discussion in this PR limited to the contents of the PR :)
## Connections
- Works towards #639, #16408.
- Spawned https://github.com/bevyengine/bevy/issues/18993.
- Need to fix RT stuff in naga_oil first
https://github.com/bevyengine/naga_oil/pull/116.
## This PR
After nearly two years, I've revived the raytraced lighting effort I
first started in https://github.com/bevyengine/bevy/pull/10000.
Unlike that PR, which has realtime techniques, I've limited this PR to:
* `RaytracingScenePlugin` - BLAS and TLAS building, geometry and texture
binding, sampling functions.
* `PathtracingPlugin` - A non-realtime path tracer intended to serve as
a testbed and reference.
## What's implemented?
* BLAS building on mesh load
* Emissive lights
* Directional lights with soft shadows
* Diffuse (lambert, not Bevy's diffuse BRDF) and emissive materials
* A reference path tracer with:
* Antialiasing
* Direct light sampling (next event estimation) with 0/1 MIS weights
* Importance-sampled BRDF bounces
* Russian roulette
## What's _not_ implemented?
* Anything realtime, including a real-time denoiser
* Integration with Bevy's rasterized gbuffer
* Specular materials
* Non-opaque geometry
* Any sort of CPU or GPU optimizations
* BLAS compaction, proper bindless, and further RT APIs are things that
we need wgpu to add
* PointLights, SpotLights, or skyboxes / environment lighting
* Support for materials other than StandardMaterial (and only a subset
of properties are supported)
* Skinned/morphed or otherwise animating/deformed meshes
* Mipmaps
* Adaptive self-intersection ray bias
* A good way for developers to detect whether the user's GPU supports RT
or not, and fallback to baked lighting.
* Documentation and actual finalized APIs (literally everything is
subject to change)
## End-user Usage
* Have a GPU that supports RT with inline ray queries
* Add `SolariPlugin` to your app
* Ensure any `Mesh` asset you want to use for raytracing has
`enable_raytracing: true` (defaults to true), and that it uses the
standard uncompressed position/normal/uv_0/tangent vertex attribute set,
triangle list topology, and 32-bit indices.
* If you don't want to build a BLAS and use the mesh for RT, set
enable_raytracing to false.
* Add the `RaytracingMesh3d` component to your entity (separate from
`Mesh3d` or `MeshletMesh3d`).
## Testing
- Did you test these changes? If so, how?
- Ran the solari example.
- Are there any parts that need more testing?
- Other test scenes probably. Normal mapping would be good to test.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- See the solari.rs example for how to setup raytracing.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
- Windows 11, NVIDIA RTX 3080.
---------
Co-authored-by: atlv <email@atlasdostal.com>
Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Currently, the observer API looks like this:
```rust
app.add_observer(|trigger: Trigger<Explode>| {
info!("Entity {} exploded!", trigger.target());
});
```
Future plans for observers also include "multi-event observers" with a
trigger that looks like this (see [Cart's
example](https://github.com/bevyengine/bevy/issues/14649#issuecomment-2960402508)):
```rust
trigger: Trigger<(
OnAdd<Pressed>,
OnRemove<Pressed>,
OnAdd<InteractionDisabled>,
OnRemove<InteractionDisabled>,
OnInsert<Hovered>,
)>,
```
In scenarios like this, there is a lot of repetition of `On`. These are
expected to be very high-traffic APIs especially in UI contexts, so
ergonomics and readability are critical.
By renaming `Trigger` to `On`, we can make these APIs read more cleanly
and get rid of the repetition:
```rust
app.add_observer(|trigger: On<Explode>| {
info!("Entity {} exploded!", trigger.target());
});
```
```rust
trigger: On<(
Add<Pressed>,
Remove<Pressed>,
Add<InteractionDisabled>,
Remove<InteractionDisabled>,
Insert<Hovered>,
)>,
```
Names like `On<Add<Pressed>>` emphasize the actual event listener nature
more than `Trigger<OnAdd<Pressed>>`, and look cleaner. This *also* frees
up the `Trigger` name if we want to use it for the observer event type,
splitting them out from buffered events (bikeshedding this is out of
scope for this PR though).
For prior art:
[`bevy_eventlistener`](https://github.com/aevyrie/bevy_eventlistener)
used
[`On`](https://docs.rs/bevy_eventlistener/latest/bevy_eventlistener/event_listener/struct.On.html)
for its event listener type. Though in our case, the observer is the
event listener, and `On` is just a type containing information about the
triggered event.
## Solution
Steal from `bevy_event_listener` by @aevyrie and use `On`.
- Rename `Trigger` to `On`
- Rename `OnAdd` to `Add`
- Rename `OnInsert` to `Insert`
- Rename `OnReplace` to `Replace`
- Rename `OnRemove` to `Remove`
- Rename `OnDespawn` to `Despawn`
## Discussion
### Naming Conflicts??
Using a name like `Add` might initially feel like a very bad idea, since
it risks conflict with `core::ops::Add`. However, I don't expect this to
be a big problem in practice.
- You rarely need to actually implement the `Add` trait, especially in
modules that would use the Bevy ECS.
- In the rare cases where you *do* get a conflict, it is very easy to
fix by just disambiguating, for example using `ops::Add`.
- The `Add` event is a struct while the `Add` trait is a trait (duh), so
the compiler error should be very obvious.
For the record, renaming `OnAdd` to `Add`, I got exactly *zero* errors
or conflicts within Bevy itself. But this is of course not entirely
representative of actual projects *using* Bevy.
You might then wonder, why not use `Added`? This would conflict with the
`Added` query filter, so it wouldn't work. Additionally, the current
naming convention for observer events does not use past tense.
### Documentation
This does make documentation slightly more awkward when referring to
`On` or its methods. Previous docs often referred to `Trigger::target`
or "sends a `Trigger`" (which is... a bit strange anyway), which would
now be `On::target` and "sends an observer `Event`".
You can see the diff in this PR to see some of the effects. I think it
should be fine though, we may just need to reword more documentation to
read better.
# Objective
Rename `JustifyText`:
* The name `JustifyText` is just ugly.
* It's inconsistent since no other `bevy_text` types have a `Text-`
suffix, only prefix.
* It's inconsistent with the other text layout enum `Linebreak` which
doesn't have a prefix or suffix.
Fixes#19521.
## Solution
Rename `JustifyText` to `Justify`.
Without other context, it's natural to assume the name `Justify` refers
to text justification.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
`Entity::PLACEHOLDER` acts as a magic number that will *probably* never
really exist, but it certainly could. And, `Entity` has a niche, so the
only reason to use `PLACEHOLDER` is as an alternative to `MaybeUninit`
that trades safety risks for logic risks.
As a result, bevy has generally advised against using `PLACEHOLDER`, but
we still use if for a lot internally. This pr starts removing internal
uses of it, starting from observers.
## Solution
Change all trigger target related types from `Entity` to
`Option<Entity>`
Small migration guide to come.
## Testing
CI
## Future Work
This turned a lot of code from
```rust
trigger.target()
```
to
```rust
trigger.target().unwrap()
```
The extra panic is no worse than before; it's just earlier than
panicking after passing the placeholder to something else.
But this is kinda annoying.
I would like to add a `TriggerMode` or something to `Event` that would
restrict what kinds of targets can be used for that event. Many events
like `Removed` etc, are always triggered with a target. We can make
those have a way to assume Some, etc. But I wanted to save that for a
future pr.
The first 4 commits are designed to be reviewed independently.
- Mark TAA non-experimental now that motion vectors are written for
skinned and morphed meshes, along with skyboxes, and add it to
DefaultPlugins
- Adjust halton sequence to match what DLSS is going to use, doesn't
really affect anything, but may as well
- Make MipBias a required component on TAA instead of inserting it in
the render world
- Remove MipBias, TemporalJitter, RenderLayers, etc from the render
world if they're removed from the main world (fixes a retained render
world bug)
- Remove TAA components from the render world properly if
TemporalAntiAliasing is removed from the main world (fixes a retained
render world bug)
- extract_taa_settings() now has to query over `Option<&mut
TemporalAntiAliasing>`, which will match every single camera, in order
to cover cameras that had TemporalAntiAliasing removed this frame. This
kind of sucks, but I can't think of anything better.
- We probably have the same bug with every other rendering feature
component we have.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- Simplify `Camera` initialization
- allow effects to require HDR
## Solution
- Split out `Camera.hdr` into a marker `Hdr` component
## Testing
- ran `bloom_3d` example
---
## Showcase
```rs
// before
commands.spawn((
Camera3d
Camera {
hdr: true
..Default::default()
}
))
// after
commands.spawn((Camera3d, Hdr));
// other rendering components can require that the camera enables hdr!
// currently implemented for Bloom, AutoExposure, and Atmosphere.
#[require(Hdr)]
pub struct Bloom;
```
# Objective
When user presses <kbd>3</kbd>, the falloff mode should be changed to
`ExponentialSquared` as described in the instructions, but it's not in
fact.
Online Example: https://bevyengine.org/examples-webgpu/3d-rendering/fog/
## Solution
Change it to `ExponentialSquared`
## Testing
- Did you test these changes? If so, how?
Yes, by `cargo run --example fog`
- Are there any parts that need more testing?
No.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
```
cargo run --example fog
```
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
N/A
# Objective
allow specifying the left/top/right/bottom border colors separately for
ui elements
fixes#14773
## Solution
- change `BorderColor` to
```rs
pub struct BorderColor {
pub left: Color,
pub top: Color,
pub right: Color,
pub bottom: Color,
}
```
- generate one ui node per distinct border color, set flags for the
active borders
- render only the active borders
i chose to do this rather than adding multiple colors to the
ExtractedUiNode in order to minimize the impact for the common case
where all border colors are the same.
## Testing
modified the `borders` example to use separate colors:
the behaviour is a bit weird but it mirrors html/css border behaviour.
---
## Migration:
To keep the existing behaviour, just change `BorderColor(color)` into
`BorderColor::all(color)`.
---------
Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
Stores mesh names from glTF files in GltfMeshName component rather than
Name component, making both GltfMeshName and GltfMaterialName behave
like strings via Deref.
# Objective
Fixed the side effects of #19287
Fixes Examples that modify gltf materials are broken #19322
## Solution
Add GltfMeshName component and Deref implementations
Stores mesh names from glTF files in GltfMeshName component rather than
Name component, making both GltfMeshName and GltfMaterialName behave
like strings via Deref.
## Testing
cargo run --example depth_of_field
cargo run --example lightmaps
cargo run --example mixed_lighting
They are consistent with the situation before the error occurred.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
# Objective
Fixes a part of #14274.
Bevy has an incredibly inconsistent naming convention for its system
sets, both internally and across the ecosystem.
<img alt="System sets in Bevy"
src="https://github.com/user-attachments/assets/d16e2027-793f-4ba4-9cc9-e780b14a5a1b"
width="450" />
*Names of public system set types in Bevy*
Most Bevy types use a naming of `FooSystem` or just `Foo`, but there are
also a few `FooSystems` and `FooSet` types. In ecosystem crates on the
other hand, `FooSet` is perhaps the most commonly used name in general.
Conventions being so wildly inconsistent can make it harder for users to
pick names for their own types, to search for system sets on docs.rs, or
to even discern which types *are* system sets.
To reign in the inconsistency a bit and help unify the ecosystem, it
would be good to establish a common recommended naming convention for
system sets in Bevy itself, similar to how plugins are commonly suffixed
with `Plugin` (ex: `TimePlugin`). By adopting a consistent naming
convention in first-party Bevy, we can softly nudge ecosystem crates to
follow suit (for types where it makes sense to do so).
Choosing a naming convention is also relevant now, as the [`bevy_cli`
recently adopted
lints](https://github.com/TheBevyFlock/bevy_cli/pull/345) to enforce
naming for plugins and system sets, and the recommended naming used for
system sets is still a bit open.
## Which Name To Use?
Now the contentious part: what naming convention should we actually
adopt?
This was discussed on the Bevy Discord at the end of last year, starting
[here](<https://discord.com/channels/691052431525675048/692572690833473578/1310659954683936789>).
`FooSet` and `FooSystems` were the clear favorites, with `FooSet` very
narrowly winning an unofficial poll. However, it seems to me like the
consensus was broadly moving towards `FooSystems` at the end and after
the poll, with Cart
([source](https://discord.com/channels/691052431525675048/692572690833473578/1311140204974706708))
and later Alice
([source](https://discord.com/channels/691052431525675048/692572690833473578/1311092530732859533))
and also me being in favor of it.
Let's do a quick pros and cons list! Of course these are just what I
thought of, so take it with a grain of salt.
`FooSet`:
- Pro: Nice and short!
- Pro: Used by many ecosystem crates.
- Pro: The `Set` suffix comes directly from the trait name `SystemSet`.
- Pro: Pairs nicely with existing APIs like `in_set` and
`configure_sets`.
- Con: `Set` by itself doesn't actually indicate that it's related to
systems *at all*, apart from the implemented trait. A set of what?
- Con: Is `FooSet` a set of `Foo`s or a system set related to `Foo`? Ex:
`ContactSet`, `MeshSet`, `EnemySet`...
`FooSystems`:
- Pro: Very clearly indicates that the type represents a collection of
systems. The actual core concept, system(s), is in the name.
- Pro: Parallels nicely with `FooPlugins` for plugin groups.
- Pro: Low risk of conflicts with other names or misunderstandings about
what the type is.
- Pro: In most cases, reads *very* nicely and clearly. Ex:
`PhysicsSystems` and `AnimationSystems` as opposed to `PhysicsSet` and
`AnimationSet`.
- Pro: Easy to search for on docs.rs.
- Con: Usually results in longer names.
- Con: Not yet as widely used.
Really the big problem with `FooSet` is that it doesn't actually
describe what it is. It describes what *kind of thing* it is (a set of
something), but not *what it is a set of*, unless you know the type or
check its docs or implemented traits. `FooSystems` on the other hand is
much more self-descriptive in this regard, at the cost of being a bit
longer to type.
Ultimately, in some ways it comes down to preference and how you think
of system sets. Personally, I was originally in favor of `FooSet`, but
have been increasingly on the side of `FooSystems`, especially after
seeing what the new names would actually look like in Avian and now
Bevy. I prefer it because it usually reads better, is much more clearly
related to groups of systems than `FooSet`, and overall *feels* more
correct and natural to me in the long term.
For these reasons, and because Alice and Cart also seemed to share a
preference for it when it was previously being discussed, I propose that
we adopt a `FooSystems` naming convention where applicable.
## Solution
Rename Bevy's system set types to use a consistent `FooSet` naming where
applicable.
- `AccessibilitySystem` → `AccessibilitySystems`
- `GizmoRenderSystem` → `GizmoRenderSystems`
- `PickSet` → `PickingSystems`
- `RunFixedMainLoopSystem` → `RunFixedMainLoopSystems`
- `TransformSystem` → `TransformSystems`
- `RemoteSet` → `RemoteSystems`
- `RenderSet` → `RenderSystems`
- `SpriteSystem` → `SpriteSystems`
- `StateTransitionSteps` → `StateTransitionSystems`
- `RenderUiSystem` → `RenderUiSystems`
- `UiSystem` → `UiSystems`
- `Animation` → `AnimationSystems`
- `AssetEvents` → `AssetEventSystems`
- `TrackAssets` → `AssetTrackingSystems`
- `UpdateGizmoMeshes` → `GizmoMeshSystems`
- `InputSystem` → `InputSystems`
- `InputFocusSet` → `InputFocusSystems`
- `ExtractMaterialsSet` → `MaterialExtractionSystems`
- `ExtractMeshesSet` → `MeshExtractionSystems`
- `RumbleSystem` → `RumbleSystems`
- `CameraUpdateSystem` → `CameraUpdateSystems`
- `ExtractAssetsSet` → `AssetExtractionSystems`
- `Update2dText` → `Text2dUpdateSystems`
- `TimeSystem` → `TimeSystems`
- `AudioPlaySet` → `AudioPlaybackSystems`
- `SendEvents` → `EventSenderSystems`
- `EventUpdates` → `EventUpdateSystems`
A lot of the names got slightly longer, but they are also a lot more
consistent, and in my opinion the majority of them read much better. For
a few of the names I took the liberty of rewording things a bit;
definitely open to any further naming improvements.
There are still also cases where the `FooSystems` naming doesn't really
make sense, and those I left alone. This primarily includes system sets
like `Interned<dyn SystemSet>`, `EnterSchedules<S>`, `ExitSchedules<S>`,
or `TransitionSchedules<S>`, where the type has some special purpose and
semantics.
## Todo
- [x] Should I keep all the old names as deprecated type aliases? I can
do this, but to avoid wasting work I'd prefer to first reach consensus
on whether these renames are even desired.
- [x] Migration guide
- [x] Release notes
The occlusion culling plugin checks for a GPU feature by looking at
`RenderAdapter`. This is wrong - it should be checking `RenderDevice`.
See these notes for background:
https://github.com/bevyengine/bevy/discussions/18973
I don't have any evidence that this was causing any bugs, so right now
it's just a precaution.
## Testing
```
cargo run --example occlusion_culling
```
Tested on Win10/Nvidia across Vulkan, WebGL/Chrome, WebGPU/Chrome.
# Objective
- The tonemapping example allows using a local image to try out
different color grading. However, using a local file stopped working
when we added the `UnapprovedPathMode` setting to the assets plugin.
## Solution
- Set `unapproved_path_mode: UnapprovedPathMode::Allow` in the example
## Testing
- I tried out the example with local images, previously it would fail
saying it's an untrusted path.
# Objective
The goal of `bevy_platform_support` is to provide a set of platform
agnostic APIs, alongside platform-specific functionality. This is a high
traffic crate (providing things like HashMap and Instant). Especially in
light of https://github.com/bevyengine/bevy/discussions/18799, it
deserves a friendlier / shorter name.
Given that it hasn't had a full release yet, getting this change in
before Bevy 0.16 makes sense.
## Solution
- Rename `bevy_platform_support` to `bevy_platform`.
# Objective
Fixes#16896Fixes#17737
## Solution
Adds a new render phase, including all the new cold specialization
patterns, for wireframes. There's a *lot* of regrettable duplication
here between 3d/2d.
## Testing
All the examples.
## Migration Guide
- `WireframePlugin` must now be created with
`WireframePlugin::default()`.
## Objective
The `MotionBlur` component exposes renderer internals. Users shouldn't
have to deal with this.
```rust
MotionBlur {
shutter_angle: 1.0,
samples: 2,
#[cfg(all(feature = "webgl2", target_arch = "wasm32", not(feature = "webgpu")))]
_webgl2_padding: Default::default(),
},
```
## Solution
The renderer now uses a separate `MotionBlurUniform` struct for its
internals. `MotionBlur` no longer needs padding.
I was a bit unsure about the name `MotionBlurUniform`. Other modules use
a mix of `Uniform` and `Uniforms`.
## Testing
```
cargo run --example motion_blur
```
Tested on Win10/Nvidia across Vulkan, WebGL/Chrome, WebGPU/Chrome.
# Objective
In #17905 we swapped to a named field on `ChildOf` to help resolve
variable naming ambiguity of child vs parent (ex: `child_of.parent`
clearly reads as "I am accessing the parent of the child_of
relationship", whereas `child_of.0` is less clear).
Unfortunately this has the side effect of making initialization less
ideal. `ChildOf { parent }` reads just as well as `ChildOf(parent)`, but
`ChildOf { parent: root }` doesn't read nearly as well as
`ChildOf(root)`.
## Solution
Move back to `ChildOf(pub Entity)` but add a `child_of.parent()`
function and use it for all accesses. The downside here is that users
are no longer "forced" to access the parent field with `parent`
nomenclature, but I think this strikes the right balance.
Take a look at the diff. I think the results provide strong evidence for
this change. Initialization has the benefit of reading much better _and_
of taking up significantly less space, as many lines go from 3 to 1, and
we're cutting out a bunch of syntax in some cases.
Sadly I do think this should land in 0.16 as the cost of doing this
_after_ the relationships migration is high.
# Objective
- bevy_core_pipeline is getting really big and it's a big bottleneck for
compilation time. A lot of parts of it can be broken up
## Solution
- Add a new bevy_anti_aliasing crate that contains all the anti_aliasing
implementations
- I didn't move any MSAA related code to this new crate because that's a
lot more invasive
## Testing
- Tested the anti_aliasing example to make sure all methods still worked
---
## Showcase
before:
after:
Notice that now bevy_core_pipeline is 1s shorter and bevy_anti_aliasing
now compiles in parallel with bevy_pbr.
## Migration Guide
When using anti aliasing features, you now need to import them from
`bevy::anti_aliasing` instead of `bevy::core_pipeline`
This reverts commit 0b5302d96a.
# Objective
- Fixes#18158
- #17482 introduced rendering changes and was merged a bit too fast
## Solution
- Revert #17482 so that it can be redone and rendering changes discussed
before being merged. This will make it easier to compare changes with
main in the known "valid" state
This is not an issue with the work done in #17482 that is still
interesting
# Objective
Transparently uses simple `EnvironmentMapLight`s to mimic
`AmbientLight`s. Implements the first part of #17468, but I can
implement hemispherical lights in this PR too if needed.
## Solution
- A function `EnvironmentMapLight::solid_color(&mut Assets<Image>,
Color)` is provided to make an environment light with a solid color.
- A new system is added to `SimulationLightSystems` that maps
`AmbientLight`s on views or the world to a corresponding
`EnvironmentMapLight`.
I have never worked with (or on) Bevy before, so nitpicky comments on
how I did things are appreciated :).
## Testing
Testing was done on a modified version of the `3d/lighting` example,
where I removed all lights except the ambient light. I have not included
the example, but can if required.
## Migration
`bevy_pbr::AmbientLight` has been deprecated, so all usages of it should
be replaced by a `bevy_pbr::EnvironmentMapLight` created with
`EnvironmentMapLight::solid_color` placed on the camera. There is no
alternative to ambient lights as resources.
# Objective
Fixes#18095
## Solution
Update the feature gates so that `Taa`, etc are added if
- Not on wasm
- OR using webgpu
## Testing
Check that `Taa` is disabled with appropriate messaging on webgl2
```
cargo run -p build-wasm-example -- --api webgl2 transmission && basic-http-server examples/wasm/
```
Check that `Taa` works on webgpu in chrome
```
cargo run -p build-wasm-example -- --api webgpu transmission && basic-http-server examples/wasm/
```
Check that `Taa` still works in a native build
```
cargo run -example transmission
```
# Objective
As discussed in #14275, Bevy is currently too prone to panic, and makes
the easy / beginner-friendly way to do a large number of operations just
to panic on failure.
This is seriously frustrating in library code, but also slows down
development, as many of the `Query::single` panics can actually safely
be an early return (these panics are often due to a small ordering issue
or a change in game state.
More critically, in most "finished" products, panics are unacceptable:
any unexpected failures should be handled elsewhere. That's where the
new
With the advent of good system error handling, we can now remove this.
Note: I was instrumental in a) introducing this idea in the first place
and b) pushing to make the panicking variant the default. The
introduction of both `let else` statements in Rust and the fancy system
error handling work in 0.16 have changed my mind on the right balance
here.
## Solution
1. Make `Query::single` and `Query::single_mut` (and other random
related methods) return a `Result`.
2. Handle all of Bevy's internal usage of these APIs.
3. Deprecate `Query::get_single` and friends, since we've moved their
functionality to the nice names.
4. Add detailed advice on how to best handle these errors.
Generally I like the diff here, although `get_single().unwrap()` in
tests is a bit of a downgrade.
## Testing
I've done a global search for `.single` to track down any missed
deprecated usages.
As to whether or not all the migrations were successful, that's what CI
is for :)
## Future work
~~Rename `Query::get_single` and friends to `Query::single`!~~
~~I've opted not to do this in this PR, and smear it across two releases
in order to ease the migration. Successive deprecations are much easier
to manage than the semantics and types shifting under your feet.~~
Cart has convinced me to change my mind on this; see
https://github.com/bevyengine/bevy/pull/18082#discussion_r1974536085.
## Migration guide
`Query::single`, `Query::single_mut` and their `QueryState` equivalents
now return a `Result`. Generally, you'll want to:
1. Use Bevy 0.16's system error handling to return a `Result` using the
`?` operator.
2. Use a `let else Ok(data)` block to early return if it's an expected
failure.
3. Use `unwrap()` or `Ok` destructuring inside of tests.
The old `Query::get_single` (etc) methods which did this have been
deprecated.
# Objective
fixes#17896
## Solution
Change ChildOf ( Entity ) to ChildOf { parent: Entity }
by doing this we also allow users to use named structs for relationship
derives, When you have more than 1 field in a struct with named fields
the macro will look for a field with the attribute #[relationship] and
all of the other fields should implement the Default trait. Unnamed
fields are still supported.
When u have a unnamed struct with more than one field the macro will
fail.
Do we want to support something like this ?
```rust
#[derive(Component)]
#[relationship_target(relationship = ChildOf)]
pub struct Children (#[relationship] Entity, u8);
```
I could add this, it but doesn't seem nice.
## Testing
crates/bevy_ecs - cargo test
## Showcase
```rust
use bevy_ecs::component::Component;
use bevy_ecs::entity::Entity;
#[derive(Component)]
#[relationship(relationship_target = Children)]
pub struct ChildOf {
#[relationship]
pub parent: Entity,
internal: u8,
};
#[derive(Component)]
#[relationship_target(relationship = ChildOf)]
pub struct Children {
children: Vec<Entity>
};
```
---------
Co-authored-by: Tim Overbeek <oorbecktim@Tims-MacBook-Pro.local>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-042.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-059.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-054.client.nl.eduvpn.org>
Co-authored-by: Tim Overbeek <oorbecktim@c-001-001-027.client.nl.eduvpn.org>
# Objective
- Fixes#17960
## Solution
- Followed the [edition upgrade
guide](https://doc.rust-lang.org/edition-guide/editions/transitioning-an-existing-project-to-a-new-edition.html)
## Testing
- CI
---
## Summary of Changes
### Documentation Indentation
When using lists in documentation, proper indentation is now linted for.
This means subsequent lines within the same list item must start at the
same indentation level as the item.
```rust
/* Valid */
/// - Item 1
/// Run-on sentence.
/// - Item 2
struct Foo;
/* Invalid */
/// - Item 1
/// Run-on sentence.
/// - Item 2
struct Foo;
```
### Implicit `!` to `()` Conversion
`!` (the never return type, returned by `panic!`, etc.) no longer
implicitly converts to `()`. This is particularly painful for systems
with `todo!` or `panic!` statements, as they will no longer be functions
returning `()` (or `Result<()>`), making them invalid systems for
functions like `add_systems`. The ideal fix would be to accept functions
returning `!` (or rather, _not_ returning), but this is blocked on the
[stabilisation of the `!` type
itself](https://doc.rust-lang.org/std/primitive.never.html), which is
not done.
The "simple" fix would be to add an explicit `-> ()` to system
signatures (e.g., `|| { todo!() }` becomes `|| -> () { todo!() }`).
However, this is _also_ banned, as there is an existing lint which (IMO,
incorrectly) marks this as an unnecessary annotation.
So, the "fix" (read: workaround) is to put these kinds of `|| -> ! { ...
}` closuers into variables and give the variable an explicit type (e.g.,
`fn()`).
```rust
// Valid
let system: fn() = || todo!("Not implemented yet!");
app.add_systems(..., system);
// Invalid
app.add_systems(..., || todo!("Not implemented yet!"));
```
### Temporary Variable Lifetimes
The order in which temporary variables are dropped has changed. The
simple fix here is _usually_ to just assign temporaries to a named
variable before use.
### `gen` is a keyword
We can no longer use the name `gen` as it is reserved for a future
generator syntax. This involved replacing uses of the name `gen` with
`r#gen` (the raw-identifier syntax).
### Formatting has changed
Use statements have had the order of imports changed, causing a
substantial +/-3,000 diff when applied. For now, I have opted-out of
this change by amending `rustfmt.toml`
```toml
style_edition = "2021"
```
This preserves the original formatting for now, reducing the size of
this PR. It would be a simple followup to update this to 2024 and run
`cargo fmt`.
### New `use<>` Opt-Out Syntax
Lifetimes are now implicitly included in RPIT types. There was a handful
of instances where it needed to be added to satisfy the borrow checker,
but there may be more cases where it _should_ be added to avoid
breakages in user code.
### `MyUnitStruct { .. }` is an invalid pattern
Previously, you could match against unit structs (and unit enum
variants) with a `{ .. }` destructuring. This is no longer valid.
### Pretty much every use of `ref` and `mut` are gone
Pattern binding has changed to the point where these terms are largely
unused now. They still serve a purpose, but it is far more niche now.
### `iter::repeat(...).take(...)` is bad
New lint recommends using the more explicit `iter::repeat_n(..., ...)`
instead.
## Migration Guide
The lifetimes of functions using return-position impl-trait (RPIT) are
likely _more_ conservative than they had been previously. If you
encounter lifetime issues with such a function, please create an issue
to investigate the addition of `+ use<...>`.
## Notes
- Check the individual commits for a clearer breakdown for what
_actually_ changed.
---------
Co-authored-by: François Mockers <francois.mockers@vleue.com>
Fixes#17856.
## Migration Guide
- `EventWriter::send` has been renamed to `EventWriter::write`.
- `EventWriter::send_batch` has been renamed to
`EventWriter::write_batch`.
- `EventWriter::send_default` has been renamed to
`EventWriter::write_default`.
---------
Co-authored-by: François Mockers <mockersf@gmail.com>
The GPU can fill out many of the fields in `IndirectParametersMetadata`
using information it already has:
* `early_instance_count` and `late_instance_count` are always
initialized to zero.
* `mesh_index` is already present in the work item buffer as the
`input_index` of the first work item in each batch.
This patch moves these fields to a separate buffer, the *GPU indirect
parameters metadata* buffer. That way, it avoids having to write them on
CPU during `batch_and_prepare_binned_render_phase`. This effectively
reduces the number of bits that that function must write per mesh from
160 to 64 (in addition to the 64 bits per mesh *instance*).
Additionally, this PR refactors `UntypedPhaseIndirectParametersBuffers`
to add another layer, `MeshClassIndirectParametersBuffers`, which allows
abstracting over the buffers corresponding indexed and non-indexed
meshes. This patch doesn't make much use of this abstraction, but
forthcoming patches will, and it's overall a cleaner approach.
This didn't seem to have much of an effect by itself on
`batch_and_prepare_binned_render_phase` time, but subsequent PRs
dependent on this PR yield roughly a 2× speedup.
Currently, invocations of `batch_and_prepare_binned_render_phase` and
`batch_and_prepare_sorted_render_phase` can't run in parallel because
they write to scene-global GPU buffers. After PR #17698,
`batch_and_prepare_binned_render_phase` started accounting for the
lion's share of the CPU time, causing us to be strongly CPU bound on
scenes like Caldera when occlusion culling was on (because of the
overhead of batching for the Z-prepass). Although I eventually plan to
optimize `batch_and_prepare_binned_render_phase`, we can obtain
significant wins now by parallelizing that system across phases.
This commit splits all GPU buffers that
`batch_and_prepare_binned_render_phase` and
`batch_and_prepare_sorted_render_phase` touches into separate buffers
for each phase so that the scheduler will run those phases in parallel.
At the end of batch preparation, we gather the render phases up into a
single resource with a new *collection* phase. Because we already run
mesh preprocessing separately for each phase in order to make occlusion
culling work, this is actually a cleaner separation. For example, mesh
output indices (the unique ID that identifies each mesh instance on GPU)
are now guaranteed to be sequential starting from 0, which will simplify
the forthcoming work to remove them in favor of the compute dispatch ID.
On Caldera, this brings the frame time down to approximately 9.1 ms with
occlusion culling on.
# Objective
It's difficult to understand or make changes to the UI systems because
of how each system needs to individually track changes to scale factor,
windows and camera targets in local hashmaps, particularly for new
contributors. Any major change inevitably introduces new scale factor
bugs.
Instead of per-system resolution we can resolve the camera target info
for all UI nodes in a system at the start of `PostUpdate` and then store
it per-node in components that can be queried with change detection.
Fixes#17578Fixes#15143
## Solution
Store the UI render target's data locally per node in a component that
is updated in `PostUpdate` before any other UI systems run.
This component can be then be queried with change detection so that UI
systems no longer need to have knowledge of cameras and windows and
don't require fragile custom change detection solutions using local
hashmaps.
## Showcase
Compare `measure_text_system` from main (which has a bug the causes it
to use the wrong scale factor when a node's camera target changes):
```
pub fn measure_text_system(
mut scale_factors_buffer: Local<EntityHashMap<f32>>,
mut last_scale_factors: Local<EntityHashMap<f32>>,
fonts: Res<Assets<Font>>,
camera_query: Query<(Entity, &Camera)>,
default_ui_camera: DefaultUiCamera,
ui_scale: Res<UiScale>,
mut text_query: Query<
(
Entity,
Ref<TextLayout>,
&mut ContentSize,
&mut TextNodeFlags,
&mut ComputedTextBlock,
Option<&UiTargetCamera>,
),
With<Node>,
>,
mut text_reader: TextUiReader,
mut text_pipeline: ResMut<TextPipeline>,
mut font_system: ResMut<CosmicFontSystem>,
) {
scale_factors_buffer.clear();
let default_camera_entity = default_ui_camera.get();
for (entity, block, content_size, text_flags, computed, maybe_camera) in &mut text_query {
let Some(camera_entity) = maybe_camera
.map(UiTargetCamera::entity)
.or(default_camera_entity)
else {
continue;
};
let scale_factor = match scale_factors_buffer.entry(camera_entity) {
Entry::Occupied(entry) => *entry.get(),
Entry::Vacant(entry) => *entry.insert(
camera_query
.get(camera_entity)
.ok()
.and_then(|(_, c)| c.target_scaling_factor())
.unwrap_or(1.0)
* ui_scale.0,
),
};
if last_scale_factors.get(&camera_entity) != Some(&scale_factor)
|| computed.needs_rerender()
|| text_flags.needs_measure_fn
|| content_size.is_added()
{
create_text_measure(
entity,
&fonts,
scale_factor.into(),
text_reader.iter(entity),
block,
&mut text_pipeline,
content_size,
text_flags,
computed,
&mut font_system,
);
}
}
core::mem::swap(&mut *last_scale_factors, &mut *scale_factors_buffer);
}
```
with `measure_text_system` from this PR (which always uses the correct
scale factor):
```
pub fn measure_text_system(
fonts: Res<Assets<Font>>,
mut text_query: Query<
(
Entity,
Ref<TextLayout>,
&mut ContentSize,
&mut TextNodeFlags,
&mut ComputedTextBlock,
Ref<ComputedNodeTarget>,
),
With<Node>,
>,
mut text_reader: TextUiReader,
mut text_pipeline: ResMut<TextPipeline>,
mut font_system: ResMut<CosmicFontSystem>,
) {
for (entity, block, content_size, text_flags, computed, computed_target) in &mut text_query {
// Note: the ComputedTextBlock::needs_rerender bool is cleared in create_text_measure().
if computed_target.is_changed()
|| computed.needs_rerender()
|| text_flags.needs_measure_fn
|| content_size.is_added()
{
create_text_measure(
entity,
&fonts,
computed_target.scale_factor.into(),
text_reader.iter(entity),
block,
&mut text_pipeline,
content_size,
text_flags,
computed,
&mut font_system,
);
}
}
}
```
## Testing
I removed an alarming number of tests from the `layout` module but they
were mostly to do with the deleted camera synchronisation logic. The
remaining tests should all pass now.
The most relevant examples are `multiple_windows` and `split_screen`,
the behaviour of both should be unchanged from main.
---------
Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
