[Spice-devel] [PATCH v2 2/2] Use GdkKeymap to listen for keyboard modifier changes

[Christophe Fergeau]

Hey,

On Wed, Apr 02, 2014 at 05:02:50PM -0500, Jonathon Jongsma wrote:
> Connect to the GdkKeymap::state-changed signal to detect when the client
> keyboard modifiers have changed. This keeps the client and the guest in sync
> even when the SpiceDisplay widget isn't focused. New values are only sent down
> to the guest if the new value is different than the current value.

This change is a bit racy when the SpiceDisplay widget has focus and the
user presses caps lock (for example):
- a "key down" message is sent
- a "key modifiers" message is sent
- a "key up" message is sent

When the user presses caps lock again, these messages get sent (in a
different order than previously for some reason):
- a "key down" message is sent
- a "key up" message is sent
- a "key modifiers" message is sent

QEMU-side, in the latter case, this causes these events:
- a "key down" event is sent to the guest
- a "key up" event is sent to the guest
- the led state is still the old one, so spice-server tries to change
  it. It does it by sending fake key presses, which means it synthetizes
  a "key down"/"key up" pair and send that to the guest

All this means the caps lock state changes from On to Off, and then
again from Off to On. When the client is notified that the guest caps
lock led is On, it sends once more a "key modifiers" message, which will
send one more "key down"/"key up" event pair to the guest, finally
setting the led to the expected state.

In the end, the state is the one we would expect, but 3 caps lock
presses could be observed on the guest while the user only pressed it
once.

Arguably, this is not a bug as in the end everything is fine, but we
might want to limit the automatic synchronization to cases when the
widget does not have focus, or even revert this patch and only sync
when the widget gets focused.


> ---
>  gtk/spice-gtk-session.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
> index f0f7edf..fe77f47 100644
> --- a/gtk/spice-gtk-session.c
> +++ b/gtk/spice-gtk-session.c
> @@ -171,6 +171,12 @@ static void spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSessio
>          spice_inputs_set_key_locks(inputs, client_modifiers);
>  }
>  
> +static void keymap_modifiers_changed(GdkKeymap *keymap, gpointer data)
> +{
> +    SpiceGtkSession *self = data;
> +    spice_gtk_session_sync_keyboard_modifiers(self);
> +}
> +
>  static void guest_modifiers_changed(SpiceInputsChannel *inputs, gpointer data)
>  {
>      SpiceGtkSession *self = data;
> @@ -180,6 +186,7 @@ static void guest_modifiers_changed(SpiceInputsChannel *inputs, gpointer data)
>  static void spice_gtk_session_init(SpiceGtkSession *self)
>  {
>      SpiceGtkSessionPrivate *s;
> +    GdkKeymap *keymap = gdk_keymap_get_default();
>  
>      s = self->priv = SPICE_GTK_SESSION_GET_PRIVATE(self);
>  
> @@ -189,6 +196,8 @@ static void spice_gtk_session_init(SpiceGtkSession *self)
>      s->clipboard_primary = gtk_clipboard_get(GDK_SELECTION_PRIMARY);
>      g_signal_connect(G_OBJECT(s->clipboard_primary), "owner-change",
>                       G_CALLBACK(clipboard_owner_change), self);
> +    spice_g_signal_connect_object(keymap, "state-changed",
> +                                  G_CALLBACK(keymap_modifiers_changed), self, 0);
>  }
>  
>  static GObject *
> -- 
> 1.9.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160307/3747328c/attachment.sig>