lxqt / lxqt-panel

The LXQt desktop panel
https://lxqt-project.org
GNU Lesser General Public License v2.1
180 stars 135 forks source link

Wayland taskbar support v2 #2043

Open gfgit opened 6 months ago

gfgit commented 6 months ago

Depends on #2041 (WM abstraction) Replaces #2031

This is an experimental implementation on taskbar, desktopswitch and showdesktop functionality for KWin Wayland compositor. It uses following protocols:

(Taken from plasma-wayland-protocols repository)

Code is inspired by libtaskmanager/waylandtasksmodel.cpp

Ideally the Wayland backend should have sub-backends targeting specific compositors when no standard protocol is available.

It also uses layer shell protocol to place panel on screen edge.

NOTE: panel auto-hide is not working yet

gfgit commented 2 months ago

My gnome boxes VM has messed up the git history. This is the moment to uninstall it! So the CMakeLists.txt is empty and wayland backend is not built. I'm trying to recover it from old commits.

gfgit commented 2 months ago

Fixed the issue. Now it also has desktop environment detection. If backend is run on non-Wayland platform it will return zero score. If it's run on non-KDE platform it will return 20 (So still better than dummy backend) If it's run on KDE it will return 100 so it will not be surpassed by other backends

gfgit commented 2 months ago

There are still many direct X11 calls with KX11Extras which produce warnings when on Wayland. These calls are maily KX11Extras::activateWindow() which should not need a replacement on Wayland because QWidget::activateWindow() should be enough. Isn't it enough also on X11?

Other calls are for panel strut but are already guarded. Last remaining seems to be keyboard indicator plugin

tsujan commented 2 months ago

These calls are mainly KX11Extras::activateWindow()

Actually, it's QWidget::activateWindow() that creates those useless warnings (in any app). They should be removed from Qt. Or the user can create ~/.config/QtProject/qtlogging.ini as

[Rules]
*.warning=false

to get rid of them.

stefonarch commented 2 months ago

It loads fine under kwin_wayland with the default config but not with my panel.conf. Under X11 all fine.

I replicated my 3 panels setup from scratch without some customcommand plugins and other tunings and also this loads fine, no idea what it could be.


        Stack trace of thread 1119762:
                #0  0x00007ad14ff51c90 n/a (libxcb_backend.so + 0x4c90)
                #1  0x00007ad14ff52b7d n/a (libxcb_backend.so + 0x5b7d)
                #2  0x00005848759abd39 n/a (lxqt-panel + 0x62d39)
                #3  0x00005848759ac6f2 n/a (lxqt-panel + 0x636f2)
                #4  0x00005848759940b2 n/a (lxqt-panel + 0x4b0b2)
                #5  0x00007ad159839c88 n/a (libc.so.6 + 0x25c88)
                #6  0x00007ad159839d4c __libc_start_main (libc.so.6 + 0x25d4c)
                #7  0x0000584875994945 _start (lxqt-panel + 0x4b945)

                Stack trace of thread 1119764:
                #0  0x00007ad1598a34e9 n/a (libc.so.6 + 0x8f4e9)
                #1  0x00007ad1598a5ed9 pthread_cond_wait (libc.so.6 + 0x91ed9)
                #2  0x00007ad15a0ce120 _ZN14QWaitCondition4waitEP6QMutex14QDeadlineTimer (libQt6Core.so.6 + 0x2ce120)
                #3  0x00007ad159d741ef n/a (libQt6WaylandClient.so.6 + 0x601ef)
                #4  0x00007ad15a0c9747 n/a (libQt6Core.so.6 + 0x2c9747)
                #5  0x00007ad1598a6ded n/a (libc.so.6 + 0x92ded)
                #6  0x00007ad15992a0dc n/a (libc.so.6 + 0x1160dc)

                Stack trace of thread 1119765:
                #0  0x00007ad15991c39d __poll (libc.so.6 + 0x10839d)
                #1  0x00007ad159d74257 n/a (libQt6WaylandClient.so.6 + 0x60257)
                #2  0x00007ad15a0c9747 n/a (libQt6Core.so.6 + 0x2c9747)
                #3  0x00007ad1598a6ded n/a (libc.so.6 + 0x92ded)
                #4  0x00007ad15992a0dc n/a (libc.so.6 + 0x1160dc)

                Stack trace of thread 1119763:
                #0  0x00007ad15991c39d __poll (libc.so.6 + 0x10839d)
                #1  0x00007ad15978492d n/a (libglib-2.0.so.0 + 0xbe92d)
                #2  0x00007ad159721fc5 g_main_context_iteration (libglib-2.0.so.0 + 0x5bfc5)
                #3  0x00007ad15a1a2cbd _ZN20QEventDispatcherGlib13processEventsE6QFlagsIN10QEventLoop17ProcessEventsFlagEE (libQt6Core.so.6 + 0x3a2cbd)
                #4  0x00007ad159f4f01e _ZN10QEventLoop4execE6QFlagsINS_17ProcessEventsFlagEE (libQt6Core.so.6 + 0x14f01e)
                #5  0x00007ad15a03a410 _ZN7QThread4execEv (libQt6Core.so.6 + 0x23a410)
                #6  0x00007ad15b67ae4e n/a (libQt6DBus.so.6 + 0x2de4e)
                #7  0x00007ad15a0c9747 n/a (libQt6Core.so.6 + 0x2c9747)
                #8  0x00007ad1598a6ded n/a (libc.so.6 + 0x92ded)
                #9  0x00007ad15992a0dc n/a (libc.so.6 + 0x1160dc)
                ELF object binary architecture: AMD x86-64

