Terminate all stopped/changed touches during SYN_DROPPED in the first frame
authorPeter Hutterer <peter.hutterer@who-t.net>
Thu, 13 Feb 2020 11:11:14 +0000 (21:11 +1000)
committerPeter Hutterer <peter.hutterer@who-t.net>
Wed, 19 Feb 2020 01:06:28 +0000 (11:06 +1000)
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 <peter.hutterer@who-t.net>
libevdev/libevdev.c

index f8d59da1c060f2d16d5682562e7d1dd70b648d7d..9d93e665d9a5207525505c3bfd14b4331b224e43 100644 (file)
@@ -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);