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>

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
+	/* Handle queued button events from clickpads. For touchpads with
-	 * with separate physical buttons, ignore button pressed events so they
+	 * separate physical buttons, ignore button pressed events so they don't
-	 * don't interfere with tapping. */
+	 * 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
+		/* we ignore any event from a palm */
-		 * be who-knows-where in the state machine. Otherwise, we
+		if (t->tap.is_palm)
-		 * 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);
 			continue;
-		}
 
 		if (t->state == TOUCH_HOVERING)
 			continue;

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 {