GNU gdb (GDB) 15.1
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /usr/bin/lxqt-panel...
(No debugging symbols found in /usr/bin/lxqt-panel)
[New LWP 1119762]
[New LWP 1119764]
[New LWP 1119765]
[New LWP 1119763]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib/libthread_db.so.1".
Core was generated by `lxqt-panel'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007ad14ff51c90 in ?? () from /usr/lib/lxqt-panel/backend/libxcb_backend.so
[Current thread is 1 (Thread 0x7ad1585b4940 (LWP 1119762))]
(gdb) where
#0  0x00007ad14ff51c90 in ?? () from /usr/lib/lxqt-panel/backend/libxcb_backend.so
#1  0x00007ad14ff52b7d in ?? () from /usr/lib/lxqt-panel/backend/libxcb_backend.so
#2  0x00005848759abd39 in ?? ()
#3  0x00005848759ac6f2 in ?? ()
#4  0x00005848759940b2 in ?? ()
#5  0x00007ad159839c88 in ?? () from /usr/lib/libc.so.6
#6  0x00007ad159839d4c in __libc_start_main () from /usr/lib/libc.so.6
#7  0x0000584875994945 in _start ()
gfgit commented 2 months ago

Inside your panel.conf which does not work, do you have the settings for preferred backend? Backend selection is done only first time then the prefferred one it's cached. It's possible that your old panel.conf was loaded under X first and then under wayland but still tries to load X backend. I need to rework the logic a bit.

stefonarch commented 2 months ago

Ok, I see thanks. But it looks like once set a backend this never changes again. Removed preferred_backend=/usr/lib/lxqt-panel/backend/libxcb_backend.sopreferred_backend=/usr/lib/lxqt-panel/backend/libxcb_backend.so, panel starts in kwin_wayland with preferred_backend=/usr/lib/lxqt-panel/backend/libkwin_wayland_backend.so,

Logged into x11 and the line stays and there's no taskbar, removed the line and restarted the panel and it readded the xcb one.

Shouldn't it be removed every time the when panel quits?

tsujan commented 2 months ago

Shouldn't it be removed every time the when panel quits?

The logic should be changed. The panel shouldn't need that key in its config file — putting it there and removing it on quitting is bad design.

gfgit commented 2 months ago

Right. I suggest another approach. We use preffered as first candidate. If it return score zero (totally incopatibility) then we trigger backend search. Then we do NOT store it inside config file, it has to be done manually by the user. This way we trigger search at every startup and only if needed user can cache a fixed backend. Anyway in case where you often switch between X and Wayland you cpuld have 2 separate config files to pass at startup.

stefonarch commented 2 months ago

Right. I suggest another approach. We use preferred as first candidate. If it return score zero (totally incompatibility) then we trigger backend search. Then we do NOT store it inside config file, it has to be done manually by the user.

IMO users shouldn't care or even know about this, it should be invisible. Is the search such an issue? I ignore most of the current approach, but kind of bash like below isnt' possible?

if x11 then xcb;
 elif kwin_wayland then plasma; 
elif wayfire then wayfire; 
else wlroots;
fi

Anyway in case where you often switch between X and Wayland you could have 2 separate config files to pass at startup.

IMHO a no-go, it was a workaround I had used many time ago with a panelloader.script...

tsujan commented 2 months ago

I completely agree with @stefonarch's last comment: The practical scenario is using a single config file, without the user needing to do anything when switching between different Wayland compositors or between Wayland and X11.

Actually, I still use @marcusbritanicus's old patch (based on @gfgit's previous PR) with some modifications, and I have no problem when switching between X11, LabWC, Wayfire, and kwin_wayland.

stefonarch commented 2 months ago

, I still use @marcusbritanicus's old patch

I'm using the same, but it's a dead end. Anyway, it's very nice to see all plugins working again on kwin_wayland.

knm100 commented 2 months ago

Tried this PR, found two issue:

  1. The window can't be minimized by clicking on the taskbar.
  2. The window of pcmanfm-qt can't be minimized by clicking on icon of "show desktop"
tsujan commented 2 months ago

The window of pcmanfm-qt can't be minimized by clicking on icon of "show desktop"

As far as I remember, that's an old problem of KWin and may not be related to this PR.

stefonarch commented 2 months ago

I think 1) is a bug (it works in labwc with the wlroots panel mentioned above) while yes, 2) is a kwin issue

gfgit commented 2 months ago

I've also noted issue 1). I can minimize from context menu but not if I click on icon button. Right now I don't have time to debug this...

stefonarch commented 2 months ago

Right. I suggest another approach. We use preferred as first candidate. If it return score zero (totally incompatibility) then we trigger backend search. Then we do NOT store it inside config file, it has to be done manually by the user. This way we trigger search at every startup and only if needed user can cache a fixed backend. Anyway in case where you often switch between X and Wayland you could have 2 separate config files to pass at startup.

@gfgit @marcusbritanicus @tsujan At the point we are stuck here we can use in startlxqt

...
sed -i  's/preferred_backend.*/preferred_backend=libxcb_backend.so/' $XDG_CONFIG_HOME/lxqt/panel.conf
lxqt-session

In https://github.com/lxqt/lxqt-wayland-session/blob/677f976aad8d35475ebb76664ace076b4ee1cfc3/startlxqtwayland.in#L113

sed -i  's/preferred_backend.*/preferred_backend=libkwin_wayland_backend.so/' $XDG_CONFIG_HOME/lxqt/panel.conf
kwin_wayland options lxqt-session

For all other compositors we need a dedicated or general backend to set, otherwise the panel will crash (coming from an x11 session) or have no taskbar (coming from kwin).

The advantage of this is that we can go ahead with this PR, having finally a wayland panel in git and make public the lxqt-wayland-session for testing.

We wouldn't need any backend search too afaik. Those both script can take care of all situations. Downside is that anyone who just wants to run lxqt-panel would need adding the setting manually or having a litte startup script for it.

I still would prefer that the panel itself just detects the compositor by itself if possible.

@marcusbritanicus Would it be easy to add the wlroots backend here?

Bluey26 commented 1 month ago

Hi, been installing all lxqt programs in a new setup and adding the labwc ones accordingly.

I have the following issue with this commit:

The taskbar is empty, there are not items in it. All the opened windows are not shown in there, but they are opened(appswitcher detects them). This seems to happen to me in both openbox(x11) and labwc(wayland).

I have been using the marcus-fork lxqt-panel version for a while in the old setup, but i am unable to compile it in the new one, because of this error (which is not present in this commit):


[ 70%] Building CXX object plugin-taskbar/CMakeFiles/taskbar.dir/taskbar_autogen/mocs_compilation.cpp.o
[ 71%] Building CXX object plugin-taskbar/CMakeFiles/taskbar.dir/lxqttaskbar.cpp.o
[ 71%] Building CXX object plugin-taskbar/CMakeFiles/taskbar.dir/lxqttaskbutton.cpp.o
/home/username/lxqt-panel/src/lxqt-panel/plugin-taskbar/lxqttaskbutton.cpp: In member function ‘virtual void LXQtTaskButton::contextMenuEvent(QContextMenuEvent*)’:
/home/username/lxqt-panel/src/lxqt-panel/plugin-taskbar/lxqttaskbutton.cpp:693:87: error: ‘SetLayer’ is not a member of ‘LXQtTaskBarBackendAction’
  693 |     layerMenu->setEnabled(mBackend->supportsAction(mWindow, LXQtTaskBarBackendAction::SetLayer));
      |                                                                                       ^~~~~~~~
/home/username/lxqt-panel/src/lxqt-panel/plugin-taskbar/lxqttaskbutton.cpp:697:28: error: redeclaration of ‘LXQtTaskBarWindowLayer currentLayer’
  697 |     LXQtTaskBarWindowLayer currentLayer = mBackend->getWindowLayer(mWindow);
      |                            ^~~~~~~~~~~~
/home/username/lxqt-panel/src/lxqt-panel/plugin-taskbar/lxqttaskbutton.cpp:695:28: note: ‘LXQtTaskBarWindowLayer currentLayer’ previously declared here
  695 |     LXQtTaskBarWindowLayer currentLayer = mBackend->getWindowLayer(mWindow);
      |                            ^~~~~~~~~~~~
make[2]: *** [plugin-taskbar/CMakeFiles/taskbar.dir/build.make:311: plugin-taskbar/CMakeFiles/taskbar.dir/lxqttaskbutton.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:1972: plugin-taskbar/CMakeFiles/taskbar.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
==> ERROR: A failure occurred in build().
    Aborting...

OS is archlinux, lastest packages and relatively "clean" instalation.

PS: I know the thing about the marcus-fork panel may not be correct in here, but i thought it was worth mentioning it in here.

PS2: This PR was installed using the arch build system, PKGBUILD relevant line is:

source=("git+https://github.com/lxqt/lxqt-panel#branch=work/gfgit/wayland_taskbar")

tsujan commented 1 month ago

I have been using the marcus-fork lxqt-panel version for a while in the old setup, but i am unable to compile it in the new one

The attached patch, which can be applied to the git master, is the revival of that fork.

lxqt-panel-wlroots-taskbar.zip

stefonarch commented 1 month ago

I think we should have a branch for it, at least for users who want to test now and eventually to have a fallback solution ( after fixing the switcher in kwin) in case the multiple-wlroots approach should delay too much.

tsujan commented 1 month ago

I think we should have a branch for it...

I agree with a fallback branch, if @marcusbritanicus doesn't have any objection. The above patch is his work for wlroots-based compositors, made compatible with the master branch.

I also added some small "fixes" for my personal use under LabWC/Wayfire, but I didn't include them in the patch.

Bluey26 commented 1 month ago

Thank to both of you @stefonarch @tsujan for the fast responses. I was a bit confused with the patch and stef helped me a bit, so now its working again!

gfgit commented 1 month ago

You are right. KWin backend makes sense on KWin only

marcusbritanicus commented 1 month ago

I think we should have a branch for it, at least for users who want to test now and eventually to have a fallback solution

@stefonarch @tsujan Sorry for the late reply, guys. I am working on two PRs.

  1. Backend selection
  2. Wlroots backend

While the code for both these are nearly ready, I need a couple of changes in 2075 for them to work. Until then am trying to make patches for various files to get it compile and run properly. Once that is done (in the next 12-18 hours, my sleeping time is included in this :wink:), everyone should find it easy to test x11, kwin, and wlroots backend.

@gfgit I am waiting for your response on this.

tsujan commented 1 month ago

I need a couple of changes in 2075 for them to work

Good things come to those who wait.

everyone should find it easy to test x11, kwin, and wlroots backend.

That's very great news. Looking forward to it :)

marcusbritanicus commented 1 month ago

@tsujan @gfgit https://github.com/LXQt-Marcus-Fork/lxqt-panel/tree/wlroots_backend_patched

This branch includes 2075, 2043. It also includes the 2 PRs I intend to open. On top of that, there are a few more changes - the ones I proposed in 2075 and 2043.

Note: This branch is only for testing, and will not see the light of day in the form of any PR.

stefonarch commented 1 month ago

As far as I've tested it is working fine in any environment, including workspace switcher and showdesktop in kwin_wayland. In the latter minimize/restore on click on the window button in the taskbar isn't working yet.

tsujan commented 1 month ago

@marcusbritanicus, thanks for the hard work! I confirm @stefonarch's observation, although his tests are much more comprehensive than mine.

I'm OK with a single PR containing all changes or two PRs, whichever you and @gfgit are more comfortable with.

gfgit commented 1 month ago

Rebased on top of backend_plugin branch with requested changes

gfgit commented 1 month ago

In the latter minimize/restore on click on the window button in the taskbar isn't working yet.

I think this is because when a window is deactivated we do not know previously active window so we return null WId. It's tricky to debug since putting breakpoints activates IDE window and creates an infinite loop.

gfgit commented 1 month ago

@stefonarch My intuition was correct. Avoiding null active window being reported by backend fixes minimize/restore on click on the window button in the taskbar. To confirm can you test last commit? I'm not sure it's the right approach but it doesn't seem to have side effects.

stefonarch commented 1 month ago

@gfgit Nice, I'll test later today, already compiled several panels ;) Btw do you have a dual monitor setup? As I remember the "send to monitor" menu item wasn't working in kwin (and wlroots too). Will look into that later too.

marcusbritanicus commented 1 month ago

@gfgit Nice work. Thanks... :) Now I can rebase my work on top of this (#2043), and we can proceed.

stefonarch commented 1 month ago

To confirm can you test last commit?

For some reason the taskbar didn't work for me in this branch, so I applied the change to Marcus' branch and can confirm it works (showing a quite terrible animation on default kwin...).

stefonarch commented 1 month ago

Testing dual monitor: kwin detects it nicely and asks for how to use it, kanshictl works also fine once configured its setting.

Under wlroots one item is greyed out:

immagine

Under kwin_wayland both items are indistinct active.

stefonarch commented 1 month ago

After some more testing windows button under kwin_wayland I found an issue (which isn't present in wlroots): if a window is minimized it's icon is still marked as active and it needs therefor 2 clicks to activate the window. If more windows are minimized only the one marked as active needs 2 clicks. Basically on an empty desktop still one window is marked as active in the taskbar.

marcusbritanicus commented 1 month ago

I remember the "send to monitor" menu item wasn't working in kwin (and wlroots too).

In wlroots, we do not have any mechanism to send a window to a given output/screen. This will be possible on Wayfire, and Sway. Perhaps you can test if Hyprland supports it (via Hyprctl)

stefonarch commented 1 month ago

So basically we need to grey them out in wlroots general backend? Hyprland can surely do it withhyprctl dispatch action. In Labwc I've a titlebar menu item

    <item label="→ Other monitor">
    <action name="MoveToOutput" direction="left" wrap="yes" />
    <action name="FitToOutput" />
  </item>

Afaik configurable title bar menus exists only there.

marcusbritanicus commented 1 month ago

So basically we need to grey them out in wlroots general backend?

That's right. Since there is no method in the protocol to do it, so wlroots will miss this feature.

Hyprland can surely do it withhyprctl dispatch action. In Labwc I've a titlebar menu item

So can wayfire. With the wayfire plugin, we will be able to easily implement it. In the same way, quite a few things can be done using the hyprland plugin.

gfgit commented 1 month ago

I use "KDE" check to test panel in a Plasma Wayland session. I don't think it hurts keeping it there...

gfgit commented 1 month ago

Currently prefferd_platform is emty by default and stays empty. If set it must contain the path to the plugin shared library and will take precedence over any other plugin regardless of current platform. But if preferred platform cannot be loaded or returns a zero score (total incompatibiluty) then we ignore it and fallback to normal search logic.

marcusbritanicus commented 1 month ago

If set it must contain the path to the plugin shared library and will take precedence over any other plugin regardless of current platform.

Why restrict to one plugin? This is cause issues, for example, if the user wants to use a single plugin, and is switching between different compositors.

Currently, in a single option you can support multiple compositors. Also, why give the path? We have support for LXQTPANEL_PLUGIN_PATH. Let the users specify all the paths using that env-var. We will simply assume that all the plugins are installed in the default location, and have a specific filename heuristic.

PS: All this is with the assumption that we will have users who will develop and use multiple custom plugins for different compositors.

marcusbritanicus commented 1 month ago

I use "KDE" check to test panel in a Plasma Wayland session. I don't think it hurts keeping it there...

@gfgit FYI: If you set preferred_backend=KWIN:kde_wayland,KDE:kwin_wayland,wayfire:wfipc,sway=wlroots, we only check if the plugin loads (for the correct platform), and don't check the backend score. So, those if statements will be "just there".

Additionally, you can see the allure of writing it this way: you can use one single config file, and support multiple platforms. In this hypothetical example, KDE, KWIN, wayfire, and sway will load the given plugins (if available). We will do an auto-detect for all other compositors.