lxqt / lxqt-panel

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

Wlroots taskbar #2046

Closed marcusbritanicus closed 3 months ago

marcusbritanicus commented 8 months ago

This PR adds a generic wlroots backend support for the the taskbar and desktop-switcher plugins.

@gfgit: I took lxqt-panel:git, then merged #2041 and #2043. I hope this is what I was supposed to do. If something's amiss, please do let me know.

One of the issues we face is the wlroots backend does not support virtual desktops. However, various compositors (wayfire, sway, hyprland, etc) have their own mechanisms to provide this feature.

  1. swaymsg provides a very fine-grained control to handle windows and virtual-desktops on sway.
  2. Wayfire:git has a rudimentary IPC support (not as developed as swaymsg) with which we can develop something similar to swaymsg. Alternatively, I am developing three protocols that provide virtual-desktop and toplevel management support via protocols.
  3. Hyprthing is for Hyprland what swaymsg is for sway? I do not know much about this.

So it would be beneficial if we provide per-compositor support. As discussed in #2531, the best way forward is to develop a plugin interface for the wayland backend. Currently, I have the following structure in mind:

panel/backends/
  - wayland/
    - plasma/
      * lxqtbackendplasma.cpp
      * lxqtbackendplasma.h
    - wlroots/
      * lxqtbackendwlroots.cpp
      * lxqtbackendwlroots.h
    - wayfire/
      * lxqtbackendwayfire.cpp
      * lxqtbackendwayfire.h
    - sway/
      * lxqtbackendsway.cpp
      * lxqtbackendsway.h
    - hyprland/
      * lxqtbackendhyprland.cpp
      * lxqtbackendhyprland.h
    - lxqtwaylandbackend.cpp
    - lxqtwaylandbackend.h
  - xcb/
    - lxqttaskbarbackend_x11.cpp
    - lxqttaskbarbackend_x11.h
  - ilxqttaskbarabstractbackend.cpp
  - ilxqttaskbarabstractbackend.h
  - lxqttaskbardummybackend.cpp
  - lxqttaskbardummybackend.h

Few points:

  1. lxqtwaylandbackend.cpp should attempt to detect the DE and load the suitable plugin. The DE detection can be based on a user settings or XDG_CURRENT_DESKTOP.
  2. If we're using a protocol (say, like wlr-foreign-toplevel, or wayfire-desktop-shell), we should also attempt to verify that the compositor is advertising the said protocol.
  3. If the compositor-specific protocol/support is unavailable (for example, when using wayfire, if ipc plugin is not loaded or similar), we should fallback to wlroots.
  4. If wlroots (i.e. wlr-foreign-toplevel) is not available (for example, foregin-toplevel plugin is not loaded in wayfire), the default dummy wayland backend (as provided by lxqtwaylandbackend.cpp will be used).

Changes made:

  1. I have moved all the files from panel/backends/wayland/ to panel/backends/wayland/plasma
  2. panel/backends/wayland/wlroots implements the wlroots backend.
  3. Line 50/51 panel/lxqtpanelapplication.cpp to use only wlroots backend. (This needs to be fixed)

