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

[Daniel Stone]

On Thu, Dec 23, 2010 at 10:34:35AM +1000, Peter Hutterer wrote:
> 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:
> > > 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."

Sure, but the same touch being owned by different clients is not OK.  If
client A grabs touches on that device, and client B grabs touches on all
MDs, then both clients will be the owner of that touch, which is quite
obviously broken.  I can't think of a way to post touch events twice
that doesn't result in instant breakage with the first usecase you can
think of.

Hence:
> > Do you have any suggestions here? I'm taking the 'touch must only be
> > delivered once' thing as axiomatic, so ...

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

Sure.

> > > 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 :)

:)

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

Well, event selections can change without the resource changing.  So a
TouchBegin|TouchMotion|TouchEnd selection can be changed by the client
into a MotionNotify|ButtonPress|ButtonRelease selection and we'd still
be delivering it touch events.  In this case, I'd rather respect the
client's wishes and stop delivering it touch events.

For grabs, you've just made me realise it's broken, as re-grabbing will
free the existing grab before adding the new one with a new ID.  So we
might need something more intelligent for that corner case, I guess.

tl;dr: You're right for grabs, but we still need it for selections.

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

God no, I'm not suggesting that _now_. :)

Cheers,
Daniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20101223/8db41ddb/attachment.pgp>