touchpad: correct the tap state transitions for a palm on TOUCH_BEGIN
authorPeter Hutterer <peter.hutterer@who-t.net>
Thu, 28 May 2020 00:58:06 +0000 (10:58 +1000)
committerPeter Hutterer <peter.hutterer@who-t.net>
Wed, 3 Jun 2020 21:50:44 +0000 (21:50 +0000)
Where a touch is labelled as palm on TOUCH_BEGIN (edge palms) we would still
feed the touch through the tap state machine. This would trigger the PALM
transition for each state, usually reducing the touch count.

When the touches were later released, the touch count was out of sync,
resulting in an error message. In the case of #488, the trigger was
a single evdev frame with three fingers down, the third of which was an edge
palm:
- touch 1 transitions from IDLE to TOUCH
- touch 2 transitions from TOUCH to TOUCH_2
- touch 3 (the palm) transitioned from TOUCH_2 back to TOUCH

That third transition is invalid, the palm hasn't been seen by the tap state
machine so it should just be ignored.

Fix this by moving making the tap state processing conditional on a touch
state other than TOUCH_BEGIN.

Fixes #488

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
src/evdev-mt-touchpad-tap.c
test/test-touchpad-tap.c

index d5d4a79d9ffee68df971bc5494be3c04dcf036ca..f5cf73149e7eb1ad2f00b704d0a9bb10e4b11b3f 100644 (file)
@@ -894,10 +894,10 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
 
                if (t->palm.state != PALM_NONE) {
                        assert(!t->tap.is_palm);
-                       tp_tap_handle_event(tp, t, TAP_EVENT_PALM, time);
                        t->tap.is_palm = true;
                        t->tap.state = TAP_TOUCH_STATE_DEAD;
                        if (t->state != TOUCH_BEGIN) {
+                               tp_tap_handle_event(tp, t, TAP_EVENT_PALM, time);
                                assert(tp->tap.nfingers_down > 0);
                                tp->tap.nfingers_down--;
                        }
index 3d131a9b7874b73039e31d4fc8175f1996f3ae46..74e03ccb634f9354947f68c300c06826892a031d 100644 (file)
@@ -3654,6 +3654,40 @@ START_TEST(touchpad_tap_palm_dwt_tap)
 }
 END_TEST
 
+START_TEST(touchpad_tap_palm_3fg_start)
+{
+       struct litest_device *dev = litest_current_device();
+       struct libinput *li = dev->libinput;
+
+       if (litest_slot_count(dev) < 3 ||
+           !litest_has_palm_detect_size(dev))
+               return;
+
+       litest_enable_tap(dev->libinput_device);
+       litest_drain_events(li);
+
+       litest_push_event_frame(dev);
+       litest_touch_down(dev, 0, 50, 50);
+       litest_touch_down(dev, 1, 55, 55);
+       litest_touch_down(dev, 2, 99, 55); /* edge palm */
+       litest_pop_event_frame(dev);
+       libinput_dispatch(li);
+
+       litest_touch_up(dev, 2); /* release the palm */
+       litest_assert_empty_queue(li);
+
+       litest_touch_up(dev, 0);
+       litest_touch_up(dev, 1);
+
+       libinput_dispatch(li);
+       litest_assert_button_event(li, BTN_RIGHT,
+                                  LIBINPUT_BUTTON_STATE_PRESSED);
+       litest_assert_button_event(li, BTN_RIGHT,
+                                  LIBINPUT_BUTTON_STATE_RELEASED);
+       litest_assert_empty_queue(li);
+}
+END_TEST
+
 TEST_COLLECTION(touchpad_tap)
 {
        struct range any_tap_range = {1, 4};
@@ -3769,4 +3803,5 @@ TEST_COLLECTION(touchpad_tap)
        litest_add_ranged("tap:palm", touchpad_tap_palm_multitap_click, LITEST_TOUCHPAD, LITEST_ANY, &multitap_range);
        litest_add("tap:palm", touchpad_tap_palm_click_then_tap, LITEST_TOUCHPAD, LITEST_ANY);
        litest_add("tap:palm", touchpad_tap_palm_dwt_tap, LITEST_TOUCHPAD, LITEST_ANY);
+       litest_add("tap:palm", touchpad_tap_palm_3fg_start, LITEST_TOUCHPAD, LITEST_ANY);
 }