From d2d7b59245ca170d3fd71ac7390750b652326f5d Mon Sep 17 00:00:00 2001
From: satrmb
<10471-satrmb_true-email-is-private_contact-via-web@gitlab.freedesktop.org>
Date: Sat, 1 Aug 2020 22:59:09 +0200
Subject: [PATCH] touchpad: handle clickpad button release events in the
tapping code
This commit fixes a bug that eats one tap event after the clickpad button
has been pressed and released with no fingers lifted, turned into palm,
or palms lifted, e.g. by pressing it with a non-finger object or with an
otherwise ignored palm that stays down.
After one tap is eaten, the state machine would get un-stuck from its DEAD
state and end up in IDLE as it is supposed to, permitting subsequent taps to
have their intended effect. However, that would even happen when the
clickpad button was still held down, which is also wrong.
The correct solution is to exit to IDLE when both the button and all touches
are up. This is achieved by splitting the DEAD tapping state into one state
for the button down (BUTTON) and one for the button up (DEAD).
Working around this (in the case of a button pressed by a palm lifted
afterwards) was the only remaining justification for the palm up
event, which this commit also removes.
Signed-off-by: satrmb <10471-satrmb@users.noreply.gitlab.freedesktop.org>
---
doc/touchpad-tap-state-machine.svg | 2 +-
src/evdev-mt-touchpad-tap.c | 182 ++++++++++++++++++-----------
src/evdev-mt-touchpad.h | 3 +-
3 files changed, 118 insertions(+), 69 deletions(-)
diff --git a/doc/touchpad-tap-state-machine.svg b/doc/touchpad-tap-state-machine.svg
index e62dd83e..d4630c1f 100644
--- a/doc/touchpad-tap-state-machine.svg
+++ b/doc/touchpad-tap-state-machine.svg
@@ -1,3 +1,3 @@
-
\ No newline at end of file
+
\ No newline at end of file
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index 35a334b9..7077715b 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -37,13 +37,13 @@
enum tap_event {
TAP_EVENT_TOUCH = 12,
- TAP_EVENT_MOTION,
TAP_EVENT_RELEASE,
- TAP_EVENT_BUTTON,
+ TAP_EVENT_MOTION,
TAP_EVENT_TIMEOUT,
+ TAP_EVENT_BUTTON,
+ TAP_EVENT_BUTTON_UP,
TAP_EVENT_THUMB,
TAP_EVENT_PALM,
- TAP_EVENT_PALM_UP,
TAP_EVENT_1FGTAP, /**< valid for dragging state machine only */
TAP_EVENT_2FGTAP, /**< valid for dragging state machine only */
TAP_EVENT_3FGTAP, /**< valid for dragging state machine only */
@@ -72,6 +72,7 @@ tap_state_to_str(enum tp_tap_state state)
CASE_RETURN_STRING(TAP_STATE_TOUCH_3_HOLD);
CASE_RETURN_STRING(TAP_STATE_TOUCH_3_RELEASE);
CASE_RETURN_STRING(TAP_STATE_TOUCH_3_RELEASE_2);
+ CASE_RETURN_STRING(TAP_STATE_BUTTON);
CASE_RETURN_STRING(TAP_STATE_DEAD);
}
return NULL;
@@ -107,13 +108,13 @@ tap_event_to_str(enum tap_event event)
{
switch (event) {
CASE_RETURN_STRING(TAP_EVENT_TOUCH);
- CASE_RETURN_STRING(TAP_EVENT_MOTION);
CASE_RETURN_STRING(TAP_EVENT_RELEASE);
+ CASE_RETURN_STRING(TAP_EVENT_MOTION);
CASE_RETURN_STRING(TAP_EVENT_TIMEOUT);
CASE_RETURN_STRING(TAP_EVENT_BUTTON);
+ CASE_RETURN_STRING(TAP_EVENT_BUTTON_UP);
CASE_RETURN_STRING(TAP_EVENT_THUMB);
CASE_RETURN_STRING(TAP_EVENT_PALM);
- CASE_RETURN_STRING(TAP_EVENT_PALM_UP);
CASE_RETURN_STRING(TAP_EVENT_1FGTAP);
CASE_RETURN_STRING(TAP_EVENT_2FGTAP);
CASE_RETURN_STRING(TAP_EVENT_3FGTAP);
@@ -224,9 +225,11 @@ tp_drag_idle_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_BUTTON:
tp->tap.drag_state = DRAG_STATE_BUTTON;
break;
+ case TAP_EVENT_BUTTON_UP:
+ log_drag_bug(tp, t, event);
+ break;
case TAP_EVENT_THUMB:
case TAP_EVENT_PALM:
- case TAP_EVENT_PALM_UP:
break;
case TAP_EVENT_1FGTAP:
tp_tap_notify(tp,
@@ -321,6 +324,9 @@ tp_drag_tapped_handle_event(struct tp_dispatch *tp,
LIBINPUT_BUTTON_STATE_RELEASED);
tp->tap.drag_state = DRAG_STATE_BUTTON;
break;
+ case TAP_EVENT_BUTTON_UP:
+ log_drag_bug(tp, t, event);
+ break;
case TAP_EVENT_THUMB:
log_drag_bug(tp, t, event);
break;
@@ -328,8 +334,6 @@ tp_drag_tapped_handle_event(struct tp_dispatch *tp,
/* same as a release event; the palm may have been the last
* touch out of multiple short ones triggering a tap */
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -382,6 +386,9 @@ tp_drag_dragging_or_doubletap_handle_event(struct tp_dispatch *tp,
LIBINPUT_BUTTON_STATE_RELEASED);
tp->tap.drag_state = DRAG_STATE_BUTTON;
break;
+ case TAP_EVENT_BUTTON_UP:
+ log_drag_bug(tp, t, event);
+ break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM: {
@@ -394,8 +401,6 @@ tp_drag_dragging_or_doubletap_handle_event(struct tp_dispatch *tp,
tp->tap.drag_state = dest[nfingers_tapped - 1];
break;
}
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
tp_tap_notify(tp,
tp->tap.saved_multitap_release_time,
@@ -464,6 +469,9 @@ tp_drag_dragging_handle_event(struct tp_dispatch *tp,
LIBINPUT_BUTTON_STATE_RELEASED);
tp->tap.drag_state = DRAG_STATE_BUTTON;
break;
+ case TAP_EVENT_BUTTON_UP:
+ log_drag_bug(tp, t, event);
+ break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM:
@@ -475,8 +483,6 @@ tp_drag_dragging_handle_event(struct tp_dispatch *tp,
tp->tap.drag_state = DRAG_STATE_IDLE;
}
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -521,12 +527,13 @@ tp_drag_draglock_wait_handle_event(struct tp_dispatch *tp,
LIBINPUT_BUTTON_STATE_RELEASED);
tp->tap.drag_state = DRAG_STATE_BUTTON;
break;
+ case TAP_EVENT_BUTTON_UP:
+ log_drag_bug(tp, t, event);
+ break;
case TAP_EVENT_THUMB:
case TAP_EVENT_PALM:
log_drag_bug(tp, t, event);
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -584,6 +591,9 @@ tp_drag_draglock_continue_handle_event(struct tp_dispatch *tp,
LIBINPUT_BUTTON_STATE_RELEASED);
tp->tap.drag_state = DRAG_STATE_BUTTON;
break;
+ case TAP_EVENT_BUTTON_UP:
+ log_drag_bug(tp, t, event);
+ break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM: {
@@ -596,8 +606,6 @@ tp_drag_draglock_continue_handle_event(struct tp_dispatch *tp,
tp->tap.drag_state = dest[nfingers_tapped - 1];
break;
}
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
tp_tap_notify(tp,
time,
@@ -620,9 +628,6 @@ tp_drag_button_handle_event(struct tp_dispatch *tp,
switch (event) {
case TAP_EVENT_RELEASE:
- if (tp->tap.nfingers_down == 0)
- tp->tap.drag_state = DRAG_STATE_IDLE;
- break;
case TAP_EVENT_TOUCH:
case TAP_EVENT_MOTION:
case TAP_EVENT_TIMEOUT:
@@ -630,12 +635,11 @@ tp_drag_button_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_BUTTON:
log_drag_bug(tp, t, event);
break;
- case TAP_EVENT_THUMB:
+ case TAP_EVENT_BUTTON_UP:
+ tp->tap.drag_state = DRAG_STATE_IDLE;
break;
+ case TAP_EVENT_THUMB:
case TAP_EVENT_PALM:
- case TAP_EVENT_PALM_UP:
- if (tp->tap.nfingers_down == 0)
- tp->tap.drag_state = DRAG_STATE_IDLE;
break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
@@ -744,7 +748,10 @@ tp_tap_idle_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_TIMEOUT:
break;
case TAP_EVENT_BUTTON:
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
log_tap_bug(tp, t, event);
@@ -752,8 +759,6 @@ tp_tap_idle_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_PALM:
tp->tap.state = TAP_STATE_IDLE;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -791,7 +796,10 @@ tp_tap_touch_handle_event(struct tp_dispatch *tp,
tp_gesture_tap_timeout(tp, time);
break;
case TAP_EVENT_BUTTON:
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
if (tp->tap.drag_state == DRAG_STATE_IDLE) {
@@ -805,8 +813,6 @@ tp_tap_touch_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_PALM:
tp->tap.state = TAP_STATE_IDLE;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -836,7 +842,10 @@ tp_tap_hold_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_TIMEOUT:
break;
case TAP_EVENT_BUTTON:
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
tp->tap.state = TAP_STATE_IDLE;
@@ -847,8 +856,6 @@ tp_tap_hold_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_PALM:
tp->tap.state = TAP_STATE_IDLE;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -885,15 +892,16 @@ tp_tap_touch2_handle_event(struct tp_dispatch *tp,
tp_gesture_tap_timeout(tp, time);
break;
case TAP_EVENT_BUTTON:
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM:
tp->tap.state = TAP_STATE_TOUCH;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -923,15 +931,16 @@ tp_tap_touch2_hold_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_TIMEOUT:
break;
case TAP_EVENT_BUTTON:
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM:
tp->tap.state = TAP_STATE_HOLD;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -974,7 +983,10 @@ tp_tap_touch2_release_handle_event(struct tp_dispatch *tp,
tp->tap.state = TAP_STATE_DEAD;
break;
case TAP_EVENT_BUTTON:
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
break;
@@ -989,8 +1001,6 @@ tp_tap_touch2_release_handle_event(struct tp_dispatch *tp,
tp_drag_handle_event(tp, t, TAP_EVENT_1FGTAP, time);
tp->tap.state = TAP_STATE_IDLE;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -1027,15 +1037,16 @@ tp_tap_touch3_handle_event(struct tp_dispatch *tp,
tp_tap_set_timer(tp, time);
break;
case TAP_EVENT_BUTTON:
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM:
tp->tap.state = TAP_STATE_TOUCH_2;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -1063,15 +1074,16 @@ tp_tap_touch3_hold_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_TIMEOUT:
break;
case TAP_EVENT_BUTTON:
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM:
tp->tap.state = TAP_STATE_TOUCH_2_HOLD;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -1107,15 +1119,16 @@ tp_tap_touch3_release_handle_event(struct tp_dispatch *tp,
break;
case TAP_EVENT_BUTTON:
tp_drag_handle_event(tp, t, TAP_EVENT_3FGTAP, time);
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM:
tp->tap.state = TAP_STATE_TOUCH_2_RELEASE;
break;
- case TAP_EVENT_PALM_UP:
- break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
case TAP_EVENT_3FGTAP:
@@ -1151,7 +1164,10 @@ tp_tap_touch3_release2_handle_event(struct tp_dispatch *tp,
break;
case TAP_EVENT_BUTTON:
tp_drag_handle_event(tp, t, TAP_EVENT_3FGTAP, time);
- tp->tap.state = TAP_STATE_DEAD;
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
break;
@@ -1159,7 +1175,38 @@ tp_tap_touch3_release2_handle_event(struct tp_dispatch *tp,
tp_drag_handle_event(tp, t, TAP_EVENT_2FGTAP, time);
tp->tap.state = TAP_STATE_IDLE;
break;
- case TAP_EVENT_PALM_UP:
+ case TAP_EVENT_1FGTAP:
+ case TAP_EVENT_2FGTAP:
+ case TAP_EVENT_3FGTAP:
+ log_tap_bug(tp, t, event);
+ break;
+ }
+}
+
+static void
+tp_tap_button_handle_event(struct tp_dispatch *tp,
+ struct tp_touch *t,
+ enum tap_event event,
+ uint64_t time)
+{
+
+ switch (event) {
+ case TAP_EVENT_TOUCH:
+ case TAP_EVENT_RELEASE:
+ case TAP_EVENT_MOTION:
+ case TAP_EVENT_TIMEOUT:
+ break;
+ case TAP_EVENT_BUTTON:
+ log_tap_bug(tp, t, event);
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ if (tp->tap.nfingers_down == 0)
+ tp->tap.state = TAP_STATE_IDLE;
+ else
+ tp->tap.state = TAP_STATE_DEAD;
+ break;
+ case TAP_EVENT_THUMB:
+ case TAP_EVENT_PALM:
break;
case TAP_EVENT_1FGTAP:
case TAP_EVENT_2FGTAP:
@@ -1184,12 +1231,16 @@ tp_tap_dead_handle_event(struct tp_dispatch *tp,
case TAP_EVENT_TOUCH:
case TAP_EVENT_MOTION:
case TAP_EVENT_TIMEOUT:
+ break;
case TAP_EVENT_BUTTON:
+ tp->tap.state = TAP_STATE_BUTTON;
+ break;
+ case TAP_EVENT_BUTTON_UP:
+ log_tap_bug(tp, t, event);
break;
case TAP_EVENT_THUMB:
break;
case TAP_EVENT_PALM:
- case TAP_EVENT_PALM_UP:
if (tp->tap.nfingers_down == 0)
tp->tap.state = TAP_STATE_IDLE;
break;
@@ -1242,6 +1293,9 @@ tp_tap_handle_event(struct tp_dispatch *tp,
case TAP_STATE_TOUCH_3_RELEASE_2:
tp_tap_touch3_release2_handle_event(tp, t, event, time);
break;
+ case TAP_STATE_BUTTON:
+ tp_tap_button_handle_event(tp, t, event, time);
+ break;
case TAP_STATE_DEAD:
tp_tap_dead_handle_event(tp, t, event, time);
break;
@@ -1265,6 +1319,7 @@ tp_tap_handle_event(struct tp_dispatch *tp,
tp->tap.state == TAP_STATE_HOLD ||
tp->tap.state == TAP_STATE_TOUCH_2_HOLD ||
tp->tap.state == TAP_STATE_TOUCH_3_HOLD ||
+ tp->tap.state == TAP_STATE_BUTTON ||
tp->tap.state == TAP_STATE_DEAD) &&
(tp->tap.drag_state == DRAG_STATE_IDLE ||
tp->tap.drag_state == DRAG_STATE_1FGTAP_DRAGGING ||
@@ -1320,11 +1375,13 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
if (!tp_tap_enabled(tp))
return 0;
- /* Handle queued button pressed events from clickpads. For touchpads
- * with separate physical buttons, ignore button pressed events so they
- * don't interfere with tapping. */
+ /* Handle queued button events from clickpads. For touchpads with
+ * separate physical buttons, ignore button pressed events so they don't
+ * interfere with tapping. */
if (tp->buttons.is_clickpad && tp->queued & TOUCHPAD_EVENT_BUTTON_PRESS)
tp_tap_handle_event(tp, NULL, TAP_EVENT_BUTTON, time);
+ if (tp->buttons.is_clickpad && tp->queued & TOUCHPAD_EVENT_BUTTON_RELEASE)
+ tp_tap_handle_event(tp, NULL, TAP_EVENT_BUTTON_UP, time);
tp_for_each_touch(tp, t) {
if (!t->dirty || t->state == TOUCH_NONE)
@@ -1339,18 +1396,9 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
if (t->tap.is_thumb)
continue;
- /* A palm tap needs to be properly released because we might
- * be who-knows-where in the state machine. Otherwise, we
- * ignore any event from it.
- */
- if (t->tap.is_palm) {
- if (t->state == TOUCH_END)
- tp_tap_handle_event(tp,
- t,
- TAP_EVENT_PALM_UP,
- time);
+ /* we ignore any event from a palm */
+ if (t->tap.is_palm)
continue;
- }
if (t->state == TOUCH_HOVERING)
continue;
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 991e3a87..d9d75867 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -113,7 +113,8 @@ enum tp_tap_state {
TAP_STATE_TOUCH_3_HOLD,
TAP_STATE_TOUCH_3_RELEASE,
TAP_STATE_TOUCH_3_RELEASE_2,
- TAP_STATE_DEAD, /**< finger count exceeded */
+ TAP_STATE_BUTTON, /**< clickpad button pressed */
+ TAP_STATE_DEAD, /**< clickpad button released, or finger moved */
};
enum tp_drag_state {