-
-
Notifications
You must be signed in to change notification settings - Fork 118
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
Wayland support for tasklist panel #1148
Conversation
Running under x11 I got a segfault with this backtrace on running
|
Since the first backtrace appeared to point to the show desktop button, I tried again (under x11 again)with that applet removed from the panel. Got another segfault, this time in
|
Retested on my very old Wayfire install-and it worked quite well, with an entirely functional taskbar on a wayfire install that dates all the way back to July 2019. Now we just need to make it work on x11 as well: the segfaults are x11-only it seems. Note that if both X11 and wayfire sessions are running at the same time on different tty's as usual since the dbus work you must launch the panel from the existing mate/x11 session and it will show up in (and only in) the wayfire session |
Ah! I changed the signature of a function not noticing it was being used as a callback. Two instances of the same mistake in the desktop button an workspace switcher. Now should be good to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This now works, gives me a working tasklist both in x11 and in wayfire (even my old version). As far as function is concerned, this looks good to go.
This in fact is the biggest difference in making a Mate on Wayland environment functional. The others (now that what I thought was the toughest part has a solution) is to port the other panel applets (including the indicator applet) to in-process, port our remaining tray icons to indicators or applets, and ideally write a native wayland desktop icon window for Caja. Since Wayland is always a compositor, we would basically need to take the existing caja navigation window that can already run under wayland, strip it down, and force it to be always on the bottom (which the desktop on xwayland is NOT)
@@ -361,6 +361,7 @@ void | |||
wayland_tasklist_set_orientation (GtkWidget* tasklist_widget, GtkOrientation orient) | |||
{ | |||
TasklistManager *tasklist = tasklist_widget_get_tasklist (tasklist_widget); | |||
g_return_if_fail(tasklist); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A space is missing before opening bracket.
I mean this is your code-style in this file :)
@@ -194,7 +194,7 @@ AC_ARG_ENABLE(wayland, | |||
# Check if we have gtk-layer-shell installed, and thus should build with Wayland support | |||
have_wayland=no | |||
if test "x$enable_wayland" != "xno"; then | |||
PKG_CHECK_MODULES(GTK_LAYER_SHELL, gtk-layer-shell-0, have_wayland=yes, [ | |||
PKG_CHECK_MODULES(WAYLAND, gtk-layer-shell-0 wayland-client, have_wayland=yes, [ | |||
if test "x$enable_wayland" = "xyes"; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, mate-panel depends on libwayland and gtk-layer-shell now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only if Wayland support is to be enabled:
if test "x$enable_wayland" != "xno"; then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question wasn't if wayland support is enabled or not. I always build with wayland and X11 support since wayland is supported.
But Ok, i have to add wayland-devel as build requires
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. Previously all of the Wayland interaction was done via GTK and GTK Layer Shell, but to support the foreign toplevel protocol the applet needs to use libwayland directly.
First commit is a bit misleading because |
The windows-list-applet does work under wayland with minimal function, other wncklet applets like show-desktop, workspace-switcher or windows-menu are disabled. |
Interesting, the applet shows all windows of all workspaces under wayfire. |
Only because it was disabled on X11 and so never created for it |
The foreign toplevel management protocol is not workspace-aware, so there is no way to limit which windows are shown. That will need to be addressed in a new protocol version. |
That's implemented in Wncklet, and will need to be re-implemented. This PR is just a usable starting point, and does not claim feature parity with X11. Since the tasklist currently doesn't work at all on Wayland, I don't see why being feature-complete would be required. I'll look into disabling the preferences options on Monday. |
Nope, this isn't true. All wnck applets were never disabled on X11.
Is my English really so bad? |
I agree that this is a starting point, and we need to get this fully working one piece at a time. No reason not to merge things that work, and in this case this PR makes what for me is the FIRST usable panel on Wayland, as you can minimize a window to the taskbar and restore it. That is the single most important part, and it works. If we cannot stop from getting the wrong menu on the taskbar buttons, it would be enough for now to simply disable showing any menu on them until the necessary code from wnck can be reimplemented later, in another PR. That's my judgement anyway |
You would think by now I would know the difference between X11 and Wayland, but clearly I do not. I will fix that commit message when I clean up this PR. I'll also try to disable the context menu if it's easy, but might not bother if that's tricky for some reason. I said I would do things today, which I did not do. Maybe I'll do them tomorrow. We'll see. |
65d1240
to
230d2ce
Compare
Work has been busy for a few days, but I'm back now. The only thing I changed in the rebase was that first commit message. I switched to the old comment style and suppressed the context menu for window buttons. Anything else needed? |
Any chance to disable the options in preferences Window for wayland? I think none of them can be used actually for wayland. Edit: Or maybe it is possible to grey out the Preferences menu-entry for wayland? Might be easier. But not a blocker for me if it is unneeded work or not possible, because only a few people will use this early stage of wayland support. |
I can confirm that context-menu is disabled on tasklist buttons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Maybe the 2 fixes for the C comments can be squashed before merging?
Done |
Anyone else like to approve it after latest changes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thank you |
Just tested this on master as merged, works quite well in my old wayfire install, and on x11 |
This uses the
wlr-foreign-toplevel-management-unstable-v1.xml
protocol which is supported by Sway >1.5 and Mir >2.1. The task list is a lot more basic than the one provided by wncklet (no right-click menu, no grouping, etc) but forking wncklet and porting it to Wayland would be more work. I also fixed the other wncklet applets so they wouldn't crash on Wayland, but since they don't have a useful implementation I didn't enable them. This is an update to a previous iteration of the same idea, which has been what themate-wayland
snap has been using.