[Spice-devel] [PATCH spice-gtk] RFC: widget: improve undesired key repeatition

Marc-André Lureau marcandre.lureau at gmail.com
Wed Apr 18 18:17:15 PDT 2012


The remote end receives key press and key release seperately, but this
can cause extra key repetition between the two event messages:
- if the network is slow
- if the server is busy and doesn't handle the events quickly enough
- if the client is slow or busy

(I think the guest/os couldn't be responsible, otherwise any computer
would be affected when it hangs a little, although I might be wrong, it
could be the events are queued and software properly check state before
repeating)

The client should send a single message MsgcDownUp instead.
This patch prepares for this change, and serves for discussion before
addition of new message/capability:
- delay non-modifier key press event until the event is repeated
- send extra Down & Up event when the key is released

The minor downside of this approach is that it accumulates the initial delay
before repeat. If locally and remotely, we have 400ms before repeat, the
actual repeat on remote will happen after 800ms. There is no easy
solution for that, we could lower the initial delay to decide between
DownUp and Down, probably 100ms would be enough. It could
enable this distinction for remote servers only. Finally the remote
repeat delay could be tweaked via the agent.

This improves the situation for me with remote servers, I no longer get
spurious key repeat, although it might still happen, but less often.
Depending on feeback, I'll follow up with MsgcDownUp addition.
---
 gtk/spice-widget-priv.h |    1 +
 gtk/spice-widget.c      |   59 ++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/gtk/spice-widget-priv.h b/gtk/spice-widget-priv.h
index 898d86c..dfc9010 100644
--- a/gtk/spice-widget-priv.h
+++ b/gtk/spice-widget-priv.h
@@ -101,6 +101,7 @@ struct _SpiceDisplayPrivate {
     const guint16 const     *keycode_map;
     size_t                  keycode_maplen;
     uint32_t                key_state[512 / 32];
+    int                     key_delayed_scancode;
     SpiceGrabSequence         *grabseq; /* the configured key sequence */
     gboolean                *activeseq; /* the currently pressed keys */
     gint                    mark;
diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index d355f09..9403513 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -888,8 +888,12 @@ static gboolean expose_event(GtkWidget *widget, GdkEventExpose *expose)
 #endif
 
 /* ---------------------------------------------------------------- */
+typedef enum {
+    SEND_KEY_PRESS,
+    SEND_KEY_RELEASE,
+} SendKeyType;
 
-static void send_key(SpiceDisplay *display, int scancode, int down)
+static void send_key(SpiceDisplay *display, int scancode, SendKeyType type, gboolean press_delayed)
 {
     SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
     uint32_t i, b, m;
@@ -905,15 +909,43 @@ static void send_key(SpiceDisplay *display, int scancode, int down)
     m = (1 << b);
     g_return_if_fail(i < SPICE_N_ELEMENTS(d->key_state));
 
-    if (down) {
-        spice_inputs_key_press(d->inputs, scancode);
+    switch (type) {
+    case SEND_KEY_PRESS:
+        if (d->key_delayed_scancode) {
+            /* ensure delayed key is pressed before any new input event */
+            spice_inputs_key_press(d->inputs, d->key_delayed_scancode);
+            d->key_delayed_scancode = 0;
+        }
+
+        /* let the guest handle key repeatition */
+        if (d->key_state[i] & m)
+            break;
+
+        if (press_delayed)
+            d->key_delayed_scancode = scancode;
+        else
+            spice_inputs_key_press(d->inputs, scancode);
+
         d->key_state[i] |= m;
-    } else {
-        if (!(d->key_state[i] & m)) {
-            return;
+        break;
+
+    case SEND_KEY_RELEASE:
+        if (!(d->key_state[i] & m))
+            break;
+
+        if (d->key_delayed_scancode) {
+            /* FIXME: should send a single PRESS & RELEASE message
+             if key_delayed_scancode == scancode */
+            spice_inputs_key_press(d->inputs, d->key_delayed_scancode);
+            d->key_delayed_scancode = 0;
         }
+
         spice_inputs_key_release(d->inputs, scancode);
         d->key_state[i] &= ~m;
+        break;
+
+    default:
+        g_warn_if_reached();
     }
 }
 
@@ -928,7 +960,7 @@ static void release_keys(SpiceDisplay *display)
             continue;
         }
         for (b = 0; b < 32; b++) {
-            send_key(display, i * 32 + b, 0);
+            send_key(display, i * 32 + b, SEND_KEY_RELEASE, FALSE);
         }
     }
 }
@@ -966,9 +998,10 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey *key)
     SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
     int scancode;
 
-    SPICE_DEBUG("%s %s: keycode: %d  state: %d  group %d",
+
+    SPICE_DEBUG("%s %s: keycode: %d  state: %d  group %d modifier %d",
             __FUNCTION__, key->type == GDK_KEY_PRESS ? "press" : "release",
-            key->hardware_keycode, key->state, key->group);
+            key->hardware_keycode, key->state, key->group, key->is_modifier);
 
     if (check_for_grab_key(display, key->type, key->keyval)) {
         g_signal_emit(widget, signals[SPICE_DISPLAY_GRAB_KEY_PRESSED], 0);
@@ -990,10 +1023,10 @@ static gboolean key_event(GtkWidget *widget, GdkEventKey *key)
                                             key->hardware_keycode);
     switch (key->type) {
     case GDK_KEY_PRESS:
-        send_key(display, scancode, 1);
+        send_key(display, scancode, SEND_KEY_PRESS, !key->is_modifier);
         break;
     case GDK_KEY_RELEASE:
-        send_key(display, scancode, 0);
+        send_key(display, scancode, SEND_KEY_RELEASE, !key->is_modifier);
         break;
     default:
         break;
@@ -1031,12 +1064,12 @@ void spice_display_send_keys(SpiceDisplay *display, const guint *keyvals,
 
     if (kind & SPICE_DISPLAY_KEY_EVENT_PRESS) {
         for (i = 0 ; i < nkeyvals ; i++)
-            send_key(display, get_scancode_from_keyval(display, keyvals[i]), 1);
+            send_key(display, get_scancode_from_keyval(display, keyvals[i]), SEND_KEY_PRESS, FALSE);
     }
 
     if (kind & SPICE_DISPLAY_KEY_EVENT_RELEASE) {
         for (i = (nkeyvals-1) ; i >= 0 ; i--)
-            send_key(display, get_scancode_from_keyval(display, keyvals[i]), 0);
+            send_key(display, get_scancode_from_keyval(display, keyvals[i]), SEND_KEY_RELEASE, FALSE);
     }
 }
 
-- 
1.7.10



More information about the Spice-devel mailing list