[PATCH 2/2] input: Record grab pointer in TouchListener

Peter Hutterer peter.hutterer at who-t.net
Wed Nov 28 21:05:12 PST 2012


On Thu, Nov 29, 2012 at 02:05:20PM +1000, Peter Hutterer wrote:
> On Tue, Nov 27, 2012 at 11:21:17AM -0800, Keith Packard wrote:
> > This places a pointer to the grab related to a TouchListener directly
> > in the TouchListener structure rather than hoping to find the grab
> > later on using the resource ID.
> > 
> > Passive grabs have resource ID in the resource DB so they can be
> > removed when a client exits, and those resource IDs get copied when
> > activated, but implicit grabs are constructed on-the-fly and have no
> > resource DB entry.
> > 
> > Signed-off-by: Keith Packard <keithp at keithp.com>
> 
> works for me, nice one.
> 
> confirmed it fixes the bugs and does not introduce any new test-case
> failures.
> 
> both Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>, merged into my
> branch. thanks

actually, cancel that, this does introduce a regression. 

All TouchEventHistoryTest.EventHistoryReplay tests now fail.
http://cgit.freedesktop.org/~whot/xorg-integration-tests/tree/tests/server/touch.cpp#n688

Specifically, after rejecting the touch on a window, a TouchBegin should be
sent to the next grabbing client or event selection. With this patch, this
event is now missing.

didn't spot that at first, because I didn't have Thomas' patches (in my pull
request) in the tree when testing yours, so the test results just changed
from "wrong coordinates" to "no events being sent no more".

Cheers,
   Peter

