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

StyleBoxFlat antialiasing not applied with corner radius 0 #87226

Open
lostminds opened this issue Jan 15, 2024 · 11 comments
Open

StyleBoxFlat antialiasing not applied with corner radius 0 #87226

lostminds opened this issue Jan 15, 2024 · 11 comments

Comments

@lostminds
Copy link

Tested versions

4.2.1.stable

System information

macOS 14.2.1

Issue description

Screenshot 2024-01-15 at 17 29 16

StyleBoxFlat has an anti-aliasing setting that adds a little blurry fade around the edge, helping to make rounded corners look nicer. But as you can see above it also affects the straight parts of the border.

However, the issue I'm encountering is that the antialiasing only seems to be rendered if the StyleBox has at least one non-zero corner radius. So in the example above the border settings and antialiasing settings are exactly the same, but as you can see the border renders quite differently with a single 1 px corner radius difference to the right.

In the documentation it says
Antialiasing draws a small ring around the edges, which fades to transparency. As a result, edges look much smoother. This is only noticeable when using rounded corners or skew.
Which might imply that it was assumed antialiasing wouldn't make any difference on a sharp-edge stylebox, so the anitaliasing is just skipped. But as you can see this is not really true, and the current implementation makes it difficult to get the same appearance of styleboxes with and without rounded corners even if they have the same border and anti-aliasing settings.

What I would have expected would be to have antialiasing applied the same with all corner 0 radius and have the lines and corners in this case appear like the zero-radius corners in the right example above.

Steps to reproduce

  • Add a control with a StyleBoxFlat, for example a Panel

  • Set the theme override panel Stylebox to be a new stylebox and set it to have a black 1 px border and 0 corner radius with antialiasing on (turn it up a little if you want a clearer effect).
    This should now look like the box to the left above.

  • Zoom in so you can see the border rendering clearly and toggle one of the corner radius parameters between 1 and 0 and observe the entire edge vary in appearance.

Minimal reproduction project (MRP)

Any stylebox will do, I do not think this is renderer or project specific.

@Calinou
Copy link
Member

Calinou commented Jan 15, 2024

This is done by design to reduce CPU/GPU utilization, but I agree there should be a way to force antialiasing regardless (likely with a new boolean property) as it's sometimes desired for art direction or rotated StyleBoxFlats.

(This CPU/GPU utilization can be non-negligible on integrated graphics, Raspberry Pi, etc.)

@lostminds
Copy link
Author

Yes, I assumed as much, but there's already the option to disable anti-aliasing for performance or appearance reasons.

So to me it would feel a little strange to add another boolean you have to turn on to render antialiasing when you've already turned it on once. But I guess for compatibility it might still be the best option since antialiasing is on by default I think now, and starting to render it would change the appearance of all existing styleboxes with 0 radius and borders.

@Calinou
Copy link
Member

Calinou commented Jan 15, 2024

So to me it would feel a little strange to add another boolean you have to turn on to render antialiasing when you've already turned it on once.

We could change the antialiasing property's type from boolean to enum but since this is a property, I don't know if it can be done without breaking GDExtension compatibility. Implicit casting would do the trick otherwise if "Disabled" was 0 and "Enabled" was 1 (with the proposed "Force Enabled" being 2).

@lostminds
Copy link
Author

Implicit casting would do the trick otherwise if "Disabled" was 0 and "Enabled" was 1 (with the proposed "Force Enabled" being 2)

If this would be possible and not to much hassle this sounds like a good solution. But perhaps the 1 option (current default) should be called AUTOMATIC to indicate that it will turn on/off based of radius, leaving 2 to be just ENABLED (the new always enabled).

@YuriSizov
Copy link
Contributor

I think if AA + corner radius are a performance sink, we should document it. Making the toggle more complex doesn't seem like a good solution to me.

@Calinou
Copy link
Member

Calinou commented Jan 15, 2024

I think if AA + corner radius are a performance sink, we should document it. Making the toggle more complex doesn't seem like a good solution to me.

Even if performance isn't an issue, there's also the concern that using antialiasing makes the final result less crisp – it's no longer pixel-perfect. This happens even with perfectly straight lines. This would likely break a fair amount of themes in pixel art-styled projects, but also themes in non-game applications that would suddenly start looking softer than intended (including the editor).

@YuriSizov
Copy link
Contributor

@Calinou Okay, but what's the point in us forcefully disabling it? Can't users disable it themselves if the look is not what they desire?

@Calinou
Copy link
Member

Calinou commented Jan 15, 2024

@Calinou Okay, but what's the point in us forcefully disabling it? Can't users disable it themselves if the look is not what they desire?

StyleBoxFlat antialiasing is enabled by default, so making it apply to all StyleBoxFlats retroactively would break the look of many existing projects. This is bound to result in a lot of support questions for people upgrading to a newer version.

It would be different if it was disabled by default, but we chose to enable it by default back in 3.0.

@davthedev
Copy link
Contributor

StyleBoxFlat is drawn a lot of times in the editor - basically all widgets that draw rectangles use it!
If you draw one extra polygon when a single stylebox is rendered, this may translate into something in the order of hundreds or thousands more in the entire editor window.

I guess you may have a scaled-up display on a Retina or high DPI monitor, or your project is rendered at a scale factor greater than 1. In those cases, the antialiasing algorithm is not optimal regarding its intended effect.

I made a previous fix to ease off the unwanted blurriness in some situations (#76132) and the working principle is explained there.

The suggested option is a good idea. Could be a dropdown with "Optimized for UI" and "Accurate antialiasing" choices.

Another possibility I thought about is to make the antialiasing adaptative to the actual rendered size of the StyleBox instead of its original size before scaling, so that you would have a result similar to when you zoom in a vector drawing software like Inkscape. But this is a different debate and may be technically complex.

@lostminds
Copy link
Author

Another possibility I thought about is to make the antialiasing adaptative to the actual rendered size of the StyleBox instead of its original size before scaling, so that you would have a result similar to when you zoom in a vector drawing software like Inkscape. But this is a different debate and may be technically complex.

Yes, this sounds much more complex, or at least lower level. But in a sense a more "real" solution, since I've noticed the same issue with other antialiased rendering of lines using draw_line etc. Where enabling antialiasing will make the line appear wider at low widths with blurry edges. If this was changed so the anti-aliasing doesn't make the line appear wider it would allow the optimization of not rendering these extra triangles for completely vertical or horizontal lines, since the antialiasing should only be needed for diagonal lines.

@Calinou
Copy link
Member

Calinou commented Jun 10, 2024

Another possibility I thought about is to make the antialiasing adaptative to the actual rendered size of the StyleBox instead of its original size before scaling, so that you would have a result similar to when you zoom in a vector drawing software like Inkscape. But this is a different debate and may be technically complex.

Done in #92997 for the canvas_items stretch mode and content_scale_factor. That PR doesn't take Control scaling into account though, as it would require separating StyleBoxFlats into unique resources (different Controls that use the same StyleBoxFlat can be scaled differently).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants