lxqt / lxqt-panel

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

Refactor Window Manager interaction (Qt 6 version) #2029

Closed gfgit closed 8 months ago

gfgit commented 9 months ago

Depends on #2024 (Qt6 port)

This is a replacement for #2007 rebased on Qt6 port

Main differences with initial idea:

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

gfgit commented 9 months ago

@stefonarch I've made some testing on KWin Wayland. This is the desktop file I'm using:

[Desktop Entry]
GenericName=Panel
Name=LXQt Panel
Comment=LXQt Panel
Categories=Qt;KDE;Utility;
Keywords=LXQt;Panel
Exec=/usr/local/bin/lxqt-panel
Icon=spectacle
Type=Application
StartupNotify=false
X-KDE-Wayland-Interfaces=org_kde_plasma_window_management

Named lxqt-panel.desktop

It works putting in one of these locations:

  1. /usr/share/applications
  2. /usr/local/share/applications
  3. ~/.local/share/applications/lxqt-panel.desktop

Editing the autostart file inside /etc/xdg/autostart does not affect KWin granting org_kde_plasma_window_management access.

Please test again just to make sure you don't have a typo in your file

stefonarch commented 9 months ago

Please test again just to make sure you don't have a typo in your file

Done, negative. There has to be some hidden difference somewhere. I'll test on the other Vm with the dev edition later.

stefonarch commented 9 months ago

Do you launch it with dolphin? I always used pcmanfm and removed dolphin, at least on the VM with stable.

gfgit commented 9 months ago

Do you launch it with dolphin? I always used pcmanfm and removed dolphin, at least on the VM with stable.

Test launching from:

  1. Qt Creator "Run" button
  2. Konsole lxqt-panel
  3. Directly from plasma panel's application launcher

Qt Creator and Konsole are opened from plasma application launcher

gfgit commented 9 months ago

Also in my .desktop the Exec path is /usr/local vecause that's where CMake installs by default. Did you change install location?

gfgit commented 9 months ago

Did some more tests. Apparently Exec path must be absolute for it to work. Putting just:

Exec=lxqt-panel

will not work and taskbar will be empty.

I also tried setting desktop file in application start with:

QGuiApplication::setDesktopFileName(QLatin1String("lxqt-panel"));

but it doesn't seem to change anything. KDE has KService class which is able to deduce desktop file based on executable path so probably it was already detecting it.

gfgit commented 9 months ago

Also putting OnlyShowIn=LXQt; seems to break behavior. Maybe this makes KWin skip the file so behavior is identical to situation without desktop file.

OnlyShowIn=LXQt;KDE

Fixes the problem. This problem might not appear if running plain LXQt session and not a Plasma session + LXQt Panel

stefonarch commented 9 months ago

Now on the dev edition VM, tried everything except QtCreator, I can't still reproduce what happens at your side. Installation is in /usr/local/, desktop file the same...

stefonarch commented 9 months ago

Oooops, looks like I'm on wayland_taskbar branch.... recompiling

gfgit commented 9 months ago

Oooops, looks like I'm on wayland_taskbar branch.... recompiling

No wait. wayland_taskbar is the correct branch with last commit about panel alignment. I have too many branches :)

stefonarch commented 9 months ago

Oooops, looks like I'm on wayland_taskbar branch.... recompiling

No wait. wayland_taskbar is the correct branch with last commit about panel alignment. I have too many branches :)

Yep, I see, this panel from the PR here dumps core with default config on wayland ;) and without config I didn't find it (transparent in middle of the screen I guess).

It's a pity, thought I found the reason... Maybe the solution is to try with foreign-toplevel-management-unstable ;)

stefonarch commented 9 months ago

But anyway, it shouldn't dump core on wayland, sorry no C&P from that:

screen_area_gio_14:12:21_

fixed screenshot ;)

gfgit commented 9 months ago

Yep, I see, this panel from the PR here dumps core with default config on wayland ;) and without config I didn't find it (transparent in middle of the screen I guess).

This is interesting. How can I reproduce?

Maybe the solution is to try with foreign-toplevel-management-unstable ;)

It depends on compositor. Right now:

stefonarch commented 9 months ago

This is interesting. How can I reproduce?

Openbox session, make sure to be able to open konsole from right click on desktop:

