touchpad: don't allow for multifinger tapping after a move

In the current implementation, movements > threshold and timeouts usually move
to HOLD state and continue from there. Where a finger is lifted, we go back
up the diagram into the previous finger count's HOLD state.

The side-effect of this is that a tap of a finger can be counted as tap even
after a movement:

- two fingers down, move to scroll, hold down
- third finger down, third finger up

This sequence triggers an erroneous three-finger tap. Once the motion
threshold is hit by any touch, no finger must trigger 2/3 finger tap events
while any touch is down.

The false tap is only triggered where the new finger can execute a tap without
any other finger changing any property. This can be triggered on the
reporter's Dell Precision 5520 but on most other touchpads, a new finger down
will trigger slight movement, pressure or touch size updates and thus the bug
cannot be triggered.

Fixes #382

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

1 files changed, 112 insertions, 0 deletions

diff --git a/test/test-touchpad-tap.c b/test/test-touchpad-tap.c
index 08629083..cfb3dd45 100644
--- a/test/test-touchpad-tap.c
+++ b/test/test-touchpad-tap.c
@@ -2029,6 +2029,42 @@ START_TEST(touchpad_3fg_tap_slot_release_btntool)
 }
 END_TEST
 
+START_TEST(touchpad_3fg_tap_after_scroll)
+{
+	struct litest_device *dev = litest_current_device();
+	struct libinput *li = dev->libinput;
+
+	if (libevdev_get_abs_maximum(dev->evdev,
+				     ABS_MT_SLOT) <= 2)
+		return;
+
+	litest_enable_2fg_scroll(dev);
+	litest_enable_tap(dev->libinput_device);
+
+	litest_touch_down(dev, 0, 40, 20);
+	litest_touch_down(dev, 1, 50, 20);
+	litest_drain_events(li);
+
+	/* 2fg scroll */
+	litest_touch_move_two_touches(dev, 40, 20, 50, 20, 0, 20, 10);
+	litest_drain_events(li);
+
+	litest_timeout_tap();
+	libinput_dispatch(li);
+
+	/* third finger tap without the other two fingers moving */
+	litest_touch_down(dev, 2, 60, 40);
+	libinput_dispatch(li);
+	litest_touch_up(dev, 2);
+	libinput_dispatch(li);
+
+	litest_timeout_tap();
+	libinput_dispatch(li);
+
+	litest_assert_empty_queue(li);
+}
+END_TEST
+
 START_TEST(touchpad_4fg_tap)
 {
 	struct litest_device *dev = litest_current_device();
@@ -2099,6 +2135,78 @@ START_TEST(touchpad_4fg_tap_quickrelease)
 }
 END_TEST
 
+START_TEST(touchpad_move_after_touch)
+{
+	struct litest_device *dev = litest_current_device();
+	struct libinput *li = dev->libinput;
+	int nfingers = _i; /* ranged test */
+
+	if (libevdev_get_abs_maximum(dev->evdev, ABS_MT_SLOT) <= nfingers)
+		return;
+
+	litest_enable_tap(dev->libinput_device);
+	litest_drain_events(li);
+
+	/* respective number of fingers down */
+	switch(nfingers) {
+	case 5:
+		litest_touch_down(dev, 4, 70, 30);
+		/* fallthrough */
+	case 4:
+		litest_touch_down(dev, 3, 70, 30);
+		/* fallthrough */
+	case 3:
+		litest_touch_down(dev, 2, 60, 30);
+		/* fallthrough */
+	case 2:
+		litest_touch_down(dev, 1, 50, 30);
+		/* fallthrough */
+	case 1:
+		litest_touch_down(dev, 0, 40, 30);
+		/* fallthrough */
+		break;
+	default:
+		abort();
+	}
+
+	/* move finger 1 */
+	libinput_dispatch(li);
+	litest_touch_move_to(dev, 0, 70, 30, 70, 60, 10);
+	libinput_dispatch(li);
+
+	/* lift finger 1, put it back */
+	litest_touch_up(dev, 0);
+	libinput_dispatch(li);
+	litest_touch_down(dev, 0, 40, 30);
+	libinput_dispatch(li);
+
+	/* lift fingers up */
+	switch(nfingers) {
+	case 5:
+		litest_touch_up(dev, 4);
+		/* fallthrough */
+	case 4:
+		litest_touch_up(dev, 3);
+		/* fallthrough */
+	case 3:
+		litest_touch_up(dev, 2);
+		/* fallthrough */
+	case 2:
+		litest_touch_up(dev, 1);
+		/* fallthrough */
+	case 1:
+		litest_touch_up(dev, 0);
+		/* fallthrough */
+		break;
+	}
+	libinput_dispatch(li);
+	litest_timeout_tap();
+	libinput_dispatch(li);
+
+	litest_assert_no_typed_events(li, LIBINPUT_EVENT_POINTER_BUTTON);
+}
+END_TEST
+
 START_TEST(touchpad_5fg_tap)
 {
 	struct litest_device *dev = litest_current_device();
@@ -3560,6 +3668,7 @@ TEST_COLLECTION(touchpad_tap)
 	struct range range_2fg = {0, 2};
 	struct range range_3fg = {0, 3};
 	struct range range_4fg = {0, 4};
+	struct range range_multifinger = {2, 5};
 
 	litest_add("tap:1fg", touchpad_1fg_tap, LITEST_TOUCHPAD, LITEST_ANY);
 	litest_add("tap:1fg", touchpad_1fg_doubletap, LITEST_TOUCHPAD, LITEST_ANY);
@@ -3600,12 +3709,15 @@ TEST_COLLECTION(touchpad_tap)
 	litest_add("tap:3fg", touchpad_3fg_tap_pressure_btntool, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
 	litest_add_for_device("tap:3fg", touchpad_3fg_tap_btntool_pointerjump, LITEST_SYNAPTICS_TOPBUTTONPAD);
 	litest_add_for_device("tap:3fg", touchpad_3fg_tap_slot_release_btntool, LITEST_SYNAPTICS_TOPBUTTONPAD);
+	litest_add("tap:3fg", touchpad_3fg_tap_after_scroll, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
 
 	litest_add("tap:4fg", touchpad_4fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
 	litest_add("tap:4fg", touchpad_4fg_tap_quickrelease, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
 	litest_add("tap:5fg", touchpad_5fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
 	litest_add("tap:5fg", touchpad_5fg_tap_quickrelease, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
 
+	litest_add_ranged("tap:multifinger", touchpad_move_after_touch, LITEST_TOUCHPAD, LITEST_ANY, &range_multifinger);
+
 	/* Real buttons don't interfere with tapping, so don't run those for
 	   pads with buttons */
 	litest_add("tap:1fg", touchpad_1fg_double_tap_click, LITEST_CLICKPAD, LITEST_ANY);