awesome icon indicating copy to clipboard operation
awesome copied to clipboard

drawin: Add support for the desktop layer.

Open Aproxia-dev opened this issue 3 years ago • 12 comments

I've added support for drawins to be rendered on the desktop layer. this could be used for making desktop icon widgets, as well as wallpaper animations without directly interacting with the root window like i have done here:

https://user-images.githubusercontent.com/53254254/206877160-a6c1c61f-daab-40a9-8e79-a9c38b4b2f10.mp4

I've also updated the documentation.

Aproxia-dev avatar Dec 10 '22 22:12 Aproxia-dev

Codecov Report

:x: Patch coverage is 56.52174% with 10 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 90.98%. Comparing base (1239cdf) to head (11f5eff). :warning: Report is 99 commits behind head on master.

Files with missing lines Patch % Lines
objects/drawin.c 52.94% 8 Missing :warning:
stack.c 60.00% 2 Missing :warning:
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3750   +/-   ##
=======================================
  Coverage   90.97%   90.98%           
=======================================
  Files         900      900           
  Lines       57500    57521   +21     
=======================================
+ Hits        52309    52333   +24     
+ Misses       5191     5188    -3     
Flag Coverage Δ
gcov 90.98% <56.52%> (+<0.01%) :arrow_up:
luacov 93.71% <100.00%> (+0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
lib/wibox/init.lua 94.84% <100.00%> (+0.02%) :arrow_up:
objects/drawin.h 100.00% <ø> (ø)
stack.c 93.65% <60.00%> (-2.96%) :arrow_down:
objects/drawin.c 85.46% <52.94%> (-2.49%) :arrow_down:

... and 5 files with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Dec 10 '22 22:12 codecov[bot]

Hello,

Thanks for this. I think there is multiple ways to implement this feature. I think the most trivial would be to expose :raise() and :lower() for the wiboxes and set the "desktop" value to .type (see _NET_WM_WINDOW_TYPE_DESKTOP in https://specifications.freedesktop.org/wm-spec/1.3/ar01s05.html). I also think it has to be reparented somehow to the root window, but i am not sure of how that works or if it's even necessary. @psychon would know more, this is not my area of expertise. Conky, Mplayer and iDesk has the xlib equivalent and set some other hints too.

Also, more generally, the layering code, is you noticed with the if in the foreach, is not very flexible. There is, however, already a WINDOW_LAYER_DESKTOP in there. I think to be cleaner, the wibox code should use the layers like the client do. When combine, it also remove the need to expose the .desktop to Lua since it can use the existing properties and methods.

A much more labor intensive way to handling this would be to replace the hardcoded layers by recursive buckets. Where a bucket can contains either n other buckets or one client or one drawin/wibox. Then you can move buckets between parents and raise/lower/set their position in the parent bucket. This would be a more long term solution, but a lot more work. We have about a dozen known bugs with the current layering systems which would be solved by that. For example, it's not possible to "pair" a wibox and a client when raising them. This feature gets requested from time to time, mostly for people who implemented i3 like tabs. Another is when a client is on multiple tags, raising/lowering them affect the state of other tags (like the magnifier layout). This is the actual blocker for my dynamic layout system.

Elv13 avatar Dec 10 '22 23:12 Elv13

i don't think i quite understand the bucket approach. the layers are still a part of the freedesktop wm specs, aren't they? how would that still be kept? and how would you pair a client and a wibox, exactly? would it just be them having the same parent bucket?

i would honestly love to implement something like that, though. i believe that a z-index-like feature would be amazing and could have many features. if you could bring me up to speed with the details that you have worked out so far, i could try my hardest to implement something like that.

@Elv13

Aproxia-dev avatar Dec 11 '22 08:12 Aproxia-dev

Comment on the general approach (whether AwesomeWM devs want this or not is for them to decide):

I've added support for drawins to be rendered on the desktop layer.

I'd really prefer if this gets a different name. EWMH has the layers "desktop", "below", none-of-the-rest, "dock"&"above", "fullscreen". These layers are for client windows. AwesomeWM invented "ontop" for drawins. This has a special name to make it clear that this is its own layer and one cannot mix clients and drawins (drawin above a client above a drawin within the same layer). Not-ontop drawins also get their own layer. This makes the C implementation a lot easier (as @Aproxia-dev noticed, but @Elv13 apparently did not yet?)

Sadly, I do not have a good suggestion for a new name... What is the opposite of "ontop" that is not "below"? bottom?

Thanks for this. I think there is multiple ways to implement this feature. I think the most trivial would be to expose :raise() and :lower() for the wiboxes and set the "desktop" value to .type

You mean "trivial" from the Lua side, right? In C, this idea would be a nightmare to implement. The above would throw out the current way these orderings are managed and would mean that a lot of code needs to be touched.

I'm not sure whether that's a good request for someone's first contribution to a project (at least I did not find your name on https://github.com/awesomeWM/awesome/graphs/contributors ).

psychon avatar Dec 11 '22 09:12 psychon

Sadly, I do not have a good suggestion for a new name... What is the opposite of "ontop" that is not "below"? bottom?

https://www.wordhippo.com/what-is/the-opposite-of/on_top_of.html

How about beneath? There is also underside, but I do not like that name...

Adding a new layer certainly doesn't make the layering easier to understand, but that can be fixed by an addition to the docs (or just kept as-is).

psychon avatar Dec 11 '22 09:12 psychon

How about beneath? There is also underside, but I do not like that name...

Adding a new layer certainly doesn't make the layering easier to understand, but that can be fixed by an addition to the docs (or just kept as-is).

i do like beneath a lot, but underneath, or just under might be good too.

Aproxia-dev avatar Dec 11 '22 10:12 Aproxia-dev

I'm not sure whether that's a good request for someone's first contribution to a project (at least I did not find your name on https://github.com/awesomeWM/awesome/graphs/contributors ).

i made one small pr in late october this year, but it wasn't anything difficult. probably doesn't even count lol... #3734 with that said, i am willing to give it a shot. i might not be very experienced yet, but i think i could handle it with enough time and effort.

Aproxia-dev avatar Dec 11 '22 10:12 Aproxia-dev

AwesomeWM invented "ontop" for drawins. This has a special name to make it clear that this is its own layer and one cannot mix clients and drawins (drawin above a client above a drawin within the same layer).

i tried reading the freedesktop wm spec so thoroughly to not get the naming wrong and yet i somehow still missed that ontop is not there lmao the fact that clients can also be on the ontop layer confused me more than i'd like to admit

Aproxia-dev avatar Dec 11 '22 13:12 Aproxia-dev

the fact that clients can also be on the ontop layer confused me more than i'd like to admit

Hm... this is actually a good argument against my previous argument. AwesomeWM already has such a "clash". Meh...

@awesomeWM/write-access What do you think? How should this be implemented? Should this even be implemented?

psychon avatar Dec 11 '22 16:12 psychon

this patch looks quite small so i don't see a problem adding a new feature, but add some examples, to increase the tests' coverage

actionless avatar Dec 12 '22 02:12 actionless

So lets start with the parts we all agree on. 1) the problem is real, 2) it is worth fixing, 3) the current layering code is confusing. I made a pull request to start dismantling the old code without actually breaking anything (I think). It would have more iterations to remove the C part of ontop and move this 100% to Lua to the C part can handle the spec part and Lua the quality of life additions.

