1. Patchew
  2. QEMU
  3. [Qemu-devel] [RFC PATCH v2 0/7] ui: add generic keyboard state tracker, fix keymap
  4. View patch

[Qemu-devel] [RFC PATCH v2 5/7] kbd-state: use state tracker for vnc

Gerd Hoffmann posted 7 patches 7 years, 1 month ago
There is a newer version of this series
[Qemu-devel] [RFC PATCH v2 5/7] kbd-state: use state tracker for vnc
Posted by Gerd Hoffmann 7 years, 1 month ago 
Use the new keyboard state tracked for vnc.  Allows to drop the
vnc-specific modifier state tracking code.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.h |   5 ++-
 ui/vnc.c | 120 ++++++++++++++++++---------------------------------------------
 2 files changed, 35 insertions(+), 90 deletions(-)

diff --git a/ui/vnc.h b/ui/vnc.h
index a86e0610e8..56111908ce 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -44,6 +44,7 @@
 #include "keymaps.h"
 #include "vnc-palette.h"
 #include "vnc-enc-zrle.h"
+#include "ui/kbd-state.h"
 
 // #define _VNC_DEBUG 1
 
@@ -155,7 +156,7 @@ struct VncDisplay
     int lock_key_sync;
     QEMUPutLEDEntry *led;
     int ledstate;
-    int key_delay_ms;
+    KbdState *kbd;
     QemuMutex mutex;
 
     QEMUCursor *cursor;
@@ -326,8 +327,6 @@ struct VncState
 
     VncReadEvent *read_handler;
     size_t read_handler_expect;
-    /* input */
-    uint8_t modifiers_state[256];
 
     bool abort;
     QemuMutex output_mutex;
diff --git a/ui/vnc.c b/ui/vnc.c
index 0c1b477425..b56431ce3b 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -59,7 +59,6 @@ static QTAILQ_HEAD(, VncDisplay) vnc_displays =
     QTAILQ_HEAD_INITIALIZER(vnc_displays);
 
 static int vnc_cursor_define(VncState *vs);
-static void vnc_release_modifiers(VncState *vs);
 static void vnc_update_throttle_offset(VncState *vs);
 
 static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
@@ -1248,7 +1247,7 @@ void vnc_disconnect_finish(VncState *vs)
     vnc_sasl_client_cleanup(vs);
 #endif /* CONFIG_VNC_SASL */
     audio_del(vs);
-    vnc_release_modifiers(vs);
+    kbd_state_lift_all_keys(vs->vd->kbd);
 
     if (vs->mouse_mode_notifier.notify != NULL) {
         qemu_remove_mouse_mode_change_notifier(&vs->mouse_mode_notifier);
@@ -1737,26 +1736,10 @@ static void pointer_event(VncState *vs, int button_mask, int x, int y)
     qemu_input_event_sync();
 }
 
-static void reset_keys(VncState *vs)
+static void press_key(VncState *vs, QKeyCode qcode)
 {
-    int i;
-    for(i = 0; i < 256; i++) {
-        if (vs->modifiers_state[i]) {
-            qemu_input_event_send_key_number(vs->vd->dcl.con, i, false);
-            qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
-            vs->modifiers_state[i] = 0;
-        }
-    }
-}
-
-static void press_key(VncState *vs, int keysym)
-{
-    int keycode = keysym2scancode(vs->vd->kbd_layout, keysym,
-                                  false, false, false) & SCANCODE_KEYMASK;
-    qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, true);
-    qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
-    qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
-    qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
+    kbd_state_key_event(vs->vd->kbd, qcode, true);
+    kbd_state_key_event(vs->vd->kbd, qcode, false);
 }
 
 static void vnc_led_state_change(VncState *vs)
