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

[Christophe Fergeau]

I haven't looked at the patch, but the log has some typos:

On Wed, Aug 15, 2012 at 10:59:30PM +0300, Marc-André Lureau wrote:
> Until now, Spice clients only sent seperate key events for press and

separate

> release. But this may result in unwanted key repeatition from guest VM

repetition

> side. It seems OSes have various implementation. While MS Windows rely

implementations, relies

> on hardware key repeats (which are several sequential press events),
> otoh, X11 uses software key repeat (although not Linux keyboard/VT by

'repeats' maybe

> 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 exisiting

existing

> behaviour which has been working pretty well so far, sending seperate

separate


Christophe

> "press", then repeatedly "press", and an ending "release" event.
> ---
>  gtk/spice-widget-priv.h |    2 ++
>  gtk/spice-widget.c      |   76 +++++++++++++++++++++++++++++++++++++++++++----
>  spice-common            |    2 +-
>  3 files changed, 73 insertions(+), 7 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 8cb7abb..d2b00f7 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)
> +        goto end;
> +
> +    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);
> +
> +end:
> +    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 (down) {
> -        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();
>      }
>  }
>  
> diff --git a/spice-common b/spice-common
> index c2adbb0..31f1bff 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit c2adbb00dc0b29de0fe297f241fb0efeb4a81510
> +Subproject commit 31f1bff472da61ba07121ed31536c4af864c4a8f
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20120816/c0d7687a/attachment.pgp>