-
-
Notifications
You must be signed in to change notification settings - Fork 957
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
feat: add support for supercircular (e.g. squircles) window corner rounding #8943
Conversation
wasn't there an open issue to add this that we could close with this one? Anyways I'll review tomorrow it's way too late for me to review gpu code |
I guess yea |
This might result interesting https://iquilezles.org/articles/roundedboxes/ |
Hmmm, an SDF might be a pretty good way to do this. Although I don't think any of the solutions offered there work better than supercircles/superellipses here, since they either a) don't remove the discontinuity fully, or b) require a numerical solver to calculate. Now that I think about it, I don't think an exact SDF can possibly satisfy both of the above conditions, since any curve that removes the discontinuity must be at least third order, so finding it's SDF must necessarily involve solving a quintic (or higher order) equation, which requires a numerical solver because of the Abel–Ruffini theorem. Currently, I'm sort of unintentionally using an inexact SDF given by Edit: I should add that the SDF I am currently using is only inexact for the case of |
My bad if the force push wasn't done properly. I'm new to using git collaboratively. |
whoa calm down there nerd patrol what is all that math in my git repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at some point I should start passing structs to render functions the amount of args is getting ridiculous, lol.
Nice work :)
@polluxcodes I was talking (sorry for just throwing a link) about the Cubic sdf, which keeps the edges parallel but still removes the discontinuity in the derivative.
|
@leiserfg I think there is a small misunderstanding, I'm not making the whole window a single superellipse, I'm only replacing the corners with quarter-superellipses. This way the windows still have parallel edges, while also not having any discontinuities. The only benefits the cubic rounding could have is a nicer shape (I compared them on a graphing calculator, they're nearly identical if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest lgtm, needs a wiki mr
renderer: Add supercircular shadows and borders config: Add rounding_power to default and example configs rule: add `roundingpower` window rule
Squashed commits and removed fixup commit messages. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm tanks
More features is more features I guess, but this seems a little pedantic, no? Like I can barely tell the difference between these two rounding options, even with rounding set so high in the preview images... |
@Eclextic I agree the difference is fairly subtle, but I wouldn't say it's pedantic. I think the rounded corners on the right example look significantly better than the left, even at a glance, and though I can't necessarily speak for anyone else, given that this feature has been requested before (#3804) and the positive reactions from people to this PR, it seems at least a few others agree with me. Also, I'm not sure I would consider the rounding in the example to be all that high; I have it set to a pretty similar value in my actual hyprland config, though maybe it is just me. |
High => no normal person would set it higher than that.
Why don't we then just make that the default? Just remove the other rounding calculation honestly... Sure it does look "better" but to such a negligible degree, that I would assume, it can just become the way how rounding looks anyways. No confit option no nothing. Honestly I would advocate for it to just become the default rounding style. |
I think it is important to conserve behavior, especially when it comes to design choices, which are very personal. If we changed the default corner style, and provided no option to change it back, then I guarantee people will create issues asking to add an option to do so. There are plenty of situations where circular corners do look better, especially when placed in context of a desktop with other UI elements. For example, many people have rices with GTK bars that float slightly away from the edges of their screen and have rounded corners. CSS doesn't support squircle rounding easily. (You could maybe do it with |
Most of the lines in this MR are small style changes due to clang-format realigning members. 100 lines of code to add this won't hurt anyone, we already have 75k. |
Describe your PR, what does it fix/add?
This PR adds support for using generalized supercircles for rounding the corners of windows.
Supercircles are curves defined by equations of the form$x^n + y^n = R^n$ , which includes regular circles ($n=2$ ) and squircles ($n\approx4$ ). Many consider these to be a more aesthetically pleasing way to round the corners of rectangles, when compared to circles, since they don't introduce curvature discontinuities for $n\geq3$ . (See the image below.)
It adds a single configuration parameter$n$ and defaults to 2 to conserve existing behavior.
decoration:rounding_power
, which is used as the value ofHere is a comparison showing the difference between a rounding power of 2 (left, current behavior) and a rounding power of 4 (right):
Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)
This modification preserves existing behavior, although it may have a slight performance impact. I am open to benchmarking the code, but would need some tips on how to do so.
Is it ready for merging, or does it need work?
It should be ready for merging.