> > ---
> >  Xi/exevents.c      |   27 ++++++---------------------
> >  dix/events.c       |    1 +
> >  dix/touch.c        |   21 ++++++++++++++-------
> >  include/input.h    |    2 +-
> >  include/inputstr.h |    1 +
> >  5 files changed, 23 insertions(+), 29 deletions(-)
> > 
> > diff --git a/Xi/exevents.c b/Xi/exevents.c
> > index 4248b9a..22cba81 100644
> > --- a/Xi/exevents.c
> > +++ b/Xi/exevents.c
> > @@ -1197,7 +1197,6 @@ TouchRejected(DeviceIntPtr sourcedev, TouchPointInfoPtr ti, XID resource,
> >                TouchOwnershipEvent *ev)
> >  {
> >      Bool was_owner = (resource == ti->listeners[0].listener);
> > -    void *grab;
> >      int i;
> >  
> >      /* Send a TouchEnd event to the resource being removed, but only if they
> > @@ -1212,11 +1211,7 @@ TouchRejected(DeviceIntPtr sourcedev, TouchPointInfoPtr ti, XID resource,
> >  
> >      /* Remove the resource from the listener list, updating
> >       * ti->num_listeners, as well as ti->num_grabs if it was a grab. */
> > -    if (TouchRemoveListener(ti, resource)) {
> > -        if (dixLookupResourceByType(&grab, resource, RT_PASSIVEGRAB,
> > -                                    serverClient, DixGetAttrAccess) == Success)
> > -            ti->num_grabs--;
> > -    }
> > +    TouchRemoveListener(ti, resource);
> >  
> >      /* If the current owner was removed and there are further listeners, deliver
> >       * the TouchOwnership or TouchBegin event to the new owner. */
> > @@ -1310,21 +1305,10 @@ RetrieveTouchDeliveryData(DeviceIntPtr dev, TouchPointInfoPtr ti,
> >  
> >      if (listener->type == LISTENER_GRAB ||
> >          listener->type == LISTENER_POINTER_GRAB) {
> > -        rc = dixLookupResourceByType((pointer *) grab, listener->listener,
> > -                                     RT_PASSIVEGRAB,
> > -                                     serverClient, DixSendAccess);
> > -        if (rc != Success) {
> > -            /* the grab doesn't exist but we have a grabbing listener - this
> > -             * is an implicit/active grab */
> > -            rc = dixLookupClient(client, listener->listener, serverClient,
> > -                                 DixSendAccess);
> > -            if (rc != Success)
> > -                return FALSE;
> > -
> > -            *grab = dev->deviceGrab.grab;
> > -            if (!*grab)
> > -                return FALSE;
> > -        }
> > +
> > +        *grab = listener->grab;
> > +
> > +        BUG_RETURN_VAL(!*grab, FALSE);
> >  
> >          *client = rClient(*grab);
> >          *win = (*grab)->window;
> > @@ -1477,6 +1461,7 @@ DeliverTouchEmulatedEvent(DeviceIntPtr dev, TouchPointInfoPtr ti,
> >               */
> >              l = &ti->listeners[ti->num_listeners - 1];
> >              l->listener = devgrab->resource;
> > +            l->grab = devgrab;
> >  
> >              if (devgrab->grabtype != XI2 || devgrab->type != XI_TouchBegin)
> >                  l->type = LISTENER_POINTER_GRAB;
> > diff --git a/dix/events.c b/dix/events.c
> > index 3282ef8..1eff2f0 100644
> > --- a/dix/events.c
> > +++ b/dix/events.c
> > @@ -1435,6 +1435,7 @@ UpdateTouchesForGrab(DeviceIntPtr mouse)
> >                  ti->listeners[0].type = LISTENER_POINTER_GRAB;
> >              else
> >                  ti->listeners[0].type = LISTENER_GRAB;
> > +            ti->listeners[0].grab = grab;
> >          }
> >      }
> >  }
> > diff --git a/dix/touch.c b/dix/touch.c
> > index 5f77be5..2a1e201 100644
> > --- a/dix/touch.c
> > +++ b/dix/touch.c
> > @@ -684,13 +684,17 @@ TouchResourceIsOwner(TouchPointInfoPtr ti, XID resource)
> >  void
> >  TouchAddListener(TouchPointInfoPtr ti, XID resource, enum InputLevel level,
> >                   enum TouchListenerType type, enum TouchListenerState state,
> > -                 WindowPtr window)
> > +                 WindowPtr window,
> > +                 GrabPtr grab)
> >  {
> >      ti->listeners[ti->num_listeners].listener = resource;
> >      ti->listeners[ti->num_listeners].level = level;
> >      ti->listeners[ti->num_listeners].state = state;
> >      ti->listeners[ti->num_listeners].type = type;
> >      ti->listeners[ti->num_listeners].window = window;
> > +    ti->listeners[ti->num_listeners].grab = grab;
> > +    if (grab)
> > +        ti->num_grabs++;
> >      ti->num_listeners++;
> >  }
> >  
> > @@ -714,6 +718,10 @@ TouchRemoveListener(TouchPointInfoPtr ti, XID resource)
> >              ti->num_listeners--;
> >              ti->listeners[ti->num_listeners].listener = 0;
> >              ti->listeners[ti->num_listeners].state = LISTENER_AWAITING_BEGIN;
> > +            if (ti->listeners[ti->num_listeners].grab) {
> > +                ti->listeners[ti->num_listeners].grab = NULL;
> > +                ti->num_grabs--;
> > +            }
> >              return TRUE;
> >          }
> >      }
> > @@ -740,8 +748,7 @@ TouchAddGrabListener(DeviceIntPtr dev, TouchPointInfoPtr ti,
> >      }
> >  
> >      TouchAddListener(ti, grab->resource, grab->grabtype,
> > -                     type, LISTENER_AWAITING_BEGIN, grab->window);
> > -    ti->num_grabs++;
> > +                     type, LISTENER_AWAITING_BEGIN, grab->window, grab);
> >  }
> >  
> >  /**
> > @@ -797,7 +804,7 @@ TouchAddRegularListener(DeviceIntPtr dev, TouchPointInfoPtr ti,
> >                  TouchEventHistoryAllocate(ti);
> >  
> >              TouchAddListener(ti, iclients->resource, XI2,
> > -                             type, LISTENER_AWAITING_BEGIN, win);
> > +                             type, LISTENER_AWAITING_BEGIN, win, NULL);
> >              return TRUE;
> >          }
> >      }
> > @@ -813,7 +820,7 @@ TouchAddRegularListener(DeviceIntPtr dev, TouchPointInfoPtr ti,
> >              TouchEventHistoryAllocate(ti);
> >              TouchAddListener(ti, iclients->resource, XI,
> >                               LISTENER_POINTER_REGULAR, LISTENER_AWAITING_BEGIN,
> > -                             win);
> > +                             win, NULL);
> >              return TRUE;
> >          }
> >      }
> > @@ -828,7 +835,7 @@ TouchAddRegularListener(DeviceIntPtr dev, TouchPointInfoPtr ti,
> >              TouchEventHistoryAllocate(ti);
> >              TouchAddListener(ti, win->drawable.id, CORE,
> >                               LISTENER_POINTER_REGULAR, LISTENER_AWAITING_BEGIN,
> > -                             win);
> > +                             win, NULL);
> >              return TRUE;
> >          }
> >  
> > @@ -839,7 +846,7 @@ TouchAddRegularListener(DeviceIntPtr dev, TouchPointInfoPtr ti,
> >  
> >              TouchEventHistoryAllocate(ti);
> >              TouchAddListener(ti, oclients->resource, CORE,
> > -                             type, LISTENER_AWAITING_BEGIN, win);
> > +                             type, LISTENER_AWAITING_BEGIN, win, NULL);
> >              return TRUE;
> >          }
> >      }
> > diff --git a/include/input.h b/include/input.h
> > index f8459b8..d83e426 100644
> > --- a/include/input.h
> > +++ b/include/input.h
> > @@ -562,7 +562,7 @@ extern void TouchEventHistoryReplay(TouchPointInfoPtr ti, DeviceIntPtr dev,
> >  extern Bool TouchResourceIsOwner(TouchPointInfoPtr ti, XID resource);
> >  extern void TouchAddListener(TouchPointInfoPtr ti, XID resource,
> >                               enum InputLevel level, enum TouchListenerType type,
> > -                             enum TouchListenerState state, WindowPtr window);
> > +                             enum TouchListenerState state, WindowPtr window, GrabPtr grab);
> >  extern Bool TouchRemoveListener(TouchPointInfoPtr ti, XID resource);
> >  extern void TouchSetupListeners(DeviceIntPtr dev, TouchPointInfoPtr ti,
> >                                  InternalEvent *ev);
> > diff --git a/include/inputstr.h b/include/inputstr.h
> > index 8d9dd71..e21484b 100644
> > --- a/include/inputstr.h
> > +++ b/include/inputstr.h
> > @@ -305,6 +305,7 @@ typedef struct _TouchListener {
> >      enum TouchListenerState state;
> >      enum InputLevel level;  /* matters only for emulating touches */
> >      WindowPtr window;
> > +    GrabPtr grab;
> >  } TouchListener;
> >  
> >  typedef struct _TouchPointInfo {
> > -- 
> > 1.7.10.4
> > 


More information about the xorg-devel mailing list