kwin_wayland konsole and inside the nested session lxqt-panel with the default panel.conf from /usr/local/share/lxqt/ in `~/.config/lxqt/

without config it works, But I disabled all plugins which won't work anyway on wayland.

But it should segfault on plasma session too I think.

gfgit commented 9 months ago

This is interesting. How can I reproduce?

Openbox session, make sure to be able to open konsole from right click on desktop:

kwin_wayland konsole and inside the nested session lxqt-panel with the default panel.conf from /usr/local/share/lxqt/ in `~/.config/lxqt/

without config it works, But I disabled all plugins which won't work anyway on wayland.

But it should segfault on plasma session too I think.

We are talking about refactor_taskbar_qt6 branch right? With all plugins enabled compile time?

gfgit commented 9 months ago

I need porting the sysstat plugin to Qt6

Done!

stefonarch commented 9 months ago

I need porting the sysstat plugin to Qt6

Yes, and switch to dbusmenu-lxqt in statusnotifier.

We are talking about refactor_taskbar_qt6 branch right? With all plugins enabled compile time?

Yes, but disabled all those which won't work on wayland plus statusnotifier and sysstat too. No issue on openbox, crashes in Plasma-session too. If [taskbar] section is removed from panel.conf no issue, but adding it from menu it crashes again. Menu and calendar won't open nothing. There IS wayland code here ;) it's on top.

gfgit commented 9 months ago

@stefonarch Steps I've followed:

Contents of ~/.config/lxqt.conf file:

[General]
__userfile__=true
theme=/usr/local/share/lxqt/themes/KDE-Plasma/

Contents of /usr/share/applications/lxqt-panel.desktop file:

[Desktop Entry]
Type=Application
TryExec=lxqt-panel
#Exec=lxqt-panel
Exec=/usr/local/bin/lxqt-panel
OnlyShowIn=LXQt;KDE
X-LXQt-Module=true

X-KDE-Wayland-Interfaces=org_kde_plasma_window_management

Name=Panel

Tests:

  1. Open Konsole from Plasma Wayland Application Launcher. Run lxqt-panel I get ASSERT failure in createWMBackend(): "Only X11 supported!", file /home/filippo/lxqt/lxqt-panel/panel/lxqtpanelapplication.cpp, line 50 which is expected

  2. From same Konsole I run QT_QPA_PLATFORM=xcb lxqt-panel Panel opens correctly but being an X11 client it cannot access Wayland windows, so taskbar is empty. If I launch Okular from Fancy Menu (and no previous instance was running) it will inherit QT_QPA_PLATFORM=xcb So it will be the only app shown in taskbar

stefonarch commented 9 months ago

I'll test later this evening. What about dbusmenu-lxqt?

gfgit commented 9 months ago

OpenBox test:

gfgit commented 9 months ago

Yes, but disabled all those which won't work on wayland plus statusnotifier and sysstat too.

So which plugins have you enabled now exactly?

There IS wayland code here ;) it's on top.

This PR is not about wayland. wayland_taskbar has wayland code added on top of this PR If this PR also has wayland code then I've made mistakes pushing branches

stefonarch commented 9 months ago

If this PR also has wayland code then I've made mistakes pushing branches

Rechecked, was my window rule in kwin. Too many DE configured ;)

stefonarch commented 9 months ago

Built with no changes to the PR, all plugins enabled and no error. But coredump when adding a taskbar under wayland is still the same. Without [taskbar] in panel.conf no issue. Under X11 all good.

#0  0x0000560b855de331 in LXQtTaskBar::settingsChanged() ()
[Current thread is 1 (Thread 0x7f55ae01ea40 (LWP 2600))]
(gdb) where
#0  0x0000560b855de331 in LXQtTaskBar::settingsChanged() ()
#1  0x00007f55af9c9572 in QObject::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt6Core.so.6
#2  0x00007f55b0b67a66 in QFrame::event(QEvent*) () at /lib/x86_64-linux-gnu/libQt6Widgets.so.6
#3  0x00007f55b0bf40eb in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
    at /lib/x86_64-linux-gnu/libQt6Widgets.so.6
#4  0x00007f55afa63e18 in QCoreApplication::notifyInternal2(QObject*, QEvent*) ()
    at /lib/x86_64-linux-gnu/libQt6Core.so.6
#5  0x00007f55afa64530 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) ()
    at /lib/x86_64-linux-gnu/libQt6Core.so.6
#6  0x00007f55af8632f7 in  () at /lib/x86_64-linux-gnu/libQt6Core.so.6
#7  0x00007f55af11bd3b in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#8  0x00007f55af171258 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#9  0x00007f55af1193e3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x00007f55af85e1f0 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /lib/x86_64-linux-gnu/libQt6Core.so.6
#11 0x00007f55afa6604b in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /lib/x86_64-linux-gnu/libQt6Core.so.6
#12 0x00007f55afa67c7c in QCoreApplication::exec() () at /lib/x86_64-linux-gnu/libQt6Core.so.6
#13 0x0000560b8556abf9 in main ()
(gdb) 
gfgit commented 9 months ago

I cannot reproduce. Can you put a breakpoint when settingsChanged() is emitted.

Anyway you are running Xcb panel under wayland, why you don't hit the assert on X11 display?

stefonarch commented 9 months ago

I cannot reproduce. Can you put a breakpoint when settingsChanged() is emitted.

It happens on both machines here... another diff? How can I set a breakpoint?

Anyway you are running Xcb panel under wayland, why you don't hit the assert on X11 display?

No, the panel runs always in xdg-shell if started on wayland. Usually I use an openbox session with a kwin_wayland konsole window for wayland tests (the panel has to be closed for that in openbox). I tested almost all features on openbox, no issues.

stefonarch commented 9 months ago

Ok, found out about breakpoints. But I have to recompile afaik as there are no debug symbols.

gfgit commented 9 months ago

No, the panel runs always in xdg-shell if started on wayland

Ok then Qt is using wayland but taskbar has no wayland backend yet because it comes from the other PR. It should assert (crash) printing "Only X11 is supported!".

If you install an IDE like Qt Creator, open the CMakeLists.txt file it eill load the process. Then build with Debug preset. To put a brakpoint, open a .cpp file and click over line number where you want to put it. Then start debugging with the green triangle button with the small bug.

gfgit commented 9 months ago

I may have found the problem. You are building with CMake "Release" preset which has the side effect of turning Q_ASSERT_X macro to no-op. This means program will not terminate and no message will be printing, so you have no clue of what the problem is. Then the program will crash soon after because of nullptr dereference.

stefonarch commented 9 months ago

Atm rebuilding with cmake .. -DCMAKE_PREFIX_PATH=/usr/local/bin -DCMAKE_BUILD_TYPE=Debug no build type option before.

So we should use a similar approach as used in desktopswitch? Disable taskbar if on wayland atm?

gfgit commented 9 months ago

I was right, in Release build I can reproduce your backtrace.

So we should use a similar approach as used in desktopswitch? Disable taskbar if on wayland atm?

Yes, instead of crashing, we put a "dummy" taskbar which will be empty and maybe show a nice error to user, or just write a message in the log.

gfgit commented 9 months ago

@tsujan Do you think it's better to keep methods of ILXQtTaskbarAbstractBackend pure virtual and subclass in a new LXQtTaskbarDummyBackend or instead add default dummy implementation directly to the parent class?

I like the second approach choice because it would allow gradually implementing a new backend without needing to override all methods from the start and also does not need a new class. But first approach might be cleaner. Like it's done in ILXQtPanel / LXQtPanel split.

gfgit commented 9 months ago

Changed my mind. First approach is better

stefonarch commented 9 months ago

I was right, in Release build I can reproduce your backtrace.

Nice, at least now only one difference remains and not two :)

gfgit commented 9 months ago

@stefonarch Should be fixed now. It will tell you in program's output that only X11 is supported. Taskbar will be empty

gfgit commented 9 months ago

Now if you run on Wayland without the Wayland backend it will print the error and fallback to dummy backend. When adding Wayland support everything will be fine as before. Should a new window system come up, it will display again the error message for it.

stefonarch commented 9 months ago

Perfect :)

screen_area_gio_21:18:10_

tsujan commented 9 months ago

@tsujan Do you think it's better to keep methods of ILXQtTaskbarAbstractBackend pure virtual....

I think that might be better, but I haven't started to look into it yet, while you know various aspects of your approach much better than me.

stefonarch commented 9 months ago

The Readme.md needs 2 changes at the end:

gfgit commented 8 months ago

Replaced by #2041