[PATCH 1/2] Handle SYN_DROPPED, filter keys with a bitmap

Martin Minarik minarik11 at student.fiit.stuba.sk
Tue Jun 4 11:47:36 PDT 2013


Since evdev keys are unreliable, they might randomly get dropped, such
as, on SYN_DROPPED. Even SYN_DROPPED is sometimes not delivered.
Clients, compositor are not able to recover from duplicate press/release.
This fixes this bug, thereby making the compositor and clients useable
even under critical system conditions such as, but not limited to, high
system load, device malfunction. This improves reliability.

Every device contains a key bitmap. Each passing key press/release
is checked and recorded. Duplicated presses, releases are handled by
emitting the opposite type event in between (the time is faked -1 time unit).
This is a last resort solution and happens only very rarely.

When a SYN_DROPPED is detected:
1 wait until after the next SYN_REPORT.
2 Ask the kernel for the actual state of buttons
3 Compare the buttons with the internal key bitmap, to see what changed.
4 Send each different key via notify to the compositor, with timestamp.
5 Update the internal key bitmap to match the kernel's

Disadvantages of this approach
Each evdev device consumes 96 bytes more memory.
Table lookup+bit toggle for almost key.
Don't know the timestamp of missing keys.
The key_notify from kernel bitmap compare occurs later the real missing event.
That's a timestamp issue.

---
 src/evdev-touchpad.c |   51 ++++++++++++++-
 src/evdev.c          |  176 ++++++++++++++++++++++++++++++++++++++------------
 src/evdev.h          |   35 +++++++---
 3 files changed, 208 insertions(+), 54 deletions(-)

diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c
index a21ae0b..890ddad 100644
--- a/src/evdev-touchpad.c
+++ b/src/evdev-touchpad.c
@@ -533,6 +533,26 @@ on_release(struct touchpad_dispatch *touchpad)
 	push_fsm_event(touchpad, FSM_EVENT_RELEASE);
 }
 
