Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't use bevy_pbr for base bevy_gizmos plugin #17574

Closed
wants to merge 164 commits into from

Conversation

rambip
Copy link

@rambip rambip commented Jan 28, 2025

Objective

This PR enables bevy_gizmos to be used without bevy_pbr, for user who want to create their custom mesh rendering logic.

It can also be useful for user who just want to use bevy for drawing lines (why not).

This work is part of a bigger effort to make the bevy rendering pipeline more modular. I would like to contribute an exemple to render custom meshes without bevy_pbr. Something like this

Solution

Now, bevy_pbr is an optional dependency, and used only to debug lights.

I query the ViewUniforms manually, instead of using bevy_pbr to get the heavy MeshViewLayout

Testing

I'm not used to testing with bevy at all, but I was able to use successfully in my project.
It might break for some different mesh pipelines, but I don't think so.


Showcase

image
So nice ...

Migration Guide

I don't think there is any breaking change

Remaining work

Before merging it, it would be useful to:

  • remove the bevy_sprite depedency too
  • move view.rs to bevy_render, so that it can be used in a more modular way.
  • include the most recent changes from 0.16

mockersf and others added 30 commits October 22, 2024 22:44
…evyengine#16060)

# Objective

- bevy_animation publication fails because of missed dependency
- bevy_animation depends on bevy_animation_derive which is published
after

## Solution

- Reorder crates bevy_animation and bevy_animation_derive
# Objective
1. Prevent weird glitches with stray pixels scattered around the scene

