Skip to content

Clip to the UI node's content box#15442

Merged
alice-i-cecile merged 48 commits intobevyengine:mainfrom
ickshonpe:clip-inside-padding-box
Oct 15, 2024
Merged

Clip to the UI node's content box#15442
alice-i-cecile merged 48 commits intobevyengine:mainfrom
ickshonpe:clip-inside-padding-box

Conversation

@ickshonpe
Copy link
Copy Markdown
Contributor

@ickshonpe ickshonpe commented Sep 26, 2024

Objective

Change UI clipping to respect borders and padding.

Fixes #15335

Solution

Based on #15163

  1. Add a padding field to Node.
  2. In ui_layout_size copy the padding values from taffy to Node::padding.
  3. Determine the node's content box (The innermost part of the node excluding the padding and border).
  4. In update_clipping perform the clipping intersection with the node's content box.

Notes

  • Rect probably needs some helper methods for working with insets but because Rect and BorderRect are in different crates it's awkward to add them. Left for a follow up.
  • We could have another Overflow variant (probably called Overflow::Hidden) to that clips inside of the border box instead of the content box. Left it out here as I'm not certain about the naming or behaviour though. If this PR is adopted, it would be trivial to add a Hidden variant in a follow up.
  • Depending on UI scaling there are sometimes gaps in the layout:
    rounding-bug
    This is caused by existing bugs in ui_layout_system's coordinates rounding and not anything to do with the changes in this PR.

Testing

This PR also changes the overflow example to display borders on the overflow nodes so you can see how this works:

main (The image is clipped at the edges of the node, overwriting the border).

main_overflow

this PR (The image is clipped at the edges of the node's border).

content-box-clip

Migration Guide

Migration guide is on #15561

ickshonpe and others added 30 commits September 10, 2024 11:08
…ourselves in each extraction function which is really complicated and fragile and has lead to a number of bugs.

Adds an `Inset` type, a `border: Inset` field to `Node`, updates the border field in `ui_layout_system`, and removes the border calculations and extra queries from the extraction functions.
Removed the `physical_rect` method from `Node`. It doesn't transform the `Node`'s bounding rect to physical coords as the name and docs imply.
Fixed `outline_radius` comment.
@ickshonpe ickshonpe requested a review from nicoburns September 26, 2024 12:24
Copy link
Copy Markdown
Contributor

@nicoburns nicoburns left a comment

Choose a reason for hiding this comment

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

Generally looks good to me.

We could have another Overflow variant (probably called Overflow::Hidden) to that clips inside of the border box instead of the content box. Left it out here as I'm not certain about the naming or behaviour though. If this PR is adopted, it would be trivial to add a Hidden variant in a follow up.

In CSS, overflow: hidden also implies the creation of a "scrollable box" which is not scrollable by the user but is programatically scrollable. It also affects layout of things like flexbox items. For just controlling the clip region there is overflow-clip-margin which allows you to specify one of content-box, padding-box, or border-box and additionally a pixel offset from that box (which may be negative), which may be a better fit for Bevy?

@ickshonpe ickshonpe added C-Feature A new feature, making something new possible A-UI Graphical user interfaces, styles, layouts, and widgets D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Sep 26, 2024
@ickshonpe
Copy link
Copy Markdown
Contributor Author

ickshonpe commented Sep 27, 2024

Generally looks good to me.

We could have another Overflow variant (probably called Overflow::Hidden) to that clips inside of the border box instead of the content box. Left it out here as I'm not certain about the naming or behaviour though. If this PR is adopted, it would be trivial to add a Hidden variant in a follow up.

In CSS, overflow: hidden also implies the creation of a "scrollable box" which is not scrollable by the user but is programatically scrollable. It also affects layout of things like flexbox items. For just controlling the clip region there is overflow-clip-margin which allows you to specify one of content-box, padding-box, or border-box and additionally a pixel offset from that box (which may be negative), which may be a better fit for Bevy?

Yeah that makes sense, I'll make an issue about a new api.

@ickshonpe ickshonpe mentioned this pull request Oct 1, 2024
@pablo-lua pablo-lua 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 Oct 12, 2024
@ickshonpe ickshonpe force-pushed the clip-inside-padding-box branch from e554e88 to 18ba0b1 Compare October 14, 2024 11:07
@ickshonpe
Copy link
Copy Markdown
Contributor Author

I don't know why the CI is failing, the test gets cancelled?

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Oct 15, 2024
Merged via the queue into bevyengine:main with commit b78a060 Oct 15, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 16, 2024
# Objective

Limited implementation of the CSS property `overflow-clip-margin`
https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-clip-margin

Allows you to control the visible area for clipped content when using
overfllow-clip, -hidden, or -scroll and expand it with a margin.

Based on #15442

Fixes #15468

## Solution

Adds a new field to Style: `overflow_clip_margin: OverflowClipMargin`.
The field is ignored unless overflow-clip, -hidden or -scroll is set on
at least one axis.

`OverflowClipMargin` has these associated constructor functions:
```
pub const fn content_box() -> Self;
pub const fn padding_box() -> Self;
pub const fn border_box() -> Self;
```
You can also use the method `with_margin` to increases the size of the
visible area:
```
commands
  .spawn(NodeBundle {
      style: Style {
          width: Val::Px(100.),
          height: Val::Px(100.),
          padding: UiRect::all(Val::Px(20.)),
          border: UiRect::all(Val::Px(5.)),
          overflow: Overflow::clip(),
          overflow_clip_margin: OverflowClipMargin::border_box().with_margin(25.),
          ..Default::default()
      },
      border_color: Color::BLACK.into(),
      background_color: GRAY.into(),
      ..Default::default()
  })
```
`with_margin` expects a length in logical pixels, negative values are
clamped to zero.

## Notes
* To keep this PR as simple as possible I omitted responsive margin
values support. This could be added in a follow up if we want it.
* CSS also supports a `margin-box` option but we don't have access to
the margin values in `Node` so it's probably not feasible to implement
atm.

## Testing

```cargo run --example overflow_clip_margin```

<img width="396" alt="overflow-clip-margin" src="https://github.com/user-attachments/assets/07b51cd6-a565-4451-87a0-fa079429b04b">

## Migration Guide

Style has a new field `OverflowClipMargin`.  It allows users to set the visible area for clipped content when using overflow-clip, -hidden, or -scroll and expand it with a margin.

There are three associated constructor functions `content_box`, `padding_box` and `border_box`:
* `content_box`: elements painted outside of the content box area (the innermost part of the node excluding the padding and border) of the node are clipped. This is the new default behaviour.
* `padding_box`: elements painted outside outside of the padding area of the node are clipped. 
* `border_box`:  elements painted outside of the bounds of the node are clipped. This matches the behaviour from Bevy 0.14.

There is also a `with_margin` method that increases the size of the visible area by the given number in logical pixels, negative margin values are clamped to zero.

`OverflowClipMargin` is ignored unless overflow-clip, -hidden or -scroll is also set on at least one axis of the UI node.

---------

Co-authored-by: UkoeHB <[email protected]>
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 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.

OverflowAxis::Clip allows UI node contents to spill over the border

6 participants