[PATCH libinput] touchpad: count the tapping fingers separately from the main touchpad code
Jason Gerecke
killertofu at gmail.com
Wed Apr 8 10:55:24 PDT 2015
On 4/7/2015 8:54 PM, Peter Hutterer wrote:
> tp->nfingers_down gives us the current state of the touchpad but in the case
> of the tapping state we need the touchpoints separately. If all touchpoints
> end in the same SYN_REPORT frame, tp->nfingers_down is 0 when we handle the
> touch releases. This changes the tap state to IDLE on the first release and
> then logs a bug when the remaining touches are released while the touchpad is
> in IDLE.
>
> Avoid this by counting the fingers separately for the tap state, this way we
> can count up/down with the down/up events as we process them for the tapping
> state machine.
>
> This also adds tests for 4 and 5-finger tapping which is how the bug was
> discovered in the first place.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=89800
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
The summary doesn't seem to jive with what I see. In particular, the
scenario it describes doesn't seem to apply: I can have four touches go
up across two frames and see the error, or three touches all disappear
in a single frame and not see it. Additionally, it doesn't seem like
"tp->nfingers_down gives us the current state of the touchpad" since the
value is reset to 0 if five or more fingers are physically present (due
to the behavior of 'tp_unhover_touches' mentioned in the bug report).
That said, the patch *does* prevent the error messages from appearing,
so at the very least:
Tested-by: Jason Gerecke <jason.gerecke at wacom.com>
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
> ---
> src/evdev-mt-touchpad-tap.c | 5 +-
> src/evdev-mt-touchpad.h | 1 +
> test/touchpad.c | 233 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 238 insertions(+), 1 deletion(-)
>
> diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
> index 4ba4ad2..78d4a0f 100644
> --- a/src/evdev-mt-touchpad-tap.c
> +++ b/src/evdev-mt-touchpad-tap.c
> @@ -452,7 +452,7 @@ tp_tap_dead_handle_event(struct tp_dispatch *tp,
>
> switch (event) {
> case TAP_EVENT_RELEASE:
> - if (tp->nfingers_down == 0)
> + if (tp->tap.tap_finger_count == 0)
> tp->tap.state = TAP_STATE_IDLE;
> break;
> case TAP_EVENT_TOUCH:
> @@ -567,10 +567,12 @@ tp_tap_handle_state(struct tp_dispatch *tp, uint64_t time)
> t->tap.state = TAP_TOUCH_STATE_DEAD;
>
> if (t->state == TOUCH_BEGIN) {
> + tp->tap.tap_finger_count++;
> t->tap.state = TAP_TOUCH_STATE_TOUCH;
> t->tap.initial = t->point;
> tp_tap_handle_event(tp, t, TAP_EVENT_TOUCH, time);
> } else if (t->state == TOUCH_END) {
> + tp->tap.tap_finger_count--;
> tp_tap_handle_event(tp, t, TAP_EVENT_RELEASE, time);
> t->tap.state = TAP_TOUCH_STATE_IDLE;
> } else if (tp->tap.state != TAP_STATE_IDLE &&
> @@ -754,6 +756,7 @@ tp_release_all_taps(struct tp_dispatch *tp, uint64_t now)
> }
>
> tp->tap.state = tp->nfingers_down ? TAP_STATE_DEAD : TAP_STATE_IDLE;
> + tp->tap.tap_finger_count = 0;
> }
>
> void
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index 19a262e..b88dadd 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -255,6 +255,7 @@ struct tp_dispatch {
> struct libinput_timer timer;
> enum tp_tap_state state;
> uint32_t buttons_pressed;
> + unsigned int tap_finger_count;
> } tap;
>
> struct {
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 6fa2301..b06e00d 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -369,6 +369,38 @@ START_TEST(touchpad_2fg_tap_inverted)
> }
> END_TEST
>
> +START_TEST(touchpad_2fg_tap_quickrelease)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> +
> + libinput_device_config_tap_set_enabled(dev->libinput_device,
> + LIBINPUT_CONFIG_TAP_ENABLED);
> +
> + litest_drain_events(dev->libinput);
> +
> + litest_touch_down(dev, 0, 50, 50);
> + litest_touch_down(dev, 1, 70, 70);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 0);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 1);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, 0);
> + litest_event(dev, EV_KEY, BTN_TOUCH, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +
> + libinput_dispatch(li);
> +
> + litest_assert_button_event(li, BTN_RIGHT,
> + LIBINPUT_BUTTON_STATE_PRESSED);
> + litest_timeout_tap();
> + litest_assert_button_event(li, BTN_RIGHT,
> + LIBINPUT_BUTTON_STATE_RELEASED);
> +
> + litest_assert_empty_queue(li);
> +}
> +END_TEST
> +
> START_TEST(touchpad_1fg_tap_click)
> {
> struct litest_device *dev = litest_current_device();
> @@ -705,6 +737,46 @@ START_TEST(touchpad_3fg_tap)
> }
> END_TEST
>
> +START_TEST(touchpad_3fg_tap_quickrelease)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> +
> + if (libevdev_get_abs_maximum(dev->evdev,
> + ABS_MT_SLOT) <= 2)
> + return;
> +
> + libinput_device_config_tap_set_enabled(dev->libinput_device,
> + LIBINPUT_CONFIG_TAP_ENABLED);
> +
> + litest_drain_events(li);
> +
> + litest_touch_down(dev, 0, 50, 50);
> + litest_touch_down(dev, 1, 70, 50);
> + litest_touch_down(dev, 2, 80, 50);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 0);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 1);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 2);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, 0);
> + litest_event(dev, EV_KEY, BTN_TOUCH, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +
> + libinput_dispatch(li);
> +
> + litest_assert_button_event(li, BTN_MIDDLE,
> + LIBINPUT_BUTTON_STATE_PRESSED);
> + litest_timeout_tap();
> + litest_assert_button_event(li, BTN_MIDDLE,
> + LIBINPUT_BUTTON_STATE_RELEASED);
> +
> + libinput_dispatch(li);
> + litest_assert_empty_queue(li);
> +}
> +END_TEST
> +
> START_TEST(touchpad_3fg_tap_btntool)
> {
> struct litest_device *dev = litest_current_device();
> @@ -783,6 +855,161 @@ START_TEST(touchpad_3fg_tap_btntool_inverted)
> }
> END_TEST
>
> +START_TEST(touchpad_4fg_tap)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> + struct libinput_event *event;
> + int i;
> +
> + if (libevdev_get_abs_maximum(dev->evdev,
> + ABS_MT_SLOT) <= 3)
> + return;
> +
> + libinput_device_config_tap_set_enabled(dev->libinput_device,
> + LIBINPUT_CONFIG_TAP_ENABLED);
> +
> + for (i = 0; i < 4; i++) {
> + litest_drain_events(li);
> +
> + litest_touch_down(dev, 0, 50, 50);
> + litest_touch_down(dev, 1, 70, 50);
> + litest_touch_down(dev, 2, 80, 50);
> + litest_touch_down(dev, 3, 90, 50);
> +
> + litest_touch_up(dev, (i + 3) % 4);
> + litest_touch_up(dev, (i + 2) % 4);
> + litest_touch_up(dev, (i + 1) % 4);
> + litest_touch_up(dev, (i + 0) % 4);
> +
> + libinput_dispatch(li);
> + litest_assert_empty_queue(li);
> + litest_timeout_tap();
> + litest_assert_empty_queue(li);
> + event = libinput_get_event(li);
> + ck_assert(event == NULL);
> + }
> +}
> +END_TEST
> +
> +START_TEST(touchpad_4fg_tap_quickrelease)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> +
> + if (libevdev_get_abs_maximum(dev->evdev,
> + ABS_MT_SLOT) <= 3)
> + return;
> +
> + libinput_device_config_tap_set_enabled(dev->libinput_device,
> + LIBINPUT_CONFIG_TAP_ENABLED);
> +
> + litest_drain_events(li);
> +
> + litest_touch_down(dev, 0, 50, 50);
> + litest_touch_down(dev, 1, 70, 50);
> + litest_touch_down(dev, 2, 80, 50);
> + litest_touch_down(dev, 3, 90, 50);
> +
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 0);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 1);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 2);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 3);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_KEY, BTN_TOOL_QUADTAP, 0);
> + litest_event(dev, EV_KEY, BTN_TOUCH, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +
> + libinput_dispatch(li);
> + litest_assert_empty_queue(li);
> + litest_timeout_tap();
> + litest_assert_empty_queue(li);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_5fg_tap)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> + struct libinput_event *event;
> + int i;
> +
> + if (libevdev_get_abs_maximum(dev->evdev,
> + ABS_MT_SLOT) <= 4)
> + return;
> +
> + libinput_device_config_tap_set_enabled(dev->libinput_device,
> + LIBINPUT_CONFIG_TAP_ENABLED);
> +
> + for (i = 0; i < 5; i++) {
> + litest_drain_events(li);
> +
> + litest_touch_down(dev, 0, 20, 50);
> + litest_touch_down(dev, 1, 30, 50);
> + litest_touch_down(dev, 2, 40, 50);
> + litest_touch_down(dev, 3, 50, 50);
> + litest_touch_down(dev, 4, 60, 50);
> +
> + litest_touch_up(dev, (i + 4) % 5);
> + litest_touch_up(dev, (i + 3) % 5);
> + litest_touch_up(dev, (i + 2) % 5);
> + litest_touch_up(dev, (i + 1) % 5);
> + litest_touch_up(dev, (i + 0) % 5);
> +
> + libinput_dispatch(li);
> + litest_assert_empty_queue(li);
> + litest_timeout_tap();
> + litest_assert_empty_queue(li);
> + event = libinput_get_event(li);
> + ck_assert(event == NULL);
> + }
> +}
> +END_TEST
> +
> +START_TEST(touchpad_5fg_tap_quickrelease)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> +
> + if (libevdev_get_abs_maximum(dev->evdev,
> + ABS_MT_SLOT) <= 4)
> + return;
> +
> + libinput_device_config_tap_set_enabled(dev->libinput_device,
> + LIBINPUT_CONFIG_TAP_ENABLED);
> +
> + litest_drain_events(li);
> +
> + litest_touch_down(dev, 0, 20, 50);
> + litest_touch_down(dev, 1, 30, 50);
> + litest_touch_down(dev, 2, 40, 50);
> + litest_touch_down(dev, 3, 70, 50);
> + litest_touch_down(dev, 4, 90, 50);
> +
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 0);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 1);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 2);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 3);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_ABS, ABS_MT_SLOT, 4);
> + litest_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
> + litest_event(dev, EV_KEY, BTN_TOOL_QUINTTAP, 0);
> + litest_event(dev, EV_KEY, BTN_TOUCH, 0);
> + litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +
> + libinput_dispatch(li);
> + litest_assert_empty_queue(li);
> + litest_timeout_tap();
> + litest_assert_empty_queue(li);
> +}
> +END_TEST
> +
> START_TEST(touchpad_click_defaults_clickfinger)
> {
> struct litest_device *dev = litest_current_device();
> @@ -3370,6 +3597,7 @@ int main(int argc, char **argv) {
> litest_add("touchpad:tap", touchpad_2fg_tap_n_drag_3fg, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
> litest_add("touchpad:tap", touchpad_2fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
> litest_add("touchpad:tap", touchpad_2fg_tap_inverted, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
> + litest_add("touchpad:tap", touchpad_2fg_tap_quickrelease, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
> litest_add("touchpad:tap", touchpad_1fg_tap_click, LITEST_TOUCHPAD|LITEST_BUTTON, LITEST_CLICKPAD);
> litest_add("touchpad:tap", touchpad_2fg_tap_click, LITEST_TOUCHPAD|LITEST_BUTTON, LITEST_SINGLE_TOUCH|LITEST_CLICKPAD);
>
> @@ -3381,6 +3609,11 @@ int main(int argc, char **argv) {
> litest_add("touchpad:tap", touchpad_3fg_tap_btntool, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
> litest_add("touchpad:tap", touchpad_3fg_tap_btntool_inverted, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
> litest_add("touchpad:tap", touchpad_3fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
> + litest_add("touchpad:tap", touchpad_3fg_tap_quickrelease, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
> + litest_add("touchpad:tap", touchpad_4fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
> + litest_add("touchpad:tap", touchpad_4fg_tap_quickrelease, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
> + litest_add("touchpad:tap", touchpad_5fg_tap, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
> + litest_add("touchpad:tap", touchpad_5fg_tap_quickrelease, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH|LITEST_SEMI_MT);
>
> /* Real buttons don't interfere with tapping, so don't run those for
> pads with buttons */
>
More information about the wayland-devel
mailing list