Current state:

  1. On a wlroots-based compositor, following works:
    • Listing all the currently open windows.
    • Click to focus
    • Add newly opened windows
    • Remove closed windows
    • desktop-switcher plugin (It will simply show only one virtual desktop named ("Desktop 1" with id "desktop-1")
  2. On a wlroots-based compositor, following does not work:
    • Grouping does not work properly
    • Window icons
  3. On a wlroots-based compositor, what will not work:
    • Move to output, move to desktop, shade, pin on top/bottom (No support from the protocol)
    • Anything related to virtual desktops (at least until ext-workspace-* support gets added)
  4. In it's current state, taskbar and desktop-switcher plugins will crash the panel when run on any non-wlroots compositor (for example, plasma).

Note: For the purpose of our discussion here, by "non-wlroots compositor", what I mean is a compositor that does not implement wlr-foreign-toplevel protocol.

cc: @stefonarch @tsujan

marcusbritanicus commented 8 months ago

@gfgit When I first compiled and installed lxqt-panel, I faced one major error:

AutoMoc subprocess error
------------------------
The moc process failed to compile
  "SRC:/plugin-taskbar/lxqttaskbarplugin.h"
into
  "SRC:/.build/plugin-taskbar/taskbar_autogen/EWIEGA46WW/moc_lxqttaskbarplugin.cpp"
Process failed with return value 1

Command
-------
/usr/lib/qt6/moc "-DLXQT_DATA_DIR=\"/usr/share\"" "-DLXQT_ETC_XDG_DIR=\"/etc/xdg\"" "-DLXQT_GRAPHICS_DIR=\"/usr/share/lxqt/graphics\"" "-DLXQT_MAJOR_VERSION=\"2\"" "-DLXQT_MINOR_VERSION=\"0\"" "-DLXQT_PANEL_VERSION=\"2.0.0\"" "-DLXQT_PATCH_VERSION=\"0\"" "-DLXQT_RELATIVE_SHARE_DIR=\"lxqt\"" "-DLXQT_RELATIVE_SHARE_TRANSLATIONS_DIR=\"lxqt/translations\"" "-DLXQT_SHARE_DIR=\"/usr/share/lxqt\"" "-DLXQT_SHARE_TRANSLATIONS_DIR=\"/usr/share/lxqt/translations\"" "-DLXQT_VERSION=\"2.0.0\"" "-DPLUGIN_DIR=\"/usr/lib/lxqt-panel\"" -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_FOREACH -DQT_NO_URL_CAST_FROM_STRING -DQT_SVG_LIB -DQT_USE_QSTRINGBUILDER -DQT_WIDGETS_LIB -DQT_XML_LIB -DWITH_DESKTOPSWITCH_PLUGIN -DWITH_FANCYMENU_PLUGIN -DWITH_MAINMENU_PLUGIN -DWITH_QUICKLAUNCH_PLUGIN -DWITH_SHOWDESKTOP_PLUGIN -DWITH_TASKBAR_PLUGIN -I/home/cosmos/Softwares/Projects/LXQt/lxqt-panel/.build/plugin-taskbar -I/home/cosmos/Softwares/Projects/LXQt/lxqt-panel/plugin-taskbar -I/usr/include/qt6/QtWidgets -I/usr/include/qt6 -I/usr/include/qt6/QtCore -I/usr/lib/qt6/mkspecs/linux-g++ -I/usr/include/qt6/QtGui -I/usr/include/lxqt -I/usr/include/lxqt/LXQt -I/usr/include/KF6/KWindowSystem -I/usr/include/qt6/QtDBus -I/usr/include/qt6xdg -I/usr/include/qt6/QtXml -I/usr/include/qt6xdgiconloader -I/usr/include/qt6xdgiconloader/4.0.0 -I/usr/include/qt6/QtSvg -I/usr/include/lxqt-globalkeys -I/usr/include -I/opt/aocc/include -I/usr/include/c++/13.2.1 -I/usr/include/c++/13.2.1/x86_64-pc-linux-gnu -I/usr/include/c++/13.2.1/backward -I/usr/lib/gcc/x86_64-pc-linux-gnu/13.2.1/include -I/usr/local/include -I/usr/lib/gcc/x86_64-pc-linux-gnu/13.2.1/include-fixed --include /home/cosmos/Softwares/Projects/LXQt/lxqt-panel/.build/plugin-taskbar/taskbar_autogen/moc_predefs.h --output-dep-file -o /home/cosmos/Softwares/Projects/LXQt/lxqt-panel/.build/plugin-taskbar/taskbar_autogen/EWIEGA46WW/moc_lxqttaskbarplugin.cpp /home/cosmos/Softwares/Projects/LXQt/lxqt-panel/plugin-taskbar/lxqttaskbarplugin.h

Output
------
/home/cosmos/Softwares/Projects/LXQt/lxqt-panel/plugin-taskbar/lxqttaskbarplugin.h:62:1: error: Undefined interface

Note that this was the first time I installed lxqt, which means that /usr/include/lxqt/ilxqtpanelplugin.h did not exist. Once I manually copied ilxqtpanel.h, ilxqtpanelplugin.h, lxqtpanelglobals.h and pluginsettings.h to /usr/include/lxqt this error vanished. It looks to me that moc is unable to locate ilxqtpanelplugin.h in the source tree.

Secondly, another problem which I faced was:

/usr/bin/ld: backends/xcb/liblxqt-panel-backend-xcb.a(lxqttaskbarbackend_x11.cpp.o): in function `LXQtTaskbarX11Backend::moveApplication(unsigned long long)':
lxqttaskbarbackend_x11.cpp:(.text+0x209d): undefined reference to `NETRootInfo::moveResizeRequest(unsigned int, int, int, NET::Direction, unsigned char, NET::RequestSource)'
/usr/bin/ld: backends/xcb/liblxqt-panel-backend-xcb.a(lxqttaskbarbackend_x11.cpp.o): in function `LXQtTaskbarX11Backend::resizeApplication(unsigned long long)':
lxqttaskbarbackend_x11.cpp:(.text+0x21ef): undefined reference to `NETRootInfo::moveResizeRequest(unsigned int, int, int, NET::Direction, unsigned char, NET::RequestSource)'
collect2: error: ld returned 1 exit status

I ran sudo ldconfig as per @stefonarch's suggestion, but that did not work. So, currently, I have commented out the offending two lines in my copy. I have two versions of KWindowSystem: 6.0.0 and 5.115.0. Both versions have this function. So I am at a loss to understand why this error pops up.

Thirdly and lastly, I was testing lxqt-panel (wlroots-taskbar edition) on embedded kwin_wayland: XDG_CURRENT_DESKTOP="LXQt:Plasma:KDE:kwin_wayland" kwin_wayland -- lxqt-panel --config ~/.config/lxqt/panel.conf I noticed a peculiarity. Your code depends on org_kde_plasma_window_management interface. But to my great surprise, I noticed that this particular interface is not advertised by kwin_wayland!! This might be the reason why the taskbar plugin does not work on kwin_wayland even though it's loaded.

stefonarch commented 8 months ago

Issues I see

Multiple click behavior issues:

https://github.com/lxqt/lxqt-panel/assets/10681413/a1235abe-f927-4f0b-820a-1e89e299073a

https://github.com/lxqt/lxqt-panel/assets/10681413/20d1eefb-76c0-4e1e-9fb2-b379c2921b60

marcusbritanicus commented 8 months ago

sway: In future on sway minimize/restore could be greyed out, there is no issue but at the moment

Alternatively, we can send minimized views to scratchpad.

the cursors hovers the bar all buttons get inactive state, not bad but strange

I too noticed this. But not much can be done about it. On sway, the view with cursor has focus, and is activated as well. So when you move the cursor to the panel, it gains focus (because you've asked for it in layer-shell), and gets activated, meaning, the rest of the views lose their activation status.

Edit: Upon reflection, this looks like a lxqt-panel bug. It does not need keyboard focus except in certain cases. But I think it requests keyboard-interaction on demand.

gfgit commented 8 months ago

Thirdly and lastly, I was testing lxqt-panel (wlroots-taskbar edition) on embedded kwin_wayland: XDG_CURRENT_DESKTOP="LXQt:Plasma:KDE:kwin_wayland" kwin_wayland -- lxqt-panel --config ~/.config/lxqt/panel.conf I noticed a peculiarity. Your code depends on org_kde_plasma_window_management interface. But to my great surprise, I noticed that this particular interface is not advertised by kwin_wayland!! This might be the reason why the taskbar plugin does not work on kwin_wayland even though it's loaded.

org_kde_plasma_window_management is a restricted interface granted only to privileged clients. To be one:

  1. .desktop file must have absolute path in its Exec= field. I've just pushed a commit to auto generate this path based on CMake variables.
  2. .desktop file must contain X-KDE-Wayland-Interfaces=org_kde_plasma_window_management
  3. .desktop file must be in canonical locations, meaning the /etc/xdg/autostart does not count
  4. .desktop file's OnlyShowIn= must be compatible with current XDG_CURRENT_DESKTOP. Maybe we can add an Hidden=true to then hide it from applications menu.
gfgit commented 8 months ago

Note that this was the first time I installed lxqt, which means that /usr/include/lxqt/ilxqtpanelplugin.h did not exist. Once I manually copied ilxqtpanel.h, ilxqtpanelplugin.h, lxqtpanelglobals.h and pluginsettings.h to /usr/include/lxqt this error vanished. It looks to me that moc is unable to locate ilxqtpanelplugin.h in the source tree.

I had this issue but I don't remember how I solved it rigth now. Will check

stefonarch commented 8 months ago

Maybe we can add an Hidden=true to then hide it from applications menu.

It's already hidden because it is a LXQt-module which are hidden by default afaik.

stefonarch commented 8 months ago

I didn't notice it until now but the panel dumps core, sometimes several times, on session start on wlroots-based compositors when taskbar-plugin is present and some applications are in autostart. https://github.com/lxqt/lxqt/discussions/2547#discussioncomment-8974141

Core was generated by `/usr/bin/lxqt-panel'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x0000654c32139343 in ?? ()
[Current thread is 1 (Thread 0x7f3cb2f65940 (LWP 1795))]
(gdb)
(gdb) where
#0  0x0000654c32139343 in ??? ()
#1  0x00007f3cb4790ca9 in ??? () at /usr/lib/libQt6Core.so.6
#2  0x0000654c3213fb44 in ??? ()
#3  0x00007f3cb4790ca9 in ??? () at /usr/lib/libQt6Core.so.6
#4  0x0000654c3213d6c8 in ??? ()
#5  0x00007f3cb5ce3596 in ??? () at /usr/lib/libffi.so.8
#6  0x00007f3cb5ce000e in ??? () at /usr/lib/libffi.so.8
#7  0x00007f3cb5ce2bd3 in ffi_call () at /usr/lib/libffi.so.8
#8  0x00007f3cb60f2645 in ??? ()
    at /usr/lib/libwayland-client.so.0
#9  0x00007f3cb60f2e73 in ??? ()
    at /usr/lib/libwayland-client.so.0
#10 0x00007f3cb60f313c in wl_display_dispatch_queue_pending ()
    at /usr/lib/libwayland-client.so.0
#11 0x00007f3cb616198e in ??? ()
    at /usr/lib/libQt6WaylandClient.so.6
#12 0x00007f3cb477c2c7 in QObject::event(QEvent*) ()
    at /usr/lib/libQt6Core.so.6
#13 0x00007f3cb56f438b in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt6Widgets.so.6
#14 0x00007f3cb4739818 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt6Core.so.6
#15 0x00007f3cb4739b9b in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /usr/lib/libQt6Core.so.6
#16 0x00007f3cb49758a4 in ??? () at /usr/lib/libQt6Core.so.6
#17 0x00007f3cb4021199 in ??? () at /usr/lib/libglib-2.0.so.0
#18 0x00007f3cb40803bf in ??? () at /usr/lib/libglib-2.0.so.0
#19 0x00007f3cb4020712 in g_main_context_iteration ()
    at /usr/lib/libglib-2.0.so.0
#20 0x00007f3cb49739c4 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt6Core.so.6
#21 0x00007f3cb4743d6e in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt6Core.so.6
#22 0x00007f3cb473c2b8 in QCoreApplication::exec() ()
    at /usr/lib/libQt6Core.so.6
#23 0x0000654c3204922a in ??? ()
#24 0x00007f3cb5443cd0 in ??? () at /usr/lib/libc.so.6
#25 0x00007f3cb5443d8a in __libc_start_main ()
    at /usr/lib/libc.so.6
#26 0x0000654c32049ab5 in _start ()
gfgit commented 8 months ago

Maybe we can add an Hidden=true to then hide it from applications menu.

It's already hidden because it is a LXQt-module which are hidden by default afaik.

On OpenBox running LXQt Panel, I can see it in FancyMenu (first item, unnamed). By selecting "Add to desktop" you can verify it's actually lxqt-panel.desktop originated from lxqt_panel-wayland.desktop.in file.

EDIT: NoDisplay=true seems better than Hidden=true

stefonarch commented 8 months ago

On OpenBox running LXQt Panel, I can see it in FancyMenu (first item, unnamed). By selecting "Add to desktop" you can verify it's actually lxqt-panel.desktop originated from lxqt_panel-wayland.desktop.in file.

EDIT: NoDisplay=true seems better than Hidden=true

Maybe because openbox is no DE? Anyway I'm fine with all.

stefonarch commented 8 months ago

@gfgit When I first compiled and installed lxqt-panel, I faced one major error:

AutoMoc subprocess error
------------------------
The moc process failed to compile
  "SRC:/plugin-taskbar/lxqttaskbarplugin.h"
into
  "SRC:/.build/plugin-taskbar/taskbar_autogen/EWIEGA46WW/moc_lxqttaskbarplugin.cpp"
Process failed with return value 1

Command
-------
/usr/lib/qt6/moc "-DLXQT_DATA_DIR=\"/usr/share\"" "-DLXQT_ETC_XDG_DIR=\"/etc/xdg\"" "-DLXQT_GRAPHICS_DIR=\"/usr/share/lxqt/graphics\"" "-DLXQT_MAJOR_VERSION=\"2\"" "-DLXQT_MINOR_VERSION=\"0\"" "-DLXQT_PANEL_VERSION=\"2.0.0\"" "-DLXQT_PATCH_VERSION=\"0\"" "-DLXQT_RELATIVE_SHARE_DIR=\"lxqt\"" "-DLXQT_RELATIVE_SHARE_TRANSLATIONS_DIR=\"lxqt/translations\"" "-DLXQT_SHARE_DIR=\"/usr/share/lxqt\"" "-DLXQT_SHARE_TRANSLATIONS_DIR=\"/usr/share/lxqt/translations\"" "-DLXQT_VERSION=\"2.0.0\"" "-DPLUGIN_DIR=\"/usr/lib/lxqt-panel\"" -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_FOREACH -DQT_NO_URL_CAST_FROM_STRING -DQT_SVG_LIB -DQT_USE_QSTRINGBUILDER -DQT_WIDGETS_LIB -DQT_XML_LIB -DWITH_DESKTOPSWITCH_PLUGIN -DWITH_FANCYMENU_PLUGIN -DWITH_MAINMENU_PLUGIN -DWITH_QUICKLAUNCH_PLUGIN -DWITH_SHOWDESKTOP_PLUGIN -DWITH_TASKBAR_PLUGIN -I/home/cosmos/Softwares/Projects/LXQt/lxqt-panel/.build/plugin-taskbar -I/home/cosmos/Softwares/Projects/LXQt/lxqt-panel/plugin-taskbar -I/usr/include/qt6/QtWidgets -I/usr/include/qt6 -I/usr/include/qt6/QtCore -I/usr/lib/qt6/mkspecs/linux-g++ -I/usr/include/qt6/QtGui -I/usr/include/lxqt -I/usr/include/lxqt/LXQt -I/usr/include/KF6/KWindowSystem -I/usr/include/qt6/QtDBus -I/usr/include/qt6xdg -I/usr/include/qt6/QtXml -I/usr/include/qt6xdgiconloader -I/usr/include/qt6xdgiconloader/4.0.0 -I/usr/include/qt6/QtSvg -I/usr/include/lxqt-globalkeys -I/usr/include -I/opt/aocc/include -I/usr/include/c++/13.2.1 -I/usr/include/c++/13.2.1/x86_64-pc-linux-gnu -I/usr/include/c++/13.2.1/backward -I/usr/lib/gcc/x86_64-pc-linux-gnu/13.2.1/include -I/usr/local/include -I/usr/lib/gcc/x86_64-pc-linux-gnu/13.2.1/include-fixed --include /home/cosmos/Softwares/Projects/LXQt/lxqt-panel/.build/plugin-taskbar/taskbar_autogen/moc_predefs.h --output-dep-file -o /home/cosmos/Softwares/Projects/LXQt/lxqt-panel/.build/plugin-taskbar/taskbar_autogen/EWIEGA46WW/moc_lxqttaskbarplugin.cpp /home/cosmos/Softwares/Projects/LXQt/lxqt-panel/plugin-taskbar/lxqttaskbarplugin.h

Output
------
/home/cosmos/Softwares/Projects/LXQt/lxqt-panel/plugin-taskbar/lxqttaskbarplugin.h:62:1: error: Undefined interface

Note that this was the first time I installed lxqt, which means that /usr/include/lxqt/ilxqtpanelplugin.h did not exist. Once I manually copied ilxqtpanel.h, ilxqtpanelplugin.h, lxqtpanelglobals.h and pluginsettings.h to /usr/include/lxqt this error vanished. It looks to me that moc is unable to locate ilxqtpanelplugin.h in the source tree.

Compiling on a clean VM with Manjaro I run exactly into this. EDIT: looks like https://github.com/lxqt/lxqt-panel/pull/2041 doesn't have that problem, also not the ld issue and compiling ti first this PR compiles fine. Looks like something got lost in between.

gfgit commented 8 months ago

@marcusbritanicus Hi, didn't have time to properly test yet sorry. But... Could we improve the generic wayland backend proxy thing? I mean it's ju duplicated code with no added logic. I have 2 ideas:

  1. Move wayland backend creation to lxqtpanelapplication. So where we create the X11 backend we also directly create Plasma/Wlroots/other. This would introduce wayland specific code in generic code (compositor detection logic) so maybe we can make a factory function which hides this logic and put it in the toplevel wayland folder.

  2. Another option would be using only one wayland backend. The plasma and wlroots backend are quite similar so we could share more code between them. This option is a bit more difficult to implement.

marcusbritanicus commented 7 months ago

Hi, didn't have time to properly test yet sorry.

That's totally fine.

But... Could we improve the generic wayland backend proxy thing? I mean it's ju duplicated code with no added logic.

I was waiting for your input regarding this. Since it's not time-bound (i.e., we're likely to make a release without this PR), I think we can take this slow.

Absolutely: I just took the dummy-backend, and added wayland bits. All I wanted to do was enable a means for @stefonarch (and the rest of us as well) to test the code. Secondly, we (@stefonarch, and I) and come to a consensus that a plugin based approach would be better. Wayland (wlroots included) is a lot of things, but sadly, feature-rich is not one of them. And different compositors have taken their own approach to achieve their end goals. So the unfortunate truth is that we will have to invest in compositor-specific codes. And this means having a plugin-based architecture is a must.

I have 2 ideas:

  1. Move wayland backend creation to lxqtpanelapplication. So where we create the X11 backend we also directly create Plasma/Wlroots/other. This would introduce wayland specific code in generic code (compositor detection logic) so maybe we can make a factory function which hides this logic and put it in the toplevel wayland folder.
  2. Another option would be using only one wayland backend. The plasma and wlroots backend are quite similar so we could share more code between them. This option is a bit more difficult to implement.

I think we should go for the first approach. On the surface, it does look like plasma and wlroots appear the same, but I think I have rewritten both the LXQtTaskbarWlrootsBackend and LXQtTaskbarWlrootsWindowManagment classes after copying your plasma code. So putting in effort to separate out common code between them would not be worth it.

Secondly, it will be difficult to add the compositor-specific code (in approach 2). Currently, as far as I am aware, we have special code for 1) plasma, and further we can develop (sufficiently easily) compositor specific code for 2) wayfire, 3) sway, 4) hyprland. If we annoy consolatis and johanmalm sufficiently, may be labwc as well. And these are quite popular ones. It will take a at least a few years to achieve basic feature parity among these to get rid of these special codes.

gfgit commented 7 months ago

I like plugin based approach. So we can make a generic function createWaylandBackend() which internally detects compositor and chooses the best wayland plugin and returns a pointer to a new backend instance. This way generic code does not need to know about wayland specific things. We could also make each wayland plugin probe itself so in case we want to dynamically load them from shared libraries (like a real plugin) we don't need to hardcode which compositors are currently supported.

stefonarch commented 7 months ago

As for testing I can switch fine between all mentioned compositors without issues using this PR - great work from you both!

Maybe it's early to talk about but I think we should have a WIP_wayland branch for this at the end, so we could have it on AUR (or compiling) for getting advanced user's feedback. Our git checkouts were always meant to be stable, having a separate branch would give us more freedom in some sense.

marcusbritanicus commented 7 months ago

So we can make a generic function createWaylandBackend() which internally detects compositor and chooses the best wayland plugin and returns a pointer to a new backend instance.

This would be perfect.

This way generic code does not need to know about wayland specific things. We could also make each wayland plugin probe itself so in case we want to dynamically load them from shared libraries (like a real plugin) we don't need to hardcode which compositors are currently supported.

Absolutely. We can develop some ranking system (automated + user-controllable) using which we can pick the platform plugin. For example, for wayfire, we can (or people who are interested) easily develop two plugins - one based on protocols, the other based on ipc. In such cases, if we wish, we can allow the user to choose the backend.

stefonarch commented 7 months ago

NoDisplay=true seems better than Hidden=true

Yes, this works fine, noticed only now that it appears without title on top under "All Applications" otherwise.

gfgit commented 7 months ago

NoDisplay=true seems better than Hidden=true

Yes, this works fine, noticed only now that it appears without title on top under "All Applications" otherwise.

Done in #2043 @marcusbritanicus Can I force-push it to cleanup recent history and you rebase your branches on top? But we could also do this later when we are ready to merge.

EDIT: my intention is that once layer shell code is merged into master, I'll rebase may WM abstraction and Plasma backend PRs on top of new master because currently they still have old commits about layer shell which would then become duplicates of #2048

marcusbritanicus commented 7 months ago

@gfgit feel free to force push. I don't mind. If it's too messy, I don't mind opening a new PR.

gfgit commented 7 months ago

@gfgit feel free to force push. I don't mind. If it's too messy, I don't mind opening a new PR.

Your PR is fine. I just like rebasing, don't know why XD. When #2048 is merged I'll try move my branches on top of master and you can then move your PR on top

gfgit commented 7 months ago

@marcusbritanicus I've cleaned up and rebased my two PR I've also made a copy of your wlroots branch and rebased it on top of new #2043 so you don't have to waste time rebasing yourself. I've uploaded it to gfgit/wlroots-taskbar_new You could reset your branch to that.

marcusbritanicus commented 7 months ago

@gfgit thanks a lot. I'm most likely to be busy until 18th. If I find some time in between, I'll update my PR, otherwise I'll get back to it after 18th.

stefonarch commented 7 months ago

I've no idea why and where but when I use this branch the panel crashes continuously now (on login, while configuring), independent from taskbar being present or not.

marcusbritanicus commented 7 months ago

@gfgit I just updated my repo.. I hope I have done it right...

@stefonarch Maybe, now it will not crash?? Let's debug it when possible.

stefonarch commented 7 months ago

@stefonarch Maybe, now it will not crash?? Let's debug it when possible.

Unfortunately it does still crash when adding a third panel already (without taskbar present).

(gdb) where
#0  0x00005cef615bd033 in ??? ()
#1  0x0000743d09797609 in ??? () at /usr/lib/libQt6Core.so.6
#2  0x00005cef615c3834 in ??? ()
#3  0x0000743d09797609 in ??? () at /usr/lib/libQt6Core.so.6
#4  0x00005cef615c13b8 in ??? ()
#5  0x0000743d0a476596 in ??? () at /usr/lib/libffi.so.8
#6  0x0000743d0a47300e in ??? () at /usr/lib/libffi.so.8
#7  0x0000743d0a475bd3 in ffi_call () at /usr/lib/libffi.so.8
#8  0x0000743d0afb1645 in ??? ()
    at /usr/lib/libwayland-client.so.0
#9  0x0000743d0afb1e73 in ??? ()
    at /usr/lib/libwayland-client.so.0
#10 0x0000743d0afb213c in wl_display_dispatch_queue_pending ()
    at /usr/lib/libwayland-client.so.0
#11 0x0000743d0b020b5e in ??? ()
    at /usr/lib/libQt6WaylandClient.so.6
#12 0x0000743d097883e4 in QObject::event(QEvent*) ()
    at /usr/lib/libQt6Core.so.6
#13 0x0000743d0a6fbfcb in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt6Widgets.so.6                     
#14 0x0000743d0973dae8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt6Core.so.6                         
#15 0x0000743d0973de74 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /usr/lib/libQt6Core.so.6       
#16 0x0000743d099860e4 in ??? () at /usr/lib/libQt6Core.so.6
#17 0x0000743d094e0199 in ??? () at /usr/lib/libglib-2.0.so.0
#18 0x0000743d0953f3bf in ??? () at /usr/lib/libglib-2.0.so.0
#19 0x0000743d094df712 in g_main_context_iteration ()
    at /usr/lib/libglib-2.0.so.0
#20 0x0000743d09983df4 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt6Core.so.6   
#21 0x0000743d09745c7e in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt6Core.so.6                      
#22 0x0000743d097416e8 in QCoreApplication::exec() ()
    at /usr/lib/libQt6Core.so.6
#23 0x00005cef614cd2ea in ??? ()
#24 0x0000743d09043cd0 in ??? () at /usr/lib/libc.so.6
#25 0x0000743d09043d8a in __libc_start_main ()
    at /usr/lib/libc.so.6
#26 0x00005cef614cdb75 in _start ()

Another issue (present also in @gfgit 's PR if I remember well) : the lxqt-panel.desktop file is missing in /etc/xdg/autostart and therefor no module is seen in lxqt-config-session.

stefonarch commented 7 months ago

Just checked kwin: no issue with crashing, it crashes only in wlroots-based compositors: tested wayfire, labwc and Hyprland. At login, when configuring (random) and when using F12 which executes

#!/bin/bash
# Helper script for using qterminal -d (dropdown) under wayland
# Add F12 as shortcut in compositor
while IFS= read -r line; do
    qdbus "$line" / org.lxqt.QTerminal.Process.toggleDropdown
done < <(qdbus | grep QTermin | awk '{$1=$1};1')
marcusbritanicus commented 7 months ago

@stefonarch What about #2043 alone? I want to peg this down in a couple of days. It's strange that we both are using arch, I cannot for any reason make the panel crash.

Well, I'm using Artix/Arch, but I think it should not matter.

stefonarch commented 7 months ago

@stefonarch What about #2043 alone?

Made a package now for this - working fine on kwin and no crashes on labwc so far.

stefonarch commented 7 months ago

As already mentioned: if somebody compiles and installs directly it wouldn't be noticed, but if packages are built both from this PR and https://github.com/lxqt/lxqt-panel/pull/2043 the desktop module (aka /etc/xdg/autostart/lxqt-panel) will not be installed and the panel will not start automatically and isn't seen in lxqt-config-session.

Wayland taskbar PRs: screen_area_ven_19:12:28_

Refactor PR which is ok: screen_area_ven_19:12:06_

The issue is probably in the CMakeList.txt from @gfgit 's PR: https://github.com/lxqt/lxqt-panel/pull/2043/files#diff-8d86bd04367459ea2d480ddd870ec392a57368868dcaeec65beda5ef97220bd9

If this is fixed we could have an AUR package to get more testers.

tsujan commented 7 months ago

At last, I found time to try this branch. But I saw to task-bar under LabWC, Wayfire or kwin_wayland. I tried several things (a long list), but still no task-bar.

Of course, I know that @stefonarch and @marcusbritanicus have task-bars with this branch (@stefonarch even used my panel config file in terminal and had task-bar, as I tried his compiled package with no effect). There should be a hidden factor.

tsujan commented 7 months ago

OK, I found the cause by looking at the code. It's in the c-tor of LXQtTaskbarWaylandBackend (in the file lxqttaskbarbackendwayland.cpp): it expects XDG_CURRENT_DESKTOP to be the name of the Wayland compositor, while I set it to LXQt under LabWC and Wayfire (to have correct styling with all Qt apps).

The task bar showed up as soon as I started the panel with XDG_CURRENT_DESKTOP=labwc lxqt-panel -c MY_CONF. However, it crashed three times —as @stefonarch reported above, and with a backtrace which isn't informative at all:

#0  0x000055d95ae16fb3 in ??? ()
#1  0x00007f091bf97609 in ??? () at /usr/lib/libQt6Core.so.6
#2  0x000055d95ae1d7b4 in ??? ()
#3  0x00007f091bf97609 in ??? () at /usr/lib/libQt6Core.so.6
#4  0x000055d95ae1b338 in ??? ()
#5  0x00007f091d528596 in ??? () at /usr/lib/libffi.so.8
#6  0x00007f091d52500e in ??? () at /usr/lib/libffi.so.8
#7  0x00007f091d527bd3 in ffi_call () at /usr/lib/libffi.so.8
#8  0x00007f091d924645 in ??? () at /usr/lib/libwayland-client.so.0
#9  0x00007f091d924e73 in ??? () at /usr/lib/libwayland-client.so.0
#10 0x00007f091d92513c in wl_display_dispatch_queue_pending () at /usr/lib/libwayland-client.so.0
#11 0x00007f091d993b5e in ??? () at /usr/lib/libQt6WaylandClient.so.6
#12 0x00007f091bf883e4 in QObject::event(QEvent*) () at /usr/lib/libQt6Core.so.6
#13 0x00007f091cefbfcb in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt6Widgets.so.6
#14 0x00007f091bf3dae8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt6Core.so.6
#15 0x00007f091bf3de74 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
    at /usr/lib/libQt6Core.so.6
#16 0x00007f091c1860e4 in ??? () at /usr/lib/libQt6Core.so.6
#17 0x00007f091bd0d199 in ??? () at /usr/lib/libglib-2.0.so.0
#18 0x00007f091bd6c3bf in ??? () at /usr/lib/libglib-2.0.so.0
#19 0x00007f091bd0c712 in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#20 0x00007f091c183df4 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /usr/lib/libQt6Core.so.6
#21 0x00007f091bf45c7e in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt6Core.so.6
#22 0x00007f091bf416e8 in QCoreApplication::exec() () at /usr/lib/libQt6Core.so.6
#23 0x000055d95ad272ea in ??? ()
#24 0x00007f091b843cd0 in ??? () at /usr/lib/libc.so.6
#25 0x00007f091b843d8a in __libc_start_main () at /usr/lib/libc.so.6
#26 0x000055d95ad27b75 in ??? ()
stefonarch commented 7 months ago

So this enigma is solved, for wayland we use LXQt:<compositor>:wlroots , eg.

$ echo $XDG_CURRENT_DESKTOP 
LXQt:labwc:wlroots

The backtrace is the same as ever, but some time ago it was usable until you didn't log out, after an update where I couldn't identify any suspect component it started crashing even on opening windows.

So we have 2 where it crashes and 2 where not (my Manjaro VM and @marcusbritanicus ` installation).

tsujan commented 7 months ago

Here, the crash happens seconds after I start the panel.

The backtrace is the same as ever, but some time ago it was usable until you didn't log out

If the code hasn't changed since then, the problem may be related to Qt-Wayland 6.7. @marcusbritanicus, what's your Qt version?

marcusbritanicus commented 7 months ago

@tsujan I'm also using Qt 6.7. I can start the panel without issues. But as soon as a new window opens, the panel crashes. This issue is most likely due to Qt 6.7 - I have not recompiled the panel after upgrading to Qt 6.7.

stefonarch commented 7 months ago

@tsujan I'm also using Qt 6.7. I can start the panel without issues. But as soon as a new window opens, the panel crashes. This issue is most likely due to Qt 6.7 - I have not recompiled the panel after upgrading to Qt 6.7.

That is the pattern now, recompiling doesn't change this. Often reopening the same window works (the first attempt crashes it, the second not). As we have apps in autostart it will crash several times at login.

tsujan commented 7 months ago

But as soon as a new window opens, the panel crashes

Yes. I failed to mention that in my comment.

I have not recompiled the panel after upgrading to Qt 6.7.

Although recompilation is needed, it won't make a difference in this regard. I started to test the code only after Qt 6.7.

This issue is most likely due to Qt 6.7

I agree, although I can't prove it. I've dealt with some peculiarities of Qt 6.7 elsewhere and succeeded in making backward compatible workarounds/changes in an app. I'm not sure if that's possible here.

marcusbritanicus commented 7 months ago

Yes, I can see that. I just finished recompiling - and it still crashes when opening a new window. If I open a dialog (like file open/save/...) the panel does not crash. If I open a new window, it does. I am wondering if it's related to parent changed signal.

Anyways, I'll look into this tonight.

marcusbritanicus commented 7 months ago

I agree, although I can't prove it. I've dealt with some peculiarities of Qt 6.7 elsewhere and succeeded in making backward compatible workarounds/changes in an app. I'm not sure if that's possible here.

Well, I have faced this issue (panel crashing when a new window opens) when I was on Qt 6.6 as well. Initially, I believed that we might be using some private class of Qt, which is causing the crash on 6.7. However, now I see that it's not the case.

marcusbritanicus commented 7 months ago

@tsujan @stefonarch Please test now. I have pushed a fix - it did not occur to me that WId may need to be initialized to 0.

Edit: I am able to open new windows and new dialogs without any crashes. Note that I have tested this only a few times, and with only a few apps (this should not matter, but still).

tsujan commented 7 months ago

@marcusbritanicus Yes, adding that 0 fixed the crash here :) Nice catch!

EDIT: I tested with many apps (including XWayland) and by changing the workspace, panel settings…: no crash.

stefonarch commented 7 months ago

Looks really fixed here too, nice catch really!

marcusbritanicus commented 7 months ago

Thanks! I think now I know what was happening. WId is quintptr and it was holding garbage values. The problems arose when converting WId to LXQtTaskbarWlrootsWindow *. It would try to convert this garbage value into a pointer and we would get a crash. Fortunately in my case, opening a new window caused this crash, and I was able to trace it back to this.

Phew! What a ride!

stefonarch commented 7 months ago

Just a test ride on Hyprland, all fine too :)

tsujan commented 7 months ago

This patch is needed to enable auto-start and also to remove redundant codes from the tray plugin (I'd told Filippo about the latter);

diff -ruNp lxqt-panel-orig/autostart/CMakeLists.txt lxqt-panel-wlroots-taskbar/autostart/CMakeLists.txt
--- lxqt-panel-orig/autostart/CMakeLists.txt
+++ lxqt-panel-wlroots-taskbar/autostart/CMakeLists.txt
@@ -1,9 +1,9 @@
-set(DESKTOP_FILES lxqt-panel.desktop.in)
+set(AUTOSTART_DESKTOP_FILE_IN lxqt-panel.desktop.in)

 # Translations **********************************
 lxqt_translate_desktop(DESKTOP_FILES
     SOURCES
-        ${DESKTOP_FILES_IN}
+        ${AUTOSTART_DESKTOP_FILE_IN}
     USE_YAML
 )
 add_custom_target(lxqt_panel_autostart_desktop_files ALL DEPENDS ${DESKTOP_FILES})
diff -ruNp lxqt-panel-orig/autostart/lxqt-panel_wayland.desktop lxqt-panel-wlroots-taskbar/autostart/lxqt-panel_wayland.desktop
--- lxqt-panel-orig/autostart/lxqt-panel_wayland.desktop
+++ lxqt-panel-wlroots-taskbar/autostart/lxqt-panel_wayland.desktop
@@ -1,13 +0,0 @@
-[Desktop Entry]
-Type=Application
-TryExec=lxqt-panel
-
-# NOTE: KWin wants absolute path here, make sure it's correct
-Exec=/usr/local/bin/lxqt-panel
-
-# NOTE: adding KDE to make it work under Plasma Wayland session
-OnlyShowIn=LXQt;KDE
-X-LXQt-Module=true
-
-# Make KWin recognize us as priviledged client
-X-KDE-Wayland-Interfaces=org_kde_plasma_window_management
diff -ruNp lxqt-panel-orig/plugin-tray/CMakeLists.txt lxqt-panel-wlroots-taskbar/plugin-tray/CMakeLists.txt
--- lxqt-panel-orig/plugin-tray/CMakeLists.txt
+++ lxqt-panel-wlroots-taskbar/plugin-tray/CMakeLists.txt
@@ -38,7 +38,6 @@ qt_add_dbus_adaptor(SOURCES org.kde.Stat
 qt_add_dbus_interface(SOURCES org.kde.StatusNotifierWatcher.xml statusnotifierwatcher_interface)

 set(LIBRARIES
-    Qt6::GuiPrivate
     ${XCB_LIBRARIES}
     ${xtst_LDFLAGS}
 )
diff -ruNp lxqt-panel-orig/plugin-tray/sniproxy.cpp lxqt-panel-wlroots-taskbar/plugin-tray/sniproxy.cpp
--- lxqt-panel-orig/plugin-tray/sniproxy.cpp
+++ lxqt-panel-wlroots-taskbar/plugin-tray/sniproxy.cpp
@@ -38,7 +38,6 @@

 #include <KWindowSystem>
 #include <netwm.h>
-#include <qpa/qplatformnativeinterface.h> //For "gettimestamp" Xcb integration resource

 #include "kwindowinfo.h"
 #include "statusnotifieritemadaptor.h"
@@ -591,17 +590,13 @@ void SNIProxy::sendClick(uint8_t mouseBu
     auto *x11Application = qGuiApp->nativeInterface<QNativeInterface::QX11Application>();
     WId appRootWindow = XDefaultRootWindow(x11Application->display());

-    // Qt private access
-    void *ptr = qGuiApp->platformNativeInterface()->nativeResourceForScreen("gettimestamp", qGuiApp->primaryScreen());
-    xcb_timestamp_t timeStamp = reinterpret_cast<quintptr>(ptr);
-
     // mouse down
     if (m_injectMode == Direct) {
         xcb_button_press_event_t *event = new xcb_button_press_event_t;
         memset(event, 0x00, sizeof(xcb_button_press_event_t));
         event->response_type = XCB_BUTTON_PRESS;
         event->event = m_windowId;
-        event->time = timeStamp;
+        event->time = XCB_CURRENT_TIME; //NOTE: to get proper timestamp we would need Qt Private APIs
         event->same_screen = 1;
         event->root = appRootWindow;
         event->root_x = x;
@@ -624,7 +619,7 @@ void SNIProxy::sendClick(uint8_t mouseBu
         memset(event, 0x00, sizeof(xcb_button_release_event_t));
         event->response_type = XCB_BUTTON_RELEASE;
         event->event = m_windowId;
-        event->time = timeStamp;
+        event->time = XCB_CURRENT_TIME; //NOTE: to get proper timestamp we would need Qt Private APIs
         event->same_screen = 1;
         event->root = appRootWindow;
         event->root_x = x;
stefonarch commented 7 months ago

The patch didn't resolve the autostart issue for me: immagine

edit: maybe I needed to commit the patch first.

edit II: yes, all fine with the patch

gfgit commented 7 months ago

@tsujan Should I push the patch in my Plasma Wayland branch? If I understand well it's a file name issue, not about file contents

tsujan commented 7 months ago

If I understand well it's a file name issue, not about file contents

It's just about DESKTOP_FILESAUTOSTART_DESKTOP_FILE_IN. I think the rest is correct in your branch (I didn't check though).

stefonarch commented 7 months ago

@tsujan Should I push the patch in my Plasma Wayland branch? If I understand well it's a file name issue, not about file contents

The KDE plasma License should be added too.

tsujan commented 7 months ago

I replaced Waybar with lxqt-panel in labwc and wayfire. @gfgit, @marcusbritanicus, very nice work!

These are the problems I've found so far by using the task manager under labwc:

  1. Sometimes the task button isn't selected on opening a window.
  2. The task button gets deselected when the window title changes, e.g., because of tab switching in pcmanfm-qt/featherpad or changing directory in pcmanfm-qt. (I think 1 and 2 may be related).
  3. When a window is minimized, sometimes its task button remains selected (probably only if it's the single window in the current workspace). Then, if another window is opened, there might be two selected task buttons.

I don't know if these problems are related to the current PR or one of @gfgit's PRs, but since I'm testing this branch, I reported them here.

I haven't found time to test under X11 either.

Most importantly, the job of code reviewing hasn't been done yet. @palinek, could you please review https://github.com/lxqt/lxqt-panel/pull/2041?

marcusbritanicus commented 7 months ago

@tsujan Thanks.

  1. Sometimes the task button isn't selected on opening a window.

I too have noticed this, but in my case, it's always the case with certain apps. For example, with DesQ Term or QTerminal, the button never gets selected. On the other hand, with DesQ Docs, it's always selected. So I think it might be something related to keyboard focus. Interestingly, this problem does not exist with wf-shell (which uses foreign-toplevel) or desq-panel (which uses home-built toplevel-manager). So I think it's something to do with my implementation. I have tried to keep the code written by @gfgit for Plasma. Such complications may not be needed for wlroots.

  1. The task button gets deselected when the window title changes, e.g., because of tab switching in pcmanfm-qt/featherpad or changing directory in pcmanfm-qt. (I think 1 and 2 may be related).

I have not used lxqt-panel so much to come across this issue. But yes, I too think 1 and 2 might be related.

  1. When a window is minimized, sometimes its task button remains selected (probably only if it's the single window in the current workspace). Then, if another window is opened, there might be two selected task buttons.

I did a quick test for this. Again I noticed that this is the case with terminals (desq-term, qterminal, foot, coreterminal, ...), and I was partly able to reproduce this with pcmanfm-qt. When minimizing pcmanfm-qt, the button is still selected. But as soon as I open another window (geany/foot/desq-term/...), it loses the "selected state". With geany, I was just unable to reproduce this at all.

I don't know if these problems are related to the current PR or one of @gfgit's PRs, but since I'm testing this branch, I reported them here.

Wlroots is almost completely my work; I took only the skeleton from plasma code by @gfgit. So, more likely than not, the issues are related to the current PR, and are unlikey due to @gfgit's PRs.

Most importantly, the job of code reviewing hasn't been done yet.

As you suggested, let's start with #2041. I had suggested a split-PR approach on the discussion thread:

By all means, we should split this rather large PR into smaller manageable pieces:

  1. (PR-1) The first as @stefonarch suggests very aptly, should be the layer-shell.
  2. (PR-2) The second PR should be introduction of the concept of "backend" for taskbar and workspaces, and refactoring the existing X11 code to work with this backend. This, if I understand correctly, will be the foundation on which, wayland support can be built (albiet, later on).
  3. Support for plasma on kwin-wayland.
  4. Generic wlroots support. And of course, we can slowly add support for specific compositors.

PR-1 is complete now. PR-2 is, if I'm not mistaken, #2041.

It will also give us some time to fix the outstanding issues with wlroots implementations (ones listed above and the list that is being compiled by @stefonarch)

By the time that is done, I think we need to discuss how to add wayland support via plugins to the taskbar plugin. @stefonarch, @gfgit your thoughts on this?

tsujan commented 7 months ago

I had suggested a split-PR approach on the https://github.com/lxqt/lxqt/discussions/2531#discussioncomment-8966723:

Yes, that's the cleanest way of doing it.