Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP Qt6 port #2024

Merged
merged 34 commits into from
Mar 25, 2024
Merged

WIP Qt6 port #2024

merged 34 commits into from
Mar 25, 2024

Conversation

gfgit
Copy link
Member

@gfgit gfgit commented Feb 8, 2024

@AzumaHazuki
Copy link

This is amazing! Where did you learn all this coding?

@gfgit
Copy link
Member Author

gfgit commented Feb 10, 2024

Hi! Thanks. Well it does not work yet, I need more time to properly fix compile errors.
Anyway it's not that difficult, I've just read Qt online documentation and made some experiments on small test programs until I get the results I want. The key for me is having fun while coding, otherwise is not worth it.

@stefonarch
Copy link
Member

Looks like only fancymenu and few other plugins build atm. Made a list here.

@gfgit
Copy link
Member Author

gfgit commented Feb 13, 2024

Hi, thanks for testing. I've worked on a second version of the patch plus some code style fixes detected by Clazy. I will force push soon.
All plugin are working except sysstat because library is not ported yet.

@gfgit
Copy link
Member Author

gfgit commented Feb 13, 2024

XCB not found it's because CMake components are uppercase and without xcb- prefix. I wonder why it worked until now...

Also there are some issues:
Ldd does not find liblxqt.so.2 so it refuses to launch panel. I had to LD_PRELOAD it.
I don't yet understand how this works, library is in correct path just like Qt6Xdg.so and also listed in ldconfig -p.
Running sudo ldconfig seems to rebuild the cache and fix the issue.

Another issue is panel configuration not getting installed, resulting in an empty transparent panel.
Need to investigate. For now I've manually copied it from repository to home folder

@stefonarch
Copy link
Member

All plugin are working except sysstat because library is not ported yet.

That's nice to hear :)

@gfgit
Copy link
Member Author

gfgit commented Feb 13, 2024

I didn't need QTextCodec from Qt5Compat module. When I'll push, plaese check again if it's needed

@tsujan
Copy link
Member

tsujan commented Feb 13, 2024

I didn't need QTextCodec from Qt5Compat module.

Just as a general note, and not a review:

Forget about Qt5Compat everywhere. Legacy encodings aren't supported by Qt6, and so, we don't have a reason to support them by force.

@gfgit
Copy link
Member Author

gfgit commented Feb 13, 2024

NOTE: it needs recent changes from lxqt-globalkeys#278 branch which I've just pushed.

@stefonarch
Copy link
Member

stefonarch commented Feb 13, 2024

I needed first lxqt/libsysstat#47 and change dbusmenu-qt6 → dbusmenu-lxqt to start compilation but I'm stuck atm at fatal error: lxqt-globalkeys.h: No such file or directory for several plugins.
When I change it to
#include <lxqt-globalkeys/lxqt-globalkeys.h> it goes to LXQtGlobalKeys: no such file or... and when I include that again not found, but it's all present in /usr/local/include.

EDIT: Turns out this was caused by my removing of lxqt2 everywhere somehow, plain PR builds fine.

