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

[Jonas Ådahl]

On Mon, Oct 12, 2015 at 05:35:20AM -0400, Olivier Fourdan wrote:
> 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?

Unsetting the cursor in mipointer (setting it to NULL) would not trigger
the unsetting in xwayland if we just use NULL here.


Jonas

> 
> > +
> > +    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