UI node outlines #9931

ickshonpe · 2023-09-26T13:53:21Z

Objective

Add support for drawing outlines outside the borders of UI nodes.

Solution

Add a new Outline component with width, offset and color fields.
Added outline_width and outline_offset fields to Node. This is set after layout recomputation by the resolve_outlines_system.

Properties of outlines:

Unlike borders, outlines have to be the same width on each edge.
Outlines do not occupy any space in the layout.
The Outline component won't be added to any of the UI node bundles, it needs to be inserted separately.
Outlines are drawn outside the node's border, so they are clipped using the clipping rect of their entity's parent UI node (if it exists).
Val::Percent outline widths are resolved based on the width of the outlined UI node.
The offset of the Outline adds space between an outline and the edge of its node.

I was leaning towards adding an outline field to Style but a separate component seems more efficient for queries and change detection. The Outline component isn't added to bundles for the same reason.

Examples

This image is from the borders example from the Bevy UI examples but modified to include outlines. The UI nodes are the dark red rectangles, the bright red rectangles are borders and the white lines offset from each node are the outlines. The yellow rectangles are separate nodes contained with the dark red nodes:
This is from the same example but using a branch that implements border-radius. Here the the outlines are in orange and there is no offset applied. I broke the borders implementation somehow during the merge, which is why some of the borders from the first screenshot are missing 😅. The outlines work nicely though (as long as you can forgive the lack of anti-aliasing):

Notes

As I explained above, I don't think the Outline component should be added to UI node bundles. We can have helper functions though, perhaps something as simple as:

impl NodeBundle {
    pub fn with_outline(self, outline: Outline) -> (Self, Outline) {
        (self, outline)
    }
}

I didn't include anything like this as I wanted to keep the PR's scope as narrow as possible. Maybe with_outline should be in a trait that we implement for each UI node bundle.

Changelog

Added support for outlines to Bevy UI.

The Outline component adds an outline to a UI node.
The outline_width field added to Node holds the resolved width of the outline, which is set by the resolve_outlines_system after layout recomputation.
Outlines are drawn by the system extract_uinode_outlines.

* The `Outline` component adds an outline to a UI Node. * The `outline_width` field added to `Node` holds the resolved width of the outline, which is set by the `resolve_outlines_system` after layout recomputation. * Outlines are drawn by the system `extract_uinode_outlines`.

…node's border, so they are clipped using the clipping Rect of their ui node entity's parent.

nicoburns

Seems useful as-is, but I wonder how hard it would be to add https://developer.mozilla.org/en-US/docs/Web/CSS/outline-offset support to this? Could be useful for people who want to draw outlines inside the node boundaries rather than outside.

ickshonpe · 2023-09-26T20:19:41Z

Seems useful as-is, but I wonder how hard it would be to add https://developer.mozilla.org/en-US/docs/Web/CSS/outline-offset support to this? Could be useful for people who want to draw outlines inside the node boundaries rather than outside.

Really trivial, I'll add it now

* Added an `outline_offset: f32` field to Node. * Added an `offset: Val` field to `Outline`. * In `extract_uinode_outlines` the twice the outline offset is added to the node size before calculating

ickshonpe · 2023-09-26T21:24:35Z

Also have outlines working with border-radius as well:

JoJoJet · 2023-09-30T20:08:36Z

crates/bevy_ui/src/layout/mod.rs