+struct evdev_dispatch_interface touchpad_interface;
+struct evdev_dispatch_interface touchpad_syn_drop_interface;
+
+static inline void
+process_syn(struct touchpad_dispatch *touchpad,
+		 struct evdev_device *device,
+		 struct input_event *e, uint32_t time)
+{
+	switch (e->code) {
+	case SYN_DROPPED:
+		if (device->dispatch->interface == &touchpad_interface)
+			device->dispatch->interface = &touchpad_syn_drop_interface;
+		break;
+	case SYN_REPORT:
+	default:
+		touchpad->event_mask |= TOUCHPAD_EVENT_REPORT;
+		break;
+	}
+}
+
 static inline void
 process_absolute(struct touchpad_dispatch *touchpad,
 		 struct evdev_device *device,
@@ -588,7 +608,7 @@ process_key(struct touchpad_dispatch *touchpad,
 	case BTN_FORWARD:
 	case BTN_BACK:
 	case BTN_TASK:
-		notify_button(device->seat,
+			notify_button(device->seat,
 			      time, e->code,
 			      e->value ? WL_POINTER_BUTTON_STATE_PRESSED :
 			                 WL_POINTER_BUTTON_STATE_RELEASED);
@@ -634,8 +654,7 @@ touchpad_process(struct evdev_dispatch *dispatch,
 
 	switch (e->type) {
 	case EV_SYN:
-		if (e->code == SYN_REPORT)
-			touchpad->event_mask |= TOUCHPAD_EVENT_REPORT;
+		process_syn(touchpad, device, e, time);
 		break;
 	case EV_ABS:
 		process_absolute(touchpad, device, e);
@@ -664,6 +683,32 @@ struct evdev_dispatch_interface touchpad_interface = {
 	touchpad_destroy
 };
 
+static void
+touchpad_syn_drop_process(struct evdev_dispatch *dispatch,
+		 struct evdev_device *device,
+		 struct input_event *event,
+		 uint32_t time)
+{
+	if ((event->code != EV_SYN) || (event->code != SYN_REPORT))
+		return;
+
+	if (device->dispatch->interface == &touchpad_syn_drop_interface)
+		device->dispatch->interface = &touchpad_interface;
+
+	evdev_keys_state_sync(device, time);
+}
+
+static void
+touchpad_syn_drop_destroy(struct evdev_dispatch *dispatch)
+{
+	free(dispatch);
+}
+
+struct evdev_dispatch_interface touchpad_syn_drop_interface = {
+	touchpad_syn_drop_process,
+	touchpad_syn_drop_destroy
+};
+
 static int
 touchpad_init(struct touchpad_dispatch *touchpad,
 	      struct evdev_device *device)
diff --git a/src/evdev.c b/src/evdev.c
index 122a2d9..8058c8f 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -62,13 +62,29 @@ evdev_led_update(struct evdev_device *device, enum weston_led leds)
 	(void)i; /* no, we really don't care about the return value */
 }
 
+struct evdev_dispatch_interface fallback_interface;
+struct evdev_dispatch_interface syn_drop_interface;
+
 static inline void
-evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
+evdev_process_syn(struct evdev_device *device, struct input_event *e)
 {
-	if (e->value == 2)
-		return;
-
 	switch (e->code) {
+	case SYN_DROPPED:
+		if (device->dispatch->interface == &fallback_interface)
+			device->dispatch->interface = &syn_drop_interface;
+		break;
+	case SYN_REPORT:
+	default:
+		device->pending_events |= EVDEV_SYN;
+		break;
+	}
+}
+
+static void
+evdev_process_key(struct evdev_device *device,
+		  unsigned int time, unsigned int code, int value)
+{
+	switch (code) {
 	case BTN_LEFT:
 	case BTN_RIGHT:
 	case BTN_MIDDLE:
@@ -78,21 +94,56 @@ evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
 	case BTN_BACK:
 	case BTN_TASK:
 		notify_button(device->seat,
-			      time, e->code,
-			      e->value ? WL_POINTER_BUTTON_STATE_PRESSED :
+			      time, code,
+			      value ? WL_POINTER_BUTTON_STATE_PRESSED :
 					 WL_POINTER_BUTTON_STATE_RELEASED);
 		break;
 
 	default:
 		notify_key(device->seat,
-			   time, e->code,
-			   e->value ? WL_KEYBOARD_KEY_STATE_PRESSED :
+			   time, code,
+			   value ? WL_KEYBOARD_KEY_STATE_PRESSED :
 				      WL_KEYBOARD_KEY_STATE_RELEASED,
 			   STATE_UPDATE_AUTOMATIC);
 		break;
 	}
 }
 
+int
+evdev_keys_state_sync(struct evdev_device *d, unsigned int time)
+{
+	unsigned long kernel_keys[NBITS(KEY_CNT)];
+	unsigned int i, j;
+
+	if (d->deliv_state.keys == NULL)
+		return -1;
+
+	memset(kernel_keys, 0, sizeof(kernel_keys));
+
+	if (ioctl(d->fd, EVIOCGKEY(KEY_CNT), kernel_keys) < 0)
+		return -1;
+
+	for (i = 0; i < ARRAY_LENGTH(kernel_keys); i++) {
+		long changed_keys = kernel_keys[i] ^ d->deliv_state.keys[i];
+
+		if (changed_keys == 0)
+			continue;
+
+		d->deliv_state.keys[i] = kernel_keys[i];
+
+		for (j = 0; j < BITS_PER_LONG; j++) {
+			if (changed_keys & BIT(j)) {
+				int key = BITS_PER_LONG * i + j;
+				int val = TEST_BIT(kernel_keys, key);
+
+				evdev_process_key(d, time, key, val);
+			}
+		}
+	}
+
+	return 0;
+}
+
 static void
 evdev_process_touch(struct evdev_device *device, struct input_event *e)
 {
@@ -307,10 +358,19 @@ fallback_process(struct evdev_dispatch *dispatch,
 		evdev_process_absolute(device, event);
 		break;
 	case EV_KEY:
-		evdev_process_key(device, event, time);
+		if (event->value == 2)
+			break;
+
+		if (event->value == !TEST_BIT(device->deliv_state.keys, event->code)) {
+			device->deliv_state.keys[LONG(event->code)] ^= BIT(event->code);
+		} else {
+			evdev_process_key(device, time - 1, event->code, !event->value);
+		}
+
+		evdev_process_key(device, time, event->code, event->value);
 		break;
 	case EV_SYN:
-		device->pending_events |= EVDEV_SYN;
+		evdev_process_syn(device, event);
 		break;
 	}
 }
@@ -326,6 +386,32 @@ struct evdev_dispatch_interface fallback_interface = {
 	fallback_destroy
 };
 
+static void
+syn_drop_process(struct evdev_dispatch *dispatch,
+		 struct evdev_device *device,
+		 struct input_event *event,
+		 uint32_t time)
+{
+	if ((event->code != EV_SYN) || (event->code != SYN_REPORT))
+		return;
+
+	if (device->dispatch->interface == &syn_drop_interface)
+		device->dispatch->interface = &fallback_interface;
+
+	evdev_keys_state_sync(device, time);
+}
+
+static void
+syn_drop_destroy(struct evdev_dispatch *dispatch)
+{
+	free(dispatch);
+}
+
+struct evdev_dispatch_interface syn_drop_interface = {
+	syn_drop_process,
+	syn_drop_destroy
+};
+
 static struct evdev_dispatch *
 fallback_dispatch_create(void)
 {
@@ -590,11 +676,26 @@ err1:
 }
 
 void
+evdev_keys_down_release(struct evdev_device *device)
+{
+	int i;
+	for (i = 0; i < KEY_CNT; i++) {
+		if (TEST_BIT(device->deliv_state.keys, i)) {
+			notify_key(device->seat,
+				   0xFFFF, i,
+				   WL_KEYBOARD_KEY_STATE_RELEASED,
+				   STATE_UPDATE_AUTOMATIC);
+		}
+	}
+}
+
+void
 evdev_device_destroy(struct evdev_device *device)
 {
-	struct evdev_dispatch *dispatch;
+	struct evdev_dispatch *dispatch = device->dispatch;
+
+	evdev_keys_down_release(device);
 
-	dispatch = device->dispatch;
 	if (dispatch)
 		dispatch->interface->destroy(dispatch);
 
@@ -608,45 +709,38 @@ evdev_device_destroy(struct evdev_device *device)
 	free(device);
 }
 
+inline void
+evdev_keys_down_array(struct wl_list *evdev_devices, struct wl_array *keys)
+{
+	struct evdev_device *device;
+	uint32_t i;
+	uint32_t *k;
+
+	for (i = 0; i < KEY_CNT; i++) {
+		wl_list_for_each(device, evdev_devices, link) {
+			if (TEST_BIT(device->deliv_state.keys, i)) {
+				k = wl_array_add(keys, sizeof *k);
+				*k = i;
+				break;
+				/* XXX:Perhaps deliver duplicate keys too */
+			}
+		}
+	}
+}
+
 void
 evdev_notify_keyboard_focus(struct weston_seat *seat,
 			    struct wl_list *evdev_devices)
 {
-	struct evdev_device *device;
 	struct wl_array keys;
-	unsigned int i, set;
-	char evdev_keys[(KEY_CNT + 7) / 8];
-	char all_keys[(KEY_CNT + 7) / 8];
-	uint32_t *k;
-	int ret;
-
-	if (!seat->keyboard)
-		return;
+	struct evdev_device *device;
 
-	memset(all_keys, 0, sizeof all_keys);
 	wl_list_for_each(device, evdev_devices, link) {
-		memset(evdev_keys, 0, sizeof evdev_keys);
-		ret = ioctl(device->fd,
-			    EVIOCGKEY(sizeof evdev_keys), evdev_keys);
-		if (ret < 0) {
-			weston_log("failed to get keys for device %s\n",
-				device->devnode);
-			continue;
-		}
-		for (i = 0; i < ARRAY_LENGTH(evdev_keys); i++)
-			all_keys[i] |= evdev_keys[i];
+		evdev_keys_state_sync(device, 0);
 	}
 
 	wl_array_init(&keys);
-	for (i = 0; i < KEY_CNT; i++) {
-		set = all_keys[i >> 3] & (1 << (i & 7));
-		if (set) {
-			k = wl_array_add(&keys, sizeof *k);
-			*k = i;
-		}
-	}
-
+	evdev_keys_down_array(evdev_devices, &keys);
 	notify_keyboard_focus_in(seat, &keys, STATE_UPDATE_AUTOMATIC);
-
 	wl_array_release(&keys);
 }
diff --git a/src/evdev.h b/src/evdev.h
index eb5c868..e6a8e4c 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -26,6 +26,16 @@
 #include <linux/input.h>
 #include <wayland-util.h>
 
+/* copied from udev/extras/input_id/input_id.c */
+/* we must use this kernel-compatible implementation */
+#define BITS_PER_LONG (sizeof(unsigned long) * 8)
+#define NBITS(x) ((((x)-1)/BITS_PER_LONG)+1)
+#define OFF(x)  ((x)%BITS_PER_LONG)
+#define BIT(x)  (1UL<<OFF(x))
+#define LONG(x) ((x)/BITS_PER_LONG)
+#define TEST_BIT(array, bit)    ((array[LONG(bit)] >> OFF(bit)) & 1)
+/* end copied */
+
 #define MAX_SLOTS 16
 
 enum evdev_event_type {
@@ -45,6 +55,10 @@ enum evdev_device_capability {
 	EVDEV_TOUCH = (1 << 4),
 };
 
+struct evdev_bitmap {
+	unsigned long keys[NBITS(KEY_CNT)];
+};
+
 struct evdev_device {
 	struct weston_seat *seat;
 	struct wl_list link;
@@ -77,17 +91,9 @@ struct evdev_device {
 	enum evdev_device_capability caps;
 
 	int is_mt;
-};
 
-/* copied from udev/extras/input_id/input_id.c */
-/* we must use this kernel-compatible implementation */
-#define BITS_PER_LONG (sizeof(unsigned long) * 8)
-#define NBITS(x) ((((x)-1)/BITS_PER_LONG)+1)
-#define OFF(x)  ((x)%BITS_PER_LONG)
-#define BIT(x)  (1UL<<OFF(x))
-#define LONG(x) ((x)/BITS_PER_LONG)
-#define TEST_BIT(array, bit)    ((array[LONG(bit)] >> OFF(bit)) & 1)
-/* end copied */
+	struct evdev_bitmap deliv_state;
+};
 
 #define EVDEV_UNHANDLED_DEVICE ((struct evdev_device *) 1)
 
@@ -121,6 +127,15 @@ void
 evdev_device_destroy(struct evdev_device *device);
 
 void
+evdev_keys_down_release(struct evdev_device *device);
+
+void
+evdev_keys_down_array(struct wl_list *evdev_devices,
+			   struct wl_array *keys);
+int
+evdev_keys_state_sync(struct evdev_device *d, unsigned int time);
+
+void
 evdev_notify_keyboard_focus(struct weston_seat *seat,
 			    struct wl_list *evdev_devices);
 
-- 
1.7.10.4



More information about the wayland-devel mailing list