Re: [PATCH libinput] touchpad: count the tapping fingers separately from the main touchpad code
Hi,
On 08-04-15 05:54, 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.hutte...@who-t.net
Looks good, I've added my Rev-by and pushed this.
Regards,
Hans
---
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);
+
Re: [PATCH libinput] touchpad: count the tapping fingers separately from the main touchpad code
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.hutte...@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.gere...@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,
+
[PATCH libinput] touchpad: count the tapping fingers separately from the main touchpad code
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.hutte...@who-t.net
---
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);
+