![image](https://github.com/user-attachments/assets/f12adb38-5996-4dc7-bea6-bd326b7317e1)
2. Prevent weird glitchy full-screen triangles that pop-up and destroy
perf (SW rasterizing huge triangles is slow)

![image](https://github.com/user-attachments/assets/d3705427-13a5-47bc-a54b-756f0409da0b)

## Solution
1. Use floating point math in the SW rasterizer bounding box calculation
to handle negative verticss, and add backface culling
2. Force hardware raster for clusters that clip the near plane, and let
the hardware rasterizer handle the clipping

I also adjusted the SW rasterizer threshold to < 64 pixels (little bit
better perf in my test scene, but still need to do a more comprehensive
test), and enabled backface culling for the hardware raster pipeline.

## Testing

- Did you test these changes? If so, how?
  - Yes, on an example scene. Issues no longer occur.
- Are there any parts that need more testing?
  - No.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the meshlet example.
# Objective

- I made a mistake in bevyengine#15902, specifically [this
diff](bevyengine@e2faedb)
-- the `point_light_count` variable is used for all point lights, not
just shadow mapped ones, so I cannot add `.min(max_texture_cubes)`
there. (Despite `spot_light_count` having `.min(..)`)

It may have broken code like this (where `index` is index of
`point_light` vec):


https://github.com/bevyengine/bevy/blob/9930df83ed42008f7eb2c02cc7350040f0250c2e/crates/bevy_pbr/src/render/light.rs#L848-L850

and also causes panic here:

https://github.com/bevyengine/bevy/blob/9930df83ed42008f7eb2c02cc7350040f0250c2e/crates/bevy_pbr/src/render/light.rs#L1173-L1174

## Solution

- Adds `.min(max_texture_cubes)` directly to the loop where texture
views for point lights are created.

## Testing

- `lighting` example (with the directional light removed; original
example doesn't crash as only 1 directional-or-spot light in total is
shadow-mapped on webgl) no longer crashes on webgl
Take a bunch more improvements from @zeux's nanite.cpp code.

* Use position-only vertices (discard other attributes) to determine
meshlet connectivity for grouping
* Rather than using the lock borders flag when simplifying meshlet
groups, provide the locked vertices ourselves. The lock borders flag
locks the entire border of the meshlet group, but really we only want to
lock the edges between meshlet groups - outwards facing edges are fine
to unlock. This gives a really significant increase to the DAG quality.
* Add back stuck meshlets (group has only a single meshlet,
simplification failed) to the simplification queue to allow them to get
used later on and have another attempt at simplifying
* Target 8 meshlets per group instead of 4 (second biggest improvement
after manual locks)
* Provide a seed to metis for deterministic meshlet building
* Misc other improvements

We can remove the usage of unsafe after the next upstream meshopt
release, but for now we need to use the ffi function directly. I'll do
another round of improvements later, mainly attribute-aware
simplification and using spatial weights for meshlet grouping.

Need to merge bevyengine#15846 first.
# Objective
- Make the meshlet fill cluster buffers pass slightly faster
- Address bevyengine#15920 for meshlets
- Added PreviousGlobalTransform as a required meshlet component to avoid
extra archetype moves, slightly alleviating
bevyengine#14681 for meshlets
- Enforce that MeshletPlugin::cluster_buffer_slots is not greater than
2^25 (glitches will occur otherwise). Technically this field controls
post-lod/culling cluster count, and the issue is on pre-lod/culling
cluster count, but it's still valid now, and in the future this will be
more true.

Needs to be merged after bevyengine#15846
and bevyengine#15886

## Solution

- Old pass dispatched a thread per cluster, and did a binary search over
the instances to find which instance the cluster belongs to, and what
meshlet index within the instance it is.
- New pass dispatches a workgroup per instance, and has the workgroup
loop over all meshlets in the instance in order to write out the cluster
data.
- Use a push constant instead of arrayLength to fix the linked bug
- Remap 1d->2d dispatch for software raster only if actually needed to
save on spawning excess workgroups

## Testing

- Did you test these changes? If so, how?
- Ran the meshlet example, and an example with 1041 instances of 32217
meshlets per instance. Profiled the second scene with nsight, went from
0.55ms -> 0.40ms. Small savings. We're pretty much VRAM bandwidth bound
at this point.
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
  - Run the meshlet example

## Changelog (non-meshlets)
- PreviousGlobalTransform now implements the Default trait
…re-export (bevyengine#16063)

# Objective

Fixes bevyengine#16006

## Solution

We currently re-export `cosmic_text`, which is seemingly motivated by
the desire to use `cosmic_text::FontSystem` in `bevy_text` public APIs
instead of our `CosmicFontSystem` resource wrapper type.

This change makes `bevy_text` a "true" abstraction over `cosmic_text`
(it in fact, was already built to be that way generally and has this one
"leak").

This allows us to remove the `cosmic_text` re-export, which helps clean
up the Rust Analyzer imports and generally makes this a "cleaner" API.
# Objective

1. Nodes with `Display::None` set are removed from the layout and have
no position or size. Outlines should not be drawn for a node with
`Display::None` set.
2. The outline and border colors are checked for transparency together.
If only one of the two is transparent, both will get queued.
3. The `node.is_empty()` check is insufficient to check if a border is
present since a non-zero sized node can have a zero width border.

## Solution

1. Add a check to `extract_uinode_borders` and ignore the node if
`Display::None` is set.
2. Filter the border and outline optional components by
`is_fully_transparent`.
3.  Check if all the border widths are zero instead.

## Testing

I added dark cyan outlines around the left and right sections in the
`display_and_visibility` example. If you run the example and set the
outermost node to `Display::None` on the right, then you'll see the that
the outline on the left disappears.
…yengine#16058)

# Objective

- Follow up on bevyengine#16044
- `extract_uinode_borders` uses `bevy_hierarchy` directly instead of
going through the traversal utilities, meaning it won't handle
`GhostNode`s properly.

## Solution

- Replaced the use of `bevy_hierarchy::Parent` with
`UIChildren::get_parent`

## Testing

- Ran the `overflow` example, clipping looks ok.

---

---------

Co-authored-by: Carter Anderson <[email protected]>
…tlas (bevyengine#16072)

# Objective

Fixes bevyengine#16064

## Solution

- Add TextureAtlas to `UiImage::texture_atlas`
- Add `TextureAtlas::from_atlas_image` for parity with `Sprite`
- Rename `UiImage::texture` to `UiImage::image` for parity with `Sprite`
- Port relevant implementations and uses
- Remove `derive(Component)` for `TextureAtlas`

---

## Migration Guide

Before:
```rust
commands.spawn((
  UiImage::new(image),
  TextureAtlas { index, layout },
));
```

After:
```rust
commands.spawn(UiImage::from_atlas_image(image, TextureAtlas { index, layout }));
```

Before:
```rust
commands.spawn(UiImage {
    texture: some_image,
    ..default()
})
```

After:
```rust
commands.spawn(UiImage {
    image: some_image,
    ..default()
})
```
…engine#16069)

The PCSS PR bevyengine#13497 increased the size of clusterable objects from 64
bytes to 80 bytes but didn't decrease the UBO size to compensate, so we
blew past the 16kB limit on WebGL 2. This commit fixes the issue by
lowering the maximum number of clusterable objects to 204, which puts us
under the 16kB limit again.

Closes bevyengine#15998.
… samplers. (bevyengine#16068)

The two additional linear texture samplers that PCSS added caused us to
blow past the limit on Apple Silicon macOS and WebGL. To fix the issue,
this commit adds a `--feature pbr_pcss` feature gate that disables PCSS
if not present.

Closes bevyengine#15345.
Closes bevyengine#15525.
Closes bevyengine#15821.

---------

Co-authored-by: Carter Anderson <[email protected]>
Co-authored-by: IceSentry <[email protected]>
…us Bevy version (bevyengine#16027)

# Objective

This PR introduces an `AsyncSeekForwardExt` trait, which I forgot in my
previous PR bevyengine#14194.

This new trait is analogous to `AsyncSeekExt` and allows all
implementors of `AsyncSeekForward` to directly use the `seek_forward`
function in async contexts.

## Solution

- Implement a new `AsyncSeekForwardExt` trait
- Automatically implement this trait for all types that implement
`AsyncSeekForward`

## Showcase

This new trait allows a similar API to the previous Bevy version:

```rust
#[derive(Default)]
struct UniverseLoader;

#[derive(Asset, TypePath, Debug)]
struct JustALilAsteroid([u8; 128]);

impl AssetLoader for UniverseLoader {
    type Asset = JustALilAsteroid;
    type Settings = ();
    type Error = std::io::Error;

    async fn load<'a>(
        &'a self,
        reader: &'a mut Reader<'a>,
        _settings: &'a Self::Settings,
        _context: &'a mut LoadContext<'_>,
    ) -> Result<Self::Asset, Self::Error> {
        // read the asteroids entry table
        let entry_offset: u64 = /* ... */;
        let current_offset: u64 = reader.seek_forward(0).await?;

        // jump to the entry
        reader.seek_forward(entry_offset - current_offset).await?;

        let mut asteroid_buf = [0; 128];
        reader.read_exact(&mut asteroid_buf).await?;

        Ok(JustALilAsteroid(asteroid_buf))
    }
    
    fn extensions(&self) -> &[&str] {
        &["celestial"]
    }
}
```
# Objective

- `MeshPickingBackend` and `SpritePickingBackend` do not have the
`Plugin` suffix
- `DefaultPickingPlugins` is masquerading as a `Plugin` when in reality
it should be a `PluginGroup`
- Fixes bevyengine#16081.

## Solution

- Rename some structures:

|Original Name|New Name|
|-|-|
|`MeshPickingBackend`|`MeshPickingPlugin`|
|`MeshPickingBackendSettings`|`MeshPickingSettings`|
|`SpritePickingBackend`|`SpritePickingPlugin`|
|`UiPickingBackendPlugin`|`UiPickingPlugin`|

- Make `DefaultPickingPlugins` a `PluginGroup`.
- Because `DefaultPickingPlugins` is within the `DefaultPlugins` plugin
group, I also added support for nested plugin groups to the
`plugin_group!` macro.

## Testing

- I used ripgrep to ensure all references were properly renamed.
- For the `plugin_group!` macro, I used `cargo expand` to manually
inspect the expansion of `DefaultPlugins`.

---

## Migration Guide

> [!NOTE]
>
> All 3 of the changed structures were added after 0.14, so this does
not need to be included in the 0.14 to 0.15 migration guide.

- `MeshPickingBackend` is now named `MeshPickingPlugin`.
- `MeshPickingBackendSettings` is now named `MeshPickingSettings`.
- `SpritePickingBackend` is now named `SpritePickingPlugin`.
- `UiPickingBackendPlugin` is now named `UiPickingPlugin`.
- `DefaultPickingPlugins` is now a a `PluginGroup` instead of a
`Plugin`.
- bevy_dev_tools 0.15.0-rc.1 failed to build docs
- it use bevy_text feature in bevy_ui but it's not enabled by default
- https://docs.rs/crate/bevy_dev_tools/0.15.0-rc.1
-

- enable bevy_text feature of bevy_ui
This PR adds `#[doc(fake_variadic)]` to that were previously not
supported by rustdoc.

Thanks to an [upstream
contribution](rust-lang/rust#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>`.
# Objective

- Mesh picking is noisy when a non triangle list is used
- Mesh picking runs even when users don't need it
- Resolve bevyengine#16065 

## Solution

- Don't add the mesh picking plugin by default
- Remove error spam
# Objective

In `bevy_mod_picking` events are accessible through event listeners or
`EventReader`s. When I replaced event listeners with observers, I
removed the `EventReader` for simplicity. This adds it back.

## Solution

All picking events are now properly registered, and can be accessed
through `EventReader<Pointer<E>>`. `Pointer` now tracks the entity the
event targeted initially, and this can also be helpful in observers
(which don't currently do this).

## Testing

The picking examples run fine. This shouldn't really change anything.

---------

Co-authored-by: Aevyrie <[email protected]>
# Objective

This example is really confusing to look at and tell at a glance whether
it's broken or not.

It's displaying a strange shape -- a cube with two vertices stretched in
a couple dimensions at an odd angle, and doing its vertex position
modification in a way where the intent isn't obvious.

## Solution

- Change the gltf geometry so that the object is a recognizable regular
shape
- Change the vertex modification so that the entire cube top is being
"lifted" from the cube
- Adjust colors, lighting, and camera location so we can see what's
going on
- Also remove some irrelevant shadow and environment map setup

## Before


![Image](https://github.com/user-attachments/assets/e5dd5075-0480-49d4-b1ed-cf1fe6106f3c)

## After

<img width="1280" alt="image"
src="https://github.com/user-attachments/assets/59cab60d-efbc-47c3-8688-e4544b462421">
…er-level optimization (bevyengine#16090)

# Objective

Order independent transparency can filter fragment writes based on the
alpha value and it is currently hard-coded to anything higher than 0.0.
By making that value configurable, users can optimize fragment writes,
potentially reducing the number of layers needed and improving
performance in favor of some transparency quality.

## Solution

This PR adds `alpha_threshold` to the
OrderIndependentTransparencySettings component and uses the struct to
configure a corresponding shader uniform. This uniform is then used
instead of the hard-coded value.

To configure OIT with a custom alpha threshold, use:

```rust
fn setup(mut commands: Commands) {
    commands.spawn((
        Camera3d::default(),
        OrderIndependentTransparencySettings {
            layer_count: 8,
            alpha_threshold: 0.2,
        },
    ));
}
```

## Testing

I tested this change using the included OIT example, as well as with two
additional projects.

## Migration Guide

If you previously explicitly initialized
OrderIndependentTransparencySettings with your own `layer_count`, you
will now have to add either a `..default()` statement or an explicit
`alpha_threshold` value:

```rust
fn setup(mut commands: Commands) {
    commands.spawn((
        Camera3d::default(),
        OrderIndependentTransparencySettings {
            layer_count: 16,
            ..default()
        },
    ));
}
```

---------

Co-authored-by: JMS55 <[email protected]>
# Objective

- Fixes bevyengine#16098

## Solution

- Undeprecate `is_playing_animation` and copy the docs from
`animation_is_playing` to it.

## Testing

- CI

## Migration


https://github.com/bevyengine/bevy-website/blob/68e9a34e3068ed2e7db5ae0b4b32feac94a589dd/release-content/0.15/migration-guides/_guides.toml#L13-L17
needs to be removed.
…mponents (bevyengine#16083)

Missed this in the required components PR review. `ContentSize` isn't
used by regular UI nodes, only those with intrinsically sized content
that needs a measure func.

Remove `ContentSize` from `Node`'s required components and add it to the
required components of `Text` and `UiImage`.

---------

Co-authored-by: Alice Cecile <[email protected]>
# Objective

Remove `calculated_` from the name `ComputedNode::calculated_size` as
redundant, It's obvious from context that it's the resolved size value
and it's inconsistant since none of other fields of `ComputedNode` have
a `calculated_` prefix.

## Alternatives

Rename all the fields of `ComputedNode` to `calculated_*`, this seems
worse.
# Objective

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

## Solution

- Simple typo fix

## Testing

- Tested locally
# Objective

- Fixes bevyengine#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.
…ine#16133)

# Objective

- Supersedes bevyengine#16126 

## Solution

- Updated code in `file_watcher.rs` to fix breaking changes introduced
in the new version.
- Check changelog here:
https://github.com/notify-rs/notify/blob/main/CHANGELOG.md#debouncer-full-040-2024-10-25.
- Relevant PR with the breaking change:
notify-rs/notify#557.

## Testing

- CI checks passing locally

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
# Objective

Taffy added layout rounding a while ago but it had a couple of bugs and
caused some problems with the fussy `ab_glyph` text implementation. So I
disabled Taffy's builtin rounding and added some hacks ad hoc that fixed
(some) of those issues. Since then though Taffy's rounding algorithm has
improved while we've changed layout a lot and migrated to `cosmic-text`
so those hacks don't help any more and in some cases cause significant
problems.

Also our rounding implementation only rounds to the nearest logical
pixel, whereas Taffy rounds to the nearest physical pixel meaning it's
much more accurate with high dpi displays.

fixes bevyengine#15197

## Some examples of layout rounding errors visible in the UI examples

These errors are much more obvious at high scale factor, you might not
see any problems at a scale factor of 1.

`cargo run --example text_wrap_debug`

<img width="1000" alt="text_debug_gaps"
src="https://github.com/user-attachments/assets/5a584016-b8e2-487b-8842-f0f359077391">

The narrow horizontal and vertical lines are gaps in the layout caused
by errors in the coordinate rounding.

`cargo run --example text_debug`

<img width="1000" alt="text_debug"
src="https://github.com/user-attachments/assets/a4b37c02-a2fd-441c-a7bd-cd7a1a72e7dd">

The two text blocks here are aligned right to the same boundary but in
this screen shot you can see that the lower block is one pixel off to
the left. Because the size of this text node changes between frames with
the reported framerate the rounding errors cause it to jump left and
right.

## Solution

Remove all our custom rounding hacks and reenable Taffy's layout
rounding.

The gaps in the `text_wrap_debug` example are gone:
<img width="1000" alt="text_wrap_debug_fix"
src="https://github.com/user-attachments/assets/92d2dd97-30c6-4ac8-99f1-6d65358995a7">

This doesn't fix some of the gaps that occur between borders and content
but they seem appear to be a rendering problem as they disappear with
`UiAntiAlias::Off` set.

## Testing

Run the examples as described above in the `Objective` section. With
this PR the problems mentioned shouldn't appear.

Also added an example in a separate PR bevyengine#16096 `layout_rounding_debug`
for identifying these issues.

## Migration Guide

`UiSurface::get_layout` now also returns the final sizes before
rounding. Call `.0` on the `Ok` result to get the previously returned
`taffy::Layout` value.

---------

Co-authored-by: Rob Parrett <[email protected]>
The `ContentSize` requirement on `UiImage` got lost during merge
conflict fixes, causing some images such as the icons on the `game_menu`
example to disappear.

Fixes bevyengine#16136

Require `ContentSize` on `UiImage` again.

---------

Co-authored-by: Alice Cecile <[email protected]>
# Objective

Fixes bevyengine#15676

## Solution

`remove` returns the removed item

Add `take`

## Testing

None yet

## Migration Guide

If you don't need the returned value from `remove`, discard it.
jakobhellermann and others added 22 commits January 3, 2025 19:15
# Objective

The parameter names for `bevy::math::ops::atan2` are labelled such that
`x` is the first argument and `y` is the second argument, but it passes
those arguments directly to
[`f32::atan2`](https://doc.rust-lang.org/stable/std/primitive.f32.html#method.atan2),
whose parameters are expected to be `(y, x)`. This PR changes the
parameter names in the bevy documentation to use the correct order for
the operation being performed. You can verify this by doing:

```rust
fn main() {
    let x = 3.0;
    let y = 4.0;
    let angle = bevy::math::ops::atan2(x, y);
    // standard polar coordinates formula
    dbg!(5.0 * angle.cos(), 5.0 * angle.sin());
}
```

This will print `(4.0, 3.0)`, which has flipped `x` and `y`. The problem
is that the `atan2` function to calculate the angle was really expecting
`(y, x)`, not `(x, y)`.

## Solution

I flipped the parameter names for `bevy::math::ops::atan2` and updated
the documentation. I also removed references to `self` and `other` from
the documentation which seemed to be copied from the `f32::atan2`
documentation.

## Testing

Not really needed, you can compare the `f32::atan2` docs to the
`bevy::math::ops::atan2` docs to see the problem is obvious. If a test
is required I could add a short one.
## Migration Guide

I'm not sure if this counts as a breaking change, since the
implementation clearly meant to use `f32::atan2` directly, so it was
really just the parameter names that were wrong.
# Objective

Fixes bevyengine#16771 

## Solution

Fixed typo in code.

## Testing

- Did you test these changes? If so, how?
I tested on my own example, that I included in the issue. It was
behaving as I expected.

Here is the screenshot after fix, the screenshot before the fix can be
found in the issue.

![image](https://github.com/user-attachments/assets/f558363f-718d-4244-980c-d224feb2ba0b)
Instead of clipping the non-visable sections of box-shadows, the shadow
is scaled to fit into the remaining area after clipping because the
normalized coordinates that are meant to border the unclipped subsection
of the shadow are always set to `[Vec2::ZERO, Vec2::X, Vec2::ONE,
Vec2::Y]`,

Calculate the coordinates for the corners of the visible area.

Test app:

```rust
use bevy::color::palettes::css::RED;
use bevy::color::palettes::css::WHITE;
use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2d);
    commands
        .spawn(Node {
            ..Default::default()
        })
        .with_children(|commands| {
            commands
                .spawn((
                    Node {
                        width: Val::Px(100.),
                        height: Val::Px(100.),
                        margin: UiRect {
                            left: Val::Px(100.),
                            top: Val::Px(300.),
                            ..Default::default()
                        },
                        overflow: Overflow::clip(),
                        ..Default::default()
                    },
                    BackgroundColor(WHITE.into()),
                ))
                .with_children(|commands| {
                    commands.spawn((
                        Node {
                            position_type: PositionType::Absolute,
                            left: Val::Px(50.),
                            top: Val::Px(50.),
                            width: Val::Px(100.),
                            height: Val::Px(100.),
                            ..Default::default()
                        },
                        BackgroundColor(RED.into()),
                        BoxShadow::from(ShadowStyle {
                            x_offset: Val::ZERO,
                            y_offset: Val::ZERO,
                            spread_radius: Val::Px(50.),
                            blur_radius: Val::Px(6.),
                            ..Default::default()
                        }),
                    ));
                });
        });
}
```

Main:
<img width="103" alt="bad_shadow"
src="https://github.com/user-attachments/assets/6f7ade0e-959f-4d18-92e8-903630eb8cd3"
/>

This PR:
<img width="98" alt="clipped_shadow"
src="https://github.com/user-attachments/assets/7f576c94-908c-4fe6-abaa-f18fefe05207"
/>
# Objective

Scroll position uses physical coordinates. This means scrolling may go
faster or slower depending on the scroll factor. Also the scrolled
position will change when the scale factor changes.

## Solution

In `ui_layout_system` convert `max_possible_offset` to logical
coordinates before clamping the scroll position. Then convert the
clamped scroll position to physical coordinates before propagating it to
the node's children.

## Testing

Look at the `scroll` example. On main if you change your display's scale
factor the items displayed by the scrolling lists will change because
`ScrollPosition`'s displacement values don't respect scale factor. With
this PR the displacement will be scaled too, and the won't move.
This feature was tested with WASM, WebGL, and WebGPU. It should work on
these targets. I think this was an oversight in the original PR.
Fixes: bevyengine#16578

This is a patch fix, proper fix requires a breaking change.

Added `Panic` enum variant and using is as the system meta default.
Warn once behavior can be enabled same way disabling panic (originally
disabling wans) is.

To fix an issue with the current architecture, where **all** combinator
system params get checked together,
combinator systems only check params of the first system.
This will result in old, panicking behavior on subsequent systems and
will be fixed in 0.16.

Ran unit tests and `fallible_params` example.

---------

Co-authored-by: François Mockers <[email protected]>
Co-authored-by: François Mockers <[email protected]>
… doesn't exist (bevyengine#16932)

Fixes a crash when using deferred rendering but disabling the default
deferred lighting plugin.

# The Issue
The `ScreenSpaceReflectionsPlugin` references
`NodePbr::DeferredLightingPass`, which hasn't been added when
`PbrPlugin::add_default_deferred_lighting_plugin` is `false`.

This yields the following crash:
```
thread 'main' panicked at /Users/marius/Documents/dev/bevy/crates/bevy_render/src/render_graph/graph.rs:155:26:
InvalidNode(DeferredLightingPass)
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/90b35a6239c3d8bdabc530a6a0816f7ff89a0aaf/library/core/src/panicking.rs:74:14
   2: bevy_render::render_graph::graph::RenderGraph::add_node_edges
             at /Users/marius/Documents/dev/bevy/crates/bevy_render/src/render_graph/graph.rs:155:26
   3: <bevy_app::sub_app::SubApp as bevy_render::render_graph::app::RenderGraphApp>::add_render_graph_edges
             at /Users/marius/Documents/dev/bevy/crates/bevy_render/src/render_graph/app.rs:66:13
   4: <bevy_pbr::ssr::ScreenSpaceReflectionsPlugin as bevy_app::plugin::Plugin>::finish
             at /Users/marius/Documents/dev/bevy/crates/bevy_pbr/src/ssr/mod.rs:234:9
   5: bevy_app::app::App::finish
             at /Users/marius/Documents/dev/bevy/crates/bevy_app/src/app.rs:255:13
   6: bevy_winit::state::winit_runner
             at /Users/marius/Documents/dev/bevy/crates/bevy_winit/src/state.rs:859:9
   7: core::ops::function::FnOnce::call_once
             at /Users/marius/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
   8: core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /Users/marius/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
   9: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /Users/marius/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:2454:9
  10: bevy_app::app::App::run
             at /Users/marius/Documents/dev/bevy/crates/bevy_app/src/app.rs:184:9
  11: bevy_deferred_test::main
             at ./src/main.rs:9:5
  12: core::ops::function::FnOnce::call_once
             at /Users/marius/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
```


### Minimal reproduction example:
```rust
use bevy::core_pipeline::prepass::{DeferredPrepass, DepthPrepass};
use bevy::pbr::{DefaultOpaqueRendererMethod, PbrPlugin, ScreenSpaceReflections};
use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins.set(PbrPlugin {
            add_default_deferred_lighting_plugin: false,
            ..default()
        }))
        .add_systems(Startup, setup)
        .insert_resource(DefaultOpaqueRendererMethod::deferred())
        .run();
}

/// set up a camera
fn setup(
    mut commands: Commands
) {
    // camera
    commands.spawn((
        Camera3d::default(),
        Transform::from_xyz(-2.5, 4.5, 9.0).looking_at(Vec3::ZERO, Vec3::Y),
        DepthPrepass,
        DeferredPrepass,
        ScreenSpaceReflections::default(),
    ));
}
```

# The Fix
When no node under the default lighting node's label exists, this label
isn't added to the SSR's graph node edges. It's good to keep the
SSRPlugin enabled, this way, users can plug in their own lighting
system, which I have successfully done on top of this PR.

# Workarounds

A current workaround for this issue is to re-use Bevy's
`NodePbr::DeferredLightingPass` as the label for your own custom
lighting pass node.
This PR simply exposes Bevy PBR's
`TONEMAPPING_LUT_TEXTURE_BINDING_INDEX` and
`TONEMAPPING_LUT_SAMPLER_BINDING_INDEX`.

# Objective 
Alongside bevyengine#16932, this is the last required change to be able to replace
Bevy's built-in deferred lighting pass with a custom one based on the
original logic.
# Objective

- Fixes bevyengine#16563 
- Make sure bevy_image is available when needed

## Solution

- Add a new feature for `bevy_image`
- Also enable the `bevy_image` feature in `bevy_internal` for all
features that use `bevy_image` themselves
# Objective

- Fixes bevyengine#16571

## Solution

- When position delta is zero, don't trigger `Drag` or `DragOver` events

## Testing

- tested with the code from the issue
# Objective

- Fixes bevyengine#16568 

## Solution

- `bevy_winit` feature also enables `bevy_window`
# Objective

Fixes bevyengine#16850

## Solution

Add a new function `SubApp::take_extract()`, similar to
`Option::take()`, which allows stealing the currently installed extract
function of a sub-app, with the intent to replace it with a custom one
calling the original one via `set_extract()`.

This pattern enables registering a custom "world sync" function similar
to the existing one `entity_sync_system()`, to run custom world sync
logic with mutable access to both the main and render worlds.

## Testing

`cargo r -p ci` currently doesn't build locally, event after upgrading
rustc to latest and doing a `cargo update`.
Fixes bevyengine#16978

While testing, discovered that the morph weight interface in
`scene_viewer` has been broken for a while (panics when loaded model has
morph weights), probably since bevyengine#15591. Fixed that too.

While testing, saw example text in morph interface with [wrong
padding](https://bevyengine.org/learn/contribute/helping-out/creating-examples/#visual-guidelines).
Fixed that too. Left the small font size because there may be a lot of
morphs to display, so that seems intentional.

Use normal queries and bail early

Morph interface can be tested with
```
cargo run --example scene_viewer assets/models/animated/MorphStressTest.gltf
```

I noticed that this fix is different than what is happening in bevyengine#16976.
Feel free to discard this for an alternative fix. I opened this anyway
to document the issue with morph weight display.

This is on top of bevyengine#16966 which is required to test.

---------

Co-authored-by: François Mockers <[email protected]>
Co-authored-by: François Mockers <[email protected]>
# Objective

ensure that `animate_targets` runs **before**
`bevy_render::mesh::inherit_weights` to address the one-frame delay

Fixes bevyengine#16554 

## Solution

switch ordering constraints from `after` to `before`

## Testing

ran bevy_animation tests and the animated_fox example on MacOS
# Objective

- Fixes bevyengine#16959
- The `pbr.rs` example in the 3d section panicked because of the changes
in bevyengine#16638, that was not supposed to happen

## Solution

- For now it's sufficient to introduce a `never_param_warn` call when
adding the fallible system into the app

## Testing

- Tested on my machine via `cargo r --example pbr`, it built and ran
successfully

---------

Co-authored-by: Freya Pines <[email protected]>
Co-authored-by: François Mockers <[email protected]>
…yengine#16958)

# Objective

Fixes bevyengine#16879

## Solution

Moved the construction of the root path of the assets folder out of
`FileWatcher::new()` and into `source.rs`, as the path is checked there
with `path.exists()` and fails in certain configurations eg., virtual
workspaces.

## Testing

Applied fix to a private fork and tested against both standard project
setups and virtual workspaces. Works without issue on both. Have tested
under macOS and Arch Linux.

---------

Co-authored-by: JP Stringham <[email protected]>
Co-authored-by: Alice Cecile <[email protected]>
After a recent fix for a panic in the pbr example (bevyengine#16976), the code
contains the following comment:

```rust
// This system relies on system parameters that are not available at start
// Ignore parameter failures so that it will run when possible
.add_systems(Update, environment_map_load_finish.never_param_warn())
```

However, this explanation is incorrect. `EnvironmentMapLabel` is
available at start. The real issue is that it is no longer available
once it has been removed by `environment_map_load_finish`.

- Remove confusing/incorrect comment and `never_param_warn()`.
- Make `Single<Entity, With<EnvironmentMapLabel>>` optional in
`environment_map_load_finish`, and check that the entity has not yet
been despawned.

Since it is expected that an entity is no longer there once it has been
despawned, it seems better to me to handle this case in
`environment_map_load_finish`.

Ran `cargo run --example pbr`.
…vyengine#17007)

# Objective

Fix alignment calculations in our rendering code.
Fixes bevyengine#16992 

The `gpu_readback::align_byte_size` function incorrectly rounds aligned
values to the next alignment.
If we assume the alignment to be 256 (because that's what wgpu says it
its) the function would align 0 to 256, 256 to 512, etc...

## Solution

Forward the `gpu_readback::align_byte_size` to
`RenderDevice::align_copy_bytes_per_row` so we don't implement the same
method twice.
Simplify `RenderDevice::align_copy_bytes_per_row`.

## Testing

Ran the code provided in bevyengine#16992 to see if the issue has been solved +
added a test to check if `align_copy_bytes_per_row` returns the correct
values.
…bevyengine#16982)

Fixes bevyengine#16730

Make the relevant functions public. (`MaterialBindGroupAllocator` itself
was already `pub`)
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@mockersf
Copy link
Member

you should open PR against the main branch

@rambip rambip changed the base branch from release-0.15.1 to main January 28, 2025 14:37
@rambip
Copy link
Author

rambip commented Jan 28, 2025

Done, I will delete this

@rambip rambip closed this Jan 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.