From 2ab233857d42d942eb322888e2d87ba3b7f7b264 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 7 Mar 2018 10:58:17 +1000 Subject: touchpad: handle a palm down in the tapped state We expected the first event after TAPPED to be a finger down. If that finger has been recognised as palm, the finger state isn't TOUCH_BEGIN so we get an invalid state in our FSM. libinput bug: 0: invalid tap event TAP_EVENT_PALM in state TAP_STATE_TAPPED https://bugs.freedesktop.org/show_bug.cgi?id=105370 Signed-off-by: Peter Hutterer --- doc/touchpad-tap-state-machine.svg | 936 +++++++++++++++++++++++-------------- src/evdev-mt-touchpad-tap.c | 2 +- test/test-touchpad-tap.c | 43 ++ 3 files changed, 642 insertions(+), 339 deletions(-) diff --git a/doc/touchpad-tap-state-machine.svg b/doc/touchpad-tap-state-machine.svg index cae45429..5dd10361 100644 --- a/doc/touchpad-tap-state-machine.svg +++ b/doc/touchpad-tap-state-machine.svg @@ -1,6 +1,5 @@ - - + @@ -9,76 +8,96 @@ - + - IDLE + + IDLE - TOUCH + + TOUCH - first - finger down + + first + + finger down - finger up + + finger up - button 1 - press + + button 1 + + press - timeout + + timeout - move > - threshold + + move > + + threshold - second - finger down + + second + + finger down - TOUCH_2 + + TOUCH_2 - second - finger up + + second + + finger up - button 2 - press + + button 2 + + press - move > - threshold + + move > + + threshold - timeout + + timeout @@ -86,116 +105,148 @@ - button 1 - release + + button 1 + + release - button 2 - release + + button 2 + + release + + - TAPPED + + TAPPED - timeout + + timeout - first - finger down + + first + + finger down - DRAGGING + + DRAGGING - first - finger up + + first + + finger up - btn1 - release + + btn1 + + release - - + + - + - IDLE + + IDLE - third - finger down + + third + + finger down - TOUCH_3 + + TOUCH_3 - button 3 - press + + button 3 + + press - button 3 - release + + button 3 + + release - move > - threshold + + move > + + threshold - + - IDLE + + IDLE - timeout + + timeout - first - finger up + + first + + finger up - + - IDLE + + IDLE - fourth - finger down + + fourth + + finger down @@ -203,51 +254,65 @@ - DRAGGING_OR_DOUBLETAP + + DRAGGING_OR_DOUBLETAP - timeout + + timeout - first - finger up + + first + + finger up - button 1 - release + + button 1 + + release - button 1 - press + + button 1 + + press - btn1 - release + + btn1 + + release - second - finger down + + second + + finger down - move > - threshold + + move > + + threshold @@ -255,12 +320,15 @@ - HOLD + + HOLD - first - finger up + + first + + finger up @@ -268,8 +336,10 @@ - second - finger down + + second + + finger down @@ -277,28 +347,35 @@ - TOUCH_2_HOLD + + TOUCH_2_HOLD - second - finger up + + second + + finger up - first - finger up + + first + + finger up - - + + - third - finger down + + third + + finger down @@ -308,18 +385,22 @@ - TOUCH_3_HOLD + + TOUCH_3_HOLD - fourth - finger down + + fourth + + finger down - DEAD + + DEAD @@ -329,28 +410,34 @@ - any finger up + + any finger up - fourth - finger up + + fourth + + finger up - any finger up + + any finger up - - yes + + + yes - any finger up + + any finger up @@ -360,14 +447,17 @@ - + - IDLE + + IDLE - if finger - count == 0 + + if finger + + count == 0 @@ -377,12 +467,15 @@ - second - finger up + + second + + finger up - DRAGGING_2 + + DRAGGING_2 @@ -390,8 +483,10 @@ - first - finger up + + first + + finger up @@ -401,8 +496,10 @@ - second - finger down + + second + + finger down @@ -412,23 +509,30 @@ - third - finger down + + third + + finger down - btn1 - release + + btn1 + + release - phys - button - press + + phys + + button + + press @@ -444,16 +548,21 @@ - phys - button - press + + phys + + button + + press - button 1 - release + + button 1 + + release @@ -467,11 +576,13 @@ - DRAGGING_WAIT + + DRAGGING_WAIT - timeout + + timeout @@ -481,18 +592,22 @@ - first - finger down + + first + + finger down - TOUCH_TOUCH + + TOUCH_TOUCH - TOUCH_IDLE + + TOUCH_IDLE @@ -504,7 +619,8 @@ - TOUCH_DEAD + + TOUCH_DEAD @@ -516,7 +632,8 @@ - TOUCH_DEAD + + TOUCH_DEAD @@ -526,13 +643,15 @@ - TOUCH_IDLE + + TOUCH_IDLE - TOUCH_TOUCH + + TOUCH_TOUCH @@ -540,32 +659,38 @@ - TOUCH_IDLE + + TOUCH_IDLE - TOUCH_IDLE + + TOUCH_IDLE - TOUCH_TOUCH + + TOUCH_TOUCH - that finger - TOUCH_IDLE + + that finger + + TOUCH_IDLE - TOUCH_DEAD + + TOUCH_DEAD @@ -575,36 +700,43 @@ - that finger - TOUCH_IDLE + + that finger + + TOUCH_IDLE - - no + + + no - TOUCH_TOUCH + + TOUCH_TOUCH - TOUCH_IDLE + + TOUCH_IDLE - TOUCH_TOUCH + + TOUCH_TOUCH - TOUCH_DEAD + + TOUCH_DEAD @@ -612,114 +744,138 @@ - TOUCH_IDLE + + TOUCH_IDLE - - + + - TOUCH_TOUCH + + TOUCH_TOUCH - TOUCH_TOUCH + + TOUCH_TOUCH - TOUCH_IDLE + + TOUCH_IDLE - TOUCH_IDLE + + TOUCH_IDLE - TOUCH_TOUCH + + TOUCH_TOUCH - TOUCH_IDLE + + TOUCH_IDLE - TOUCH_TOUCH + + TOUCH_TOUCH - that finger - TOUCH_IDLE + + that finger + + TOUCH_IDLE - TOUCH_DEAD + + TOUCH_DEAD - TOUCH_DEAD + + TOUCH_DEAD - TOUCH_DEAD + + TOUCH_DEAD - TOUCH_DEAD + + TOUCH_DEAD - TOUCH_DEAD + + TOUCH_DEAD - TOUCH_DEAD + + TOUCH_DEAD - that finger state == - TOUCH_TOUCH + + that finger state == + + TOUCH_TOUCH - TOUCH_DEAD + + TOUCH_DEAD - TOUCH_DEAD + + TOUCH_DEAD - TOUCH_DEAD + + TOUCH_DEAD - first - finger down + + first + + finger down - MULTITAP + + MULTITAP @@ -727,15 +883,17 @@ - timeout + + timeout - + - IDLE + + IDLE @@ -745,32 +903,36 @@ - MULTITAP_DOWN + + MULTITAP_DOWN - first - finger up + + first + + finger up - - - timeout + + timeout - - - second - finger down + + second + + finger down - move > - threshold + + move > + + threshold @@ -778,23 +940,26 @@ - - - TOUCH_TOUCH + + TOUCH_TOUCH - TOUCH_IDLE + + TOUCH_IDLE - phys - button - press + + phys + + button + + press @@ -804,18 +969,22 @@ - DRAGGING_OR_TAP + + DRAGGING_OR_TAP - first - finger up + + first + + finger up - timeout + + timeout @@ -823,8 +992,10 @@ - move > - threshold + + move > + + threshold @@ -838,7 +1009,8 @@ - TOUCH_IDLE + + TOUCH_IDLE @@ -847,49 +1019,60 @@ - + - -
-
drag lock
enabled?
-
-
- [Not supported by viewer] + +
+
+ drag lock
+ enabled?
+
+
+
+ + [Not supported by viewer]
- - - + + + - -
-
no
-
-
- no + +
+
+ no
+
+
+ + no
- -
-
yes
-
-
- yes<br> + +
+
+ yes
+
+
+
+ + yes<br>
- thumb + + thumb - TOUCH_DEAD + + TOUCH_DEAD @@ -899,25 +1082,31 @@ - TOUCH_2_RELEASE + + TOUCH_2_RELEASE - second - finger up + + second + + finger up - timeout + + timeout - move > - threshold + + move > + + threshold @@ -929,8 +1118,10 @@ - first - finger down + + first + + finger down @@ -938,14 +1129,17 @@ - TOUCH_IDLE + + TOUCH_IDLE - first - finger up + + first + + finger up @@ -953,18 +1147,22 @@ - second - finger down + + second + + finger down - TOUCH_DEAD + + TOUCH_DEAD - TOUCH_DEAD + + TOUCH_DEAD @@ -975,91 +1173,112 @@ - + - -
-
drag
disabled?
-
-
- drag<br>disabled?<br> + +
+
+ drag
+ disabled?
+
+
+
+ + drag<br>disabled?<br>
- -
-
no
-
-
- no + +
+
+ no
+
+
+ + no
- - + + - -
-
yes
-
-
- yes + +
+
+ yes
+
+
+ + yes
- palm + + palm - either finger - palm + + either finger + + palm - remaining -  finger - palm + + remaining + +  finger + + palm - any finger - palm + + any finger + + palm - - + + - that finger - TOUCH_DEAD + + that finger + + TOUCH_DEAD - that finger - TOUCH_DEAD + + that finger + + TOUCH_DEAD - - + + - palm + + palm @@ -1069,15 +1288,19 @@ - any finger - palm + + any finger + + palm - that finger - TOUCH_DEAD + + that finger + + TOUCH_DEAD @@ -1085,7 +1308,8 @@ - TOUCH_DEAD + + TOUCH_DEAD @@ -1093,11 +1317,13 @@ - palm + + palm - TOUCH_DEAD + + TOUCH_DEAD @@ -1107,31 +1333,39 @@ - any finger - palm + + any finger + + palm - + - that finger - TOUCH_DEAD + + that finger + + TOUCH_DEAD - either finger - palm + + either finger + + palm - that finger - TOUCH_DEAD + + that finger + + TOUCH_DEAD @@ -1139,15 +1373,17 @@ - palm + + palm - - + + - TOUCH_DEAD + + TOUCH_DEAD @@ -1155,15 +1391,19 @@ - any finger - palm + + any finger + + palm - that finger - TOUCH_DEAD + + that finger + + TOUCH_DEAD @@ -1171,78 +1411,98 @@ - - - palm + + palm - - - + - - - button 1 - press + + button 1 + + press - - + + + + + + - TOUCH_DEAD + + TOUCH_DEAD + + + + + + + + + + + + + btn1 + + release - + - btn1 - release + + MULTITAP_PALM + + + + + first + + finger down - - - - - + - MULTITAP_PALM + + TOUCH_TOUCH - - - first - finger down - - + - TOUCH_TOUCH + + timeout + + - - - timeout - - - - phys - button - press + + phys + + button + + press + + + +
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c index fadc9535..ad0a6512 100644 --- a/src/evdev-mt-touchpad-tap.c +++ b/src/evdev-mt-touchpad-tap.c @@ -312,9 +312,9 @@ tp_tap_tapped_handle_event(struct tp_dispatch *tp, LIBINPUT_BUTTON_STATE_RELEASED); break; case TAP_EVENT_THUMB: - case TAP_EVENT_PALM: log_tap_bug(tp, t, event); break; + case TAP_EVENT_PALM: case TAP_EVENT_PALM_UP: break; } diff --git a/test/test-touchpad-tap.c b/test/test-touchpad-tap.c index aa1ea0f5..16326e72 100644 --- a/test/test-touchpad-tap.c +++ b/test/test-touchpad-tap.c @@ -2649,6 +2649,48 @@ START_TEST(touchpad_tap_palm_on_tapped) } END_TEST +START_TEST(touchpad_tap_palm_on_tapped_palm_down) +{ + struct litest_device *dev = litest_current_device(); + struct libinput *li = dev->libinput; + struct axis_replacement axes[] = { + { ABS_MT_PRESSURE, 75 }, + { -1, 0 } + }; + + if (!touchpad_has_palm_pressure(dev)) + return; + + litest_enable_tap(dev->libinput_device); + litest_drain_events(li); + + /* tap + palm down */ + + litest_touch_down(dev, 0, 50, 50); + litest_touch_up(dev, 0); + libinput_dispatch(li); + + litest_assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + + litest_touch_down_extended(dev, 0, 50, 50, axes); + litest_touch_move_to_extended(dev, 0, 50, 50, 50, 50, axes, 1, 1); + litest_touch_up(dev, 0); + + libinput_dispatch(li); + litest_timeout_tap(); + libinput_dispatch(li); + + litest_assert_button_event(li, + BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + + + litest_assert_empty_queue(li); +} +END_TEST + START_TEST(touchpad_tap_palm_on_tapped_2fg) { struct litest_device *dev = litest_current_device(); @@ -3429,6 +3471,7 @@ litest_setup_tests_touchpad_tap(void) litest_add("tap:palm", touchpad_tap_palm_on_touch_hold_timeout, LITEST_TOUCHPAD, LITEST_ANY); litest_add("tap:palm", touchpad_tap_palm_on_touch_hold_move, LITEST_TOUCHPAD, LITEST_ANY); litest_add("tap:palm", touchpad_tap_palm_on_tapped, LITEST_TOUCHPAD, LITEST_ANY); + litest_add("tap:palm", touchpad_tap_palm_on_tapped_palm_down, LITEST_TOUCHPAD, LITEST_ANY); litest_add("tap:palm", touchpad_tap_palm_on_tapped_2fg, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH); litest_add("tap:palm", touchpad_tap_palm_on_drag, LITEST_TOUCHPAD, LITEST_ANY); litest_add_ranged("tap:palm", touchpad_tap_palm_on_drag_2fg, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH, &range_2fg); -- cgit v1.2.3