[weston,v2] input: send focus events to the focused client when running a key binding

Submitted by Giulio Camuffo on Nov. 22, 2014, 9:16 a.m.

Details

Message ID 1416647816-17575-1-git-send-email-giuliocamuffo@gmail.com
State Accepted
Commit a20ca811f9f6616424e743a2c45d963ee683e36f
Headers show

Not browsing as part of any series.

Commit Message

Giulio Camuffo Nov. 22, 2014, 9:16 a.m.
When running a key binding we don't send the key press to the client
via the wl_keyboard.key event. Instead, send a wl_keyboard.leave/enter
pair so that the client knows the actual state of the keyboard.
---
 src/bindings.c | 23 ++++++++++++++++++++---
 src/input.c    |  3 +--
 2 files changed, 21 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/bindings.c b/src/bindings.c
index 5e24725..5cb031a 100644
--- a/src/bindings.c
+++ b/src/bindings.c
@@ -203,7 +203,8 @@  binding_key(struct weston_keyboard_grab *grab,
 				keyboard->grab = &keyboard->input_method_grab;
 			free(b);
 		}
-	} else if (!wl_list_empty(&keyboard->focus_resource_list)) {
+	}
+	if (!wl_list_empty(&keyboard->focus_resource_list)) {
 		serial = wl_display_next_serial(display);
 		wl_resource_for_each(resource, &keyboard->focus_resource_list) {
 			wl_keyboard_send_key(resource,
@@ -245,7 +246,8 @@  static const struct weston_keyboard_grab_interface binding_grab = {
 };
 
 static void
-install_binding_grab(struct weston_seat *seat, uint32_t time, uint32_t key)
+install_binding_grab(struct weston_seat *seat, uint32_t time, uint32_t key,
+                     struct weston_surface *focus)
 {
 	struct binding_keyboard_grab *grab;
 
@@ -253,6 +255,19 @@  install_binding_grab(struct weston_seat *seat, uint32_t time, uint32_t key)
 	grab->key = key;
 	grab->grab.interface = &binding_grab;
 	weston_keyboard_start_grab(seat->keyboard, &grab->grab);
+
+	/* Notify the surface which had the focus before this binding
+	 * triggered that we stole a keypress from under it, by forcing
+	 * a wl_keyboard leave/enter pair. The enter event will contain
+	 * the pressed key in the keys array, so the client will know
+	 * the exact state of the keyboard.
+	 * If the old focus surface is different than the new one it
+	 * means it was changed in the binding handler, so it received
+	 * the enter event already. */
+	if (focus && seat->keyboard->focus == focus) {
+		weston_keyboard_set_focus(seat->keyboard, NULL);
+		weston_keyboard_set_focus(seat->keyboard, focus);
+	}
 }
 
 WL_EXPORT void
@@ -262,6 +277,7 @@  weston_compositor_run_key_binding(struct weston_compositor *compositor,
 				  enum wl_keyboard_key_state state)
 {
 	struct weston_binding *b, *tmp;
+	struct weston_surface *focus;
 
 	if (state == WL_KEYBOARD_KEY_STATE_RELEASED)
 		return;
@@ -273,6 +289,7 @@  weston_compositor_run_key_binding(struct weston_compositor *compositor,
 	wl_list_for_each_safe(b, tmp, &compositor->key_binding_list, link) {
 		if (b->key == key && b->modifier == seat->modifier_state) {
 			weston_key_binding_handler_t handler = b->handler;
+			focus = seat->keyboard->focus;
 			handler(seat, time, key, b->data);
 
 			/* If this was a key binding and it didn't
@@ -280,7 +297,7 @@  weston_compositor_run_key_binding(struct weston_compositor *compositor,
 			 * swallow the key release. */
 			if (seat->keyboard->grab ==
 			    &seat->keyboard->default_grab)
-				install_binding_grab(seat, time, key);
+				install_binding_grab(seat, time, key, focus);
 		}
 	}
 }
diff --git a/src/input.c b/src/input.c
index 15ff6ed..093d781 100644
--- a/src/input.c
+++ b/src/input.c
@@ -753,14 +753,13 @@  weston_keyboard_set_focus(struct weston_keyboard *keyboard,
 	wl_signal_emit(&keyboard->focus_signal, keyboard);
 }
 
+/* Users of this function must manually manage the keyboard focus */
 WL_EXPORT void
 weston_keyboard_start_grab(struct weston_keyboard *keyboard,
 			   struct weston_keyboard_grab *grab)
 {
 	keyboard->grab = grab;
 	grab->keyboard = keyboard;
-
-	/* XXX focus? */
 }
 
 WL_EXPORT void

Comments

On Sat, 22 Nov 2014 11:16:56 +0200
Giulio Camuffo <giuliocamuffo@gmail.com> wrote:

> When running a key binding we don't send the key press to the client
> via the wl_keyboard.key event. Instead, send a wl_keyboard.leave/enter
> pair so that the client knows the actual state of the keyboard.
> ---
>  src/bindings.c | 23 ++++++++++++++++++++---
>  src/input.c    |  3 +--
>  2 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/src/bindings.c b/src/bindings.c
> index 5e24725..5cb031a 100644
> --- a/src/bindings.c
> +++ b/src/bindings.c
> @@ -203,7 +203,8 @@ binding_key(struct weston_keyboard_grab *grab,
>  				keyboard->grab = &keyboard->input_method_grab;
>  			free(b);
>  		}
> -	} else if (!wl_list_empty(&keyboard->focus_resource_list)) {
> +	}
> +	if (!wl_list_empty(&keyboard->focus_resource_list)) {
>  		serial = wl_display_next_serial(display);
>  		wl_resource_for_each(resource, &keyboard->focus_resource_list) {
>  			wl_keyboard_send_key(resource,
> @@ -245,7 +246,8 @@ static const struct weston_keyboard_grab_interface binding_grab = {
>  };
>  
>  static void
> -install_binding_grab(struct weston_seat *seat, uint32_t time, uint32_t key)
> +install_binding_grab(struct weston_seat *seat, uint32_t time, uint32_t key,
> +                     struct weston_surface *focus)
>  {
>  	struct binding_keyboard_grab *grab;
>  
> @@ -253,6 +255,19 @@ install_binding_grab(struct weston_seat *seat, uint32_t time, uint32_t key)
>  	grab->key = key;
>  	grab->grab.interface = &binding_grab;
>  	weston_keyboard_start_grab(seat->keyboard, &grab->grab);
> +
> +	/* Notify the surface which had the focus before this binding
> +	 * triggered that we stole a keypress from under it, by forcing
> +	 * a wl_keyboard leave/enter pair. The enter event will contain
> +	 * the pressed key in the keys array, so the client will know
> +	 * the exact state of the keyboard.
> +	 * If the old focus surface is different than the new one it
> +	 * means it was changed in the binding handler, so it received
> +	 * the enter event already. */
> +	if (focus && seat->keyboard->focus == focus) {
> +		weston_keyboard_set_focus(seat->keyboard, NULL);
> +		weston_keyboard_set_focus(seat->keyboard, focus);
> +	}
>  }
>  
>  WL_EXPORT void
> @@ -262,6 +277,7 @@ weston_compositor_run_key_binding(struct weston_compositor *compositor,
>  				  enum wl_keyboard_key_state state)
>  {
>  	struct weston_binding *b, *tmp;
> +	struct weston_surface *focus;
>  
>  	if (state == WL_KEYBOARD_KEY_STATE_RELEASED)
>  		return;
> @@ -273,6 +289,7 @@ weston_compositor_run_key_binding(struct weston_compositor *compositor,
>  	wl_list_for_each_safe(b, tmp, &compositor->key_binding_list, link) {
>  		if (b->key == key && b->modifier == seat->modifier_state) {
>  			weston_key_binding_handler_t handler = b->handler;
> +			focus = seat->keyboard->focus;
>  			handler(seat, time, key, b->data);
>  
>  			/* If this was a key binding and it didn't
> @@ -280,7 +297,7 @@ weston_compositor_run_key_binding(struct weston_compositor *compositor,
>  			 * swallow the key release. */

I think the old comment about swallowing is now wrong, because the
release will be sent a real key event.

>  			if (seat->keyboard->grab ==
>  			    &seat->keyboard->default_grab)
> -				install_binding_grab(seat, time, key);
> +				install_binding_grab(seat, time, key, focus);
>  		}
>  	}
>  }
> diff --git a/src/input.c b/src/input.c
> index 15ff6ed..093d781 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -753,14 +753,13 @@ weston_keyboard_set_focus(struct weston_keyboard *keyboard,
>  	wl_signal_emit(&keyboard->focus_signal, keyboard);
>  }
>  
> +/* Users of this function must manually manage the keyboard focus */

I think debug_binding() in shell.c is missing the focus management? Or
at least it's not obvious to me. It does not seem to cause any keyboard
enter/leave events.

What about input_method_context_grab_keyboard()? I'm not sure how that
works to begin with.

>  WL_EXPORT void
>  weston_keyboard_start_grab(struct weston_keyboard *keyboard,
>  			   struct weston_keyboard_grab *grab)
>  {
>  	keyboard->grab = grab;
>  	grab->keyboard = keyboard;
> -
> -	/* XXX focus? */
>  }
>  
>  WL_EXPORT void

Daniel gave his R-b in IRC, so I pushed this. We can do follow-ups if
there's more.

Hm, only after pushing I realized the commit message 


Thanks,
pq