Add windowsystem plugin for KWin's qpa
ClosedPublic

Authored by graesslin on Jan 13 2019, 5:05 PM.

Details

Summary

KWindowSystem provides a plugin interface to have platform specific
implementations. So far KWin relied on the implementation in
KWayland-integration repository.

This is something I find unsuited, for the following reasons:

  • any test in KWin for functionality set through the plugin would fail
  • it's not clear what's going on where
  • in worst case some code could deadlock
  • KWin shouldn't use KWindowSystem and only a small subset is allowed

to be used

The last point needs some further explanation. KWin internally does not
and cannot use KWindowSystem. KWindowSystem (especially KWindowInfo) is
exposing information which KWin sets. It's more than weird if KWin asks
KWindowSystem for the state of a window it set itself. On X11 it's just
slow, on Wayland it can result in roundtrips to KWin itself which is
dangerous.

But due to using Plasma components we have a few areas where we use
KWindowSystem. E.g. a Plasma::Dialog sets a window type, the slide in
direction, blur and background contrast. This we want to support and
need to support. Other API elements we do not want, like for examples
the available windows. KWin internal windows either have direct access
to KWin or a scripting interface exposed providing (limited) access -
there is just no need to have this in KWindowSystem.

To make it more clear what KWin supports as API of KWindowSystem for
internal windows this change implements a stripped down version of the
kwayland-integration plugin. The main difference is that it does not use
KWayland at all, but a QWindow internal side channel.

To support this EffectWindow provides an accessor for internalWindow and
the three already mentioned effects are adjusted to read from the
internal QWindow and it's dynamic properties.

This change is a first step for a further refactoring. I plan to split
the internal window out of ShellClient into a dedicated class. I think
there are nowadays too many special cases. If it moves out there is the
question whether we really want to use Wayland for the internal windows
or whether this is just historic ballast (after all we used to use
qwayland for that in the beginning).

As the change could introduce regressions I'm targetting 5.16.

Test Plan

new test case for window type, manual testing using Alt+Tab
for the effects integration. Sliding popups, blur and contrast worked fine.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
graesslin created this revision.Jan 13 2019, 5:05 PM
Restricted Application added a project: KWin. · View Herald TranscriptJan 13 2019, 5:05 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
graesslin requested review of this revision.Jan 13 2019, 5:05 PM
zzag added a subscriber: zzag.Jan 13 2019, 5:32 PM
zzag added inline comments.
autotests/test_window_paint_data.cpp
62

style

effects/blur/blur.cpp
335–336

Would it be feasible to add EffectWindow *findInternalWindow(QWindow *window) to EffectsHandler?

graesslin marked 2 inline comments as done.Jan 14 2019, 5:34 PM
graesslin updated this revision to Diff 49464.Jan 14 2019, 5:46 PM
  • Add EffectsHandler::findWindow(QWindow*) -> EffectWindow*
  • Fix coding style
zzag accepted this revision.Jan 14 2019, 7:23 PM

This change is a first step for a further refactoring. I plan to split the internal window out of ShellClient into a dedicated class.

++

If it moves out there is the > question whether we really want to use Wayland for the internal windows or whether this is just historic ballast (after all we used to use qwayland for that in the beginning).

Generally speaking, it would be great to have protocol-agnostic internal clients, but tbh I haven't thought too much about this.

This revision is now accepted and ready to land.Jan 14 2019, 7:23 PM

I plan to split the internal window out of ShellClient into a dedicated class

Ack.

If it moves out there is the question whether we really want to use Wayland for the internal windows or whether this is just historic ballast

To support Plasma::Dialog we would need to put Shadows into KWindowSystem so that it gets the plugin infrastructure before we lose the wl_surface.

Though that's something I'd quite like to for code cleanliness purposes anyway.

In D18228#393160, @zzag wrote:

Generally speaking, it would be great to have protocol-agnostic internal clients, but tbh I haven't thought too much about this.

While thinking about the idea to split out internal window I also thought about this. I think it's doable and also not too much effort and could bring us quite some advantages like context sharing with QtQuick (c.f. thumbnails in Alt+Tab). The only thing I would not be too happy about is that it would be a violation of the X11 code freeze.

To support Plasma::Dialog we would need to put Shadows into KWindowSystem so that it gets the plugin infrastructure before we lose the wl_surface.

I knew there must be something I forgot...

Though that's something I'd quite like to for code cleanliness purposes anyway.

+ 1

graesslin updated this revision to Diff 49654.Jan 16 2019, 4:44 PM

make findWindow(QWindow*) also work for X11 by looking through Unmanaged

we would need to put Shadows into KWindowSystem

FYI, I've started this.
Rollout will become quite complicated with the whole plasma frameworks release schedule.

This revision was automatically updated to reflect the committed changes.