touchpad: correct the tap state transitions for a palm on TOUCH_BEGIN

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>

diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index d5d4a79..f5cf731 100644 (file)
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -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--;
                        }

diff --git a/test/test-touchpad-tap.c b/test/test-touchpad-tap.c
index 3d131a9..74e03cc 100644 (file)
--- a/test/test-touchpad-tap.c
+++ b/test/test-touchpad-tap.c
@@ -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);
 }