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>

diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 2610b83..889e2d3 100644 (file)
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -157,6 +157,14 @@ tp_tap_clear_timer(struct tp_dispatch *tp)
 }
 
 static void
+tp_tap_move_to_dead(struct tp_dispatch *tp, struct tp_touch *t)
+{
+       tp->tap.state = TAP_STATE_DEAD;
+       t->tap.state = TAP_TOUCH_STATE_DEAD;
+       tp_tap_clear_timer(tp);
+}
+
+static void
 tp_tap_idle_handle_event(struct tp_dispatch *tp,
                         struct tp_touch *t,
                         enum tap_event event, uint64_t time)
@@ -217,8 +225,10 @@ tp_tap_touch_handle_event(struct tp_dispatch *tp,
                        tp->tap.state = TAP_STATE_IDLE;
                }
                break;
-       case TAP_EVENT_TIMEOUT:
        case TAP_EVENT_MOTION:
+               tp_tap_move_to_dead(tp, t);
+               break;
+       case TAP_EVENT_TIMEOUT:
                tp->tap.state = TAP_STATE_HOLD;
                tp_tap_clear_timer(tp);
                break;
@@ -257,6 +267,8 @@ tp_tap_hold_handle_event(struct tp_dispatch *tp,
                tp->tap.state = TAP_STATE_IDLE;
                break;
        case TAP_EVENT_MOTION:
+               tp_tap_move_to_dead(tp, t);
+               break;
        case TAP_EVENT_TIMEOUT:
                break;
        case TAP_EVENT_BUTTON:
@@ -332,8 +344,8 @@ tp_tap_touch2_handle_event(struct tp_dispatch *tp,
                tp_tap_set_timer(tp, time);
                break;
        case TAP_EVENT_MOTION:
-               tp_tap_clear_timer(tp);
-               /* fallthrough */
+               tp_tap_move_to_dead(tp, t);
+               break;
        case TAP_EVENT_TIMEOUT:
                tp->tap.state = TAP_STATE_TOUCH_2_HOLD;
                break;
@@ -367,6 +379,8 @@ tp_tap_touch2_hold_handle_event(struct tp_dispatch *tp,
                tp->tap.state = TAP_STATE_HOLD;
                break;
        case TAP_EVENT_MOTION:
+               tp_tap_move_to_dead(tp, t);
+               break;
        case TAP_EVENT_TIMEOUT:
                tp->tap.state = TAP_STATE_TOUCH_2_HOLD;
                break;
@@ -407,6 +421,8 @@ tp_tap_touch2_release_handle_event(struct tp_dispatch *tp,
                tp->tap.state = TAP_STATE_IDLE;
                break;
        case TAP_EVENT_MOTION:
+               tp_tap_move_to_dead(tp, t);
+               break;
        case TAP_EVENT_TIMEOUT:
                tp->tap.state = TAP_STATE_HOLD;
                break;
@@ -455,6 +471,8 @@ tp_tap_touch3_handle_event(struct tp_dispatch *tp,
                tp_tap_clear_timer(tp);
                break;
        case TAP_EVENT_MOTION:
+               tp_tap_move_to_dead(tp, t);
+               break;
        case TAP_EVENT_TIMEOUT:
                tp->tap.state = TAP_STATE_TOUCH_3_HOLD;
                tp_tap_clear_timer(tp);
@@ -497,6 +515,8 @@ tp_tap_touch3_hold_handle_event(struct tp_dispatch *tp,
                tp->tap.state = TAP_STATE_TOUCH_2_HOLD;
                break;
        case TAP_EVENT_MOTION:
+               tp_tap_move_to_dead(tp, t);
+               break;
        case TAP_EVENT_TIMEOUT:
                break;
        case TAP_EVENT_BUTTON:

diff --git a/test/litest.c b/test/litest.c
index 0359855..f6a8f07 100644 (file)
--- a/test/litest.c
+++ b/test/litest.c
@@ -3628,6 +3628,26 @@ litest_assert_only_typed_events(struct libinput *li,
 }
 
 void
+litest_assert_no_typed_events(struct libinput *li,
+                             enum libinput_event_type type)
+{
+       struct libinput_event *event;
+
+       litest_assert(type != LIBINPUT_EVENT_NONE);
+
+       libinput_dispatch(li);
+       event = libinput_get_event(li);
+
+       while (event) {
+               litest_assert_int_ne(libinput_event_get_type(event),
+                                     type);
+               libinput_event_destroy(event);
+               libinput_dispatch(li);
+               event = libinput_get_event(li);
+       }
+}
+
+void
 litest_assert_touch_sequence(struct libinput *li)
 {
        struct libinput_event *event;

diff --git a/test/litest.h b/test/litest.h
index fea10f7..06072c1 100644 (file)
--- a/test/litest.h
+++ b/test/litest.h
@@ -776,6 +776,10 @@ litest_assert_only_typed_events(struct libinput *li,
                                enum libinput_event_type type);
 
 void
+litest_assert_no_typed_events(struct libinput *li,
+                             enum libinput_event_type type);
+
+void
 litest_assert_tablet_button_event(struct libinput *li,
                                  unsigned int button,
                                  enum libinput_button_state state);

diff --git a/test/test-touchpad-tap.c b/test/test-touchpad-tap.c
index 0862908..cfb3dd4 100644 (file)
--- 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);