[PATCH libevdev] Support direct ABS_MT_TRACKING_ID changes
Peter Hutterer
peter.hutterer at who-t.net
Mon Apr 16 05:07:03 UTC 2018
Due to some decade-old misunderstanding of how tracking IDs can change in the
kernel, libevdev suppresses direct ABS_MT_TRACKING_ID changes. So the
transition that is ok is M → -1 → N, but a direct M → N change would be
suppressed. The cause is that most clients (at least libinput, evdev,
synaptics) have code like this:
if tracking_id == -1
end_touch()
else
begin_new_touch()
So a direct tracking ID change would cause new touches without terminating the
old touches.
To provide a gentle switch add a new flag for libevdev_next_event() to allow
for direct tracking ID changes. When provided, libevdev doesn't filter those
direct changes and clients have to be able to cope with them directly.
This does not apply to a duplicate -1 ABS_MT_TRACKING_ID but that would be a
real kernel bug.
Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
libevdev/libevdev.c | 16 +++--
libevdev/libevdev.h | 76 +++++++++++++++++++-
test/test-libevdev-events.c | 166 +++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 251 insertions(+), 7 deletions(-)
diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
index c6ec114..903abba 100644
--- a/libevdev/libevdev.c
+++ b/libevdev/libevdev.c
@@ -979,6 +979,7 @@ update_state(struct libevdev *dev, const struct input_event *e)
*/
static inline enum event_filter_status
sanitize_event(const struct libevdev *dev,
+ bool allow_tracking_id_change,
struct input_event *ev,
enum SyncState sync_state)
{
@@ -1004,7 +1005,8 @@ sanitize_event(const struct libevdev *dev,
((ev->value == -1 &&
*slot_value(dev, dev->current_slot, ABS_MT_TRACKING_ID) == -1) ||
(ev->value != -1 &&
- *slot_value(dev, dev->current_slot, ABS_MT_TRACKING_ID) != -1)))) {
+ *slot_value(dev, dev->current_slot, ABS_MT_TRACKING_ID) != -1 &&
+ !allow_tracking_id_change)))) {
log_bug(dev, "Device \"%s\" received a double tracking ID %d in slot %d.\n",
dev->name, ev->value, dev->current_slot);
return EVENT_FILTER_DISCARD;
@@ -1021,7 +1023,9 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
const unsigned int valid_flags = LIBEVDEV_READ_FLAG_NORMAL |
LIBEVDEV_READ_FLAG_SYNC |
LIBEVDEV_READ_FLAG_FORCE_SYNC |
- LIBEVDEV_READ_FLAG_BLOCKING;
+ LIBEVDEV_READ_FLAG_BLOCKING |
+ LIBEVDEV_READ_FLAG_ALLOW_TRACKING_ID_SWITCH;
+ bool allow_tracking_id_change = !!(flags & LIBEVDEV_READ_FLAG_ALLOW_TRACKING_ID_SWITCH);
if (!dev->initialized) {
log_bug(dev, "device not initialized. call libevdev_set_fd() first\n");
@@ -1054,7 +1058,8 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
of the device too */
while (queue_shift(dev, &e) == 0) {
dev->queue_nsync--;
- if (sanitize_event(dev, &e, dev->sync_state) != EVENT_FILTER_DISCARD)
+ if (sanitize_event(dev, allow_tracking_id_change,
+ &e, dev->sync_state) != EVENT_FILTER_DISCARD)
update_state(dev, &e);
}
@@ -1084,7 +1089,8 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
if (queue_shift(dev, ev) != 0)
return -EAGAIN;
- filter_status = sanitize_event(dev, ev, dev->sync_state);
+ filter_status = sanitize_event(dev, allow_tracking_id_change,
+ ev, dev->sync_state);
if (filter_status != EVENT_FILTER_DISCARD)
update_state(dev, ev);
@@ -1283,7 +1289,7 @@ libevdev_set_event_value(struct libevdev *dev, unsigned int type, unsigned int c
e.code = code;
e.value = value;
- if (sanitize_event(dev, &e, SYNC_NONE) != EVENT_FILTER_NONE)
+ if (sanitize_event(dev, false, &e, SYNC_NONE) != EVENT_FILTER_NONE)
return -1;
switch(type) {
diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h
index b36d3b4..4863989 100644
--- a/libevdev/libevdev.h
+++ b/libevdev/libevdev.h
@@ -428,6 +428,74 @@ extern "C" {
* @endcode
*/
+/**
+ * @page tracking_id_changes ABS_MT_TRACKING_ID changes
+ *
+ * By default, libevdev expects an `ABS_MT_TRACKING_ID` of -1 before the start
+ * of a new touchpoint.
+ *
+ * An event sequence may look like this:
+ *
+ * @code
+ * EV_ABS ABS_MT_SLOT 0
+ * EV_ABS ABS_MT_TRACKING_ID 45
+ * EV_ABS ABS_MT_POSITION_X 100
+ * EV_ABS ABS_MT_POSITION_Y 80
+ * EV_SYN SYN_REPORT 0
+ * ------------------------
+ * EV_ABS ABS_MT_TRACKING_ID -1
+ * EV_SYN SYN_REPORT 0
+ * ------------------------
+ * EV_ABS ABS_MT_TRACKING_ID 46
+ * EV_ABS ABS_MT_POSITION_X 200
+ * EV_ABS ABS_MT_POSITION_Y 80
+ * EV_SYN SYN_REPORT 0
+ * ------------------------
+ * @endcode
+ *
+ * However, in some cases, the kernel may change the tracking ID directly,
+ * without changing it to -1 first. An event sequence may look like
+ * this:
+ *
+ * @code
+ * EV_ABS ABS_MT_TRACKING_ID 30
+ * EV_ABS ABS_MT_POSITION_X 100
+ * EV_ABS ABS_MT_POSITION_Y 80
+ * EV_SYN SYN_REPORT 0
+ * ------------------------
+ * EV_ABS ABS_MT_TRACKING_ID 31
+ * EV_ABS ABS_MT_POSITION_X 200
+ * EV_ABS ABS_MT_POSITION_Y 80
+ * EV_SYN SYN_REPORT 0
+ * ------------------------
+ * @endcode
+ *
+ * Direct tracking ID changes are mainly caused when the kernel lost track
+ * of the touch point and cannot guarantee that the new touch point is the
+ * same as the one before. In some cases, it cannot be determined whether a
+ * finger moved a large distance quickly, or whether a finger was relased
+ * and a new finger set down within the same hardware frame.
+ *
+ * Unfortunately, direct tracking ID changes require client support and
+ * until April 2018, none of the most widely distributed libevdev clients
+ * expected direct tracking ID changes (libinput, xf86-input-evdev,
+ * xf86-input-synaptics). libevdev 1.5.x and earlier always filtered out
+ * these changes, effectively suppressing the `ABS_MT_TRACKING_ID` event
+ * unless it was preceeded by one with value -1.
+ *
+ * The message logged for the example above would be:
+ * <blockquote >
+ * Device "foo" received a double tracking ID 31 in slot 0.
+ * </blockquote>
+ *
+ * As of libevdev 1.6.0, the @ref
+ * LIBEVDEV_READ_FLAG_ALLOW_TRACKING_ID_SWITCH flag may be provided to
+ * libevdev_next_event(). When present, libevdev will **not** supress direct
+ * tracking ID changes and forward them to the client. It is the client's
+ * responsibility to keep track of direct changes vs touch changes
+ * terminated with a tracking ID of -1.
+ */
+
/**
* @page backwardscompatibility Compatibility and Behavior across kernel versions
*
@@ -756,7 +824,13 @@ enum libevdev_read_flag {
LIBEVDEV_READ_FLAG_NORMAL = 2, /**< Process data in normal mode */
LIBEVDEV_READ_FLAG_FORCE_SYNC = 4, /**< Pretend the next event is a SYN_DROPPED and
require the caller to sync */
- LIBEVDEV_READ_FLAG_BLOCKING = 8 /**< The fd is not in O_NONBLOCK and a read may block */
+ LIBEVDEV_READ_FLAG_BLOCKING = 8, /**< The fd is not in O_NONBLOCK and a read may block */
+ /**
+ * Allow for direct tracking ID changes, see @ref tracking_id_changes.
+ * All newly written clients should support this behavior and
+ * provide this flag.
+ */
+ LIBEVDEV_READ_FLAG_ALLOW_TRACKING_ID_SWITCH = 16
};
/**
diff --git a/test/test-libevdev-events.c b/test/test-libevdev-events.c
index dd2face..19602ec 100644
--- a/test/test-libevdev-events.c
+++ b/test/test-libevdev-events.c
@@ -81,7 +81,7 @@ START_TEST(test_next_event_invalid_fd)
-1);
/* invalid (missing) flag */
- rc = libevdev_next_event(dev, 0x10, &ev);
+ rc = libevdev_next_event(dev, 0x20, &ev);
ck_assert_int_eq(rc, -EINVAL);
/* set an invalid fd */
@@ -1882,6 +1882,168 @@ START_TEST(test_mt_tracking_id_discard_neg_1)
}
END_TEST
+START_TEST(test_mt_tracking_id_keep)
+{
+ struct uinput_device* uidev;
+ struct libevdev *dev;
+ int rc;
+ struct input_event ev;
+ struct input_absinfo abs[6];
+ unsigned int flags = LIBEVDEV_READ_FLAG_NORMAL |
+ LIBEVDEV_READ_FLAG_ALLOW_TRACKING_ID_SWITCH;
+
+ memset(abs, 0, sizeof(abs));
+ abs[0].value = ABS_X;
+ abs[0].maximum = 1000;
+ abs[1].value = ABS_MT_POSITION_X;
+ abs[1].maximum = 1000;
+
+ abs[2].value = ABS_Y;
+ abs[2].maximum = 1000;
+ abs[3].value = ABS_MT_POSITION_Y;
+ abs[3].maximum = 1000;
+
+ abs[4].value = ABS_MT_SLOT;
+ abs[4].maximum = 10;
+ abs[5].value = ABS_MT_TRACKING_ID;
+ abs[5].maximum = 500;
+
+ test_create_abs_device(&uidev, &dev,
+ 6, abs,
+ EV_SYN, SYN_REPORT,
+ -1);
+
+ uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 1);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 1);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+ /* second tracking ID on same slot */
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 2);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+ ck_assert_int_eq(ev.type, EV_ABS);
+ ck_assert_int_eq(ev.code, ABS_MT_SLOT);
+ ck_assert_int_eq(ev.value, 1);
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+ ck_assert_int_eq(ev.type, EV_ABS);
+ ck_assert_int_eq(ev.code, ABS_MT_TRACKING_ID);
+ ck_assert_int_eq(ev.value, 1);
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+ ck_assert_int_eq(ev.type, EV_SYN);
+ ck_assert_int_eq(ev.code, SYN_REPORT);
+ ck_assert_int_eq(ev.value, 0);
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+ ck_assert_int_eq(ev.type, EV_ABS);
+ ck_assert_int_eq(ev.code, ABS_MT_TRACKING_ID);
+ ck_assert_int_eq(ev.value, 2);
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+ ck_assert_int_eq(ev.type, EV_SYN);
+ ck_assert_int_eq(ev.code, SYN_REPORT);
+ ck_assert_int_eq(ev.value, 0);
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, -EAGAIN);
+
+ uinput_device_free(uidev);
+ libevdev_free(dev);
+}
+END_TEST
+
+START_TEST(test_mt_tracking_id_nokeep_neg_1)
+{
+ struct uinput_device* uidev;
+ struct libevdev *dev;
+ int rc;
+ struct input_event ev;
+ struct input_absinfo abs[6];
+ int pipefd[2];
+ struct input_event events[] = {
+ { .type = EV_ABS, .code = ABS_MT_TRACKING_ID, .value = -1 },
+ { .type = EV_SYN, .code = SYN_REPORT, .value = 0 },
+ };
+ unsigned int flags = LIBEVDEV_READ_FLAG_NORMAL |
+ LIBEVDEV_READ_FLAG_ALLOW_TRACKING_ID_SWITCH;
+
+ rc = pipe2(pipefd, O_NONBLOCK);
+ ck_assert_int_eq(rc, 0);
+
+ memset(abs, 0, sizeof(abs));
+ abs[0].value = ABS_X;
+ abs[0].maximum = 1000;
+ abs[1].value = ABS_MT_POSITION_X;
+ abs[1].maximum = 1000;
+
+ abs[2].value = ABS_Y;
+ abs[2].maximum = 1000;
+ abs[3].value = ABS_MT_POSITION_Y;
+ abs[3].maximum = 1000;
+
+ abs[4].value = ABS_MT_SLOT;
+ abs[4].maximum = 10;
+ abs[5].value = ABS_MT_TRACKING_ID;
+ abs[5].maximum = 500;
+
+ test_create_abs_device(&uidev, &dev,
+ 6, abs,
+ EV_SYN, SYN_REPORT,
+ -1);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 1);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 1);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+ while (libevdev_next_event(dev, flags, &ev) != -EAGAIN)
+ ;
+
+ libevdev_set_log_function(test_logfunc_ignore_error, NULL);
+
+ /* two -1 tracking ids, need to use the pipe here, the kernel will
+ filter it otherwise */
+ libevdev_change_fd(dev, pipefd[0]);
+
+ rc = write(pipefd[1], events, sizeof(events));
+ ck_assert_int_eq(rc, sizeof(events));
+ rc = write(pipefd[1], events, sizeof(events));
+ ck_assert_int_eq(rc, sizeof(events));
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+ ck_assert_int_eq(ev.type, EV_ABS);
+ ck_assert_int_eq(ev.code, ABS_MT_TRACKING_ID);
+ ck_assert_int_eq(ev.value, -1);
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+ ck_assert_int_eq(ev.type, EV_SYN);
+ ck_assert_int_eq(ev.code, SYN_REPORT);
+ ck_assert_int_eq(ev.value, 0);
+
+ /* expect second tracking ID discarded */
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+ ck_assert_int_eq(ev.type, EV_SYN);
+ ck_assert_int_eq(ev.code, SYN_REPORT);
+ ck_assert_int_eq(ev.value, 0);
+
+ rc = libevdev_next_event(dev, flags, &ev);
+ ck_assert_int_eq(rc, -EAGAIN);
+
+ libevdev_set_log_function(test_logfunc_abort_on_error, NULL);
+
+ uinput_device_free(uidev);
+ libevdev_free(dev);
+}
+END_TEST
+
START_TEST(test_ev_rep_values)
{
struct uinput_device* uidev;
@@ -2183,6 +2345,8 @@ libevdev_events(void)
tcase_add_test(tc, test_mt_slot_ranges_invalid);
tcase_add_test(tc, test_mt_tracking_id_discard);
tcase_add_test(tc, test_mt_tracking_id_discard_neg_1);
+ tcase_add_test(tc, test_mt_tracking_id_keep);
+ tcase_add_test(tc, test_mt_tracking_id_nokeep_neg_1);
tcase_add_test(tc, test_ev_rep_values);
suite_add_tcase(s, tc);
--
2.14.3
More information about the Input-tools
mailing list