[Spice-devel] [PATCH v2 1/2] Ensure keyboard modifiers are synchronized properly
Jonathon Jongsma
jjongsma at redhat.com
Wed Aug 13 07:08:19 PDT 2014
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 at redhat.com> 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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>
>
> --
> Marc-André Lureau
More information about the Spice-devel
mailing list