[PATCH weston v4] input: don't send to clients key events eaten by bindings

Pekka Paalanen ppaalanen at gmail.com
Tue Nov 11 05:10:52 PST 2014


On Tue, 11 Nov 2014 11:23:40 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

> weston key bindings are supposed to eat the key events, and not pass it
> on to clients, and indeed the wl_keyboard.key event is not sent. But
> we must also not put the key in the keys array to pass to client with
> the wl_keyboard.enter event, or else we may send the 'eaten' one too.
> In the case of a key binding hiding a surface having the keyboard focus,
> the shell may decide to give the focus to another surface, but that will
> happen before the key is released, so the new focus surface will receive
> the code of the bound key in the wl_keyboard.enter array.
> ---
> 
> v4: - initialize and release the eaten_keys array
>     - check the eaten_keys array is empty before updating the keymap
>       (i'm am not completely sure this is correct to be honest, but i think so :) )
> 
>  src/bindings.c   |  7 ++++--
>  src/compositor.h |  3 ++-
>  src/input.c      | 67 ++++++++++++++++++++++++++++++++++++++++----------------
>  3 files changed, 55 insertions(+), 22 deletions(-)
> 
> diff --git a/src/bindings.c b/src/bindings.c
> index 5e24725..369c81a 100644
> --- a/src/bindings.c
> +++ b/src/bindings.c
> @@ -255,16 +255,17 @@ install_binding_grab(struct weston_seat *seat, uint32_t time, uint32_t key)
>  	weston_keyboard_start_grab(seat->keyboard, &grab->grab);
>  }
>  
> -WL_EXPORT void
> +WL_EXPORT int
>  weston_compositor_run_key_binding(struct weston_compositor *compositor,
>  				  struct weston_seat *seat,
>  				  uint32_t time, uint32_t key,
>  				  enum wl_keyboard_key_state state)
>  {
>  	struct weston_binding *b, *tmp;
> +	int eaten = 0;
>  
>  	if (state == WL_KEYBOARD_KEY_STATE_RELEASED)
> -		return;
> +		return eaten;
>  
>  	/* Invalidate all active modifier bindings. */
>  	wl_list_for_each(b, &compositor->modifier_binding_list, link)
> @@ -281,8 +282,10 @@ weston_compositor_run_key_binding(struct weston_compositor *compositor,
>  			if (seat->keyboard->grab ==
>  			    &seat->keyboard->default_grab)
>  				install_binding_grab(seat, time, key);
> +			++eaten;
>  		}
>  	}
> +	return eaten;
>  }
>  
>  WL_EXPORT void
> diff --git a/src/compositor.h b/src/compositor.h
> index 27ea693..55f73db 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -473,6 +473,7 @@ struct weston_keyboard {
>  	uint32_t grab_time;
>  
>  	struct wl_array keys;
> +	struct wl_array eaten_keys;
>  
>  	struct {
>  		uint32_t mods_depressed;
> @@ -1144,7 +1145,7 @@ weston_binding_destroy(struct weston_binding *binding);
>  void
>  weston_binding_list_destroy_all(struct wl_list *list);
>  
> -void
> +int
>  weston_compositor_run_key_binding(struct weston_compositor *compositor,
>  				  struct weston_seat *seat, uint32_t time,
>  				  uint32_t key,
> diff --git a/src/input.c b/src/input.c
> index b504d06..5cb2703 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -526,6 +526,7 @@ weston_keyboard_create(void)
>  	wl_list_init(&keyboard->focus_resource_listener.link);
>  	keyboard->focus_resource_listener.notify = keyboard_focus_resource_destroyed;
>  	wl_array_init(&keyboard->keys);
> +	wl_array_init(&keyboard->eaten_keys);
>  	keyboard->default_grab.interface = &default_keyboard_grab_interface;
>  	keyboard->default_grab.keyboard = keyboard;
>  	keyboard->grab = &keyboard->default_grab;
> @@ -552,6 +553,7 @@ weston_keyboard_destroy(struct weston_keyboard *keyboard)
>  #endif
>  
>  	wl_array_release(&keyboard->keys);
> +	wl_array_release(&keyboard->eaten_keys);
>  	wl_list_remove(&keyboard->focus_resource_listener.link);
>  	free(keyboard);
>  }
> @@ -1307,6 +1309,26 @@ update_keymap(struct weston_seat *seat)
>  }
>  #endif
>  
> +/* remove the key from the array if it is being released,
> + * else return -1 and do nothing */
> +static int
> +remove_key(uint32_t key, enum wl_keyboard_key_state state,
> +	   struct wl_array *array)
> +{
> +	uint32_t *k, *end;
> +	end = array->data + array->size;
> +	for (k = array->data; k < end; k++) {
> +		if (*k == key) {
> +			/* Ignore server-generated repeats. */
> +			if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
> +				return -1;
> +			*k = *--end;
> +		}
> +	}
> +	array->size = (void *) end - array->data;
> +	return 0;
> +}
> +
>  WL_EXPORT void
>  notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
>  	   enum wl_keyboard_key_state state,
> @@ -1315,7 +1337,9 @@ notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
>  	struct weston_compositor *compositor = seat->compositor;
>  	struct weston_keyboard *keyboard = seat->keyboard;
>  	struct weston_keyboard_grab *grab = keyboard->grab;
> -	uint32_t *k, *end;
> +	uint32_t *k;
> +	int eaten = 0;
> +	struct wl_array *array;
>  
>  	if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
>  		weston_compositor_idle_inhibit(compositor);
> @@ -1325,32 +1349,31 @@ notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
>  		weston_compositor_idle_release(compositor);
>  	}
>  
> -	end = keyboard->keys.data + keyboard->keys.size;
> -	for (k = keyboard->keys.data; k < end; k++) {
> -		if (*k == key) {
> -			/* Ignore server-generated repeats. */
> -			if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
> -				return;
> -			*k = *--end;
> -		}
> -	}
> -	keyboard->keys.size = (void *) end - keyboard->keys.data;
> -	if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> -		k = wl_array_add(&keyboard->keys, sizeof *k);
> -		*k = key;
> -	}
> +	/* Ignore server-generated repeats, so return if remove_key()
> +	 * returns -1, signaling the key is in the array already. */
> +	if (remove_key(key, state, &keyboard->keys) == -1)
> +		return;
> +	if (remove_key(key, state, &keyboard->eaten_keys) == -1)
> +		return;
>  
>  	if (grab == &keyboard->default_grab ||
>  	    grab == &keyboard->input_method_grab) {
> -		weston_compositor_run_key_binding(compositor, seat, time, key,
> -						  state);
> +		eaten = weston_compositor_run_key_binding(compositor, seat,
> +		                                          time, key, state);
>  		grab = keyboard->grab;
>  	}
>  
> +	array = eaten == 0 ? &keyboard->keys : &keyboard->eaten_keys;
> +	if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> +		k = wl_array_add(array, sizeof *k);
> +		*k = key;
> +	}
> +
>  	grab->interface->key(grab, time, key, state);
>  
>  	if (keyboard->pending_keymap &&
> -	    keyboard->keys.size == 0)
> +	    keyboard->keys.size == 0 &&
> +	    keyboard->eaten_keys.size == 0)
>  		update_keymap(seat);
>  
>  	if (update_state == STATE_UPDATE_AUTOMATIC) {
> @@ -1430,6 +1453,11 @@ notify_keyboard_focus_out(struct weston_seat *seat)
>  		update_modifier_state(seat, serial, *k,
>  				      WL_KEYBOARD_KEY_STATE_RELEASED);
>  	}
> +	wl_array_for_each(k, &keyboard->eaten_keys) {
> +		weston_compositor_idle_release(compositor);
> +		/* No need to update the modifier state here, modifiers
> +		 * can't be in the eaten_keys array */
> +	}
>  
>  	seat->modifier_state = 0;
>  
> @@ -2083,7 +2111,8 @@ weston_seat_update_keymap(struct weston_seat *seat, struct xkb_keymap *keymap)
>  	xkb_keymap_unref(seat->keyboard->pending_keymap);
>  	seat->keyboard->pending_keymap = xkb_keymap_ref(keymap);
>  
> -	if (seat->keyboard->keys.size == 0)
> +	if (seat->keyboard->keys.size == 0 &&
> +	    seat->keyboard->eaten_keys.size == 0)
>  		update_keymap(seat);
>  #endif
>  }

Looks good enough to me, pushed.

I do wonder about notify_keyboard_focus_in(), though. That one is also
running key bindings, so should it too be filtering eaten keys?

Actually, should focus_in be running key bindings at all?


Thanks,
pq


More information about the wayland-devel mailing list