gestures: improve one finger hold detection

When one finger is used to hold, tiny pointer movement deltas can easily
end the gesture.

Add a movement threshold to avoid small movement, before or after the hold
timeout, ending the gesture and make the hold-to-interact user
interaction more reliable.

Signed-off-by: José Expósito <jose.exposito89@gmail.com>

diff --git a/src/evdev-mt-touchpad-gestures.c b/src/evdev-mt-touchpad-gestures.c
index 0022cd3b422797537c0ac8f9e16fdfab2a7bb858..aa949ae11f4147a7122e74ea0c9a8a0c99d9d192 100644 (file)
--- a/src/evdev-mt-touchpad-gestures.c
+++ b/src/evdev-mt-touchpad-gestures.c
@@ -34,12 +34,14 @@
 #define DEFAULT_GESTURE_SWIPE_TIMEOUT ms2us(150)
 #define DEFAULT_GESTURE_PINCH_TIMEOUT ms2us(300)
 
+#define HOLD_AND_MOTION_THRESHOLD 0.5 /* mm */
 #define PINCH_DISAMBIGUATION_MOVE_THRESHOLD 1.5 /* mm */
 
 enum gesture_event {
        GESTURE_EVENT_RESET,
        GESTURE_EVENT_FINGER_DETECTED,
        GESTURE_EVENT_HOLD_TIMEOUT,
+       GESTURE_EVENT_HOLD_AND_MOTION,
        GESTURE_EVENT_POINTER_MOTION,
        GESTURE_EVENT_SCROLL,
        GESTURE_EVENT_SWIPE,
@@ -62,6 +64,7 @@ gesture_state_to_str(enum tp_gesture_state state)
        CASE_RETURN_STRING(GESTURE_STATE_NONE);
        CASE_RETURN_STRING(GESTURE_STATE_UNKNOWN);
        CASE_RETURN_STRING(GESTURE_STATE_HOLD);
+       CASE_RETURN_STRING(GESTURE_STATE_HOLD_AND_MOTION);
        CASE_RETURN_STRING(GESTURE_STATE_POINTER_MOTION);
        CASE_RETURN_STRING(GESTURE_STATE_SCROLL);
        CASE_RETURN_STRING(GESTURE_STATE_PINCH);
@@ -77,6 +80,7 @@ gesture_event_to_str(enum gesture_event event)
        CASE_RETURN_STRING(GESTURE_EVENT_RESET);
        CASE_RETURN_STRING(GESTURE_EVENT_FINGER_DETECTED);
        CASE_RETURN_STRING(GESTURE_EVENT_HOLD_TIMEOUT);
+       CASE_RETURN_STRING(GESTURE_EVENT_HOLD_AND_MOTION);
        CASE_RETURN_STRING(GESTURE_EVENT_POINTER_MOTION);
        CASE_RETURN_STRING(GESTURE_EVENT_SCROLL);
        CASE_RETURN_STRING(GESTURE_EVENT_SWIPE);
@@ -159,6 +163,7 @@ tp_gesture_start(struct tp_dispatch *tp, uint64_t time)
                                       __func__);
                break;
        case GESTURE_STATE_HOLD:
+       case GESTURE_STATE_HOLD_AND_MOTION:
                gesture_notify_hold(&tp->device->base, time,
                                    tp->gesture.finger_count);
                break;
