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

[Olivier Fourdan]

Hi Jonas,

> 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 works around this by, when receiving pointer focus, getting
> the private mipointer struct changing the "current sprite" pointer to
> an invalid cursor in order to trigger the update path next time a cursor
> is displayed by dix.
> 
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> 
> This version of the patch doesn't do any API changes, but instead simply
> copies private looking parts (MIPOINTER macro) from mipointer.c and pokes
> around in the struct behind its back. Personally I think it'd be nicer to
> do this via some miPointerInvalidateCursor() call, but that'd mean
> extending the API.
> 
> 
> Jonas
> 
>  hw/xwayland/xwayland-input.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
> index 0515eb9..7494e63 100644
> --- a/hw/xwayland/xwayland-input.c
> +++ b/hw/xwayland/xwayland-input.c
> @@ -32,6 +32,14 @@
>  #include <xkbsrv.h>
>  #include <xserver-properties.h>
>  #include <inpututils.h>
> +#include <mipointer.h>
> +#include <mipointrst.h>
> +
> +/* Copied from mipointer.c */
> +#define MIPOINTER(dev) \
> +    (IsFloating(dev) ? \
> +        (miPointerPtr)dixLookupPrivate(&(dev)->devPrivates,
> miPointerPrivKey): \
> +        (miPointerPtr)dixLookupPrivate(&(GetMaster(dev,
> MASTER_POINTER))->devPrivates, miPointerPrivKey))
>  
>  static void
>  xwl_pointer_control(DeviceIntPtr device, PtrCtrl *ctrl)
> @@ -210,6 +218,8 @@ pointer_handle_enter(void *data, struct wl_pointer
> *pointer,
>  {
>      struct xwl_seat *xwl_seat = data;
>      DeviceIntPtr dev = xwl_seat->pointer;
> +    DeviceIntPtr master;
> +    miPointerPtr mipointer;
>      int i;
>      int sx = wl_fixed_to_int(sx_w);
>      int sy = wl_fixed_to_int(sy_w);
> @@ -230,8 +240,18 @@ pointer_handle_enter(void *data, struct wl_pointer
> *pointer,
>  
>      xwl_seat->focus_window = wl_surface_get_user_data(surface);
>  
> +    master = GetMaster(dev, POINTER_OR_FLOAT);
>      (*pScreen->SetCursorPosition) (dev, pScreen, sx, sy, TRUE);
> -    CheckMotion(NULL, GetMaster(dev, POINTER_OR_FLOAT));
> +
> +    /* X is very likely to have the wrong idea of what the actual cursor
> +     * sprite is, so in order to force updating the cursor lets set the
> +     * current sprite to some invalid cursor behind its back so that it
> +     * always will think it changed to the not invalid cursor.
> +     */
> +    mipointer = MIPOINTER(master);
> +    mipointer->pSpriteCursor = (CursorPtr) 1;

Out of curiosity, why not simply using NULL here?

> +
> +    CheckMotion(NULL, master);
>  
>      /* Ideally, X clients shouldn't see these button releases.  When
>       * the pointer leaves a window with buttons down, it means that
> --
> 2.4.3
> 

Cheers,
Olivier