wayland: remove top level decorator and other wlrootisms.

[james-lawrence]

resolves obvious issues with running on a mutter wayland compositor.

fundamentally this allows druid applications to render on mutter using wayland.

fixes #2126

[james-lawrence]

@maan2003 this PR hammers out the remaining bugs I had for my own application (layershell reinitialization issues, slowness during application initialization) and gets druid rendering on mutter (gnome's wayland compositor).

I've covered what I think are the remaining issues in #1956 and filed issues. should mostly be minor issues and just unimplemented internal apis. most of the issues remaining I suspect also impact the X11 backend. good news everybody!

[james-lawrence]

@derekdreery your initial work was super helpful. thanks, we wouldn't be at this point without it.
@maan2003 thanks for all your patience and time spent reviewing.

[heavyrain266]

I think you should make this as optional features instead, xdg-decorations protocol is official extension used by other compositors, not just wlroots one. GNOME refuses to support it because they don't provide any server-side decorations (and uses deprecated kde decorations in gtk). If you completely remove this, then it will cause issues on e.g. KDE or any other compositor which uses custom server-side titlebars.

[i509VCB]

Per this https://github.com/linebender/druid/blob/master/druid-shell/src/backend/wayland/application.rs#L204-L207

You wrongly assert xdg_decoration is available.

The proper fix here is to gracefully handle said protocol not being available and draw csd when the protocol isn't available.

[i509VCB]

Oh and also the layer shell is not available on mutter, a runtime test for layers will be needed.

[PolyMeilex]

As previously mentioned, XDG Decoration is an official protocol, it should not be removed. It's lack should just be gracefully handled like any other non-core protocol. (Maybe exept xdg-shell).

Your layer shell change is correct tho, xdg decor should be handled the same way.

[richard-uk1]

@PolyMeilex Could you write a bullet point list of the changes you think should be made to this PR?

[PolyMeilex]

Could you write a bullet point list of the changes you think should be made to this PR?

Basically those review comments, besides that everything looks good.

XDG decor gives us free decoration whenever user forcefully unfullscreen the window, so removing this small bit of code would be a big regression.

[james-lawrence]

People are welcome to implement CSD/SSD if they so choose. its not the point of this PR. which is about layershell fixes and toplevel initial config for application startup improvements (removes a noticeable and unnecessary delay during startup).

but outside of that scope i'm not going to make additional changes. this PR is feature complete from my perspective.

My end goal clearly communicated before starting this work was to focus on wlroot. I've done what I can without expending much effort for KDE/Gnome and 90% of the work is done. but others will have to take it over the finish line.

feel free to implement the features you feel are missing.

@i509VCB - runtime test is already implemented when it checks the registry for the layershell interface.
for CSD/SSD outside of scope for this PR. feel free to implement it.

@xStrom just going to bump to get this merged in. seems like its attracting unrelated comments and wish list items vs relevant reviews.

[james-lawrence]

@heavyrain266

If you completely remove this, then it will cause issues on e.g. KDE or any other compositor which uses custom server-side titlebars.

yes. it was already causing issues which is why I removed it. it was incorrectly implemented logic. the goal wasn't to fix it for every platform; the goal is to clean it out so someone implement it correctly later without confusion from pre-existing (and broken) code.

[PolyMeilex]

Oh sorry, just by reading the PR and the linked issue I wrongful assumed that this PR is meant to fix something, but the point is just removal, not a fix. That was most likely discussed somewhere else, my bad. Have a nice day.

[james-lawrence]

@PolyMeilex understandable; title is confusing given the decorations being removed (since they're a general api as noted). basically the top level decorations predate my work. there where a bunch of partially implemented areas. I've fixed up most of them; but CSD and SSD are of little interest for my work and this allows the application to render its contents on mutter and wlroots. I'm assuming it'd also render on KDE.

i decided it was better to let someone start with a clean slate if they decided that area interested them vs having a broken partial implementation lying around causing issues and confusion.

[heavyrain266]

yes. it was already causing issues which is why I removed it. it was incorrectly implemented logic. the goal wasn't to fix it for every platform; the goal is to clean it out so someone implement it correctly later without confusion from pre-existing (and broken) code.

@james-lawrence Thanks for explaination, got confused by title and my goal with druid was to build shell for my compositor using layershell and bunch of apps which will require xdg decor for SSD 🥲

[james-lawrence]

@PolyMeilex it does fix an issue; right now in main there is a 1sish delay during startup and some issues with restoring layershell when a monitor restarts. the CSD/SSD stuff also caused bugs due to not being implemented properly.

@maan2003 is there any interest in pulling this in if I resolve the conflict? otherwise I'm liable to just unsubscribe and stop watching =)

[PolyMeilex]

I don't necessarily agree with the mindset, but I'm fine with the change btw, at least druid will start up without the need to edit the source code.
So I'm in favor of the merge 👍

[maan2003]

LGTM!

Here is my understanding:

We were using xdg_decoration, which is not supported by mutter. The implementation had bugs which causes delay during startup.
So for now, we are just removing the top level decorator.

I will be able to test in 2-3 days.

[PolyMeilex]

LGTM!

Here is my understanding:

We were using xdg_decoration, which is not supported by mutter. The implementation had bugs which causes delay during startup.
So for now, we are just removing the top level decorator.

I will be able to test in 2-3 days.

Those are two unrelated changes as far as I know.

Binding a protocol or even version that is not advertised by the compositor is treated as error in Druid. Therefore druid does not run at all on reference implementation Weston and Gnome's Mutter. As it expects optional xdg-decoration protocol and nonstandard layer shell. This PR removes xdg decor and makes layer shell optional and binds it only if available.
(It won't work properly even after this is merged because the same problem manifest in hardcoded protocol versions, won't run on Wston due to xdg shell version, and on Mutter it will lack input due to wlseat version, but it's a step in the right direction 👍)

Second part of the PR fixes toplevel initialization speed, by changing the order a bit. And I think it also fixed a deadlock in WlOutput code for me.

[richard-uk1]

People on this thread seem to know a lot more about Wayland than I do, and so I won't comment on the details, but it does seem to me that whichever way that discussion goes this PR is an improvement over what is already there, and so we should get it merged ASAP.

Nothing here stops us from modifying or reverting part of changes in the future, if it becomes necessary.

[james-lawrence]

@maan2003 all cleaned up and ready to go. thanks as always.