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

Corner Radius Fix for GLX #1261

Closed
wants to merge 38 commits into from
Closed

Corner Radius Fix for GLX #1261

wants to merge 38 commits into from

Conversation

pijulius
Copy link
Contributor

This should fix the corner radius in the glx backend to be almost exactly the same as xrender.
Not that good at math myself but I think the problem lies behind the circle already being the right size just the center of it was positioned off a bit and that's why it looked a bit "rectanglish".

Please see attached screenshots.
corner-glxfix
corner-glxfix-zoomed

@pijulius pijulius changed the title Corder Radius Fix for GLX Corner Radius Fix for GLX May 18, 2024
@yshui
Copy link
Owner

yshui commented May 18, 2024

hmmm, thanks for the patch, but i want to understand this problem better.

what corner radius did you use to get these screenshots?

@pijulius
Copy link
Contributor Author

hi @yshui , for sure! I used:
corner-radius = 10

and tested with corner radius 1 too and indeed looks fine with that too.

@yshui
Copy link
Owner

yshui commented May 19, 2024

hmm, i wonder if the off by 1 error is actually here:

		if (rect_distance > 0.0f) { // <----
			c = (1.0f - clamp(rect_distance, 0.0f, 1.0f)) * rim_color;
		} else {
			float factor = clamp(rect_distance + border_width, 0.0f, 1.0f);
			c = (1.0f - factor) * c + factor * border_color;
		}

maybe it should be

		if (rect_distance > -0.5f) { // <----
			c = (0.5f - clamp(rect_distance, -0.5f, 0.5f)) * rim_color;
		} else {
			float factor = clamp(rect_distance + border_width, 0.0f, 1.0f);
			c = (1.0f - factor) * c + factor * border_color;
		}

?

@pijulius
Copy link
Contributor Author

Hmmm, I tried that change but it makes shadows disappear, ok, not disappear but to see them I need to set shadow-opacity to 0.9 or so, so it affects the shadow in big way. The -1 I added doesn't seem to affect the shadow at all.

@yshui
Copy link
Owner

yshui commented May 19, 2024

So my current thought is this: texcoord is at center of pixels (I think this is the case for fragment shaders), so rect_distance == 0 means the pixel center is right at the edge of the circle, which means it should be color at approx. 50%. But we are currently coloring it 100%

This adds support for desktop switching animations by keeping track of _NET_CURRENT_DESKTOP atom on the root window. 

As far as I understand this atom is set by window managers and so if it changes we can know that it's a desktop switch happening. Unfortunately window manager may need to set this BEFORE hiding/showing windows so we can animate correctly, me personally using FluxBox and this quick change pijulius/fluxbox@83ee4db  makes it work just fine.

It adds the following animation triggers:
* workspace-out
* workspace-out-inverse
* workspace-in
* workspace-in-inverse

Unfortunately had to add inverse variables too as you may navigate to the next workspace from for e.g. 1st to 2nd but you may also go to 2nd from 1st and in that case the animations have to be totally different.

Here is a config example for switching workspace:

animations = ({
    triggers = ["workspace-out"];
    offset-y = {
        timing = "0.2s cubic-bezier(0.21, 0.02, 0.76, 0.36)";
        start = "0";
        end = "-window-height";
    };
    shadow-offset-y = "offset-y";
    opacity = {
        timing = "0.2s linear";
        start = "window-raw-opacity-before";
        end = "window-raw-opacity";
    };
    blur-opacity = "opacity";
    shadow-opacity = "opacity";
},
{
    triggers = ["workspace-out-inverse"];
    offset-y = {
        timing = "0.2s cubic-bezier(0.21, 0.02, 0.76, 0.36)";
        start = "0";
        end = "window-height + window-y";
    };
    shadow-offset-y = "offset-y";
    opacity = {
        timing = "0.2s linear";
        start = "window-raw-opacity-before";
        end = "window-raw-opacity";
    };
    blur-opacity = "opacity";
    shadow-opacity = "opacity";
},
{
    triggers = ["workspace-in"];
    offset-y = {
        timing = "0.2s cubic-bezier(0.24, 0.64, 0.79, 0.98)";
        start = "window-height + window-y";
        end = "0";
    };
    shadow-offset-y = "offset-y";
    opacity = {
        timing = "0.2s linear";
        start = "0";
        end = "window-raw-opacity";
    };
    blur-opacity = "opacity";
    shadow-opacity = "opacity";
},
{
    triggers = ["workspace-in-inverse"];
    offset-y = {
        timing = "0.2s cubic-bezier(0.24, 0.64, 0.79, 0.98)";
        start = "-window-height";
        end = "0";
    };
    shadow-offset-y = "offset-y";
    opacity = {
        timing = "0.2s linear";
        start = "0";
        end = "window-raw-opacity";
    };
    blur-opacity = "opacity";
    shadow-opacity = "opacity";
})
This adds the possibility to define the frame opacity but also include all colors in the window that match the frame color and make those also transparent. It even supports the possibility to define the tolerance for color difference between the frame and other parts of the window and make those or gradually more opaque or also transparent just like the frame.

NOTE: tested with flat window frame so not sure how it will work with gradient frames and also only added for GLX atm, lets see if it's interesting enough to be included in the core or otherwise will just keep it for myself.

