[Spice-devel] [spice-gtk] RFC Sync only on focus change

Frediano Ziglio fziglio at redhat.com
Fri May 20 14:03:16 UTC 2016


Limit the virtual keystrokes sent to the remote machine.
The modifiers are synced only when the application receive or lose
the focus. This reduce a lot the possible virtual keystrokes sent
to the guest to synchronize the modifiers.
This affect the situations where modifiers are configured
differently in client and guest.
When the application receive the focus the synchronization is
attempted from client to guest while when the application lose
focus is attempted guest to client (basically is moved following
user moving).

This patch is actually not complete but more an RFC:
- only X11 is currently supported and there are some
  embedded constants which should be changed even for X11
  (from commit 063c1b9c0627c87eb7f5369c4a6b9776a22e5c7d);
  I think to move the function in a separate file (you will
  understand the reason with Windows...);
- what happen with multimonitors? I don't think this patch
  it's causing regressions anyway;
- instead of calling spice_inputs_set_key_locks to send
  a message to spice-server which will insert the keystrokes
  based on spice-server knowledge of the modifiers use
  client knowledge, this will also help when some more knowledge
  of the guest status will be implemented in the guest;
- there are some possible changes in behavior for
  keymap_modifiers_changed;
- one possible regression is that if you are using virt-viewer
  and the guest is booted it's possible the boot process will switch
  modifiers status. Honestly I consider this more of an improvement
  than a regression.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 src/spice-gtk-session-priv.h |   2 +-
 src/spice-gtk-session.c      | 104 +++++++++++++++++++++++++++++++++++++++----
 src/spice-widget.c           |   5 ++-
 3 files changed, 101 insertions(+), 10 deletions(-)

diff --git a/src/spice-gtk-session-priv.h b/src/spice-gtk-session-priv.h
index d7fe313..abf90d6 100644
--- a/src/spice-gtk-session-priv.h
+++ b/src/spice-gtk-session-priv.h
@@ -38,13 +38,13 @@ struct _SpiceGtkSessionClass
 void spice_gtk_session_request_auto_usbredir(SpiceGtkSession *self,
                                              gboolean state);
 gboolean spice_gtk_session_get_read_only(SpiceGtkSession *self);
-void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self);
 void spice_gtk_session_set_pointer_grabbed(SpiceGtkSession *self, gboolean grabbed);
 gboolean spice_gtk_session_get_pointer_grabbed(SpiceGtkSession *self);
 void spice_gtk_session_set_keyboard_has_focus(SpiceGtkSession *self, gboolean keyboard_has_focus);
 void spice_gtk_session_set_mouse_has_pointer(SpiceGtkSession *self, gboolean  mouse_has_pointer);
 gboolean spice_gtk_session_get_keyboard_has_focus(SpiceGtkSession *self);
 gboolean spice_gtk_session_get_mouse_has_pointer(SpiceGtkSession *self);
+void spice_gtk_session_set_focus(SpiceGtkSession *self, gboolean focus);
 
 G_END_DECLS
 
diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
index bbcbeeb..476098e 100644
--- a/src/spice-gtk-session.c
+++ b/src/spice-gtk-session.c
@@ -66,6 +66,8 @@ struct _SpiceGtkSessionPrivate {
     gboolean                keyboard_has_focus;
     gboolean                mouse_has_pointer;
     gboolean                sync_modifiers;
+    /** the session has a window with the focus */
+    gboolean                has_focus;
 };
 
 /**
@@ -103,6 +105,13 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
 static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
                             gpointer user_data);
 static gboolean read_only(SpiceGtkSession *self);
+typedef enum {
+    from_guest_to_client,
+    from_client_to_guest
+} keyboard_sync_dir;
+static void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self,
+                                                      gboolean force,
+                                                      keyboard_sync_dir dir);
 
 /* ------------------------------------------------------------------ */
 /* gobject glue                                                       */
@@ -179,9 +188,45 @@ static guint32 get_keyboard_lock_modifiers(void)
     return modifiers;
 }
 
+static void set_keyboard_lock_modifiers(guint32 modifiers)
+{
+#ifdef HAVE_X11_XKBLIB_H
+    // TODO fixed ??
+    enum {
+        XKB_CAPS_LOCK = 2,
+        XKB_NUM_LOCK = 0x10,
+        XKB_SCROLL_LOCK = 0 // TODO
+    };
+    Display *x_display = NULL;
+    int x_modifiers = 0;
+
+    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;
+    }
+
+    x_display = GDK_SCREEN_XDISPLAY(screen);
+
+    if ((modifiers & SPICE_INPUTS_CAPS_LOCK) != 0) {
+        x_modifiers |= XKB_CAPS_LOCK;
+    }
+    if ((modifiers & SPICE_INPUTS_NUM_LOCK) != 0) {
+        x_modifiers |= XKB_NUM_LOCK;
+    }
+    if ((modifiers & SPICE_INPUTS_SCROLL_LOCK) != 0) {
+        x_modifiers |= XKB_SCROLL_LOCK;
+    }
+    XkbLockModifiers(x_display, XkbUseCoreKbd, XKB_CAPS_LOCK|XKB_NUM_LOCK|XKB_SCROLL_LOCK, x_modifiers);
+#else
+    g_warning("set_keyboard_lock_modifiers not implemented");
+#endif // HAVE_X11_XKBLIB_H
+}
+
 static void spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSession *self,
                                                                   SpiceInputsChannel* inputs,
