Even though opaque deferred entities aren't placed into the `Opaque3d`
bin, we still want to cache them as though they were, so that we don't
have to re-queue them every frame. This commit implements that logic,
reducing the time of `queue_material_meshes` to near-zero on Caldera.
Currently, the structure-level `#[uniform]` attribute of `AsBindGroup`
creates a binding array of individual buffers, each of which contains
data for a single material. A more efficient approach would be to
provide a single buffer with an array containing all of the data for all
materials in the bind group. Because `StandardMaterial` uses
`#[uniform]`, this can be notably inefficient with large numbers of
materials.
This patch introduces a new attribute on `AsBindGroup`, `#[data]`, which
works identically to `#[uniform]` except that it concatenates all the
data into a single buffer that the material bind group allocator itself
manages. It also converts `StandardMaterial` to use this new
functionality. This effectively provides the "material data in arrays"
feature.
# Objective
Continuation of #17589 and #16547.
`get_many` is last of the `many` methods with a missing `unique`
counterpart.
It both takes and returns arrays, thus necessitates a matching
`UniqueEntityArray` type!
Plus, some slice methods involve returning arrays, which are currently
missing from `UniqueEntitySlice`.
## Solution
Add the type, the related methods and trait impls.
Note that for this PR, we abstain from some methods/trait impls that
create `&mut UniqueEntityArray`, because it can be successfully
mem-swapped. This can potentially invalidate a larger slice, which is
the same reason we punted on some mutable slice methods in #17589. We
can follow-up on all of these together in a following PR.
The new `unique_array` module is not glob-exported, because the trait
alias `unique_array::IntoIter` would conflict with
`unique_vec::IntoIter`.
The solution for this is to make the various `unique_*` modules public,
which I intend to do in yet another PR.
# Objective
Fixes#17945
## Solution
Check if the view being extracted has OIT enabled and incorporate the
associated bit into the mesh pipeline key.
I basically have no idea what's going on in the renderer, so let me know
if I missed something, which is extraordinarily possible.
## Testing
I modified the `order_independent_transparency` example to put
everything on the default render layer and render a gizmo at the origin.
Previously, this would cause the application to panic.
# Objective
- Fixes#17897.
## Solution
- When removing components, we filter the list of components in the
removed bundle based on whether they are actually in the archetype.
## Testing
- Added a test.
# Objective
Allow prepass to run without ATTRIBUTE_NORMAL.
This is needed for custom materials with non-standard vertex attributes.
For example a voxel material with manually packed vertex data.
Fixes#13054.
This PR covers the first part of the **stale** PR #13569 to only focus
on fixing #13054.
## Solution
- Only push normals `vertex_attributes` when the layout contains
`Mesh::ATTRIBUTE_NORMAL`
## Testing
- Did you test these changes? If so, how?
**Tested the fix on my own project with a mesh without normal
attribute.**
- Are there any parts that need more testing?
**I don't think so.**
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
**Prepass should not be blocked on a mesh without normal attributes
(with or without custom material).**
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
**Probably irrelevant, but Windows/Vulkan.**
# Objective
Implements and closes#17515
## Solution
Add `uv_transform` to `ColorMaterial`
## Testing
Create a example similar to `repeated_texture` but for `Mesh2d` and
`MeshMaterial2d<ColorMaterial>`
## Showcase
## Migration Guide
Add `uv_transform` field to constructors of `ColorMaterial`
# Objective
Fix https://github.com/bevyengine/bevy/issues/17108
See
https://github.com/bevyengine/bevy/issues/17108#issuecomment-2653020889
## Solution
- Make the query match `&Pickable` instead `Option<&Pickable>`
## Testing
- Run the `sprite_picking` example and everything still work
## Migration Guide
- Sprite picking are now opt-in, make sure you insert `Pickable`
component when using sprite picking.
```diff
-commands.spawn(Sprite { .. } );
+commands.spawn((Sprite { .. }, Pickable::default());
```
# Objective
Fixes#17761
## Solution
- Added core error to InvalidDirectionError
## Testing
- Did you test these changes? If so, how?
- An added test that pulls in anyhow as a dev dependency to ensure the
conversion is accounted for in creation via From
- Are there any parts that need more testing?
- I'm unsure but probably not due to being a trivial change
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- I did not try a fully built version of Bevy. Relied purely on tests.
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
- Only windows
---------
Co-authored-by: Chanceler Shaffer <cshaffer2@lululemon.com>
Co-authored-by: Chanceler Shaffer <chancelershaffer@lululemon.com>
# Objective
A `TransparentUI` phase's items all target the same camera so there is
no need to store the current camera entity in `UiBatch` and ending the
current `UiBatch` on camera changes is pointless as the camera doesn't
change.
## Solution
Remove the `camera` fields from `UiBatch`, `UiShadowsBatch` and
`UiTextureSliceBatch`.
Remove the camera changed check from `prepare_uinodes`.
## Testing
The `multiple_windows` and `split_screen` examples both render UI
elements to multiple cameras and can be used to test these changes.
The UI material plugin already didn't store the camera entity per batch
and worked fine without it.
# Objective
- Contributes to #15460
- Reduce quantity and complexity of feature gates across Bevy
## Solution
- Used `target_has_atomic` configuration variable to automatically
detect impartial atomic support and automatically switch to
`portable-atomic` over the standard library on an as-required basis.
## Testing
- CI
## Notes
To explain the technique employed here, consider getting `Arc` either
from `alloc::sync` _or_ `portable-atomic-util`. First, we can inspect
the `alloc` crate to see that you only have access to `Arc` _if_
`target_has_atomic = "ptr"`. We add a target dependency for this
particular configuration _inverted_:
```toml
[target.'cfg(not(target_has_atomic = "ptr"))'.dependencies]
portable-atomic-util = { version = "0.2.4", default-features = false }
```
This ensures we only have the dependency when it is needed, and it is
entirely excluded from the dependency graph when it is not. Next, we
adjust our configuration flags to instead of checking for `feature =
"portable-atomic"` to instead check for `target_has_atomic = "ptr"`:
```rust
// `alloc` feature flag hidden for brevity
#[cfg(not(target_has_atomic = "ptr"))]
use portable_atomic_util as arc;
#[cfg(target_has_atomic = "ptr")]
use alloc::sync as arc;
pub use arc::{Arc, Weak};
```
The benefits of this technique are three-fold:
1. For platforms without full atomic support, the functionality is
enabled automatically.
2. For platforms with atomic support, the dependency is never included,
even if a feature was enabled using `--all-features` (for example)
3. The `portable-atomic` feature no longer needs to virally spread to
all user-facing crates, it's instead something handled within
`bevy_platform_support` (with some extras where other dependencies also
need their features enabled).
# Objective
- The previous implementation of automatically inserting sync points did
not consider explicitly added sync points. This created additional sync
points. For example:
```
A-B
C-D-E
```
If `A` and `B` needed a sync point, and `D` was an `ApplyDeferred`, an
additional sync point would be generated between `A` and `B`.
```
A-D2-B
C-D -E
```
This can result in the following system ordering:
```
A-D2-(B-C)-D-E
```
Where only `B` and `C` run in parallel. If we could reuse `D` as the
sync point, we would get the following ordering:
```
(A-C)-D-(B-E)
```
Now we have two more opportunities for parallelism!
## Solution
- In the first pass, we:
- Compute the number of sync points before each node
- This was already happening but now we consider `ApplyDeferred` nodes
as creating a sync point.
- Pick an arbitrary explicit `ApplyDeferred` node for each "sync point
index" that we can (some required sync points may be missing!)
- In the second pass, we:
- For each edge, if two nodes have a different number of sync points
before them then there must be a sync point between them.
- Look for an explicit `ApplyDeferred`. If one exists, use it as the
sync point.
- Otherwise, generate a new sync point.
I believe this should also gracefully handle changes to the
`ScheduleGraph`. Since automatically inserted sync points are inserted
as systems, they won't look any different to explicit sync points, so
they are also candidates for "reusing" sync points.
One thing this solution does not handle is "deduping" sync points. If
you add 10 sync points explicitly, there will be at least 10 sync
points. You could keep track of all the sync points at the same
"distance" and then hack apart the graph to dedup those, but that could
be a follow-up step (and it's more complicated since you have to worry
about transferring edges between nodes).
## Testing
- Added a test to test the feature.
- The existing tests from all our crates still pass.
## Showcase
- Automatically inserted sync points can now reuse explicitly inserted
`ApplyDeferred` systems! Previously, Bevy would add new sync points
between systems, ignoring the explicitly added sync points. This would
reduce parallelism of systems in some situations. Now, the parallelism
has been improved!
# Objective
- Closes#12944.
## Solution
- Load `R8G8B8` textures by transcoding to an rgba format since `wgpu`
does not support texture formats with 3 channels.
- Switch to erroring out instead of panicking on an invalid dds file.
---
## Changelog
### Added
- DDS Textures with the `R8G8B8` format are now supported. They require
an additional conversion step, so using `R8G8B8A8` or a similar format
is preferable for texture loading performance.
# 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>
# Objective
`Eq`/`PartialEq` are currently implemented for `MeshMaterial{2|3}d` only
through the derive macro. Since we don't have perfect derive yet, the
impls are only present for `M: Eq` and `M: PartialEq`. On the other
hand, I want to be able to compare material components for my toy
reactivity project.
## Solution
Switch to manual `Eq`/`PartialEq` impl.
## Testing
Boy I hope this didn't break anything!
# Objective
Fixes#17488
## Solution
The world update logic happened in the the `about_to_wait` winit window
callback, but this is is not correct as (1) the winit documentation
states that the callback should not be used for that purpose and (2) the
callback is not fired when the window is resized or being dragged.
However, that callback was used in #11245 to fix an iOS bug (which
caused the regression). The solution presented here is a workaround
until the event loop code can be re-written.
## Testing
I confirmed that the `eased_motion` example continued to be animated
when dragging or resizing the window.
https://github.com/user-attachments/assets/ffaf0abf-4cd7-479b-83e9-e1850aaf3513
- Remove references to the short-lived `CommandError` type.
- Add a sentence to the explanation of error handlers.
- Clean up spacing/linebreaks.
- Use `where` notation for command-related trait `impl`s to make the big
ones easier to parse.
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>
# Objective
- Make transform propagation faster.
## Solution
- Work sharing worker threads
- Parallel tree traversal excluding leaves
- Second cache friendly wide pass over all leaves
- 3-10x faster than main
## Testing
- Tracy
- Caldera hotel is showing 3-7x faster on my M4 Max. Timing for bevy's
existing transform system shifts wildly run to run, so I don't know that
I would advertise a particular number. But this implementation is faster
in a... statistically significant way.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: François Mockers <mockersf@gmail.com>
PR #17898 regressed this, causing much of #17970. This commit fixes the
issue by freeing and reallocating materials in the
`MaterialBindGroupAllocator` on change. Note that more efficiency is
possible, but I opted for the simple approach because (1) we should fix
this bug ASAP; (2) I'd like #17965 to land first, because that unlocks
the biggest potential optimization, which is not recreating the bind
group if it isn't necessary to do so.
We might not be able to prepare a material on the first frame we
encounter a mesh using it for various reasons, including that the
material hasn't been loaded yet or that preparing the material is
exceeding the per-frame cap on number of bytes to load. When this
happens, we currently try to find the material in the
`MaterialBindGroupAllocator`, fail, and then fall back to group 0, slot
0, the default `MaterialBindGroupId`, which is obviously incorrect.
Worse, we then fail to dirty the mesh and reextract it when we *do*
finish preparing the material, so the mesh will continue to be rendered
with an incorrect material.
This patch fixes both problems. In `collect_meshes_for_gpu_building`, if
we fail to find a mesh's material in the `MeshBindGroupAllocator`, then
we detect that case, bail out, and add it to a list,
`MeshesToReextractNextFrame`. On subsequent frames, we process all the
meshes in `MeshesToReextractNextFrame` as though they were changed. This
ensures both that we don't render a mesh if its material hasn't been
loaded and that we start rendering the mesh once its material does load.
This was first noticed in the intermittent Pixel Eagle failures in the
`testbed_3d` patch in #17898, although the problem has actually existed
for some time. I believe it just so happened that the changes to the
allocator in that PR caused the problem to appear more commonly than it
did before.
This patch fixes two bugs in the new non-bindless material allocator
that landed in PR #17898:
1. A debug assertion to prevent double frees had been flipped: we
checked to see whether the slot was empty before freeing, while we
should have checked to see whether the slot was full.
2. The non-bindless allocator returned `None` when querying a slab that
hadn't been prepared yet instead of returning a handle to that slab.
This resulted in a 1-frame delay when modifying materials. In the
`animated_material` example, this resulted in the meshes never showing
up at all, because that example changes every material every frame.
Together with #17979, this patch locally fixes the problems with
`animated_material` on macOS that were reported in #17970.
# Objective
Fixes#8615
## Solution
Bevy currently interprets 1x1 dds textures as 1-dimensional. I think it
might be more common for game engines to assume two dimensions in this
ambiguous case. [citation needed]
I reworked the dimension choosing logic to only use 1d if there's a
dimension > 1, and assume 2d otherwise. I kept the assumption that
compressed textures are probably 2d.
## Testing
Modified `sprite.rs` to use `Tex_0012_0.dds` from the linked issue.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fix#17924
## Solution
Use fully qualified syntax (`usize::from` rather than `.into()`).
## Testing
Ran a build for the platform specified in the issue.
---------
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
Fixes#17883
# Objective + Solution
When doing normal scene root entity despawns (which are notably now
recursive), do not despawn instanced entities that are no longer in the
hierarchy.
(I would not classify this as a bug, but rather a behavior change)
## Migration Guide
If you previously relied on scene entities no longer in the hierarchy
being despawned when the scene root is despawned , use
`SceneSpawner::despawn_instance()` instead.
# Objective
Calling `define_label!` in a `no_std` 3rd party crate currently requires
the user to import `Box` themselves due to a non-fully-specified
reference to `Box`.
## Solution
Add a fully specified path for `Box` in the one location necessary, to
match all of the other cases.
Two-phase occlusion culling can be helpful for shadow maps just as it
can for a prepass, in order to reduce vertex and alpha mask fragment
shading overhead. This patch implements occlusion culling for shadow
maps from directional lights, when the `OcclusionCulling` component is
present on the entities containing the lights. Shadow maps from point
lights are deferred to a follow-up patch. Much of this patch involves
expanding the hierarchical Z-buffer to cover shadow maps in addition to
standard view depth buffers.
The `scene_viewer` example has been updated to add `OcclusionCulling` to
the directional light that it creates.
This improved the performance of the rend3 sci-fi test scene when
enabling shadows.
Currently, Bevy's implementation of bindless resources is rather
unusual: every binding in an object that implements `AsBindGroup` (most
commonly, a material) becomes its own separate binding array in the
shader. This is inefficient for two reasons:
1. If multiple materials reference the same texture or other resource,
the reference to that resource will be duplicated many times. This
increases `wgpu` validation overhead.
2. It creates many unused binding array slots. This increases `wgpu` and
driver overhead and makes it easier to hit limits on APIs that `wgpu`
currently imposes tight resource limits on, like Metal.
This PR fixes these issues by switching Bevy to use the standard
approach in GPU-driven renderers, in which resources are de-duplicated
and passed as global arrays, one for each type of resource.
Along the way, this patch introduces per-platform resource limits and
bumps them from 16 resources per binding array to 64 resources per bind
group on Metal and 2048 resources per bind group on other platforms.
(Note that the number of resources per *binding array* isn't the same as
the number of resources per *bind group*; as it currently stands, if all
the PBR features are turned on, Bevy could pack as many as 496 resources
into a single slab.) The limits have been increased because `wgpu` now
has universal support for partially-bound binding arrays, which mean
that we no longer need to fill the binding arrays with fallback
resources on Direct3D 12. The `#[bindless(LIMIT)]` declaration when
deriving `AsBindGroup` can now simply be written `#[bindless]` in order
to have Bevy choose a default limit size for the current platform.
Custom limits are still available with the new
`#[bindless(limit(LIMIT))]` syntax: e.g. `#[bindless(limit(8))]`.
The material bind group allocator has been completely rewritten. Now
there are two allocators: one for bindless materials and one for
non-bindless materials. The new non-bindless material allocator simply
maintains a 1:1 mapping from material to bind group. The new bindless
material allocator maintains a list of slabs and allocates materials
into slabs on a first-fit basis. This unfortunately makes its
performance O(number of resources per object * number of slabs), but the
number of slabs is likely to be low, and it's planned to become even
lower in the future with `wgpu` improvements. Resources are
de-duplicated with in a slab and reference counted. So, for instance, if
multiple materials refer to the same texture, that texture will exist
only once in the appropriate binding array.
To support these new features, this patch adds the concept of a
*bindless descriptor* to the `AsBindGroup` trait. The bindless
descriptor allows the material bind group allocator to probe the layout
of the material, now that an array of `BindGroupLayoutEntry` records is
insufficient to describe the group. The `#[derive(AsBindGroup)]` has
been heavily modified to support the new features. The most important
user-facing change to that macro is that the struct-level `uniform`
attribute, `#[uniform(BINDING_NUMBER, StandardMaterial)]`, now reads
`#[uniform(BINDLESS_INDEX, MATERIAL_UNIFORM_TYPE,
binding_array(BINDING_NUMBER)]`, allowing the material to specify the
binding number for the binding array that holds the uniform data.
To make this patch simpler, I removed support for bindless
`ExtendedMaterial`s, as well as field-level bindless uniform and storage
buffers. I intend to add back support for these as a follow-up. Because
they aren't in any released Bevy version yet, I figured this was OK.
Finally, this patch updates `StandardMaterial` for the new bindless
changes. Generally, code throughout the PBR shaders that looked like
`base_color_texture[slot]` now looks like
`bindless_2d_textures[material_indices[slot].base_color_texture]`.
This patch fixes a system hang that I experienced on the [Caldera test]
when running with `caldera --random-materials --texture-count 100`. The
time per frame is around 19.75 ms, down from 154.2 ms in Bevy 0.14: a
7.8× speedup.
[Caldera test]: https://github.com/DGriffin91/bevy_caldera_scene
Deferred rendering currently doesn't support occlusion culling. This PR
implements it in a straightforward way, mirroring what we already do for
the non-deferred pipeline.
On the rend3 sci-fi base test scene, this resulted in roughly a 2×
speedup when applied on top of my other patches. For that scene, it was
useful to add another option, `--add-light`, which forces the addition
of a shadow-casting light, to the scene viewer, which I included in this
patch.
This commit restructures the multidrawable batch set builder for better
performance in various ways:
* The bin traversal is optimized to make the best use of the CPU cache.
* The inner loop that iterates over the bins, which is the hottest part
of `batch_and_prepare_binned_render_phase`, has been shrunk as small as
possible.
* Where possible, multiple elements are added to or reserved from GPU
buffers as a batch instead of one at a time.
* Methods that LLVM wasn't inlining have been marked `#[inline]` where
doing so would unlock optimizations.
This code has also been refactored to avoid duplication between the
logic for indexed and non-indexed meshes via the introduction of a
`MultidrawableBatchSetPreparer` object.
Together, this improved the `batch_and_prepare_binned_render_phase` time
on Caldera by approximately 2×.
Eventually, we should optimize the batchable-but-not-multidrawable and
unbatchable logic as well, but these meshes are much rarer, so in the
interests of keeping this patch relatively small I opted to leave those
to a follow-up.
# Objective
Fix incorrect mesh culling where objects (particularly directional
shadows) were being incorrectly culled during the early preprocessing
phase. The issue manifested specifically on Apple M1 GPUs but not on
newer devices like the M4. The bug was in the
`view_frustum_intersects_obb` function, where including the w component
(plane distance) in the dot product calculations led to false positive
culling results. This caused objects to be incorrectly culled before
shadow casting could begin.
## Issue Details
The problem of missing shadows is reproducible on Apple M1 GPUs as of
this commit (bisected):
```
00722b8d0 Make indirect drawing opt-out instead of opt-in, enabling multidraw by default. (#16757)
```
and as recent as this commit:
```
c818c9214 Add option to animate materials in many_cubes (#17927)
```
- The frustum culling calculation incorrectly included the w component
(plane distance) when transforming basis vectors
- The relative radius calculation should only consider directional
transformation (xyz), not positional information (w)
- This caused false positive culling specifically on M1 devices likely
due to different device-specific floating-point behavior
- When objects were incorrectly culled, `early_instance_count` never
incremented, leading to missing geometry in the shadow pass
## Testing
- Tested on M1 and M4 devices to verify the fix
- Verified shadows and geometry render correctly on both platforms
- Confirmed the solution matches the existing Rust implementation's
behavior for calculating the relative radius:
c818c92143/crates/bevy_render/src/primitives/mod.rs (L77-L87)
- The fix resolves a mathematical error in the frustum culling
calculation while maintaining correct culling behavior across all
platforms.
---
## Showcase
`c818c9214`
<img width="1284" alt="c818c9214"
src="https://github.com/user-attachments/assets/fe1c7ea9-b13d-422e-b12d-f1cd74475213"
/>
`mate-h/frustum-cull-fix`
<img width="1283" alt="frustum-cull-fix"
src="https://github.com/user-attachments/assets/8a9ccb2a-64b6-4d5e-a17d-ac4798da5b51"
/>
# Objective
Make checked vs unchecked shaders configurable
Fixes#17786
## Solution
Added `ValidateShaders` enum to `Shader` and added
`create_and_validate_shader_module` to `RenderDevice`
## Testing
I tested the shader examples locally and they all worked. I'd like to
write a few tests to verify but am unsure how to start.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
The `check_visibility` system currently follows this algorithm:
1. Store all meshes that were visible last frame in the
`PreviousVisibleMeshes` set.
2. Determine which meshes are visible. For each such visible mesh,
remove it from `PreviousVisibleMeshes`.
3. Mark all meshes that remain in `PreviousVisibleMeshes` as invisible.
This algorithm would be correct if the `check_visibility` were the only
system that marked meshes visible. However, it's not: the shadow-related
systems `check_dir_light_mesh_visibility` and
`check_point_light_mesh_visibility` can as well. This results in the
following sequence of events for meshes that are in a shadow map but
*not* visible from a camera:
A. `check_visibility` runs, finds that no camera contains these meshes,
and marks them hidden, which sets the changed flag.
B. `check_dir_light_mesh_visibility` and/or
`check_point_light_mesh_visibility` run, discover that these meshes
are visible in the shadow map, and marks them as visible, again
setting the `ViewVisibility` changed flag.
C. During the extraction phase, the mesh extraction system sees that
`ViewVisibility` is changed and re-extracts the mesh.
This is inefficient and results in needless work during rendering.
This patch fixes the issue in two ways:
* The `check_dir_light_mesh_visibility` and
`check_point_light_mesh_visibility` systems now remove meshes that they
discover from `PreviousVisibleMeshes`.
* Step (3) above has been moved from `check_visibility` to a separate
system, `mark_newly_hidden_entities_invisible`. This system runs after
all visibility-determining systems, ensuring that
`PreviousVisibleMeshes` contains only those meshes that truly became
invisible on this frame.
This fix dramatically improves the performance of [the Caldera
benchmark], when combined with several other patches I've submitted.
[the Caldera benchmark]:
https://github.com/DGriffin91/bevy_caldera_scene
PR #17688 broke motion vector computation, and therefore motion blur,
because it enabled retention of `MeshInputUniform`s, and
`MeshInputUniform`s contain the indices of the previous frame's
transform and the previous frame's skinned mesh joint matrices. On frame
N, if a `MeshInputUniform` is retained on GPU from the previous frame,
the `previous_input_index` and `previous_skin_index` would refer to the
indices for frame N - 2, not the index for frame N - 1.
This patch fixes the problems. It solves these issues in two different
ways, one for transforms and one for skins:
1. To fix transforms, this patch supplies the *frame index* to the
shader as part of the view uniforms, and specifies which frame index
each mesh's previous transform refers to. So, in the situation described
above, the frame index would be N, the previous frame index would be N -
1, and the `previous_input_frame_number` would be N - 2. The shader can
now detect this situation and infer that the mesh has been retained, and
can therefore conclude that the mesh's transform hasn't changed.
2. To fix skins, this patch replaces the explicit `previous_skin_index`
with an invariant that the index of the joints for the current frame and
the index of the joints for the previous frame are the same. This means
that the `MeshInputUniform` never has to be updated even if the skin is
animated. The downside is that we have to copy joint matrices from the
previous frame's buffer to the current frame's buffer in
`extract_skins`.
The rationale behind (2) is that we currently have no mechanism to
detect when joints that affect a skin have been updated, short of
comparing all the transforms and setting a flag for
`extract_meshes_for_gpu_building` to consume, which would regress
performance as we want `extract_skins` and
`extract_meshes_for_gpu_building` to be able to run in parallel.
To test this change, use `cargo run --example motion_blur`.
Currently, the specialized pipeline cache maps a (view entity, mesh
entity) tuple to the retained pipeline for that entity. This causes two
problems:
1. Using the view entity is incorrect, because the view entity isn't
stable from frame to frame.
2. Switching the view entity to a `RetainedViewEntity`, which is
necessary for correctness, significantly regresses performance of
`specialize_material_meshes` and `specialize_shadows` because of the
loss of the fast `EntityHash`.
This patch fixes both problems by switching to a *two-level* hash table.
The outer level of the table maps each `RetainedViewEntity` to an inner
table, which maps each `MainEntity` to its pipeline ID and change tick.
Because we loop over views first and, within that loop, loop over
entities visible from that view, we hoist the slow lookup of the view
entity out of the inner entity loop.
Additionally, this patch fixes a bug whereby pipeline IDs were leaked
when removing the view. We still have a problem with leaking pipeline
IDs for deleted entities, but that won't be fixed until the specialized
pipeline cache is retained.
This patch improves performance of the [Caldera benchmark] from 7.8×
faster than 0.14 to 9.0× faster than 0.14, when applied on top of the
global binding arrays PR, #17898.
[Caldera benchmark]: https://github.com/DGriffin91/bevy_caldera_scene
Currently, Bevy rebuilds the buffer containing all the transforms for
joints every frame, during the extraction phase. This is inefficient in
cases in which many skins are present in the scene and their joints
don't move, such as the Caldera test scene.
To address this problem, this commit switches skin extraction to use a
set of retained GPU buffers with allocations managed by the offset
allocator. I use fine-grained change detection in order to determine
which skins need updating. Note that the granularity is on the level of
an entire skin, not individual joints. Using the change detection at
that level would yield poor performance in common cases in which an
entire skin is animated at once. Also, this patch yields additional
performance from the fact that changing joint transforms no longer
requires the skinned mesh to be re-extracted.
Note that this optimization can be a double-edged sword. In
`many_foxes`, fine-grained change detection regressed the performance of
`extract_skins` by 3.4x. This is because every joint is updated every
frame in that example, so change detection is pointless and is pure
overhead. Because the `many_foxes` workload is actually representative
of animated scenes, this patch includes a heuristic that disables
fine-grained change detection if the number of transformed entities in
the frame exceeds a certain fraction of the total number of joints.
Currently, this threshold is set to 25%. Note that this is a crude
heuristic, because it doesn't distinguish between the number of
transformed *joints* and the number of transformed *entities*; however,
it should be good enough to yield the optimum code path most of the
time.
Finally, this patch fixes a bug whereby skinned meshes are actually
being incorrectly retained if the buffer offsets of the joints of those
skinned meshes changes from frame to frame. To fix this without
retaining skins, we would have to re-extract every skinned mesh every
frame. Doing this was a significant regression on Caldera. With this PR,
by contrast, mesh joints stay at the same buffer offset, so we don't
have to update the `MeshInputUniform` containing the buffer offset every
frame. This also makes PR #17717 easier to implement, because that PR
uses the buffer offset from the previous frame, and the logic for
calculating that is simplified if the previous frame's buffer offset is
guaranteed to be identical to that of the current frame.
On Caldera, this patch reduces the time spent in `extract_skins` from
1.79 ms to near zero. On `many_foxes`, this patch regresses the
performance of `extract_skins` by approximately 10%-25%, depending on
the number of foxes. This has only a small impact on frame rate.
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.
# Objective
- #17787 removed sweeping of binned render phases from 2D by accident
due to them not using the `BinnedRenderPhasePlugin`.
- Fixes#17885
## Solution
- Schedule `sweep_old_entities` in `QueueSweep` like
`BinnedRenderPhasePlugin` does, but for 2D where that plugin is not
used.
## Testing
Tested with the modified `shader_defs` example in #17885 .
Fixes#17290.
<details>
<summary>Compilation errors before fix</summary>
`cargo clippy --tests --all-features --package bevy_image`:
```rust
error[E0061]: this function takes 7 arguments but 6 arguments were supplied
--> crates/bevy_core_pipeline/src/tonemapping/mod.rs:451:5
|
451 | Image::from_buffer(
| ^^^^^^^^^^^^^^^^^^
...
454 | bytes,
| ----- argument #1 of type `std::string::String` is missing
|
note: associated function defined here
--> /Users/josiahnelson/Desktop/Programming/Rust/bevy/crates/bevy_image/src/image.rs:930:12
|
930 | pub fn from_buffer(
| ^^^^^^^^^^^
help: provide the argument
|
451 | Image::from_buffer(/* std::string::String */, bytes, image_type, CompressedImageFormats::NONE, false, image_sampler, RenderAssetUsages::RENDER_WORLD)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
`cargo clippy --tests --all-features --package bevy_gltf`:
```rust
error[E0560]: struct `bevy_pbr::StandardMaterial` has no field named `specular_channel`
--> crates/bevy_gltf/src/loader.rs:1343:13
|
1343 | specular_channel: specular.specular_channel,
| ^^^^^^^^^^^^^^^^ `bevy_pbr::StandardMaterial` does not have this field
|
= note: available fields are: `emissive_exposure_weight`, `diffuse_transmission`, `diffuse_transmission_channel`, `diffuse_transmission_texture`, `flip_normal_map_y` ... and 9 others
error[E0560]: struct `bevy_pbr::StandardMaterial` has no field named `specular_texture`
--> crates/bevy_gltf/src/loader.rs:1345:13
|
1345 | specular_texture: specular.specular_texture,
| ^^^^^^^^^^^^^^^^ `bevy_pbr::StandardMaterial` does not have this field
|
= note: available fields are: `emissive_exposure_weight`, `diffuse_transmission`, `diffuse_transmission_channel`, `diffuse_transmission_texture`, `flip_normal_map_y` ... and 9 others
error[E0560]: struct `bevy_pbr::StandardMaterial` has no field named `specular_tint_channel`
--> crates/bevy_gltf/src/loader.rs:1351:13
|
1351 | specular_tint_channel: specular.specular_color_channel,
| ^^^^^^^^^^^^^^^^^^^^^ `bevy_pbr::StandardMaterial` does not have this field
|
= note: available fields are: `emissive_exposure_weight`, `diffuse_transmission`, `diffuse_transmission_channel`, `diffuse_transmission_texture`, `flip_normal_map_y` ... and 9 others
error[E0560]: struct `bevy_pbr::StandardMaterial` has no field named `specular_tint_texture`
--> crates/bevy_gltf/src/loader.rs:1353:13
|
1353 | specular_tint_texture: specular.specular_color_texture,
| ^^^^^^^^^^^^^^^^^^^^^ `bevy_pbr::StandardMaterial` does not have this field
|
= note: available fields are: `emissive_exposure_weight`, `diffuse_transmission`, `diffuse_transmission_channel`, `diffuse_transmission_texture`, `flip_normal_map_y` ... and 9 others
```
</details>
# Objective
Fixes#17022
## Solution
Only enable `bevy_gltf/dds` if `bevy_gltf` is already enabled.
## Testing
Tested with empty project
```toml
[dependencies]
bevy = { version = "0.16.0-dev", path = "../bevy", default-features = false, features = [
"dds",
] }
```
### Before
```
cargo tree --depth 1 -i bevy_gltf
bevy_gltf v0.16.0-dev (/Users/robparrett/src/bevy/crates/bevy_gltf)
└── bevy_internal v0.16.0-dev (/Users/robparrett/src/bevy/crates/bevy_internal)
```
### After
```
cargo tree --depth 1 -i bevy_gltf
warning: nothing to print.
To find dependencies that require specific target platforms, try to use option `--target all` first, and then narrow your search scope accordingly.
```
# Context
Renaming `Parent` to `ChildOf` in #17247 has been contentious. While
those users concerns are valid (especially around legibility of code
IMO!), @cart [has
decided](https://discord.com/channels/691052431525675048/749335865876021248/1340434322833932430)
to stick with the new name.
> In general this conversation is unsurprising to me, as it played out
essentially the same way when I asked for opinions in my PR. There are
strong opinions on both sides. Everyone is right in their own way.
>
> I chose ChildOf for the following reasons:
>
> 1. I think it derives naturally from the system we have built, the
concepts we have chosen, and how we generally name the types that
implement a trait in Rust. This is the name of the type implementing
Relationship. We are adding that Relationship component to a given
entity (whether it "is" the relationship or "has" the relationship is
kind of immaterial ... we are naming the relationship that it "is" or
"has"). What is the name of the relationship that a child has to its
parent? It is a "child" of the parent of course!
> 2. In general the non-parent/child relationships I've seen in the wild
generally benefit from (or need to) use the naming convention in (1)
(aka calling the Relationship the name of the relationship the entity
has). Many relationships don't have an equivalent to the Parent/Child
name concept.
> 3. I do think we could get away with using (1) for pretty much
everything else and special casing Parent/Children. But by embracing the
naming convention, we help establish that this is in fact a pattern, and
we help prime people to think about these things in a consistent way.
Consistency and predictability is a generally desirable property. And
for something as divisive and polarizing as relationship naming, I think
drawing a hard line in the sand is to the benefit of the community as a
whole.
> 4. I believe the fact that we dont see as much of the XOf naming style
elsewhere is to our benefit. When people see things in that style, they
are primed to think of them as relationships (after some exposure to
Bevy and the ecosystem). I consider this a useful hint.
> 5. Most of the practical confusion from using ChildOf seems to be from
calling the value of the target field we read from the relationship
child_of. The name of the target field should be parent (we could even
consider renaming child_of.0 to child_of.parent for clarity). I suspect
that existing Bevy users renaming their existing code will feel the most
friction here, as this requires a reframing. Imo it is natural and
expected to receive pushback from these users hitting this case.
## Objective
The new documentation doesn't do a particularly good job at quickly
explaining the meaning of each component or how to work with them;
making a tricky migration more painful and slowing down new users as
they learn about some of the most fundamental types in Bevy.
## Solution
1. Clearly explain what each component does in the very first line,
assuming no background knowledge. This is the first relationships that
99% of users will encounter, so explaining that they are relationships
is unhelpful as an introduction.
2. Add doc aliases for the rejected `IsParent`/`IsChild`/`Parent` names,
to improve autocomplete and doc searching.
3. Do some assorted docs cleanup while we're here.
---------
Co-authored-by: Eagster <79881080+ElliottjPierce@users.noreply.github.com>
## Objective
There's no general error for when an entity doesn't exist, and some
methods are going to need one when they get Resultified. The closest
thing is `EntityFetchError`, but that error has a slightly more specific
purpose.
## Solution
- Added `EntityDoesNotExistError`.
- Contains `Entity` and `EntityDoesNotExistDetails`.
- Changed `EntityFetchError` and `QueryEntityError`:
- Changed `NoSuchEntity` variant to wrap `EntityDoesNotExistError` and
renamed the variant to `EntityDoesNotExist`.
- Renamed `EntityFetchError` to `EntityMutableFetchError` to make its
purpose clearer.
- Renamed `TryDespawnError` to `EntityDespawnError` to make it more
general.
- Changed `World::inspect_entity` to return `Result<[ok],
EntityDoesNotExistError>` instead of panicking.
- Changed `World::get_entity` and `WorldEntityFetch::fetch_ref` to
return `Result<[ok], EntityDoesNotExistError>` instead of `Result<[ok],
Entity>`.
- Changed `UnsafeWorldCell::get_entity` to return
`Result<UnsafeEntityCell, EntityDoesNotExistError>` instead of
`Option<UnsafeEntityCell>`.
## Migration Guide
- `World::inspect_entity` now returns `Result<impl Iterator<Item =
&ComponentInfo>, EntityDoesNotExistError>` instead of `impl
Iterator<Item = &ComponentInfo>`.
- `World::get_entity` now returns `EntityDoesNotExistError` as an error
instead of `Entity`. You can still access the entity's ID through the
error's `entity` field.
- `UnsafeWorldCell::get_entity` now returns `Result<UnsafeEntityCell,
EntityDoesNotExistError>` instead of `Option<UnsafeEntityCell>`.
Appending to these vectors is performance-critical in
`batch_and_prepare_binned_render_phase`, so `RawBufferVec`, which
doesn't have the overhead of `encase`, is more appropriate.
The `collect_buffers_for_phase` system tries to reuse these buffers, but
its efforts are stymied by the fact that
`clear_batched_gpu_instance_buffers` clears the containing hash table
and therefore frees the buffers. This patch makes
`clear_batched_gpu_instance_buffers` stop doing that so that the
allocations can be reused.
# Objective
Simplify the API surface by removing duplicated functionality between
`Query` and `QueryState`.
Reduce the amount of `unsafe` code required in `QueryState`.
This is a follow-up to #15858.
## Solution
Move implementations of `Query` methods from `QueryState` to `Query`.
Instead of the original methods being on `QueryState`, with `Query`
methods calling them by passing the individual parameters, the original
methods are now on `Query`, with `QueryState` methods calling them by
constructing a `Query`.
This also adds two `_inner` methods that were missed in #15858:
`iter_many_unique_inner` and `single_inner`.
One goal here is to be able to deprecate and eventually remove many of
the methods on `QueryState`, reducing the overall API surface. (I
expected to do that in this PR, but this change was large enough on its
own!) Now that the `QueryState` methods each consist of a simple
expression like `self.query(world).get_inner(entity)`, a future PR can
deprecate some or all of them with simple migration instructions.
The other goal is to reduce the amount of `unsafe` code. The current
implementation of a read-only method like `QueryState::get` directly
calls the `unsafe fn get_unchecked_manual` and needs to repeat the proof
that `&World` has enough access. With this change, `QueryState::get` is
entirely safe code, with the proof that `&World` has enough access done
by the `query()` method and shared across all read-only operations.
## Future Work
The next step will be to mark the `QueryState` methods as
`#[deprecated]` and migrate callers to the methods on `Query`.
# Objective
Support accessing resources using reflection when using
`FilteredResources` in a dynamic system. This is similar to how
components can be queried using reflection when using
`FilteredEntityRef|Mut`.
## Solution
Change `ReflectResource` from taking `&World` and `&mut World` to taking
`impl Into<FilteredResources>` and `impl Into<FilteredResourcesMut>`,
similar to how `ReflectComponent` takes `impl Into<FilteredEntityRef>`
and `impl Into<FilteredEntityMut>`. There are `From` impls that ensure
code passing `&World` and `&mut World` continues to work as before.
## Migration Guide
If you are manually creating a `ReflectComponentFns` struct, the
`reflect` function now takes `FilteredResources` instead `&World`, and
there is a new `reflect_mut` function that takes `FilteredResourcesMut`.
