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

Fix various bugs with UI rounded borders #13523

Merged
merged 9 commits into from
May 27, 2024

Conversation

hymm
Copy link
Contributor

@hymm hymm commented May 27, 2024

Objective

Solution

  • Change the antialiasing(AA) method. It was using fwidth which is the derivative between pixels, but there were a lot of artifacts being added from this. So just use the sdf value. This aa method probably isn't as smooth looking, but better than having artifacts. Below is a visualization of the fwidth.
    image
  • Use the internal sdf for drawing the background instead of the external sdf and extract the border for these type of nodes. This fixed 2 bugs, one with the background coloring the AA pixels on the edge of rounded borders. And also allows for the border to use a transparent color.
  • Don't extract borders if all the widths are zero.

Testing

  • played a bunch with the example in the linked issue.
    This PR:
    image
    Main:
    image

  • ran the borders and rounded_borders examples


Changelog

  • Fixed various antialiasing issues to do with rounded ui borders.

@alice-i-cecile alice-i-cecile added this to the 0.14 milestone May 27, 2024
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-UI Graphical user interfaces, styles, layouts, and widgets S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 27, 2024
Copy link
Contributor

@Weibye Weibye left a comment

Choose a reason for hiding this comment

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

This looks good to me, nice and clean implementation.

Comment on lines +240 to +241
// Both vertical and horizontal percentage border values are calculated based on the width of the parent node
// <https://developer.mozilla.org/en-US/docs/Web/CSS/border-width>
Copy link
Contributor

Choose a reason for hiding this comment

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

layout and style is a complex topic, having these comments that refers back to source material helps a lot, thanks!

Comment on lines +242 to +255
let parent_width = parent
.and_then(|parent| node_query.get(parent.get()).ok())
.map(|parent_node| parent_node.size().x)
.unwrap_or(ui_logical_viewport_size.x);
let left =
resolve_border_thickness(style.border.left, parent_width, ui_logical_viewport_size);
let right =
resolve_border_thickness(style.border.right, parent_width, ui_logical_viewport_size);
let top =
resolve_border_thickness(style.border.top, parent_width, ui_logical_viewport_size);
let bottom =
resolve_border_thickness(style.border.bottom, parent_width, ui_logical_viewport_size);

let border = [left, top, right, bottom];
Copy link
Contributor

Choose a reason for hiding this comment

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

All of this makes sense and is straightforward to read 👍

crates/bevy_ui/src/render/ui.wgsl Outdated Show resolved Hide resolved
Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com>
@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 May 27, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 27, 2024
Merged via the queue into bevyengine:main with commit cef31ff May 27, 2024
28 checks passed
github-merge-queue bot pushed a commit that referenced this pull request May 27, 2024
# Objective

- #13523 introduced a new bug on rounded corners in UI on buttons
- there are artefacts outside of the border, and the text in buttons is
more gray than it should be
- example `color_grading`:

<img width="1280" alt="Screenshot 2024-05-27 at 22 19 13"
src="https://github.com/bevyengine/bevy/assets/8672791/fbb6a8ba-2096-4fcc-9c94-3764e9d16d2f">

## Solution

- Clamp alpha to be between 0.0 and 1.0

<img width="1280" alt="Screenshot 2024-05-27 at 22 18 19"
src="https://github.com/bevyengine/bevy/assets/8672791/295d8e16-30eb-40cc-8d61-4995fca6dded">
github-merge-queue bot pushed a commit that referenced this pull request May 29, 2024
# Objective

- since #13523, UI is broken in WebGPU

```
Compilation log for [Invalid ShaderModule (unlabeled)]:
1 error(s) generated while compiling the shader:
:108:27 error: 'textureSample' must only be called from uniform control flow
    let texture_color_1 = textureSample(sprite_texture, sprite_sampler, in_2.uv);
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

:151:19 note: called by 'draw_background' from 'fragment'
        let _e5 = draw_background(in);
                  ^^^^^^^^^^^^^^^^^^^

:147:5 note: control flow depends on possibly non-uniform value
    if _e3 {
    ^^

:146:23 note: parameter 'in' of 'fragment' may be non-uniform
    let _e3 = enabled(in.flags, BORDER);
```


## Solution

- call `textureSample` from outside the if. both branches are using the
same parameters
@hymm hymm deleted the fix-ui-aliasing branch August 12, 2024 03:29
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-Bug An unexpected or incorrect behavior 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.

Ui border is 1 pixel too large, or is getting aliased.
3 participants