[ksplash] Better support for Wayland with multiple screens
ClosedPublic

Authored by graesslin on Jul 21 2016, 10:52 AM.

Details

Summary

On Wayland ksplash only showed on one screen. This is due to QtWayland
not passing any hint to the compositor on which output to show the
fullscreen window. Thus the compositor placed all windows on the first
output.

This change uses KWayland to work around this problem by performing
a deeper integration. It doesn't use fullscreen windows any more, but
normal windows in combination with the PlasmaShell interface. That way
the windows can perform absolute positioning.

As the window is no longer fullscreen, it's marked as OnScreenDisplay
which makes sure that KWin raises it on top of all other windows. Also
ensures that opened windows aren't raised above it as used to happen.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
graesslin updated this revision to Diff 5381.Jul 21 2016, 10:52 AM
graesslin retitled this revision from to [ksplash] Better support for Wayland with multiple screens.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added a reviewer: Plasma.
Restricted Application added a project: Plasma. · View Herald TranscriptJul 21 2016, 10:52 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ivan added a subscriber: ivan.Jul 21 2016, 11:23 AM

Mostly looks ok, though I can not comment on wayland-specific things.

ksplash/ksplashqml/splashapp.cpp
156

Maybe it would be a good idea to add this as a static method to KWayland somwehere?

ksplash/ksplashqml/splashwindow.cpp
150

Can this be done differently - the splash is also used as a part of StartActive which is not a SplashApp?

graesslin added inline comments.Jul 21 2016, 11:39 AM
ksplash/ksplashqml/splashapp.cpp
156

nah, if at all to KWindowSystem.

ksplash/ksplashqml/splashwindow.cpp
150

What's "StartActive"?

ivan added inline comments.Jul 21 2016, 12:39 PM
ksplash/ksplashqml/splashapp.cpp
156

Ok, to KWindowSystem then. It could be a method with inner cached static result so that the consecutive calls to it do not need to perform any string comparisons.

This is not really a nice thing to have in all the places copy-pasted.

Maybe a enum-returning thing such as { Unknown = 0, X11 = 1, Wayland = 2, ForExtensibility = 255 }

ksplash/ksplashqml/splashwindow.cpp
150

We had a systemd-like startkde replacement in Plasma Active called StartActive, and sebas recently told me that it was agreed it should be revived (no idea ho was in that meeting :) ). And... it is revived.

And it shares the code with ksplashqml - the splash is integrated.

graesslin added inline comments.Jul 21 2016, 1:44 PM
ksplash/ksplashqml/splashapp.cpp
156

yes that's on my todo list since last Akademy... - in pretty much exactly the way you just outlined here.

ksplash/ksplashqml/splashwindow.cpp
150

well tricky then. I need to get to the kwayland integration done in KSplashApp. I need a way to get to that. And if it's not using KSplashApp that needs to be done somewhere else. If you have ideas how to solve this problem, please tell.

I don't know how start active is working and how it's integrating the code, but as a session controller I would assume it's getting started before the Wayland server is started. So the whole thing becomes mood anyway.

Anyway we had so many contenders for replacing ksmserver over the last years that I'm not going to consider hypothetical problems with possible contenders. Sorry.

ivan added inline comments.Jul 21 2016, 2:40 PM
ksplash/ksplashqml/splashwindow.cpp
150

It is not a ksmserver replacement, just a startkde replacement.

You can ignore this for now. Since we are now mostly in bug fixing mode, and 5.8 is LTS, startplasma (startactive) is not going to replace startkde just yet. I'll see what to do about it later then.

ivan added inline comments.Jul 21 2016, 2:43 PM
ksplash/ksplashqml/splashapp.cpp
156

Please do, I have grepped plasma codebases, and we have platformName() calls in quite a few places.

sebas accepted this revision.Jul 26 2016, 12:10 PM
sebas added a reviewer: sebas.
sebas added a subscriber: sebas.

The wayland portions look good, so as soon as you guys are happy, I am.

I guess the platform name stuff can happen async?

This revision is now accepted and ready to land.Jul 26 2016, 12:10 PM
In D2242#42628, @sebas wrote:

The wayland portions look good, so as soon as you guys are happy, I am.

I guess the platform name stuff can happen async?

Yes, that can be done later on. We need to mass adopt code anyway once we have it.

This revision was automatically updated to reflect the committed changes.