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

[Olivier Fourdan]

Hi Jonas,

----- Original Message -----
> 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.

Do you know where we stand wit hregard to this patch?

I believe there are quite a few downstream bugs that would benefit from this patch (if acceptable).

Cheers,
Olivier