From 050bca91a1e8dd2e46c219c0b282092cb053166c Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 7 Apr 2014 15:16:28 +1000 Subject: [PATCH] Drain all events before synchronizing after SYN_DROPPED The kernel ring buffer drops all events on SYN_DROPPED, but then continues to fill up again. So by the time we read the events, the kernel's client buffer is essentially like this: SYN_DROPPED, ev1, ev2, ev3, ...., evN The kernel's device state represents the device after evN, and that is what the ioctls return. For EV_KEY, EV_SND, EV_LED and EV_SW the kernel removes potential duplicates from the client buffer [1], it doesn't do so for EV_ABS. So we can't actually sync while there are events on the wire because the events represent an earlier state. So simply discard all events in the kernel buffer, synchronize, and then start processing again. We lose some granularity but at least the events are correct. [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/input/evdev.c?id=483180281f0ac60d1138710eb21f4b9961901294 Signed-off-by: Peter Hutterer --- libevdev/libevdev.c | 58 ++++++++---- libevdev/libevdev.h | 45 +++++++++ test/test-libevdev-events.c | 219 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 302 insertions(+), 20 deletions(-) diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c index 7e8d3db..a316831 100644 --- a/libevdev/libevdev.c +++ b/libevdev/libevdev.c @@ -726,32 +726,50 @@ read_more_events(struct libevdev *dev) return 0; } -static int -sync_state(struct libevdev *dev) +static inline void +drain_events(struct libevdev *dev) { - int i; - int rc = 0; - struct input_event *ev; + int rc; + size_t nelem; + int iterations = 0; + const int max_iterations = 8; /* EVDEV_BUF_PACKETS in + kernel/drivers/input/evedev.c */ + + queue_shift_multiple(dev, queue_num_elements(dev), NULL); - /* FIXME: if we have events in the queue after the SYN_DROPPED (which was - queue[0]) we need to shift this backwards. Except that chances are that the - queue may be either full or too full to prepend all the events needed for - SYNC_IN_PROGRESS. + do { + rc = read_more_events(dev); + if (rc == -EAGAIN) + return; - so we search for the last sync event in the queue and drop everything before - including that event and rely on the kernel to tell us the right value for that - bitfield during the sync process. + if (rc < 0) { + log_error("Failed to drain events before sync.\n"); + return; + } + + nelem = queue_num_elements(dev); + queue_shift_multiple(dev, nelem, NULL); + } while (iterations++ < max_iterations && nelem >= queue_size(dev)); + + /* Our buffer should be roughly the same or bigger than the kernel + buffer in most cases, so we usually don't expect to recurse. If + we do, make sure we stop after max_iterations and proceed with + what we have. This could happen if events queue up faster than + we can drain them. */ + if (iterations >= max_iterations) + log_info("Unable to drain events, buffer size mismatch.\n"); +} - for (i = queue_num_elements(dev) - 1; i >= 0; i--) { - struct input_event e = {{0,0}, 0, 0, 0}; - queue_peek(dev, i, &e); - if (e.type == EV_SYN) - break; - } +static int +sync_state(struct libevdev *dev) +{ + int rc = 0; + struct input_event *ev; - if (i > 0) - queue_shift_multiple(dev, i + 1, NULL); + /* see section "Discarding events before synchronizing" in + * libevdev/libevdev.h */ + drain_events(dev); if (libevdev_has_event_type(dev, EV_KEY)) rc = sync_key_state(dev); diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h index 9ec1cb8..ef142ea 100644 --- a/libevdev/libevdev.h +++ b/libevdev/libevdev.h @@ -368,6 +368,51 @@ extern "C" { * The axis events do not reflect the position of a current touch point, a * client must take care not to generate a new touch point based on those * updates. + * + * Discarding events before synchronizing + * ===================================== + * + * The kernel implements the client buffer as a ring buffer. SYN_DROPPED + * events are handled when the buffer is full and a new event is received + * from a device. All existing events are discarded, a SYN_DROPPED is added + * to the buffer followed by the actual device event. Further events will be + * appended to the buffer until it is either read by the client, or filled + * again, at which point the sequence repeats. + * + * When the client reads the buffer, the buffer will thus always consist of + * exactly one SYN_DROPPED event followed by an unspecified number of real + * events. The data the ioctls return is the current state of the device, + * i.e. the state after all these events have been processed. For example, + * assume the buffer contains the following sequence: + * + * @code + EV_SYN SYN_DROPPED + EV_ABS ABS_X 1 + EV_SYN SYN_REPORT 0 + EV_ABS ABS_X 2 + EV_SYN SYN_REPORT 0 + EV_ABS ABS_X 3 + EV_SYN SYN_REPORT 0 + EV_ABS ABS_X 4 + EV_SYN SYN_REPORT 0 + EV_ABS ABS_X 5 + EV_SYN SYN_REPORT 0 + EV_ABS ABS_X 6 + EV_SYN SYN_REPORT 0 + * @endcode + * An ioctl at any time in this sequence will return a value of 6 for ABS_X. + * + * libevdev discards all events after a SYN_DROPPED to ensure the events + * during @ref LIBEVDEV_READ_FLAG_SYNC represent the last known state of the + * device. This loses some granularity of the events especially as the time + * between the SYN_DROPPED and the sync process increases. It does however + * avoid spurious cursor movements. In the above example, the event sequence + * by libevdev is: + * @code + EV_SYN SYN_DROPPED + EV_ABS ABS_X 6 + EV_SYN SYN_REPORT 0 + @endcode */ /** diff --git a/test/test-libevdev-events.c b/test/test-libevdev-events.c index dc412a7..ddc509d 100644 --- a/test/test-libevdev-events.c +++ b/test/test-libevdev-events.c @@ -927,6 +927,224 @@ START_TEST(test_syn_delta_tracking_ids) } END_TEST +START_TEST(test_syn_delta_late_sync) +{ + struct uinput_device* uidev; + struct libevdev *dev; + int rc; + struct input_event ev; + struct input_absinfo abs[6]; + int i, slot; + + 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 = 1; + + abs[5].minimum = -1; + abs[5].maximum = 255; + abs[5].value = ABS_MT_TRACKING_ID; + + test_create_abs_device(&uidev, &dev, + 6, abs, + EV_SYN, SYN_REPORT, + -1); + + /* emulate a touch down, make sure libevdev sees it */ + uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0); + uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 1); + uinput_device_event(uidev, EV_ABS, ABS_X, 100); + uinput_device_event(uidev, EV_ABS, ABS_Y, 500); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 100); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 500); + uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); + do { + rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev); + ck_assert_int_ne(rc, LIBEVDEV_READ_STATUS_SYNC); + } while (rc >= 0); + + /* force enough events to trigger a SYN_DROPPED */ + for (i = 0; i < 100; i++) { + uinput_device_event(uidev, EV_ABS, ABS_X, 100 + i); + uinput_device_event(uidev, EV_ABS, ABS_Y, 500 + i); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 100 + i); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 500 + i); + uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); + } + + rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev); + ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC); + + /* trigger the tracking ID change after getting the SYN_DROPPED */ + uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0); + uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, -1); + uinput_device_event(uidev, EV_ABS, ABS_X, 200); + uinput_device_event(uidev, EV_ABS, ABS_Y, 600); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 200); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 600); + uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); + + slot = 0; + + /* Now sync the device, expect the data to be equal to the last event*/ + while ((rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev)) != -EAGAIN) { + if (ev.type == EV_SYN) + continue; + + ck_assert_int_eq(ev.type, EV_ABS); + switch(ev.code) { + case ABS_MT_SLOT: + slot = ev.value; + break; + case ABS_MT_TRACKING_ID: + if (slot == 0) + ck_assert_int_eq(ev.value, -1); + break; + case ABS_X: + case ABS_MT_POSITION_X: + ck_assert_int_eq(ev.value, 200); + break; + case ABS_Y: + case ABS_MT_POSITION_Y: + ck_assert_int_eq(ev.value, 600); + break; + } + } + + /* And a new tracking ID */ + uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0); + uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 2); + uinput_device_event(uidev, EV_ABS, ABS_X, 201); + uinput_device_event(uidev, EV_ABS, ABS_Y, 601); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 201); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 601); + uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); + + while ((rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev)) != -EAGAIN) { + ck_assert_int_ne(rc, LIBEVDEV_READ_STATUS_SYNC); + + if (ev.type == EV_SYN) + continue; + + ck_assert_int_eq(ev.type, EV_ABS); + + switch(ev.code) { + case ABS_MT_SLOT: + ck_assert_int_eq(ev.value, 0); + break; + case ABS_MT_TRACKING_ID: + ck_assert_int_eq(ev.value, 2); + break; + case ABS_X: + case ABS_MT_POSITION_X: + ck_assert_int_eq(ev.value, 201); + break; + case ABS_Y: + case ABS_MT_POSITION_Y: + ck_assert_int_eq(ev.value, 601); + break; + } + } + + + /* Now we basically re-do the exact same test, just with the + tracking ID order inverted */ + + /* drop the tracking ID, make sure libevdev sees it */ + uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0); + uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, -1); + uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); + do { + rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev); + ck_assert_int_ne(rc, LIBEVDEV_READ_STATUS_SYNC); + } while (rc >= 0); + + /* force enough events to trigger a SYN_DROPPED */ + for (i = 0; i < 100; i++) { + uinput_device_event(uidev, EV_ABS, ABS_X, 100 + i); + uinput_device_event(uidev, EV_ABS, ABS_Y, 500 + i); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 100 + i); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 500 + i); + uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); + } + + rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev); + ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC); + + /* trigger the new tracking ID after getting the SYN_DROPPED */ + uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0); + uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 5); + uinput_device_event(uidev, EV_ABS, ABS_X, 200); + uinput_device_event(uidev, EV_ABS, ABS_Y, 600); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 200); + uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 600); + uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); + + slot = 0; + + /* Now sync the device, expect the data to be equal to the last event*/ + while ((rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev)) != -EAGAIN) { + if (ev.type == EV_SYN) + continue; + + ck_assert_int_eq(ev.type, EV_ABS); + switch(ev.code) { + case ABS_MT_SLOT: + slot = ev.value; + break; + case ABS_MT_TRACKING_ID: + if (slot == 0) + ck_assert_int_eq(ev.value, 5); + break; + case ABS_X: + case ABS_MT_POSITION_X: + ck_assert_int_eq(ev.value, 200); + break; + case ABS_Y: + case ABS_MT_POSITION_Y: + ck_assert_int_eq(ev.value, 600); + break; + } + } + + /* Drop the tracking ID */ + uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0); + uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, -1); + uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0); + + while ((rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev)) != -EAGAIN) { + ck_assert_int_ne(rc, LIBEVDEV_READ_STATUS_SYNC); + + if (ev.type == EV_SYN) + continue; + + ck_assert_int_eq(ev.type, EV_ABS); + + switch(ev.code) { + case ABS_MT_SLOT: + ck_assert_int_eq(ev.value, 0); + break; + case ABS_MT_TRACKING_ID: + ck_assert_int_eq(ev.value, -1); + break; + } + } + + + uinput_device_free(uidev); + libevdev_free(dev); +} +END_TEST + START_TEST(test_syn_delta_fake_mt) { struct uinput_device* uidev; @@ -1863,6 +2081,7 @@ libevdev_events(void) tcase_add_test(tc, test_syn_delta_sw); tcase_add_test(tc, test_syn_delta_fake_mt); tcase_add_test(tc, test_syn_delta_tracking_ids); + tcase_add_test(tc, test_syn_delta_late_sync); suite_add_tcase(s, tc); tc = tcase_create("skipped syncs"); -- 2.7.4