[Spice-devel] [PATCH spice-gtk 5/5] widget: differentiate key press & release from press only events

[Marc-André Lureau]

Until now, Spice clients only sent separate key events for press and
release. But this may result in unwanted key repetition from guest VM
side. It seems OSes have various implementations. While MS Windows
relies on hardware key repeats (which are several sequential press
events), otoh, X11 uses software key repeat (although not Linux
keyboard/VT by default).

We can't easily disable guest side repeaters, as it may be enforced by
other components (a X11 client can adjust each key individually, or
the desktop settings may change it etc.). Neither can we rely only on
guest software repeater as Windows doesn't seem to have one by
default, so we need to keep sending multiple press events as of today.

It seems a nice way to improve the situation is to send a single
"press&release" key event when the user released the key within a
short delay. If the key is pressed for longer, we keep the existing
behaviour which has been working pretty well so far, sending separate
"press", then repeatedly "press", and an ending "release" event.

v2:
- fix some commit message spelling spotted by Alon & Christophe
- simplify a bit the timer handling code after Hans review
- remove the submodule change (will be updated in earler patch once
  pushed upstream)
---
 gtk/spice-widget-priv.h |    2 ++
 gtk/spice-widget.c      |   76 +++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/gtk/spice-widget-priv.h b/gtk/spice-widget-priv.h
index 97c6489..87d1de9 100644
--- a/gtk/spice-widget-priv.h
+++ b/gtk/spice-widget-priv.h
@@ -106,6 +106,8 @@ struct _SpiceDisplayPrivate {
     const guint16 const     *keycode_map;
     size_t                  keycode_maplen;
     uint32_t                key_state[512 / 32];
+    int                     key_delayed_scancode;
+    guint                   key_delayed_id;
     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 014fdaa..5fe36de 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -393,6 +393,11 @@ static void spice_display_dispose(GObject *obj)
     g_clear_object(&d->session);
     d->gtk_session = NULL;
 
+    if (d->key_delayed_id) {
+        g_source_remove(d->key_delayed_id);
+        d->key_delayed_id = 0;
+    }
+
     G_OBJECT_CLASS(spice_display_parent_class)->dispose(obj);
 }
 
@@ -994,6 +999,40 @@ typedef enum {
     SEND_KEY_RELEASE,
 } SendKeyType;
 
+static void key_press_and_release(SpiceDisplay *display)
+{
+    SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
+
+    if (d->key_delayed_scancode == 0)
+        return;
+
+    spice_inputs_key_press_and_release(d->inputs, d->key_delayed_scancode);
+    d->key_delayed_scancode = 0;
+
+    if (d->key_delayed_id) {
+        g_source_remove(d->key_delayed_id);
+        d->key_delayed_id = 0;
+    }
+}
+
+static gboolean key_press_delayed(gpointer data)
+{
+    SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(data);
+
+    if (d->key_delayed_scancode == 0)
+        return FALSE;
+
+    spice_inputs_key_press(d->inputs, d->key_delayed_scancode);
+    d->key_delayed_scancode = 0;
+
+    if (d->key_delayed_id) {
+        g_source_remove(d->key_delayed_id);
+        d->key_delayed_id = 0;
+    }
+
+    return FALSE;
+}
+
 static void send_key(SpiceDisplay *display, int scancode, SendKeyType type, gboolean press_delayed)
 {
     SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
@@ -1010,15 +1049,40 @@ static void send_key(SpiceDisplay *display, int scancode, SendKeyType type, gboo
     m = (1 << b);
     g_return_if_fail(i < SPICE_N_ELEMENTS(d->key_state));
 
-    if (type == SEND_KEY_PRESS) {
-        spice_inputs_key_press(d->inputs, scancode);
+    switch (type) {
+    case SEND_KEY_PRESS:
+        /* ensure delayed key is pressed before any new input event */
+        key_press_delayed(display);
+
+        if (press_delayed &&
+            d->keypress_delay != 0 &&
+            !(d->key_state[i] & m)) {
+            g_warn_if_fail(d->key_delayed_id == 0);
+            d->key_delayed_id = g_timeout_add(d->keypress_delay, key_press_delayed, display);
+            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 == scancode)
+            key_press_and_release(display);
+        else {
+            /* ensure delayed key is pressed before other key are released */
+            key_press_delayed(display);
+            spice_inputs_key_release(d->inputs, scancode);
         }
-        spice_inputs_key_release(d->inputs, scancode);
+
         d->key_state[i] &= ~m;
+        break;
+
+    default:
+        g_warn_if_reached();
     }
 }
 
-- 
1.7.10.4