Commit graph

1431 commits

Author SHA1 Message Date
Patrick Walton
d80b809ea1
Only use the AABB center for mesh visibility range testing if specified. (#16468)
PR #15164 made Bevy consider the center of the mesh to be the center of
the axis-aligned bounding box (AABB). Unfortunately, this breaks
crossfading in many cases. LODs may have different AABBs and so the
center of the AABB may differ for different LODs of the same mesh. The
crossfading, however, relies on all LODs having *precisely* the same
position.

To address this problem, this PR adds a new field, `use_aabb`, to
`VisibilityRange`, which makes the AABB center point behavior opt-in.

@BenjaminBrienen first noticed this issue when reviewing PR #16286. That
PR contains a video showing the effects of this regression on the
`visibility_range` example. This commit fixes that example.

## Migration Guide

* The `VisibilityRange` component now has an extra field, `use_aabb`.
Generally, you can safely set it to false.
2024-11-22 18:18:20 +00:00
Patrick Walton
9fed566cf7
Refresh RenderVisibilityRanges when a visibility range is removed from the scene. (#16467)
We have an early-out to avoid updating `RenderVisibilityRanges` when a
`VisibilityRange` component is *modified*, but not when one is
*removed*. This means that removing `VisibilityRange` from an entity
might not update the rendering.

This PR fixes the issue by adding a check for removed
`VisibilityRange`s.
2024-11-22 18:17:17 +00:00
Carter Anderson
deda3f2522
Fix detailed_trace! (#16452)
Alternative to #16450 

# Objective

detailed_trace! in its current form does not work  (and breaks CI)

## Solution

Fix detailed_trace by checking for the feature properly, adding it to
the correct crates, and removing it from the incorrect crates
2024-11-20 22:01:33 +00:00
atlv
701ccdec51
add docs to clip_from_view (#16373)
more docs
2024-11-18 00:33:37 +00:00
atlv
c6fe275b21
add docs to view uniform frustum field (#16369)
just some docs to save future me some clicking around
2024-11-18 00:33:24 +00:00
atlv
1cb5604a17
remove gcd impl from bevy_render (#16419)
# Objective

- bevy_render (poorly) implements gcd (which should be in bevy_math but
theres not enough justification to have it there either anyways cus its
just one usage)

## Solution

- hardcoded LUT replacement for the one usage

## Testing

- verified the alternative implementation of 4/gcd(4,x) agreed with
original for 0..200
2024-11-17 21:37:41 +00:00
JMS55
ed6508363e
Bind only the written parts of storage buffers. (#16405)
# Objective
- Fix part of #15920

## Solution

- Keep track of the last written amount of bytes, and bind only that
much of the buffer.

## Testing

- Did you test these changes? If so, how? No
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?

---

## Migration Guide

- Fixed a bug with StorageBuffer and DynamicStorageBuffer binding data
from the previous frame(s) due to caching GPU buffers between frames.
2024-11-17 09:11:26 +00:00
François Mockers
bce19c1012
remove ViewUniformOffset from inactive cameras (#16399)
# Objective

- Fixes #16285 
- Inactive camera are keeping the component `ViewUniformOffset` from
when they were active, still matching some queries trying to render to
them

## Solution

- Remove component `ViewUniformOffset` from cameras that are inactive

## Testing

- Ran example `render_primitives` and switched camera
2024-11-16 20:39:13 +00:00
ickshonpe
aab36f3951
UI anti-aliasing fix (#16181)
# Objective

UI Anti-aliasing is incorrectly implemented. It always uses an edge
radius of 0.25 logical pixels, and ignores the physical resolution. For
low dpi screens 0.25 is is too low and on higher dpi screens the
physical edge radius is much too large, resulting in visual artifacts.

## Solution

Multiply the distance by the scale factor in the `antialias` function so
that the edge radius stays constant in physical pixels.

## Testing

To see the problem really clearly run the button example with `UiScale`
set really high. With `UiScale(25.)` on main if you examine the button's
border you can see a thick gradient fading away from the edges:

<img width="127" alt="edgg"
src="https://github.com/user-attachments/assets/7c852030-c0e8-4aef-8d3e-768cb2464cab">

With this PR the edges are sharp and smooth at all scale factors: 

<img width="127" alt="edge"
src="https://github.com/user-attachments/assets/b3231140-1bbc-4a4f-a1d3-dde21f287988">
2024-11-13 21:41:02 +00:00
Marco Buono
ef23f465ce
Do not re-check visibility or re-render shadow maps for point and spot lights for each view (#15156)
# Objective

_If I understand it correctly_, we were checking mesh visibility, as
well as re-rendering point and spot light shadow maps for each view.
This makes it so that M views and N lights produce M x N complexity.
This PR aims to fix that, as well as introduce a stress test for this
specific scenario.

## Solution

- Keep track of what lights have already had mesh visibility calculated
and do not calculate it again;
- Reuse shadow depth textures and attachments across all views, and only
render shadow maps for the _first_ time a light is encountered on a
view;
- Directional lights remain unaltered, since their shadow map cascades
are view-dependent;
- Add a new `many_cameras_lights` stress test example to verify the
solution

## Showcase

110% speed up on the stress test
83% reduction of memory usage in stress test

### Before (5.35 FPS on stress test)
<img width="1392" alt="Screenshot 2024-09-11 at 12 25 57"
src="https://github.com/user-attachments/assets/136b0785-e9a4-44df-9a22-f99cc465e126">

### After (11.34 FPS on stress test)
<img width="1392" alt="Screenshot 2024-09-11 at 12 24 35"
src="https://github.com/user-attachments/assets/b8dd858f-5e19-467f-8344-2b46ca039630">


## Testing

- Did you test these changes? If so, how? 
- On my game project where I have two cameras, and many shadow casting
lights I managed to get pretty much double the FPS.
  - Also included a stress test, see the comparison above
- Are there any parts that need more testing?
- Yes, I would like help verifying that this fix is indeed correct, and
that we were really re-rendering the shadow maps by mistake and it's
indeed okay to not do that
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the `many_cameras_lights` example
- On the `main` branch, cherry pick the commit with the example (`git
cherry-pick --no-commit 1ed4ace01`) and run it
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
  - macOS

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-11-11 18:49:09 +00:00
Benjamin Brienen
40640fdf42
Don't reëxport bevy_image from bevy_render (#16163)
# Objective

Fixes #15940

## Solution

Remove the `pub use` and fix the compile errors.
Make `bevy_image` available as `bevy::image`.

## Testing

Feature Frenzy would be good here! Maybe I'll learn how to use it if I
have some time this weekend, or maybe a reviewer can use it.

## Migration Guide

Use `bevy_image` instead of `bevy_render::texture` items.

---------

Co-authored-by: chompaa <antony.m.3012@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
2024-11-10 06:54:38 +00:00
atlv
c29e67153b
Expose Pipeline Compilation Zero Initialize Workgroup Memory Option (#16301)
# Objective

- wgpu 0.20 made workgroup vars stop being zero-init by default. this
broke some applications (cough foresight cough) and now we workaround
it. wgpu exposes a compilation option that zero initializes workgroup
memory by default, but bevy does not expose it.

## Solution

- expose the compilation option wgpu gives us

## Testing

- ran examples: 3d_scene, compute_shader_game_of_life, gpu_readback,
lines, specialized_mesh_pipeline. they all work
- confirmed fix for our own problems

---

</details>

## Migration Guide

- add `zero_initialize_workgroup_memory: false,` to
`ComputePipelineDescriptor` or `RenderPipelineDescriptor` structs to
preserve 0.14 functionality, add `zero_initialize_workgroup_memory:
true,` to restore bevy 0.13 functionality.
2024-11-08 21:42:37 +00:00
Benjamin Brienen
1e1b6e5b6d
Make BinnedRenderPhase fields for accessing batchable and unbatchable entities public (#16142)
# Objective

Fixes #16080

## Solution

Make the fields and struct pub as per the suggested solution.

## Testing

None
2024-11-07 18:03:47 +00:00
charlotte
4b05d2f4d8
Upgrade to wgpu 23 (#15988)
Fixes https://github.com/bevyengine/bevy/issues/15893

---------

Co-authored-by: François Mockers <mockersf@gmail.com>
2024-11-05 21:18:48 +00:00
Joona Aalto
46566980a6
Fix and improve MSAA documentation (#16196)
# Objective

#14273 changed `Msaa` to be a component rather than a resource. However,
the documentation still says that it is a resource. This tripped me up
during migration to 0.15 until I looked at the type definition.

Additionally, the docs have some unnecessary repetition and some grammar
mistakes, and they don't link to camera documentation.

## Solution

Fix up the docs!
2024-10-31 21:34:04 +00:00
Thierry Berger
88d1692105
Derive same attributes as MainEntity for RenderEntity (#16191)
Spotted while working on updating bevy_egui.

Discord context:
https://discordapp.com/channels/691052431525675048/1301212128115687454/1301469954465464320
2024-10-31 14:56:27 +00:00
Eero Lehtinen
af279073aa
Fix linux nvidia + xwayland freeze at startup (#16123)
# Objective

- Fixes #16122

When the wayland feature is not enabled, xwayland is used on wayland.
Nvidia drivers are somewhat bugged on linux and return outdated surfaces
on xwayland for seemingly no reason. Oftentimes at startup we get into
an infine loop where the surface is permanently outdated and nothing (or
sometimes only the first frame) is drawn on the screen.

## Solution

After experimenting I found that we can safely call configure again and
the issue seems to resolve itsef. After this change I couldn't reproduce
the original issue after many tries. More testing is probably needed
though.

The main issue is that `get_current_texture` fails sometimes because the
surface remains outdated even after configuring. It would be better to
just properly handle and never panic when `get_current_texture` fails.
This way we always call configure when outdated and bail when getting
the swapchain fails instead of crashing. The number of special cases is
also reduced.

## Testing

I tested the example "rotation" manually by trying to move around.

It works with X11 and Xwayland and the non panicing code paths didn't
change so other platforms aren't affected.
2024-10-28 22:17:59 +00:00
Gilles Henaux
5d1d073c14
Fix AsBindGroupError display for InvalidSamplerType (#16079)
# Objective

- Display message for `AsBindGroupError::InvalidSamplerType` was not
correctly displaying the binding index

## Solution

- Simple typo fix

## Testing

- Tested locally
2024-10-28 22:10:52 +00:00
Tau Gärtli
a644ac73f7
More #[doc(fake_variadic)] goodness (#16108)
This PR adds `#[doc(fake_variadic)]` to that were previously not
supported by rustdoc.

Thanks to an [upstream
contribution](https://github.com/rust-lang/rust/pull/132115) by yours
truly, `#[doc(fake_variadic)]` is now supported on impls such as `impl
QueryData for AnyOf<(T, ...)>` 🎉
Requires the latest nightly compiler (2024-10-25) which is already
available on [docs.rs](https://docs.rs/about/builds).


![image](https://github.com/user-attachments/assets/68589c7e-f68f-44fb-9a7b-09d24ccf19c9)

![image](https://github.com/user-attachments/assets/f09d20d6-d89b-471b-9a81-4a72c8968178)

This means that the impl sections for `QueryData` and `QueryFilter` are
now nice and tidy 

---

I also added `fake_variadic` to some impls that use
`all_tuples_with_size`, however I'm not entirely happy because the docs
are slightly misleading now:


![image](https://github.com/user-attachments/assets/fac93d08-dc02-430f-9f34-c97456256c56)

Note that the docs say `IntoBindGroupLayoutEntryBuilderArray<1>` instead
of
`IntoBindGroupLayoutEntryBuilderArray<N>`.
2024-10-27 19:01:50 +00:00
Rob Parrett
30d84519a2
Use en-us locale for typos (#16037)
# Objective

Bevy seems to want to standardize on "American English" spellings. Not
sure if this is laid out anywhere in writing, but see also #15947.

While perusing the docs for `typos`, I noticed that it has a `locale`
config option and tried it out.

## Solution

Switch to `en-us` locale in the `typos` config and run `typos -w`

## Migration Guide

The following methods or fields have been renamed from `*dependants*` to
`*dependents*`.

- `ProcessorAssetInfo::dependants`
- `ProcessorAssetInfos::add_dependant`
- `ProcessorAssetInfos::non_existent_dependants`
- `AssetInfo::dependants_waiting_on_load`
- `AssetInfo::dependants_waiting_on_recursive_dep_load`
- `AssetInfos::loader_dependants`
- `AssetInfos::remove_dependants_and_labels`
2024-10-20 18:55:17 +00:00
Rafał Harabień
fe7f98f7f0
Fix deactivated camera still being used in render world (#15946)
# Objective

Switch to retained render world causes the extracted cameras in render
world to not be removed until camera in main world is despawned. When
extracting data from main world inactive cameras are skipped. Therefore
camera that was active and became inactive has a retained
`ExtractedCamera` component from previous frames (when it was active)
and is processed the same way as if it were active (there is no `active`
field on `ExtractedCamera`). This breakes switching between cameras in
`render_primitives` example.
Fixes #15822

## Solution

Fix it by removing `ExtractedCamera` and related components from
inactive cameras.
Note that despawning inactive camera seems to be bad option because they
are spawned using `SyncToRenderWorld` component.

## Testing

Switching camera in `render_primitives` example now works correctly.

---------

Co-authored-by: akimakinai <105044389+akimakinai@users.noreply.github.com>
2024-10-19 15:13:14 +00:00
Alice Cecile
2bd328220b
Improve API for scaling orthographic cameras (#15969)
# Objective

Fixes #15791.

As raised in #11022, scaling orthographic cameras is confusing! In Bevy
0.14, there were multiple completely redundant ways to do this, and no
clear guidance on which to use.

As a result, #15075 removed the `scale` field from
`OrthographicProjection` completely, solving the redundancy issue.

However, this resulted in an unintuitive API and a painful migration, as
discussed in #15791. Users simply want to change a single parameter to
zoom, rather than deal with the irrelevant details of how the camera is
being scaled.

## Solution

This PR reverts #15075, and takes an alternate, more nuanced approach to
the redundancy problem. `ScalingMode::WindowSize` was by far the biggest
offender. This was the default variant, and stored a float that was
*fully* redundant to setting `scale`.

All of the other variants contained meaningful semantic information and
had an intuitive scale. I could have made these unitless, storing an
aspect ratio, but this would have been a worse API and resulted in a
pointlessly painful migration.

In the course of this work I've also:

- improved the documentation to explain that you should just set `scale`
to zoom cameras
- swapped to named fields for all of the variants in `ScalingMode` for
more clarity about the parameter meanings
- substantially improved the `projection_zoom` example
- removed the footgunny `Mul` and `Div` impls for `ScalingMode`,
especially since these no longer have the intended effect on
`ScalingMode::WindowSize`.
- removed a rounding step because this is now redundant 🎉 

## Testing

I've tested these changes as part of my work in the `projection_zoom`
example, and things seem to work fine.

## Migration Guide

`ScalingMode` has been refactored for clarity, especially on how to zoom
orthographic cameras and their projections:

- `ScalingMode::WindowSize` no longer stores a float, and acts as if its
value was 1. Divide your camera's scale by any previous value to achieve
identical results.
- `ScalingMode::FixedVertical` and `FixedHorizontal` now use named
fields.

---------

Co-authored-by: MiniaczQ <xnetroidpl@gmail.com>
2024-10-17 17:50:06 +00:00
andristarr
7482a0d26d
aligning public apis of Time,Timer and Stopwatch (#15962)
Fixes #15834

## Migration Guide

The APIs of `Time`, `Timer` and `Stopwatch` have been cleaned up for
consistency with each other and the standard library's `Duration` type.
The following methods have been renamed:

- `Stowatch::paused` -> `Stopwatch::is_paused`
- `Time::elapsed_seconds` -> `Time::elasped_secs` (including `_f64` and
`_wrapped` variants)
2024-10-16 21:09:32 +00:00
akimakinai
c78886e649
Remove ExtractComponent::Out (#15926)
# Objective

- `C: ExtractComponent` inserts `C::Out` instead of `C`, so we need to
remove `C::Out`. cc #15904.

## Solution

- `C` -> `C::Out`

## Testing

- CAS has `<ContrastAdaptiveSharpening as ExtractComponent>::Out =
(DenoiseCas, CasUniform)`. Setting its strength to zero correctly
removes the effect after this change.
2024-10-15 23:42:35 +00:00
Joona Aalto
c1a4b82762
Revert default mesh materials (#15930)
# Objective

Closes #15799.

Many rendering people and maintainers are in favor of reverting default
mesh materials added in #15524, especially as the migration to required
component is already large and heavily breaking.

## Solution

Revert default mesh materials, and adjust docs accordingly.

- Remove `extract_default_materials`
- Remove `clear_material_instances`, and move the logic back into
`extract_mesh_materials`
- Remove `HasMaterial2d` and `HasMaterial3d`
- Change default material handles back to pink instead of white
- 2D uses `Color::srgb(1.0, 0.0, 1.0)`, while 3D uses `Color::srgb(1.0,
0.0, 0.5)`. Not sure if this is intended.

There is now no indication at all about missing materials for `Mesh2d`
and `Mesh3d`. Having a mesh without a material renders nothing.

## Testing

I ran `2d_shapes`, `mesh2d_manual`, and `3d_shapes`, with and without
mesh material components.
2024-10-15 19:47:40 +00:00
andriyDev
15440c189b
Move SUPPORTED_FILE_EXTENSIONS to ImageLoader and remove unsupported formats. (#15917)
# Objective

Fixes #15730.

## Solution

As part of #15586, we made a constant to store all the supported image
formats. However since the `ImageFormat` does actually include Hdr and
OpenExr, it also included the `"hdr"` and `"exr"` file extensions. These
are supported by separate loaders though: `HdrTextureLoader` and
`ExrTextureLoader`. This led to a warning about duplicate asset loaders.

Therefore, instead of having the constant for `ImageFormat`, I made the
constant just for `ImageLoader`. This lets us correctly remove `"hdr"`
and `"exr"` from the image formats supported by `ImageLoader`, returning
us to having a single asset loader for every image format.

Note: we could have just removed `hdr` and `exr` from
`ImageFormat::SUPPORTED_FILE_EXTENSIONS`, but this would be very
confusing. Then the list of `ImageFormat`s would not match the list of
supported formats!

## Testing

- I ran the `sprite` example and got no warning! I also replaced the
sprite in that example with an HDR file and everything worked as
expected.
2024-10-15 18:06:34 +00:00
charlotte
acbed6040e
Attempt to remove component from render world if not extracted. (#15904)
# Objective

Ensure that components that are conditionally extracted do not linger in
the render world when not extracted from the main world.

## Solution

If the `ExtractComponent` returns `None`, we'll remove the render world
component. I think this is the most sensible behavior here. In the
future if there really is a use case for keeping the previous render
component around, we could add a `Option<Self::Out>` parameter for the
previous render component to the method, or something similar. I think
that this follows the principle of least surprise here relative to what
`None` would suggest and the way that render nodes are typically
written. The alternative would be to add an `enabled` field to pretty
much every camera settings component, or duplicate the extraction
condition as #15856 does.

## Testing

`transmission` no longer crashes.

## Migration Guide

Components that implement `ExtractComponent` and return `None` will
cause the extracted component to be removed from the render world.
2024-10-15 04:21:53 +00:00
andriyDev
73f7fd0c12
Move ImageLoader and CompressedImageSaver to bevy_image. (#15812)
# Objective

This is a follow-up to #15650. While the core `Image` stuff moved from
`bevy_render` to `bevy_image`, the `ImageLoader` and the
`CompressedImageSaver` remained in `bevy_render`.

## Solution

I moved `ImageLoader` and `CompressedImageSaver` to `bevy_image` and
re-exported everything out from `bevy_render`. The second step isn't
strictly necessary, but `bevy_render` is already doing this for all the
other `bevy_image` types, so I kept it the same for consistency.

Unfortunately I had to give `ImageLoader` a constructor so I can keep
the `RenderDevice` stuff in `bevy_render`.

## Testing

It compiles!

## Migration Guide

- `ImageLoader` can no longer be initialized directly through
`init_asset_loader`. Now you must use
`app.register_asset_loader(ImageLoader::new(supported_compressed_formats))`
(check out the implementation of `bevy_render::ImagePlugin`). This only
affects you if you are initializing the loader manually and does not
affect users of `bevy_render::ImagePlugin`.

## Followup work

- We should be able to move most of the `ImagePlugin` to `bevy_image`.
This would likely require an `ImagePlugin` and a `RenderImagePlugin` or
something though.
2024-10-15 02:18:10 +00:00
Clar Fon
e79bc7811d
Fix *most* clippy lints (#15906)
# Objective

Another clippy-lint fix: the goal is so that `ci lints` actually
displays the problems that a contributor caused, and not a bunch of
existing stuff in the repo. (when run on nightly)

## Solution

This fixes all but the `clippy::needless_lifetimes` lint, which will
result in substantially more fixes and be in other PR(s). I also
explicitly allow `non_local_definitions` since it is [not working
correctly, but will be
fixed](https://github.com/rust-lang/rust/issues/131643).

A few things were manually fixed: for example, some places had an
explicitly defined `div_ceil` function that was used, which is no longer
needed since this function is stable on unsigned integers. Also, empty
lines in doc comments were handled individually.

## Testing

I ran `cargo clippy --workspace --all-targets --all-features --fix
--allow-staged` with the `clippy::needless_lifetimes` lint marked as
`allow` in `Cargo.toml` to avoid fixing that too. It now passes with all
but the listed lint.
2024-10-14 20:52:35 +00:00
Alice Cecile
a7e9330af9
Implement WorldQuery for MainWorld and RenderWorld components (#15745)
# Objective

#15320 is a particularly painful breaking change, and the new
`RenderEntity` in particular is very noisy, with a lot of `let entity =
entity.id()` spam.

## Solution

Implement `WorldQuery`, `QueryData` and `ReadOnlyQueryData` for
`RenderEntity` and `WorldEntity`.

These work the same as the `Entity` impls from a user-facing
perspective: they simply return an owned (copied) `Entity` identifier.
This dramatically reduces noise and eases migration.

Under the hood, these impls defer to the implementations for `&T` for
everything other than the "call .id() for the user" bit, as they involve
read-only access to component data. Doing it this way (as opposed to
implementing a custom fetch, as tried in the first commit) dramatically
reduces the maintenance risk of complex unsafe code outside of
`bevy_ecs`.

To make this easier (and encourage users to do this themselves!), I've
made `ReadFetch` and `WriteFetch` slightly more public: they're no
longer `doc(hidden)`. This is a good change, since trying to vendor the
logic is much worse than just deferring to the existing tested impls.

## Testing

I've run a handful of rendering examples (breakout, alien_cake_addict,
auto_exposure, fog_volumes, box_shadow) and nothing broke.

## Follow-up

We should lint for the uses of `&RenderEntity` and `&MainEntity` in
queries: this is just less nice for no reason.

---------

Co-authored-by: Trashtalk217 <trashtalk217@gmail.com>
2024-10-13 20:58:46 +00:00
NiseVoid
bdd0af6bfb
Deprecate SpatialBundle (#15830)
# Objective

- Required components replace bundles, but `SpatialBundle` is yet to be
deprecated

## Solution

- Deprecate `SpatialBundle`
- Insert `Transform` and `Visibility` instead in examples using it
- In `spawn` or `insert` inserting a default `Transform` or `Visibility`
with component already requiring either, remove those components from
the tuple

## Testing

- Did you test these changes? If so, how?
Yes, I ran the examples I changed and tests
- Are there any parts that need more testing?
The `gamepad_viewer` and and `custom_shader_instancing` examples don't
work as intended due to entirely unrelated code, didn't check main.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
Run examples, or just check that all spawned values are identical
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
Linux, wayland trough x11 (cause that's the default feature)

---

## Migration Guide

`SpatialBundle` is now deprecated, insert `Transform` and `Visibility`
instead which will automatically insert all other components that were
in the bundle. If you do not specify these values and any other
components in your `spawn`/`insert` call already requires either of
these components you can leave that one out.

before:
```rust
commands.spawn(SpatialBundle::default());
```

after:
```rust
commands.spawn((Transform::default(), Visibility::default());
```
2024-10-13 17:28:22 +00:00
Benjamin Brienen
37501e1c21
spirv_shader_passthrough must enable wgpu/spirv (#15873)
# Objective

Fixes #15515

## Solution

I went for the simplest solution because "format" in
`shader_format_spirv` didn't sound directly related.

## Testing

The command `cargo b -p bevy --no-default-features -F
spirv_shader_passthrough,x11` failed before, but works now.
2024-10-13 17:25:27 +00:00
charlotte
dd812b3e49
Type safe retained render world (#15756)
# Objective

In the Render World, there are a number of collections that are derived
from Main World entities and are used to drive rendering. The most
notable are:
- `VisibleEntities`, which is generated in the `check_visibility` system
and contains visible entities for a view.
- `ExtractedInstances`, which maps entity ids to asset ids.

In the old model, these collections were trivially kept in sync -- any
extracted phase item could look itself up because the render entity id
was guaranteed to always match the corresponding main world id.

After #15320, this became much more complicated, and was leading to a
number of subtle bugs in the Render World. The main rendering systems,
i.e. `queue_material_meshes` and `queue_material2d_meshes`, follow a
similar pattern:

```rust
for visible_entity in visible_entities.iter::<With<Mesh2d>>() {
    let Some(mesh_instance) = render_mesh_instances.get_mut(visible_entity) else {
        continue;
    };
            
    // Look some more stuff up and specialize the pipeline...
            
    let bin_key = Opaque2dBinKey {
        pipeline: pipeline_id,
        draw_function: draw_opaque_2d,
        asset_id: mesh_instance.mesh_asset_id.into(),
        material_bind_group_id: material_2d.get_bind_group_id().0,
    };
    opaque_phase.add(
        bin_key,
        *visible_entity,
        BinnedRenderPhaseType::mesh(mesh_instance.automatic_batching),
    );
}
```

In this case, `visible_entities` and `render_mesh_instances` are both
collections that are created and keyed by Main World entity ids, and so
this lookup happens to work by coincidence. However, there is a major
unintentional bug here: namely, because `visible_entities` is a
collection of Main World ids, the phase item being queued is created
with a Main World id rather than its correct Render World id.

This happens to not break mesh rendering because the render commands
used for drawing meshes do not access the `ItemQuery` parameter, but
demonstrates the confusion that is now possible: our UI phase items are
correctly being queued with Render World ids while our meshes aren't.

Additionally, this makes it very easy and error prone to use the wrong
entity id to look up things like assets. For example, if instead we
ignored visibility checks and queued our meshes via a query, we'd have
to be extra careful to use `&MainEntity` instead of the natural
`Entity`.

## Solution

Make all collections that are derived from Main World data use
`MainEntity` as their key, to ensure type safety and avoid accidentally
looking up data with the wrong entity id:

```rust
pub type MainEntityHashMap<V> = hashbrown::HashMap<MainEntity, V, EntityHash>;
```

Additionally, we make all `PhaseItem` be able to provide both their Main
and Render World ids, to allow render phase implementors maximum
flexibility as to what id should be used to look up data.

You can think of this like tracking at the type level whether something
in the Render World should use it's "primary key", i.e. entity id, or
needs to use a foreign key, i.e. `MainEntity`.

## Testing

##### TODO:

This will require extensive testing to make sure things didn't break!
Additionally, some extraction logic has become more complicated and
needs to be checked for regressions.

## Migration Guide

With the advent of the retained render world, collections that contain
references to `Entity` that are extracted into the render world have
been changed to contain `MainEntity` in order to prevent errors where a
render world entity id is used to look up an item by accident. Custom
rendering code may need to be changed to query for `&MainEntity` in
order to look up the correct item from such a collection. Additionally,
users who implement their own extraction logic for collections of main
world entity should strongly consider extracting into a different
collection that uses `MainEntity` as a key.

Additionally, render phases now require specifying both the `Entity` and
`MainEntity` for a given `PhaseItem`. Custom render phases should ensure
`MainEntity` is available when queuing a phase item.
2024-10-10 18:47:04 +00:00
Tim
e19c53ebbd
Remove Handle<T> trait implementations that are dependent on Component (#15749)
# Objective

- Another step towards #15716
- Remove trait implementations that are dependent on `Handle<T>` being a
`Component`

## Solution

- Remove unused `ExtractComponent` trait implementation for `Handle<T>`
- Remove unused `ExtractInstance` trait implementation for `AssetId`
- Although the `ExtractInstance` trait wasn't used, the `AssetId`s were
being stored inside of `ExtractedInstances` which has an
`ExtractInstance` trait bound on its contents.
I've upgraded the `RenderMaterialInstances` type alias to be its own
resource, identical to `ExtractedInstances<AssetId<M>>` to get around
that with minimal breakage.
## Testing

Tested `many_cubes`, rendering did not explode
2024-10-09 17:12:27 +00:00
Christian Hughes
219b5930f1
Rename App/World::observe to add_observer, EntityWorldMut::observe_entity to observe. (#15754)
# Objective

- Closes #15752

Calling the functions `App::observe` and `World::observe` doesn't make
sense because you're not "observing" the `App` or `World`, you're adding
an observer that listens for an event that occurs *within* the `World`.
We should rename them to better fit this.

## Solution

Renames:
- `App::observe` -> `App::add_observer`
- `World::observe` -> `World::add_observer`
- `Commands::observe` -> `Commands::add_observer`
- `EntityWorldMut::observe_entity` -> `EntityWorldMut::observe`

(Note this isn't a breaking change as the original rename was introduced
earlier this cycle.)

## Testing

Reusing current tests.
2024-10-09 15:39:29 +00:00
Zachary Harrold
8718adc74f
Remove thiserror from bevy_render (#15765)
# Objective

- Contributes to #15460

## Solution

- Removed `thiserror` from `bevy_render`
2024-10-09 14:26:28 +00:00
Shoyu Vanilla (Flint)
a89ae8e9d9
fix: Skip some rendering logics when the viewport width or height is zero (#15654)
# Objective

- Fixes #15285

## Solution

`winit` sends resized to zero events when the window is minimized only
on Windows OS(rust-windowing/winit#2015).
This makes updating window viewport size to `(0, 0)` and panicking when
calculating aspect ratio.

~~So, just skip these kinds of events - resizing to (0, 0) when the
window is minimized - on Windows OS~~

Idially, the camera extraction excludes the cameras whose target size
width or height is zero here;


25bfa80e60/crates/bevy_render/src/camera/camera.rs (L1060-L1074)

but it seems that winit event loop sends resize events after extraction
and before post update schedule, so they might panics before the
extraction filters them out.

Alternatively, it might be possible to change event loop evaluating
order or defer them to the right schedule but I'm afraid that it might
cause some breaking changes, so just skip rendering logics for such
windows and they will be all filtered out by the extractions on the next
frame and thereafter.

## Testing

Running the example in the original issue and minimizing causes panic,
or just running `tests/window/minimising.rs` with `cargo run --example
minimising` panics without this PR and doesn't panics with this PR.

I think that we should run it in CI on Windows OS btw
2024-10-08 22:52:19 +00:00
Antony
0837ade0fc
Ensure Bevy's rendering byte usage is little-endian (#15750)
# Objective

- Fixes (partially) #15701.

## Solution

- Use little-endian bytes over native-endian bytes where applicable.

## Testing

- Ran CI.

## Open Questions

- Should we config-gate these for big-endian targets? It looks like
there are [very few
targets](https://doc.rust-lang.org/nightly/rustc/platform-support.html)
that use big-endian.
2024-10-08 22:43:35 +00:00
Kristoffer Søholm
2d1b4939d2
Synchronize removed components with the render world (#15582)
# Objective

Fixes #15560
Fixes (most of) #15570

Currently a lot of examples (and presumably some user code) depend on
toggling certain render features by adding/removing a single component
to an entity, e.g. `SpotLight` to toggle a light. Because of the
retained render world this no longer works: Extract will add any new
components, but when it is removed the entity persists unchanged in the
render world.

## Solution

Add `SyncComponentPlugin<C: Component>` that registers
`SyncToRenderWorld` as a required component for `C`, and adds a
component hook that will clear all components from the render world
entity when `C` is removed. We add this plugin to
`ExtractComponentPlugin` which fixes most instances of the problem. For
custom extraction logic we can manually add `SyncComponentPlugin` for
that component.

We also rename `WorldSyncPlugin` to `SyncWorldPlugin` so we start a
naming convention like all the `Extract` plugins.

In this PR I also fixed a bunch of breakage related to the retained
render world, stemming from old code that assumed that `Entity` would be
the same in both worlds.

I found that using the `RenderEntity` wrapper instead of `Entity` in
data structures when referring to render world entities makes intent
much clearer, so I propose we make this an official pattern.

## Testing

Run examples like

```
cargo run --features pbr_multi_layer_material_textures --example clearcoat
cargo run --example volumetric_fog
```

and see that they work, and that toggles work correctly. But really we
should test every single example, as we might not even have caught all
the breakage yet.

---

## Migration Guide

The retained render world notes should be updated to explain this edge
case and `SyncComponentPlugin`

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Trashtalk217 <trashtalk217@gmail.com>
2024-10-08 22:23:17 +00:00
charlotte
b48f9e2a4b
Fix oit webgl (#15728)
The previous fixes were breaking pretty much everything on main due to
naga-oil complaining about the OIT shader not being loaded, since
apparently webgl is a default feature. This fix is a bit messier, but
properly warns the user and is probably what we should have gone for in
the first place.
2024-10-08 14:50:35 +00:00
Clar Fon
8adc9e9d6e
Feature-gate all image formats (#15586)
# Objective

Bevy supports feature gates for each format it supports, but several
formats that it loads via the `image` crate do not have feature gates.
Additionally, the QOI format is supported by the `image` crate and
wasn't available at all. This fixes that.

## Solution

The following feature gates are added:

* `avif`
* `ff` (Farbfeld)
* `gif`
* `ico`
* `qoi`
* `tiff`

None of these formats are enabled by default, despite the fact that all
these formats appeared to be enabled by default before. Since
`default-features` was disabled for the `image` crate, it's likely that
using any of these formats would have errored by default before this
change, although this probably needs additional testing.

## Testing

The changes seemed minimal enough that a compile test would be
sufficient.

## Migration guide

Image formats that previously weren't feature-gated are now
feature-gated, meaning they will have to be enabled if you use them:

* `avif`
* `ff` (Farbfeld)
* `gif`
* `ico`
* `tiff`

Additionally, the `qoi` feature has been added to support loading QOI
format images.

Previously, these formats appeared in the enum by default, but weren't
actually enabled via the `image` crate, potentially resulting in weird
bugs. Now, you should be able to add these features to your projects to
support them properly.
2024-10-07 16:37:45 +00:00
Trashtalk217
d1bd46d45e
Deprecate get_or_spawn (#15652)
# Objective

After merging retained rendering world #15320, we now have a good way of
creating a link between worlds (*HIYAA intensifies*). This means that
`get_or_spawn` is no longer necessary for that function. Entity should
be opaque as the warning above `get_or_spawn` says. This is also part of
#15459.

I'm deprecating `get_or_spawn_batch` in a different PR in order to keep
the PR small in size.

## Solution

Deprecate `get_or_spawn` and replace it with `get_entity` in most
contexts. If it's possible to query `&RenderEntity`, then the entity is
synced and `render_entity.id()` is initialized in the render world.

## Migration Guide

If you are given an `Entity` and you want to do something with it, use
`Commands.entity(...)` or `World.entity(...)`. If instead you want to
spawn something use `Commands.spawn(...)` or `World.spawn(...)`. If you
are not sure if an entity exists, you can always use `get_entity` and
match on the `Option<...>` that is returned.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
2024-10-07 16:08:22 +00:00
charlotte
037464800e
Use global clear color for camera driver node. (#15688)
When no cameras are configured, the `ClearColor` resource has no effect
on the default window.

Fixes
https://discord.com/channels/691052431525675048/866787577687310356/1292601838075379796


![image](https://github.com/user-attachments/assets/f42479c0-b239-4660-acd0-daa859b1f815)

![image](https://github.com/user-attachments/assets/4d625960-f105-4a29-91a3-44f4baadac30)
2024-10-07 15:59:51 +00:00
Christian Hughes
584d14808a
Allow World::entity family of functions to take multiple entities and get multiple references back (#15614)
# Objective

Following the pattern established in #15593, we can reduce the API
surface of `World` by providing a single function to grab both a
singular entity reference, or multiple entity references.

## Solution

The following functions can now also take multiple entity IDs and will
return multiple entity references back:
- `World::entity`
- `World::get_entity`
- `World::entity_mut`
- `World::get_entity_mut`
- `DeferredWorld::entity_mut`
- `DeferredWorld::get_entity_mut`

If you pass in X, you receive Y:
- give a single `Entity`, receive a single `EntityRef`/`EntityWorldMut`
(matches current behavior)
- give a `[Entity; N]`/`&[Entity; N]` (array), receive an equally-sized
`[EntityRef; N]`/`[EntityMut; N]`
- give a `&[Entity]` (slice), receive a
`Vec<EntityRef>`/`Vec<EntityMut>`
- give a `&EntityHashSet`, receive a
`EntityHashMap<EntityRef>`/`EntityHashMap<EntityMut>`

Note that `EntityWorldMut` is only returned in the single-entity case,
because having multiple at the same time would lead to UB. Also,
`DeferredWorld` receives an `EntityMut` in the single-entity case
because it does not allow structural access.

## Testing

- Added doc-tests on `World::entity`, `World::entity_mut`, and
`DeferredWorld::entity_mut`
- Added tests for aliased mutability and entity existence

---

## Showcase

<details>
  <summary>Click to view showcase</summary>

The APIs for fetching `EntityRef`s and `EntityMut`s from the `World`
have been unified.

```rust
// This code will be referred to by subsequent code blocks.
let world = World::new();
let e1 = world.spawn_empty().id();
let e2 = world.spawn_empty().id();
let e3 = world.spawn_empty().id();
```

Querying for a single entity remains mostly the same:

```rust
// 0.14
let eref: EntityRef = world.entity(e1);
let emut: EntityWorldMut = world.entity_mut(e1);
let eref: Option<EntityRef> = world.get_entity(e1);
let emut: Option<EntityWorldMut> = world.get_entity_mut(e1);

// 0.15
let eref: EntityRef = world.entity(e1);
let emut: EntityWorldMut = world.entity_mut(e1);
let eref: Result<EntityRef, Entity> = world.get_entity(e1);
let emut: Result<EntityWorldMut, Entity> = world.get_entity_mut(e1);
```

Querying for multiple entities with an array has changed:

```rust
// 0.14
let erefs: [EntityRef; 2] = world.many_entities([e1, e2]);
let emuts: [EntityMut; 2] = world.many_entities_mut([e1, e2]);
let erefs: Result<[EntityRef; 2], Entity> = world.get_many_entities([e1, e2]);
let emuts: Result<[EntityMut; 2], QueryEntityError> = world.get_many_entities_mut([e1, e2]);

// 0.15
let erefs: [EntityRef; 2] = world.entity([e1, e2]);
let emuts: [EntityMut; 2] = world.entity_mut([e1, e2]);
let erefs: Result<[EntityRef; 2], Entity> = world.get_entity([e1, e2]);
let emuts: Result<[EntityMut; 2], EntityFetchError> = world.get_entity_mut([e1, e2]);
```

Querying for multiple entities with a slice has changed:

```rust
let ids = vec![e1, e2, e3]);

// 0.14
let erefs: Result<Vec<EntityRef>, Entity> = world.get_many_entities_dynamic(&ids[..]);
let emuts: Result<Vec<EntityMut>, QueryEntityError> = world.get_many_entities_dynamic_mut(&ids[..]);

// 0.15
let erefs: Result<Vec<EntityRef>, Entity> = world.get_entity(&ids[..]);
let emuts: Result<Vec<EntityMut>, EntityFetchError> = world.get_entity_mut(&ids[..]);
let erefs: Vec<EntityRef> = world.entity(&ids[..]); // Newly possible!
let emuts: Vec<EntityMut> = world.entity_mut(&ids[..]); // Newly possible!
```

Querying for multiple entities with an `EntityHashSet` has changed:

```rust
let set = EntityHashSet::from_iter([e1, e2, e3]);

// 0.14
let emuts: Result<Vec<EntityMut>, QueryEntityError> = world.get_many_entities_from_set_mut(&set);

// 0.15
let emuts: Result<EntityHashMap<EntityMut>, EntityFetchError> = world.get_entity_mut(&set);
let erefs: Result<EntityHashMap<EntityRef>, EntityFetchError> = world.get_entity(&set); // Newly possible!
let emuts: EntityHashMap<EntityMut> = world.entity_mut(&set); // Newly possible!
let erefs: EntityHashMap<EntityRef> = world.entity(&set); // Newly possible!
```

</details>

## Migration Guide

- `World::get_entity` now returns `Result<_, Entity>` instead of
`Option<_>`.
- Use `world.get_entity(..).ok()` to return to the previous behavior.
- `World::get_entity_mut` and `DeferredWorld::get_entity_mut` now return
`Result<_, EntityFetchError>` instead of `Option<_>`.
- Use `world.get_entity_mut(..).ok()` to return to the previous
behavior.
- Type inference for `World::entity`, `World::entity_mut`,
`World::get_entity`, `World::get_entity_mut`,
`DeferredWorld::entity_mut`, and `DeferredWorld::get_entity_mut` has
changed, and might now require the input argument's type to be
explicitly written when inside closures.
- The following functions have been deprecated, and should be replaced
as such:
    - `World::many_entities` -> `World::entity::<[Entity; N]>`
    - `World::many_entities_mut` -> `World::entity_mut::<[Entity; N]>`
    - `World::get_many_entities` -> `World::get_entity::<[Entity; N]>`
- `World::get_many_entities_dynamic` -> `World::get_entity::<&[Entity]>`
- `World::get_many_entities_mut` -> `World::get_entity_mut::<[Entity;
N]>`
- The equivalent return type has changed from `Result<_,
QueryEntityError>` to `Result<_, EntityFetchError>`
- `World::get_many_entities_dynamic_mut` ->
`World::get_entity_mut::<&[Entity]>1
- The equivalent return type has changed from `Result<_,
QueryEntityError>` to `Result<_, EntityFetchError>`
- `World::get_many_entities_from_set_mut` ->
`World::get_entity_mut::<&EntityHashSet>`
- The equivalent return type has changed from `Result<Vec<EntityMut>,
QueryEntityError>` to `Result<EntityHashMap<EntityMut>,
EntityFetchError>`. If necessary, you can still convert the
`EntityHashMap` into a `Vec`.
2024-10-07 15:21:40 +00:00
Bude
6edb78a8c3
Inverse bevy_render bevy_winit dependency and move cursor to bevy_winit (#15649)
# Objective

- `bevy_render` should not depend on `bevy_winit`
- Fixes #15565

## Solution

- `bevy_render` no longer depends on `bevy_winit`
- The following is behind the `custom_cursor` feature
- Move custom cursor code from `bevy_render` to `bevy_winit` behind the
`custom_cursor` feature
- `bevy_winit` now depends on `bevy_render` (for `Image` and
`TextureFormat`)
- `bevy_winit` now depends on `bevy_asset` (for `Assets`, `Handle` and
`AssetId`)
  - `bevy_winit` now depends on `bytemuck` (already in tree)
- Custom cursor code in `bevy_winit` reworked to use `AssetId` (other
than that it is taken over 1:1)
- Rework `bevy_winit` custom cursor interface visibility now that the
logic is all contained in `bevy_winit`

## Testing

- I ran the screenshot and window_settings examples
- Tested on linux wayland so far

---

## Migration Guide

`CursorIcon` and `CustomCursor` previously provided by
`bevy::render::view::cursor` is now available from `bevy::winit`.
A new feature `custom_cursor` enables this functionality (default
feature).
2024-10-06 18:25:50 +00:00
vero
4a23dc4216
Split out bevy_mesh from bevy_render (#15666)
# Objective

- bevy_render is gargantuan

## Solution

- Split out bevy_mesh

## Testing

- Ran some examples, everything looks fine

## Migration Guide

`bevy_render::mesh::morph::inherit_weights` is now
`bevy_render::mesh::inherit_weights`

if you were using `Mesh::compute_aabb`, you will need to `use
bevy_render::mesh::MeshAabb;` now

---------

Co-authored-by: Joona Aalto <jondolf.dev@gmail.com>
2024-10-06 14:18:11 +00:00
Joona Aalto
25bfa80e60
Migrate cameras to required components (#15641)
# Objective

Yet another PR for migrating stuff to required components. This time,
cameras!

## Solution

As per the [selected
proposal](https://hackmd.io/tsYID4CGRiWxzsgawzxG_g#Combined-Proposal-1-Selected),
deprecate `Camera2dBundle` and `Camera3dBundle` in favor of `Camera2d`
and `Camera3d`.

Adding a `Camera` without `Camera2d` or `Camera3d` now logs a warning,
as suggested by Cart [on
Discord](https://discord.com/channels/691052431525675048/1264881140007702558/1291506402832945273).
I would personally like cameras to work a bit differently and be split
into a few more components, to avoid some footguns and confusing
semantics, but that is more controversial, and shouldn't block this core
migration.

## Testing

I ran a few 2D and 3D examples, and tried cameras with and without
render graphs.

---

## Migration Guide

`Camera2dBundle` and `Camera3dBundle` have been deprecated in favor of
`Camera2d` and `Camera3d`. Inserting them will now also insert the other
components required by them automatically.
2024-10-05 01:59:52 +00:00
m-edlund
ac9b0c848c
fix: corrected projection calculation of camera sub view (#15646)
# Objective

- Fixes #15600

## Solution

- The projection calculations did not use the aspect ratio of
`full_size`, this was amended

## Testing

- I created a test example on [this
fork](https://github.com/m-edlund/bevy/tree/bug/main/subViewProjectionBroken)
to allow testing with different aspect ratios and offsets
- The sub view is bound to a view port that can move across the screen.
The image in the moving sub view should "match" the full image exactly

## Showcase

Current version:


https://github.com/user-attachments/assets/17ad1213-d5ae-4181-89c1-81146edede7d

Fixed version:


https://github.com/user-attachments/assets/398e0927-e1dd-4880-897d-8157aa4398e6
2024-10-05 01:36:47 +00:00
vero
7eadc1d467
Zero Copy Mesh (#15569)
# Objective

- Another step towards #15558

## Solution

- Instead of allocating a Vec and then having wgpu copy it into a
staging buffer, write directly into the staging buffer.
- gets rid of another hidden copy, in `pad_to_alignment`.

future work:
- why is there a gcd implementation in here (and its subpar, use
binary_gcd. its in the hot path, run twice for every mesh, every frame i
think?) make it better and put it in bevy_math
- zero-copy custom mesh api to avoid having to write out a Mesh from a
custom rep

## Testing

- lighting and many_cubes run fine (and slightly faster. havent
benchmarked though)

---

## Showcase

- look ma... no copies

at least when RenderAssetUsage is GPU only :3

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
2024-10-04 21:24:44 +00:00
vero
8b0388c74a
Split off bevy_image from bevy_render (#15650)
# Objective

- bevy_render is gargantuan

## Solution

- Split off bevy_image

## Testing

- Ran some examples
2024-10-04 20:16:47 +00:00