[PATCH xwayland] xwayland: Always update the wl_pointer cursor on pointer focus

Jonas Ådahl jadahl at gmail.com
Mon Sep 21 01:52:50 PDT 2015


In Wayland, a client (in this case XWayland) should set the cursor
surface when it receives pointer focus. Not doing this will leave the
curser at whatever it was previously.

When running on XWayland, the X server will not be the entity that controls
what actual pointer cursor is displayed, and it wont be notified about the
pointer cursor changes done by the Wayland compositor. This causes X11 clients
running via XWayland to end up with incorrect pointer cursors because the X
server believes that, if the cursor was previously set to the cursor C, if we
receive Wayland pointer focus over window W which also has the pointer cursor
C, we do not need to update it. This will cause us to end up with the wrong
cursor if cursor C was not the same one that was already set by the Wayland
compositor.

This patch introduces new API to make it possible for a XWayland to trigger
resetting of the pointer cursor sprite through the various layers that
deals with that. The new function that does this is InvalidateCursor().

XWayland will now call this function upon receiving pointer focus,
making sure that subsequent calls will trigger resetting of the pointer
cursor sprite.

Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
---

Hi,

This patch fixes the issue that X11 clients running via XWayland won't
update the pointer cursor when receiving pointer focus. It fixes it by
causing the existing path that may update the pointer cursor to always
trigger an update. This requires the new API introduced. It could
potentially be fixed with less API changes for example by always just
resending the wl_pointer_set_cursor request every time we receive
pointer focus, but this would mean sometimes send the request twice per
enter event.

Reproducing the issue is fairly easy:

 1. Open a Wayland window
 2. Open an X11 window
 3. Raise the Wayland window and place it so that it partly covers the
    X11 window
 4. Move the pointer slowly from the Wayland window onto the X11 window,
    making sure the resize cursor is displayed while getting close to the
    edge.
 5. Observe how the cursor stays the same (resize cursor) even after the
    focus has left the Wayland window and is now on the X11 window.

I suspect that it is undesirable to add these type of API, and if so
please let me know.


Jonas


 dix/events.c                 | 53 +++++++++++++++++++++++++++++++++++---------
 hw/xwayland/xwayland-input.c |  5 ++++-
 include/dix.h                |  4 ++++
 include/inputstr.h           |  1 +
 include/scrnintstr.h         |  4 ++++
 mi/mipointer.c               | 17 +++++++++++++-
 mi/mipointrst.h              |  1 +
 7 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/dix/events.c b/dix/events.c
index efaf91d..983867a 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -902,6 +902,18 @@ PointerConfinedToScreen(DeviceIntPtr pDev)
     return pDev->spriteInfo->sprite->confined;
 }
 
+static ScreenPtr
+ScreenFromSprite(SpritePtr pSprite)
+{
+#ifdef PANORAMIX
+    /* XXX: is this really necessary?? (whot) */
+    if (!noPanoramiXExtension)
+        return pSprite->screen;
+    else
+#endif
+        return pSprite->hotPhys.pScreen;
+}
+
 /**
  * Update the sprite cursor to the given cursor.
  *
@@ -914,23 +926,23 @@ ChangeToCursor(DeviceIntPtr pDev, CursorPtr cursor)
 {
     SpritePtr pSprite = pDev->spriteInfo->sprite;
     ScreenPtr pScreen;
+    CursorPtr prevCursor;
 
-    if (cursor != pSprite->current) {
-        if ((pSprite->current->bits->xhot != cursor->bits->xhot) ||
+    if (cursor != pSprite->current || pSprite->invalidated) {
+        pSprite->invalidated = FALSE;
+        if (!pSprite->current ||
+            (pSprite->current->bits->xhot != cursor->bits->xhot) ||
             (pSprite->current->bits->yhot != cursor->bits->yhot))
             CheckPhysLimits(pDev, cursor, FALSE, pSprite->confined,
                             (ScreenPtr) NULL);
-#ifdef PANORAMIX
-        /* XXX: is this really necessary?? (whot) */
-        if (!noPanoramiXExtension)
-            pScreen = pSprite->screen;
-        else
-#endif
-            pScreen = pSprite->hotPhys.pScreen;
+        pScreen = ScreenFromSprite(pSprite);
 
         (*pScreen->DisplayCursor) (pDev, pScreen, cursor);
