[PATCH libinput] touchpad: always reset the motion history on finger changes

[Hans de Goede]

Hi,

On 29-08-16 06:25, Peter Hutterer wrote:
> We've already been doing this for semi-mt devices and for non-clickpads but
> let's do it for clickpads as well. On Synaptics touchpads (PS/2 and RMI4)
> we see slot jumps where two slots are active, slot X ends but slot Y continues
> with the other slot's positional data. This causes a cursor jump on finger
> lift after a two-finger scrolling motion. Simply resetting the motion history fixes it.
>
> The only multi-finger interaction where a user could expect perfect fluid
> motion is when using a second finger to touch cone of the software button
> areas. Let's see if we have complaints first before we implement something
> more complex.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=91695
>
> Signed-off-by:Peter Hutterer <peter.hutterer at who-t.net>

A nice and KISS solution, I like:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans

> ---
> I had a nice and more complicated patch to address the issue where touches
> change slots but really, this on is enough.
>
>  src/evdev-mt-touchpad.c | 10 +++++-----
>  test/touchpad.c         | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 3baed09..65b0abf 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -957,11 +957,11 @@ tp_need_motion_history_reset(struct tp_dispatch *tp)
>  {
>  	bool rc = false;
>
> -	/* Semi-mt finger postions may "jump" when nfingers changes. And on
> -	 * a non-clickpad the only reason to have more than one finger down
> -	 * is scrolling/gesture, so a reset just makes things sane again */
> -	if ((tp->semi_mt || !tp->buttons.is_clickpad) &&
> -	    tp->nfingers_down != tp->old_nfingers_down)
> +	/* Changing the numbers of fingers can cause a jump in the
> +	 * coordinates, always reset the motion history for all touches when
> +	 * that happens.
> +	 */
> +	if (tp->nfingers_down != tp->old_nfingers_down)
>  		return true;
>
>  	/* if we're transitioning between slots and fake touches in either
> diff --git a/test/touchpad.c b/test/touchpad.c
> index ea20036..6e0d310 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -4315,6 +4315,56 @@ START_TEST(touchpad_tool_tripletap_touch_count)
>  }
>  END_TEST
>
> +START_TEST(touchpad_slot_swap)
> +{
> +	struct litest_device *dev = litest_current_device();
> +	struct libinput *li = dev->libinput;
> +	struct libinput_event *event;
> +	struct libinput_event_pointer *ptrev;
> +	int first, second;
> +
> +	/* Synaptics touchpads sometimes end the wrong touchpoint on finger
> +	 * up, causing the remaining slot to continue with the other slot's
> +	 * coordinates.
> +	 * https://bugs.freedesktop.org/show_bug.cgi?id=91352
> +	 */
> +	litest_drain_events(li);
> +
> +	for (first = 0; first <= 1; first++) {
> +		second = 1 - first;
> +
> +		litest_touch_down(dev, 0, 50, 50);
> +		libinput_dispatch(li);
> +		litest_touch_down(dev, 1, 70, 70);
> +		libinput_dispatch(li);
> +
> +		litest_touch_move_to(dev, first, 50, 50, 50, 30, 10, 1);
> +		litest_drain_events(li);
> +
> +		/* release touch 0, continue other slot with 0's coords */
> +		litest_push_event_frame(dev);
> +		litest_touch_up(dev, first);
> +		litest_touch_move(dev, second, 50, 30.1);
> +		litest_pop_event_frame(dev);
> +		libinput_dispatch(li);
> +		litest_touch_move_to(dev, second, 50, 30, 50, 11, 10, 1);
> +		libinput_dispatch(li);
> +		event = libinput_get_event(li);
> +		do {
> +			ptrev = litest_is_motion_event(event);
> +			ck_assert_double_eq(libinput_event_pointer_get_dx(ptrev), 0.0);
> +			ck_assert_double_lt(libinput_event_pointer_get_dy(ptrev), 1.0);
> +
> +			libinput_event_destroy(event);
> +			event = libinput_get_event(li);
> +		} while (event);
> +		litest_assert_empty_queue(li);
> +
> +		litest_touch_up(dev, second);
> +	}
> +}
> +END_TEST
> +
>  START_TEST(touchpad_time_usec)
>  {
>  	struct litest_device *dev = litest_current_device();
> @@ -4513,6 +4563,7 @@ litest_setup_tests_touchpad(void)
>  	litest_add("touchpad:thumb", touchpad_thumb_tap_hold_2ndfg_tap, LITEST_CLICKPAD, LITEST_SINGLE_TOUCH);
>
>  	litest_add_for_device("touchpad:bugs", touchpad_tool_tripletap_touch_count, LITEST_SYNAPTICS_TOPBUTTONPAD);
> +	litest_add_for_device("touchpad:bugs", touchpad_slot_swap, LITEST_SYNAPTICS_TOPBUTTONPAD);
>
>  	litest_add("touchpad:time", touchpad_time_usec, LITEST_TOUCHPAD, LITEST_ANY);
>
>