[PATCH synaptics] Limit the movement to 20 mm per event

[Alexander E. Patrakov]

16.09.2014 07:08, Peter Hutterer wrote:
> Touchpads are limited by a fixed sampling rate (usually 80Hz). Some finger
> changes may happen too fast for this sampling rate, resulting in two distinct
> event sequences:
> * finger 1 up and finger 2 down in the same EV_SYN frame. Synaptics sees one
>    finger down before and after and the changed coordinates
> * finger 1 up and finger 2 down _between_ two EV_SYN frames. Synaptics sees one
>    touchpoint move from f1 position to f2 position.
>
> That move causes a large cursor jump. The former could be solved (with
> difficulty) by adding fake EV_SYN handling after releasing touchpoints but
> that won't fix the latter case.

Hello.

Thanks for recognizing the problem and opting for a simple criterion for 
detecting such jumps.

However, AFAICS, the patch just discards the motion without telling the 
FSM that it is in fact a new touch. So it may be bad for gestures.

Attached is a patch that I use to work around this issue since July that 
does attempt to tell the FSM about a different finger.

> So as a solution for now limit the finger movement to 20mm per event.
> Tests on a T440 and an x220 showed that this is just above what a reasonable
> finger movement would trigger. If a movement is greater than that limit, reset
> it to 0/0.
>
> On devices without resolution, use 0.25 the touchpad's diagonal instead.

I have no objection to the resolution-aware threshold. My patch doesn't 
have it, but you can obviously replace the heuristic there with your 
version.

>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> See the git repo below for a simple script to verify valid movement deltas
> on your device if you feel like it:
> https://github.com/whot/input-data-analysis/tree/master/touchpad-max-delta
> Max I got was 9mm on the x220 and 17mm on the T440s but the latter was
> really ripping the cursor around the screen beyond what I'd deem useful.
>
>   src/synaptics.c    | 33 +++++++++++++++++++++++++++++++++
>   src/synapticsstr.h |  2 ++
>   2 files changed, 35 insertions(+)
>
> diff --git a/src/synaptics.c b/src/synaptics.c
> index 2e3ad0c..756751d 100644
> --- a/src/synaptics.c
> +++ b/src/synaptics.c
> @@ -780,6 +780,23 @@ set_default_parameters(InputInfoPtr pInfo)
>           pars->resolution_vert = 1;
>       }
>
> +    /* Touchpad sampling rate is too low to detect all movements.
> +       A user may lift one finger and put another one down within the same
> +       EV_SYN or even between samplings so the driver doesn't notice at all.
> +
> +       We limit the movement to 20 mm within one event, that is more than
> +       recordings showed is needed (17mm on a T440).
> +      */
> +    if (pars->resolution_horiz > 1 &&
> +        pars->resolution_vert > 1)
> +        pars->maxDeltaMM = 20;
> +    else {
> +        /* on devices without resolution set the vector length to 0.25 of
> +           the touchpad diagonal */
> +        pars->maxDeltaMM = diag * 0.25;
> +    }
> +
> +
>       /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge parameters */
>       if (pars->top_edge > pars->bottom_edge) {
>           int tmp = pars->top_edge;
> @@ -2229,6 +2246,13 @@ get_delta(SynapticsPrivate *priv, const struct SynapticsHwState *hw,
>       *dy = integral;
>   }
>
> +/* Vector length, but not sqrt'ed, we only need it for comparison */
> +static inline double
> +vlenpow2(double x, double y)
> +{
> +    return x * x + y * y;
> +}
> +
>   /**
>    * Compute relative motion ('deltas') including edge motion.
>    */
> @@ -2238,6 +2262,7 @@ ComputeDeltas(SynapticsPrivate * priv, const struct SynapticsHwState *hw,
>   {
>       enum MovingState moving_state;
>       double dx, dy;
> +    double vlen;
>       int delay = 1000000000;
>
>       dx = dy = 0;
> @@ -2283,6 +2308,14 @@ ComputeDeltas(SynapticsPrivate * priv, const struct SynapticsHwState *hw,
>    out:
>       priv->prevFingers = hw->numFingers;
>
> +    vlen = vlenpow2(dx/priv->synpara.resolution_horiz,
> +                    dy/priv->synpara.resolution_vert);
> +
> +    if (vlen > priv->synpara.maxDeltaMM * priv->synpara.maxDeltaMM) {
> +        dx = 0;
> +        dy = 0;
> +    }
> +
>       *dxP = dx;
>       *dyP = dy;
>
> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> index 4bd32ac..75f52d5 100644
> --- a/src/synapticsstr.h
> +++ b/src/synapticsstr.h
> @@ -226,6 +226,8 @@ typedef struct _SynapticsParameters {
>       int area_left_edge, area_right_edge, area_top_edge, area_bottom_edge;       /* area coordinates absolute */
>       int softbutton_areas[4][4]; /* soft button area coordinates, 0 => right, 1 => middle , 2 => secondary right, 3 => secondary middle button */
>       int hyst_x, hyst_y;         /* x and y width of hysteresis box */
> +
> +    int maxDeltaMM;               /* maximum delta movement (vector length) in mm */
>   } SynapticsParameters;
>
>   struct _SynapticsPrivateRec {
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Detect-and-discard-huge-coordinate-jumps-on-touchpad.patch
Type: text/x-patch
Size: 3418 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20140916/903cb27f/attachment.bin>