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

Multiple box shadow support #16502

Merged
merged 10 commits into from
Dec 1, 2024

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Nov 25, 2024

Objective

Add support for multiple box shadows on a single Node.

Solution

  • Rename BoxShadow to ShadowStyle and remove its Component derive.
  • Create a new BoxShadow component that newtypes a Vec<ShadowStyle>.
  • Add a new constructor method to BoxShadow for single shadows.
  • Change extract_shadows to iterate through a list of shadows per node.

Render order is determined implicitly from the order of the shadows stored in the BoxShadow component, back-to-front.
Might be more efficient to use a SmallVec<[ShadowStyle; 1]> for the list of shadows but not sure if the extra friction is worth it.

Testing

Added a node with four differently coloured shadows to the box_shadow example.


Showcase

cargo run --example box_shadow
four-shadow

Migration Guide

Bevy UI now supports multiple shadows per node. A new struct ShadowStyle is used to set the style for each shadow. And the BoxShadow component is changed to a tuple struct wrapping a vector containing a list of ShadowStyles. To spawn a node with a single shadow you can use the new constructor function:

commands.spawn((
    Node::default(),
    BoxShadow::new(
        Color::BLACK.with_alpha(0.8),
        Val::Percent(offset.x),
        Val::Percent(offset.y),
        Val::Percent(spread),
        Val::Px(blur),
    )
));

* `BoxShadow` is renamed to `DropShadow`.
* New `BoxShadow` component newtyping a `Vec<DropShadow`.
* Added a node with multiple shadows to the `BoxShadow` example
@ickshonpe ickshonpe added A-UI Graphical user interfaces, styles, layouts, and widgets D-Straightforward Simple bug fixes and API improvements, docs, test and examples C-Feature A new feature, making something new possible S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 25, 2024
@alice-i-cecile alice-i-cecile added this to the 0.16 milestone Nov 25, 2024
@alice-i-cecile alice-i-cecile added M-Needs-Release-Note Work that should be called out in the blog due to impact M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Nov 25, 2024
@alice-i-cecile
Copy link
Member

I like the feature and the API. I'd probably do a smallvec myself, but we should bench that and do it in follow-up.

@ickshonpe
Copy link
Contributor Author

Wondering if it's more or less confusing to call optional components non-required/unrequired instead 😕

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Nov 25, 2024

I like the feature and the API. I'd probably do a smallvec myself, but we should bench that and do it in follow-up.

Yeah it seems to me in practical terms it's unusual to use a very large number of shadows and if you do it's most likely because you are are using multiple shadows in which case the smallvec would allocate on the heap anyway.

Copy link
Contributor

@doup doup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking it was going to be more complex, same but in a loop. 👌

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Nov 28, 2024
@mockersf mockersf added this pull request to the merge queue Dec 1, 2024
Merged via the queue into bevyengine:main with commit 56d5591 Dec 1, 2024
29 checks passed
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Dec 2, 2024
# Objective

Add support for multiple box shadows on a single `Node`.

## Solution

* Rename `BoxShadow` to `ShadowStyle` and remove its `Component` derive.
* Create a new `BoxShadow` component that newtypes a `Vec<ShadowStyle>`.
* Add a `new` constructor method to `BoxShadow` for single shadows.
* Change `extract_shadows` to iterate through a list of shadows per
node.

Render order is determined implicitly from the order of the shadows
stored in the `BoxShadow` component, back-to-front.
Might be more efficient to use a `SmallVec<[ShadowStyle; 1]>` for the
list of shadows but not sure if the extra friction is worth it.

## Testing

Added a node with four differently coloured shadows to the `box_shadow`
example.

---

## Showcase

```
cargo run --example box_shadow
```

<img width="460" alt="four-shadow"
src="https://github.com/user-attachments/assets/2f728c47-33b4-42e1-96ba-28a774b94b24">

## Migration Guide

Bevy UI now supports multiple shadows per node. A new struct
`ShadowStyle` is used to set the style for each shadow. And the
`BoxShadow` component is changed to a tuple struct wrapping a vector
containing a list of `ShadowStyle`s. To spawn a node with a single
shadow you can use the `new` constructor function:
```rust
commands.spawn((
    Node::default(),
    BoxShadow::new(
        Color::BLACK.with_alpha(0.8),
        Val::Percent(offset.x),
        Val::Percent(offset.y),
        Val::Percent(spread),
        Val::Px(blur),
    )
));
```

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
# Objective

Add support for multiple box shadows on a single `Node`.

## Solution

* Rename `BoxShadow` to `ShadowStyle` and remove its `Component` derive.
* Create a new `BoxShadow` component that newtypes a `Vec<ShadowStyle>`.
* Add a `new` constructor method to `BoxShadow` for single shadows.
* Change `extract_shadows` to iterate through a list of shadows per
node.

Render order is determined implicitly from the order of the shadows
stored in the `BoxShadow` component, back-to-front.
Might be more efficient to use a `SmallVec<[ShadowStyle; 1]>` for the
list of shadows but not sure if the extra friction is worth it.

## Testing

Added a node with four differently coloured shadows to the `box_shadow`
example.

---

## Showcase

```
cargo run --example box_shadow
```

<img width="460" alt="four-shadow"
src="https://github.com/user-attachments/assets/2f728c47-33b4-42e1-96ba-28a774b94b24">

## Migration Guide

Bevy UI now supports multiple shadows per node. A new struct
`ShadowStyle` is used to set the style for each shadow. And the
`BoxShadow` component is changed to a tuple struct wrapping a vector
containing a list of `ShadowStyle`s. To spawn a node with a single
shadow you can use the `new` constructor function:
```rust
commands.spawn((
    Node::default(),
    BoxShadow::new(
        Color::BLACK.with_alpha(0.8),
        Val::Percent(offset.x),
        Val::Percent(offset.y),
        Val::Percent(spread),
        Val::Px(blur),
    )
));
```

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Needs-Release-Note Work that should be called out in the blog due to impact S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants