<div dir="ltr"><div><div>Hi, I just noticed spice_gtk_session_sync_keyboard_modifiers() is not actually exported, and not documented etc. </div>Imho, it's not really helping to put in public API, I would rather move declaration to spice-gtk-session-priv.h. </div><div>What do you think? </div></div><div class="gmail_extra"> <div class="gmail_quote">On Thu, Apr 3, 2014 at 12:38 AM, Marc-André Lureau <<a href="mailto:mlureau@redhat.com" target="_blank">mlureau@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">looks good, ack <div class="HOEnZb"><div class="h5"> ----- 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 > <a href="mailto:Spice-devel@lists.freedesktop.org">Spice-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/spice-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/spice-devel</a> > _______________________________________________ Spice-devel mailing list <a href="mailto:Spice-devel@lists.freedesktop.org">Spice-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/spice-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/spice-devel</a> </div></div></blockquote></div> -- Marc-André Lureau </div>