[PATCH] evdev: Handle SYN_DROPPED, keep transaction buffer

Pekka Paalanen ppaalanen at gmail.com
Tue Apr 23 00:46:03 PDT 2013


On Sat, 6 Apr 2013 00:27:10 +0200
Martin Minarik <minarik11 at student.fiit.stuba.sk> wrote:

> Each evdev device keeps a key press bitmap,
> the incoming events are filtered to the following constraints:
> 
> 1. only notify releases for previously pressed keys
> 2. only notify presses for previously released keys
> 3. on device destroy, notify a releases for all pressed keys
> 
> Notes:
> 
> 1. For example when switching VTs, we recieve only key releases
>    Nothing special, the lost key presses were of course not
>    recorded (happens outside weston). So key releases are simply
>    dropped with warning (double release).
> 2. This is solved by injecting a fake key release notify, we
>    assume evdev doesn't send double key press without key
>    release in between.
> 3. Solved by broadcasting fake release events.

Hi,

how is this explanation related to SYN_DROPPED?

It looks like the title promises one thing, and then the explanation
discusses a completely different thing, and as far as I understood from
a quick glance, the patch attemps to do both.

I think this should be split into two patches, one handling
SYN_DROPPED, and the other the rest.

Also, I do not think you can fake key events, if that is what I think
it is. You can't get a real timestamp for them, and in general, I think
we just do not want to fake anything. If you need a "reset all state"
action, write one.

Was there some related irc discussion?


Thanks,
pq


