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

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

 



On Wed, 2014-08-13 at 11:08 +0200, Marc-André Lureau wrote:
> Hi,
> 
> I just noticed spice_gtk_session_sync_keyboard_modifiers() is not
> actually exported, and not documented etc.
> 
> 
> Imho, it's not really helping to put in public API, I would rather
> move declaration to spice-gtk-session-priv.h.
> 
> 
> What do you think?


Hm, you're right.  Let's move it to the private header.


> 
> 
> 
> On Thu, Apr 3, 2014 at 12:38 AM, Marc-André Lureau
> <mlureau@xxxxxxxxxx> wrote:
>         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
>         
> 
> 
> 
> -- 
> Marc-André Lureau


_______________________________________________
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]