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

[Hans de Goede]

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.

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.

Regards,

Hans


> 
> Cheers,
>    Peter
> 
>> ---
>>  src/evdev-mt-touchpad.c | 8 ++++++++
>>  src/evdev-mt-touchpad.h | 2 ++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
>> index b0520c7..315c531 100644
>> --- a/src/evdev-mt-touchpad.c
>> +++ b/src/evdev-mt-touchpad.c
>> @@ -406,6 +406,11 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
>>  
>>  	for (i = 0; i < tp->ntouches; i++) {
>>  		t = tp_get_touch(tp, i);
>> +
>> +		/* semi-mt finger postions may "jump" when nfingers changes */
>> +		if (tp->semi_mt && tp->nfingers_down != tp->old_nfingers_down)
>> +			tp_motion_history_reset(t);
>> +
>>  		if (i >= tp->real_touches && t->state != TOUCH_NONE) {
>>  			t->x = first->x;
>>  			t->y = first->y;
>> @@ -454,6 +459,7 @@ tp_post_process_state(struct tp_dispatch *tp, uint64_t time)
>>  		t->dirty = false;
>>  	}
>>  
>> +	tp->old_nfingers_down = tp->nfingers_down;
>>  	tp->buttons.old_state = tp->buttons.state;
>>  
>>  	tp->queued = TOUCHPAD_EVENT_NONE;
>> @@ -668,6 +674,8 @@ tp_init_slots(struct tp_dispatch *tp,
>>  		tp->has_mt = false;
>>  	}
>>  
>> +	tp->semi_mt = libevdev_has_property(device->evdev, INPUT_PROP_SEMI_MT);
>> +
>>  	ARRAY_FOR_EACH(max_touches, m) {
>>  		if (libevdev_has_event_code(device->evdev,
>>  					    EV_KEY,
>> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
>> index af6a3a3..83edf4f 100644
>> --- a/src/evdev-mt-touchpad.h
>> +++ b/src/evdev-mt-touchpad.h
>> @@ -152,8 +152,10 @@ struct tp_dispatch {
>>  	struct evdev_dispatch base;
>>  	struct evdev_device *device;
>>  	unsigned int nfingers_down;		/* number of fingers down */
>> +	unsigned int old_nfingers_down;		/* previous no fingers down */
>>  	unsigned int slot;			/* current slot */
>>  	bool has_mt;
>> +	bool semi_mt;
>>  
>>  	unsigned int real_touches;		/* number of slots */
>>  	unsigned int ntouches;			/* no slots inc. fakes */
>> -- 
>> 2.0.1
>>
>> _______________________________________________
>> wayland-devel mailing list
>> wayland-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: latitude-e6430-semi-mt-jump.log
Type: text/x-log
Size: 19763 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140722/ff0935ec/attachment-0001.bin>