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 blurry lines by aligning to pixel grid #4943

Merged
merged 19 commits into from
Aug 30, 2024

Conversation

juancampa
Copy link
Contributor

@juancampa juancampa commented Aug 10, 2024

I've been meaning to look into this for a while but finally bit the bullet this week. Contrary to what I initially thought, the problem of blurry lines is unrelated to feathering because it also happens with feathering disabled.

The root cause is that lines tend to land on pixel boundaries, and because of that, frequently used strokes (e.g. 1pt), end up partially covering pixels. This is especially noticeable on 1ppp displays.

There were a couple of things to fix, namely: individual lines like separators and indents but also shape strokes (e.g. Frame).

Lines were easy, I just made sure we round them to the nearest pixel center, instead of the nearest pixel boundary.

Strokes were a little more complicated. To illustrate why, here’s an example: if we're rendering a 5x5 rect (black fill, red stroke), we would expect to see something like this:

Screenshot 2024-08-11 at 15 01 41

The fill and the stroke to cover entire pixels. Instead, egui was painting the stroke partially inside and partially outside, centered around the shape’s path (blue line):

Screenshot 2024-08-11 at 15 00 57

Both methods are valid for different use-cases but the first one is what we’d typically want for UIs to feel crisp and pixel perfect. It's also how CSS borders work (related to #4019 and #3284).

Luckily, we can use the normal computed for each PathPoint to adjust the location of the stroke to be outside, inside, or in the middle. These also are the 3 types of strokes available in tools like Photoshop.

This PR introduces an enum StrokeKind which determines if a PathStroke should be tessellated outside, inside, or on the path itself. Where "outside" is defined by the directions normals point to.

Tessellator will now use StrokeKind::Outside for closed shapes like rect, ellipse, etc. And StrokeKind::Middle for the rest since there's no meaningful "outside" concept for open paths. This PR doesn't expose StrokeKind to user-land, but we can implement that later so that users can render shapes and decide where to place the stroke.

Strokes test

(blue lines represent the size of the rect being rendered)

Stroke::Middle (current behavior, 1px and 3px are blurry)
Screenshot 2024-08-09 at 23 55 48

Stroke::Outside (proposed default behavior for closed paths)
Screenshot 2024-08-09 at 23 51 55

Stroke::Inside (for completeness but unused at the moment)
Screenshot 2024-08-09 at 23 54 49

Demo App

The best way to review this PR is to run the demo on a 1ppp display, especially to test hover effects. Everything should look crisper. Also run it in a higher dpi screen to test that nothing broke 🙏.

Before:
egui_old

After (notice the sharper lines):
egui_new

@juancampa
Copy link
Contributor Author

juancampa commented Aug 10, 2024

The main challenge I'm seeing at the moment is that if the rasterization of the fill doesn't exactly match the rasterization of the stroke, you'll see a gap between the two 🤔. This only happens when rounding is enabled afaict.

Screenshot 2024-08-10 at 00 14 58

(zoom in)

Update: this artifact is actually already present in the current version and it’s almost imperceptible in most cases. We might be able to fix it later by adding a tiny fudge factor when shapes are filled and stroked but IMO it’s out of scope of this PR.

@juancampa juancampa changed the title Support stroke position (outside/middle/inside) to enable sharp strokes Sharp lines/strokes Aug 10, 2024
@juancampa juancampa changed the title Sharp lines/strokes Fix blurry lines Aug 11, 2024
@juancampa juancampa marked this pull request as ready for review August 11, 2024 19:30
Comment on lines -439 to -441
let border_padding = window_frame.stroke.width / 2.0;
// Add border padding to the inner margin to prevent it from covering the contents
window_frame.inner_margin += border_padding;
Copy link
Contributor Author

@juancampa juancampa Aug 11, 2024

Choose a reason for hiding this comment

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

As an added benefit, spacing calculations for windows got simplified because the stroke is 100% outside, whereas before it was half in, half out.

@polina4096
Copy link

polina4096 commented Aug 12, 2024

There is a visible gap where the line ends on the "after" screenshot (above the window cross).
Screenshot 2024-08-12 at 13 33 53

@juancampa
Copy link
Contributor Author

Screenshot 2024-08-12 at 15 45 44

There is a visible gap where the line ends on the "after" screen shot

Good catch. Fixed!

@invisageable
Copy link

Thanks the egoat!

@emilk emilk added visuals Renderings / graphics releated epaint labels Aug 26, 2024
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

This is great - thanks for fixing this!


I think we need to fix the gap between stroke and fill - it is very noticable:

gap

Compared to master:
no-gap