+        / ui_scale.0 as f32;
+
+    for (outline, mut node) in outlines_query.iter_mut() {
+        let node = node.bypass_change_detection();


How come you're calling bypass_change_detection here?

The text system queries for changes to Node so it can recompute the text layout on changes to the UI node's size.

So, do we not care about detecting changes to the outline_width and outline_offset fields?

Not at the moment. Maybe we could have a separate component for every property but it doesn't seem very tractable.

Fair enough. I'd put a comment here describing that we don't care about change detection for these fields because they're private and only used as a cache.

JoJoJet · 2023-09-30T21:07:07Z

crates/bevy_ui/src/layout/mod.rs

+        / ui_scale.0 as f32;
+
+    for (outline, mut node) in outlines_query.iter_mut() {
+        let node = node.bypass_change_detection();


Fair enough. I'd put a comment here describing that we don't care about change detection for these fields because they're private and only used as a cache.

wilk10 · 2023-10-01T09:47:44Z

Love this feature!

I have one question about the implementation: one of the main use-cases I can see would be to show the outline on the currently hovered element in a list or grid. In that case, as a user I would either be removing and adding the Outline component very frequently, or I guess I can set the widthto 0 or the value I want.

IMO it would be more explicit and discoverable to have an is_visible flag as field on Outline and add Outline to all relevant node bundles, so that then I can just toggle it. What do you think?

nicopap · 2023-10-01T17:09:27Z

I think setting the outline color to transparent is clear enough as is. Maybe adding a doc line saying "Consider setting the outline color to transparent if you need to toggle the outline visibility" could fill the gap. Also the current node bundles are already way too large, and results in a lot of extra machine codes. It's reasonable to want to avoid adding more and more components to them.

Also, my thinking is that is_visible adds more concepts that are redundant with pre-existing ones (color), and therefore should be eliminated, as they add more concepts to deal with with no upsides.

ickshonpe · 2023-10-01T17:27:43Z

Love this feature!

I have one question about the implementation: one of the main use-cases I can see would be to show the outline on the currently hovered element in a list or grid. In that case, as a user I would either be removing and adding the Outline component very frequently, or I guess I can set the widthto 0 or the value I want.

IMO it would be more explicit and discoverable to have an is_visible flag as field on Outline and add Outline to all relevant node bundles, so that then I can just toggle it. What do you think?

Setting the outline's color to Color::NONE makes the most sense to me for toggling outlines on and off. An is_visible method feels like it would be too easily confused with Bevy's existing visibility systems.

Most UI nodes won't use outlines, so it wouldn't be efficient to add it to every bundle. If you do need to create a lot of outlined bundles you can always use a builder method or a type alias for (NodeBundle, Outline).

wilk10 · 2023-10-02T08:03:08Z

Love this feature!
I have one question about the implementation: one of the main use-cases I can see would be to show the outline on the currently hovered element in a list or grid. In that case, as a user I would either be removing and adding the Outline component very frequently, or I guess I can set the widthto 0 or the value I want.
IMO it would be more explicit and discoverable to have an is_visible flag as field on Outline and add Outline to all relevant node bundles, so that then I can just toggle it. What do you think?

Setting the outline's color to Color::NONE makes the most sense to me for toggling outlines on and off. An is_visible method feels like it would be too easily confused with Bevy's existing visibility systems.

Most UI nodes won't use outlines, so it wouldn't be efficient to add it to every bundle. If you do need to create a lot of outlined bundles you can always use a builder method or a type alias for (NodeBundle, Outline).

Thanks for the reply. I understand and setting the color to Color::NONE should work and be clear enough. Does it make sense to add methods on Outlineto show / hide (or draw or other alternative names) to control color, in order to point the user in the right direction? Maybe at least a comment on the color field?

My main concern regards table moves when the Outline component is added / removed frequently (eg: an inventory system implemented as a grid, where only the hovered element needs an outline). Setting the color to Color::NONE is clearly the better solution than inserting / removing the component, IMO.

ickshonpe · 2023-10-02T09:46:33Z

Love this feature!
I have one question about the implementation: one of the main use-cases I can see would be to show the outline on the currently hovered element in a list or grid. In that case, as a user I would either be removing and adding the Outline component very frequently, or I guess I can set the widthto 0 or the value I want.
IMO it would be more explicit and discoverable to have an is_visible flag as field on Outline and add Outline to all relevant node bundles, so that then I can just toggle it. What do you think?

Setting the outline's color to Color::NONE makes the most sense to me for toggling outlines on and off. An is_visible method feels like it would be too easily confused with Bevy's existing visibility systems.
Most UI nodes won't use outlines, so it wouldn't be efficient to add it to every bundle. If you do need to create a lot of outlined bundles you can always use a builder method or a type alias for (NodeBundle, Outline).

Thanks for the reply. I understand and setting the color to Color::NONE should work and be clear enough. Does it make sense to add methods on Outlineto show / hide (or draw or other alternative names) to control color, in order to point the user in the right direction? Maybe at least a comment on the color field?

My main concern regards table moves when the Outline component is added / removed frequently (eg: an inventory system implemented as a grid, where only the hovered element needs an outline). Setting the color to Color::NONE is clearly the better solution than inserting / removing the component, IMO.

I wouldn't consider the API here final by any means.

I understand your concerns about table moves. As well as the performance issues, at the moment in Bevy UI they can affect the order of UI elements atm in some circumstances (this should be fixed soon though, we've fixed some of the problems already, and I've been working on changes that should fix the remaining issues).

Show/Hide helper methods have similar problems to the is_visible method though, it feels like they could be confused with the Style property settings. I added what you suggested to the color field's description and some examples to the struct's doc comments that should help I think. Maybe we could add an outlined buttons example in a separate PR as well.

alice-i-cecile · 2023-10-02T12:59:48Z

@ickshonpe once merge conflicts are resolved I'll merge this.

ickshonpe · 2023-10-05T07:47:10Z

this is ready to merge

# Objective Add support for drawing outlines outside the borders of UI nodes. ## Solution Add a new `Outline` component with `width`, `offset` and `color` fields. Added `outline_width` and `outline_offset` fields to `Node`. This is set after layout recomputation by the `resolve_outlines_system`. Properties of outlines: * Unlike borders, outlines have to be the same width on each edge. * Outlines do not occupy any space in the layout. * The `Outline` component won't be added to any of the UI node bundles, it needs to be inserted separately. * Outlines are drawn outside the node's border, so they are clipped using the clipping rect of their entity's parent UI node (if it exists). * `Val::Percent` outline widths are resolved based on the width of the outlined UI node. * The offset of the `Outline` adds space between an outline and the edge of its node. I was leaning towards adding an `outline` field to `Style` but a separate component seems more efficient for queries and change detection. The `Outline` component isn't added to bundles for the same reason. --- ## Examples * This image is from the `borders` example from the Bevy UI examples but modified to include outlines. The UI nodes are the dark red rectangles, the bright red rectangles are borders and the white lines offset from each node are the outlines. The yellow rectangles are separate nodes contained with the dark red nodes: <img width="406" alt="outlines" src="https://github.com/bevyengine/bevy/assets/27962798/4e6f315a-019f-42a4-94ee-cca8e684d64a"> * This is from the same example but using a branch that implements border-radius. Here the the outlines are in orange and there is no offset applied. I broke the borders implementation somehow during the merge, which is why some of the borders from the first screenshot are missing 😅. The outlines work nicely though (as long as you can forgive the lack of anti-aliasing): ![image](https://github.com/bevyengine/bevy/assets/27962798/d15560b6-6cd6-42e5-907b-56ccf2ad5e02) --- ## Notes As I explained above, I don't think the `Outline` component should be added to UI node bundles. We can have helper functions though, perhaps something as simple as: ```rust impl NodeBundle { pub fn with_outline(self, outline: Outline) -> (Self, Outline) { (self, outline) } } ``` I didn't include anything like this as I wanted to keep the PR's scope as narrow as possible. Maybe `with_outline` should be in a trait that we implement for each UI node bundle. --- ## Changelog Added support for outlines to Bevy UI. * The `Outline` component adds an outline to a UI node. * The `outline_width` field added to `Node` holds the resolved width of the outline, which is set by the `resolve_outlines_system` after layout recomputation. * Outlines are drawn by the system `extract_uinode_outlines`.

ickshonpe added 8 commits September 26, 2023 14:12

Use &Outline query not Ref<Outline> in resolve_outlines_system

f2a5595

Outline width should be resolved with max(0) not min(0).

0e82b9e

Implement clipping for outlines. Outlines are drawn outside out of a …

53a5e9f

…node's border, so they are clipped using the clipping Rect of their ui node entity's parent.

cargo fmt --all

cbdbf85

Fixed broken doc comment link

0d9df14

replaced map-flatten with and_then in extract_uinode_outlines

677dcda

removed unnecessary semicolon

472c14f

nicoburns approved these changes Sep 26, 2023

View reviewed changes

nicoburns added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets labels Sep 26, 2023

Allow Outlines to be drawn with an offset:

fd63961

* Added an `outline_offset: f32` field to Node. * Added an `offset: Val` field to `Outline`. * In `extract_uinode_outlines` the twice the outline offset is added to the node size before calculating

nicoburns mentioned this pull request Sep 26, 2023

There should be a way to position bevy_ui children independently of parent Flex parameters #9167

Open

JoJoJet reviewed Sep 30, 2023

View reviewed changes

JoJoJet approved these changes Sep 30, 2023

View reviewed changes

nicopap approved these changes Oct 1, 2023

View reviewed changes

added examples to the doc comments for Outline

a01e290

nicopap added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Oct 2, 2023

edit comments

975813b

Merge branch 'main' into ui-node-outlines

7bf07c7

alice-i-cecile enabled auto-merge October 2, 2023 22:38

fixed doc comment example errors

39b390e

auto-merge was automatically disabled October 3, 2023 11:26
Head branch was pushed to by a user without write access

ickshonpe added 2 commits October 3, 2023 13:03

fixed doc comments

8cd8e55

comments fixed again

32c6c09

alice-i-cecile added this pull request to the merge queue Oct 5, 2023

Merged via the queue into bevyengine:main with commit 2e887b8 Oct 5, 2023
21 checks passed

cart mentioned this pull request Oct 13, 2023

News: Release 0.12 bevyengine/bevy-website#754

Merged

43 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI node outlines #9931

UI node outlines #9931

ickshonpe commented Sep 26, 2023 •

edited

Loading

nicoburns left a comment

ickshonpe commented Sep 26, 2023

ickshonpe commented Sep 26, 2023 •

edited

Loading

JoJoJet Sep 30, 2023

ickshonpe Sep 30, 2023

JoJoJet Sep 30, 2023

ickshonpe Sep 30, 2023

JoJoJet Sep 30, 2023

JoJoJet Sep 30, 2023

wilk10 commented Oct 1, 2023

nicopap commented Oct 1, 2023 •

edited

Loading

ickshonpe commented Oct 1, 2023

wilk10 commented Oct 2, 2023

ickshonpe commented Oct 2, 2023 •

edited

Loading

alice-i-cecile commented Oct 2, 2023

ickshonpe commented Oct 5, 2023

UI node outlines #9931

UI node outlines #9931

Conversation

ickshonpe commented Sep 26, 2023 • edited Loading

Objective

Solution

Examples

Notes

Changelog

nicoburns left a comment

Choose a reason for hiding this comment

ickshonpe commented Sep 26, 2023

ickshonpe commented Sep 26, 2023 • edited Loading

JoJoJet Sep 30, 2023

Choose a reason for hiding this comment

ickshonpe Sep 30, 2023

Choose a reason for hiding this comment

JoJoJet Sep 30, 2023

Choose a reason for hiding this comment

ickshonpe Sep 30, 2023

Choose a reason for hiding this comment

JoJoJet Sep 30, 2023

Choose a reason for hiding this comment

JoJoJet Sep 30, 2023

Choose a reason for hiding this comment

wilk10 commented Oct 1, 2023

nicopap commented Oct 1, 2023 • edited Loading

ickshonpe commented Oct 1, 2023

wilk10 commented Oct 2, 2023

ickshonpe commented Oct 2, 2023 • edited Loading

alice-i-cecile commented Oct 2, 2023

ickshonpe commented Oct 5, 2023

ickshonpe commented Sep 26, 2023 •

edited

Loading

ickshonpe commented Sep 26, 2023 •

edited

Loading

nicopap commented Oct 1, 2023 •

edited

Loading

ickshonpe commented Oct 2, 2023 •

edited

Loading