Systat-plugin has to be disabled as it needs porting to QRegularExpression and I ran into the same ldconfig issue but it prevented installing (sudo ldconfig didn't help):


[ 99%] Linking CXX executable lxqt-panel
/usr/bin/ld: CMakeFiles/lxqt-panel.dir/lxqt-panel_autogen/mocs_compilation.cpp.o: in function `QtPrivate::QDebugStreamOperatorForType<LXQt::PluginInfo, true>::debugStream(QtPrivate::QMetaTypeInterface const*, QDebug&, void const*)':
mocs_compilation.cpp:(.text._ZN9QtPrivate27QDebugStreamOperatorForTypeIN4LXQt10PluginInfoELb1EE11debugStreamEPKNS_18QMetaTypeInterfaceER6QDebugPKv[_ZN9QtPrivate27QDebugStreamOperatorForTypeIN4LXQt10PluginInfoELb1EE11debugStreamEPKNS_18QMetaTypeInterfaceER6QDebugPKv]+0x36): undefined reference to `operator<<(QDebug, LXQt::PluginInfo const&)'
collect2: error: ld returned 1 exit status

@gfgit
Copy link
Member Author

gfgit commented Feb 13, 2024

Ah yes, this is because QDebug operators are not exported. I've fixed it but forgot to push

@gfgit
Copy link
Member Author

gfgit commented Feb 14, 2024

@stefonarch I've pushed fix in liblxqt#337

@gfgit
Copy link
Member Author

gfgit commented Feb 14, 2024

Ah yes, this is because QDebug operators are not exported. I've fixed it but forgot to push

Another issue is panel configuration not getting installed, resulting in an empty transparent panel.
Need to investigate. For now I've manually copied it from repository to home folder

I think this is because I'm installing to /usr/local
The file is there but panel does not consider this path

@stefonarch
Copy link
Member

Compiles perfectly fine now without any warning, except that I had to disable statusnotifier plugin as I didn't manage to include dbusmenu-lxqt anymore, not sure if I changed something as I hit a git stash by mistake.

Panel conf is installed but not loaded at first run, don't remember how this is supposed to wirk

-- Installing: /usr/local/bin/lxqt-panel
-- Set non-toolchain portion of runtime path of "/usr/local/bin/lxqt-panel" to ""
-- Installing: /usr/local/share/lxqt/panel.conf
-- Installing: /usr/local/include/lxqt/lxqtpanelglobals.h

@gfgit
Copy link
Member Author

gfgit commented Feb 14, 2024

What's the difference between dbusmenu-qt and dbusmenu-lxqt?

I can build it with neon package for dbusmenu qt6 dev

@tsujan
Copy link
Member

tsujan commented Feb 14, 2024

First, libdbusmenu-qt6 couldn't be compiled anymore. Second, it didn't exist in all distros (Arch used a self-made patch). So, we got rid of all problems by forking it (as KDE did years ago). In this way, we could fix its probable bugs too.

The Qt6 brannch of lxqt-qtplugin already depends on libdbusmenu-lxqt.

@stefonarch
Copy link
Member

stefonarch commented Feb 14, 2024

Found my error, forgot the second part here #include <dbusmenu-lxqt/dbusmenuimporter.h>.

So only sysstat plugin is missing.

@stefonarch
Copy link
Member

Just found out in a nested labwc window that the qt6-panel won't work on wayland:
screen_area_gio_13:53:45_

@gfgit
Copy link
Member Author

gfgit commented Feb 15, 2024

Just found out in a nested labwc window that the qt6-panel won't work on wayland: screen_area_gio_13:53:45_

Well of course KX11Extras does not work on wayland and we use this library. As a workaround force it to use xwayland. Taskbar will be empty

@tsujan
Copy link
Member

tsujan commented Feb 15, 2024

Haven't we already disabled X11-dependent plugins under Wayland? If not, we should do it.

@stefonarch
Copy link
Member

stefonarch commented Feb 15, 2024

Well of course KX11Extras does not work on wayland and we use this library. As a workaround force it to use xwayland. Taskbar will be empty

Haven't we already disabled X11-dependent plugins under Wayland? If not, we should do it.

Only the desktopswitch is disabled as it will crash the panel, the others can be added but do nothing. Now it's about the panel itself too.

I use without issues the qt5 version natively, so something has changed there in KF6 now.

@stefonarch
Copy link
Member

stefonarch commented Feb 15, 2024

screen_area_gio_14:23:53_

all files which use it, at least lxqtpanel.cpp and plugin.cpp would be needed for now.

@tsujan
Copy link
Member

tsujan commented Feb 15, 2024

If KF6 doesn't exclude them, we could do it. Not a theoretical hassle; just a boring job.

@gfgit
Copy link
Member Author

gfgit commented Feb 15, 2024

Well I have added some assert on X11 connection somwhere. It was just for testing but it's also correct because we should exclude X11 specific parts on Wayland

@tsujan
Copy link
Member

tsujan commented Feb 15, 2024

I may be wrong (being busy with other codes), but I think KF5 did it for its methods.

@stefonarch
Copy link
Member

Looking at KX11Extras it looks (for the panel) some functions on wayland should use foreing-toplevel-protocol, like static QList<WId> windows(); and others, if I got it right. Other functions (number of desktops and related) are just not available as protocol yet.

Running the panel in xwayland mode is nice to see and working fine now on labwc, but all apps launched by the menu inherit xwayland too.

@stefonarch
Copy link
Member

stefonarch commented Feb 15, 2024

Rechecked for good and saw that the panel opens, so those are just warnings not blockers. Looks like I had to stop the panel in the openbox session first.

@stefonarch
Copy link
Member

stefonarch commented Feb 19, 2024

After recent updates of KF6 I had to recompile it. I see (also before) this:

$ lxqt-panel
starting
WinIdChange 1600009 handle QWidgetWindow(0x56007fda3e90, name="LXQtPanel panel1Window") QScreen(0x56007fa91b30, name="Virtual1")
StatusNotifierProxy, services: QList()
Manager selection claimed
trying to dock window  18874385
adding damage watch for  18874385
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.Menu was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.IconThemePath was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.ToolTip was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.OverlayIconName was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.IconName was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.AttentionIconName was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.IconThemePath was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.ToolTip was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.OverlayIconPixmap was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.AttentionIconPixmap was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.IconName was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.IconThemePath was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.ToolTip was not found in object /StatusNotifierItem)
Error on DBus request(:1.71,/StatusNotifierItem): QDBusError(org.freedesktop.DBus.Error.UnknownProperty, Property org.kde.StatusNotifierItem.IconName was not found in object /StatusNotifierItem)

