Re: [PATCH v2 1/2] Ensure keyboard modifiers are synchronized properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



looks good, ack

----- Original Message -----
> In certain circumstances, the keyboard modifiers get out-of-sync between the
> guest and the client. This is easy to reproduce with the following steps:
>  - launch virt-viewer with a guest that is not running
>  - start the guest
>  - while guest is booting, enable CAPS LOCK on the client
>  - after guest finishes booting, it will set its modifiers to a default value
>    (e.g. numlock on, capslock off)
>  - now capslock is OFF in the guest, but ON in the client
>  - toggle caps lock
>  - now capslock is ON in the guest, but OFF in the client
> 
> This moves the responsibility for synchronizing client and guest modifiers
> into
> SpiceGtkSession. It can't be handled easily within the SpiceDisplay widget
> since
> there can be multiple display widgets for each inputs channel.
> 
> A new function (spice_gtk_session_sync_keyboard_modifiers()) was added so
> that
> synchronization can be triggered manually if desired. But it also registers a
> signal handler for the InputsChannel::inputs-modifiers signal to detect when
> the
> guest has changed its modifiers. The signal handler simply overrides the
> guests
> modifiers and sets them back to the value from the client.
> ---
>  gtk/spice-gtk-session.c | 97
>  +++++++++++++++++++++++++++++++++++++++++++++++++
>  gtk/spice-gtk-session.h |  1 +
>  gtk/spice-widget.c      | 91 +---------------------------------------------
>  3 files changed, 100 insertions(+), 89 deletions(-)
> 
> diff --git a/gtk/spice-gtk-session.c b/gtk/spice-gtk-session.c
> index a9ce025..f0f7edf 100644
> --- a/gtk/spice-gtk-session.c
> +++ b/gtk/spice-gtk-session.c
> @@ -17,6 +17,22 @@
>  */
>  #include "config.h"
>  
> +#if HAVE_X11_XKBLIB_H
> +#include <X11/XKBlib.h>
> +#include <gdk/gdkx.h>
> +#endif
> +#ifdef GDK_WINDOWING_X11
> +#include <X11/Xlib.h>
> +#include <gdk/gdkx.h>
> +#endif
> +#ifdef WIN32
> +#include <windows.h>
> +#include <gdk/gdkwin32.h>
> +#ifndef MAPVK_VK_TO_VSC /* may be undefined in older mingw-headers */
> +#define MAPVK_VK_TO_VSC 0
> +#endif
> +#endif
> +
>  #include <gtk/gtk.h>
>  #include <spice/vd_agent.h>
>  #include "desktop-integration.h"
> @@ -97,6 +113,70 @@ enum {
>      PROP_AUTO_USBREDIR,
>  };
>  
> +static guint32 get_keyboard_lock_modifiers(void)
> +{
> +    guint32 modifiers = 0;
> +#if HAVE_X11_XKBLIB_H
> +    Display *x_display = NULL;
> +    XKeyboardState keyboard_state;
> +
> +    GdkScreen *screen = gdk_screen_get_default();
> +    if (!GDK_IS_X11_DISPLAY(gdk_screen_get_display(screen))) {
> +        SPICE_DEBUG("FIXME: gtk backend is not X11");
> +        return 0;
> +    }
> +
> +    x_display = GDK_SCREEN_XDISPLAY(screen);
> +    XGetKeyboardControl(x_display, &keyboard_state);
> +
> +    if (keyboard_state.led_mask & 0x01) {
> +        modifiers |= SPICE_INPUTS_CAPS_LOCK;
> +    }
> +    if (keyboard_state.led_mask & 0x02) {
> +        modifiers |= SPICE_INPUTS_NUM_LOCK;
> +    }
> +    if (keyboard_state.led_mask & 0x04) {
> +        modifiers |= SPICE_INPUTS_SCROLL_LOCK;
> +    }
> +#elif defined(win32)
> +    if (GetKeyState(VK_CAPITAL) & 1) {
> +        modifiers |= SPICE_INPUTS_CAPS_LOCK;
> +    }
> +    if (GetKeyState(VK_NUMLOCK) & 1) {
> +        modifiers |= SPICE_INPUTS_NUM_LOCK;
> +    }
> +    if (GetKeyState(VK_SCROLL) & 1) {
> +        modifiers |= SPICE_INPUTS_SCROLL_LOCK;
> +    }
> +#else
> +    g_warning("get_keyboard_lock_modifiers not implemented");
> +#endif // HAVE_X11_XKBLIB_H
> +    return modifiers;
> +}
> +
> +static void
> spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSession *self,
> +
> SpiceInputsChannel*
> inputs)
> +{
> +    gint guest_modifiers = 0, client_modifiers = 0;
> +
> +    g_return_if_fail(SPICE_IS_INPUTS_CHANNEL(inputs));
> +
> +    g_object_get(inputs, "key-modifiers", &guest_modifiers, NULL);
> +
> +    client_modifiers = get_keyboard_lock_modifiers();
> +    g_debug("%s: input:%p client_modifiers:0x%x, guest_modifiers:0x%x",
> +            G_STRFUNC, inputs, client_modifiers, guest_modifiers);
> +
> +    if (client_modifiers != guest_modifiers)
> +        spice_inputs_set_key_locks(inputs, client_modifiers);
> +}
> +
> +static void guest_modifiers_changed(SpiceInputsChannel *inputs, gpointer
> data)
> +{
> +    SpiceGtkSession *self = data;
> +    spice_gtk_session_sync_keyboard_modifiers_for_channel(self, inputs);
> +}
> +
>  static void spice_gtk_session_init(SpiceGtkSession *self)
>  {
>      SpiceGtkSessionPrivate *s;
> @@ -872,6 +952,11 @@ static void channel_new(SpiceSession *session,
> SpiceChannel *channel,
>          g_signal_connect(channel, "main-clipboard-selection-release",
>                           G_CALLBACK(clipboard_release), self);
>      }
> +    if (SPICE_IS_INPUTS_CHANNEL(channel)) {
> +        spice_g_signal_connect_object(channel, "inputs-modifiers",
> +                                      G_CALLBACK(guest_modifiers_changed),
> self, 0);
> +        spice_gtk_session_sync_keyboard_modifiers_for_channel(self,
> SPICE_INPUTS_CHANNEL(channel));
> +    }
>  }
>  
>  static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
> @@ -1029,3 +1114,15 @@ void
> spice_gtk_session_paste_from_guest(SpiceGtkSession *self)
>      s->clipboard_by_guest[selection] = TRUE;
>      s->clip_hasdata[selection] = FALSE;
>  }
> +
> +void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self)
> +{
> +    GList *l = NULL, *channels =
> spice_session_get_channels(self->priv->session);
> +
> +    for (l = channels; l != NULL; l = l->next) {
> +        if (SPICE_IS_INPUTS_CHANNEL(l->data)) {
> +            SpiceInputsChannel *inputs = SPICE_INPUTS_CHANNEL(l->data);
> +            spice_gtk_session_sync_keyboard_modifiers_for_channel(self,
> inputs);
> +        }
> +    }
> +}
> diff --git a/gtk/spice-gtk-session.h b/gtk/spice-gtk-session.h
> index 3b4eac6..fbcc353 100644
> --- a/gtk/spice-gtk-session.h
> +++ b/gtk/spice-gtk-session.h
> @@ -59,6 +59,7 @@ GType spice_gtk_session_get_type(void);
>  SpiceGtkSession *spice_gtk_session_get(SpiceSession *session);
>  void spice_gtk_session_copy_to_guest(SpiceGtkSession *self);
>  void spice_gtk_session_paste_from_guest(SpiceGtkSession *self);
> +void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self);
>  
>  G_END_DECLS
>  
> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
> index 0e4a0de..2044513 100644
> --- a/gtk/spice-widget.c
> +++ b/gtk/spice-widget.c
> @@ -131,7 +131,6 @@ static void try_mouse_ungrab(SpiceDisplay *display);
>  static void recalc_geometry(GtkWidget *widget);
>  static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer
>  data);
>  static void channel_destroy(SpiceSession *s, SpiceChannel *channel, gpointer
>  data);
> -static void sync_keyboard_lock_modifiers(SpiceDisplay *display);
>  static void cursor_invalidate(SpiceDisplay *display);
>  static void update_area(SpiceDisplay *display, gint x, gint y, gint width,
>  gint height);
>  static void release_keys(SpiceDisplay *display);
> @@ -1457,7 +1456,8 @@ static gboolean focus_in_event(GtkWidget *widget,
> GdkEventFocus *focus G_GNUC_UN
>          return true;
>  
>      release_keys(display);
> -    sync_keyboard_lock_modifiers(display);
> +    if (!d->disable_inputs)
> +        spice_gtk_session_sync_keyboard_modifiers(d->gtk_session);
>      update_keyboard_focus(display, true);
>      try_keyboard_grab(display);
>      update_display(display);
> @@ -2411,7 +2411,6 @@ static void channel_new(SpiceSession *s, SpiceChannel
> *channel, gpointer data)
>      if (SPICE_IS_INPUTS_CHANNEL(channel)) {
>          d->inputs = SPICE_INPUTS_CHANNEL(channel);
>          spice_channel_connect(channel);
> -        sync_keyboard_lock_modifiers(display);
>          return;
>      }
>  
> @@ -2600,89 +2599,3 @@ GdkPixbuf *spice_display_get_pixbuf(SpiceDisplay
> *display)
>      return pixbuf;
>  }
>  
> -#if HAVE_X11_XKBLIB_H
> -static guint32 get_keyboard_lock_modifiers(Display *x_display)
> -{
> -    XKeyboardState keyboard_state;
> -    guint32 modifiers = 0;
> -
> -    XGetKeyboardControl(x_display, &keyboard_state);
> -
> -    if (keyboard_state.led_mask & 0x01) {
> -        modifiers |= SPICE_INPUTS_CAPS_LOCK;
> -    }
> -    if (keyboard_state.led_mask & 0x02) {
> -        modifiers |= SPICE_INPUTS_NUM_LOCK;
> -    }
> -    if (keyboard_state.led_mask & 0x04) {
> -        modifiers |= SPICE_INPUTS_SCROLL_LOCK;
> -    }
> -    return modifiers;
> -}
> -
> -static void sync_keyboard_lock_modifiers(SpiceDisplay *display)
> -{
> -    Display *x_display;
> -    SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
> -    guint32 modifiers;
> -    GdkWindow *w;
> -
> -    if (d->disable_inputs)
> -        return;
> -
> -    w = gtk_widget_get_parent_window(GTK_WIDGET(display));
> -    if (w == NULL) /* it can happen if the display is not yet shown */
> -        return;
> -
> -    if (!GDK_IS_X11_DISPLAY(gdk_window_get_display(w))) {
> -        SPICE_DEBUG("FIXME: gtk backend is not X11");
> -        return;
> -    }
> -
> -    x_display = GDK_WINDOW_XDISPLAY(w);
> -    modifiers = get_keyboard_lock_modifiers(x_display);
> -    if (d->inputs)
> -        spice_inputs_set_key_locks(d->inputs, modifiers);
> -}
> -
> -#elif defined (WIN32)
> -static guint32 get_keyboard_lock_modifiers(void)
> -{
> -    guint32 modifiers = 0;
> -
> -    if (GetKeyState(VK_CAPITAL) & 1) {
> -        modifiers |= SPICE_INPUTS_CAPS_LOCK;
> -    }
> -    if (GetKeyState(VK_NUMLOCK) & 1) {
> -        modifiers |= SPICE_INPUTS_NUM_LOCK;
> -    }
> -    if (GetKeyState(VK_SCROLL) & 1) {
> -        modifiers |= SPICE_INPUTS_SCROLL_LOCK;
> -    }
> -
> -    return modifiers;
> -}
> -
> -static void sync_keyboard_lock_modifiers(SpiceDisplay *display)
> -{
> -    SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
> -    guint32 modifiers;
> -    GdkWindow *w;
> -
> -    if (d->disable_inputs)
> -        return;
> -
> -    w = gtk_widget_get_parent_window(GTK_WIDGET(display));
> -    if (w == NULL) /* it can happen if the display is not yet shown */
> -        return;
> -
> -    modifiers = get_keyboard_lock_modifiers();
> -    if (d->inputs)
> -        spice_inputs_set_key_locks(d->inputs, modifiers);
> -}
> -#else
> -static void sync_keyboard_lock_modifiers(SpiceDisplay *display)
> -{
> -    g_warning("sync_keyboard_lock_modifiers not implemented");
> -}
> -#endif // HAVE_X11_XKBLIB_H
> --
> 1.9.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]