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

UI Node Border Radius and Shadows #8973

Open
wants to merge 76 commits into
base: main
Choose a base branch
from

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Jun 27, 2023

Objective

Implement border radius and shadows for Bevy UI nodes.

fixes #5924

closely related PRs #7795, #8381, #3991

Solution

Add a component UiBorderRadius which contains a radius value for each corner of the UI node.
The fragment shader uses a signed distance function to generate the rounded corners effect.

ui_shadow borders_borders_example tiles

Remaining issues and Limitations

* UiBorderRadius only takes values in logical pixels. Supporting all the Val variants shouldn't be a problem. Mainly just need to work out sensible rules for how percentage values should be resolved.

* The border radius value is clamped before being sent to the shader but with very unbalanced opposing borders you can get weird-looking results.

  • Clipping and Interaction still use unrounded rects.

* Anti-aliasing still seems slightly off.


Changelog

  • Style
    Added border_radius: UiBorderRadius field.

  • UiBorderRadius
    New struct that holds the border radius values.

  • extract_uinode_borders
    Stripped down, most of the work is done in the shader now. Borders are no longer assembled from multiple rects, instead the shader uses a signed distance function to draw the border.

  • ExtractedUiNode
    Added border and corner_radius fields.

  • UiVertex
    Added size, border and radius fields.

  • UiPipeline
    Added three vertex attributes to the vertex buffer layout, to accept the UI node's size, border thickness and border radius.

  • examples
    Added rounded corners to UI elements in the button, ui, size_constraints and borders examples.

Migration Guide

`UiCornerRadius`
* New component that sets the radius of rounded corners for nodes. Wraps an f32 value.
* Currently all corners of a node share the same radius.
* Not tested yet, but should be broken for UI texture atlas images. Need to make changes to `prepare_uinodes`.

`node_bundles` module
* Added the `UiCornerRadius` component to all UI node bundles except for `TextBundle`.

`ExtractedUiNode`
* Added a `corner_radius` field.

`extract_uinode_borders`
* Repurposed the UV coords to control which part of the border is drawn.

`UiVertex`
* Added `radius` and `size` fields which are needed to draw the rounded corners.

`UiPipeline`
* Added vertex attributes for radius and size.

`ui.wgsl`
* Checks to see if we are outside a corner edge and if so sets the alpha channel to zero.
…e otherwise it always draws a border middle section
Replaced the multiple if statements with a switch statement.
* Refactored to remove duplicate code.

`ui.wgsl`
* Switched colors for the inverted mix function, which were in the wrong the wrong order.

`button` example
* Added a rounded border to the button.
@alice-i-cecile alice-i-cecile added C-Enhancement A new feature A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets labels Jun 27, 2023
Fixes bulging in corner curves.
Updates examples to use new features.

`UiCornerRadius`
* Now wraps a `[f32; 4]` value.
* Added a method `all` that sets all four corners to the same radius.

`UiPipeline`
* Added a `Float32x4` attribute to take the border thickness.
* Radius attribute changed to a `Float32x4`

`extract_uinode_borders`
* No longer assembles a border from multiple rects, much simpler now. Most of this function was deleted.

`ExtractedUiNode`
* Added an [f32; 4] `thickness` field
* Type of `corner_radius`  changed to `[f32; 4]`

`ui.wgsl`
* Reworked to support new features and bug fixes.
…ed edges.

`UiCornerRadius`
* Now has named fields `top_left`, `top_right`, `bottom_left` and `bottom_right`.
* Added lots of helper functions.
* Implemented `From<UiCornerRadius>` for `[f32; 4]

`prepare_uinodes`
* Changed order of thickness values when added to `UiVertex`. Don't remember why.

`ui.wgsl`
* Cleaned up and fixed some bugs where corners and edges were taking wrong radius and thickness values.
@ickshonpe
Copy link
Contributor Author

Fixed all the obvious bugs. Just percentage and viewport Val support left to implement.

The fields are now type `Val`.
Full support for all variants, `Auto` maps to `0.`

`ExtractUiNodes`:
`thickness` field renamed to `border`.

`UiVertex`:
`thickness` field renamed to `border`.
@ickshonpe ickshonpe changed the title UI Corner Radius UI Border Radius Jun 30, 2023
@nicoburns
Copy link
Contributor

nicoburns commented Jun 30, 2023

The inner radii don't look right to me. I think the inner radius should be the outer radius minus the border width. So if you have a 5px border and a corner radius of 5px or less then the inner corner shouldn't be rounded at all.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Jun 30, 2023

The inner radii don't look right to me. I think the inner radius should be the outer radius minus the border width. So if you have a 5px border and a corner radius of 5px or less then the inner corner shouldn't be rounded at all.

That is how I'm calculating the inner radius:

commands.spawn(NodeBundle {
    style: Style {
        width: Val::Px(400.),
        height: Val::Px(400.),                
        border: UiRect::px(100., 100., 100., 100.),
        border_radius: UiBorderRadius::px(50., 100., 150., 200.),
        ..Default::default()
    },
    border_color: Color::RED.into(),
    background_color: Color::WHITE.into(),
    ..Default::default()
});
rounded_rect

It's because of problems with the anti-aliasing that some of the example images look a little bit off. I'm leaving fixing the AA until last.

* `UiBorderRadius` no longer a component and removed  from all node bundles.

`UiBorderRadius`
* Renamed from `UiCornerRadius`
* No longer a component, moved to a field of  `Style` instead.
* Added a const DEFAULT.
* Default radii changed to `Val::Px(0.)` from `Val::Auto`.

`Style`
* Added `border_radius: UiBorderRadius` field.

`ExtractedUiNodes`
* `corner_radius` renamed to `border_radius`

`extract_uinodes`, `extract_uinode_borders`, and `extract_atlas_uinodes`:
Now query for `Style` instead of `UiBorderRadius`.
* Added a border to each button.
* Borders can be disabled with the "no-borders" commandline argument.
@tadeohepperle
Copy link
Contributor

I think there might be some weird interactions with UiScale resource. Setting the UiScale.scale to values other than 1 causes the borders to be rendered differently. The images below use border_radius: UiBorderRadius::MAX
scale = 0.5:
image
scale = 1.0:
image
scale = 2.0:
image

@nicoburns nicoburns mentioned this pull request Aug 13, 2023
@superdump
Copy link
Contributor

Random question - why does the shadow on the light, transparent circle increase the brightness? That's not a shadow :)

@gabriel-gheorghe
Copy link
Contributor

It would also be nice to be able to choose the shadow color.

@JMS55 JMS55 modified the milestones: 0.12, 0.13 Oct 24, 2023
@Bcompartment
Copy link

Squircles
I propose to add option to use squircle corner rounding.
The rounding starts deep within the straight line and gradually grows into a visible curve.
Squircles are complex curves that remove the discontinuity between the straight and curved sections of a surface. Look at a curved surface that isn’t a squircle and you’ll be able to see where the straight line ends and the curve starts.

demo

https://en.wikipedia.org/wiki/Squircle#/media/File:Squircle_rounded_square.svg

@viridia
Copy link
Contributor

viridia commented Dec 11, 2023

I just wanted to add that if clipping is holding up this PR, I would say go ahead and commit even if clipping is not there yet, and finish it in a future PR. While round-corner clipping is very handy to have, there are lots of use cases that don't require clipping. And since no one is forced to use rounded corners, it is "strictly no worse" than what they had previously.

The same logic holds for shadows.

@eerii eerii mentioned this pull request Dec 18, 2023
4 tasks
@JMS55
Copy link
Contributor

JMS55 commented Dec 30, 2023

Found an article on how to render box shadows well on the GPU https://madebyevan.com/shaders/fast-rounded-rectangle-shadows

Copy link
Contributor

@doonv doonv left a comment

Choose a reason for hiding this comment

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

Looks pretty good!
I have a lot of nitpicks here, so I wanted to tell you that the main problem is examples/games/game_menu.rs:709. Here you used UiBorderRadius::max() which doesn't exist. Use UiBorderRadius::MAX instead.

@@ -698,6 +706,7 @@ mod menu {
width: Val::Px(200.0),
height: Val::Px(65.0),
margin: UiRect::all(Val::Px(20.0)),
border_radius: UiBorderRadius::max(),
Copy link
Contributor

Choose a reason for hiding this comment

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

UiBorderRadius::max() doesn't exist, did you mean UiBorderRadius::MAX?

Suggested change
border_radius: UiBorderRadius::max(),
border_radius: UiBorderRadius::MAX,

/// assert_eq!(ui_rect.bottom, Val::Vw(25.0));
/// ```
#[inline]
pub fn with_left(mut self, left: Val) -> UiRect {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably use Self instead of UiRect here. (Repeat with similar methods)

Suggested change
pub fn with_left(mut self, left: Val) -> UiRect {
pub fn with_left(mut self, left: Val) -> Self {

@@ -465,6 +465,7 @@ mod tests {
],
grid_column: GridPlacement::start(4),
grid_row: GridPlacement::span(3),
border_radius: Default::default(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border_radius: Default::default(),
border_radius: UiBorderRadius::default(),

@@ -163,7 +168,7 @@ fn spawn_button_row(parent: &mut ChildBuilder, constraint: Constraint, text_styl
align_items: AlignItems::Stretch,
..Default::default()
},
background_color: Color::BLACK.into(),
//background_color: Color::BLACK.into(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//background_color: Color::BLACK.into(),

Comment on lines +1750 to +1755
pub const MAX: Self = Self {
top_left: Val::Px(f32::MAX),
top_right: Val::Px(f32::MAX),
bottom_right: Val::Px(f32::MAX),
bottom_left: Val::Px(f32::MAX),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub const MAX: Self = Self {
top_left: Val::Px(f32::MAX),
top_right: Val::Px(f32::MAX),
bottom_right: Val::Px(f32::MAX),
bottom_left: Val::Px(f32::MAX),
};
pub const MAX: Self = Self::all(Val::Px(f32::MAX)),

height: Val::Px(100.0),
left: Val::Px(-10.),
bottom: Val::Px(-10.),
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
border_radius: UiBorderRadius::MAX,

position_type: PositionType::Absolute,
left: Val::Px(30.),
bottom: Val::Px(20.),
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
border_radius: UiBorderRadius::MAX,

position_type: PositionType::Absolute,
left: Val::Px(70.),
bottom: Val::Px(50.),
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
border_radius: UiBorderRadius::MAX,

position_type: PositionType::Absolute,
left: Val::Px(110.),
bottom: Val::Px(80.),
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
border_radius: UiBorderRadius::MAX,

position_type: PositionType::Absolute,
left: Val::Px(150.),
bottom: Val::Px(110.),
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border_radius: UiBorderRadius::all(Val::Px(f32::MAX)),
border_radius: UiBorderRadius::MAX,

@JMS55
Copy link
Contributor

JMS55 commented Jan 21, 2024

@doonv
Copy link
Contributor

doonv commented Jan 26, 2024

@ickshonpe Are you still working on this PR?

@alice-i-cecile alice-i-cecile modified the milestones: 0.13, 0.14 Jan 26, 2024
@viridia
Copy link
Contributor

viridia commented Jan 28, 2024

Q: Are there any plans to support "inset" shadows? In CSS, I occasionally use this feature for things like text input fields, slider tracks, and other widgets that require a "depressed" look. Implementation-wise, this should just involve drawing the shadow on top of the background (inside the border) and inverting the signed distance calculation.

Also, given that this appears to be stalled, would it make sense to split out the changes to UiRect as a separate PR and commit that now? Those seem relatively uncontroversial.

@doonv
Copy link
Contributor

doonv commented Feb 18, 2024

The creator of this PR has been inactive for a while. Maybe it's time to add the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label?

@alice-i-cecile alice-i-cecile added the C-Needs-Release-Note Work that should be called out in the blog due to impact label Feb 22, 2024
github-merge-queue bot pushed a commit that referenced this pull request Mar 15, 2024
…12487)

# Objective

Originally proposed as part of #8973. Adds `with_` methods for each side
of `UiRect`

## Solution

Add `with_left`, `with_right`, `with_top`, `with_bottom` to `UiRect`.
github-merge-queue bot pushed a commit that referenced this pull request Mar 19, 2024
# Objective

Implements border radius for UI nodes. Adopted from #8973, but excludes
shadows.

## Solution

- Add a component `BorderRadius` which contains a radius value for each
corner of the UI node.
- Use a fragment shader to generate the rounded corners using a signed
distance function.

<img width="50%"
src="https://github.com/bevyengine/bevy/assets/26204416/16b2ba95-e274-4ce7-adb2-34cc41a776a5"></img>

## Changelog

- `BorderRadius`: New component that holds the border radius values.
- `NodeBundle` & `ButtonBundle`: Added a `border_radius: BorderRadius`
field.
- `extract_uinode_borders`: Stripped down, most of the work is done in
the shader now. Borders are no longer assembled from multiple rects,
instead the shader uses a signed distance function to draw the border.
- `UiVertex`: Added size, border and radius fields.
- `UiPipeline`: Added three vertex attributes to the vertex buffer
layout, to accept the UI node's size, border thickness and border
radius.
- Examples: Added rounded corners to the UI element in the `button`
example, and a `rounded_borders` example.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
Co-authored-by: Pablo Reinhardt <126117294+pablo-lua@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this pull request Mar 20, 2024
# Objective

- since #12500, text is a little bit more gray in UI

## Solution

- don't multiply color by alpha. I think this was done in the original
PR (#8973) for shadows which were not added in #12500
@togetherwithasteria
Copy link
Contributor

togetherwithasteria commented Apr 2, 2024

The creator of this PR has been inactive for a while. Maybe it's time to add the S-Adopt-Me The original PR author has no intent to complete this work. Pick me up! label?

Hi! Are there any other work that needs to be done other than the code style changes?

@togetherwithasteria
Copy link
Contributor

Oh I see. With #12500 merged, I believe the next work for this PR is to get the shadows in?

@JMS55 JMS55 removed this from the 0.14 milestone Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Enhancement A new feature C-Needs-Release-Note Work that should be called out in the blog due to impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add proper border support for UI nodes