@@ -1797,32 +1780,20 @@ static void kbd_leds(void *opaque, int ledstate)
 
 static void do_key_event(VncState *vs, int down, int keycode, int sym)
 {
+    QKeyCode qcode = qemu_input_key_number_to_qcode(keycode);
+
     /* QEMU console switch */
-    switch(keycode) {
-    case 0x2a:                          /* Left Shift */
-    case 0x36:                          /* Right Shift */
-    case 0x1d:                          /* Left CTRL */
-    case 0x9d:                          /* Right CTRL */
-    case 0x38:                          /* Left ALT */
-    case 0xb8:                          /* Right ALT */
-        if (down)
-            vs->modifiers_state[keycode] = 1;
-        else
-            vs->modifiers_state[keycode] = 0;
-        break;
-    case 0x02 ... 0x0a: /* '1' to '9' keys */
-        if (vs->vd->dcl.con == NULL &&
-            down && vs->modifiers_state[0x1d] && vs->modifiers_state[0x38]) {
+    switch (qcode) {
+    case Q_KEY_CODE_1 ... Q_KEY_CODE_9: /* '1' to '9' keys */
+        if (vs->vd->dcl.con == NULL && down &&
+            kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_CTRL) &&
+            kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_ALT)) {
             /* Reset the modifiers sent to the current console */
-            reset_keys(vs);
-            console_select(keycode - 0x02);
+            kbd_state_lift_all_keys(vs->vd->kbd);
+            console_select(qcode - Q_KEY_CODE_1);
             return;
         }
-        break;
-    case 0x3a:                        /* CapsLock */
-    case 0x45:                        /* NumLock */
-        if (down)
-            vs->modifiers_state[keycode] ^= 1;
+    default:
         break;
     }
 
@@ -1837,16 +1808,14 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
            toggles numlock away from the VNC window.
         */
         if (keysym_is_numlock(vs->vd->kbd_layout, sym & 0xFFFF)) {
-            if (!vs->modifiers_state[0x45]) {
+            if (!kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_NUMLOCK)) {
                 trace_vnc_key_sync_numlock(true);
-                vs->modifiers_state[0x45] = 1;
-                press_key(vs, 0xff7f);
+                press_key(vs, Q_KEY_CODE_NUM_LOCK);
             }
         } else {
-            if (vs->modifiers_state[0x45]) {
+            if (kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_NUMLOCK)) {
                 trace_vnc_key_sync_numlock(false);
-                vs->modifiers_state[0x45] = 0;
-                press_key(vs, 0xff7f);
+                press_key(vs, Q_KEY_CODE_NUM_LOCK);
             }
         }
     }
@@ -1859,30 +1828,25 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
            toggles capslock away from the VNC window.
         */
         int uppercase = !!(sym >= 'A' && sym <= 'Z');
-        int shift = !!(vs->modifiers_state[0x2a] | vs->modifiers_state[0x36]);
-        int capslock = !!(vs->modifiers_state[0x3a]);
+        bool shift = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_SHIFT);
+        bool capslock = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_CAPSLOCK);
         if (capslock) {
             if (uppercase == shift) {
                 trace_vnc_key_sync_capslock(false);
-                vs->modifiers_state[0x3a] = 0;
-                press_key(vs, 0xffe5);
+                press_key(vs, Q_KEY_CODE_CAPS_LOCK);
             }
         } else {
             if (uppercase != shift) {
                 trace_vnc_key_sync_capslock(true);
-                vs->modifiers_state[0x3a] = 1;
-                press_key(vs, 0xffe5);
+                press_key(vs, Q_KEY_CODE_CAPS_LOCK);
             }
         }
     }
 
-    if (qemu_console_is_graphic(NULL)) {
-        qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, down);
-        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
-    } else {
-        bool numlock = vs->modifiers_state[0x45];
-        bool control = (vs->modifiers_state[0x1d] ||
-                        vs->modifiers_state[0x9d]);
+    kbd_state_key_event(vs->vd->kbd, qcode, down);
+    if (!qemu_console_is_graphic(NULL)) {
+        bool numlock = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_NUMLOCK);
+        bool control = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_CTRL);
         /* QEMU console emulation */
         if (down) {
             switch (keycode) {
@@ -1983,27 +1947,6 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
     }
 }
 
