evdev: workaround for missing ABS_X/Y on multitouch devices

[Peter Hutterer]

On Sat, Dec 20, 2014 at 10:57:12PM +0000, Colin Macdonald wrote:
> On 27/06/14 02:47, Peter Hutterer wrote:
> > right idea, but I think this is too fancy for what we need. Something like
> > this before the loop should be enough:
> > 
> > /* curse you, android! */
> > if (libevdev_has_event_code(EV_ABS, ABS_MT_POSITION_X) &&
> >     !libevdev_has_event_code(EV_ABS, ABS_X))
> > {
> >     const struct input_absinfo* abs;
> >     abs = libevdev_get_abs_info(pEvdev->dev, axis);
> >     libevdev_enable_event_code(pEvdev->dev, EV_ABS, ABS_X, abs);
> >     num_axes++;
> > }
> > 
> > repeat for ABS_Y
> 
> Hi Peter,
> 
> Sorry so long to get back to this, as I'm sure you've long forgotten.
> But please find patch attached.  I followed your advice and did this
> before the mapping loop.
> 
> However, I did the full mt_axis_mappings table, not just
> ABS_MT_POSITION_X and _Y.  I think this is the right thing to do here in
> case some driver has ABS_MT_PRESSURE or ABS_MT_DISTANCE.

It should be, but I don't think I've see a device that is lacking
pressure/distance but has the MT codes for it anyway.

> I've lightly tested on an Android device where it does make the
> touchscreen work.
> 
> It also seems ok (i.e., doesn't break) my touchscreen laptop (Fedora
> GNU/Linux).
> 
> Colin

> From baeb2b0a71387a9ed83737b5cf58c195882588df Mon Sep 17 00:00:00 2001
> From: "Colin B. Macdonald" <macdonald at maths.ox.ac.uk>
> Date: Thu, 26 Jun 2014 12:17:59 +0100
> Subject: [PATCH] Workaround lack of ABS_X, bug #80470
> 
> Often on Android, we have ABS_MT_POSITION_X without
> ABS_X (which is contrary to spec).  We add fake
> ABS_X axis in that case.  I also commented some of
> the surrouding code as I did this.
> ---
>  src/evdev.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 9cfc982..f6834be 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1211,7 +1211,8 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device, int num_scroll_axes)
>  {
>      InputInfoPtr pInfo;
>      EvdevPtr pEvdev;
> -    int num_axes = 0, axis, i = 0;
> +    int axis, i = 0;
> +    int num_axes = 0; /* number of non-MT axes */
>      int num_mt_axes = 0, /* number of MT-only axes */
>          num_mt_axes_total = 0; /* total number of MT axes, including
>                                    double-counted ones, excluding blacklisted */
> @@ -1224,6 +1225,7 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device, int num_scroll_axes)
>      if (!libevdev_has_event_type(pEvdev->dev, EV_ABS))
>          goto out;
>  
> +    /* Find number of absolute axis, including MT ones, will decrease later. */
>      for (i = 0; i < ABS_MAX; i++)
>          if (libevdev_has_event_code(pEvdev->dev, EV_ABS, i))
>              num_axes++;
> @@ -1232,6 +1234,31 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device, int num_scroll_axes)
>          goto out;
>  
>  #ifdef MULTITOUCH
> +    /* Android drivers often have ABS_MT_POSITION_X but not ABS_X.
> +       Loop over the MT->legacy axis table and add fake axes. */
> +    for (i = 0; i < ArrayLength(mt_axis_mappings); i++)
> +    {
> +        int mt_code = mt_axis_mappings[i].mt_code;
> +        int code = mt_axis_mappings[i].code;
> +        if (libevdev_has_event_code(pEvdev->dev, EV_ABS, mt_code) &&
> +           !libevdev_has_event_code(pEvdev->dev, EV_ABS, code))

needs an extra space here for indentation

> +        {
> +            const struct input_absinfo* abs;
> +            abs = libevdev_get_abs_info(pEvdev->dev, mt_code);
> +            if (libevdev_enable_event_code(pEvdev->dev, EV_ABS, code, abs))
> +            {
> +                xf86IDrvMsg(pInfo, X_ERROR, "Failed to fake %s as a copy of %s\n",
> +                            abs_labels[code], abs_labels[mt_code]);

please use libevdev_event_code_get_name() here instead of the axis labels,
it's a bit easier to parse.

> +
> +                goto out;
> +            }
> +            xf86IDrvMsg(pInfo, X_WARNING, "Faking %s as a copy of axis %s\n",
> +                        abs_labels[code], abs_labels[mt_code]);

same here (and use X_INFO here). for both messages I think it's enough
to just say "faking %s", no need to print both axes, the code is
straightforward to deduce it.

> +            num_axes++;
> +        }
> +    }
> +
> +    /* Absolute multitouch axes: adjust mapping and axes counts. */
>      for (axis = ABS_MT_SLOT; axis < ABS_MAX; axis++)
>      {
>          if (libevdev_has_event_code(pEvdev->dev, EV_ABS, axis))
> @@ -1239,6 +1266,7 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device, int num_scroll_axes)
>              int j;
>              Bool skip = FALSE;
>  
> +            /* Setup mapping if axis is in MT->legacy axis table. */
>              for (j = 0; j < ArrayLength(mt_axis_mappings); j++)
>              {
>                  if (mt_axis_mappings[j].mt_code == axis &&
> @@ -1258,9 +1286,10 @@ EvdevAddAbsValuatorClass(DeviceIntPtr device, int num_scroll_axes)
>              num_axes--;
>          }
>      }
> +    xf86IDrvMsg(pInfo, X_INFO, "Have %d multitouch, %d non-multitouch axes\n",
> +		num_mt_axes, num_axes);

skip this one please

these are all just nitpicks, the approach looks correct. Send me an updated
version please and I'll run it against the test suites here.

Cheers,
   Peter

>  
> -    /* device only has mt-axes. the kernel should give us ABS_X etc for
> -       backwards compat but some devices don't have it. */
> +    /* Panic if, after faking ABS_X etc, we still only have mt-axes. */
>      if (num_axes == 0 && num_mt_axes > 0) {
>          xf86IDrvMsg(pInfo, X_ERROR,
>                      "found only multitouch-axes. That shouldn't happen.\n");
> -- 
> 2.1.0
> 




> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel