baskerville / bspwm

A tiling window manager based on binary space partitioning
BSD 2-Clause "Simplified" License
7.81k stars 416 forks source link

New Feature: Rounded Corners #848

Closed Javyre closed 5 years ago

Javyre commented 6 years ago

I'd love to have rounded corners for non-maximized windows (for purely aesthetic reasons of course). I have a WIP fork for this and am wondering if you'd be interested in a PR for this.

This would require to compile with the libxcb-shape extension library but we can check for compatibility for the protocol extension at runtime.

Relevant links:

Javyre commented 6 years ago

The X shape extension protocol allows only masking the shape with 1bit deep pixmaps. So this results in jagged "rounded corners" This is not too noticeable with a high resolution display but I'm looking deeper into the issue...

See my entry on the xcb mailing list

Matrix89 commented 6 years ago

Does this work with borders?

Javyre commented 6 years ago

@Matrix89 of course of course

Matrix89 commented 6 years ago

Im sold. I was trying to implement something like this a year ago but didn't have time to learn xlib and the rest of things. I've also implemented dobule borders. Unfortunately my implementation was really glithy.

Javyre commented 6 years ago

I'm interested in seeing the code for that. I'm guessing it's something along the lines of having a custom pixmap set for the border: xcb_change_window_attributes(con, win, XCB_CW_BORDER_PIXMAP, &pixmap)

Matrix89 commented 6 years ago

I can't find the code, sorry. I must have lost it during a reinstall

Matrix89 commented 6 years ago

Can you share your code?

Javyre commented 6 years ago

The code is over on my fork (relevant branch is round_corners)

Check out the commit history and you'll see the changes. The first commit on develop is a fake-fullscreen feature that might eventually deserve its own PR but all the other commits are relevant to this issue.

Matrix89 commented 6 years ago

Im getting a seg fault here when launching firefox

Javyre commented 6 years ago

should be fixed with https://github.com/Javyre/bspwm/commit/faa4bfe928339318c9b0a504492699e138ae72be

Javyre commented 6 years ago

If anybody would like to know what the status is on the antialiasing issue I mentioned above see @psychon's response https://github.com/Javyre/bspwm/commit/94ba788802ea1d5d0e4419f7b9a357574b1e1451#r30542158

Basically, it wouldn't be possible without turning bspwm into a reparenting wm. And it would then require a compositor to be running.

Although I'm willing to implement this, it seems like a very intrusive change to bspwm and would like to hear from @baskerville before starting anything crazy.

baskerville commented 6 years ago

Basically, it wouldn't be possible without turning bspwm into a reparenting wm.

I'm fine with the idea of having anti-aliased rounded corners.

The preselection feedback will also need to become partially rounded.

Javyre commented 6 years ago

Great then! How do you see the antialiased corner option being implemented?

The way to do the antialiased corners is pretty different from the way my current code does it (i.e.: with a combination of the RENDER and SHAPE extension)

I'm thinking maybe keep the current code for users using no compositor and have a separate PR for the antialiased implementation. This would then require having border_radius (int) and border_antialias (bool)...

I will open a PR for some more fine grained code review discussion once I fix preselection feedback areas.

Javyre commented 6 years ago

Preselection feedback is now properly rounded https://github.com/Javyre/bspwm/commit/d925c6af0fb3d2acb1f5433a9f2649e3a96af136

Javyre commented 6 years ago

Non-gapless monocle is a weird case... I'm not sure whether it should or shouldn't have rounded corners since it has gaps around the window but border isn't drawn...

psychon commented 6 years ago

@Javyre Could you test what your code does with xeyes? I think it bspwm and xeyes might overwrite each other's changes to the window shape and whoever loses the race (=sets the shape last) wins. Which, I guess, would be xeyes, because bspwm sets the shape immediately while xeyes only does so after the ConfigureNotify event arrives.

Javyre commented 6 years ago

@psychon The behavior with a window that also modifies its shape is a bit chaotic. It does indeed lose the race but not all the time...

Most of the time bspwm overrides the shape.

But I don't think we should rely on the timing since that seems quite unstable and unpredictable. Is there a way to verify if the client is setting its shape already? Maybe a signal the program sends to say it's using shape-ext or something?

psychon commented 6 years ago

There is xcb_shape_select_input that allows you to ask the X11 server to send you ShapeNotify events when the shape of a window changes. Since you will also get this event for your own requests, you can have a counter per window (initialized to zero) that you increase each time you modify the shape and decrease each time you get a ShapeNotify event. If the counter becomes negative, "something else" is also changing the shape. However, this will only detect the situation after "the damage" is already done...

I just checked with xtrace / x11trace: xeyes sets a shape before mapping its window. Actually, it has to, to avoid some flickering. So, by the time you get a MapRequest, you can check via xcb_shape_query_extents if the window already has a shape and then not set any shape (answer has fields bounding_shaped and clip_shaped).