-        FreeCursor(pSprite->current, (Cursor) 0);
+
+        prevCursor = pSprite->current;
         pSprite->current = RefCursor(cursor);
+        if (prevCursor)
+            FreeCursor(prevCursor, (Cursor) 0);
     }
 }
 
@@ -3262,6 +3274,27 @@ InitializeSprite(DeviceIntPtr pDev, WindowPtr pWin)
 #endif
 }
 
+void
+InvalidateSprite(DeviceIntPtr dev)
+{
+    SpritePtr pSprite;
+    ScreenPtr pScreen;
+
+    if (!DevHasCursor(dev))
+        return;
+
+    pSprite = dev->spriteInfo->sprite;
+    if (!pSprite)
+        return;
+
+    if (!pSprite->current)
+        return;
+
+    pSprite->invalidated = TRUE;
+    pScreen = ScreenFromSprite(pSprite);
+    (*pScreen->InvalidateCursor) (dev, pScreen, pSprite->current);
+}
+
 void FreeSprite(DeviceIntPtr dev)
 {
     if (DevHasCursor(dev) && dev->spriteInfo->sprite) {
diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
index 32007de..ef9b275 100644
--- a/hw/xwayland/xwayland-input.c
+++ b/hw/xwayland/xwayland-input.c
@@ -155,6 +155,7 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer,
 {
     struct xwl_seat *xwl_seat = data;
     DeviceIntPtr dev = xwl_seat->pointer;
+    DeviceIntPtr master;
     int i;
     int sx = wl_fixed_to_int(sx_w);
     int sy = wl_fixed_to_int(sy_w);
@@ -175,8 +176,10 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer,
 
     xwl_seat->focus_window = wl_surface_get_user_data(surface);
 
+    master = GetMaster(dev, MASTER_POINTER);
     (*pScreen->SetCursorPosition) (dev, pScreen, sx, sy, TRUE);
-    CheckMotion(NULL, GetMaster(dev, MASTER_POINTER));
+    InvalidateSprite(master);
+    CheckMotion(NULL, master);
 
     /* Ideally, X clients shouldn't see these button releases.  When
      * the pointer leaves a window with buttons down, it means that
diff --git a/include/dix.h b/include/dix.h
index 921156b..d1ce155 100644
--- a/include/dix.h
+++ b/include/dix.h
@@ -413,6 +413,10 @@ DeliverTouchEvents(DeviceIntPtr /* dev */ ,
 extern void
 InitializeSprite(DeviceIntPtr /* pDev */ ,
                  WindowPtr /* pWin */ );
+
+extern void
+InvalidateSprite(DeviceIntPtr /* pDev */);
+
 extern void
 FreeSprite(DeviceIntPtr pDev);
 
diff --git a/include/inputstr.h b/include/inputstr.h
index 568f5f9..325f456 100644
--- a/include/inputstr.h
+++ b/include/inputstr.h
@@ -212,6 +212,7 @@ typedef struct _GrabRec {
  */
 typedef struct _SpriteRec {
     CursorPtr current;
+    Bool invalidated;
     BoxRec hotLimits;           /* logical constraints of hot spot */
     Bool confined;              /* confined to screen */
     RegionPtr hotShape;         /* additional logical shape constraint */
diff --git a/include/scrnintstr.h b/include/scrnintstr.h
index 2e617c4..2e3af7b 100644
--- a/include/scrnintstr.h
+++ b/include/scrnintstr.h
@@ -232,6 +232,9 @@ typedef Bool (*SetCursorPositionProcPtr) (DeviceIntPtr /* pDev */ ,
                                           int /*x */ ,
                                           int /*y */ ,
                                           Bool /*generateEvent */ );
+typedef void (*InvalidateCursorProcPtr) (DeviceIntPtr /* pDev */ ,
+                                         ScreenPtr /*pScreen */,
+                                         CursorPtr /*pCursor */);
 
 typedef Bool (*CreateGCProcPtr) (GCPtr /*pGC */ );
 
@@ -524,6 +527,7 @@ typedef struct _Screen {
     UnrealizeCursorProcPtr UnrealizeCursor;
     RecolorCursorProcPtr RecolorCursor;
     SetCursorPositionProcPtr SetCursorPosition;
+    InvalidateCursorProcPtr InvalidateCursor;
 
     /* GC procedures */
 
diff --git a/mi/mipointer.c b/mi/mipointer.c
index ada1ab5..7538adc 100644
--- a/mi/mipointer.c
+++ b/mi/mipointer.c
@@ -36,6 +36,7 @@ in this Software without prior written authorization from The Open Group.
  *      miPointerRealizeCursor
  *      miPointerUnrealizeCursor
  *      miPointerSetCursorPosition
+ *      miPointerInvalidateCursor
  *      miRecolorCursor
  *      miPointerDeviceInitialize
  *      miPointerDeviceCleanup
@@ -91,6 +92,8 @@ static void miPointerCursorLimits(DeviceIntPtr pDev, ScreenPtr pScreen,
                                   BoxPtr pTopLeftBox);
 static Bool miPointerSetCursorPosition(DeviceIntPtr pDev, ScreenPtr pScreen,
                                        int x, int y, Bool generateEvent);
+static void miPointerInvalidateCursor(DeviceIntPtr pDev, ScreenPtr pScreen,
+                                      CursorPtr pCursor);
 static Bool miPointerCloseScreen(ScreenPtr pScreen);
 static void miPointerMove(DeviceIntPtr pDev, ScreenPtr pScreen, int x, int y);
 static Bool miPointerDeviceInitialize(DeviceIntPtr pDev, ScreenPtr pScreen);
@@ -132,6 +135,7 @@ miPointerInitialize(ScreenPtr pScreen,
     pScreen->RealizeCursor = miPointerRealizeCursor;
     pScreen->UnrealizeCursor = miPointerUnrealizeCursor;
     pScreen->SetCursorPosition = miPointerSetCursorPosition;
+    pScreen->InvalidateCursor = miPointerInvalidateCursor;
     pScreen->RecolorCursor = miRecolorCursor;
     pScreen->DeviceCursorInitialize = miPointerDeviceInitialize;
     pScreen->DeviceCursorCleanup = miPointerDeviceCleanup;
@@ -273,6 +277,15 @@ miPointerSetCursorPosition(DeviceIntPtr pDev, ScreenPtr pScreen,
     return TRUE;
 }
 
+static void
+miPointerInvalidateCursor(DeviceIntPtr pDev, ScreenPtr pScreen,
+                          CursorPtr pCursor)
+{
+    miPointerPtr pPointer = MIPOINTER(pDev);
+
+    pPointer->cursorInvalidated = TRUE;
+}
+
 void
 miRecolorCursor(DeviceIntPtr pDev, ScreenPtr pScr,
                 CursorPtr pCurs, Bool displayed)
@@ -448,7 +461,8 @@ miPointerUpdateSprite(DeviceIntPtr pDev)
     /*
      * if the cursor has changed, display the new one
      */
-    else if (pPointer->pCursor != pPointer->pSpriteCursor) {
+    else if (pPointer->pCursor != pPointer->pSpriteCursor ||
+             pPointer->cursorInvalidated) {
         pCursor = pPointer->pCursor;
         if (!pCursor ||
             (pCursor->bits->emptyMask && !pScreenPriv->showTransparent))
@@ -465,6 +479,7 @@ miPointerUpdateSprite(DeviceIntPtr pDev)
         if (pPointer->pCursor && !pPointer->pCursor->bits->emptyMask)
             (*pScreenPriv->spriteFuncs->MoveCursor) (pDev, pScreen, x, y);
     }
+    pPointer->cursorInvalidated = FALSE;
 }
 
 /**
diff --git a/mi/mipointrst.h b/mi/mipointrst.h
index 104e45c..38a9f6f 100644
--- a/mi/mipointrst.h
+++ b/mi/mipointrst.h
@@ -44,6 +44,7 @@ typedef struct {
     int x, y;                   /* hot spot location */
     int devx, devy;             /* sprite position */
     Bool generateEvent;         /* generate an event during warping? */
+    Bool cursorInvalidated;     /* update cursor even if it didn't change */
 } miPointerRec, *miPointerPtr;
 
 typedef struct {
-- 
2.4.3



More information about the xorg-devel mailing list