| davidedmundson | |
| romangg | |
| fvogt |
| Plasma |
As discussed the env variables are no longer exported. Thus Qt
applications follow the default qpa platform they are compiled with and
thus still function if they are packaged with a Qt without QtWayland.
Plasma's internal processes pick the qpa platform depending on the
session type as well as our flatpak apps.
KRunner and Plasmashell are adjusted to not leak the env variable they
set for themselves.
Started a wayland session, verified with KWin's debug console
that plasmashell and krunner are wayland. Launched kwrite from both plasma
and krunner and verified that it's xcb
| No Linters Available |
| No Unit Test Coverage |
If KWorkSpace::detectPlatform(argc, argv); were changed to edit argc and argv to pass -platform foo instead of setting QT_QPA_PLATFORM, the qunsetenv calls would become unnecessary.
but that sounds rather dangerous. I don't want to mess with the command line arguments
Why is that more dangerous than environment variables?
If argc and argv are passed to QApplication, you already know how it'll be parsed.
KWorkSpace::detectPlatform(argc, argv); is already testing the arguments if --platform flag is set and in this case would do nothing. Therefore I think it's safe to set the flag instead of the env variable as proposed by @fvogt.
Because I don't see how I could add something to argv without leaking memory and potentially crash. I consider it as dangerous to modify the argv array and adding elements to it. It's something I do not dare.
Right it's a pointer array. So detectPlatform would need to do some pointer and memory rearranging, what is not worth the hassle.
You don't modify it, you create a new one.
new_argv = new char*[argc+2]; new_argv[0] = argv[0]; new_argv[1] = "-platform"; new_argv[2] = "foo"; for(int i = 1...argc) new_argv[i+2] = argv[i];
Yes, you deliberately have to "leak" argc*sizeof(char*), but IMO it's more than worth not having to undo a very opaque hack everywhere where a child process could spawn.
Please move the qunsetenv into a method in KWorkSpace at least.
Please move the qunsetenv into a method in KWorkSpace at least.
Hm, that's not much prettier either. I really don't know how to make this look less hacky with editing env vars.
So I'm still very much in favor of the "-platform foo" idea.
Could we use a wrapper for QApplication being called by
QApplication app = KWorkSpace::createPlattformAwareQApplication(argc, argv);
and that sets and unsets the env variable in the wrapper call? With above code there might be a copy involved though.
I don't think Qt allows this, the copy and move ctor should be deleted.
There's another solution which might be cleaner and which KWin uses for that problem: modifying the process environment. I'll give a try whether this could work here.
Btw it was a surprise to me that this unsetting is needed at all.
Doesn't work for our case. KWin calls QProcess::setProcessEnvironment on every QProcess it starts. It's a clean solution but unsuited for KRunner and Plasmashell.
I think it is fine as it is now. We won't need it anymore anyway after we can expect Qt 5.11 and then can remove the "hack" again.
We won't need it anymore anyway after we can expect Qt 5.11 and then can remove the "hack" again.
I'm not sure about that - Qt 5.11 allows a fallback, but one reason for this patch was to use xcb + Xwayland instead of native qtwayland as it has certain issues...