Commit graph

5968 commits

Author SHA1 Message Date
Benjamin Brienen
b744fb486b
Document why MAX_JOINTS and MAX_MORPH_WEIGHTS are set (#16324)
# Objective

Fixes #15974

## Solution

Add the comment from @mockersf, adapted to fix in-context.
2024-12-10 02:03:51 +00:00
Lynn
fcaa271693
Polygon simplicity (#15981)
# Objective

- This PR adds the ability to determine whether a `Polygon<N>` or
`BoxedPolygon` is simple (aka. not self-intersecting) by calling
`my_polygon.is_simple()`.
- This may be useful information for users to determine whether their
polygons are 'valid' and will be useful when adding meshing for
polygons.
  - As such this is a step towards fixing #15255

## Solution

- Implemented the Shamos-Hoey algorithm in its own module `polygon`.

## Testing

- Tests are included, and can be verified visually.

---

## Performance

- The Shamos-Hoey algorithm runs in O(n * log n)
- In reality, the results look more linear to me.
- Determining simplicity for a simple polygon (the worst case) with less
than 100 vertices takes less than 0.2ms.


![image](https://github.com/user-attachments/assets/23c62234-abdc-4710-a3b4-feaad5929133)
2024-12-10 02:02:12 +00:00
Gino Valente
d21c7a1911
bevy_reflect: Function Overloading (Generic & Variadic Functions) (#15074)
# Objective

Currently function reflection requires users to manually monomorphize
their generic functions. For example:

```rust
fn add<T: Add<Output=T>>(a: T, b: T) -> T {
    a + b
}

// We have to specify the type of `T`:
let reflect_add = add::<i32>.into_function();
```

This PR doesn't aim to solve that problem—this is just a limitation in
Rust. However, it also means that reflected functions can only ever work
for a single monomorphization. If we wanted to support other types for
`T`, we'd have to create a separate function for each one:

```rust
let reflect_add_i32 = add::<i32>.into_function();
let reflect_add_u32 = add::<u32>.into_function();
let reflect_add_f32 = add::<f32>.into_function();
// ...
```

So in addition to requiring manual monomorphization, we also lose the
benefit of having a single function handle multiple argument types.

If a user wanted to create a small modding script that utilized function
reflection, they'd have to either:
- Store all sets of supported monomorphizations and require users to
call the correct one
- Write out some logic to find the correct function based on the given
arguments

While the first option would work, it wouldn't be very ergonomic. The
second option is better, but it adds additional complexity to the user's
logic—complexity that `bevy_reflect` could instead take on.

## Solution

Introduce [function
overloading](https://en.wikipedia.org/wiki/Function_overloading).

A `DynamicFunction` can now be overloaded with other `DynamicFunction`s.
We can rewrite the above code like so:

```rust
let reflect_add = add::<i32>
    .into_function()
    .with_overload(add::<u32>)
    .with_overload(add::<f32>);
```

When invoked, the `DynamicFunction` will attempt to find a matching
overload for the given set of arguments.

And while I went into this PR only looking to improve generic function
reflection, I accidentally added support for variadic functions as well
(hence why I use the broader term "overload" over "generic").

```rust
// Supports 1 to 4 arguments
let multiply_all = (|a: i32| a)
    .into_function()
    .with_overload(|a: i32, b: i32| a * b)
    .with_overload(|a: i32, b: i32, c: i32| a * b * c)
    .with_overload(|a: i32, b: i32, c: i32, d: i32| a * b * c * d);
```

This is simply an added bonus to this particular implementation. ~~Full
variadic support (i.e. allowing for an indefinite number of arguments)
will be added in a later PR.~~ I actually decided to limit the maximum
number of arguments to 63 to supplement faster lookups, a reduced memory
footprint, and faster cloning.

### Alternatives & Rationale

I explored a few options for handling generic functions. This PR is the
one I feel the most confident in, but I feel I should mention the others
and why I ultimately didn't move forward with them.

#### Adding `GenericDynamicFunction`

**TL;DR:** Adding a distinct `GenericDynamicFunction` type unnecessarily
splits and complicates the API.

<details>
<summary>Details</summary>

My initial explorations involved a dedicated `GenericDynamicFunction` to
contain and handle the mappings.

This was initially started back when `DynamicFunction` was distinct from
`DynamicClosure`. My goal was to not prevent us from being able to
somehow make `DynamicFunction` implement `Copy`. But once we reverted
back to a single `DynamicFunction`, that became a non-issue.

But that aside, the real problem was that it created a split in the API.
If I'm using a third-party library that uses function reflection, I have
to know whether to request a `DynamicFunction` or a
`GenericDynamicFunction`. I might not even know ahead of time which one
I want. It might need to be determined at runtime.

And if I'm creating a library, I might want a type to contain both
`DynamicFunction` and `GenericDynamicFunction`. This might not be
possible if, for example, I need to store the function in a `HashMap`.

The other concern is with `IntoFunction`. Right now `DynamicFunction`
trivially implements `IntoFunction` since it can just return itself. But
what should `GenericDynamicFunction` do? It could return itself wrapped
into a `DynamicFunction`, but then the API for `DynamicFunction` would
have to account for this. So then what was the point of having a
separate `GenericDynamicFunction` anyways?

And even apart from `IntoFunction`, there's nothing stopping someone
from manually creating a generic `DynamicFunction` through lying about
its `FunctionInfo` and wrapping a `GenericDynamicFunction`.

That being said, this is probably the "best" alternative if we added a
`Function` trait and stored functions as `Box<dyn Function>`.

However, I'm not convinced we gain much from this. Sure, we could keep
the API for `DynamicFunction` the same, but consumers of `Function` will
need to account for `GenericDynamicFunction` regardless (e.g. handling
multiple `FunctionInfo`, a ranged argument count, etc.). And for all
cases, except where using `DynamicFunction` directly, you end up
treating them all like `GenericDynamicFunction`.

Right now, if we did go with `GenericDynamicFunction`, the only major
benefit we'd gain would be saving 24 bytes. If memory ever does become
an issue here, we could swap over. But I think for the time being it's
better for us to pursue a clearer mental model and end-user ergonomics
through unification.

</details>

##### Using the `FunctionRegistry`

**TL;DR:** Having overloads only exist in the `FunctionRegistry`
unnecessarily splits and complicates the API.

<details>
<summary>Details</summary>

Another idea was to store the overloads in the `FunctionRegistry`. Users
would then just call functions directly through the registry (i.e.
`registry.call("my_func", my_args)`).

I didn't go with this option because of how it specifically relies on
the functions being registered. You'd not only always need access to the
registry, but you'd need to ensure that the functions you want to call
are even registered.

It also means you can't just store a generic `DynamicFunction` on a
type. Instead, you'll need to store the function's name and use that to
look up the function in the registry—even if it's only ever used by that
type.

Doing so also removes all the benefits of `DynamicFunction`, such as the
ability to pass it to functions accepting `IntoFunction`, modify it if
needed, and so on.

Like `GenericDynamicFunction` this introduces a split in the ecosystem:
you either store `DynamicFunction`, store a string to look up the
function, or force `DynamicFunction` to wrap your generic function
anyways. Or worse yet: have `DynamicFunction` wrap the lookup function
using `FunctionRegistryArc`.

</details>

#### Generic `ArgInfo`

**TL;DR:** Allowing `ArgInfo` and `ReturnInfo` to store the generic
information introduces a footgun when interpreting `FunctionInfo`.

<details>
<summary>Details</summary>

Regardless of how we represent a generic function, one thing is clear:
we need to be able to represent the information for such a function.

This PR does so by introducing a `FunctionInfoType` enum to wrap one or
more `FunctionInfo` values.

Originally, I didn't do this. I had `ArgInfo` and `ReturnInfo` allow for
generic types. This allowed us to have a single `FunctionInfo` to
represent our function, but then I realized that it actually lies about
our function.

If we have two `ArgInfo` that both allow for either `i32` or `u32`, what
does this tell us about our function? It turns out: nothing! We can't
know whether our function takes `(i32, i32)`, `(u32, u32)`, `(i32,
u32)`, or `(u32, i32)`.

It therefore makes more sense to just represent a function with multiple
`FunctionInfo` since that's really what it's made up of.

</details>

#### Flatten `FunctionInfo`

**TL;DR:** Flattening removes additional per-overload information some
users may desire and prevents us from adding more information in the
future.

<details>
<summary>Details</summary>

Why don't we just flatten multiple `FunctionInfo` into just one that can
contain multiple signatures?

This is something we could do, but I decided against it for a few
reasons:
- The only thing we'd be able to get rid of for each signature would be
the `name`. While not enough to not do it, it doesn't really suggest we
*have* to either.
- Some consumers may want access to the names of the functions that make
up the overloaded function. For example, to track a bug where an
undesirable function is being added as an overload. Or to more easily
locate the original function of an overload.
- We may eventually allow for more information to be stored on
`FunctionInfo`. For example, we may allow for documentation to be stored
like we do for `TypeInfo`. Consumers of this documentation may want
access to the documentation of each overload as they may provide
documentation specific to that overload.

</details>

## Testing

This PR adds lots of tests and benchmarks, and also adds to the example.

To run the tests:

```
cargo test --package bevy_reflect --all-features
```

To run the benchmarks:

```
cargo bench --bench reflect_function --all-features
```

To run the example:

```
cargo run --package bevy --example function_reflection --all-features
```

### Benchmarks

One of my goals with this PR was to leave the typical case of
non-overloaded functions largely unaffected by the changes introduced in
this PR. ~~And while the static size of `DynamicFunction` has increased
by 17% (from 136 to 160 bytes), the performance has generally stayed the
same~~ The static size of `DynamicFunction` has decreased from 136 to
112 bytes, while calling performance has generally stayed the same:

|                                     | `main` | 7d293ab | 252f3897d |
|-------------------------------------|--------|---------|-----------|
| `into/function`                     | 37 ns  | 46 ns   | 142 ns    |
| `with_overload/01_simple_overload`  | -      | 149 ns  | 268 ns    |
| `with_overload/01_complex_overload` | -      | 332 ns  | 431 ns    |
| `with_overload/10_simple_overload`  | -      | 1266 ns | 2618 ns   |
| `with_overload/10_complex_overload` | -      | 2544 ns | 4170 ns   |
| `call/function`                     | 57 ns  | 58 ns   | 61 ns     |
| `call/01_simple_overload`           | -      | 255 ns  | 242 ns    |
| `call/01_complex_overload`          | -      | 595 ns  | 431 ns    |
| `call/10_simple_overload`           | -      | 740 ns  | 699 ns    |
| `call/10_complex_overload`          | -      | 1824 ns | 1618 ns   |

For the overloaded function tests, the leading number indicates how many
overloads there are: `01` indicates 1 overload, `10` indicates 10
overloads. The `complex` cases have 10 unique generic types and 10
arguments, compared to the `simple` 1 generic type and 2 arguments.

I aimed to prioritize the performance of calling the functions over
creating them, hence creation speed tends to be a bit slower.

There may be other optimizations we can look into but that's probably
best saved for a future PR.

The important bit is that the standard ~~`into/function`~~ and
`call/function` benchmarks show minimal regressions. Since the latest
changes, `into/function` does have some regressions, but again the
priority was `call/function`. We can probably optimize `into/function`
if needed in the future.

---

## Showcase

Function reflection now supports [function
overloading](https://en.wikipedia.org/wiki/Function_overloading)! This
can be used to simulate generic functions:

```rust
fn add<T: Add<Output=T>>(a: T, b: T) -> T {
    a + b
}

let reflect_add = add::<i32>
    .into_function()
    .with_overload(add::<u32>)
    .with_overload(add::<f32>);

let args = ArgList::default().push_owned(25_i32).push_owned(75_i32);  
let result = func.call(args).unwrap().unwrap_owned();  
assert_eq!(result.try_take::<i32>().unwrap(), 100);  
  
let args = ArgList::default().push_owned(25.0_f32).push_owned(75.0_f32);  
let result = func.call(args).unwrap().unwrap_owned();  
assert_eq!(result.try_take::<f32>().unwrap(), 100.0);
```

You can also simulate variadic functions:

```rust
#[derive(Reflect, PartialEq, Debug)]
struct Player {
    name: Option<String>,
    health: u32,
}

// Creates a `Player` with one of the following:  
// - No name and 100 health  
// - A name and 100 health  
// - No name and custom health  
// - A name and custom health
let create_player = (|| Player {
        name: None,
        health: 100,
    })
    .into_function()
    .with_overload(|name: String| Player {
        name: Some(name),
        health: 100,
    })
    .with_overload(|health: u32| Player {
        name: None,
        health
    })
    .with_overload(|name: String, health: u32| Player {
        name: Some(name),
        health,
    });

let args = ArgList::default()
    .push_owned(String::from("Urist"))
    .push_owned(55_u32);
    
let player = create_player
    .call(args)
    .unwrap()
    .unwrap_owned()
    .try_take::<Player>()
    .unwrap();
	
assert_eq!(
    player,
    Player {
        name: Some(String::from("Urist")),
        health: 55
    }
);
```
2024-12-10 01:51:47 +00:00
Patrick Walton
1e7b89cdf5
Allow holes in the MeshInputUniform buffer. (#16695)
This commit removes the logic that attempted to keep the
`MeshInputUniform` buffer contiguous. Not only was it slow and complex,
but it was also incorrect, which caused #16686 and #16690. I changed the
logic to simply maintain a free list of unused slots in the buffer and
preferentially fill them when pushing new mesh input uniforms.

Closes #16686.
Closes #16690.
2024-12-09 02:11:27 +00:00
Aevyrie
61b98ec80f
Rename trigger.entity() to trigger.target() (#16716)
# Objective

- A `Trigger` has multiple associated `Entity`s - the entity observing
the event, and the entity that was targeted by the event.
- The field `entity: Entity` encodes no semantic information about what
the entity is used for, you can already tell that it's an `Entity` by
the type signature!

## Solution

- Rename `trigger.entity()` to `trigger.target()`

---

## Changelog

- `Trigger`s are associated with multiple entities. `Trigger::entity()`
has been renamed to `Trigger::target()` to reflect the semantics of the
entity being returned.

## Migration Guide

- Rename `Trigger::entity()` to `Trigger::target()`.
- Rename `ObserverTrigger::entity` to `ObserverTrigger::target`
2024-12-08 21:55:09 +00:00
homersimpsons
a6b5f80715
⬆️ Upgrade typos and its configuration (#16712)
# Objective

Fixes #16610, related to #16702

## Solution

Upgrade typos and its configuration

## Testing

- Did you test these changes? If so, how? No
- 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? No
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test? Not applicable
2024-12-08 17:25:10 +00:00
poopy
4aed2ca74c
Add World::try_resource_scope (#16707)
# Objective

Fixes #16706

## Solution 

- Added new method: `try_resource_scope` which returns `None` if the
requested resource doesn't exist.
- Changed the `resource_scope` test to use `try_resource_scope` as well
to test for the `None` case.

---

## Showcase

```rust
world.try_resource_scope::<MyResource, _>(|world, mut my_resource| {
    // do something with the resource if it exists
});
```
2024-12-08 15:40:09 +00:00
homersimpsons
0707c0717b
✏️ Fix typos across bevy (#16702)
# Objective

Fixes typos in bevy project, following suggestion in
https://github.com/bevyengine/bevy-website/pull/1912#pullrequestreview-2483499337

## Solution

I used https://github.com/crate-ci/typos to find them.

I included only the ones that feel undebatable too me, but I am not in
game engine so maybe some terms are expected.

I left out the following typos:
- `reparametrize` => `reparameterize`: There are a lot of occurences, I
believe this was expected
- `semicircles` => `hemicircles`: 2 occurences, may mean something
specific in geometry
- `invertation` => `inversion`: may mean something specific
- `unparented` => `parentless`: may mean something specific
- `metalness` => `metallicity`: may mean something specific

## Testing

- Did you test these changes? If so, how? I did not test the changes,
most changes are related to raw text. I expect the others to be tested
by the CI.
- Are there any parts that need more testing? I do not think
- How can other people (reviewers) test your changes? Is there anything
specific they need to know? To me there is nothing to test
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?

---

## Migration Guide

> This section is optional. If there are no breaking changes, you can
delete this section.

(kept in case I include the `reparameterize` change here)

- If this PR is a breaking change (relative to the last release of
Bevy), describe how a user might need to migrate their code to support
these changes
- Simply adding new functionality is not a breaking change.
- Fixing behavior that was definitely a bug, rather than a questionable
design choice is not a breaking change.

## Questions

- [x] Should I include the above typos? No
(https://github.com/bevyengine/bevy/pull/16702#issuecomment-2525271152)
- [ ] Should I add `typos` to the CI? (I will check how to configure it
properly)

This project looks awesome, I really enjoy reading the progress made,
thanks to everyone involved.
2024-12-08 01:18:39 +00:00
Patrick Walton
f5de3f08fb
Use multidraw for opaque meshes when GPU culling is in use. (#16427)
This commit adds support for *multidraw*, which is a feature that allows
multiple meshes to be drawn in a single drawcall. `wgpu` currently
implements multidraw on Vulkan, so this feature is only enabled there.
Multiple meshes can be drawn at once if they're in the same vertex and
index buffers and are otherwise placed in the same bin. (Thus, for
example, at present the materials and textures must be identical, but
see #16368.) Multidraw is a significant performance improvement during
the draw phase because it reduces the number of rebindings, as well as
the number of drawcalls.

This feature is currently only enabled when GPU culling is used: i.e.
when `GpuCulling` is present on a camera. Therefore, if you run for
example `scene_viewer`, you will not see any performance improvements,
because `scene_viewer` doesn't add the `GpuCulling` component to its
camera.

Additionally, the multidraw feature is only implemented for opaque 3D
meshes and not for shadows or 2D meshes. I plan to make GPU culling the
default and to extend the feature to shadows in the future. Also, in the
future I suspect that polyfilling multidraw on APIs that don't support
it will be fruitful, as even without driver-level support use of
multidraw allows us to avoid expensive `wgpu` rebindings.
2024-12-06 17:22:03 +00:00
Joona Aalto
4d6b02af89
Fix mehses -> meshes typo (#16688)
# Objective

The "mehses" typo introduced in a review comment
[here](https://github.com/bevyengine/bevy/pull/16657#discussion_r1870834999)
hurts my soul, it was merged right as I was about to comment about it :(

## Solution

Fix it :D

(also, why didn't the CI typo checker catch this?)
2024-12-06 17:09:10 +00:00
Zachary Harrold
a6adced9ed
Deny derive_more error feature and replace it with thiserror (#16684)
# Objective

- Remove `derive_more`'s error derivation and replace it with
`thiserror`

## Solution

- Added `derive_more`'s `error` feature to `deny.toml` to prevent it
sneaking back in.
- Reverted to `thiserror` error derivation

## Notes

Merge conflicts were too numerous to revert the individual changes, so
this reversion was done manually. Please scrutinise carefully during
review.
2024-12-06 17:03:55 +00:00
JaySpruce
d0afdc6b45
Move clone_entity commands to EntityCommands (#16672)
## Objective

I was resolving a conflict between #16132 and my PR #15929 and thought
the `clone_entity` commands made more sense in `EntityCommands`.

## Solution

Moved `Commands::clone_entity` to `EntityCommands::clone`, moved
`Commands::clone_entity_with` to `EntityCommands::clone_with`.

## Testing

Ran the two tests that used the old methods.

## Showcase

```
// Create a new entity and keep its EntityCommands.
let mut entity = commands.spawn((ComponentA(10), ComponentB(20)));

// Create a clone of the first entity
let mut entity_clone = entity.clone();
```

The only potential downside is that the method name is now the same as
the one from the `Clone` trait. `EntityCommands` doesn't implement
`Clone` though, so there's no actual conflict.

Maybe I'm biased because this'll work better with my PR, but I think the
UX is nicer regardless.
2024-12-06 15:54:35 +00:00
Jakob Hellermann
10e3cc72ad
add missing type registration for Monitor (#16685)
# Objective


![image](https://github.com/user-attachments/assets/4b8d6a2c-86ed-4353-8133-0e0efdb3a697)
make `Monitor` reflectable by default

## Solution

- register type
2024-12-06 15:20:10 +00:00
Rob Parrett
5b1f0b1ef5
Fix error in volumetric fog shader (#16677)
# Objective

Volumetric fog was broken by #13746.

Looks like this particular shader just got missed. I don't see any other
instances of `unpack_offset_and_counts` in the codebase.

```
2024-12-06T03:18:42.297494Z ERROR bevy_render::render_resource::pipeline_cache: failed to process shader:
error: no definition in scope for identifier: 'bevy_pbr::clustered_forward::unpack_offset_and_counts'
    ┌─ crates/bevy_pbr/src/volumetric_fog/volumetric_fog.wgsl:312:29
    │
312 │     let offset_and_counts = bevy_pbr::clustered_forward::unpack_offset_and_counts(cluster_index);
    │                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unknown identifier
    │
    = no definition in scope for identifier: 'bevy_pbr::clustered_forward::unpack_offset_and_counts'
```

## Solution

Use `unpack_clusterable_object_index_ranges` to get the indices for
point/spot lights.

## Testing

`cargo run --example volumetric_fog`
`cargo run --example fog_volumes`
`cargo run --example scrolling_fog`
2024-12-06 08:49:18 +00:00
Zachary Harrold
72f096c91e
Add no_std support to bevy_tasks (#15464)
# Objective

- Contributes to #15460

## Solution

- Added the following features:
  - `std` (default)
  - `async_executor` (default)
  - `edge_executor`
  - `critical-section`
  - `portable-atomic`
- Added [`edge-executor`](https://crates.io/crates/edge-executor) as a
`no_std` alternative to `async-executor`.
- Updated the `single_threaded_task_pool` to work in `no_std`
environments by gating its reliance on `thread_local`.

## Testing

- Added to `compile-check-no-std` CI command

## Notes

- In previous iterations of this PR, a custom `async-executor`
alternative was vendored in. This raised concerns around maintenance and
testing. In this iteration, an existing version of that same vendoring
is now used, but _only_ in `no_std` contexts. For existing `std`
contexts, the original `async-executor` is used.
- Due to the way statics work, certain `TaskPool` operations have added
restrictions around `Send`/`Sync` in `no_std`. This is because there
isn't a straightforward way to create a thread-local in `no_std`. If
these added constraints pose an issue we can revisit this at a later
date.
- If a user enables both the `async_executor` and `edge_executor`
features, we will default to using `async-executor`. Since enabling
`async_executor` requires `std`, we can safely assume we are in an `std`
context and use the original library.

---------

Co-authored-by: Mike <2180432+hymm@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-12-06 02:14:54 +00:00
Talin
bc572cd270
bevy_input_focus improvements (follow-up PR) (#16665)
This adds a few minor items which were left out of the previous PR:

- Added synchronization from bevy_input_focus to bevy_a11y.
- Initialize InputFocusVisible resource.
- Make `input_focus` available from `bevy` module.

I've tested this using VoiceOver on Mac OS. It works, but it needs
considerable polish.
2024-12-06 01:16:52 +00:00
Nuutti Kotivuori
912da04699
Run observers before hooks for on_replace and on_remove (#16499)
# Objective

- Fixes #16498 

## Solution

- Trivially swaps ordering of hooks and observers for all call sites
where they are triggered for `on_replace` or `on_remove`

## Testing

- Just CI

---

## Migration Guide

The order of hooks and observers for `on_replace` and `on_remove` has
been swapped. Observers are now run before hooks. This is a more natural
ordering where the removal ordering is inverted compared to the
insertion ordering.
2024-12-06 00:24:27 +00:00
Miles Silberling-Cook
0070514f54
Fallible systems (#16589)
# Objective

Error handling in bevy is hard. See for reference
https://github.com/bevyengine/bevy/issues/11562,
https://github.com/bevyengine/bevy/issues/10874 and
https://github.com/bevyengine/bevy/issues/12660. The goal of this PR is
to make it better, by allowing users to optionally return `Result` from
systems as outlined by Cart in
<https://github.com/bevyengine/bevy/issues/14275#issuecomment-2223708314>.

## Solution

This PR introduces a new `ScheuleSystem` type to represent systems that
can be added to schedules. Instances of this type contain either an
infallible `BoxedSystem<(), ()>` or a fallible `BoxedSystem<(),
Result>`. `ScheuleSystem` implements `System<In = (), Out = Result>` and
replaces all uses of `BoxedSystem` in schedules. The async executor now
receives a result after executing a system, which for infallible systems
is always `Ok(())`. Currently it ignores this result, but more useful
error handling could also be implemented.

Aliases for `Error` and `Result` have been added to the `bevy_ecs`
prelude, as well as const `OK` which new users may find more friendly
than `Ok(())`.

## Testing

- Currently there are not actual semantics changes that really require
new tests, but I added a basic one just to make sure we don't break
stuff in the future.
- The behavior of existing systems is totally unchanged, including
logging.
- All of the existing systems tests pass, and I have not noticed
anything strange while playing with the examples

## Showcase

The following minimal example prints "hello world" once, then completes.

```rust
use bevy::prelude::*;

fn main() {
    App::new().add_systems(Update, hello_world_system).run();
}

fn hello_world_system() -> Result {
    println!("hello world");
    Err("string")?;
    println!("goodbye world");
    OK
}
```

## Migration Guide

This change should be pretty much non-breaking, except for users who
have implemented their own custom executors. Those users should use
`ScheduleSystem` in place of `BoxedSystem<(), ()>` and import the
`System` trait where needed. They can choose to do whatever they wish
with the result.

## Current Work

+ [x] Fix tests & doc comments
+ [x] Write more tests
+ [x] Add examples
+ [X] Draft release notes

## Draft Release Notes

As of this release, systems can now return results.

First a bit of background: Bevy has hisotrically expected systems to
return the empty type `()`. While this makes sense in the context of the
ecs, it's at odds with how error handling is typically done in rust:
returning `Result::Error` to indicate failure, and using the
short-circuiting `?` operator to propagate that error up the call stack
to where it can be properly handled. Users of functional languages will
tell you this is called "monadic error handling".

Not being able to return `Results` from systems left bevy users with a
quandry. They could add custom error handling logic to every system, or
manually pipe every system into an error handler, or perhaps sidestep
the issue with some combination of fallible assignents, logging, macros,
and early returns. Often, users would just litter their systems with
unwraps and possible panics.

While any one of these approaches might be fine for a particular user,
each of them has their own drawbacks, and none makes good use of the
language. Serious issues could also arrise when two different crates
used by the same project made different choices about error handling.

Now, by returning results, systems can defer error handling to the
application itself. It looks like this:

```rust
// Previous, handling internally
app.add_systems(my_system)
fn my_system(window: Query<&Window>) {
   let Ok(window) = query.get_single() else {
       return;
   };
   // ... do something to the window here
}

// Previous, handling externally
app.add_systems(my_system.pipe(my_error_handler))
fn my_system(window: Query<&Window>) -> Result<(), impl Error> {
   let window = query.get_single()?;
   // ... do something to the window here
   Ok(())
}

// Previous, panicking
app.add_systems(my_system)
fn my_system(window: Query<&Window>) {
   let window = query.single();
   // ... do something to the window here
}

// Now 
app.add_systems(my_system)
fn my_system(window: Query<&Window>) -> Result {
    let window = query.get_single()?;
    // ... do something to the window here
    Ok(())
}
```

There are currently some limitations. Systems must either return `()` or
`Result<(), Box<dyn Error + Send + Sync + 'static>>`, with no
in-between. Results are also ignored by default, and though implementing
a custom handler is possible, it involves writing your own custom ecs
executor (which is *not* recomended).

Systems should return errors when they cannot perform their normal
behavior. In turn, errors returned to the executor while running the
schedule will (eventually) be treated as unexpected. Users and library
authors should prefer to return errors for anything that disrupts the
normal expected behavior of a system, and should only handle expected
cases internally.

We have big plans for improving error handling further:
+ Allowing users to change the error handling logic of the default
executors.
+ Adding source tracking and optional backtraces to errors.
+ Possibly adding tracing-levels (Error/Warn/Info/Debug/Trace) to
errors.
+ Generally making the default error logging more helpful and
inteligent.
+ Adding monadic system combininators for fallible systems.
+ Possibly removing all panicking variants from our api.

---------

Co-authored-by: Zachary Harrold <zac@harrold.com.au>
2024-12-05 22:29:06 +00:00
Robin Gloster
e763b71591
picking: disable raycast backface culling for Mesh2d (#16657)
# Objective

- This fixes raycast picking with lyon
- reverse winding of 2D meshes currently results in them being rendered
but not pickable as the raycast passes through the backface and would
only hit "from below"

## Solution

- Disables backface culling for Mesh2d

## Testing

- Tested picking with bevy_prototype_lyon
- Could probably use testing with Mesh3d (should not be affected) and
SimplifiedMesh (no experience with that, could have the same issue if
used for 2D?)

---------

Co-authored-by: Aevyrie <aevyrie@gmail.com>
2024-12-05 21:22:29 +00:00
Patrick Walton
d3241c4f8d
Fix the texture_binding_array, specialized_mesh_pipeline, and custom_shader_instancing examples after the bindless change. (#16641)
The bindless PR (#16368) broke some examples:

* `specialized_mesh_pipeline` and `custom_shader_instancing` failed
because they expect to be able to render a mesh with no material, by
overriding enough of the render pipeline to be able to do so. This PR
fixes the issue by restoring the old behavior in which we extract meshes
even if they have no material.

* `texture_binding_array` broke because it doesn't implement
`AsBindGroup::unprepared_bind_group`. This was tricky to fix because
there's a very good reason why `texture_binding_array` doesn't implement
that method: there's no sensible way to do so with `wgpu`'s current
bindless API, due to its multiple levels of borrowed references. To fix
the example, I split `MaterialBindGroup` into
`MaterialBindlessBindGroup` and `MaterialNonBindlessBindGroup`, and
allow direct custom implementations of `AsBindGroup::as_bind_group` for
the latter type of bind groups. To opt in to the new behavior, return
the `AsBindGroupError::CreateBindGroupDirectly` error from your
`AsBindGroup::unprepared_bind_group` implementation, and Bevy will call
your custom `AsBindGroup::as_bind_group` method as before.

## Migration Guide

* Bevy will now unconditionally call
`AsBindGroup::unprepared_bind_group` for your materials, so you must no
longer panic in that function. Instead, return the new
`AsBindGroupError::CreateBindGroupDirectly` error, and Bevy will fall
back to calling `AsBindGroup::as_bind_group` as before.
2024-12-05 21:22:14 +00:00
Zachary Harrold
73c6479f65
Add no_std support to bevy_color (#16633)
# Objective

- Contributes to #15460

## Solution

- Added the following new features: 
  - `std` (default)
  - `alloc`
  - `encase` (default)
  - `libm`

## Testing

- Added to `compile-check-no-std` CI command

## Notes

- `ColorCurve` requires `alloc` due to how the underlying `EvenCore`
type works.
- `Srgba::to_hex` requires `alloc` to return a `String`.
- This was otherwise a _very_ simple change
2024-12-05 21:21:45 +00:00
Patrick Walton
8c2c07b1c8
Retain RenderMeshInstance and MeshInputUniform data from frame to frame. (#16385)
This commit moves the front end of the rendering pipeline to a retained
model when GPU preprocessing is in use (i.e. by default, except in
constrained environments). `RenderMeshInstance` and `MeshUniformData`
are stored from frame to frame and are updated only for the entities
that changed state. This was rather tricky and requires some careful
surgery to keep the data valid in the case of removals.

This patch is built on top of Bevy's change detection. Generally, this
worked, except that `ViewVisibility` isn't currently properly tracked.
Therefore, this commit adds proper change tracking for `ViewVisibility`.
Doing this required adding a new system that runs after all
`check_visibility` invocations, as no single `check_visibility`
invocation has enough global information to detect changes.

On the Bistro exterior scene, with all textures forced to opaque, this
patch improves steady-state `extract_meshes_for_gpu_building` from
93.8us to 34.5us and steady-state `collect_meshes_for_gpu_building` from
195.7us to 4.28us. Altogether this constitutes an improvement from 290us
to 38us, which is a 7.46x speedup.

![Screenshot 2024-11-13
143841](https://github.com/user-attachments/assets/40b1aacc-373d-4016-b7fd-b0284bc33de4)

![Screenshot 2024-11-13
143850](https://github.com/user-attachments/assets/53b401c3-7461-43b3-918b-cff89ea780d6)

This patch is only lightly tested and shouldn't land before 0.15 is
released anyway, so I'm releasing it as a draft.
2024-12-05 21:16:04 +00:00
Zachary Harrold
bf765e61b5
Add no_std support to bevy_reflect (#16256)
# Objective

- Contributes to #15460

## Solution

- Added `std` feature (enabled by default)

## Testing

- CI
- `cargo check -p bevy_reflect --no-default-features --target
"x86_64-unknown-none"`
- UEFI demo application runs with this branch of `bevy_reflect`,
allowing `derive(Reflect)`

## Notes

- The [`spin`](https://crates.io/crates/spin) crate has been included to
provide `RwLock` and `Once` (as an alternative to `OnceLock`) when the
`std` feature is not enabled. Another alternative may be more desirable,
please provide feedback if you have a strong opinion here!
- Certain items (`Box`, `String`, `ToString`) provided by `alloc` have
been added to `__macro_exports` as a way to avoid `alloc` vs `std`
namespacing. I'm personally quite annoyed that we can't rely on `alloc`
as a crate name in `std` environments within macros. I'd love an
alternative to my approach here, but I suspect it's the least-bad
option.
- I would've liked to have an `alloc` feature (for allocation-free
`bevy_reflect`), unfortunately, `erased_serde` unconditionally requires
access to `Box`. Maybe one day we could design around this, but for now
it just means `bevy_reflect` requires `alloc`.

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-12-05 21:15:21 +00:00
Miles Silberling-Cook
09b0b5df91
Window picking (#16103)
# Objective

On the web, it's common to attach observers to windows. As @viridia has
discovered, this can be quite a nice paradigm in bevy as well when
applied to observers. The changes here are intended to make this
possible.
+ Adds a new default picking back-end as part to the core picking plugin
(which can be disabled) that causes pointers on windows to treat the
window entity as the final hit, behind everything else. This means
clicking empty space now dispatches normal picking events to the window,
and is especially nice for drag-and-drop functionality.
+ Adds a new traversal type, specific to picking events, that causes
them to bubble up to the window entity after they reach the root of the
hierarchy.

## Solution

The window picking back-end is extremely simple, but the bubbling
changes are much more complex, since they require doing a different
traversal depending on the picking event.

To achieve this, `Traversal` has been made generic over an associated
sized data type `D`. Observer bounds have been changed such that
`Event::Traversal<D>` is required for `Trigger<D>`. A blanket
implementation has been added for `()` and `Parent` that preserves the
existing functionality. A new `PointerTraversal` traversal has been
implemented, with a blanket implementation for `Traversal<Pointer<E>>`.

It is still possible to use `Parent` as the traversal for any event,
because of the blanket implementation. It is now possible for users to
add other custom traversals, which read event data during traversal.

## Testing

I tested these changes locally on some picking UI prototypes I have been
playing with. I also tested them on the picking examples.

---------

Co-authored-by: Martín Maita <47983254+mnmaita@users.noreply.github.com>
2024-12-05 21:14:39 +00:00
Nuutti Kotivuori
76d610d465
Flush commands after every mutation in WorldEntityMut (#16219)
# Objective

- Currently adding observers spawns an entity which implicitly flushes
the command queue, which can cause undefined behaviour if the
`WorldEntityMut` is used after this
- The reason `WorldEntityMut` attempted to (unsuccessfully) avoid
flushing commands until finished was that such commands may move or
despawn the entity being referenced, invalidating the cached location.
- With the introduction of hooks and observers, this isn't sensible
anymore as running the commands generated by hooks immediately is
required to maintain correct ordering of operations and to not expose
the world in an inconsistent state
- Objective is to make command flushing deterministic and fix the
related issues
- Fixes #16212
- Fixes #14621 
- Fixes #16034

## Solution

- Allow `WorldEntityMut` to exist even when it refers to a despawned
entity by allowing `EntityLocation` to be marked invalid
- Add checks to all methods to panic if trying to access a despawned
entity
- Flush command queue after every operation that might trigger hooks or
observers
- Update entity location always after flushing command queue

## Testing

- Added test cases for currently broken behaviour
- Added test cases that flushes happen in all operations
- Added test cases to ensure hooks and commands are run exactly in
correct order when nested

---

Todo:

- [x] Write migration guide
- [x] Add tests that using `EntityWorldMut` on a despawned entity panics
- [x] Add tests that commands are flushed after every operation that is
supposed to flush them
- [x] Add tests that hooks, observers and their spawned commands are run
in the correct order when nested

---

## Migration Guide

Previously `EntityWorldMut` triggered command queue flushes in
unpredictable places, which could interfere with hooks and observers.
Now the command queue is flushed always immediately after any call in
`EntityWorldMut` that spawns or despawns an entity, or adds, removes or
replaces a component. This means hooks and observers will run their
commands in the correct order.

As a side effect, there is a possibility that a hook or observer could
despawn the entity that is being referred to by `EntityWorldMut`. This
could already currently happen if an observer was added while keeping an
`EntityWorldMut` referece and would cause unsound behaviour. If the
entity has been despawned, calling any methods which require the entity
location will panic. This matches the behaviour that `Commands` will
panic if called on an already despawned entity. In the extremely rare
case where taking a new `EntityWorldMut` reference or otherwise
restructuring the code so that this case does not happen is not
possible, there's a new `is_despawned` method that can be used to check
if the referred entity has been despawned.
2024-12-05 20:30:12 +00:00
Christian Hughes
f87b9fe20c
Turn apply_deferred into a ZST System (#16642)
# Objective

- Required by #16622 due to differing implementations of `System` by
`FunctionSystem` and `ExclusiveFunctionSystem`.
- Optimize the memory usage of instances of `apply_deferred` in system
schedules.

## Solution

By changing `apply_deferred` from being an ordinary system that ends up
as an `ExclusiveFunctionSystem`, and instead into a ZST struct that
implements `System` manually, we save ~320 bytes per instance of
`apply_deferred` in any schedule.

## Testing

- All current tests pass.

---

## Migration Guide

- If you were previously calling the special `apply_deferred` system via
`apply_deferred(world)`, don't.
2024-12-05 18:14:05 +00:00
vil'mo
67bd2b00e1
Expose SystemMeta's access field as part of public API (#16625)
# Objective

Outside of the `bevy_ecs` crate it's hard to implement `SystemParam`
trait on params that require access to the `World`, because `init_state`
expects user to extend access in `SystemMeta` and access-related fields
of `SystemMeta` are private.

## Solution

Expose those fields as a functions
2024-12-05 18:10:58 +00:00
Talin
ea33fc04ab
Add "bevy_input_focus" crate. (#15611)
# Objective

Define a framework for handling keyboard focus and bubbled keyboard
events, as discussed in #15374.

## Solution

Introduces a new crate, `bevy_input_focus`. This crate provides:

* A resource for tracking which entity has keyboard focus.
* Methods for getting and setting keyboard focus.
* Event definitions for triggering bubble-able keyboard input events to
the focused entity.
* A system for dispatching keyboard input events to the focused entity.

This crate does *not* provide any integration with UI widgets, or
provide functions for
tab navigation or gamepad-based focus navigation, as those are typically
application-specific.

## Testing

Most of the code has been copied from a different project, one that has
been well tested. However, most of what's in this module consists of
type definitions, with relatively small amounts of executable code. That
being said, I expect that there will be substantial bikeshedding on the
design, and I would prefer to hold off writing tests until after things
have settled.

I think that an example would be appropriate, however I'm waiting on a
few other pending changes to Bevy before doing so. In particular, I can
see a simple example with four buttons, with focus navigation between
them, and which can be triggered by the keyboard.

@alice-i-cecile
2024-12-05 18:08:31 +00:00
Zachary Harrold
a35811d088
Add Immutable Component Support (#16372)
# Objective

- Fixes #16208

## Solution

- Added an associated type to `Component`, `Mutability`, which flags
whether a component is mutable, or immutable. If `Mutability= Mutable`,
the component is mutable. If `Mutability= Immutable`, the component is
immutable.
- Updated `derive_component` to default to mutable unless an
`#[component(immutable)]` attribute is added.
- Updated `ReflectComponent` to check if a component is mutable and, if
not, panic when attempting to mutate.

## Testing

- CI
- `immutable_components` example.

---

## Showcase

Users can now mark a component as `#[component(immutable)]` to prevent
safe mutation of a component while it is attached to an entity:

```rust
#[derive(Component)]
#[component(immutable)]
struct Foo {
    // ...
}
```

This prevents creating an exclusive reference to the component while it
is attached to an entity. This is particularly powerful when combined
with component hooks, as you can now fully track a component's value,
ensuring whatever invariants you desire are upheld. Before this would be
done my making a component private, and manually creating a `QueryData`
implementation which only permitted read access.

<details>
  <summary>Using immutable components as an index</summary>
  
```rust
/// This is an example of a component like [`Name`](bevy::prelude::Name), but immutable.
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Component)]
#[component(
    immutable,
    on_insert = on_insert_name,
    on_replace = on_replace_name,
)]
pub struct Name(pub &'static str);

/// This index allows for O(1) lookups of an [`Entity`] by its [`Name`].
#[derive(Resource, Default)]
struct NameIndex {
    name_to_entity: HashMap<Name, Entity>,
}

impl NameIndex {
    fn get_entity(&self, name: &'static str) -> Option<Entity> {
        self.name_to_entity.get(&Name(name)).copied()
    }
}

fn on_insert_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) {
    let Some(&name) = world.entity(entity).get::<Name>() else {
        unreachable!()
    };
    let Some(mut index) = world.get_resource_mut::<NameIndex>() else {
        return;
    };

    index.name_to_entity.insert(name, entity);
}

fn on_replace_name(mut world: DeferredWorld<'_>, entity: Entity, _component: ComponentId) {
    let Some(&name) = world.entity(entity).get::<Name>() else {
        unreachable!()
    };
    let Some(mut index) = world.get_resource_mut::<NameIndex>() else {
        return;
    };

    index.name_to_entity.remove(&name);
}

// Setup our name index
world.init_resource::<NameIndex>();

// Spawn some entities!
let alyssa = world.spawn(Name("Alyssa")).id();
let javier = world.spawn(Name("Javier")).id();

// Check our index
let index = world.resource::<NameIndex>();

assert_eq!(index.get_entity("Alyssa"), Some(alyssa));
assert_eq!(index.get_entity("Javier"), Some(javier));

// Changing the name of an entity is also fully capture by our index
world.entity_mut(javier).insert(Name("Steven"));

// Javier changed their name to Steven
let steven = javier;

// Check our index
let index = world.resource::<NameIndex>();

assert_eq!(index.get_entity("Javier"), None);
assert_eq!(index.get_entity("Steven"), Some(steven));
```
  
</details>

Additionally, users can use `Component<Mutability = ...>` in trait
bounds to enforce that a component _is_ mutable or _is_ immutable. When
using `Component` as a trait bound without specifying `Mutability`, any
component is applicable. However, methods which only work on mutable or
immutable components are unavailable, since the compiler must be
pessimistic about the type.

## Migration Guide

- When implementing `Component` manually, you must now provide a type
for `Mutability`. The type `Mutable` provides equivalent behaviour to
earlier versions of `Component`:
```rust
impl Component for Foo {
    type Mutability = Mutable;
    // ...
}
```
- When working with generic components, you may need to specify that
your generic parameter implements `Component<Mutability = Mutable>`
rather than `Component` if you require mutable access to said component.
- The entity entry API has had to have some changes made to minimise
friction when working with immutable components. Methods which
previously returned a `Mut<T>` will now typically return an
`OccupiedEntry<T>` instead, requiring you to add an `into_mut()` to get
the `Mut<T>` item again.

## Draft Release Notes

Components can now be made immutable while stored within the ECS.

Components are the fundamental unit of data within an ECS, and Bevy
provides a number of ways to work with them that align with Rust's rules
around ownership and borrowing. One part of this is hooks, which allow
for defining custom behavior at key points in a component's lifecycle,
such as addition and removal. However, there is currently no way to
respond to _mutation_ of a component using hooks. The reasons for this
are quite technical, but to summarize, their addition poses a
significant challenge to Bevy's core promises around performance.
Without mutation hooks, it's relatively trivial to modify a component in
such a way that breaks invariants it intends to uphold. For example, you
can use `core::mem::swap` to swap the components of two entities,
bypassing the insertion and removal hooks.

This means the only way to react to this modification is via change
detection in a system, which then begs the question of what happens
_between_ that alteration and the next run of that system?
Alternatively, you could make your component private to prevent
mutation, but now you need to provide commands and a custom `QueryData`
implementation to allow users to interact with your component at all.

Immutable components solve this problem by preventing the creation of an
exclusive reference to the component entirely. Without an exclusive
reference, the only way to modify an immutable component is via removal
or replacement, which is fully captured by component hooks. To make a
component immutable, simply add `#[component(immutable)]`:

```rust
#[derive(Component)]
#[component(immutable)]
struct Foo {
    // ...
}
```

When implementing `Component` manually, there is an associated type
`Mutability` which controls this behavior:

```rust
impl Component for Foo {
    type Mutability = Mutable;
    // ...
}
```

Note that this means when working with generic components, you may need
to specify that a component is mutable to gain access to certain
methods:

```rust
// Before
fn bar<C: Component>() {
    // ...
}

// After
fn bar<C: Component<Mutability = Mutable>>() {
    // ...
}
```

With this new tool, creating index components, or caching data on an
entity should be more user friendly, allowing libraries to provide APIs
relying on components and hooks to uphold their invariants.

## Notes

- ~~I've done my best to implement this feature, but I'm not happy with
how reflection has turned out. If any reflection SMEs know a way to
improve this situation I'd greatly appreciate it.~~ There is an
outstanding issue around the fallibility of mutable methods on
`ReflectComponent`, but the DX is largely unchanged from `main` now.
- I've attempted to prevent all safe mutable access to a component that
does not implement `Component<Mutability = Mutable>`, but there may
still be some methods I have missed. Please indicate so and I will
address them, as they are bugs.
- Unsafe is an escape hatch I am _not_ attempting to prevent. Whatever
you do with unsafe is between you and your compiler.
- I am marking this PR as ready, but I suspect it will undergo fairly
major revisions based on SME feedback.
- I've marked this PR as _Uncontroversial_ based on the feature, not the
implementation.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Co-authored-by: Nuutti Kotivuori <naked@iki.fi>
2024-12-05 14:27:48 +00:00
Patrick Walton
b7bcd313ca
Cluster light probes using conservative spherical bounds. (#13746)
This commit allows the Bevy renderer to use the clustering
infrastructure for light probes (reflection probes and irradiance
volumes) on platforms where at least 3 storage buffers are available. On
such platforms (the vast majority), we stop performing brute-force
searches of light probes for each fragment and instead only search the
light probes with bounding spheres that intersect the current cluster.
This should dramatically improve scalability of irradiance volumes and
reflection probes.

The primary platform that doesn't support 3 storage buffers is WebGL 2,
and we continue using a brute-force search of light probes on that
platform, as the UBO that stores per-cluster indices is too small to fit
the light probe counts. Note, however, that that platform also doesn't
support bindless textures (indeed, it would be very odd for a platform
to support bindless textures but not SSBOs), so we only support one of
each type of light probe per drawcall there in the first place.
Consequently, this isn't a performance problem, as the search will only
have one light probe to consider. (In fact, clustering would probably
end up being a performance loss.)

Known potential improvements include:

1. We currently cull based on a conservative bounding sphere test and
not based on the oriented bounding box (OBB) of the light probe. This is
improvable, but in the interests of simplicity, I opted to keep the
bounding sphere test for now. The OBB improvement can be a follow-up.

2. This patch doesn't change the fact that each fragment only takes a
single light probe into account. Typical light probe implementations
detect the case in which multiple light probes cover the current
fragment and perform some sort of weighted blend between them. As the
light probe fetch function presently returns only a single light probe,
implementing that feature would require more code restructuring, so I
left it out for now. It can be added as a follow-up.

3. Light probe implementations typically have a falloff range. Although
this is a wanted feature in Bevy, this particular commit also doesn't
implement that feature, as it's out of scope.

4. This commit doesn't raise the maximum number of light probes past its
current value of 8 for each type. This should be addressed later, but
would possibly require more bindings on platforms with storage buffers,
which would increase this patch's complexity. Even without raising the
limit, this patch should constitute a significant performance
improvement for scenes that get anywhere close to this limit. In the
interest of keeping this patch small, I opted to leave raising the limit
to a follow-up.

## Changelog

### Changed

* Light probes (reflection probes and irradiance volumes) are now
clustered on most platforms, improving performance when many light
probes are present.

---------

Co-authored-by: Benjamin Brienen <Benjamin.Brienen@outlook.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-12-05 13:07:10 +00:00
Tim
d2a07f9f72
Retained Gizmos (#15473)
# Objective
Add a way to use the gizmo API in a retained manner, for increased
performance.

## Solution
- Move gizmo API from `Gizmos` to `GizmoBuffer`, ~ab~using `Deref` to
keep usage the same as before.
- Merge non-strip and strip variant of `LineGizmo` into one, storing the
data in a `GizmoBuffer` to have the same API for retained `LineGizmo`s.

### Review guide
- The meat of the changes are in `lib.rs`, `retained.rs`, `gizmos.rs`,
`pipeline_3d.rs` and `pipeline_2d.rs`
- The other files contain almost exclusively the churn from moving the
gizmo API from `Gizmos` to `GizmoBuffer`

## Testing
### Performance

Performance compared to the immediate mode API is from 65 to 80 times
better for static lines.

```
7900 XTX, 3700X
1707.9k lines/ms: gizmos_retained (21.3ms)
3488.5k lines/ms: gizmos_retained_continuous_polyline (31.3ms)
   0.5k lines/ms: gizmos_retained_separate (97.7ms)

3054.9k lines/ms: bevy_polyline_retained_nan (16.8ms)
3596.3k lines/ms: bevy_polyline_retained_continuous_polyline (14.2ms)
   0.6k lines/ms: bevy_polyline_retained_separate (78.9ms)

  26.9k lines/ms: gizmos_immediate (14.9ms)
  43.8k lines/ms: gizmos_immediate_continuous_polyline (18.3ms)
```
Looks like performance is good enough, being close to par with
`bevy_polyline`.

Benchmarks can be found here: 
This branch:
https://github.com/tim-blackbird/line_racing/tree/retained-gizmos
Bevy 0.14: https://github.com/DGriffin91/line_racing

## Showcase
```rust 
fn setup(
    mut commands: Commands,
    mut gizmo_assets: ResMut<Assets<GizmoAsset>>
) {
    let mut gizmo = GizmoAsset::default();

    // A sphere made out of one million lines!
    gizmo
        .sphere(default(), 1., CRIMSON)
        .resolution(1_000_000 / 3);

    commands.spawn(Gizmo {
        handle: gizmo_assets.add(gizmo),
        ..default()
    });
}
```

## Follow-up work
- Port over to the retained rendering world proper
- Calculate visibility and cull `Gizmo`s
2024-12-04 21:21:06 +00:00
MevLyshkin
f59ae0f5e8
BrpQueryRow has field deserialization fix (#16613)
# Objective

BrpQueryRow doesn't serialize `has` field if it is empty. That is okay
until you try to deserialize it after. Then it will fail to deserialize
due to missing field.

## Solution

Serde support using default value when field is missing, this PR adds
that.
2024-12-04 18:26:33 +00:00
Marco Buono
7dfc77b999
Add missing registration for TextEntity (#16649)
# Objective

Fix a [Blenvy](https://github.com/kaosat-dev/Blenvy) crash due to a
missing type registration for `TextEntity` (as the type is used by
`ComputedTextBlock` but wasn't itself registered.)

## Solution

- Added the missing type registration

## Testing

- N/A
2024-12-04 18:16:48 +00:00
Patrick Walton
56c70f8463
Make visibility range (HLOD) dithering work when prepasses are enabled. (#16286)
Currently, the prepass has no support for visibility ranges, so
artifacts appear when using dithering visibility ranges in conjunction
with a prepass. This patch fixes that problem.

Note that this patch changes the prepass to use sparse bind group
indices instead of sequential ones. I figured this is cleaner, because
it allows for greater sharing of WGSL code between the forward pipeline
and the prepass pipeline.

The `visibility_range` example has been updated to allow the prepass to
be toggled on and off.
2024-12-04 17:34:36 +00:00
Zachary Harrold
c9fa975977
Remove petgraph from bevy_ecs (#15519)
# Objective

- Contributes to #15460

## Solution

- Removed `petgraph` as a dependency from the `bevy_ecs` crate.
- Replaced `TarjanScc` and `GraphMap` with specialised in-tree
alternatives.

## Testing

- Ran CI locally.
- Added new unit tests to check ordering invariants.
- Confirmed `petgraph` is no longer present in `cargo tree -p bevy_ecs`

## Migration Guide

The `Dag::graph` method no longer returns a `petgraph` `DiGraph` and
instead returns the new `DiGraph` type within `bevy_ecs`. Edge and node
iteration methods are provided so conversion to the `petgraph` type
should be trivial if required.

## Notes

- `indexmap` was already in the dependency graph for `bevy_ecs`, so its
inclusion here makes no difference to compilation time for Bevy.
- The implementation for `Graph` is heavily inspired from the `petgraph`
original, with specialisations added to simplify and improve the type.
- `petgraph` does have public plans for `no_std` support, however there
is no timeframe on if or when that functionality will be available.
Moving to an in-house solution in the interim allows Bevy to continue
developing its `no_std` offerings and further explore alternate graphing
options.

---------

Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com>
Co-authored-by: vero <11307157+atlv24@users.noreply.github.com>
2024-12-03 20:01:55 +00:00
SpecificProtagonist
410f3c478a
Use disqualified for B0001 (#16623)
# Objective

Fix #16553
2024-12-03 19:51:50 +00:00
atlv
e77efcc8d7
switch bevy_mesh to use wgpu-types instead of wgpu (#16619)
# Objective

- dont depend on wgpu if we dont have to

## Solution

- works towards this, but doesnt fully accomplish it. bevy_mesh depends
on bevy_image

## Testing

- 3d_scene runs

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: BD103 <59022059+BD103@users.noreply.github.com>
2024-12-03 19:49:49 +00:00
Phoqinu
c6147e57f4
Make StateTransitionSteps public (#16612)
# Objective

- Fixes #16594 

## Solution

- `StateTransitionSteps` was made public

## Testing

- _Did you test these changes? If so, how?_
 I am now able to import it and use it.

---

## Showcase

- Users can now write their own state scoped resources

```rust
    fn init_state_scoped_resource<R: Resource + FromWorld>(
        &mut self,
        state: impl States,
    ) -> &mut Self {
        use bevy::state::state::StateTransitionSteps; // this PR in action

        self.add_systems(
            StateTransition,
            (
                clear_state_scoped_resource_impl::<_, R>(state.clone())
                    .in_set(StateTransitionSteps::ExitSchedules), // and here
                init_state_scoped_resource_impl::<_, R>(state)
                    .in_set(StateTransitionSteps::EnterSchedules), // here too
            ),
        );

        self
    }
```
2024-12-03 19:48:00 +00:00
SpecificProtagonist
d92fc1e456
Move required components doc to type doc (#16575)
# Objective

Make documentation of a component's required components more visible by
moving it to the type's docs

## Solution

Change `#[require]` from a derive macro helper to an attribute macro.

Disadvantages:
- this silences any unused code warnings on the component, as it is used
by the macro!
- need to import `require` if not using the ecs prelude (I have not
included this in the migration guilde as Rust tooling already suggests
the fix)

---

## Showcase
![Documentation of
Camera](https://github.com/user-attachments/assets/3329511b-747a-4c8d-a43e-57f7c9c71a3c)

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
2024-12-03 19:45:20 +00:00
ickshonpe
dd49dc71d2
Rename UiBoxShadowSamples to BoxShadowSamples. (#16505)
# Objective

The `UiBoxShadowSamples` resource should be renamed to
`BoxShadowSamples` so it matches the `BoxShadow` component.

## Migration Guide

`UiBoxShadowSamples` has been renamed to `BoxShadowSamples`
2024-12-03 19:43:26 +00:00
SpecificProtagonist
1a6b94c5e8
Remove flush_and_reserve_invalid_assuming_no_entities (#16460)
# Objective

`flush_and_reserve_invalid_assuming_no_entities` was made for the old
rendering world (which was reset every frame) and is usused since the
0.15 retained rendering world, but wasn't removed yet. It is pub, but is
undocumented apart from the safety comment.

## Solution

Remove `flush_and_reserve_invalid_assuming_no_entities` and the safety
invariants this method required for `EntityMeta`, `EntityLocation`,
`TableId` and `TableRow`. This reduces the amount of unsafe code &
safety invariants and makes #16047 easier.

## Alternatives
- Document `flush_and_reserve_invalid_assuming_no_entities` and keep it
unchanged
- Document `flush_and_reserve_invalid_assuming_no_entities` and change
it to be based on `EntityMeta::INVALID`


## Migration Guide
- exchange `Entities::flush_and_reserve_invalid_assuming_no_entities`
for `reserve` and `flush_as_invalid` and notify us if that's
insufficient

---------

Co-authored-by: Benjamin Brienen <benjamin.brienen@outlook.com>
2024-12-03 19:42:22 +00:00
ickshonpe
343a52a789
Remove the min and max fields from LayoutContext. (#16459)
# Objective

Remove the `min` and `max` fields from `LayoutContext`. 
It doesn't seem useful to cache these values, it's simpler just to call
`min_element` and `max_element` on the `physical_size`
field.

##  Migration Guide

The `min` and `max` fields have been removed from `LayoutContext`. To
retrieve these values call `min_element` and `max_element` on
`LayoutContent::physical_size` instead.
2024-12-03 19:39:45 +00:00
Matty Weatherley
9d142a8025
Use IntoIterator instead of Into<Vec<..>> in cubic splines interfaces (#16402)
# Objective

This was always a bit weird; `IntoIterator` is considered more idiomatic
in Rust.

The reason these used `Into<Vec<..>>` in the first place was (to my
knowledge) because of concerns that passing an already-owned vector
would cause a redundant allocation if the iterator API was used instead.
However, I have looked at simple examples for this scenario and the
generated assembly is identical (i.e. `into_iter().collect()` is
effectively converted to a no-op).

## Solution

As described in the title.

## Testing

It compiles. Ran existing tests.

## Migration Guide

The cubic splines API now uses `IntoIterator` in places where it used
`Into<Vec<..>>`. For most users, this will have little to no effect (it
is largely more permissive). However, in case you were using some
unusual input type that implements `Into<Vec<..>>` without implementing
`IntoIterator`, you can migrate by converting the input to a `Vec<..>`
before passing it into the interface.
2024-12-03 19:35:14 +00:00
Michael Walter Van Der Velden
93dc596d2e
Add optional transparency passthrough for sprite backend with bevy_picking (#16388)
# Objective

- Allow bevy_sprite_picking backend to pass through transparent sections
of the sprite.
- Fixes #14929

## Solution

- After sprite picking detects the cursor is within a sprites rect,
check the pixel at that location on the texture and check that it meets
an optional transparency cutoff. Change originally created for
mod_picking on bevy 0.14
(https://github.com/aevyrie/bevy_mod_picking/pull/373)

## Testing

- Ran Sprite Picking example to check it was working both with
transparency enabled and disabled
- ModPicking version is currently in use in my own isometric game where
this has been an extremely noticeable issue

## Showcase

![Sprite Picking
Text](https://github.com/user-attachments/assets/76568c0d-c359-422b-942d-17c84d3d3009)

## Migration Guide

Sprite picking now ignores transparent regions (with an alpha value less
than or equal to 0.1). To configure this, modify the
`SpriteBackendSettings` resource.

---------

Co-authored-by: andriyDev <andriydzikh@gmail.com>
2024-12-03 19:32:52 +00:00
Patrick Walton
5adf831b42
Add a bindless mode to AsBindGroup. (#16368)
This patch adds the infrastructure necessary for Bevy to support
*bindless resources*, by adding a new `#[bindless]` attribute to
`AsBindGroup`.

Classically, only a single texture (or sampler, or buffer) can be
attached to each shader binding. This means that switching materials
requires breaking a batch and issuing a new drawcall, even if the mesh
is otherwise identical. This adds significant overhead not only in the
driver but also in `wgpu`, as switching bind groups increases the amount
of validation work that `wgpu` must do.

*Bindless resources* are the typical solution to this problem. Instead
of switching bindings between each texture, the renderer instead
supplies a large *array* of all textures in the scene up front, and the
material contains an index into that array. This pattern is repeated for
buffers and samplers as well. The renderer now no longer needs to switch
binding descriptor sets while drawing the scene.

Unfortunately, as things currently stand, this approach won't quite work
for Bevy. Two aspects of `wgpu` conspire to make this ideal approach
unacceptably slow:

1. In the DX12 backend, all binding arrays (bindless resources) must
have a constant size declared in the shader, and all textures in an
array must be bound to actual textures. Changing the size requires a
recompile.

2. Changing even one texture incurs revalidation of all textures, a
process that takes time that's linear in the total size of the binding
array.

This means that declaring a large array of textures big enough to
encompass the entire scene is presently unacceptably slow. For example,
if you declare 4096 textures, then `wgpu` will have to revalidate all
4096 textures if even a single one changes. This process can take
multiple frames.

To work around this problem, this PR groups bindless resources into
small *slabs* and maintains a free list for each. The size of each slab
for the bindless arrays associated with a material is specified via the
`#[bindless(N)]` attribute. For instance, consider the following
declaration:

```rust
#[derive(AsBindGroup)]
#[bindless(16)]
struct MyMaterial {
    #[buffer(0)]
    color: Vec4,
    #[texture(1)]
    #[sampler(2)]
    diffuse: Handle<Image>,
}
```

The `#[bindless(N)]` attribute specifies that, if bindless arrays are
supported on the current platform, each resource becomes a binding array
of N instances of that resource. So, for `MyMaterial` above, the `color`
attribute is exposed to the shader as `binding_array<vec4<f32>, 16>`,
the `diffuse` texture is exposed to the shader as
`binding_array<texture_2d<f32>, 16>`, and the `diffuse` sampler is
exposed to the shader as `binding_array<sampler, 16>`. Inside the
material's vertex and fragment shaders, the applicable index is
available via the `material_bind_group_slot` field of the `Mesh`
structure. So, for instance, you can access the current color like so:

```wgsl
// `uniform` binding arrays are a non-sequitur, so `uniform` is automatically promoted
// to `storage` in bindless mode.
@group(2) @binding(0) var<storage> material_color: binding_array<Color, 4>;
...
@fragment
fn fragment(in: VertexOutput) -> @location(0) vec4<f32> {
    let color = material_color[mesh[in.instance_index].material_bind_group_slot];
    ...
}
```

Note that portable shader code can't guarantee that the current platform
supports bindless textures. Indeed, bindless mode is only available in
Vulkan and DX12. The `BINDLESS` shader definition is available for your
use to determine whether you're on a bindless platform or not. Thus a
portable version of the shader above would look like:

```wgsl
#ifdef BINDLESS
@group(2) @binding(0) var<storage> material_color: binding_array<Color, 4>;
#else // BINDLESS
@group(2) @binding(0) var<uniform> material_color: Color;
#endif // BINDLESS
...
@fragment
fn fragment(in: VertexOutput) -> @location(0) vec4<f32> {
#ifdef BINDLESS
    let color = material_color[mesh[in.instance_index].material_bind_group_slot];
#else // BINDLESS
    let color = material_color;
#endif // BINDLESS
    ...
}
```

Importantly, this PR *doesn't* update `StandardMaterial` to be bindless.
So, for example, `scene_viewer` will currently not run any faster. I
intend to update `StandardMaterial` to use bindless mode in a follow-up
patch.

A new example, `shaders/shader_material_bindless`, has been added to
demonstrate how to use this new feature.

Here's a Tracy profile of `submit_graph_commands` of this patch and an
additional patch (not submitted yet) that makes `StandardMaterial` use
bindless. Red is those patches; yellow is `main`. The scene was Bistro
Exterior with a hack that forces all textures to opaque. You can see a
1.47x mean speedup.
![Screenshot 2024-11-12
161713](https://github.com/user-attachments/assets/4334b362-42c8-4d64-9cfb-6835f019b95c)

## Migration Guide

* `RenderAssets::prepare_asset` now takes an `AssetId` parameter.
* Bin keys now have Bevy-specific material bind group indices instead of
`wgpu` material bind group IDs, as part of the bindless change. Use the
new `MaterialBindGroupAllocator` to map from bind group index to bind
group ID.
2024-12-03 18:00:34 +00:00
Martín Maita
bfb2ff16e8
Update rodio requirement from 0.19 to 0.20 (#16359)
# Objective

- Supersedes #16344

## Solution

- Updated `rodio` version requirement to `0.20`.
-
[Changelog](https://github.com/RustAudio/rodio/blob/master/CHANGELOG.md)

## Testing

- CI checks.

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2024-12-03 17:55:42 +00:00
Benjamin Brienen
afd0f1322d
Move all_tuples to a new crate (#16161)
# Objective

Fixes #15941

## Solution

Created https://crates.io/crates/variadics_please and moved the code
there; updating references

`bevy_utils/macros` is deleted.

## Testing

cargo check

## Migration Guide

Use `variadics_please::{all_tuples, all_tuples_with_size}` instead of
`bevy::utils::{all_tuples, all_tuples_with_size}`.
2024-12-03 17:41:09 +00:00
eugineerd
2e267bba5a
Entity cloning (#16132)
## Objective

Fixes #1515 

This PR implements a flexible entity cloning system. The primary use
case for it is to clone dynamically-generated entities.

Example:
```rs
#[derive(Component, Clone)]
pub struct Projectile;

#[derive(Component, Clone)]
pub struct Damage {
    value: f32,
}

fn player_input(
    mut commands: Commands,
    projectiles: Query<Entity, With<Projectile>>,
    input: Res<ButtonInput<KeyCode>>,
) {
    // Fire a projectile
    if input.just_pressed(KeyCode::KeyF) {
        commands.spawn((Projectile, Damage { value: 10.0 }));
    }

    // Triplicate all active projectiles
    if input.just_pressed(KeyCode::KeyT) {
        for projectile in projectiles.iter() {
            // To triplicate a projectile we need to create 2 more clones
            for _ in 0..2{
                commands.clone_entity(projectile)
            }
        }
    }
}
```

## Solution

### Commands
Add a `clone_entity` command to create a clone of an entity with all
components that can be cloned. Components that can't be cloned will be
ignored.
```rs
commands.clone_entity(entity)
```
If there is a need to configure the cloning process (like set to clone
recursively), there is a second command:
```rs
commands.clone_entity_with(entity, |builder| {
    builder.recursive(true)
});
```
Both of these commands return `EntityCommands` of the cloned entity, so
the copy can be modified afterwards.

### Builder
All these commands use `EntityCloneBuilder` internally. If there is a
need to clone an entity using `World` instead, it is also possible:
```rs
let entity = world.spawn(Component).id();
let entity_clone = world.spawn_empty().id();
EntityCloneBuilder::new(&mut world).clone_entity(entity, entity_clone);
```

Builder has methods to `allow` or `deny` certain components during
cloning if required and can be extended by implementing traits on it.
This PR includes two `EntityCloneBuilder` extensions:
`CloneEntityWithObserversExt` to configure adding cloned entity to
observers of the original entity, and `CloneEntityRecursiveExt` to
configure cloning an entity recursively.

### Clone implementations
By default, all components that implement either `Clone` or `Reflect`
will be cloned (with `Clone`-based implementation preferred in case
component implements both).

This can be overriden on a per-component basis:
```rs
impl Component for SomeComponent {
    const STORAGE_TYPE: StorageType = StorageType::Table;

    fn get_component_clone_handler() -> ComponentCloneHandler {
        // Don't clone this component
        ComponentCloneHandler::Ignore
    }
}
```

### `ComponentCloneHandlers`
Clone implementation specified in `get_component_clone_handler` will get
registered in `ComponentCloneHandlers` (stored in
`bevy_ecs::component::Components`) at component registration time.

The clone handler implementation provided by a component can be
overriden after registration like so:
```rs
let component_id = world.components().component_id::<Component>().unwrap()
world.get_component_clone_handlers_mut()
     .set_component_handler(component_id, ComponentCloneHandler::Custom(component_clone_custom))
```
The default clone handler for all components that do not explicitly
define one (or don't derive `Component`) is
`component_clone_via_reflect` if `bevy_reflect` feature is enabled, and
`component_clone_ignore` (noop) otherwise.
Default handler can be overriden using
`ComponentCloneHandlers::set_default_handler`

### Handlers
Component clone handlers can be used to modify component cloning
behavior. The general signature for a handler that can be used in
`ComponentCloneHandler::Custom` is as follows:
```rs
pub fn component_clone_custom(
    world: &mut DeferredWorld,
    entity_cloner: &EntityCloner,
) {
    // implementation
}
```
The `EntityCloner` implementation (used internally by
`EntityCloneBuilder`) assumes that after calling this custom handler,
the `target` entity has the desired version of the component from the
`source` entity.

### Builder handler overrides
Besides component-defined and world-overriden handlers,
`EntityCloneBuilder` also has a way to override handlers locally. It is
mainly used to allow configuration methods like `recursive` and
`add_observers`.
```rs
// From observer clone handler implementation
impl CloneEntityWithObserversExt for EntityCloneBuilder<'_> {
    fn add_observers(&mut self, add_observers: bool) -> &mut Self {
        if add_observers {
            self.override_component_clone_handler::<ObservedBy>(ComponentCloneHandler::Custom(
                component_clone_observed_by,
            ))
        } else {
            self.remove_component_clone_handler_override::<ObservedBy>()
        }
    }
}
```

## Testing
Includes some basic functionality tests and doctests.

Performance-wise this feature is the same as calling `clone` followed by
`insert` for every entity component. There is also some inherent
overhead due to every component clone handler having to access component
data through `World`, but this can be reduced without breaking current
public API in a later PR.
2024-12-03 17:38:10 +00:00
JMS55
d221665386
Native unclipped depth on supported platforms (#16095)
# Objective
- Fixes #16078

## Solution

- Rename things to clarify that we _want_ unclipped depth for
directional light shadow views, and need some way of disabling the GPU's
builtin depth clipping
- Use DEPTH_CLIP_CONTROL instead of the fragment shader emulation on
supported platforms
- Pass only the clip position depth instead of the whole clip position
between vertex->fragment shader (no idea if this helps performance or
not, compiler might optimize it anyways)
- Meshlets
- HW raster always uses DEPTH_CLIP_CONTROL since it targets a more
limited set of platforms
- SW raster was not handling DEPTH_CLAMP_ORTHO correctly, it ended up
pretty much doing nothing.
- This PR made me realize that SW raster technically should have depth
clipping for all views that are not directional light shadows, but I
decided not to bother writing it. I'm not sure that it ever matters in
practice. If proven otherwise, I can add it.

## Testing

- Did you test these changes? If so, how?
- Lighting example. Both opaque (no fragment shader) and alpha masked
geometry (fragment shader emulation) are working with
depth_clip_control, and both work when it's turned off. Also tested
meshlet example.
- Are there any parts that need more testing?
  - Performance. I can't figure out a good test scene.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- Toggle depth_clip_control_supported in prepass/mod.rs line 323 to turn
this PR on or off.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - Native

---

## Migration Guide
- `MeshPipelineKey::DEPTH_CLAMP_ORTHO` is now
`MeshPipelineKey::UNCLIPPED_DEPTH_ORTHO`
- The `DEPTH_CLAMP_ORTHO` shaderdef has been renamed to
`UNCLIPPED_DEPTH_ORTHO_EMULATION`
- `clip_position_unclamped: vec4<f32>` is now `unclipped_depth: f32`
2024-12-03 17:30:14 +00:00
Shane Celis
f375422ddd
Compute better smooth normals for cheaper, maybe (#16050)
# Objective

Avoid a premature normalize operation and get better smooth normals for
it.

## Inspiration

@IceSentry suggested `face_normal()` could have its normalize removed
based on [this article](https://iquilezles.org/articles/normals/) in PR
#16039.

## Solution

I did not want to change `face_normal()` to return a vector that's not
normalized. The name "normal" implies it'll be normalized. Instead I
added the `face_area_normal()` function, whose result is not normalized.
Its magnitude is equal two times the triangle's area. I've noted why
this is the case in its doc comment.

I changed `compute_smooth_normals()` from computing normals from
adjacent faces with equal weight to use the area of the faces as a
weight. This has the benefit of being cheaper computationally and
hopefully produces better normals.

The `compute_flat_normals()` is unchanged and still uses
`face_normal()`.

## Testing

One test was added which shows the bigger triangle having an effect on
the normal, but the previous test that uses the same size triangles is
unchanged.

**WARNING:** No visual test has been done yet. No example exists that
demonstrates the compute_smooth_normals(). Perhaps there's a good model
to demonstrate what the differences are. I would love to have some input
on this.

I'd suggest @IceSentry and @stepancheg to review this PR.

## Further Considerations

It's possible weighting normals by their area is not definitely better
than unweighted. It's possible there may be aesthetic reasons to prefer
one over the other. In such a case, we could offer two variants:
weighted or unweighted. Or we could offer another function perhaps like
this: `compute_smooth_normals_with_weights(|normal, area| 1.0)` which
would restore the original unweighted sum of normals.

---

## Showcase

Smooth normal calculation now weights adjacent face normals by their
area.

## Migration Guide
2024-12-03 17:25:10 +00:00