-                                                                  gboolean force)
+                                                                  gboolean force,
+                                                                  keyboard_sync_dir dir)
 {
     gint guest_modifiers = 0, client_modifiers = 0;
 
@@ -192,28 +237,43 @@ static void spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSessio
         return;
     }
 
+    // if client synchronization is enabled this is done
+    // by widget
+    // TODO check if possible to do client sync
+    if (!force) {
+        return;
+    }
+
     g_object_get(inputs, "key-modifiers", &guest_modifiers, NULL);
     client_modifiers = get_keyboard_lock_modifiers();
 
-    if (force || client_modifiers != guest_modifiers) {
+    if (!force && client_modifiers == guest_modifiers) {
+        return;
+    }
+
+    if (dir == from_client_to_guest) {
         CHANNEL_DEBUG(inputs, "client_modifiers:0x%x, guest_modifiers:0x%x",
                       client_modifiers, guest_modifiers);
         spice_inputs_set_key_locks(inputs, client_modifiers);
+    } else {
+        set_keyboard_lock_modifiers(guest_modifiers);
     }
 }
 
+/* Keyboard state changed in the client, try to sync
+ */
 static void keymap_modifiers_changed(GdkKeymap *keymap, gpointer data)
 {
     SpiceGtkSession *self = data;
 
-    spice_gtk_session_sync_keyboard_modifiers(self);
+    spice_gtk_session_sync_keyboard_modifiers(self, FALSE, from_client_to_guest);
 }
 
 static void guest_modifiers_changed(SpiceInputsChannel *inputs, gpointer data)
 {
     SpiceGtkSession *self = data;
 
-    spice_gtk_session_sync_keyboard_modifiers_for_channel(self, inputs, FALSE);
+    spice_gtk_session_sync_keyboard_modifiers_for_channel(self, inputs, FALSE, from_client_to_guest);
 }
 
 static void spice_gtk_session_init(SpiceGtkSession *self)
@@ -1066,7 +1126,8 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
     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), TRUE);
+        spice_gtk_session_sync_keyboard_modifiers_for_channel(self, SPICE_INPUTS_CHANNEL(channel), TRUE,
+                                                              from_client_to_guest);
     }
 }
 
@@ -1226,15 +1287,16 @@ void spice_gtk_session_paste_from_guest(SpiceGtkSession *self)
     s->clip_hasdata[selection] = FALSE;
 }
 
-G_GNUC_INTERNAL
-void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self)
+static void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self,
+                                                      gboolean force,
+                                                      keyboard_sync_dir dir)
 {
     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, TRUE);
+            spice_gtk_session_sync_keyboard_modifiers_for_channel(self, inputs, force, dir);
         }
     }
     g_list_free(channels);
@@ -1250,6 +1312,32 @@ void spice_gtk_session_set_pointer_grabbed(SpiceGtkSession *self, gboolean grabb
 }
 
 G_GNUC_INTERNAL
+void spice_gtk_session_set_focus(SpiceGtkSession *self, gboolean focus)
+{
+    g_return_if_fail(SPICE_IS_GTK_SESSION(self));
+
+    /* TODO detect switch between monitors */
+
+    /* not changed ? */
+    if (self->priv->has_focus == focus)
+        return;
+
+    if (focus) {
+        /* User switched to the viewer, try to syncronize the keyboard
+         * modifiers.
+         * This should be called before setting has_focus
+         */
+        spice_gtk_session_sync_keyboard_modifiers(self, TRUE, from_client_to_guest);
+    } else {
+        /* User swicthed to another application, syncronize the client
+         * keyboard */
+        spice_gtk_session_sync_keyboard_modifiers(self, TRUE, from_guest_to_client);
+    }
+
+    self->priv->has_focus = focus;
+}
+
+G_GNUC_INTERNAL
 gboolean spice_gtk_session_get_pointer_grabbed(SpiceGtkSession *self)
 {
     g_return_val_if_fail(SPICE_IS_GTK_SESSION(self), FALSE);
diff --git a/src/spice-widget.c b/src/spice-widget.c
index cbca5dc..07c0f79 100644
--- a/src/spice-widget.c
+++ b/src/spice-widget.c
@@ -1681,7 +1681,7 @@ static gboolean focus_in_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_UN
 
     release_keys(display);
     if (!d->disable_inputs)
-        spice_gtk_session_sync_keyboard_modifiers(d->gtk_session);
+        spice_gtk_session_set_focus(d->gtk_session, TRUE);
     if (d->keyboard_grab_released)
         memset(d->activeseq, 0, sizeof(gboolean) * d->grabseq->nkeysyms);
     update_keyboard_focus(display, true);
@@ -1708,6 +1708,9 @@ static gboolean focus_out_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_U
     if (d->keyboard_grab_active)
         return true;
 
+    if (!d->disable_inputs)
+        spice_gtk_session_set_focus(d->gtk_session, FALSE);
+
     release_keys(display);
     update_keyboard_focus(display, false);
 
-- 
2.7.4



More information about the Spice-devel mailing list