It adds the following config options:

# Enable frame opacity for colors that match the frame
frame-opacity-for-same-colors = true;

# Tolerance for similar colors (0 exact match, 1 all colors, default 0.5)
frame-opacity-for-same-colors-constraint = 0.5;

# Make different colors opaque by a factor of x (default 5)
frame-opacity-for-same-colors-multiplier = 5;

and for them to work you will have to active frame opacity for e.g.
frame-opacity = 0.7;

With these options you can now have blurred transparent frame + menubar + toolbar or on popup like File Open have transparent window background but list of files be opaque and so on.
@pijulius
Copy link
Contributor Author

FYI: have now fixed rounded corners for the mask too, please see:
Fix GLX corner radis for the mask too

Also added this new feature:
Add possibility for frame opacity to include menubar/toolbar and so on

screenshot-20240526123536

not sure what you think about it, if you like it at all can work on formatting and other backends too but for now to showcase and for myself this was enough to achieve what I wanted.

@frebib
Copy link

frebib commented Aug 5, 2024

Are we likely to see this, or something like it land in master at some point @yshui?

@absolutelynothelix
Copy link
Collaborator

@frebib, if you're looking for smoother rounded corners you can also try my squircles branch, see #609 (comment) (no eta of landing in the next branch, i guess it has some demand but there are open questions about how it should be implemented in production and missing implementation for the xrender backend).

@yshui
Copy link
Owner

yshui commented Aug 5, 2024

Yes, I want to have this fixed in next. Just haven't gotten around to it yet.

@yshui
Copy link
Owner

yshui commented Aug 6, 2024

@pijulius hi, can you rebase your branch on top of next? thanks.

@pijulius
Copy link
Contributor Author

pijulius commented Aug 6, 2024

hi @yshui not sure what you mean by that, I have updated my repo to the next and all my changes are now up to date with your repo. If there is something I need to change please let me know and will get onto it asap.

Please note, there are only 3 main changes in my repo:

  1. Desktop Switching Animation support using _NET_CURRENT_DESKTOP atom
  2. Corner radius "fix" (not sure if it's correct but so far haven't noticed any problems at all and do look waaay better)
  3. Possibility for frame opacity to include menubar/toolbar and so on
    Corner Radius Fix for GLX #1261 (comment)
    968112a

Please feel free to use any of these in the core as you wish, I don't mind if you don't pull but add the changes you find useful manually as the only thing that is important is the end result :)

@yshui
Copy link
Owner

yshui commented Aug 6, 2024

@pijulius hi, you updated your branch by merging next into it. here what we prefer is rebasing the branch on top of next, because that gives us a cleaner git history.

yshui pushed a commit that referenced this pull request Aug 12, 2024
Closes #1261

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
Co-authored-by: Istvan Petres <pijulius@users.noreply.github.com>
@yshui yshui closed this in 0585e31 Aug 12, 2024
@pijulius
Copy link
Contributor Author

hi @yshui not sure what you did as I see you added the corner radius fix of your own but unfortunately there are some problems:

  1. if you set corner-radius = 0 all the windows will remain fully transparent and so when you start picom you won't see anything at all on your screen
  2. Shadows have disappeared, not sure what is causing this but you don't have any shadows at all now

Please note: this is all using current next brunch directly from your repo and using the picom.sample.conf from it too so no modifications at all nor any custom config settings.

Also tested my fork of your repo and that works just fine with the above tests cases (using your own picom.sample.conf and changing as noted above) so it must be something within the latest 14 commits:
pijulius/picom@next...yshui:picom:next

@pijulius
Copy link
Contributor Author

have tracked it down to these changes:
0585e31

if you undo these changes everything seems to be working just fine.

@pijulius
Copy link
Contributor Author

pijulius commented Aug 13, 2024

hi @yshui
Don't want to bother you with this, you really do great work with picom, love all the animations, the scripts, the stability of it and so on but re this case not sure why you are so against this quick fix.

It only adds two small numbers, nothing more, just two -1 and that's it and that makes it work in any situation without having to add any special case for 0 or non zero values and it also matches xrender corner radius too.

Your latest fix here:
6b858b8

brakes again other things, please see attached screenshots and also as you can see it doesn't make the bottom borders any different at all.

corners
corners-zoomed

This small popup window showing the position showcases the additional border around borderless windows
cornersborder

Really hope you reconsider.
Thank you again for all your hard work!

pijulius referenced this pull request Aug 13, 2024
Fixes #1311

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
@yshui
Copy link
Owner

yshui commented Aug 13, 2024

why you are so against this quick fix.

because it's technically wrong to subtract from inner_size since that moves the center of the circle. obviously my solution is poorly thought out too, i should have put more thought into it, so i apologize.

@yshui
Copy link
Owner

yshui commented Aug 13, 2024

@pijulius try https://github.com/yshui/picom/tree/rounded-corner-anti-alias, i think this is the best option i can come up with.

@pijulius
Copy link
Contributor Author

pijulius commented Aug 13, 2024

@pijulius try https://github.com/yshui/picom/tree/rounded-corner-anti-alias, i think this is the best option i can come up with.

@yshui this is looking good! thank you!

ps: "since that moves the center of the circle." exactly what was trying to achieve :)

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

Successfully merging this pull request may close these issues.

4 participants