Edit: To clarify: The above has two ideas. Idea (1) only detects "uses shapes" after the shape was already corrupted. Idea (2) only detects "uses shapes" if a shape is set before the window is mapped, and does not detect anything if it starts using shapes only later. Your pick.

Javyre commented 6 years ago

The second option seems less hacky to me, but wouldn't I get back my own modified shapes from xcb_shape_query_extents for all calls except for the initial one?

Maybe if I stored the shape pixmaps and compared them to the reply to see if xcb_shape_query_extents is giving me back my own shapes... Would this be very memory/process-heavy? I would be storing two pixmaps per window. I would also be comparing pixmaps many times a second when the user is resizing with their mouse.

psychon commented 6 years ago

SHAPE does not give you access to the pixmap, I think, but only to a list of rectangles describing the shape. That's basically why I said that you would only do this check at the time you get a MapRequest event: If the window has a shape before it becomes visible, well, it is shaped. If it starts having a shape only later, then the race happens again.

Alternatively, but also uglily: You could track sequence numbers. Instead of xcb_shape_set_shape_whatever(..), you would do xcb_grab_server(); seq = set_shape().sequence; xcb_no_operation(); xcb_ungrab_server(). You save the values seq in an array and when a ShapeNotify event comes in, you check if its sequence number is in the array or not to check if you or "something else" changed the shape. However, this might still only detect the race after it already happened.

Javyre commented 6 years ago

Well I tried option 1 because it seems to me to be the simplest one to implement. But there are some issues:

So I guess either I'm missing something or this option should be discarded.

psychon commented 6 years ago

Sorry if you did not see my edit: https://github.com/baskerville/bspwm/issues/848#issuecomment-423175909

Idea (1) only detects "uses shapes" after the shape was already corrupted.

...and then would e.g. "make things work" after a resize (where the WM now stops touching the shape).

Javyre commented 6 years ago

Alright. Time to move on to Idea (2).

But with this method I would have to wait for the first mapping of a window before starting applying shapes to it?

Edit: I just realized I can apply the shape in the MapRequest handler after checking if it's safe. So there shouldn't be any visual glitches of the window waiting for its second remap to have a proper shape.

re1 commented 6 years ago

Basically, it wouldn't be possible without turning bspwm into a reparenting wm. And it would then require a compositor to be running.

I guess reparenting would also change the situation for some other issues, especially sided borders and title bars¹ as mentioned by @neeasade in #606.

¹ I am still unable to get colored title bars in bspwm

Javyre commented 6 years ago

@re1 Yes they would all be very possible.

Though whether the features fit into bspwm's goals/needs/scope mainly depends on @baskerville I think.

With reference to #606, most of the features described by @quite seem very complex... except for the {N,S,E,W} border widths. That would easily be implemented using the shape extension I use for rounded corners (no need for reparenting)

Javyre commented 6 years ago

Aaaaand Idea (2) works just fine! commit: https://github.com/Javyre/bspwm/commit/f31110db57eba30991a07992bd37887eaea86362

success (I'm guessing you can start to tell the look I'm going for with these rounded corners :wink:)

Now, this works just fine and properly detects it for the reasons @psychon explained. Just to be clear though, it has one drawback: It will only detect it if the window sets its shape before it's first mapping.

This a good enough fix?

Still TODO: decide on whether to detect support for shape extension at runtime or not. Judging by the rest of the bspwm code, I havent seen much error handling code reguarding xcb requests at all... I'd like to know what you think @baskerville

baskerville commented 6 years ago

I only do checks at runtime. And I don't remember checking for any extensions besides RandR and Xinerama. Please note that bspwm already uses xcb-shape to make the preselection feedback transparent, this dependency was introduced by 3b96688.

Javyre commented 6 years ago

Wow I completely missed that. (I think my initial fork was out of date and I rebased it later not noticing the new xcb-shape dependency) As you say, https://github.com/baskerville/bspwm/commit/3b96688d13ebc75e1a2883eda6efa74223359b37 doesn't do any checks so I guess I won't either.

Javyre commented 6 years ago

Almost ready for PR but found an issue with this download dialog in firefox this (the bottom corners are not rounded)

I suspect I'm missing a resize event somewhere (or maybe firefox just doesn't send one for some reason) and I'll start looking into it but I also want to know if anybody has some idea..

psychon commented 6 years ago

but I also want to know if anybody has some idea..

Shadows? Could it be that the window is larger than you think it is and the extra space is used to draw the window shadows?

Javyre commented 6 years ago

I killed my compositor and restarted firefox. Here is the result... As soon as i resize the window with my mouse (even by just one pixel) the bottom corners are properly rounded again. (this happens with & without my compositor running)

baskerville commented 6 years ago

The new preselection feedback should look like this:

bspwm-preselection

It spans the entire preselected node, and is filled with two transparent colors that reflect the split ratio and direction. The most opaque region is where the new window will appear.

Javyre commented 6 years ago

