From 990da54aa69b6e4aeba77573427615163105cb41 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 28 Feb 2018 14:24:27 +1000 Subject: [PATCH 1/5] test: don't run the MT pressure test on devices without MT pressure This test worked because on devices that don't use pressure the touches were reset when BTN_TOUCH when to 0, triggering the 'ignore fake fingers when no real fingers are down' behavior. But this is a different code path than the pressure handling, so let's separate those tests. Signed-off-by: Peter Hutterer --- test/test-touchpad-tap.c | 63 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/test/test-touchpad-tap.c b/test/test-touchpad-tap.c index a57677c2..948be965 100644 --- a/test/test-touchpad-tap.c +++ b/test/test-touchpad-tap.c @@ -1591,13 +1591,19 @@ START_TEST(touchpad_3fg_tap_quickrelease) } END_TEST -START_TEST(touchpad_3fg_tap_hover_btntool) +START_TEST(touchpad_3fg_tap_pressure_btntool) { struct litest_device *dev = litest_current_device(); struct libinput *li = dev->libinput; - if (libevdev_get_abs_maximum(dev->evdev, - ABS_MT_SLOT) >= 2) + if (libevdev_get_abs_maximum(dev->evdev, ABS_MT_SLOT) >= 2) + return; + + /* libinput doesn't export when it uses pressure detection, so we + * need to reconstruct this here. Specifically, semi-mt devices are + * non-mt in libinput, so if they have ABS_PRESSURE, they'll use it. + */ + if (!libevdev_has_event_code(dev->evdev, EV_ABS, ABS_MT_PRESSURE)) return; litest_enable_tap(dev->libinput_device); @@ -1617,6 +1623,56 @@ START_TEST(touchpad_3fg_tap_hover_btntool) * third touch */ litest_event(dev, EV_ABS, ABS_MT_PRESSURE, 3); litest_event(dev, EV_ABS, ABS_PRESSURE, 3); + litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0); + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + + litest_push_event_frame(dev); + litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 1); + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 0); + litest_pop_event_frame(dev); + litest_assert_empty_queue(li); + + litest_touch_up(dev, 0); + litest_touch_up(dev, 1); +} +END_TEST + +START_TEST(touchpad_3fg_tap_hover_btntool) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev->libinput; + + if (libevdev_get_abs_maximum(dev->evdev, ABS_MT_SLOT) >= 2) + return; + + /* libinput doesn't export when it uses pressure detection, so we + * need to reconstruct this here. Specifically, semi-mt devices are + * non-mt in libinput, so if they have ABS_PRESSURE, they'll use it. + */ + if (libevdev_has_event_code(dev->evdev, EV_ABS, ABS_MT_PRESSURE)) + return; + + if (libevdev_has_property(dev->evdev, INPUT_PROP_SEMI_MT) && + libevdev_has_event_code(dev->evdev, EV_ABS, ABS_PRESSURE)) + return; + + litest_enable_tap(dev->libinput_device); + litest_enable_edge_scroll(dev); + + litest_drain_events(li); + + litest_touch_down(dev, 0, 50, 50); + litest_touch_down(dev, 1, 70, 50); + libinput_dispatch(li); + + litest_touch_move_to(dev, 0, 50, 50, 50, 70, 10, 0); + litest_touch_move_to(dev, 1, 70, 50, 50, 70, 10, 0); + litest_drain_events(li); + + /* drop below the pressure threshold in the same frame as starting a + * third touch */ litest_event(dev, EV_KEY, BTN_TOUCH, 0); litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0); litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 1); @@ -3317,6 +3373,7 @@ litest_setup_tests_touchpad_tap(void) litest_add("tap-3fg:3fg", touchpad_3fg_tap_tap_again, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); litest_add("tap-3fg:3fg", touchpad_3fg_tap_quickrelease, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); litest_add("tap-3fg:3fg", touchpad_3fg_tap_hover_btntool, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); + litest_add("tap-3fg:3fg", touchpad_3fg_tap_pressure_btntool, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); litest_add_for_device("tap-3fg:3fg", touchpad_3fg_tap_btntool_pointerjump, LITEST_SYNAPTICS_TOPBUTTONPAD); litest_add("tap-4fg:4fg", touchpad_4fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT); litest_add("tap-4fg:4fg", touchpad_4fg_tap_quickrelease, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT); From 21b83dfd0b0096326ab003f2143a1965b677ceaa Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 28 Feb 2018 14:05:31 +1000 Subject: [PATCH 2/5] test: don't run the 2fg pressure test on single-touch touchpads Only the appletouch has pressure and thus executed that code path Signed-off-by: Peter Hutterer --- test/test-touchpad.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-touchpad.c b/test/test-touchpad.c index 836640b7..bf342b95 100644 --- a/test/test-touchpad.c +++ b/test/test-touchpad.c @@ -5805,7 +5805,7 @@ litest_setup_tests_touchpad(void) litest_add("touchpad:pressure", touchpad_pressure_2fg_st, LITEST_TOUCHPAD|LITEST_SINGLE_TOUCH, LITEST_ANY); litest_add("touchpad:pressure", touchpad_pressure_tap, LITEST_TOUCHPAD, LITEST_ANY); litest_add("touchpad:pressure", touchpad_pressure_tap_2fg, LITEST_TOUCHPAD, LITEST_ANY); - litest_add("touchpad:pressure", touchpad_pressure_tap_2fg_1fg_light, LITEST_TOUCHPAD, LITEST_ANY); + litest_add("touchpad:pressure", touchpad_pressure_tap_2fg_1fg_light, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); litest_add("touchpad:touch-size", touchpad_touch_size, LITEST_APPLE_CLICKPAD, LITEST_ANY); litest_add("touchpad:touch-size", touchpad_touch_size_2fg, LITEST_APPLE_CLICKPAD, LITEST_ANY); From 85e5d80cd41e0921f4b77248ff521e1cdad8e9b2 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 28 Feb 2018 11:02:41 +1000 Subject: [PATCH 3/5] touchpad: add the pressure thresholds to the debugging output Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 2a6cdcef..eaa9215a 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -3073,7 +3073,9 @@ tp_init_pressure(struct tp_dispatch *tp, tp->pressure.low = lo; evdev_log_debug(device, - "using pressure-based touch detection\n"); + "using pressure-based touch detection (%d:%d)\n", + lo, + hi); } static bool From 3979b9e16a5ed141506d95f80ddfd7b94651dcfa Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 28 Feb 2018 11:38:12 +1000 Subject: [PATCH 4/5] touchpad: don't end below-threshold pressure touches if nfake_fingers > nslots If we have more BTN_TOOL_*TAP fingers down than we have slots, ignore any below-threshold pressure changes on the slots. When a touchpad only detects two touches, guessing whether the third touch has sufficient pressure is unreliable. Instead, always assume that all touches have sufficient pressure when we exceed the slot number. Exception: if all real fingers are below the pressure threshold, the fake fingers are ignored too. Related to https://bugs.freedesktop.org/show_bug.cgi?id=105258 Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad.c | 4 +++- test/test-touchpad-tap.c | 11 ++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index eaa9215a..f9bec925 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -1103,7 +1103,9 @@ tp_unhover_pressure(struct tp_dispatch *tp, uint64_t time) tp_motion_history_reset(t); tp_begin_touch(tp, t, time); } - } else { + /* don't unhover for pressure if we have too many + * fake fingers down, see comment below */ + } else if (nfake_touches <= tp->num_slots) { if (t->pressure < tp->pressure.low) { evdev_log_debug(tp->device, "pressure: end touch %d\n", diff --git a/test/test-touchpad-tap.c b/test/test-touchpad-tap.c index 948be965..aa1ea0f5 100644 --- a/test/test-touchpad-tap.c +++ b/test/test-touchpad-tap.c @@ -1632,10 +1632,19 @@ START_TEST(touchpad_3fg_tap_pressure_btntool) litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 1); litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 0); litest_pop_event_frame(dev); - litest_assert_empty_queue(li); litest_touch_up(dev, 0); litest_touch_up(dev, 1); + libinput_dispatch(li); + litest_timeout_tap(); + libinput_dispatch(li); + + litest_assert_button_event(li, + BTN_MIDDLE, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_button_event(li, + BTN_MIDDLE, + LIBINPUT_BUTTON_STATE_RELEASED); } END_TEST From 6ccd8e934f965150173866db265ca544031c6e6b Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 28 Feb 2018 12:51:27 +1000 Subject: [PATCH 5/5] touchpad: add a TOUCH_MAYBE_END state This state is used by the pre-processing of the touch states to indicate that the touch point has ended and is changed to TOUCH_END as soon as that pre-processing is finished. Sometimes we have to resurrect a touch point that has physically or logically ended but needs to be kept around to keep the BTN_TOOL_* fake finger count happy. Particularly on Synaptics touchpads, where a BTN_TOOL_TRIPLETAP can cause a touch point to end (i.e. 1 touch down + TRIPLETAP) but that touch restarts in the next sequence. We had a quirk for this in place already, but if we end the touch and then re-instate it with tp_begin_touch(), we may lose some information about thumb/palm/etc. states that touch already had. As a result, the state machines can get confused and a touch that was previously ignored as thumb suddenly isn't one anymore and triggers assertions. The specific sequence in bug 10528 is: * touch T1 down * touch T2 down, detected as speed-based thumb, tap state machine ignores it * frame F: TRIPLETAP down, touch T2 up * frame F+1: touch T2 down in next frame, but without the thumb bit * frame F+n: touch T2 ends, tap state machine gets confused because that touch should not trigger a release https://bugs.freedesktop.org/show_bug.cgi?id=105258 Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad-edge-scroll.c | 8 +++ src/evdev-mt-touchpad.c | 93 +++++++++++++++++++++++------ src/evdev-mt-touchpad.h | 9 +-- test/test-touchpad.c | 67 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 23 deletions(-) diff --git a/src/evdev-mt-touchpad-edge-scroll.c b/src/evdev-mt-touchpad-edge-scroll.c index a29d9aff..b218415f 100644 --- a/src/evdev-mt-touchpad-edge-scroll.c +++ b/src/evdev-mt-touchpad-edge-scroll.c @@ -366,6 +366,14 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp, uint64_t time) case TOUCH_UPDATE: tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_MOTION); break; + case TOUCH_MAYBE_END: + /* This shouldn't happen we transfer to TOUCH_END + * before processing state */ + evdev_log_debug(tp->device, + "touch %d: unexpected state %d\n", + t->index, + t->state); + /* fallthrough */ case TOUCH_END: tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_RELEASE); break; diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index f9bec925..2c460829 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -302,22 +302,66 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) } /** - * End a touch, even if the touch sequence is still active. + * Schedule a touch to be ended, based on either the events or some + * attributes of the touch (size, pressure). In some cases we need to + * resurrect a touch that has ended, so this doesn't actually end the touch + * yet. All the TOUCH_MAYBE_END touches get properly ended once the device + * state has been processed once and we know how many zombie touches we + * need. */ static inline void -tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) +tp_maybe_end_touch(struct tp_dispatch *tp, + struct tp_touch *t, + uint64_t time) { switch (t->state) { - case TOUCH_HOVERING: - t->state = TOUCH_NONE; - /* fallthough */ case TOUCH_NONE: + case TOUCH_MAYBE_END: + case TOUCH_HOVERING: + return; case TOUCH_END: + evdev_log_bug_libinput(tp->device, + "touch %d: already in TOUCH_END\n", + t->index); return; case TOUCH_BEGIN: case TOUCH_UPDATE: break; + } + t->dirty = true; + t->state = TOUCH_MAYBE_END; + + assert(tp->nfingers_down >= 1); + tp->nfingers_down--; +} + +/** + * Inverse to tp_maybe_end_touch(), restores a touch back to its previous + * state. + */ +static inline void +tp_recover_ended_touch(struct tp_dispatch *tp, + struct tp_touch *t) +{ + t->dirty = true; + t->state = TOUCH_UPDATE; + tp->nfingers_down++; +} + +/** + * End a touch, even if the touch sequence is still active. + * Use tp_maybe_end_touch() instead. + */ +static inline void +tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) +{ + if (t->state != TOUCH_MAYBE_END) { + evdev_log_bug_libinput(tp->device, + "touch %d should be MAYBE_END, is %d\n", + t->index, + t->state); + return; } t->dirty = true; @@ -326,8 +370,6 @@ tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) t->pinned.is_pinned = false; t->time = time; t->palm.time = 0; - assert(tp->nfingers_down >= 1); - tp->nfingers_down--; tp->queued |= TOUCHPAD_EVENT_MOTION; } @@ -338,7 +380,7 @@ static inline void tp_end_sequence(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) { t->has_ended = true; - tp_end_touch(tp, t, time); + tp_maybe_end_touch(tp, t, time); } static void @@ -485,13 +527,11 @@ tp_restore_synaptics_touches(struct tp_dispatch *tp, for (i = 0; i < tp->num_slots; i++) { struct tp_touch *t = tp_get_touch(tp, i); - if (t->state != TOUCH_END) + if (t->state != TOUCH_MAYBE_END) continue; /* new touch, move it through begin to update immediately */ - tp_new_touch(tp, t, time); - tp_begin_touch(tp, t, time); - t->state = TOUCH_UPDATE; + tp_recover_ended_touch(tp, t); } } @@ -1110,7 +1150,7 @@ tp_unhover_pressure(struct tp_dispatch *tp, uint64_t time) evdev_log_debug(tp->device, "pressure: end touch %d\n", t->index); - tp_end_touch(tp, t, time); + tp_maybe_end_touch(tp, t, time); } } } @@ -1147,10 +1187,11 @@ tp_unhover_pressure(struct tp_dispatch *tp, uint64_t time) t = tp_get_touch(tp, i); if (t->state == TOUCH_HOVERING || - t->state == TOUCH_NONE) + t->state == TOUCH_NONE || + t->state == TOUCH_MAYBE_END) continue; - tp_end_touch(tp, t, time); + tp_maybe_end_touch(tp, t, time); if (real_fingers_down > 0 && tp->nfingers_down == nfake_touches) @@ -1194,7 +1235,7 @@ tp_unhover_size(struct tp_dispatch *tp, uint64_t time) evdev_log_debug(tp->device, "touch-size: end touch %d\n", t->index); - tp_end_touch(tp, t, time); + tp_maybe_end_touch(tp, t, time); } } } @@ -1248,7 +1289,7 @@ tp_unhover_fake_touches(struct tp_dispatch *tp, uint64_t time) t->state == TOUCH_NONE) continue; - tp_end_touch(tp, t, time); + tp_maybe_end_touch(tp, t, time); if (tp_fake_finger_is_touching(tp) && tp->nfingers_down == nfake_touches) @@ -1416,6 +1457,21 @@ tp_detect_thumb_while_moving(struct tp_dispatch *tp) second->thumb.state = THUMB_STATE_YES; } +static void +tp_pre_process_state(struct tp_dispatch *tp, uint64_t time) +{ + struct tp_touch *t; + + tp_process_fake_touches(tp, time); + tp_unhover_touches(tp, time); + + tp_for_each_touch(tp, t) { + if (t->state == TOUCH_MAYBE_END) + tp_end_touch(tp, t, time); + } + +} + static void tp_process_state(struct tp_dispatch *tp, uint64_t time) { @@ -1425,8 +1481,6 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time) bool have_new_touch = false; unsigned int speed_exceeded_count = 0; - tp_process_fake_touches(tp, time); - tp_unhover_touches(tp, time); tp_position_fake_touches(tp); want_motion_reset = tp_need_motion_history_reset(tp); @@ -1588,6 +1642,7 @@ tp_handle_state(struct tp_dispatch *tp, if (tp->hysteresis.enabled) tp_maybe_disable_hysteresis(tp, time); + tp_pre_process_state(tp, time); tp_process_state(tp, time); tp_post_events(tp, time); tp_post_process_state(tp, time); diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index afd0983d..c14572ad 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -46,10 +46,11 @@ enum touchpad_event { enum touch_state { TOUCH_NONE = 0, - TOUCH_HOVERING, - TOUCH_BEGIN, - TOUCH_UPDATE, - TOUCH_END + TOUCH_HOVERING = 1, + TOUCH_BEGIN = 2, + TOUCH_UPDATE = 3, + TOUCH_MAYBE_END = 4, + TOUCH_END = 5, }; enum touch_palm_state { diff --git a/test/test-touchpad.c b/test/test-touchpad.c index bf342b95..98f9b620 100644 --- a/test/test-touchpad.c +++ b/test/test-touchpad.c @@ -5423,6 +5423,72 @@ START_TEST(touchpad_pressure_tap_2fg_1fg_light) } END_TEST +START_TEST(touchpad_pressure_btntool) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev->libinput; + struct axis_replacement axes[] = { + { ABS_MT_PRESSURE, 5 }, + { ABS_PRESSURE, 5 }, + { -1, 0 } + }; + + /* we only have tripletap, can't test 4 slots because nothing will + * happen */ + if (libevdev_get_num_slots(dev->evdev) != 2) + return; + + if (!touchpad_has_pressure(dev)) + return; + + litest_enable_tap(dev->libinput_device); + litest_drain_events(li); + + /* Two light touches down, doesn't count */ + litest_touch_down_extended(dev, 0, 40, 50, axes); + litest_touch_down_extended(dev, 1, 45, 50, axes); + libinput_dispatch(li); + litest_assert_empty_queue(li); + + /* Tripletap but since no finger is logically down, it doesn't count */ + litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0); + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + litest_assert_empty_queue(li); + + /* back to two fingers */ + litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 1); + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + + /* make one finger real */ + litest_touch_move_to(dev, 0, 40, 50, 41, 52, 10, 10); + litest_drain_events(li); + + /* tripletap should now be 3 fingers tap */ + litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0); + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 1); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + + litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 1); + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 0); + litest_event(dev, EV_SYN, SYN_REPORT, 0); + libinput_dispatch(li); + + litest_timeout_tap(); + libinput_dispatch(li); + + litest_assert_button_event(li, + BTN_MIDDLE, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_button_event(li, + BTN_MIDDLE, + LIBINPUT_BUTTON_STATE_RELEASED); +} +END_TEST + static inline bool touchpad_has_touch_size(struct litest_device *dev) { @@ -5806,6 +5872,7 @@ litest_setup_tests_touchpad(void) litest_add("touchpad:pressure", touchpad_pressure_tap, LITEST_TOUCHPAD, LITEST_ANY); litest_add("touchpad:pressure", touchpad_pressure_tap_2fg, LITEST_TOUCHPAD, LITEST_ANY); litest_add("touchpad:pressure", touchpad_pressure_tap_2fg_1fg_light, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); + litest_add("touchpad:pressure", touchpad_pressure_btntool, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); litest_add("touchpad:touch-size", touchpad_touch_size, LITEST_APPLE_CLICKPAD, LITEST_ANY); litest_add("touchpad:touch-size", touchpad_touch_size_2fg, LITEST_APPLE_CLICKPAD, LITEST_ANY);