@@ -570,6 +575,7 @@ tp_gesture_none_handle_event(struct tp_dispatch *tp,
        case GESTURE_EVENT_SCROLL:
                tp->gesture.state = GESTURE_STATE_SCROLL;
                break;
+       case GESTURE_EVENT_HOLD_AND_MOTION:
        case GESTURE_EVENT_SWIPE:
        case GESTURE_EVENT_PINCH:
                log_gesture_bug(tp, event);
@@ -592,7 +598,8 @@ tp_gesture_unknown_handle_event(struct tp_dispatch *tp,
                tp_gesture_start(tp, time);
                break;
        case GESTURE_EVENT_POINTER_MOTION:
-               libinput_timer_cancel(&tp->gesture.hold_timer);
+               /* Don't cancel the hold timer. This pointer motion can end up
+                * being recognised as hold and motion. */
                tp->gesture.state = GESTURE_STATE_POINTER_MOTION;
                break;
        case GESTURE_EVENT_SCROLL:
@@ -609,6 +616,7 @@ tp_gesture_unknown_handle_event(struct tp_dispatch *tp,
                tp_gesture_init_pinch(tp);
                tp->gesture.state = GESTURE_STATE_PINCH;
                break;
+       case GESTURE_EVENT_HOLD_AND_MOTION:
        case GESTURE_EVENT_FINGER_DETECTED:
                log_gesture_bug(tp, event);
                break;
@@ -625,6 +633,9 @@ tp_gesture_hold_handle_event(struct tp_dispatch *tp,
                libinput_timer_cancel(&tp->gesture.hold_timer);
                tp->gesture.state = GESTURE_STATE_NONE;
                break;
+       case GESTURE_EVENT_HOLD_AND_MOTION:
+               tp->gesture.state = GESTURE_STATE_HOLD_AND_MOTION;
+               break;
        case GESTURE_EVENT_POINTER_MOTION:
                tp_gesture_cancel(tp, time);
                tp->gesture.state = GESTURE_STATE_POINTER_MOTION;
@@ -650,18 +661,60 @@ tp_gesture_hold_handle_event(struct tp_dispatch *tp,
        }
 }
 
+static void
+tp_gesture_hold_and_motion_handle_event(struct tp_dispatch *tp,
+                                       enum gesture_event event,
+                                       uint64_t time)
+{
+       switch(event) {
+       case GESTURE_EVENT_RESET:
+               libinput_timer_cancel(&tp->gesture.hold_timer);
+               tp->gesture.state = GESTURE_STATE_NONE;
+               break;
+       case GESTURE_EVENT_POINTER_MOTION:
+               tp_gesture_cancel(tp, time);
+               tp->gesture.state = GESTURE_STATE_POINTER_MOTION;
+               break;
+       case GESTURE_EVENT_HOLD_AND_MOTION:
+       case GESTURE_EVENT_FINGER_DETECTED:
+       case GESTURE_EVENT_HOLD_TIMEOUT:
+       case GESTURE_EVENT_SCROLL:
+       case GESTURE_EVENT_SWIPE:
+       case GESTURE_EVENT_PINCH:
+               log_gesture_bug(tp, event);
+               break;
+       }
+}
+
 static void
 tp_gesture_pointer_motion_handle_event(struct tp_dispatch *tp,
                                       enum gesture_event event,
                                       uint64_t time)
 {
+       struct tp_touch *first;
+       struct phys_coords first_moved;
+       double first_mm;
+
        switch(event) {
        case GESTURE_EVENT_RESET:
                libinput_timer_cancel(&tp->gesture.hold_timer);
                tp->gesture.state = GESTURE_STATE_NONE;
                break;
-       case GESTURE_EVENT_FINGER_DETECTED:
        case GESTURE_EVENT_HOLD_TIMEOUT:
+               if (tp->gesture.finger_count != 1)
+                       break;
+
+               first = tp->gesture.touches[0];
+               first_moved = tp_gesture_mm_moved(tp, first);
+               first_mm = hypot(first_moved.x, first_moved.y);
+
+               if (first_mm < HOLD_AND_MOTION_THRESHOLD) {
+                       tp->gesture.state = GESTURE_STATE_HOLD_AND_MOTION;
+                       tp_gesture_start(tp, time);
+               }
+               break;
+       case GESTURE_EVENT_HOLD_AND_MOTION:
+       case GESTURE_EVENT_FINGER_DETECTED:
        case GESTURE_EVENT_POINTER_MOTION:
        case GESTURE_EVENT_SCROLL:
        case GESTURE_EVENT_SWIPE:
@@ -681,6 +734,7 @@ tp_gesture_scroll_handle_event(struct tp_dispatch *tp,
                libinput_timer_cancel(&tp->gesture.hold_timer);
                tp->gesture.state = GESTURE_STATE_NONE;
                break;
+       case GESTURE_EVENT_HOLD_AND_MOTION:
        case GESTURE_EVENT_FINGER_DETECTED:
        case GESTURE_EVENT_HOLD_TIMEOUT:
        case GESTURE_EVENT_POINTER_MOTION:
@@ -702,6 +756,7 @@ tp_gesture_pinch_handle_event(struct tp_dispatch *tp,
                libinput_timer_cancel(&tp->gesture.hold_timer);
                tp->gesture.state = GESTURE_STATE_NONE;
                break;
+       case GESTURE_EVENT_HOLD_AND_MOTION:
        case GESTURE_EVENT_FINGER_DETECTED:
        case GESTURE_EVENT_HOLD_TIMEOUT:
        case GESTURE_EVENT_POINTER_MOTION:
@@ -723,6 +778,7 @@ tp_gesture_swipe_handle_event(struct tp_dispatch *tp,
                libinput_timer_cancel(&tp->gesture.hold_timer);
                tp->gesture.state = GESTURE_STATE_NONE;
                break;
+       case GESTURE_EVENT_HOLD_AND_MOTION:
        case GESTURE_EVENT_FINGER_DETECTED:
        case GESTURE_EVENT_HOLD_TIMEOUT:
        case GESTURE_EVENT_POINTER_MOTION:
@@ -753,6 +809,9 @@ tp_gesture_handle_event(struct tp_dispatch *tp,
        case GESTURE_STATE_HOLD:
                tp_gesture_hold_handle_event(tp, event, time);
                break;
+       case GESTURE_STATE_HOLD_AND_MOTION:
+               tp_gesture_hold_and_motion_handle_event(tp, event, time);
+               break;
        case GESTURE_STATE_POINTER_MOTION:
                tp_gesture_pointer_motion_handle_event(tp, event, time);
                break;
@@ -810,16 +869,32 @@ tp_gesture_detect_motion_gestures(struct tp_dispatch *tp, uint64_t time)
        double thumb_mm, finger_mm;
        double min_move = 1.5; /* min movement threshold in mm - count this touch */
        double max_move = 4.0; /* max movement threshold in mm - ignore other touch */
+       bool is_hold_and_motion;
 
        first_moved = tp_gesture_mm_moved(tp, first);
        first_mm = hypot(first_moved.x, first_moved.y);
 
        if (tp->gesture.finger_count == 1) {
-               if (tp_has_pending_pointer_motion(tp, time)) {
+               if (!tp_has_pending_pointer_motion(tp, time))
+                       return;
+
+               is_hold_and_motion = (first_mm < HOLD_AND_MOTION_THRESHOLD);
+
+               if (tp->gesture.state == GESTURE_STATE_HOLD &&
+                   is_hold_and_motion) {
                        tp_gesture_handle_event(tp,
-                                               GESTURE_EVENT_POINTER_MOTION,
+                                               GESTURE_EVENT_HOLD_AND_MOTION,
                                                time);
+                       return;
                }
+
+               if (tp->gesture.state == GESTURE_STATE_HOLD_AND_MOTION &&
+                   is_hold_and_motion)
+                       return;
+
+               tp_gesture_handle_event(tp,
+                                       GESTURE_EVENT_POINTER_MOTION,
+                                       time);
                return;
        }
 
@@ -1061,6 +1136,15 @@ tp_gesture_handle_state_hold(struct tp_dispatch *tp, uint64_t time,
                tp_gesture_detect_motion_gestures(tp, time);
 }
 
+static void
+tp_gesture_handle_state_hold_and_pointer_motion(struct tp_dispatch *tp, uint64_t time)
+{
+       if (tp->queued & TOUCHPAD_EVENT_MOTION)
+               tp_gesture_post_pointer_motion(tp, time);
+
+       tp_gesture_detect_motion_gestures(tp, time);
+}
+
 static void
 tp_gesture_handle_state_pointer_motion(struct tp_dispatch *tp, uint64_t time)
 {
@@ -1172,6 +1256,9 @@ tp_gesture_post_gesture(struct tp_dispatch *tp, uint64_t time,
        if (tp->gesture.state == GESTURE_STATE_HOLD)
                tp_gesture_handle_state_hold(tp, time, ignore_motion);
 
+       if (tp->gesture.state == GESTURE_STATE_HOLD_AND_MOTION)
+               tp_gesture_handle_state_hold_and_pointer_motion(tp, time);
+
        if (tp->gesture.state == GESTURE_STATE_POINTER_MOTION)
                tp_gesture_handle_state_pointer_motion(tp, time);
 
@@ -1269,6 +1356,7 @@ tp_gesture_end(struct tp_dispatch *tp, uint64_t time, bool cancelled)
                                       __func__);
                break;
        case GESTURE_STATE_HOLD:
+       case GESTURE_STATE_HOLD_AND_MOTION:
                gesture_notify_hold_end(&tp->device->base, time,
                                        tp->gesture.finger_count, cancelled);
                break;

diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index c44c7514b76b8d9a71b6f6bc7fa36cd6583602f3..ad80cded30a3cfe4ea56a59a9543ba19e7090bed 100644 (file)
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -158,6 +158,7 @@ enum tp_gesture_state {
        GESTURE_STATE_NONE,
        GESTURE_STATE_UNKNOWN,
        GESTURE_STATE_HOLD,
+       GESTURE_STATE_HOLD_AND_MOTION,
        GESTURE_STATE_POINTER_MOTION,
        GESTURE_STATE_SCROLL,
        GESTURE_STATE_PINCH,

diff --git a/test/test-gestures.c b/test/test-gestures.c
index 7f8701fd380fd8666547dfdf3a222ec0cd85ee85..0a5566ad289691b19a042db4e552ee8fd93b0eda 100644 (file)
--- a/test/test-gestures.c
+++ b/test/test-gestures.c
@@ -1707,6 +1707,76 @@ START_TEST(gestures_hold_once_tap_n_drag)
 }
 END_TEST
 
+START_TEST(gestures_hold_and_motion_before_timeout)
+{
+       struct litest_device *dev = litest_current_device();
+       struct libinput *li = dev->libinput;
+
+       if (!libinput_device_has_capability(dev->libinput_device,
+                                           LIBINPUT_DEVICE_CAP_GESTURE))
+               return;
+
+       litest_drain_events(li);
+
+       litest_touch_down(dev, 0, 50, 50);
+       libinput_dispatch(li);
+
+       litest_touch_move_to(dev, 0, 50, 50, 51, 51, 1);
+       litest_touch_move_to(dev, 0, 51, 51, 50, 50, 1);
+       libinput_dispatch(li);
+
+       litest_timeout_gesture_quick_hold();
+
+       litest_drain_events_of_type(li, LIBINPUT_EVENT_POINTER_MOTION, -1);
+
+       litest_assert_gesture_event(li,
+                                   LIBINPUT_EVENT_GESTURE_HOLD_BEGIN,
+                                   1);
+
+       litest_touch_up(dev, 0);
+       libinput_dispatch(li);
+
+       litest_assert_gesture_event(li,
+                                   LIBINPUT_EVENT_GESTURE_HOLD_END,
+                                   1);
+       litest_assert_empty_queue(li);
+}
+END_TEST
+
+START_TEST(gestures_hold_and_motion_after_timeout)
+{
+       struct litest_device *dev = litest_current_device();
+       struct libinput *li = dev->libinput;
+
+       if (!libinput_device_has_capability(dev->libinput_device,
+                                           LIBINPUT_DEVICE_CAP_GESTURE))
+               return;
+
+       litest_drain_events(li);
+
+       litest_touch_down(dev, 0, 50, 50);
+       libinput_dispatch(li);
+       litest_timeout_gesture_quick_hold();
+
+       litest_assert_gesture_event(li,
+                                   LIBINPUT_EVENT_GESTURE_HOLD_BEGIN,
+                                   1);
+
+       litest_touch_move_to(dev, 0, 50, 50, 51, 51, 1);
+       litest_touch_move_to(dev, 0, 51, 51, 50, 50, 1);
+       libinput_dispatch(li);
+       litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
+
+       litest_touch_up(dev, 0);
+       libinput_dispatch(li);
+
+       litest_assert_gesture_event(li,
+                                   LIBINPUT_EVENT_GESTURE_HOLD_END,
+                                   1);
+       litest_assert_empty_queue(li);
+}
+END_TEST
+
 TEST_COLLECTION(gestures)
 {
        struct range cardinals = { N, N + NCARDINALS };
@@ -1752,6 +1822,9 @@ TEST_COLLECTION(gestures)
        litest_add(gestures_hold_once_on_double_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
        litest_add_ranged(gestures_hold_once_tap_n_drag, LITEST_TOUCHPAD, LITEST_ANY, &range_multifinger_tap);
 
+       litest_add(gestures_hold_and_motion_before_timeout, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
+       litest_add(gestures_hold_and_motion_after_timeout, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
+
        /* Timing-sensitive test, valgrind is too slow */
        if (!RUNNING_ON_VALGRIND)
                litest_add(gestures_swipe_3fg_unaccel, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);

diff --git a/test/test-touchpad-buttons.c b/test/test-touchpad-buttons.c
index e55da5c1d5fcbc289daa374b41582c0e297a70d7..666e2b95409d6d34cdeca2d4ab1a0bc07020a098 100644 (file)
--- a/test/test-touchpad-buttons.c
+++ b/test/test-touchpad-buttons.c
@@ -1107,9 +1107,9 @@ START_TEST(clickpad_finger_pin)
        /* make sure the movement generates pointer events when
           not pinned */
        litest_touch_down(dev, 0, 50, 50);
-       litest_touch_move_to(dev, 0, 50, 50, 52, 52, 10);
-       litest_touch_move_to(dev, 0, 52, 52, 48, 48, 10);
-       litest_touch_move_to(dev, 0, 48, 48, 50, 50, 10);
+       litest_touch_move_to(dev, 0, 50, 50, 54, 54, 10);
+       litest_touch_move_to(dev, 0, 54, 54, 46, 46, 10);
+       litest_touch_move_to(dev, 0, 46, 46, 50, 50, 10);
        litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
 
        litest_button_click(dev, BTN_LEFT, true);