[PATCH 10/18] Add multi-touch support

[Chris Bagwell]

On Wed, Oct 13, 2010 at 1:12 AM, Takashi Iwai <tiwai at suse.de> wrote:
> At Wed, 13 Oct 2010 14:25:16 +1000,
> Peter Hutterer wrote:
>>
>> On Fri, Oct 08, 2010 at 07:22:34PM +0200, Takashi Iwai wrote:
>>
>> > +    if (priv->model != MODEL_SYNAPTICS)
>> > +   return;
>> > +    SYSCALL(rc = ioctl(local->fd, EVIOCGBIT(EV_ABS, sizeof(absbits)), absbits));
>> > +    if (rc >= 0 && TEST_BIT(ABS_MT_POSITION_X, absbits)) {
>> > +   priv->can_multi_touch = TRUE;
>> > +   xf86Msg(X_INFO, "%s: supports multi-touch finger detection\n", local->name);
>> > +    }
>> > +}
>> > +

Peter mentioned the intent was to keep this generic from start but he
didn't specifically call out that return statement.  Nothing jumped
out at me as very synaptics specific in patches (except maybe clickpad
detection) so hopefully we can simply delete that return statement.


>
>> >  static void
>> >  event_query_clickpad(LocalDevicePtr local)
>> >  {
>> > @@ -175,7 +202,7 @@ event_query_clickpad(LocalDevicePtr local)
>> >
>> >      /* clickpad device reports only the single left button mask */
>> >      if (priv->has_left && !priv->has_right && !priv->has_middle &&
>> > -   !priv->has_double &&
>> > +   (!priv->has_double || priv->can_multi_touch) &&
>> >     priv->model == MODEL_SYNAPTICS) {
>> >     priv->is_clickpad = TRUE;
>> >     /* enable right/middle button caps; otherwise gnome-settings-daemon
>> > @@ -383,21 +410,27 @@ EventReadHwState(InputInfoPtr pInfo,
>> >     switch (ev.type) {
>> >     case EV_SYN:
>> >         switch (ev.code) {
>> > +       case SYN_MT_REPORT:
>> > +           hw->multi_touch_count++;
>> > +           break;
>> >         case SYN_REPORT:
>> >             if (comm->oneFinger)
>> > -               hw->numFingers = 1;
>> > +               hw->numFingers = hw->multi_touch_count ? hw->multi_touch_count : 1;
>> >             else if (comm->twoFingers)
>> >                 hw->numFingers = 2;
>> >             else if (comm->threeFingers)
>> >                 hw->numFingers = 3;
>> >             else
>> >                 hw->numFingers = 0;
>> > +           hw->multi_touch = hw->multi_touch_count;
>> > +           hw->multi_touch_count = 0;
>> >             /* if the coord is out of range, we filter it out */
>> >             if (priv->is_clickpad && hw->z > 0 && (hw->x < minx || hw->x > maxx || hw->y < miny || hw->y > maxy))
>> >                     return FALSE;
>> >             *hwRet = *hw;
>> >             return TRUE;
>> >         }
>> > +       break;
>> >     case EV_KEY:
>> >         v = (ev.value ? TRUE : FALSE);
>> >         switch (ev.code) {
>> > @@ -458,13 +491,25 @@ EventReadHwState(InputInfoPtr pInfo,
>> >     case EV_ABS:
>> >         switch (ev.code) {
>> >         case ABS_X:
>> > -           hw->x = ev.value;
>> > +       case ABS_MT_POSITION_X:
>> > +           if (hw->multi_touch_count)
>> > +               hw->multi_touch_x = ev.value;
>> > +           else
>> > +               hw->x = ev.value;
>>
>> if I read this correctly this patch doesn't add multi-touch support but only
>> two-finger support, otherwise you'd be overwriting the value after the
>> second finger.
>
> The odd thing is that there is no third finger tracking.  Synaptics devices
> track up to two finger points although it can detect number of fingers up to
> three.

In another email I asked if it should be OK if BTN_TOOL_DOUBLETAP=0
while sending 2 MT sets.  This is kinda the opposite.
BTN_TOOL_TRIPLETAP=1 but sending only 2 MT sets.  Sounds like we may
need to be relaxed and allow these type mismatches to occur.

I'll have to think a little bit to see if that also means
priv->numFingers should also be calculated based on multi_touch_count.

>
> So, in my code, I only put the primary and the rest fingers for simplicity
> (so that hw->{x,y,z} notions remain).

Hmm, maybe I'm misreading the above code when combined with kernel
patches.  Kernel side patches were always sending the "at rest"
fingers as first MT set and the moving fingers second.  The above
seems to give preference to the "at rest" values which seems odd to
me.  But surely I'm missing something since I believe you've stated
click-and-drag is working.

Also, I'm sure you've already thought about issue when we add
ABS_X/ABS_Y reports back into MT-mode.  Unless we are able to make
ABS_X/ABS_Y always the preferred values on kernel side then we'll need
some flag above to say we are in MT mode and to ignore ST values.

Chris