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 border is 1 pixel too large, or is getting aliased. #13503

Closed
ImDanTheDev opened this issue May 25, 2024 · 11 comments · Fixed by #13523
Closed

Ui border is 1 pixel too large, or is getting aliased. #13503

ImDanTheDev opened this issue May 25, 2024 · 11 comments · Fixed by #13523
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior

Comments

@ImDanTheDev
Copy link
Contributor

Bevy version

Main, commit 0ec6347

Relevant system information

  • Windows 11
  • Rust Nightly, 1.80.0 (8387315ab 2024-05-14)
  • Also tested on Rust Stable, 1.78.0 (9b00956e5 2024-04-29)
AdapterInfo { name: "NVIDIA GeForce RTX 2080 Ti", vendor: 4318, device: 7684, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "555.85", backend: Vulkan }

What you did

Create a NodeBundle with a 1 pixel wide border on all sides.

What went wrong

The border appears to be 1 pixel too large on all sides, or is getting aliased, causing it to blend into the node's background. I would expect a 1 pixel border to be exactly 1 pixel on the screen(see the 0.13.2 screenshot below).

This gist reproduces the issue on commit 0ec6347.
And this gist does not show the issue on 0.13.2.

Possible Fix

I'm not at all fluent with shader code, but while poking around in ui.wgsl, I found that on line 161, changing external_distance < internal_distance to external_distance > internal_distance fixes the issue. After doing that, the border appears exactly the same as the 0.13.2 screenshot below. I have no idea what the implications are for making that change, so it may very well break other things.

True scale:
true_scale

Scaled up to highlight issue:
scaled_up

Expected(using bevy 0.13.2):
image

@ImDanTheDev ImDanTheDev added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels May 25, 2024
@mockersf
Copy link
Member

could you try before and after #12725?

commit before: debcf3f
commit with that PR: 8da4fcb

@mockersf mockersf added A-UI Graphical user interfaces, styles, layouts, and widgets A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels May 25, 2024
@ImDanTheDev
Copy link
Contributor Author

The issue still exists on both of those commits, with debcf3f looking slightly different.
image

@ImDanTheDev
Copy link
Contributor Author

#12500, commit e7a31d0 has this issue.
The commit before that PR, 7c7d1e8, does not.

@hymm
Copy link
Contributor

hymm commented May 25, 2024

looking into this rn. I think there's a bug with how the sdfs are calculated. Visualizing external_distance(left) and internal_distance(right).

image

We see a darker red around the external_distance. It being darker is saying that the outside pixel is half in and half out of the border, but it should be bright red saying that the outside pixel is fully part of the border.

I think the calculations are off by half a pixel somewhere, but not sure how to fix it yet.

@hymm
Copy link
Contributor

hymm commented May 25, 2024

getting close. left is the border distance and right is the fwidth(border_distance). The border_distance looks correct. Notice that the borders have black pixels now and the darker red is gone. But the fwidth(border_distance) looks weird, which is making artifacts in the final rendering.

image

@hymm
Copy link
Contributor

hymm commented May 26, 2024

looks like something weird is going on on the diagonals of the sdf

image

@hymm
Copy link
Contributor

hymm commented May 26, 2024

I think I got it. Will do a little more testing tomorrow and then open a pr.
image

@ImDanTheDev
Copy link
Contributor Author

Thanks for working on this issue! That looks much better, but there is still 1px of the border bleeding into the middle on some sides of some of those squares. It is visible only on the top right black/white square, the 2 green/orange, and the 2 purple/yellow squares. Is this an artifact caused by the screenshot?

@hymm
Copy link
Contributor

hymm commented May 26, 2024

Not an artifact. Here's a blown up screenshot.

image

For this I changed border_distance to be

let border_distance = select(external_distance, -internal_distance, internal_distance <= 0);

Which is more correct than it was before, but probably still a little wrong. I'll see if I can come up with a better formula.

@hymm
Copy link
Contributor

hymm commented May 26, 2024

Darn above formula is still pretty wrong. The red area should be darkening again as it gets closer to inner border in the below screenshot.
image

@hymm
Copy link
Contributor

hymm commented May 26, 2024

Using

    let border_distance = select(max(external_distance, -internal_distance), -internal_distance, internal_distance <= 0);

got me
image
just some single pixel artifacts left in the corners.

github-merge-queue bot pushed a commit that referenced this issue May 27, 2024
# Objective

- Fixes #13503 
- Fix other various bugs I noticed while debugging above issue.

## 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](https://github.com/bevyengine/bevy/assets/2180432/4e475ad0-c9d0-4a40-af39-5f4422a78392)
- 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](https://github.com/bevyengine/bevy/assets/2180432/d7797e0e-e348-4daa-8646-554dc2032523)
Main:

![image](https://github.com/bevyengine/bevy/assets/2180432/4d46c17e-a12d-4e20-aaef-0ffc950cefe2)

- ran the `borders` and `rounded_borders` examples

---

## Changelog

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

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com>
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-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants