[PATCH xserver v2 5/5] xwayland: use _XWAYLAND_ALLOW_COMMITS property

Pekka Paalanen ppaalanen at gmail.com
Fri Dec 9 12:24:16 UTC 2016


From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

The X11 window manager (XWM) of a Wayland compositor can use the
_XWAYLAND_ALLOW_COMMITS property to control when Xwayland sends
wl_surface.commit requests. If the property is not set, the behaviour
remains what it was.

XWM uses the property to inhibit commits until the window is ready to be
shown. This gives XWM time to set up the window decorations and internal
state before Xwayland does the first commit. XWM can use this to ensure
the first commit carries fully drawn decorations and the window
management state is correct when the window becomes visible.

Setting the property to zero inhibits further commits, and setting it to
non-zero allows commits. Deleting the property allows commits.

When the property is changed from zero to non-zero, there will be a
commit on next block_handler() call provided that some damage has been
recorded.

Without this patch (i.e. with the old behaviour) Xwayland can and will
commit the surface very soon as the application window has been realized
and drawn into.  This races with XWM and may cause visible glitches.

v2:
- use PropertyStateCallback instead of XACE, based on the patch
  "xwayland: Track per-window support for netwm frame sync" by
  Adam Jackson
- check property type is XA_CARDINAL
- drop a useless memcpy()

Weston Bug: https://phabricator.freedesktop.org/T7622
Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
---
 hw/xwayland/xwayland.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/xwayland/xwayland.h |   3 ++
 2 files changed, 117 insertions(+)

diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 9d79554..04a068f 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -27,6 +27,7 @@
 
 #include <stdio.h>
 
+#include <X11/Xatom.h>
 #include <selection.h>
 #include <micmap.h>
 #include <misyncshm.h>
@@ -34,6 +35,7 @@
 #include <glx_extinit.h>
 #include <os.h>
 #include <xserver_poll.h>
+#include <propertyst.h>
 
 #ifdef XF86VIDMODE
 #include <X11/extensions/xf86vmproto.h>
@@ -115,6 +117,86 @@ xwl_screen_get(ScreenPtr screen)
     return dixLookupPrivate(&screen->devPrivates, &xwl_screen_private_key);
 }
 
+static void
+xwl_window_set_allow_commits_from_property(struct xwl_window *xwl_window,
+                                           PropertyPtr prop)
+{
+    static Bool warned = FALSE;
+    CARD32 *propdata;
+
+    if (prop->propertyName != xwl_window->xwl_screen->allow_commits_prop)
+        FatalError("Xwayland internal error: prop mismatch in %s.\n", __func__);
+
+    if (prop->type != XA_CARDINAL || prop->format != 32 || prop->size != 1) {
+        /* Not properly set, so fall back to safe and glitchy */
+        xwl_window->allow_commits = TRUE;
+
+        if (!warned) {
+            LogMessage(X_WARNING, "Window manager is misusing property %s.\n",
+                       NameForAtom(prop->propertyName));
+            warned = TRUE;
+        }
+        return;
+    }
+
+    propdata = prop->data;
+    xwl_window->allow_commits = !!propdata[0];
+    DebugF("xwayland: win %d allow_commits = %d\n",
+           xwl_window->window->drawable.id, xwl_window->allow_commits);
+}
+
+static void
+xwl_property_callback(CallbackListPtr *pcbl, void *closure,
+                      void *calldata)
+{
+    ScreenPtr screen = closure;
+    PropertyStateRec *rec = calldata;
+    struct xwl_screen *xwl_screen;
+    struct xwl_window *xwl_window;
+    Bool old_allow_commits;
+
+    if (rec->win->drawable.pScreen != screen)
+        return;
+
+    xwl_window = xwl_window_get(rec->win);
+    if (!xwl_window)
+        return;
+
+    xwl_screen = xwl_screen_get(screen);
+    if (rec->prop->propertyName != xwl_screen->allow_commits_prop)
+        return;
+
+    /* Handle _XWAYLAND_ALLOW_COMMITS property changes */
+
+    old_allow_commits = xwl_window->allow_commits;
+
+    switch (rec->state) {
+    case PropertyNewValue:
+        xwl_window_set_allow_commits_from_property(xwl_window, rec->prop);
+        break;
+
+    case PropertyDelete:
+        xwl_window->allow_commits = TRUE;
+        DebugF("xwayland: win %d allow_commit reverts to uncontrolled.\n",
+               xwl_window->window->drawable.id);
+        break;
+
+    default:
+        break;
+    }
+
+    /* If allow_commits turned from off to on, discard any frame
+     * callback we might be waiting for so that a new buffer is posted
+     * immediately through block_handler() if there is damage to post.
+     */
+    if (!old_allow_commits && xwl_window->allow_commits) {
+        if (xwl_window->frame_callback) {
+            wl_callback_destroy(xwl_window->frame_callback);
+            xwl_window->frame_callback = NULL;
+        }
+    }
+}
+
 static Bool
 xwl_close_screen(ScreenPtr screen)
 {
@@ -122,6 +204,8 @@ xwl_close_screen(ScreenPtr screen)
     struct xwl_output *xwl_output, *next_xwl_output;
     struct xwl_seat *xwl_seat, *next_xwl_seat;
 
+    DeleteCallback(&PropertyStateCallback, xwl_property_callback, screen);
+
     xorg_list_for_each_entry_safe(xwl_output, next_xwl_output,
                                   &xwl_screen->output_list, link)
         xwl_output_destroy(xwl_output);
@@ -262,6 +346,25 @@ xwl_pixmap_get(PixmapPtr pixmap)
 }
 
 static void
+xwl_window_init_allow_commits(struct xwl_window *xwl_window)
+{
+    PropertyPtr prop = NULL;
+    int ret;
+
+    ret = dixLookupProperty(&prop, xwl_window->window,
+                            xwl_window->xwl_screen->allow_commits_prop,
+                            serverClient, DixReadAccess);
+    if (ret == Success && prop) {
+        xwl_window_set_allow_commits_from_property(xwl_window, prop);
+    }
+    else {
+        xwl_window->allow_commits = TRUE;
+        DebugF("xwayland: win %d allow_commit is uncontrolled.\n",
+               xwl_window->window->drawable.id);
+    }
+}
+
+static void
 send_surface_id_event(struct xwl_window *xwl_window)
 {
     static const char atom_name[] = "WL_SURFACE_ID";
@@ -376,6 +479,8 @@ xwl_realize_window(WindowPtr window)
     dixSetPrivate(&window->devPrivates, &xwl_window_private_key, xwl_window);
     xorg_list_init(&xwl_window->link_damage);
 
+    xwl_window_init_allow_commits(xwl_window);
+
     return ret;
 
 err_surf:
@@ -505,6 +610,9 @@ xwl_screen_post_damage(struct xwl_screen *xwl_screen)
         if (xwl_window->frame_callback)
             continue;
 
+        if (!xwl_window->allow_commits)
+            continue;
+
         xwl_window_post_damage(xwl_window);
     }
 }
@@ -694,6 +802,7 @@ wm_selection_callback(CallbackListPtr *p, void *data, void *arg)
 static Bool
 xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
 {
+    static const char allow_commits[] = "_XWAYLAND_ALLOW_COMMITS";
     struct xwl_screen *xwl_screen;
     Pixel red_mask, blue_mask, green_mask;
     int ret, bpc, green_bpc, i;
@@ -849,6 +958,11 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
     pScreen->CursorWarpedTo = xwl_cursor_warped_to;
     pScreen->CursorConfinedTo = xwl_cursor_confined_to;
 
+    xwl_screen->allow_commits_prop = MakeAtom(allow_commits,
+                                              strlen(allow_commits),
+                                              TRUE);
+    AddCallback(&PropertyStateCallback, xwl_property_callback, pScreen);
+
     return ret;
 }
 
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index 5e5624b..91b7620 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -99,6 +99,8 @@ struct xwl_screen {
     void *egl_display, *egl_context;
     struct gbm_device *gbm;
     struct glamor_context *glamor_ctx;
+
+    Atom allow_commits_prop;
 };
 
 struct xwl_window {
@@ -109,6 +111,7 @@ struct xwl_window {
     DamagePtr damage;
     struct xorg_list link_damage;
     struct wl_callback *frame_callback;
+    Bool allow_commits;
 };
 
 #define MODIFIER_META 0x01
-- 
2.7.3



More information about the xorg-devel mailing list