crates/egui/src/containers/frame.rs Outdated Show resolved Hide resolved
crates/egui/src/containers/panel.rs Show resolved Hide resolved
crates/egui/src/context.rs Outdated Show resolved Hide resolved
crates/egui/src/context.rs Outdated Show resolved Hide resolved
crates/egui/src/painter.rs Outdated Show resolved Hide resolved
crates/egui/src/painter.rs Outdated Show resolved Hide resolved
crates/epaint/src/stroke.rs Outdated Show resolved Hide resolved
crates/epaint/src/stroke.rs Outdated Show resolved Hide resolved
crates/epaint/src/stroke.rs Outdated Show resolved Hide resolved
crates/epaint/src/tessellator.rs Outdated Show resolved Hide resolved
let bbox = Rect::from_points(
&path
.iter()
.map(|p| translate_stroke_point(p, stroke).pos)
Copy link
Owner

Choose a reason for hiding this comment

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

I think running translate_stroke_point once on all the points (mutating the input) would be simpler, and also fix the gap between the stroke and the fill

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That did simplify things but it didn't completely close the gap. The issue is that feathering is slightly shrinking both the fill and stroke in opposite directions. The gap does disappear when feathering is disabled. I haven't looked into it but I'm wondering if you have any suggestions.

1px feathering
Screenshot 2024-08-28 at 21 49 07

5px feathering
Screenshot 2024-08-28 at 21 55 09

No feathering
Screenshot 2024-08-28 at 21 54 36

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I spent some time thinking about this and landed on: the fill should feather to the stroke color (instead of transparent), and vice versa.

I first thought that it would be okay to just grow the fill but that wouldn't work if the stroke is translucent.

@juancampa
Copy link
Contributor Author

juancampa commented Aug 29, 2024

I modified fill_closed_path to feather the edges to the stroke color (instead of transparent) and that gets the job done.

Screenshot 2024-08-29 at 11 35 31

@juancampa juancampa requested a review from emilk August 29, 2024 15:38
Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

This is an amazing improvement on low-dpi screens ❤️

@emilk emilk added the egui label Aug 30, 2024
@emilk emilk merged commit f2815b4 into emilk:master Aug 30, 2024
21 checks passed
@emilk emilk changed the title Fix blurry lines Fix blurry lines by aligning to pixel grid Sep 25, 2024
@emilk
Copy link
Owner

emilk commented Sep 25, 2024

486c pushed a commit to 486c/egui that referenced this pull request Oct 9, 2024
<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->


* Closes <emilk#4776>
* [x] I have followed the instructions in the PR template



I've been meaning to look into this for a while but finally bit the
bullet this week. Contrary to what I initially thought, the problem of
blurry lines is unrelated to feathering because it also happens with
feathering disabled.

The root cause is that lines tend to land on pixel boundaries, and
because of that, frequently used strokes (e.g. 1pt), end up partially
covering pixels. This is especially noticeable on 1ppp displays.

There were a couple of things to fix, namely: individual lines like
separators and indents but also shape strokes (e.g. Frame).

Lines were easy, I just made sure we round them to the nearest pixel
_center_, instead of the nearest pixel boundary.

Strokes were a little more complicated. To illustrate why, here’s an
example: if we're rendering a 5x5 rect (black fill, red stroke), we
would expect to see something like this:

![Screenshot 2024-08-11 at 15 01
41](https://github.com/user-attachments/assets/5a5d4434-0814-451b-8179-2864dc73c6a6)

The fill and the stroke to cover entire pixels. Instead, egui was
painting the stroke partially inside and partially outside, centered
around the shape’s path (blue line):

![Screenshot 2024-08-11 at 15 00
57](https://github.com/user-attachments/assets/4284dc91-5b6e-4422-994a-17d527a6f13b)

Both methods are valid for different use-cases but the first one is what
we’d typically want for UIs to feel crisp and pixel perfect. It's also
how CSS borders work (related to emilk#4019 and emilk#3284).

Luckily, we can use the normal computed for each `PathPoint` to adjust
the location of the stroke to be outside, inside, or in the middle.
These also are the 3 types of strokes available in tools like Photoshop.

This PR introduces an enum `StrokeKind` which determines if a
`PathStroke` should be tessellated outside, inside, or _on_ the path
itself. Where "outside" is defined by the directions normals point to.

Tessellator will now use `StrokeKind::Outside` for closed shapes like
rect, ellipse, etc. And `StrokeKind::Middle` for the rest since there's
no meaningful "outside" concept for open paths. This PR doesn't expose
`StrokeKind` to user-land, but we can implement that later so that users
can render shapes and decide where to place the stroke.

### Strokes test
(blue lines represent the size of the rect being rendered)

`Stroke::Middle` (current behavior, 1px and 3px are blurry)
![Screenshot 2024-08-09 at 23 55
48](https://github.com/user-attachments/assets/dabeaa9e-2010-4eb6-bd7e-b9cb3660542e)


`Stroke::Outside` (proposed default behavior for closed paths)
![Screenshot 2024-08-09 at 23 51
55](https://github.com/user-attachments/assets/509c261f-0ae1-46a0-b9b8-08de31c3bd85)



`Stroke::Inside` (for completeness but unused at the moment)
![Screenshot 2024-08-09 at 23 54
49](https://github.com/user-attachments/assets/c011b1c1-60ab-4577-baa9-14c36267438a)



### Demo App
The best way to review this PR is to run the demo on a 1ppp display,
especially to test hover effects. Everything should look crisper. Also
run it in a higher dpi screen to test that nothing broke 🙏.

Before:

![egui_old](https://github.com/user-attachments/assets/cd6e9032-d44f-4cb0-bb41-f9eb4c3ae810)


After (notice the sharper lines):

![egui_new](https://github.com/user-attachments/assets/3365fc96-6eb2-4e7d-a2f5-b4712625a702)
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to test and add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->


* Closes <emilk#4776>
* [x] I have followed the instructions in the PR template



I've been meaning to look into this for a while but finally bit the
bullet this week. Contrary to what I initially thought, the problem of
blurry lines is unrelated to feathering because it also happens with
feathering disabled.

The root cause is that lines tend to land on pixel boundaries, and
because of that, frequently used strokes (e.g. 1pt), end up partially
covering pixels. This is especially noticeable on 1ppp displays.

There were a couple of things to fix, namely: individual lines like
separators and indents but also shape strokes (e.g. Frame).

Lines were easy, I just made sure we round them to the nearest pixel
_center_, instead of the nearest pixel boundary.

Strokes were a little more complicated. To illustrate why, here’s an
example: if we're rendering a 5x5 rect (black fill, red stroke), we
would expect to see something like this:

![Screenshot 2024-08-11 at 15 01
41](https://github.com/user-attachments/assets/5a5d4434-0814-451b-8179-2864dc73c6a6)

The fill and the stroke to cover entire pixels. Instead, egui was
painting the stroke partially inside and partially outside, centered
around the shape’s path (blue line):

![Screenshot 2024-08-11 at 15 00
57](https://github.com/user-attachments/assets/4284dc91-5b6e-4422-994a-17d527a6f13b)

Both methods are valid for different use-cases but the first one is what
we’d typically want for UIs to feel crisp and pixel perfect. It's also
how CSS borders work (related to emilk#4019 and emilk#3284).

Luckily, we can use the normal computed for each `PathPoint` to adjust
the location of the stroke to be outside, inside, or in the middle.
These also are the 3 types of strokes available in tools like Photoshop.

This PR introduces an enum `StrokeKind` which determines if a
`PathStroke` should be tessellated outside, inside, or _on_ the path
itself. Where "outside" is defined by the directions normals point to.

Tessellator will now use `StrokeKind::Outside` for closed shapes like
rect, ellipse, etc. And `StrokeKind::Middle` for the rest since there's
no meaningful "outside" concept for open paths. This PR doesn't expose
`StrokeKind` to user-land, but we can implement that later so that users
can render shapes and decide where to place the stroke.

### Strokes test
(blue lines represent the size of the rect being rendered)

`Stroke::Middle` (current behavior, 1px and 3px are blurry)
![Screenshot 2024-08-09 at 23 55
48](https://github.com/user-attachments/assets/dabeaa9e-2010-4eb6-bd7e-b9cb3660542e)


`Stroke::Outside` (proposed default behavior for closed paths)
![Screenshot 2024-08-09 at 23 51
55](https://github.com/user-attachments/assets/509c261f-0ae1-46a0-b9b8-08de31c3bd85)



`Stroke::Inside` (for completeness but unused at the moment)
![Screenshot 2024-08-09 at 23 54
49](https://github.com/user-attachments/assets/c011b1c1-60ab-4577-baa9-14c36267438a)



### Demo App
The best way to review this PR is to run the demo on a 1ppp display,
especially to test hover effects. Everything should look crisper. Also
run it in a higher dpi screen to test that nothing broke 🙏.

Before:

![egui_old](https://github.com/user-attachments/assets/cd6e9032-d44f-4cb0-bb41-f9eb4c3ae810)


After (notice the sharper lines):

![egui_new](https://github.com/user-attachments/assets/3365fc96-6eb2-4e7d-a2f5-b4712625a702)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui epaint visuals Renderings / graphics releated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1-pixel separator lines are feathered
4 participants