[PATCH xserver 08/10] Input: Add initial multitouch support from Xi 2.1

[Peter Hutterer]

On Wed, Dec 22, 2010 at 11:56:30AM +0000, Daniel Stone wrote:
> On Wed, Dec 22, 2010 at 03:10:07PM +1000, Peter Hutterer wrote:
> > On Fri, Dec 17, 2010 at 05:13:33PM +0000, Daniel Stone wrote:
> > >  /**
> > > + * Processes and delivers a TouchBegin, TouchMotion or a TouchEnd event.
> > > + *
> > > + * Due to having rather different delivery semantics (see the Xi 2.1 protocol
> > > + * spec for more information), this implements its own grab and selection
> > > + * delivery logic.
> > > + */
> > > +static void
> > > +ProcessTouchEvent(DeviceEvent *ev, DeviceIntPtr sourcedev)
> > > +{
> > > +    TouchClassPtr t;
> > > +    TouchPointInfoPtr ti;
> > > +    DeviceIntPtr masterdev = NULL, deliverdev = NULL;
> > > +    Window child;
> > > +    WindowPtr win;
> > > +    SpritePtr sprite;
> > > +    xXIDeviceEvent *xi2;
> > > +    Mask filter;
> > > +    int err, touch, i, j, deliveries;
> > > +
> > > +    /* We handle deliveries to MDs through the SD, rather than copying
> > > +     * the event and processing it twice. */
> > 
> > no. if the current hierarchy event processing isn't good enough, fix it but
> > don't shortcut it like this.
> 
> Well, we can only have a single delivery tree for each touchpoint: if we
> deliver once through the SD and once through the MD, we run the risk of
> a single touch being delivered in parallel to two sets of clients (e.g.
> one tree which has grabs on the device ID, and another which has grabs
> on XIAllMasterDevices), or at best, delivering the same touch twice to
> the same clients.

the same is true for pointer events. we rely on clients to be smart enough
to know that if they select for events from device X and for events from
XIAllMasterDevices, they will get duplicate events (with different states
too).

"don't shoot yourself in the foot. oh, btw. here's a gun. good luck."

> The if (!dev->touch) test will reject MDs anyway, and we could put in a
> test to ignore TouchBegins if they already have a delivery tree, but I
> don't see how that's any different.  So I thought it'd be better to make
> it explicit up top.
> 
> Do you have any suggestions here? I'm taking the 'touch must only be
> delivered once' thing as axiomatic, so ...
 

> > > +    /* First search the stack going from root to child looking for grabs,
> > > +     * delivering to every grab we find ... */
> > > +    ti->num_grabs = 0;
> > > +    for (i = 0; i < sprite->spriteTraceGood; i++)
> > > +    {
> > > +        GrabPtr grab;
> > > +        enum EventType saved_evtype = ev->type;
> > > +
> > > +        win = sprite->spriteTrace[i];
> > > +
> > > +        /* Grabs can only be established on one type, so fool
> > > +         * CheckPassiveGrabsOnWindow into thinking that it's a TouchBegin. */
> > 
> > no. fix CheckPassiveGrabsOnWindow that it works with other touch events too.
> 
> So something like this in CheckPassiveGrabsOnWindow:
> if (ev->evtype == ET_TouchMotion || ev->evtype == ET_TouchEnd)
>     evtype = ET_TouchBegin;
> else
>     evtype = ev->evtype;
> ?

quick check of CheckPassiveGrabsOnWindow() looks like if you add the
special-casing for the touch types in GrabMatchesSecond(), that should be
enough.

> The other option is to look up the saved resource IDs for TouchMotion
> and TouchEnd, but that involves verifying that the grabs haven't
> changed, etc, etc.  I'm happy to do that if you think that's the best
> way though.



> > > +        ev->type = ET_TouchBegin;
> > > +        deliverdev = sourcedev;
> > > +        grab = CheckPassiveGrabsOnWindow(win, deliverdev, ev, FALSE, FALSE);
> > > +        if (!grab && masterdev)
> > > +        {
> > > +            deliverdev = masterdev;
> > > +            grab = CheckPassiveGrabsOnWindow(win, deliverdev, ev, FALSE, FALSE);
> > > +        }
> > > +        ev->type = saved_evtype;
> > > +        if (!grab)
> > > +            continue;
> > > +        xi2->deviceid = deliverdev->id;
> > 
> > I really don't like calling EventToXI2() above and then messing with the
> > fields manually down here. the main reason we have internal and protocol
> > events is that we can just pass the internal event to the conversion func
> > and not worry about the details.
> 
> I don't particularly like it either, but given that the flags need to
> change during delivery - should we be calling EventToXI2() again? If so,
> we need to make sure that it's completely deterministic.

I think EventToXI2() has a test? if not, we should have one anyway, and it
should be trivial to write one :)

> > > +        /* Stash the list of currently-active listeners away at TouchBegin time,
> > > +         * then subsequently check against that list to make sure we only
> > > +         * deliver to that same list later on, so we don't deliver TouchMotion
> > > +         * events to clients which have never seen the corresponding
> > > +         * TouchBegin. */
> > > +        if (ev->type != ET_TouchBegin)
> > > +        {
> > > +            for (j = 0; j < ti->num_listeners; j++)
> > > +            {
> > > +                if (ti->listeners[j] == grab->resource)
> > > +                    break;
> > > +            }
> > > +            if (j == ti->num_listeners)
> > > +                continue;
> > > +        }
> > 
> > I don't quite understand the need for this. Once the sprite trace is
> > established, anything in listeners should be ok to deliver to, right?
> > if so, we don't need this for loop and can just run it on TouchBegin.
> 
> Sure, but grabs can (and will) change at any time, so if one client
> ungrabs and then another client regrabs mid-stream,
> CheckPassiveGrabsOnWindow will hand us back a grab for a client which
> has never seen that TouchBegin.

I'm confused. if you run through the window hierarchy at TouchBegin and you
store all clients that will receive the TouchBegin and subsequent other
events, you are good until TouchEnd time, right?

for all subsequent events, you can just go through the list (and remove
clients from that list if the reject grabs). I don't quite understand why
CheckPassiveGrabsOnWindow() would be called at any other time. am I missing
something here?

> > > +int
> > > +ProcXIAllowTouchEvents(ClientPtr client)
> > > +{
> > > +    DeviceIntPtr dev;
> > > +    TouchPointInfoPtr ti;
> > > +    int ret, touch, i, nev;
> > > +    ValuatorMask *mask = valuator_mask_new(0);
> > > +    EventList *events = InitEventList(GetMaximumEventsNum());
> > 
> > size of 1 seems to be sufficient? unless you call GPE from GetTouchEvent(),
> > but in this case you need to bump the value returned by
> > GetMaximumEventsNum().
> 
> Yeah, it was done like that to allow for future pointer emulation.  Mind
> you, it would be even better if GetTouchEvent/GetPointerEvents and
> friends took an 'enqueue or process now' parameter, and we didn't have
> to do the whole event list/for loop/process events dance in every
> caller ...

just write a little wrapper for the enqueuing or processing part. I think
there's a few spots where GPE and friends just return the events to the
caller, without them being enqueued or processed. One thing at a time, GPE
is complicated enough as it is.

Cheers,
  Peter