[wayland] Enable synchronized resizing for Xwayland clients
ClosedPublic

Authored by zzag on Apr 28 2020, 11:32 AM.

Details

Summary

Currently, synchronized resizing for X11 clients on Wayland is disabled
because updateXTime() is implemented only for the standalone X11 platform.
We cannot make roundtrips to Xwayland to query its current X11 time stamp
because we may end up in a dead lock. On the other hand, we cannot query
the time stamp asynchronously because doing so may introduce a lot of
regressions. The best option what we've got is to guess Xwayland's X11
time stamp. Fortunately, we know that Xwayland uses the system monotonic
time for time stamps. So, we can query the monotonic time in
updateXTime(). Note that when KWin is not running as a Wayland window
manager, it still does roundtrips to the X server.

Unfortunately, providing an implementation for updateXTime() and moving
the sync alarm filter to the toplevel directory is not enough. The latest
stable release of Xwayland (1.20.8) doesn't use multiple window buffers.
So, it's quite possible that an X11 client may update the buffer after
it's been committed and not released by the compositor yet. In our case,
when the frame window is resized, the buffer will be destroyed. This in
its turn _may_ render the previous and the current window pixmap invalid.

There are several ways to work around the window pixmap issue. (a) Just
ignore it. We probably don't want to do it because visual artifacts
caused by invalid window pixmaps are relatively severe. (b) Scratch the
contents of the buffer when the surface is committed rather than late in
the rendering process. The main drawback of this approach is that it
means we need to perform unnecessary copy operations if the client has
attached more than one shm buffer during a vblank interval. (c) Enable
synchronize resizing for Xwayland that uses multiple window buffers.

Even with all of its drawbacks, the option (b) is quite tempting since
it makes window pixmap management a little bit robust. However, it'd
require changes in the buffer management code, which I personally would
like to avoid. So, instead, I've picked the option (c). With the addition
of multiple window buffers in Xwayland 1.21, there should not be any
severe visual artifacts during a resize operation and we don't have to
tweak our buffer management code.

In order to disable synchronized resizing for Xwayland clients, this
patch series has introduced a new client class - XwaylandClient. The new
class is used to represent X11 clients that are connected to Xwayland.
The X11Client class is used to represent X11 clients when KWin runs as
not a Wayland window manager. The new class also allows to specialize
some code that is irrelevant to X11Client.

Test Plan

Built xserver from git, resized an X11 client that supports
_NET_WM_SYNC_REQUEST.

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.
zzag created this revision.Apr 28 2020, 11:32 AM
Restricted Application added a project: KWin. · View Herald TranscriptApr 28 2020, 11:32 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Apr 28 2020, 11:32 AM
ahiemstra added inline comments.
platform.cpp
508

You can use std::chrono::steady_clock instead for a nicer C++ API.

zzag added inline comments.Apr 29 2020, 5:17 AM
platform.cpp
508

Yeah, std::chrono provides nice time conversion helpers, but I still think it's better to use this implementation because it will yell when clock_gettime() fails. I doubt that we ever hit such an esoteric case, but if we do, the log message will be very helpful.

zzag added a reviewer: meven.Apr 29 2020, 8:13 AM
meven accepted this revision.Apr 29 2020, 11:13 AM

Did you test with XWayland 1.21 ?

All in all, I think this is the proper way to do it : leveraging the improvements from upstream.

platform.cpp
521

I was thinking we could remove the argument Application::TimestampUpdate::Always by simply doing an overflow check in setX11Time.

This revision is now accepted and ready to land.Apr 29 2020, 11:13 AM
zzag added a comment.Apr 29 2020, 11:32 AM

Did you test with XWayland 1.21 ?

I tested with Xwayland from git master (1.20.99). The fix should be available in 1.21.

platform.cpp
521

If we want to do something about the X11 time stamps wrapping around, I suggest to do it in another patch.

This revision was automatically updated to reflect the committed changes.