[PATCH synaptics] Don't count fingers twice when guessing distance (#48316)

[Chase Douglas]

On 04/10/2012 06:14 PM, Peter Hutterer wrote:
> On Tue, Apr 10, 2012 at 05:24:26PM -0700, Chase Douglas wrote:
>> On 04/10/2012 05:03 PM, Peter Hutterer wrote:
>>> A finger may be closer than the required distance to more than one finger.
>>> e.g. for fingers A, B, C, AC and BC could both trigger the check and count
>>> C twice.
>>
>> The above description actually results in the correct count for three
>> touches. A better description would mention the case where AC, BC, and
>> AB are all close enough to trigger the check, in which case we have too
>> many fingers.
> 
> right, sorry, amended to:
>    A finger may be closer than the required distance to more than one
>    finger.e.g. for fingers A, B, C, AB, AC and BC may trigger the check and
>    count C twice -resulting in a 4 finger click.
> 
>>> Avoid double-counting by marking those fingers already close enough to a
>>> previous finger to avoid overcounting.
>>>
>>> X.Org Bug 48316 <http://bugs.freedesktop.org/show_bug.cgi?id=48316>
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>>  src/synaptics.c |    9 ++++++++-
>>>  1 files changed, 8 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/synaptics.c b/src/synaptics.c
>>> index 918dc6f..0e10aeb 100644
>>> --- a/src/synaptics.c
>>> +++ b/src/synaptics.c
>>> @@ -2624,7 +2624,9 @@ clickpad_guess_clickfingers(SynapticsPrivate *priv, struct SynapticsHwState *hw)
>>>  {
>>>      int nfingers = 0;
>>>  #if HAVE_MULTITOUCH
>>> +    int skip[SYNAPTICS_MAX_TOUCHES] = {0};
>>>      int i, j;
>>> +
>>>      for (i = 0; i < hw->num_mt_mask - 1; i++) {
>>>          ValuatorMask *f1;
>>>  
>>> @@ -2642,6 +2644,9 @@ clickpad_guess_clickfingers(SynapticsPrivate *priv, struct SynapticsHwState *hw)
>>>                  hw->slot_state[j] == SLOTSTATE_CLOSE)
>>>                  continue;
>>>  
>>> +            if (skip[j])
>>> +                continue;
>>> +
>>>              f2 = hw->mt_mask[j];
>>>  
>>>              x1 = valuator_mask_get_double(f1, 0);
>>> @@ -2655,8 +2660,10 @@ clickpad_guess_clickfingers(SynapticsPrivate *priv, struct SynapticsHwState *hw)
>>>               * you'll need to find a touchpad that doesn't lie about it's
>>>               * size. Good luck. */
>>>              if (abs(x1 - x2) < (priv->maxx - priv->minx) * .3 &&
>>> -                abs(y1 - y2) < (priv->maxy - priv->miny) * .3)
>>> +                abs(y1 - y2) < (priv->maxy - priv->miny) * .3) {
>>>                  nfingers++;
>>> +                skip[j] = 1;
>>> +            }
>>>          }
>>>      }
>>>  #endif
>>
>> I think we would get the same results if we merely added a "break;"
>> after "nfingers++;". This would ensure we only increment the finger
>> count once for each outer loop iteration. In the example, we would see
>> AB, and BC, but not AC because we broke out of the inner loop after AB.
> 
> you can't guarantee that BC are close enough together when you see the first
> match though. so for the case of physical B left of A left of C, you have AB
> and AC being close but BC may not be. If you break after finding the AB
> match, you'd skip the AC and get two-finger click only.

Good point.

Wouldn't this code break in the same way if you have touches ordered as
A-C-B? We would match A-C, which would mark C for skipping. When we look
at B-C, we would skip it. We wouldn't test C-B because of how the loops
are structured.

I think we need a better algorithm. This one works almost perfect, but I
don't see a way to fix it to cover 100% of cases. Unfortunately, I'm
about to fall asleep in my chair, so I don't have any proposals right now.

-- Chase