damage watch looks interesting: https://wayland-book.com/surfaces-in-depth/damaging-surfaces.html

TODO: Still don't know why prefix does not work

- Remove includes to QDesktopWidget
@gfgit
Copy link
Member Author

gfgit commented Mar 20, 2024

I suggest this time to NOT squash the commit before merging.
It would result in an commit too big too be read.
I've carefully divided commits to make changes easily understandable and this could be useful as a reference
for other people wanting to port their software to Qt 6.
Some commits have "TODO" in their messages, this of course will be removed if changes.
Too reduce the number of commits some of them can be grouped based on topic.

@gfgit
Copy link
Member Author

gfgit commented Mar 20, 2024

CI doesn't find Qt6 dependencies

gfgit added 16 commits March 21, 2024 00:07
NOTE: Fix compilation with Xlib.h

Xlib.h is tricky because at line 82 has:

`#define Bool int`

And QtCore/qjsonvalue.h at line 28 has:

```
enum Type {
   ...
   Bool = 0x1,
   ...
}
```

This is fixed temporarily by moving Xlib.h
include to last and undefine Bool macro

Long term we want to hide X11 related code and encapsulate in a backend.
This backend will not expose X11 headers.
Many X11 specific functions got moved away from KWindowSystem header.

Long term we will move everything to a backend.
Teporary objects get destroyed after call ends.
State is not updated.
- Reported by Clazy
- Also fix formatting a bit
- Use Qt::CaseInsensitive instead
- Pass argument by reference
- Format braces
NOTE: QX11Info::getTimestamp() was replaced with XCB_CURRENT_TIME macro.

I don't know if this breaks functionality.
Getting real timestamp involves messing with Qt private APIs
- Forward declare classes
- More refactory could be done here
- Use Xcb::ScopedCPointer for POD types
In Qt6 it's an alias
TODO: is behavior equivalent? Conversion flags...
@tsujan
Copy link
Member

tsujan commented Mar 20, 2024

I suggest this time to NOT squash the commit before merging.

OK.

CI doesn't find Qt6 dependencies

No worries; we've got used to CI's temporary issues and tolerate them.

Will merge all Qt6-related PRs in the proper order tomorrow.

@stefonarch stefonarch marked this pull request as ready for review March 25, 2024 06:30
@tsujan tsujan merged commit 64f8a5a into lxqt:master Mar 25, 2024
1 check failed
@gfgit
Copy link
Member Author

gfgit commented Mar 25, 2024

@tsujan It's quite difficult to understand changes from the squashed single commit

@tsujan
Copy link
Member

tsujan commented Mar 25, 2024

PRs should be squashed before being merged. A clean history is a must. I just agreed that you don't do it yourself.

As for code readers, it's impossible to make everything straightforward for them; they need to delve into codes.

@gfgit
Copy link
Member Author

gfgit commented Mar 25, 2024

PRs should be squashed before being merged. A clean history is a must. I just agreed that you don't do it yourself.

As for code readers, it's impossible to make everything straightforward for them; they need to delve into codes.

Ok for clean history but at least squashing in groups leading to say 3 final commits instead to a single one coild have been a good trade off

@tsujan
Copy link
Member

tsujan commented Mar 25, 2024

@gfgit
It wasn't my personal decision. I did it according to a sort of tradition that has existed in LXQt for a long time (I got familiar with it after joining LXQt years ago). Some devs even believe that the PR maker himself should squash his commits, because a PR is about a single change.

@tsujan
Copy link
Member

tsujan commented Mar 25, 2024

Oh, and it's important that each PR should be about a single subject — again, not my decision, although I fully agree with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants