[PATCH xinput 1/1] Add experimental multitouch support from XI 2.1

[Daniel Stone]

On Wed, Dec 22, 2010 at 12:56:48PM +1000, Peter Hutterer wrote:
> On Fri, Dec 17, 2010 at 05:14:49PM +0000, Daniel Stone wrote:
> > diff --git a/src/list.c b/src/list.c
> > index 8633c62..8920cad 100644
> > --- a/src/list.c
> > +++ b/src/list.c
> > @@ -24,6 +24,9 @@
> >  #include "xinput.h"
> >  #include <string.h>
> >  #include <X11/extensions/XIproto.h> /* for XI_Device***ChangedNotify */
> > +#ifdef HAVE_XI2_1
> > +#include <X11/extensions/XI2proto.h> /* for XITouch* */
> > +#endif
> 
> this strikes me as odd. a client should never have to include the protocol
> header unless it's doing something special (and most likely wrong).
> I just checked the XIproto.h include and it still builds fine without it.
> the line was added in 2007 when XI2 was still a bit too much in flux.

Heh.  OK, I'll remove both in a separate patch.

> > +            case XITouchValuatorClass:
> > +                {
> > +                    XITouchValuatorClassInfo *tv =
> > +                        (XITouchValuatorClassInfo *)classes[i];
> > +                    char *name = tv->label ?
> > +                        XGetAtomName(display, tv->label) : NULL;
> > +
> > +                    printf("\t\tDetail for Touch Valuator %d:\n", tv->number);
> > +                    printf("\t\t  Label: %s\n",  (name) ? name : "None");
> > +                    printf("\t\t  Range: %f - %f\n", tv->min, tv->max);
> > +                    printf("\t\t  Resolution: %d units/m\n", tv->resolution);
> > +                }
> 
> not that it really matters, but free(name) would close the leak.

Yep.

> >  static void print_deviceevent(XIDeviceEvent* event)
> >  {
> >      double *val;
> >      int i;
> > +    static int touch_events_received = 0;
> > +    static int thong = 0;
> 
> thong or flip-flop? this may be a source of much confusion.

:)

This is only really for testing, so I don't think I'll push it in the
final version.  As it is, it provides a pretty good (and dead easy) grab
accept/reject testcase, as well as a nice hanging grab when the client
exits if you generate an odd number of touch sequences.

> >      printf("    device: %d (%d)\n", event->deviceid, event->sourceid);
> >      printf("    detail: %d\n", event->detail);
> > -    printf("    flags: %s\n", (event->flags & XIKeyRepeat) ? "repeat" : "");
> > +
> > +    switch (event->evtype)
> > +    {
> > +    case XI_KeyPress:
> 
> indentation inside the switch please.

Sure.

> > +    if (event->evtype == XI_TouchBegin)
> > +        touch_events_received = 0;
> > +    else if (event->evtype == XI_TouchMotion && event->event != event->child &&
> > +             (event->flags & XITouchOwner) && ++touch_events_received == 5)
> > +        XIAllowTouchEvents(event->display, event->sourceid, event->detail,
> > +                           (thong ^= 1) ? XITouchOwnerAccept :
> >  
> > +                                          XITouchOwnerReject);
> 
> some comment about what the effect of this is would be nice. afaict, it
> releases the touch after 5 events, once with accept, once with reject?

Yeah, it flip-flops between accepting and rejecting.

> Any specific reason for this? xinput isn't a demo app but more to test the
> event stream from a device for debugging. not sure how the above behaviour
> will help with this. 

Sure, I don't plan to push this one to the final repo.  That being said,
it does fit in with the grabs for pointer/keyboard stuff below ...

> > @@ -294,14 +322,18 @@ test_xi2(Display	*display,
> >           char	*desc)
> >  {
> >      XIEventMask mask;
> > -    Window win;
> > +    Window win, subwin;
> >  
> >      list(display, argc, argv, name, desc);
> > -    win = create_win(display);
> > +    create_win(display, &win, &subwin);
> >  
> >      /* Select for motion events */
> >      mask.deviceid = XIAllDevices;
> > +#ifdef HAVE_XI2_1
> > +    mask.mask_len = XIMaskLen(XI_TouchMotion);
> > +#else
> >      mask.mask_len = XIMaskLen(XI_RawMotion);
> > +#endif
> 
> XI_LASTEVENT to avoid the ifdef?

Cunning!

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/20101222/3ed8213b/attachment-0001.pgp>