-static void vnc_release_modifiers(VncState *vs)
-{
-    static const int keycodes[] = {
-        /* shift, control, alt keys, both left & right */
-        0x2a, 0x36, 0x1d, 0x9d, 0x38, 0xb8,
-    };
-    int i, keycode;
-
-    if (!qemu_console_is_graphic(NULL)) {
-        return;
-    }
-    for (i = 0; i < ARRAY_SIZE(keycodes); i++) {
-        keycode = keycodes[i];
-        if (!vs->modifiers_state[keycode]) {
-            continue;
-        }
-        qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
-        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
-    }
-}
-
 static const char *code2name(int keycode)
 {
     return QKeyCode_str(qemu_input_key_number_to_qcode(keycode));
@@ -2011,9 +1954,9 @@ static const char *code2name(int keycode)
 
 static void key_event(VncState *vs, int down, uint32_t sym)
 {
-    bool shift = vs->modifiers_state[0x2a] || vs->modifiers_state[0x36];
-    bool altgr = vs->modifiers_state[0xb8];
-    bool ctrl  = vs->modifiers_state[0x1d] || vs->modifiers_state[0x9d];
+    bool shift = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_SHIFT);
+    bool altgr = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_ALTGR);
+    bool ctrl  = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_CTRL);
     int keycode;
     int lsym = sym;
 
@@ -3240,6 +3183,7 @@ void vnc_display_init(const char *id, Error **errp)
 
     vd->dcl.ops = &dcl_ops;
     register_displaychangelistener(&vd->dcl);
+    vd->kbd = kbd_state_init(vd->dcl.con);
 }
 
 
@@ -3976,7 +3920,6 @@ void vnc_display_open(const char *id, Error **errp)
         vd->led = qemu_add_led_event_handler(kbd_leds, vd);
     }
     vd->ledstate = 0;
-    vd->key_delay_ms = key_delay_ms;
 
     device_id = qemu_opt_get(opts, "display");
     if (device_id) {
@@ -3993,10 +3936,13 @@ void vnc_display_open(const char *id, Error **errp)
     }
 
     if (con != vd->dcl.con) {
+        kbd_state_free(vd->kbd);
         unregister_displaychangelistener(&vd->dcl);
         vd->dcl.con = con;
         register_displaychangelistener(&vd->dcl);
+        vd->kbd = kbd_state_init(vd->dcl.con);
     }