This involves setting alpha values of the feedback windows. Are we assuming a compositor is always running alongside bspwm?

(maybe I'm missing something and it isn't necessary?)

baskerville commented 6 years ago

Are we assuming a compositor is always running alongside bspwm?

From time to time, users complain about the preselection feedback being a solid color: the problem is that it was designed with transparency in mind.

What might work is to have one window for each region (with different instance names).

Users of compton can then set the transparency of each region with opacity rules.

Javyre commented 6 years ago

That could work but then bspwm wouldn't fully work "right out of the box" anymore..

Maybe if we detect if a compositor is running and have the old style (half-opaque presel feedback window) as a fallback?

Then because we know at runtime if a compositor is running, we can have nice things like this with opacity and also anti-aliased round corners.

baskerville commented 6 years ago

Maybe if we detect if a compositor is running and have the old style (half-opaque presel feedback window) as a fallback?

The p.f. should have two halves even when a compositor isn't running.

But never mind my request: I'll add this later, as an enhancement to the p.f.

The same remark goes for the ability to set irregular borders.

Then because we know at runtime if a compositor is running, we can have nice things like this with opacity and also anti-aliased round corners.

Compositors might set properties on the root window.

Having a look at compton's code might be a good way to check that hypothesis.

psychon commented 6 years ago

Compositors aquire the _NET_WM_SMs selection to "announce their presence". I don't know if any specs actually require this, but see e.g.: https://github.com/awesomeWM/awesome/blob/c21e8b980a9e9b25fe9874c4f3fbb96f103bfc5e/luaa.c#L176-L211 (Disclaimer: Written by me)

Javyre commented 6 years ago

I think this is ready for a PR then now.

I couldn't find the cause of the firefox issue. I thought it was because the window starts as floating and doesn't get resized like other windows right away for tiling but it turns out it really only happens with firefox's download dialog...

@psychon yeah from what I've seen looking around compositors' code this seems to be at least an unofficial standard

@baskerville is reparenting + antialiased borders still on the table for a future PR?

ascandella commented 6 years ago

I just tested the branch and didn't find any issues with firefox download windows. I'm running 0.9.5-29-g095c5bf

https://youtu.be/bNWc1ZOCwRU

Now perhaps I'm missing the issue, but I'm not seeing any of the bottom rounded corner issues OP is seeing:

Distro: Arch
pacman -Qi firefox
Name            : firefox
Version         : 62.0.3-1

Compositor: compton, but same results with/tithout

(apologies if these comments should be directed elsewhere, just trying to get this awesome feature in!)

gscreenshot_2018-10-20-062359

Javyre commented 6 years ago

@sectioneight the issue is with the dialog before you choose to "save file" Maybe this dialog didn't show up because you are saving an image on the page but you can try clicking download zip on the project main page and see what happens.

ascandella commented 6 years ago

@Javyre you're absolutely correct. Clicking a "download zip" on GitHub triggers the behavior you're seeing :|

Either way, let me know if you'd like some help with this (I recently quit my job to work on open source, and I love love LOVE bspwm)

Javyre commented 6 years ago

Well at this point I'm basically just waiting on @baskerville for a response on the PR and some of the other topics.

I'd be happy to work together with you for antialiased borders for example.

ascandella commented 6 years ago

Sounds like a fun project if you're willing to take on a noob... the last time I did graphics programming was GLSL in college...

baskerville commented 6 years ago

This post, from the author of 2bwm, might be helpful.

phuhl commented 6 years ago

@Javyre

I couldn't find the cause of the firefox issue. I thought it was because the window starts as floating and doesn't get resized like other windows right away for tiling but it turns out it really only happens with firefox's download dialog...

I found another case where it happens: My emacs in floating mode does this sometimes. When I resize I can find states where I have corners and states where I have not (it also only effects the bottom two corners).

gnaqvi commented 5 years ago

Is there any possibility for this PR to be merged any time soon?

bandithijo commented 5 years ago

some GTK and Qt application have this "firefox:Dialog" issue too when state=floating like "gnome-calculator", "spectacle". It can be emergency fix with add rectangle= rule. (^_^)

Another thing I have found, When I am using forking version from @Javyre on developer branch for rounded corner, everytime I click "fullscreen" button on web video player like YouTube, Vimeo, etc, it (web video player) did not fullscreen and make Firefox window state to fullscreen. After I change mode to Tiled, Firefox will be super lag and need to reopen. But it's fine with Google Chrome.

solenum commented 5 years ago

I also get the issue with firefox, can't seem to make any videos fullscreen and attempting to do so just bugs firefox out and requires me to restart it. Works fine in chrome though.

Anyone aware of a workaround for the time being?

lachlanshoesmith commented 5 years ago

Any update on whether or not this is being merged sometime?

BlueDrink9 commented 5 years ago

If this only has bugs in a couple of programs, and would be an option to turn on anyway, could it not be merged with warnings/caveats for those that use the affected programs?