From: José Expósito Date: Mon, 31 May 2021 15:58:39 +0000 (+0200) Subject: gestures: improve one finger hold detection X-Git-Tag: 1.18.901~50 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1f548864bc5b800bee63a505aa69157af4b013a2;p=platform%2Fupstream%2Flibinput.git 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 --- diff --git a/src/evdev-mt-touchpad-gestures.c b/src/evdev-mt-touchpad-gestures.c index 0022cd3b..aa949ae1 100644 --- 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 c44c7514..ad80cded 100644 --- 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 7f8701fd..0a5566ad 100644 --- 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 e55da5c1..666e2b95 100644 --- 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);