+    kbd_state_set_delay(vd->kbd, key_delay_ms);
 
     if (saddr == NULL) {
         goto cleanup;
-- 
2.9.3
Re: [Qemu-devel] [RFC PATCH v2 5/7] kbd-state: use state tracker for vnc
Posted by Daniel P. Berrangé 7 years, 1 month ago 
On Wed, Dec 19, 2018 at 01:09:02PM +0100, Gerd Hoffmann wrote:
> Use the new keyboard state tracked for vnc.  Allows to drop the
> vnc-specific modifier state tracking code.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc.h |   5 ++-
>  ui/vnc.c | 120 ++++++++++++++++++---------------------------------------------
>  2 files changed, 35 insertions(+), 90 deletions(-)
> 
> diff --git a/ui/vnc.h b/ui/vnc.h
> index a86e0610e8..56111908ce 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -44,6 +44,7 @@
>  #include "keymaps.h"
>  #include "vnc-palette.h"
>  #include "vnc-enc-zrle.h"
> +#include "ui/kbd-state.h"
>  
>  // #define _VNC_DEBUG 1
>  
> @@ -155,7 +156,7 @@ struct VncDisplay
>      int lock_key_sync;
>      QEMUPutLEDEntry *led;
>      int ledstate;
> -    int key_delay_ms;
> +    KbdState *kbd;
>      QemuMutex mutex;
>  
>      QEMUCursor *cursor;
> @@ -326,8 +327,6 @@ struct VncState
>  
>      VncReadEvent *read_handler;
>      size_t read_handler_expect;
> -    /* input */
> -    uint8_t modifiers_state[256];
>  
>      bool abort;
>      QemuMutex output_mutex;
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 0c1b477425..b56431ce3b 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c


> @@ -1797,32 +1780,20 @@ static void kbd_leds(void *opaque, int ledstate)
>  
>  static void do_key_event(VncState *vs, int down, int keycode, int sym)
>  {
> +    QKeyCode qcode = qemu_input_key_number_to_qcode(keycode);
> +
>      /* QEMU console switch */
> -    switch(keycode) {
> -    case 0x2a:                          /* Left Shift */
> -    case 0x36:                          /* Right Shift */
> -    case 0x1d:                          /* Left CTRL */
> -    case 0x9d:                          /* Right CTRL */
> -    case 0x38:                          /* Left ALT */
> -    case 0xb8:                          /* Right ALT */
> -        if (down)
> -            vs->modifiers_state[keycode] = 1;
> -        else
> -            vs->modifiers_state[keycode] = 0;
> -        break;


This code updated modifier state as the first thing in do_key_event


[snip]

> @@ -1859,30 +1828,25 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>             toggles capslock away from the VNC window.
>          */
>          int uppercase = !!(sym >= 'A' && sym <= 'Z');
> -        int shift = !!(vs->modifiers_state[0x2a] | vs->modifiers_state[0x36]);
> -        int capslock = !!(vs->modifiers_state[0x3a]);
> +        bool shift = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_SHIFT);
> +        bool capslock = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_CAPSLOCK);

This uses the modifier state

In old code it would use the newly updated modifier state

In new code it now uses the old modifier state.

>          if (capslock) {
>              if (uppercase == shift) {
>                  trace_vnc_key_sync_capslock(false);
> -                vs->modifiers_state[0x3a] = 0;
> -                press_key(vs, 0xffe5);
> +                press_key(vs, Q_KEY_CODE_CAPS_LOCK);
>              }
>          } else {
>              if (uppercase != shift) {
>                  trace_vnc_key_sync_capslock(true);
> -                vs->modifiers_state[0x3a] = 1;
> -                press_key(vs, 0xffe5);
> +                press_key(vs, Q_KEY_CODE_CAPS_LOCK);
>              }
>          }
>      }
>  
> -    if (qemu_console_is_graphic(NULL)) {
> -        qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, down);
> -        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
> -    } else {
> -        bool numlock = vs->modifiers_state[0x45];
> -        bool control = (vs->modifiers_state[0x1d] ||
> -                        vs->modifiers_state[0x9d]);
> +    kbd_state_key_event(vs->vd->kbd, qcode, down);

This updates modifier state in the new code, after the code above that
uses modifier state.

Is this ordering change intentional ?


> +    if (!qemu_console_is_graphic(NULL)) {
> +        bool numlock = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_NUMLOCK);
> +        bool control = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_CTRL);
>          /* QEMU console emulation */
>          if (down) {
>              switch (keycode) {
> @@ -1983,27 +1947,6 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>      }
>  }

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [RFC PATCH v2 5/7] kbd-state: use state tracker for vnc
Posted by Gerd Hoffmann 7 years ago 
> > @@ -1859,30 +1828,25 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
> >             toggles capslock away from the VNC window.
> >          */
> >          int uppercase = !!(sym >= 'A' && sym <= 'Z');
> > -        int shift = !!(vs->modifiers_state[0x2a] | vs->modifiers_state[0x36]);
> > -        int capslock = !!(vs->modifiers_state[0x3a]);
> > +        bool shift = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_SHIFT);
> > +        bool capslock = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_CAPSLOCK);
> 
> This uses the modifier state
> 
> In old code it would use the newly updated modifier state
> 
> In new code it now uses the old modifier state.

Ordering doesn't matter.  This code doesn't run on modifier key events.
On letter keydown events it checks whenever shift and capslock state are
consistent, and if not it generates a capslock keypress.

cheers,
  Gerd
Re: [Qemu-devel] [RFC PATCH v2 5/7] kbd-state: use state tracker for vnc
Posted by Daniel P. Berrangé 7 years ago 
On Tue, Jan 22, 2019 at 10:00:36AM +0100, Gerd Hoffmann wrote:
> > > @@ -1859,30 +1828,25 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
> > >             toggles capslock away from the VNC window.
> > >          */
> > >          int uppercase = !!(sym >= 'A' && sym <= 'Z');
> > > -        int shift = !!(vs->modifiers_state[0x2a] | vs->modifiers_state[0x36]);
> > > -        int capslock = !!(vs->modifiers_state[0x3a]);
> > > +        bool shift = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_SHIFT);
> > > +        bool capslock = kbd_state_modifier_get(vs->vd->kbd, KBD_MOD_CAPSLOCK);
> > 
> > This uses the modifier state
> > 
> > In old code it would use the newly updated modifier state
> > 
> > In new code it now uses the old modifier state.
> 
> Ordering doesn't matter.  This code doesn't run on modifier key events.
> On letter keydown events it checks whenever shift and capslock state are
> consistent, and if not it generates a capslock keypress.

Ok, then

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.