> ---
>  src/evdev-touchpad.c |   57 +++++++++++-
>  src/evdev.c          |  240
> ++++++++++++++++++++++++++++++++++++++++++--------
> src/evdev.h          |   31 +++++++ 3 files changed, 289
> insertions(+), 39 deletions(-)
> 
> diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c
> index c25a199..8f1e0c3 100644
> --- a/src/evdev-touchpad.c
> +++ b/src/evdev-touchpad.c
> @@ -523,6 +523,27 @@ 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;
> +		weston_log("warning: evdev: Syn drop at %u on %s
> \n", time, device->devname);
> +		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,
> @@ -578,7 +599,8 @@ process_key(struct touchpad_dispatch *touchpad,
>  	case BTN_FORWARD:
>  	case BTN_BACK:
>  	case BTN_TASK:
> -		notify_button(device->seat,
> +		if (evdev_tx(device, e->code, e->value))
> +			notify_button(device->seat,
>  			      time, e->code,
>  			      e->value ?
> WL_POINTER_BUTTON_STATE_PRESSED : WL_POINTER_BUTTON_STATE_RELEASED);
> @@ -624,8 +646,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);
> @@ -654,6 +679,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_tx_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 d2954b5..5417149 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -60,6 +60,130 @@ 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_syn(struct evdev_device *device, struct input_event
> *e, int time) +{
> +	switch (e->code) {
> +	case SYN_DROPPED:
> +		if (device->dispatch->interface ==
> &fallback_interface)
> +			device->dispatch->interface =
> &syn_drop_interface;
> +		weston_log("warning: evdev: Syn drop at %u on %s
> \n", time, device->devname);
> +		break;
> +	case SYN_REPORT:
> +	default:
> +		device->pending_events |= EVDEV_SYN;
> +		break;
> +	}
> +}
> +
> +static void
> +evdev_tx_fake_key_up(struct evdev_device *d, unsigned int time,
> unsigned int key) +{
> +	struct evdev_dispatch *dispatch = d->dispatch;
> +	struct input_event fake;
> +	fake.type = EV_KEY;
> +	fake.code = key;
> +	fake.value = 0;
> +
> +	dispatch->interface->process(dispatch, d, &fake, time);
> +}
> +
> +
> +static int
> +evdev_tx_press(struct evdev_device *device, uint32_t key)
> +{
> +	if (device->tx.delivered_keys == NULL) {
> +		device->tx.delivered_keys = calloc(NBITS(KEY_CNT),
> sizeof(unsigned long));
> +	} else if (TEST_BIT(device->tx.delivered_keys, key)) {
> +		weston_log("warning: evdev: double-press key%u\n",
> key);
> +		return 0;
> +	}
> +	if (device->tx.delivered_keys == NULL) {
> +		weston_log("warning: evdev: register alloc\n");
> +		return 0;
> +	} else {
> +		device->tx.delivered_keys[LONG(key)] |= BIT(key);
> +		return 1;
> +	}
> +}
> +
> +static int
> +evdev_tx_release(struct evdev_device *device, uint32_t key)
> +{
> +	if (device->tx.delivered_keys == NULL) {
> +		weston_log("warning: evdev: no register, release
> key%u\n", key);
> +		return 0;
> +	} else if (TEST_BIT(device->tx.delivered_keys, key)) {
> +		device->tx.delivered_keys[LONG(key)] &= ~(BIT(key));
> +		return 1;
> +	} else {
> +		weston_log("warning: evdev: double-release key%u\n",
> key);
> +		return 0;
> +	}
> +}
> +
> +int
> +evdev_tx(struct evdev_device *d, uint32_t key, uint32_t state)
> +{
> +	switch (state) {
> +		case 0:
> +			return evdev_tx_release(d, key);
> +		case 1:
> +			return evdev_tx_press(d, key);
> +		default:
> +			return 1;
> +	}
> +}
> +
> +int
> +evdev_tx_sync(struct evdev_device *d, unsigned int time)
> +{
> +	unsigned long kernel_keys[NBITS(KEY_CNT)];
> +	int ret;
> +	unsigned int i;
> +
> +	if (d->tx.delivered_keys == NULL)
> +		return -1;
> +
> +	memset(kernel_keys, 0, sizeof(kernel_keys));
> +	ret = ioctl(d->fd, EVIOCGKEY(KEY_CNT), kernel_keys);
> +	
> +	if (ret < 0)
> +		return -1;
> +
> +	for (i = 0; i < ARRAY_LENGTH(kernel_keys); i++) {
> +		const unsigned long deliv = d->tx.delivered_keys[i];
> +		const unsigned long released_keys = ~kernel_keys[i]
> & deliv; +
> +		if (released_keys) {
> +			unsigned int j;
> +			for (j = 0; j < BITS_PER_LONG; j++) {
> +				if (released_keys & BIT(j)) {
> +					const unsigned int key =
> BITS_PER_LONG * i + j;
> +					evdev_tx_fake_key_up(d,
> time, key);
> +				}
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static void
> +evdev_tx_fake_terminate_all(struct evdev_device *device)
> +{
> +	unsigned int key;
> +
> +	for (key = 0; key < KEY_CNT; key++) {
> +		if (TEST_BIT(device->tx.delivered_keys, key)) {
> +			evdev_tx_fake_key_up(device, 0x1337, key);
> +		}
> +	}
> +}
> +
>  static inline void
>  evdev_process_key(struct evdev_device *device, struct input_event
> *e, int time) {
> @@ -75,18 +199,21 @@ evdev_process_key(struct evdev_device *device,
> struct input_event *e, int time) case BTN_FORWARD:
>  	case BTN_BACK:
>  	case BTN_TASK:
> -		notify_button(device->seat,
> -			      time, e->code,
> -			      e->value ?
> WL_POINTER_BUTTON_STATE_PRESSED :
> -
> WL_POINTER_BUTTON_STATE_RELEASED);
> -		break;
> +		if (evdev_tx(device, e->code, e->value))
> +			notify_button(device->seat,
> +				      time, e->code,
> +				      e->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 :
> -				      WL_KEYBOARD_KEY_STATE_RELEASED,
> -			   STATE_UPDATE_AUTOMATIC);
> +		if (evdev_tx(device, e->code, e->value))
> +			notify_key(device->seat,
> +				   time, e->code,
> +				   e->value ?
> WL_KEYBOARD_KEY_STATE_PRESSED :
> +
> WL_KEYBOARD_KEY_STATE_RELEASED,
> +				   STATE_UPDATE_AUTOMATIC);
> +
>  		break;
>  	}
>  }
> @@ -308,7 +435,7 @@ fallback_process(struct evdev_dispatch *dispatch,
>  		evdev_process_key(device, event, time);
>  		break;
>  	case EV_SYN:
> -		device->pending_events |= EVDEV_SYN;
> +		evdev_process_syn(device, event, time);
>  		break;
>  	}
>  }
> @@ -324,6 +451,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_tx_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)
>  {
> @@ -543,6 +696,7 @@ evdev_device_create(struct weston_seat *seat,
> const char *path, int device_fd) device->rel.dy = 0;
>  	device->dispatch = NULL;
>  	device->fd = device_fd;
> +	device->tx.delivered_keys = NULL;
>  
>  	ioctl(device->fd, EVIOCGNAME(sizeof(devname)), devname);
>  	device->devname = strdup(devname);
> @@ -592,6 +746,11 @@ evdev_device_destroy(struct evdev_device *device)
>  {
>  	struct evdev_dispatch *dispatch;
>  
> +	if (device->tx.delivered_keys) {
> +		evdev_tx_fake_terminate_all(device);
> +		free(device->tx.delivered_keys);
> +	}
> +
>  	dispatch = device->dispatch;
>  	if (dispatch)
>  		dispatch->interface->destroy(dispatch);
> @@ -600,51 +759,60 @@ evdev_device_destroy(struct evdev_device
> *device) wl_list_remove(&device->link);
>  	if (device->mtdev)
>  		mtdev_close_delete(device->mtdev);
> +
>  	close(device->fd);
>  	free(device->devname);
>  	free(device->devnode);
>  	free(device);
>  }
>  
> +
>  void
> -evdev_notify_keyboard_focus(struct weston_seat *seat,
> -			    struct wl_list *evdev_devices)
> +evdev_keys_states_query(struct wl_list *evdev_devices,
> +			unsigned long all_keys[NBITS(KEY_CNT)])
>  {
>  	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->seat.keyboard)
> -		return;
>  
> -	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);
> +		unsigned long evdev_keys[NBITS(KEY_CNT)];
> +		unsigned int i;
> +
> +		if (evdev_tx_sync(device, 0x1337))
>  			continue;
> -		}
> +
> +		memset(evdev_keys, 0, sizeof(evdev_keys));
> +
>  		for (i = 0; i < ARRAY_LENGTH(evdev_keys); i++)
> -			all_keys[i] |= evdev_keys[i];
> +			all_keys[i] |= device->tx.delivered_keys[i];
>  	}
> +}
> +
> +void
> +evdev_keys_depressed_array(struct wl_list *evdev_devices, struct
> wl_array *keys) +{
> +	unsigned long all_keys[NBITS(KEY_CNT)];
> +	uint32_t i;
> +	uint32_t *k;
> +
> +	memset(all_keys, 0, sizeof(all_keys));
> +	evdev_keys_states_query(evdev_devices, all_keys);
>  
> -	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);
> +		if (TEST_BIT(all_keys,i)) {
> +			k = wl_array_add(keys, sizeof *k);
>  			*k = i;
>  		}
>  	}
> +}
>  
> -	notify_keyboard_focus_in(seat, &keys,
> STATE_UPDATE_AUTOMATIC); +void
> +evdev_notify_keyboard_focus(struct weston_seat *seat,
> +			    struct wl_list *evdev_devices)
> +{
> +	struct wl_array keys;
>  
> +	wl_array_init(&keys);
> +	evdev_keys_depressed_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..a0b6639 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -45,6 +45,15 @@ enum evdev_device_capability {
>  	EVDEV_TOUCH = (1 << 4),
>  };
>  
> +struct evdev_stale_keypress {
> +	uint16_t code;
> +	uint16_t count;
> +};
> +
> +struct evdev_transactional {
> +	unsigned long *delivered_keys;
> +};
> +
>  struct evdev_device {
>  	struct weston_seat *seat;
>  	struct wl_list link;
> @@ -77,6 +86,8 @@ struct evdev_device {
>  	enum evdev_device_capability caps;
>  
>  	int is_mt;
> +
> +	struct evdev_transactional tx;
>  };
>  
>  /* copied from udev/extras/input_id/input_id.c */
> @@ -121,7 +132,27 @@ void
>  evdev_device_destroy(struct evdev_device *device);
>  
>  void
> +evdev_keys_states_count(struct wl_list *evdev_devices, uint8_t
> *keys_counts,
> +			uint32_t key_min, uint32_t key_max);
> +void
> +evdev_keys_states_query(struct wl_list *evdev_devices,
> +			unsigned long all_keys[NBITS(KEY_CNT)]);
> +void
> +evdev_keys_depressed_array(struct wl_list *evdev_devices,struct
> wl_array *keys); +
> +void
>  evdev_notify_keyboard_focus(struct weston_seat *seat,
>  			    struct wl_list *evdev_devices);
>  
> +void
> +evdev_notify_key_status(struct weston_seat *seat,
> +			struct wl_list *evdev_devices,
> +			uint32_t key_min, uint32_t key_max);
> +
> +int
> +evdev_tx(struct evdev_device *d, uint32_t key, uint32_t state);
> +
> +int
> +evdev_tx_sync(struct evdev_device *d, unsigned int time);
> +
>  #endif /* EVDEV_H */



More information about the wayland-devel mailing list