[PATCH] touchpad: reset motion history when nfingers changes on semi-mt pads

[Peter Hutterer]

On Tue, Jul 22, 2014 at 09:18:26AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 07/22/2014 01:34 AM, Peter Hutterer wrote:
> > On Mon, Jul 21, 2014 at 03:25:47PM +0200, Hans de Goede wrote:
> >> On semi-mt touchpads the reported position of the first finger down may
> >> jump when the pad switches from st to mt mode. When this happens a large
> >> delta gets seen on the first finger at the same time the second fingers
> >> is first seen down, causing a spurious 2 finger scroll event.
> >>
> >> Reset the motion history when nfingers changes on semi-mt pads to avoid this.
> >>
> >> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> > 
> > I don't seem to have any good recordings here that show the SEMI_MT jumps.
> > Any chance you can get one and add a test device for this?
> 
> Attached is a recording.
> 
> The jump starts at line 315, there slot 0 x / y are aprox. 640 / 1300, then
> at line 326 they jump to 1400 / 1272. This jump happens because in 2 finger
> mode the touchpad no longer reports exact locations, instead we get
> a bounding box, where the fingers occupy 2 opposing corners, and in this case
> the fingers are on 2 different corners then the ones the kernel chooses to
> always report.
> 
> I guess the kernel could / should be made smarter here, and could decide which
> 2 corners to pick based on the last single touch coordinate, instead of always
> picking the upper left and bottom right corners. I'll take a look at that,
> resetting the motion history is still the right thing to do though, since
> the 2 finger coordinates basically use a different coordinate system which
> gets scaled to the single touch coordinate system.

if the kernel is no option (as you said in the other email), can we add a
semi-mt specific touch filtering system so that we fix the touchpoints
_before_ we get to the main touchpad processing code? That way we don't have
to care at all about semi-mt once the touchpoints are fixed. It won't be
perfect, but it's better than having to worry about semi-mt everywhere.
 
> Adding a test device for this based on the recording should be easy.
> 
> > ACK to the patch, but this is really something I'd like to see a specific
> > test case for so we know what we're fixing here. plus, adding a semi-mt
> > device to the test suite means we can figure out if everything else works
> > fine with it.
> 
> I already expected you would want a test-case, the problem is that currently
> in litest we cannot send a motion event with it automatically getting a sync
> added at the end of it, making it impossible to reproduce the behavior from
> the recording in litest. I've been thinking a bit about this and I think
> that the best way to deal with this is a suppress_syn flag which will basically
> suppress all syn sending when set, then special cases like this one can
> just set that flag, queue up events, clear the flag and do an explicit syn.

How about an API that takes multiple touch down events and
strips the SYN_REPORTS out automatically. So e.g.

    litest_multi_touch_down(0,  10, 10,
                            1,  50, 50,
                            -1);

and the matching move/up calls. That won't cater for this particular case
though, I think for this use-case you really need to just litest_write the
specific event sequence. Or, even better, overwrite touch_down in the
litest-device interface to mangle the touchpoints per slot to replicate the
kernel behaviour, which would make the device applicable for all tests then.

Cheers,
   Peter