Add Border and CornerRadius components to bevy_ui

[oceantume]

Objective

• Add a simple set of components to change the visual appearance of UI nodes borders, while introducing as few changes to the actual state of bevy_ui as possible.

I know that bevy_ui will be changed to reflect requirements for the bevy editor, so I tried to make this in a way that it may still be useful if the implementation of UI layout and rendering were to change.

My personal objective with this PR was to make a simple change while getting to know more about some bevy internals.

Solution

Two new components were added. These can be added to any ui node to enable the features on them. No existing component was changed.

New components

BorderRadius

Similarly to CSS's border-radius property, this component describes a set of 4 radii, in pixels, one for each corner. This allows making a lot of different basic widget shapes without relying on textures.

Border

Similarly to CSS's border property, this component describes a width (thickness) and a color to be given to this component's border.

I was not successful at implementing border radius with a variable border width for each side of the rect. If that was supported, then it may be smarter use the Style.border field instead for the width and to replace this with a simple BorderColor component.

UI Renderer changes

Changes were made to the bevy_ui renderer to support this, including changes to ui.wgsl where the magic happens.

At the moment of starting this as a draft, all of the data used to render the borders is sent via the vertex buffer. This is not optimal at all since it triples the amount of data sent for each vertex.

If there's interest, I'd like to look into storing this information into a uniform buffer instead. This would greatly reduce the amount of data stored and uploaded to the GPU, but will add a little more complexity to the rendering code, especially since it would need to support batching.

Examples changes

UI Example

Changes were made to the "ui" example to show that you don't need an extra node to render borders anymore since you can use the Border component. Here's a close-up comparison of before and after on that example (they're the same result).

Button Example

Changes were made to the "button" example to showcase both Border and BorderRadius components.

[alice-i-cecile]

Super useful, and the code quality is pretty good! I've left a few suggestions; I'd want to see those fixed up + the more efficient rendering scheme used before approving this to be merged.

[alice-i-cecile]

@oceantume can you resolve the comments that you've addressed on this PR so it's easier for reviewers to follow what still needs to be done? :)

[cometflaretech]

@oceantume I would create a BorderSide struct which contains a width and a color and then I would use pub border_side: Option<[BorderSide; 4]> just above the corner_radius. This way is much more customizable.

[oceantume]

I clicked the close button by accident😬

@cometflaretech I haven't been able to successfully implement 4 different borders in the shader code, so this wouldn't be possible at the moment. If you have some suggestions on that side I would love to have them. I tried a few things but I couldn't get the maths to actually make sense. This would be something that could be added after, when I or someone else figure it out.

[oceantume]

I just pushed a commit that adds uniform buffer support. Here are a few notes on what I think could be the next steps done in follow-up PR's:

I think the extraction UI node extraction system could use a small refactor to be more similar to the sprite extraction, at least in how the loop and batching works. There's also a last_z value in the current loop that isn't used for anything after extraction.

At the moment, the ui.wgsl shader is also used for the text nodes but they don't need borders or rounded corners, so we could look into removing that overhead for text, maybe by using the shader preprocessor. However, I don't think this is a blocker because the code is pretty simple and the overhead is likely small.

Finally, border width and color for individual borders is not supported at the moment and could be very useful. I failed at getting the maths right for this (especially because it needs to work with border radius), but I'm down to look into it again soon.

[xiaopengli89]

If radius is 0, we can skip calculating the distance.

[oceantume]

If radius is 0, we can skip calculating the distance.

Distance is also used for border color, but your point still stands if we check for both. The tradeoff is that we'll be adding branching to the fragment shader. I'll look into adding this.

[paul-hansen]

Hey @oceantume I'm just a random user that wants to see this merged. It looks like this is pretty out of date with main. Would you be able to update this so it has a better chance of being merged?

Looks to me to be the only thing holding this up, lmk if there is something else. I'd be willing to take a stab at updating it if you aren't able to.

[oceantume]

Hey @oceantume I'm just a random user that want's to see this merged. It looks like this is pretty out of date with main. Would you be able to update this so it has a better chance of being merged?

Looks to me to be the only thing holding this up, lmk if there is something else. I'd be willing to take a stab at updating it if you aren't able to.

I have a feeling this wasn't merged because it made a lot of changes to the pipeline and shaders.

I don't have time to update it at this moment (I'm moving in 5 days), but I'd be willing to look into it. However, I wouldn't be surprised if Cart didn't want to merge it before seeing if there's a better way to implement the pipeline changes for this.

[paul-hansen]

Sounds good, thanks for the update. I'll probably leave it alone then, pipeline decisions are going to be bit over my head still. If someone can confirm we don't need changes to the approach lmk and I might be able to get what you already had working.

[mockersf]

By adding a new Border component outside of the existing Style component, this border width won't be used for the layout and will overlap neighbour nodes

[alice-i-cecile]

My preferred fix would be to strip the border field from Style, and then populate taffy's version internally during the interface. This information is distinct, and belongs in its own component (so then we can do things like reuse the component for sprites).

[mockersf]

the border from style is a lot more rich than a f32 from this PR, and that would mean handling all those cases for rounded corners

[Weibye]

By adding a new Border component outside of the existing Style component, this border width won't be used for the layout and will overlap neighbour nodes

I think we should hold off on this until we have figured out #5511, then we should look into whether it makes sense to populate the new border component with field required for border rendering (or if they should be entirely separate)

My hope here is that the user can define border in one place and that it gets picked up by both layout and rendering

[nicoburns]

Just wanted to note that work on #5511 is now on hold and as such this PR is not blocked. Regarding the API, I think the existing border property in Style should be renamed to border_width and either:

• border_color and border_radius properties added directly to Style
  or
• border_width should be moved to the new Border component alongside the border_color and border_radius properties.

I do think ideally this would support setting different widths for each side. It should also support corner clips that extend beyond the border and into the main content region if that is not already supported.

Finally, it should also be noted that there is an implemention for borders without radii in #7795 which could potentially be combined with this work.

@oceantume Do you have time to update this and drive it towards being merged?

[togetherwithasteria]

Just wanted to note that work on #5511 is now on hold and as such this PR is not blocked. Regarding the API, I think the existing border property in Style should be renamed to border_width and either:

* `border_color` and `border_radius` properties added directly to `Style`
  or

* `border_width` should be moved to the new `Border` component alongside the `border_color` and `border_radius` properties.

I do think ideally this would support setting different widths for each side. It should also support corner clips that extend beyond the border and into the main content region if that is not already supported.

Finally, it should also be noted that there is an implemention for borders without radii in #7795 which could potentially be combined with this work.

@oceantume Do you have time to update this and drive it towards being merged?

FYI. I have ported the PR code to 0.11.0, over at my Bevy fork here
https://github.com/project-flara/bevy/tree/ui-renderer-0.11.0

[togetherwithasteria]

I've managed to get things compiled and running, though I can't see the border and border radius..

This is the first time I'm handling shaders. T_T

[oceantume]

Hi, I would check with Bevy members before putting too much work into this. If I remember correctly this was blocked for a long time and semi-declined because of the amount of renderer code required to make it work.

There's also a good question of whether this is really how it should be done, especially the "fake anti aliasing" done in the shader.

[togetherwithasteria]

There's also a good question of whether this is really how it should be done, especially the "fake anti aliasing" done in the shader.

I personally don't mind any specific ways to do it, as long as it works and it works well for now.

It's just weird that Bevy doesn't have a border radius support, it's certainly not great for people who would like to make pretty UIs.

[alice-i-cecile]

Border radius was added in #12500.