Make EffectsHandlerImpl::announceSupportProperty work without X11
ClosedPublic

Authored by graesslin on Aug 23 2017, 9:44 AM.

Details

Summary

announceSupportProperty is called from the effects on startup. It
registers the property on the X11 root window. If we would start
kwin_wayland without XWayland support this would result in a crash.

This change refactors the code so that it still registers the property,
but does not try to interact with X11. Once X11 support is available it
does the actual registering.

But this means that the effects get an incorrect atom returned. This
needs additional changes. E.g. they could also react to the
x11ConnectionChanged and register again, then they would get the proper
atom. This would also support restart of XWayland.

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.Aug 23 2017, 9:44 AM
Restricted Application added a project: KWin. · View Herald TranscriptAug 23 2017, 9:44 AM
Restricted Application added subscribers: kwin, plasma-devel. · View Herald Transcript

I assume this is all to make xwayland socket activated so X11ConnectionChanged will come later.

You're going out of your way to make sure we when we do get an X11 connection we register any atoms that had previously called announceSupportProperty. Which is great, except it won't work for:
SlidingPopups, KScreen, PresentWindows which all cache the first returned atom by this method.

They'll need updating to either monitor the x11ConnectionChanged signal themselves or to just not cache the atoms themselves.

I assume this is all to make xwayland socket activated so X11ConnectionChanged will come later.

Also. First of all I'm interested in being able to start without X support.

You're going out of your way to make sure we when we do get an X11 connection we register any atoms that had previously called announceSupportProperty. Which is great, except it won't work for:
SlidingPopups, KScreen, PresentWindows which all cache the first returned atom by this method.

See the description: I'm aware of it.

davidedmundson accepted this revision.Aug 23 2017, 11:39 AM
This revision is now accepted and ready to land.Aug 23 2017, 11:39 AM

Pretty uninformed consideration:
When dealing with X11/wayland hybrids, won't one end up with two property indexes which are hard -if not impossible- to align (because one will be assigned by kwin internally and the other by the casual xwayland server)?
Might the xwayland property even change over the runtime of kwin (is the xwayland server conditionally removed)?

Would it in this case not be required to abstract the property query with a mapper in general (or abort x11 properties and always ask kwin via dbus resp. the effects API for a purely internally kept property)?

Random sidenote:
registerPropertyType() has a hash trap (unregistering a non-existing atom will insert it forever)

@luebking I don't really follow you. KWin can only talk with one X server. As the properties are resolved against the one and only X server there is no alignement problem. What might happen in future is that we want to support a crashing XWayland server. In this case of cause all cached properties need to be discarded.

This revision was automatically updated to reflect the committed changes.

I take there's no such support announcement on native wayland at all?

I take there's no such support announcement on native wayland at all?

It's different. We have dedicated protocols for them and those are announced through the Wayland protocol registry.