-
-
Notifications
You must be signed in to change notification settings - Fork 598
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
Add rounded corners #229
Add rounded corners #229
Conversation
Codecov Report
@@ Coverage Diff @@
## next #229 +/- ##
==========================================
- Coverage 32.67% 30.69% -1.98%
==========================================
Files 46 46
Lines 8524 9128 +604
==========================================
+ Hits 2785 2802 +17
- Misses 5739 6326 +587
|
First of all, big thanks for spending time on this. It seems to me, at current stage, this pull request is fairly incomplete. Because it only adds rounded corners to the old xrender backend (we are moving away from the old backends). Also I am not sure if what's implemented here meets the criteria of #214 (that's for the issue reporter to decide, of course). As you said, Also, some nitpicks:
Many thanks. |
Thank you for working on this, @sdhand! That's an exciting update. I've tested your branch a bit and it works nice in most cases. But as @yshui already said it should support at least the "glx" backend apart from the old "xrender" backend. I'm not familiar enough with compton architecture so I hope that @yshui will have time to review your updates regarding this particular point and other technical details. What's about the criteria of #214:
I understand that it should be the trivial thing for someone from community or for myself to implement but it would be nice to have that feature included in this PR, because it should be possible to disable rounding for panels, docks and some other special windows. At least, it should make all the bounty bakers satisfied with the result. Other integration problems I've noticed so far:
|
@zezic window exclude/include rules should be pretty trivial, I'll add that in soon. I'm currently working on adding back the reg_include optimization. (Completely obscured areas of windows are currently drawn, to avoid corruption in the transparent corners. Fixing this requires me to properly calculate the region occupied by a window with a rounded corner). Getting this to work with the other backends will be challenging, but I do plan on doing so eventually. The window blur problem is likely slightly fiddly, but I should be able to solve it. |
Here's a list of what remains to do:
|
@sdhand yeah, I agree with you that exclude/include rules should be trivial. The implementation for other backends is not easy and the blur problem is also can be challenging though. |
78efcc7
to
afe7492
Compare
I've got a fix for both I could try and calculate a completely accurate window bound, but I'm not sure if this is worth the extra computational cost? |
@sdhand I just tested the latest changes. As you said, there are little non-blurred quads at window corners now. I think, that users who like to use big blur radius will be very unhappy to see such behavior. There is many of them. The difference between blurred area and non-blurred quad will be especially dramatic and noticeable after merging the kawase blur method from the tryone fork. That's my opinion from the end-user point of view. I don't know, maybe this operation is not so performance-heavy at all or maybe it's possible to cache something as long as corner radius is the same for all windows? Maybe @yshui will be able to suggest something because I have no enough knowledge of all those |
if you use a mask picture to achieve the rounded corners, you can cache the mask the same way we cache the shadow picture. |
Should this round shadows as well? |
@GaugeK ideally - yes. But for now it would be better to leave them as is to make the rounded corners feature available as soon as possible. |
After a while this seems to build up a fair bit of ram usage. |
Apologies for the slow progress on this, I've been away and without access to a computer. @yshui I am using a mask picture (current an I imagine I just need to compute the region for the bound separately, but unfortunately it doesn't seem like pixman provides a way for building a region from triangles or trapezoids. @GaugeK I've probably got a memory leak somewhere, will investigate. |
@sdhand hmmm, yes, the ignored region thing is a bit tricky. but i don't think you need to be completely precise since it's just an optimization anyway. maybe you can use the biggest rectangle that can fit inside the rounded window (or just any rectangle inside the window) as the bound for ignored region (and for damages, use the non-rounded rectangle)? |
@yshui currently I just use a rectangle the size of the window, with squares the length of the corner radius subtracted from each corner. Unless I am mistaken, the issue with this is the fact that this region is used to determine where to apply blur affects, meaning that a small area near each corner of a transparent window will not be blurred. (I personally don't think this is too noticeable unless you're looking for it, but @zezic did not seem to agree). |
@sdhand ah that's simple, you just blur the whole window, then use your mask to clip the blurred image before putting it on screen |
Oh I see, fantastic, I'll get that sorted. |
For porting to the new backends, I think it's best to add a |
When using |
I hope this gets implemented in the future, looks really cool! |
@Baitinq I would like to see this too. |
Hi! Any chance to see this soon? It looks really neat and seems fairly usable by now. |
I added a Example usage: Add the following lines to your configuration file in order to prevent Polybar and all windows of type
Pull request : sdhand#3 |
@sdhand Hi, just want to catch up. What is the current status of this PR? What is your plan with it? |
@sdhand you might want to edit this to make the exclude bit checked. |
Since @sdhand added me as a contributor to his fork I took the liberty of merging my latest commit into his fork so it now contains rounded corners code for all backends including In addition I also fixed the misc warnings to pass the tests because I couldn't take these red X's everywhere :-) I also added the rounded borders code that currently only works with the glx backends (old + experimental) with 2 configuration options to control the behavior, e.g:
I hope this is satisfactory and we can hopefully now merge the rounded corners code into the main branch in a future version (I read somewhere the plan is for v9 to include this code?). |
@ibhagwan Can you rebase your branch? I will take a look of the code when I have time. The change is sizable so it might take a while. |
@ibhagwan Also the commit history is a bit messy, it would be nice if you could re-organize them (e.g. squash relevant WIP commits together, merge fix commits into the commits they fix, etc.) That will help me a lot in reviewing the code. Thanks. |
@ysui I can certainly do so (rebase) on my own fork but my one contains the dual kawase blur methods from @tryone144 forks which we wanted to separate. Only the last 3 commits in @sdhand‘a fork are mine, even though @sdhand gave me permissions on his repo I’d prefer not to mess with his repo too much with rebasing and other git manipulations as I’m not sure what other work is dependent on his repo. Perhaps @sdhand is better suited to rebase his own repo? |
Yeay I'm exited to finally get rounded corners merged! |
x2 |
x3. But will there be a patch allowing rounded shadows? |
Alright, since I didn't get a reply, I will now be taking on the task of cleaning this PR up and getting this merged. |
I’m not sure if it’s supposed to work as I’m not sure how Herbsluftwm draws its borders, from the image it seems non standard (due to the inner border 90 degree angles) - in this case you can overwrite the WM border and have picom draw its own rounded border by using the |
Apologies but I don't really understand this syntax, I'm testing this with my terminal by doing |
You’re doing it correctly, 42 is the pixel width of the border, just a wild guess, perhaps herbsluftwm draws its border after picom thus overwriting the picom border? What happens if you set a bigger pixel width (just for testing), say 100 pixels? Can you test your Alacritty rule with the an exclude is your |
i've tested -- for me it works only for values btween 1 and 4-5 starting from 4-5 to 7 it works, but inner border radius is slighty smaller than expected and starting from 8 inner border radius dissapers i wasn't looking into the code, but seems to be it has smth to do with the formula for inner border shape UPD: ive used |
I'll have to look into it, here's how mine appears with border width set to 55, it does work but it also has the 90 degree internal angles, perhaps it's indeed the shader formula, I will have to look at the code. |
I believe all the features implemented for the legacy backends here exist in the So I will close this PR now, porting of this to the new backends is still in progress. |
@yshui |
@actionless Yes, implementing this for the experimental backends is still in progress. |
ah, sorry, i understood it like it's there but unstable |
Hello, I've been wanting to try the rounded corner features for my i3-gaps setup. So i compiled picom's As you can see from this screenshot, the i3 borders are truncated and I'm not quite sure how to fix it. The screenshot also contains my current rounded corner settings. Does anyone have any idea on why it doesn't work for me? thanks |
AFAIK, the main fork hasn’t implemented |
Fairly experimental. This only works with the xrender backend, and also won't work with transparent window frames.
Also, in order to properly render the transparent parts of the corner I had to tear out all the reg_ignore stuff. There may will be a better way of doing this (like calculating a region for each window that does not include the transparent area). I will look into this.
Fixes #214
Edit: I have noticed that my indentation is not consistent with the rest of the codebase, I will fix this tomorrow.