The PR is rather simple, even if it touches more code than I hoped. The old hardcoded spaghetti is converted to signals. Then Lua is responsible to assemble the final stack of drawin/clients. The current implementation copy pasted from the C code and ported to Lua inline. It still has most of the issues, but changing that would make the PR much bigger and much more complex.

From there, after a few more round of cleanup to split the wm-spec atoms from the API layers, I propose to add a hook to let individual layouts implement their own stacking. The tree based layouts like layout-machi, treesome and dynamite (#644) can use the bucket thing I mentioned above while the other layouts can leave it unimplemented and use the current code.

You mean "trivial" from the Lua side, right? In C, this idea would be a nightmare to implement. The above would throw out the current way these orderings are managed and would mean that a lot of code needs to be touched.

Well, I argue it is worth throwing away :P. I started the process with https://github.com/awesomeWM/awesome/pull/3751 This is actually nearly enough to implement raise/lower (like 20 lines away). But first things first, a review.

What do you think? How should this be implemented? Should this even be implemented?

Should it be implemented? Absolutely. This is actually not the first time this was mentioned like, in the past month. It's not even the second or third time. Literally, people have been hitting multiple problems on Discord. 2 of them related to the desktop layer, 1 for a drag and drop tooltip and one for creating a tabbar. Plus there are workaround in my dynamite module, layout-machine and even the main codebase.

The problems are very real and affecting many people. My main issue with the original patch is that it adds more hardcoded stuff rather than solve the larger issue. It also added more public APIs which might not survive fixing the larger problem.

@Aproxia-dev Sorry for the pain and extra steps. What do you think about #3751? Note that I moved x11_layers_keys.WINDOW_LAYER_DESKTOP to the top of the translator function rather than the bottom like the old C code. This should make it easier to implement your desktop layer. I did zero test on the code beside loading the default rc.lua and clicking on things. Would you like to test it and port this PR on top of the other one? It's mostly the same code, just more Lua and less C. I think we can avoid the .desktop property and piggy-back on the existing type property?

Elv13 avatar Dec 12 '22 07:12 Elv13

What do you think about #3751?

@Elv13 I've suggested some changes, but other than that it seems really good. The wallpaper animation that I've made actually works perfect with the changes I suggested, because the new PR actually implements layers for drawins, which wasn't the case with the old code there was only the default layer and the ontop layer. Due to this, I'd say that this PR could be left unmerged and we should move the work over to #3751.

Aproxia-dev avatar Dec 12 '22 11:12 Aproxia-dev