From 41de1b0e1ade95dec46c10500db759e16e35a981 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 13 Feb 2020 21:11:14 +1000 Subject: [PATCH] Terminate all stopped/changed touches during SYN_DROPPED in the first frame The previous event processing had subtle issues with touches stopping during SYN_DROPPED. All of the device state was processed in the same frame, but if any touch changed tracking ID during SYN_DROPPED, an inserted SYN_REPORT resulted in a weird split of events: - the first frame had all key/sw/abs updates including those slots that changed tracking ID, but not the ones that were fully terminated. - the second frame had only the slots states for newly started touches **and** the slot state for touches terminated during SYN_DROPPED but not restarted. In other words, where three fingers were on the touchpad and slot 0 was lifted and put down again and slot 1 was lifted but *not* put down again, our frames contained: - frame 1: terminate slot 0, BTN_TOOL_TRIPLETAP 0, BTN_TOOL_DOUBLETAP 1 - frame 2: start slot 0, terminate slot 1 Where there was no touch changing tracking ID, only one frame was generated. The BTN_TOOL updates were buggy, they may not match the number of fingers down as seen on a frame-by-frame basis. This triggered libinput bug https://gitlab.freedesktop.org/libinput/libinput/issues/422 This patch changes the above example to - frame 1: terminate slot 0, terminate slot 1 - frame 2: start slot 0, BTN_TOOL_TRIPLETAP 0, BTN_TOOL_DOUBLETAP 1 Notably, the first frame no longer contains the BTN_TOOL bits. This patch is one of two, the BTN_TOOL sync bits are part of a follow-up patch. Signed-off-by: Peter Hutterer --- libevdev/libevdev.c | 44 +++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c index f8d59da..9d93e66 100644 --- a/libevdev/libevdev.c +++ b/libevdev/libevdev.c @@ -740,7 +740,8 @@ terminate_slots(struct libevdev *dev, bool touches_stopped = false; for (int slot = 0; slot < dev->num_slots; slot++) { - if (changes[slot].state == TOUCH_CHANGED) { + if (changes[slot].state == TOUCH_CHANGED || + changes[slot].state == TOUCH_STOPPED) { queue_push_event(dev, EV_ABS, ABS_MT_SLOT, slot); queue_push_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1); @@ -765,7 +766,10 @@ push_mt_sync_events(struct libevdev *dev, int rc; for (int slot = 0; slot < dev->num_slots; slot++) { - if (!bit_is_set(changes[slot].axes, ABS_MT_SLOT)) + /* stopped touches were already terminated in + * terminate_slots */ + if (changes[slot].state == TOUCH_STOPPED || + !bit_is_set(changes[slot].axes, ABS_MT_SLOT)) continue; queue_push_event(dev, EV_ABS, ABS_MT_SLOT, slot); @@ -863,11 +867,34 @@ static int sync_state(struct libevdev *dev) { int rc = 0; + bool want_mt_sync = false; + int last_reported_slot = 0; + struct slot_change_state changes[dev->num_slots > 0 ? dev->num_slots : 1]; + memset(changes, 0, sizeof(changes)); /* see section "Discarding events before synchronizing" in * libevdev/libevdev.h */ drain_events(dev); + /* We generate one or two event frames during sync. + * The first one (if it exists) terminates all slots that have + * either terminated during SYN_DROPPED or changed their tracking + * ID. + * + * The second frame syncs everything up to the current state of the + * device - including re-starting those slots that have a changed + * tracking id. + */ + if (dev->num_slots > -1 && + libevdev_has_event_code(dev, EV_ABS, ABS_MT_SLOT)) { + want_mt_sync = true; + rc = sync_mt_state(dev, changes); + if (rc == 0) + terminate_slots(dev, changes, &last_reported_slot); + else + want_mt_sync = false; + } + if (libevdev_has_event_type(dev, EV_KEY)) rc = sync_key_state(dev); if (libevdev_has_event_type(dev, EV_LED)) @@ -876,17 +903,8 @@ sync_state(struct libevdev *dev) rc = sync_sw_state(dev); if (rc == 0 && libevdev_has_event_type(dev, EV_ABS)) rc = sync_abs_state(dev); - if (rc == 0 && dev->num_slots > -1 && - libevdev_has_event_code(dev, EV_ABS, ABS_MT_SLOT)) { - struct slot_change_state changes[dev->num_slots]; - int last_reported_slot = 0; - - rc = sync_mt_state(dev, changes); - if (rc == 0) { - terminate_slots(dev, changes, &last_reported_slot); - push_mt_sync_events(dev, changes, last_reported_slot); - } - } + if (rc == 0 && want_mt_sync) + push_mt_sync_events(dev, changes, last_reported_slot); dev->queue_nsync = queue_num_elements(dev); -- 2.34.1