From ce53d2b6195fcb175e0ed81bf741d8c453788db5 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 20 Jun 2014 14:16:13 +1000 Subject: [PATCH libinput] touchpad: disable tapping for fingers exceeding the timeout/motion threshold The current code triggers multi-finger tapping even if the finger released was previously held on the touchpad for a while. For an event sequence of: 1. first finger down 2. first finger move past threshold/wait past timeout 3. second finger down 4. first finger up The second finger initiates the two-finger tap state, but the button event is sent when the first finger releases - despite that finger not meeting the usual tap constraints. This sequence can happen whenever a user swaps fingers. Add the finger state to the actual touchpoints and update them whenever the constrains are broken. Then, discard button events if the respective touch did not meet the conditions. http://bugs.freedesktop.org/76760 Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad-tap.c | 154 +++++++++++++++++++++++++++++--------------- src/evdev-mt-touchpad.h | 10 +++ test/touchpad.c | 49 ++++++++++++++ 3 files changed, 161 insertions(+), 52 deletions(-) diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index 34bb0d0..4a73423 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -95,12 +95,16 @@ tap_event_to_str(enum tap_event event) { static void tp_tap_notify(struct tp_dispatch *tp, + struct tp_touch *t, uint64_t time, int nfingers, enum libinput_button_state state) { int32_t button; + if (t && t->tap.state == TAP_TOUCH_STATE_DEAD) + return; + switch (nfingers) { case 1: button = BTN_LEFT; break; case 2: button = BTN_RIGHT; break; @@ -128,7 +132,9 @@ tp_tap_clear_timer(struct tp_dispatch *tp) } static void -tp_tap_idle_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_idle_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -149,7 +155,9 @@ tp_tap_idle_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t } static void -tp_tap_touch_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_touch_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -159,7 +167,7 @@ tp_tap_touch_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t break; case TAP_EVENT_RELEASE: tp->tap.state = TAP_STATE_TAPPED; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_PRESSED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_PRESSED); tp_tap_set_timer(tp, time); break; case TAP_EVENT_TIMEOUT: @@ -174,7 +182,9 @@ tp_tap_touch_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t } static void -tp_tap_hold_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_hold_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -195,7 +205,9 @@ tp_tap_hold_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t } static void -tp_tap_tapped_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_tapped_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -209,17 +221,19 @@ tp_tap_tapped_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_ break; case TAP_EVENT_TIMEOUT: tp->tap.state = TAP_STATE_IDLE; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); break; case TAP_EVENT_BUTTON: tp->tap.state = TAP_STATE_DEAD; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); break; } } static void -tp_tap_touch2_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_touch2_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -229,8 +243,8 @@ tp_tap_touch2_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_ break; case TAP_EVENT_RELEASE: tp->tap.state = TAP_STATE_HOLD; - tp_tap_notify(tp, time, 2, LIBINPUT_BUTTON_STATE_PRESSED); - tp_tap_notify(tp, time, 2, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 2, LIBINPUT_BUTTON_STATE_PRESSED); + tp_tap_notify(tp, t, time, 2, LIBINPUT_BUTTON_STATE_RELEASED); tp_tap_clear_timer(tp); break; case TAP_EVENT_MOTION: @@ -245,7 +259,9 @@ tp_tap_touch2_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_ } static void -tp_tap_touch2_hold_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_touch2_hold_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -267,7 +283,9 @@ tp_tap_touch2_hold_handle_event(struct tp_dispatch *tp, enum tap_event event, ui } static void -tp_tap_touch3_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_touch3_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -282,8 +300,8 @@ tp_tap_touch3_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_ break; case TAP_EVENT_RELEASE: tp->tap.state = TAP_STATE_TOUCH_2_HOLD; - tp_tap_notify(tp, time, 3, LIBINPUT_BUTTON_STATE_PRESSED); - tp_tap_notify(tp, time, 3, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 3, LIBINPUT_BUTTON_STATE_PRESSED); + tp_tap_notify(tp, t, time, 3, LIBINPUT_BUTTON_STATE_RELEASED); break; case TAP_EVENT_BUTTON: tp->tap.state = TAP_STATE_DEAD; @@ -292,7 +310,9 @@ tp_tap_touch3_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_ } static void -tp_tap_touch3_hold_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_touch3_hold_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -313,7 +333,9 @@ tp_tap_touch3_hold_handle_event(struct tp_dispatch *tp, enum tap_event event, ui } static void -tp_tap_dragging_or_doubletap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_dragging_or_doubletap_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { case TAP_EVENT_TOUCH: @@ -321,9 +343,9 @@ tp_tap_dragging_or_doubletap_handle_event(struct tp_dispatch *tp, enum tap_event break; case TAP_EVENT_RELEASE: tp->tap.state = TAP_STATE_IDLE; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_PRESSED); - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_PRESSED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); tp_tap_clear_timer(tp); break; case TAP_EVENT_MOTION: @@ -332,13 +354,15 @@ tp_tap_dragging_or_doubletap_handle_event(struct tp_dispatch *tp, enum tap_event break; case TAP_EVENT_BUTTON: tp->tap.state = TAP_STATE_DEAD; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); break; } } static void -tp_tap_dragging_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_dragging_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -355,13 +379,15 @@ tp_tap_dragging_handle_event(struct tp_dispatch *tp, enum tap_event event, uint6 break; case TAP_EVENT_BUTTON: tp->tap.state = TAP_STATE_DEAD; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); break; } } static void -tp_tap_dragging_wait_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_dragging_wait_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -374,17 +400,19 @@ tp_tap_dragging_wait_handle_event(struct tp_dispatch *tp, enum tap_event event, break; case TAP_EVENT_TIMEOUT: tp->tap.state = TAP_STATE_IDLE; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); break; case TAP_EVENT_BUTTON: tp->tap.state = TAP_STATE_DEAD; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); break; } } static void -tp_tap_dragging2_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_dragging2_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, uint64_t time) { switch (event) { @@ -393,7 +421,7 @@ tp_tap_dragging2_handle_event(struct tp_dispatch *tp, enum tap_event event, uint break; case TAP_EVENT_TOUCH: tp->tap.state = TAP_STATE_DEAD; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); break; case TAP_EVENT_MOTION: case TAP_EVENT_TIMEOUT: @@ -401,13 +429,16 @@ tp_tap_dragging2_handle_event(struct tp_dispatch *tp, enum tap_event event, uint break; case TAP_EVENT_BUTTON: tp->tap.state = TAP_STATE_DEAD; - tp_tap_notify(tp, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); + tp_tap_notify(tp, t, time, 1, LIBINPUT_BUTTON_STATE_RELEASED); break; } } static void -tp_tap_dead_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_dead_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, + uint64_t time) { switch (event) { @@ -424,7 +455,10 @@ tp_tap_dead_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t } static void -tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) +tp_tap_handle_event(struct tp_dispatch *tp, + struct tp_touch *t, + enum tap_event event, + uint64_t time) { enum tp_tap_state current; if (!tp->tap.enabled) @@ -434,43 +468,43 @@ tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t time) switch(tp->tap.state) { case TAP_STATE_IDLE: - tp_tap_idle_handle_event(tp, event, time); + tp_tap_idle_handle_event(tp, t, event, time); break; case TAP_STATE_TOUCH: - tp_tap_touch_handle_event(tp, event, time); + tp_tap_touch_handle_event(tp, t, event, time); break; case TAP_STATE_HOLD: - tp_tap_hold_handle_event(tp, event, time); + tp_tap_hold_handle_event(tp, t, event, time); break; case TAP_STATE_TAPPED: - tp_tap_tapped_handle_event(tp, event, time); + tp_tap_tapped_handle_event(tp, t, event, time); break; case TAP_STATE_TOUCH_2: - tp_tap_touch2_handle_event(tp, event, time); + tp_tap_touch2_handle_event(tp, t, event, time); break; case TAP_STATE_TOUCH_2_HOLD: - tp_tap_touch2_hold_handle_event(tp, event, time); + tp_tap_touch2_hold_handle_event(tp, t, event, time); break; case TAP_STATE_TOUCH_3: - tp_tap_touch3_handle_event(tp, event, time); + tp_tap_touch3_handle_event(tp, t, event, time); break; case TAP_STATE_TOUCH_3_HOLD: - tp_tap_touch3_hold_handle_event(tp, event, time); + tp_tap_touch3_hold_handle_event(tp, t, event, time); break; case TAP_STATE_DRAGGING_OR_DOUBLETAP: - tp_tap_dragging_or_doubletap_handle_event(tp, event, time); + tp_tap_dragging_or_doubletap_handle_event(tp, t, event, time); break; case TAP_STATE_DRAGGING: - tp_tap_dragging_handle_event(tp, event, time); + tp_tap_dragging_handle_event(tp, t, event, time); break; case TAP_STATE_DRAGGING_WAIT: - tp_tap_dragging_wait_handle_event(tp, event, time); + tp_tap_dragging_wait_handle_event(tp, t, event, time); break; case TAP_STATE_DRAGGING_2: - tp_tap_dragging2_handle_event(tp, event, time); + tp_tap_dragging2_handle_event(tp, t, event, time); break; case TAP_STATE_DEAD: - tp_tap_dead_handle_event(tp, event, time); + tp_tap_dead_handle_event(tp, t, event, time); break; } @@ -501,19 +535,26 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time) int filter_motion = 0; if (tp->queued & TOUCHPAD_EVENT_BUTTON_PRESS) - tp_tap_handle_event(tp, TAP_EVENT_BUTTON, time); + tp_tap_handle_event(tp, NULL, TAP_EVENT_BUTTON, time); tp_for_each_touch(tp, t) { if (!t->dirty || t->state == TOUCH_NONE) continue; - if (t->state == TOUCH_BEGIN) - tp_tap_handle_event(tp, TAP_EVENT_TOUCH, time); - else if (t->state == TOUCH_END) - tp_tap_handle_event(tp, TAP_EVENT_RELEASE, time); - else if (tp->tap.state != TAP_STATE_IDLE && - tp_tap_exceeds_motion_threshold(tp, t)) - tp_tap_handle_event(tp, TAP_EVENT_MOTION, time); + if (tp->queued & TOUCHPAD_EVENT_BUTTON_PRESS) + t->tap.state = TAP_TOUCH_STATE_DEAD; + + if (t->state == TOUCH_BEGIN) { + t->tap.state = TAP_TOUCH_STATE_TOUCH; + tp_tap_handle_event(tp, t, TAP_EVENT_TOUCH, time); + } else if (t->state == TOUCH_END) { + tp_tap_handle_event(tp, t, TAP_EVENT_RELEASE, time); + t->tap.state = TAP_TOUCH_STATE_DEAD; + } else if (tp->tap.state != TAP_STATE_IDLE && + tp_tap_exceeds_motion_threshold(tp, t)) { + t->tap.state = TAP_TOUCH_STATE_DEAD; + tp_tap_handle_event(tp, t, TAP_EVENT_MOTION, time); + } } /** @@ -543,8 +584,17 @@ static void tp_tap_handle_timeout(uint64_t time, void *data) { struct tp_dispatch *tp = data; + struct tp_touch *t; - tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time); + tp_tap_handle_event(tp, NULL, TAP_EVENT_TIMEOUT, time); + + tp_for_each_touch(tp, t) { + if (t->state == TOUCH_NONE || + t->tap.state == TAP_TOUCH_STATE_IDLE) + continue; + + t->tap.state = TAP_TOUCH_STATE_DEAD; + } } int diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 0b1457d..83a4b5b 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -93,6 +93,12 @@ enum tp_tap_state { TAP_STATE_DEAD, /**< finger count exceeded */ }; +enum tp_tap_touch_state { + TAP_TOUCH_STATE_IDLE = 16, /**< not in touch */ + TAP_TOUCH_STATE_TOUCH, /**< touching, may tap */ + TAP_TOUCH_STATE_DEAD, /**< exceeded motion/timeout */ +}; + struct tp_motion { int32_t x; int32_t y; @@ -136,6 +142,10 @@ struct tp_touch { enum button_event curr; struct libinput_timer timer; } button; + + struct { + enum tp_tap_touch_state state; + } tap; }; struct tp_dispatch { diff --git a/test/touchpad.c b/test/touchpad.c index ca0a700..7350c21 100644 --- a/test/touchpad.c +++ b/test/touchpad.c @@ -320,6 +320,52 @@ START_TEST(touchpad_2fg_tap_click_apple) } END_TEST +START_TEST(touchpad_no_2fg_tap_after_move) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev->libinput; + + litest_drain_events(dev->libinput); + + /* one finger down, move past threshold, + second finger down, first finger up + -> no event + */ + litest_touch_down(dev, 0, 50, 50); + litest_touch_move_to(dev, 0, 50, 50, 90, 90, 10); + litest_drain_events(dev->libinput); + + litest_touch_down(dev, 1, 70, 50); + litest_touch_up(dev, 0); + + litest_assert_empty_queue(li); +} +END_TEST + +START_TEST(touchpad_no_2fg_tap_after_timeout) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev->libinput; + + litest_drain_events(dev->libinput); + + /* one finger down, wait past tap timeout, + second finger down, first finger up + -> no event + */ + litest_touch_down(dev, 0, 50, 50); + libinput_dispatch(dev->libinput); + msleep(300); + libinput_dispatch(dev->libinput); + litest_drain_events(dev->libinput); + + litest_touch_down(dev, 1, 70, 50); + litest_touch_up(dev, 0); + + litest_assert_empty_queue(li); +} +END_TEST + START_TEST(touchpad_1fg_double_tap_click) { struct litest_device *dev = litest_current_device(); @@ -1038,6 +1084,9 @@ int main(int argc, char **argv) { litest_add("touchpad:tap", touchpad_1fg_tap_click, LITEST_TOUCHPAD, LITEST_ANY); litest_add("touchpad:tap", touchpad_2fg_tap_click, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_APPLE_CLICKPAD); litest_add("touchpad:tap", touchpad_2fg_tap_click_apple, LITEST_APPLE_CLICKPAD, LITEST_ANY); + litest_add("touchpad:tap", touchpad_no_2fg_tap_after_move, LITEST_APPLE_CLICKPAD, LITEST_ANY); + litest_add("touchpad:tap", touchpad_no_2fg_tap_after_timeout, LITEST_APPLE_CLICKPAD, LITEST_ANY); + /* Real buttons don't interfere with tapping, so don't run those for pads with buttons */ litest_add("touchpad:tap", touchpad_1fg_double_tap_click, LITEST_CLICKPAD, LITEST_ANY); -- 1.9.3