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

Alexander E. Patrakov patrakov at gmail.com
Mon Sep 15 20:17:12 PDT 2014


Oops, sorry. I really must sleep more. I replied with a libinput patch 
to a synaptics patch.

As I no longer use synaptics and don't want to figure out whether the 
code there allows to say "this is a new finger", I just slap an ACK on 
the original patch.

16.09.2014 08:57, Alexander E. Patrakov wrote:
> 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 {
>>


More information about the xorg-devel mailing list