search

lxqt / lxqt-panel

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

Refactor Window Manager interaction #2041

Closed gfgit closed 1 month ago

gfgit commented 3 months ago

This is a replacement for #2029 rebased on latest master

Main differences with initial idea:

TODO: port other plugins which currently directly use X11 to use this abstract interface

tsujan commented 1 month ago

When compiling your latest branch:

/usr/bin/ld: CMakeFiles/lxqt-panel.dir/lxqt-panel_autogen/mocs_compilation.cpp.o:(.data.rel.ro._ZTV23LXQtTaskBarDummyBackend[_ZTV23LXQtTaskBarDummyBackend]+0x130): undefined reference to `LXQtTaskBarDummyBackend::setDesktopLayout(Qt::Orientation, int, int, bool)'
collect2: error: ld returned 1 exit status
make[2]: *** [panel/CMakeFiles/lxqt-panel.dir/build.make:681: panel/lxqt-panel] Error 1
make[1]: *** [CMakeFiles/Makefile2:2509: panel/CMakeFiles/lxqt-panel.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
tsujan commented 1 month ago

Sadly, setting of the desktop layout from lxqt-panel's Switcher doesn't work with KWin 6.0.4 anymore. That may not be related to this PR because it doesn't work with lxqt-panel 2.0.1 either, although I'm sure that the Qt5-based version of KWin followed lxqt-panel's Switcher when "Number of rows" was changed in the latter (ignore KDE System Settings).

This needs to be tested with another WM.

tsujan commented 1 month ago

OK, I tested with xfwm4, and the latest change to this PR could really change the desktop layout. So, Kwin's problem is a regression of KWin. Since xfwm4 didn't have a way of showing the desktop grid, the only way of knowing that it really followed Switcher was activating xfwm4's Window Manager → Advanced → Wrap workspaces when... → With a dragged window and then dragging a window with mouse to other desktops.

So far, so good. I didn't find another problem in the PR's code. But since lots of changes have been made and my time is limited, I can't be sure there isn't any code issue that I may have missed. However, it passed my various tests. So, I think it's a good idea to merge it after checking whether the patch can be cleanly applied to lxqt-panel 2.0.1.

stefonarch commented 1 month ago

There is basic issue with this PR and the ones based upon. If no lxqt-panel is installed and ilxqtpanel.h ilxqtpanelplugin.h lxqtpanelglobals.h pluginsettings.h aren't already available in /usr/include/lxqt build fails:

[  2%] Built target sysstat_autogen

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_FOREACH -DQT_NO_URL_CAST_FROM_STRING -DQT_STRICT_ITERATORS -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/stef/git/lxqt/lxqt-panel/build/plugin-taskbar -I/home/stef/git/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/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/stef/git/lxqt/lxqt-panel/build/plugin-taskbar/taskbar_autogen/moc_predefs.h --output-dep-file -o /home/stef/git/lxqt/lxqt-panel/build/plugin-taskbar/taskbar_autogen/EWIEGA46WW/moc_lxqttaskbarplugin.cpp /home/stef/git/lxqt/lxqt-panel/plugin-taskbar/lxqttaskbarplugin.h

Output
------
/home/stef/git/lxqt/lxqt-panel/plugin-taskbar/lxqttaskbarplugin.h:62:1: error: Undefined interface

People without an existing installation of lxqt-panel got /lxqt-panel/plugin-taskbar/lxqttaskbarplugin.h:62:1: error: Undefined interface at 72%.

tsujan commented 1 month ago

Confirming @stefonarch's surprising observation.

stefonarch commented 1 month ago

@gfgit Do you have some time to look at the remaining issues?

gfgit commented 1 month ago

Hi, I'm back. Compile issues should be fixed by latest commit

gfgit commented 1 month ago

@tsujan I've reverted commit regarding panel positioning. It only affects Wayland which is not yet merged. To be honest lxqtpanel.cpp is a bit messy, with multiple "delayed" calls and workarounds for various issues which include also desktopswitch.cpp warning about realign(). Since this must change anyway for Wayland because we cannot rely on global coordinates there, it might be better to clean the logic a bit.

stefonarch commented 1 month ago

Compile issues should be fixed by latest commit

Confirmed. I see that now there are 6 files instead of the previous 4:

stef@archlinux:lxqt-panel$ cd /usr/include/lxqt
stef@archlinux:lxqt$ ls
ilxqtpanel.h        LXQt                pluginsettings.h
ilxqtpanelplugin.h  lxqtpanelglobals.h
stef@archlinux:lxqt$ sudo mv *.h /tmp
stef@archlinux:lxqt$ ls
LXQt
stef@archlinux:lxqt$ ls
ilxqtpanel.h                   lxqtpanelglobals.h
ilxqtpanelplugin.h             lxqttaskbartypes.h
ilxqttaskbarabstractbackend.h  pluginsettings.h
LXQt

Will do some test later on X11.

tsujan commented 1 month ago

Since this must change anyway for Wayland because we cannot rely on global coordinates there

Under Wayland, Qt's global coordinates are actually local and calculated relative to the window.

Please resolve the conflicts with the master branch.

gfgit commented 1 month ago

@tsujan Conflicts are fixed now

tsujan commented 1 month ago

Conflicts are fixed now

Thanks! I'll continue reviewing tomorrow.

stefonarch commented 1 month ago

@gfgit I get compiling errors now:

[ 28%] Building CXX object plugin-fancymenu/CMakeFiles/fancymenu.dir/lxqtfancymenu.cpp.o                                              
/tmp/makepkg/lxqt-panel-git/src/lxqt-panel/plugin-desktopswitch/desktopswitch.cpp: In constructor 'DesktopSwitch::DesktopSwitch(const ILXQtPanelPluginStartupInfo&)':
/tmp/makepkg/lxqt-panel-git/src/lxqt-panel/plugin-desktopswitch/desktopswitch.cpp:55:5: error: class 'DesktopSwitch' does not have any field named 'mDesktops'
   55 |     mDesktops(nullptr),
gfgit commented 1 month ago

Hi, it may be a left over from the merge conflict. Does it build fine if you remove the line?

Il 6 giugno 2024 13:38:09 CEST, Standreas @.***> ha scritto:

@gfgit I get compiling errors now:

[ 28%] Building CXX object plugin-fancymenu/CMakeFiles/fancymenu.dir/lxqtfancymenu.cpp.o                                              
/tmp/makepkg/lxqt-panel-git/src/lxqt-panel/plugin-desktopswitch/desktopswitch.cpp: In constructor 'DesktopSwitch::DesktopSwitch(const ILXQtPanelPluginStartupInfo&)':
/tmp/makepkg/lxqt-panel-git/src/lxqt-panel/plugin-desktopswitch/desktopswitch.cpp:55:5: error: class 'DesktopSwitch' does not have any field named 'mDesktops'
  55 |     mDesktops(nullptr),

-- Reply to this email directly or view it on GitHub: https://github.com/lxqt/lxqt-panel/pull/2041#issuecomment-2152154117 You are receiving this because you were mentioned.

Message ID: @.***>

stefonarch commented 1 month ago

Does it build fine if you remove the line?

Yes, thanks.

gfgit commented 1 month ago

Thanks!

tsujan commented 1 month ago

Thanks!

Thank you for doing this great job!

Please also rebase your other PR if it needs that. I hope we could merge it and @marcusbritanicus's PR soon because we need more testers.

marcusbritanicus commented 1 month ago

I am having a rather strange compilation issue: I have not seen anyone else complain about it, so it might be a problem in my setup. But it would be nice to figure out the issue.

FAILED: panel/lxqt-panel                                                                                                                                                                                                                                                   
: && /usr/bin/c++ -fno-exceptions -Wall -Wextra -Wchar-subscripts -Wno-long-long -Wpointer-arith -Wundef -Wformat-security -Wnon-virtual-dtor -Woverloaded-virtual -Wpedantic -fdiagnostics-color=always -O3 -DNDEBUG -Wl,-Bsymbolic-functions    -Wl,--export-dynamic -rdynamic panel/CMakeFiles/lxqt-panel.dir/lxqt-panel_autogen/mocs_compilation.cpp.o panel/CMakeFiles/lxqt-panel.dir/main.cpp.o panel/CMakeFiles/lxqt-panel.dir/panelpluginsmodel.cpp.o panel/CMakeFiles/lxqt-panel.dir/windownotifier.cpp.o panel/CMakeFiles/lxqt-panel.dir/lxqtpanel.cpp.o panel/CMakeFiles/lxqt-panel.dir/lxqtpanelapplication.cpp.o panel/CMakeFiles/lxqt-panel.dir/lxqtpanellayout.cpp.o panel/CMakeFiles/lxqt-panel.dir/plugin.cpp.o panel/CMakeFiles/lxqt-panel.dir/pluginsettings.cpp.o panel/CMakeFiles/lxqt-panel.dir/popupmenu.cpp.o panel/CMakeFiles/lxqt-panel.dir/pluginmoveprocessor.cpp.o panel/CMakeFiles/lxqt-panel.dir/lxqtpanelpluginconfigdialog.cpp.o panel/CMakeFiles/lxqt-panel.dir/config/configpaneldialog.cpp.o panel/CMakeFiles/lxqt-panel.dir/config/configplacement.cpp.o panel/CMakeFiles/lxqt-panel.dir/config/configstyling.cpp.o panel/CMakeFiles/lxqt-panel.dir/config/configpluginswidget.cpp.o panel/CMakeFiles/lxqt-panel.dir/config/addplugindialog.cpp.o panel/CMakeFiles/lxqt-panel.dir/backends/ilxqttaskbarabstractbackend.cpp.o panel/CMakeFiles/lxqt-panel.dir/backends/lxqttaskbardummybackend.cpp.o panel/CMakeFiles/lxqt-panel.dir/backends/xcb/lxqttaskbarbackend_x11.cpp.o panel/CMakeFiles/lxqt-panel.dir/LXQtAppTranslationLoader.cpp.o -o panel/lxqt-panel  /usr/lib/libLayerShellQtInterface.so.6.0.5  plugin-desktopswitch/libdesktopswitch.a  plugin-fancymenu/libfancymenu.a  plugin-mainmenu/libmainmenu.a  plugin-quicklaunch/libquicklaunch.a  plugin-showdesktop/libshowdesktop.a  plugin-taskbar/libtaskbar.a  plugin-statusnotifier/libstatusnotifier.a  plugin-tray/libtray.a  plugin-worldclock/libworldclock.a  plugin-spacer/libspacer.a  /usr/lib/liblxqt-globalkeys-ui.so.2.0.0  /usr/lib/liblxqt-globalkeys.so.2.0.0  /usr/lib/libdbusmenu-lxqt.so.0.1.0  /usr/lib/libQt6Concurrent.so.6.7.1  /usr/lib/libxcb.so  /usr/lib/libxcb-render.so  /usr/lib/libxcb-shape.so  /usr/lib/libxcb-xfixes.so  /usr/lib/libxcb-shm.so  /usr/lib/libxcb-composite.so  /usr/lib/libxcb-damage.so  /usr/lib/libxcb-image.so  /usr/lib/libxcb-randr.so  /usr/lib/libxcb-util.so  -L/usr/lib  -lXtst  /usr/lib/liblxqt.so.2.0.0  /usr/lib/libQt6Xdg.so.4.0.0  /usr/lib/libQt6Xml.so.6.7.1  /usr/lib/libQt6DBus.so.6.7.1  /usr/lib/libQt6XdgIconLoader.so.4.0.0  /usr/lib/libQt6Svg.so.6.7.1  /usr/lib/libglib-2.0.so  /usr/lib/libgobject-2.0.so  /usr/lib/libgio-2.0.so  /usr/lib/libQt6Widgets.so.6.7.1  /usr/lib/libKF6WindowSystem.so.5.249.0  /usr/lib/libQt6Gui.so.6.7.1  /usr/lib/libQt6Core.so.6.7.1  /usr/lib/libGLX.so  /usr/lib/libOpenGL.so  /usr/lib/libX11.so && :                                                                                                                               
/usr/bin/ld: panel/CMakeFiles/lxqt-panel.dir/backends/xcb/lxqttaskbarbackend_x11.cpp.o: in function `LXQtTaskbarX11Backend::moveApplication(unsigned long long)':                                                                                                          
lxqttaskbarbackend_x11.cpp:(.text+0x229b): undefined reference to `NETRootInfo::moveResizeRequest(unsigned int, int, int, NET::Direction, unsigned char, NET::RequestSource)'
/usr/bin/ld: panel/CMakeFiles/lxqt-panel.dir/backends/xcb/lxqttaskbarbackend_x11.cpp.o: in function `LXQtTaskbarX11Backend::resizeApplication(unsigned long long)':
lxqttaskbarbackend_x11.cpp:(.text+0x23dd): undefined reference to `NETRootInfo::moveResizeRequest(unsigned int, int, int, NET::Direction, unsigned char, NET::RequestSource)'
collect2: error: ld returned 1 exit status
ninja: build stopped: cannot make progress due to previous errors.

Edit: Note: In my PR, these two lines are commented out so that I could test it.