All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap
@ 2019-01-22  9:28 Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 1/8] kbd-state: add keyboard state tracker Gerd Hoffmann
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

v3:
 - s/kbd/qkbd/ to avoid collisons.
 - add api docs.
 - misc minor fixes.

Gerd Hoffmann (8):
  kbd-state: add keyboard state tracker
  sdl2: remove sdl2_reset_keys() function
  kbd-state: use state tracker for sdl2
  sdl2: use only QKeyCode in sdl2_process_key()
  kbd-state: use state tracker for gtk
  kbd-state: use state tracker for vnc
  keymap: pass full keyboard state to keysym2scancode
  keymap: fix keyup mappings

 include/ui/gtk.h       |   2 +
 include/ui/kbd-state.h |  96 +++++++++++++++++++++++++++++++++++++
 include/ui/sdl2.h      |   3 +-
 ui/keymaps.h           |   3 +-
 ui/vnc.h               |   5 +-
 ui/curses.c            |   2 +-
 ui/gtk.c               |  38 +++------------
 ui/kbd-state.c         | 125 +++++++++++++++++++++++++++++++++++++++++++++++++
 ui/keymaps.c           |  55 +++++++++++++---------
 ui/sdl2-input.c        |  50 ++------------------
 ui/sdl2.c              |  12 ++---
 ui/vnc.c               | 119 ++++++++++++----------------------------------
 ui/Makefile.objs       |   2 +-
 13 files changed, 310 insertions(+), 202 deletions(-)
 create mode 100644 include/ui/kbd-state.h
 create mode 100644 ui/kbd-state.c

-- 
2.9.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC PATCH v3 1/8] kbd-state: add keyboard state tracker
  2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
@ 2019-01-22  9:28 ` Gerd Hoffmann
  2019-01-22  9:47   ` Daniel P. Berrangé
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 2/8] sdl2: remove sdl2_reset_keys() function Gerd Hoffmann
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

Now that most user interfaces are using QKeyCodes it is easier to have
common keyboard code useable by all user interfaces.

This patch adds helper code to track the state of all keyboard keys,
using a bitmap indexed by QKeyCode.  Modifier state is tracked too,
as separate bitmap.  That makes checking modifier state easier.
Likewise we can easily apply special handling for capslock & numlock
(toggles on keypress) and ctrl + shift (we have two keys for that).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/kbd-state.h |  96 +++++++++++++++++++++++++++++++++++++
 ui/kbd-state.c         | 125 +++++++++++++++++++++++++++++++++++++++++++++++++
 ui/Makefile.objs       |   2 +-
 3 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 include/ui/kbd-state.h
 create mode 100644 ui/kbd-state.c

diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
new file mode 100644
index 0000000000..277198f169
--- /dev/null
+++ b/include/ui/kbd-state.h
@@ -0,0 +1,96 @@
+#ifndef QEMU_UI_KBD_STATE_H
+#define QEMU_UI_KBD_STATE_H 1
+
+#include "qapi/qapi-types-ui.h"
+
+typedef enum QKbdModifier QKbdModifier;
+
+enum QKbdModifier {
+    QKBD_MOD_NONE = 0,
+
+    QKBD_MOD_SHIFT,
+    QKBD_MOD_CTRL,
+    QKBD_MOD_ALT,
+    QKBD_MOD_ALTGR,
+
+    QKBD_MOD_NUMLOCK,
+    QKBD_MOD_CAPSLOCK,
+
+    QKBD_MOD__MAX
+};
+
+typedef struct QKbdState QKbdState;
+
+/**
+ * qkbd_state_init: init keyboard state tracker.
+ *
+ * Allocates and initializes keyboard state struct.
+ *
+ * @con: QemuConsole for this state tracker.  Gets passed down to
+ * qemu_input_*() functions when sending key events to the guest.
+ */
+QKbdState *qkbd_state_init(QemuConsole *con);
+
+/**
+ * qkbd_state_free: free keyboard tracker state.
+ *
+ * @kbd: state tracker state.
+ */
+void qkbd_state_free(QKbdState *kbd);
+
+/**
+ * qkbd_state_key_event: process key event.
+ *
+ * Update keyboard state, send event to the guest.
+ *
+ * This function takes care to not send suspious events (keyup event
+ * for a key not pressed for example).
+ *
+ * @kbd: state tracker state.
+ * @qcode: the key pressed or released.
+ * @down: true for key down events, false otherwise.
+ */
+void qkbd_state_key_event(QKbdState *kbd, QKeyCode qcode, bool down);
+
+/**
+ * qkbd_state_set_delay: set key press delay.
+ *
+ * When set the specified delay will be added after each key event,
+ * using qemu_input_event_send_key_delay().
+ *
+ * @kbd: state tracker state.
+ * @delay_ms: the delay in miliseconds.
+ */
+void qkbd_state_set_delay(QKbdState *kbd, int delay_ms);
+
+/**
+ * qkbd_state_key_get: get key state.
+ *
+ * Returns true when the key is down.
+ *
+ * @kbd: state tracker state.
+ * @qcode: the key to query.
+ */
+bool qkbd_state_key_get(QKbdState *kbd, QKeyCode qcode);
+
+/**
+ * qkbd_state_modifier_get: get modifier state.
+ *
+ * Returns true when the modifier is active.
+ *
+ * @kbd: state tracker state.
+ * @mod: the modifier to query.
+ */
+bool qkbd_state_modifier_get(QKbdState *kbd, QKbdModifier mod);
+
+/**
+ * qkbd_state_lift_all_keys: lift all pressed keys.
+ *
+ * This sends key up events to the guest for all keys which are in
+ * down state.
+ *
+ * @kbd: state tracker state.
+ */
+void qkbd_state_lift_all_keys(QKbdState *kbd);
+
+#endif /* QEMU_UI_KBD_STATE_H */
diff --git a/ui/kbd-state.c b/ui/kbd-state.c
new file mode 100644
index 0000000000..c8397ebed4
--- /dev/null
+++ b/ui/kbd-state.c
@@ -0,0 +1,125 @@
+#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
+#include "qemu/queue.h"
+#include "ui/console.h"
+#include "ui/input.h"
+#include "ui/kbd-state.h"
+
+struct QKbdState {
+    QemuConsole *con;
+    int key_delay_ms;
+    DECLARE_BITMAP(keys, Q_KEY_CODE__MAX);
+    DECLARE_BITMAP(mods, QKBD_MOD__MAX);
+};
+
+static void qkbd_state_modifier_update(QKbdState *kbd,
+                                      QKeyCode qcode1, QKeyCode qcode2,
+                                      QKbdModifier mod)
+{
+    if (test_bit(qcode1, kbd->keys) || test_bit(qcode2, kbd->keys)) {
+        set_bit(mod, kbd->mods);
+    } else {
+        clear_bit(mod, kbd->mods);
+    }
+}
+
+bool qkbd_state_modifier_get(QKbdState *kbd, QKbdModifier mod)
+{
+    return test_bit(mod, kbd->mods);
+}
+
+bool qkbd_state_key_get(QKbdState *kbd, QKeyCode qcode)
+{
+    return test_bit(qcode, kbd->keys);
+}
+
+void qkbd_state_key_event(QKbdState *kbd, QKeyCode qcode, bool down)
+{
+    bool state = test_bit(qcode, kbd->keys);
+
+    if (state == down) {
+        /*
+         * Filter out events which don't change the keyboard state.
+         *
+         * Most notably this allows to simply send along all key-up
+         * events, and this function will filter out everything where
+         * the corresponding key-down event wasn't send to the guest,
+         * for example due to being a host hotkey.
+         */
+        return;
+    }
+
+    /* update key and modifier state */
+    change_bit(qcode, kbd->keys);
+    switch (qcode) {
+    case Q_KEY_CODE_SHIFT:
+    case Q_KEY_CODE_SHIFT_R:
+        qkbd_state_modifier_update(kbd, Q_KEY_CODE_SHIFT, Q_KEY_CODE_SHIFT_R,
+                                   QKBD_MOD_SHIFT);
+        break;
+    case Q_KEY_CODE_CTRL:
+    case Q_KEY_CODE_CTRL_R:
+        qkbd_state_modifier_update(kbd, Q_KEY_CODE_CTRL, Q_KEY_CODE_CTRL_R,
+                                   QKBD_MOD_CTRL);
+        break;
+    case Q_KEY_CODE_ALT:
+        qkbd_state_modifier_update(kbd, Q_KEY_CODE_ALT, Q_KEY_CODE_ALT,
+                                   QKBD_MOD_ALT);
+        break;
+    case Q_KEY_CODE_ALT_R:
+        qkbd_state_modifier_update(kbd, Q_KEY_CODE_ALT_R, Q_KEY_CODE_ALT_R,
+                                   QKBD_MOD_ALTGR);
+        break;
+    case Q_KEY_CODE_CAPS_LOCK:
+        if (down) {
+            change_bit(QKBD_MOD_CAPSLOCK, kbd->mods);
+        }
+        break;
+    case Q_KEY_CODE_NUM_LOCK:
+        if (down) {
+            change_bit(QKBD_MOD_NUMLOCK, kbd->mods);
+        }
+        break;
+    default:
+        /* keep gcc happy */
+        break;
+    }
+
+    /* send to guest */
+    if (qemu_console_is_graphic(kbd->con)) {
+        qemu_input_event_send_key_qcode(kbd->con, qcode, down);
+        if (kbd->key_delay_ms) {
+            qemu_input_event_send_key_delay(kbd->key_delay_ms);
+        }
+    }
+}
+
+void qkbd_state_lift_all_keys(QKbdState *kbd)
+{
+    int qcode;
+
+    for (qcode = 0; qcode < Q_KEY_CODE__MAX; qcode++) {
+        if (test_bit(qcode, kbd->keys)) {
+            qkbd_state_key_event(kbd, qcode, false);
+        }
+    }
+}
+
+void qkbd_state_set_delay(QKbdState *kbd, int delay_ms)
+{
+    kbd->key_delay_ms = delay_ms;
+}
+
+void qkbd_state_free(QKbdState *kbd)
+{
+    g_free(kbd);
+}
+
+QKbdState *qkbd_state_init(QemuConsole *con)
+{
+    QKbdState *kbd = g_new0(QKbdState, 1);
+
+    kbd->con = con;
+
+    return kbd;
+}
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 00f6976c30..3c04bdda94 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -8,7 +8,7 @@ vnc-obj-y += vnc-ws.o
 vnc-obj-y += vnc-jobs.o
 
 common-obj-y += keymaps.o console.o cursor.o qemu-pixman.o
-common-obj-y += input.o input-keymap.o input-legacy.o
+common-obj-y += input.o input-keymap.o input-legacy.o kbd-state.o
 common-obj-$(CONFIG_LINUX) += input-linux.o
 common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
 common-obj-$(CONFIG_COCOA) += cocoa.o
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC PATCH v3 2/8] sdl2: remove sdl2_reset_keys() function
  2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 1/8] kbd-state: add keyboard state tracker Gerd Hoffmann
@ 2019-01-22  9:28 ` Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 3/8] kbd-state: use state tracker for sdl2 Gerd Hoffmann
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

No users left, dead code.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/sdl2.h |  1 -
 ui/sdl2-input.c   | 16 ----------------
 2 files changed, 17 deletions(-)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index f43eecdbd6..fffbbfaee4 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -41,7 +41,6 @@ void sdl2_window_destroy(struct sdl2_console *scon);
 void sdl2_window_resize(struct sdl2_console *scon);
 void sdl2_poll_events(struct sdl2_console *scon);
 
-void sdl2_reset_keys(struct sdl2_console *scon);
 void sdl2_process_key(struct sdl2_console *scon,
                       SDL_KeyboardEvent *ev);
 
diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 1378b63dd9..208266c6a5 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -32,22 +32,6 @@
 
 static uint8_t modifiers_state[SDL_NUM_SCANCODES];
 
-void sdl2_reset_keys(struct sdl2_console *scon)
-{
-    QemuConsole *con = scon ? scon->dcl.con : NULL;
-    int i;
-
-    for (i = 0 ;
-         i < SDL_NUM_SCANCODES && i < qemu_input_map_usb_to_qcode_len ;
-         i++) {
-        if (modifiers_state[i]) {
-            int qcode = qemu_input_map_usb_to_qcode[i];
-            qemu_input_event_send_key_qcode(con, qcode, false);
-            modifiers_state[i] = 0;
-        }
-    }
-}
-
 void sdl2_process_key(struct sdl2_console *scon,
                       SDL_KeyboardEvent *ev)
 {
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC PATCH v3 3/8] kbd-state: use state tracker for sdl2
  2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 1/8] kbd-state: add keyboard state tracker Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 2/8] sdl2: remove sdl2_reset_keys() function Gerd Hoffmann
@ 2019-01-22  9:28 ` Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 4/8] sdl2: use only QKeyCode in sdl2_process_key() Gerd Hoffmann
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

Use the new keyboard state tracked for sdl2.  We can drop the modifier
state tracking from sdl2.  Also keyup code is simpler, the state tracker
will take care to not send suspious keyup events to the guest.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/ui/sdl2.h |  2 ++
 ui/sdl2-input.c   | 27 ++-------------------------
 ui/sdl2.c         | 12 +++---------
 3 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index fffbbfaee4..6627fe05b0 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -7,6 +7,7 @@
 #include <SDL.h>
 #include <SDL_syswm.h>
 
+#include "ui/kbd-state.h"
 #ifdef CONFIG_OPENGL
 # include "ui/egl-helpers.h"
 #endif
@@ -27,6 +28,7 @@ struct sdl2_console {
     int idle_counter;
     int ignore_hotkeys;
     SDL_GLContext winctx;
+    QKbdState *kbd;
 #ifdef CONFIG_OPENGL
     QemuGLShader *gls;
     egl_fb guest_fb;
diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 208266c6a5..22e3336aab 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -30,8 +30,6 @@
 #include "ui/sdl2.h"
 #include "sysemu/sysemu.h"
 
-static uint8_t modifiers_state[SDL_NUM_SCANCODES];
-
 void sdl2_process_key(struct sdl2_console *scon,
                       SDL_KeyboardEvent *ev)
 {
@@ -43,31 +41,10 @@ void sdl2_process_key(struct sdl2_console *scon,
     }
 
     qcode = qemu_input_map_usb_to_qcode[ev->keysym.scancode];
-
-    /* modifier state tracking */
-    switch (ev->keysym.scancode) {
-    case SDL_SCANCODE_LCTRL:
-    case SDL_SCANCODE_LSHIFT:
-    case SDL_SCANCODE_LALT:
-    case SDL_SCANCODE_LGUI:
-    case SDL_SCANCODE_RCTRL:
-    case SDL_SCANCODE_RSHIFT:
-    case SDL_SCANCODE_RALT:
-    case SDL_SCANCODE_RGUI:
-        if (ev->type == SDL_KEYUP) {
-            modifiers_state[ev->keysym.scancode] = 0;
-        } else {
-            modifiers_state[ev->keysym.scancode] = 1;
-        }
-        break;
-    default:
-        /* nothing */
-        break;
-    }
+    qkbd_state_key_event(scon->kbd, qcode, ev->type == SDL_KEYDOWN);
 
     if (!qemu_console_is_graphic(con)) {
-        bool ctrl = (modifiers_state[SDL_SCANCODE_LCTRL] ||
-                     modifiers_state[SDL_SCANCODE_RCTRL]);
+        bool ctrl = qkbd_state_modifier_get(scon->kbd, QKBD_MOD_CTRL);
         if (ev->type == SDL_KEYDOWN) {
             switch (ev->keysym.scancode) {
             case SDL_SCANCODE_RETURN:
diff --git a/ui/sdl2.c b/ui/sdl2.c
index a10b6e3a08..13ad4a76eb 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -38,7 +38,6 @@ static int gui_grab; /* if true, all keyboard/mouse events are grabbed */
 
 static int gui_saved_grab;
 static int gui_fullscreen;
-static int gui_keysym;
 static int gui_grab_code = KMOD_LALT | KMOD_LCTRL;
 static SDL_Cursor *sdl_cursor_normal;
 static SDL_Cursor *sdl_cursor_hidden;
@@ -330,6 +329,7 @@ static void handle_keydown(SDL_Event *ev)
     int win;
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
     int gui_key_modifier_pressed = get_mod_state();
+    int gui_keysym = 0;
 
     if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
         switch (ev->key.keysym.scancode) {
@@ -410,16 +410,9 @@ static void handle_keydown(SDL_Event *ev)
 static void handle_keyup(SDL_Event *ev)
 {
     struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
-    int gui_key_modifier_pressed = get_mod_state();
 
     scon->ignore_hotkeys = false;
-
-    if (!gui_key_modifier_pressed) {
-        gui_keysym = 0;
-    }
-    if (!gui_keysym) {
-        sdl2_process_key(scon, &ev->key);
-    }
+    sdl2_process_key(scon, &ev->key);
 }
 
 static void handle_textinput(SDL_Event *ev)
@@ -823,6 +816,7 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
         sdl2_console[i].dcl.ops = &dcl_2d_ops;
 #endif
         sdl2_console[i].dcl.con = con;
+        sdl2_console[i].kbd = qkbd_state_init(con);
         register_displaychangelistener(&sdl2_console[i].dcl);
 
 #if defined(SDL_VIDEO_DRIVER_WINDOWS) || defined(SDL_VIDEO_DRIVER_X11)
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC PATCH v3 4/8] sdl2: use only QKeyCode in sdl2_process_key()
  2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 3/8] kbd-state: use state tracker for sdl2 Gerd Hoffmann
@ 2019-01-22  9:28 ` Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 5/8] kbd-state: use state tracker for gtk Gerd Hoffmann
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

Also: sdl2_process_key is never called with scon == NULL.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 ui/sdl2-input.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 22e3336aab..664364a5e5 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -34,20 +34,19 @@ void sdl2_process_key(struct sdl2_console *scon,
                       SDL_KeyboardEvent *ev)
 {
     int qcode;
-    QemuConsole *con = scon ? scon->dcl.con : NULL;
+    QemuConsole *con = scon->dcl.con;
 
     if (ev->keysym.scancode >= qemu_input_map_usb_to_qcode_len) {
         return;
     }
-
     qcode = qemu_input_map_usb_to_qcode[ev->keysym.scancode];
     qkbd_state_key_event(scon->kbd, qcode, ev->type == SDL_KEYDOWN);
 
     if (!qemu_console_is_graphic(con)) {
         bool ctrl = qkbd_state_modifier_get(scon->kbd, QKBD_MOD_CTRL);
         if (ev->type == SDL_KEYDOWN) {
-            switch (ev->keysym.scancode) {
-            case SDL_SCANCODE_RETURN:
+            switch (qcode) {
+            case Q_KEY_CODE_RET:
                 kbd_put_keysym_console(con, '\n');
                 break;
             default:
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC PATCH v3 5/8] kbd-state: use state tracker for gtk
  2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 4/8] sdl2: use only QKeyCode in sdl2_process_key() Gerd Hoffmann
@ 2019-01-22  9:28 ` Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 6/8] kbd-state: use state tracker for vnc Gerd Hoffmann
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

Use the new keyboard state tracked for gtk.  Allows to drop the
gtk-specific modifier state tracking code.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/ui/gtk.h |  2 ++
 ui/gtk.c         | 38 ++++++--------------------------------
 2 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 99edd3c085..d9eedad976 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -22,6 +22,7 @@
 #include <gdk/gdkwayland.h>
 #endif
 
+#include "ui/kbd-state.h"
 #if defined(CONFIG_OPENGL)
 #include "ui/egl-helpers.h"
 #include "ui/egl-context.h"
@@ -32,6 +33,7 @@ typedef struct GtkDisplayState GtkDisplayState;
 typedef struct VirtualGfxConsole {
     GtkWidget *drawing_area;
     DisplayChangeListener dcl;
+    QKbdState *kbd;
     DisplaySurface *ds;
     pixman_image_t *convert;
     cairo_surface_t *surface;
diff --git a/ui/gtk.c b/ui/gtk.c
index 579990b865..1be65025f2 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -122,17 +122,6 @@
 
 #define HOTKEY_MODIFIERS        (GDK_CONTROL_MASK | GDK_MOD1_MASK)
 
-static const int modifier_keycode[] = {
-    Q_KEY_CODE_SHIFT,
-    Q_KEY_CODE_SHIFT_R,
-    Q_KEY_CODE_CTRL,
-    Q_KEY_CODE_CTRL_R,
-    Q_KEY_CODE_ALT,
-    Q_KEY_CODE_ALT_R,
-    Q_KEY_CODE_META_L,
-    Q_KEY_CODE_META_R,
-};
-
 static const guint16 *keycode_map;
 static size_t keycode_maplen;
 
@@ -187,7 +176,6 @@ struct GtkDisplayState {
 
     bool external_pause_update;
 
-    bool modifier_pressed[ARRAY_SIZE(modifier_keycode)];
     bool ignore_keys;
 
     DisplayOptions *opts;
@@ -426,20 +414,12 @@ static void gd_update_full_redraw(VirtualConsole *vc)
 static void gtk_release_modifiers(GtkDisplayState *s)
 {
     VirtualConsole *vc = gd_vc_find_current(s);
-    int i, qcode;
 
     if (vc->type != GD_VC_GFX ||
         !qemu_console_is_graphic(vc->gfx.dcl.con)) {
         return;
     }
-    for (i = 0; i < ARRAY_SIZE(modifier_keycode); i++) {
-        qcode = modifier_keycode[i];
-        if (!s->modifier_pressed[i]) {
-            continue;
-        }
-        qemu_input_event_send_key_qcode(vc->gfx.dcl.con, qcode, false);
-        s->modifier_pressed[i] = false;
-    }
+    qkbd_state_lift_all_keys(vc->gfx.kbd);
 }
 
 static void gd_widget_reparent(GtkWidget *from, GtkWidget *to,
@@ -1113,7 +1093,6 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
     VirtualConsole *vc = opaque;
     GtkDisplayState *s = vc->s;
     int qcode;
-    int i;
 
     if (s->ignore_keys) {
         s->ignore_keys = (key->type == GDK_KEY_PRESS);
@@ -1134,8 +1113,8 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
         || key->hardware_keycode == VK_PAUSE
 #endif
         ) {
-        qemu_input_event_send_key_qcode(vc->gfx.dcl.con, Q_KEY_CODE_PAUSE,
-                                        key->type == GDK_KEY_PRESS);
+        qkbd_state_key_event(vc->gfx.kbd, Q_KEY_CODE_PAUSE,
+                             key->type == GDK_KEY_PRESS);
         return TRUE;
     }
 
@@ -1144,14 +1123,8 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
     trace_gd_key_event(vc->label, key->hardware_keycode, qcode,
                        (key->type == GDK_KEY_PRESS) ? "down" : "up");
 
-    for (i = 0; i < ARRAY_SIZE(modifier_keycode); i++) {
-        if (qcode == modifier_keycode[i]) {
-            s->modifier_pressed[i] = (key->type == GDK_KEY_PRESS);
-        }
-    }
-
-    qemu_input_event_send_key_qcode(vc->gfx.dcl.con, qcode,
-                                    key->type == GDK_KEY_PRESS);
+    qkbd_state_key_event(vc->gfx.kbd, qcode,
+                         key->type == GDK_KEY_PRESS);
 
     return TRUE;
 }
@@ -2052,6 +2025,7 @@ static GSList *gd_vc_gfx_init(GtkDisplayState *s, VirtualConsole *vc,
     gtk_notebook_append_page(GTK_NOTEBOOK(s->notebook),
                              vc->tab_item, gtk_label_new(vc->label));
 
+    vc->gfx.kbd = qkbd_state_init(con);
     vc->gfx.dcl.con = con;
     register_displaychangelistener(&vc->gfx.dcl);
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC PATCH v3 6/8] kbd-state: use state tracker for vnc
  2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
                   ` (4 preceding siblings ...)
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 5/8] kbd-state: use state tracker for gtk Gerd Hoffmann
@ 2019-01-22  9:28 ` Gerd Hoffmann
  2019-01-22  9:47   ` Daniel P. Berrangé
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 7/8] keymap: pass full keyboard state to keysym2scancode Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 8/8] keymap: fix keyup mappings Gerd Hoffmann
  7 siblings, 1 reply; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

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..81daa7a0eb 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;
+    QKbdState *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 9e4b2beb71..a9fe166ee8 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);
+    qkbd_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);
+    qkbd_state_key_event(vs->vd->kbd, qcode, true);
+    qkbd_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 &&
+            qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_CTRL) &&
+            qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_ALT)) {
             /* Reset the modifiers sent to the current console */
-            reset_keys(vs);
-            console_select(keycode - 0x02);
+            qkbd_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 (!qkbd_state_modifier_get(vs->vd->kbd, QKBD_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 (qkbd_state_modifier_get(vs->vd->kbd, QKBD_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 = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_SHIFT);
+        bool capslock = qkbd_state_modifier_get(vs->vd->kbd, QKBD_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]);
+    qkbd_state_key_event(vs->vd->kbd, qcode, down);
+    if (!qemu_console_is_graphic(NULL)) {
+        bool numlock = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_NUMLOCK);
+        bool control = qkbd_state_modifier_get(vs->vd->kbd, QKBD_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 = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_SHIFT);
+    bool altgr = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_ALTGR);
+    bool ctrl  = qkbd_state_modifier_get(vs->vd->kbd, QKBD_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 = qkbd_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) {
+        qkbd_state_free(vd->kbd);
         unregister_displaychangelistener(&vd->dcl);
         vd->dcl.con = con;
         register_displaychangelistener(&vd->dcl);
+        vd->kbd = qkbd_state_init(vd->dcl.con);
     }
+    qkbd_state_set_delay(vd->kbd, key_delay_ms);
 
     if (saddr == NULL) {
         goto cleanup;
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC PATCH v3 7/8] keymap: pass full keyboard state to keysym2scancode
  2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
                   ` (5 preceding siblings ...)
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 6/8] kbd-state: use state tracker for vnc Gerd Hoffmann
@ 2019-01-22  9:28 ` Gerd Hoffmann
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 8/8] keymap: fix keyup mappings Gerd Hoffmann
  7 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

Pass the keyboard state tracker handle down to keysym2scancode(),
so the code can fully inspect the keyboard state as needed.  No
functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 ui/keymaps.h | 3 ++-
 ui/curses.c  | 2 +-
 ui/keymaps.c | 8 ++++----
 ui/vnc.c     | 5 +----
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/ui/keymaps.h b/ui/keymaps.h
index 4e9c87fb8f..d8652b8a5a 100644
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -26,6 +26,7 @@
 #define QEMU_KEYMAPS_H
 
 #include "qemu-common.h"
+#include "ui/kbd-state.h"
 
 typedef struct {
     const char* name;
@@ -55,7 +56,7 @@ typedef struct kbd_layout_t kbd_layout_t;
 kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
                                    const char *language, Error **errp);
 int keysym2scancode(kbd_layout_t *k, int keysym,
-                    bool shift, bool altgr, bool ctrl);
+                    QKbdState *kbd);
 int keycode_is_keypad(kbd_layout_t *k, int keycode);
 int keysym_is_numlock(kbd_layout_t *k, int keysym);
 
diff --git a/ui/curses.c b/ui/curses.c
index f4e7a12f74..54687589d4 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -273,7 +273,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
             }
 
             keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK,
-                                      false, false, false);
+                                      NULL);
             if (keycode == 0)
                 continue;
 
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 6e44f738ed..c8b2135340 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -188,7 +188,7 @@ kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
 
 
 int keysym2scancode(kbd_layout_t *k, int keysym,
-                    bool shift, bool altgr, bool ctrl)
+                    QKbdState *kbd)
 {
     static const uint32_t mask =
         SCANCODE_SHIFT | SCANCODE_ALTGR | SCANCODE_CTRL;
@@ -220,13 +220,13 @@ int keysym2scancode(kbd_layout_t *k, int keysym,
      * If so, prefer that one.
      */
     mods = 0;
-    if (shift) {
+    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_SHIFT)) {
         mods |= SCANCODE_SHIFT;
     }
-    if (altgr) {
+    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_ALTGR)) {
         mods |= SCANCODE_ALTGR;
     }
-    if (ctrl) {
+    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_CTRL)) {
         mods |= SCANCODE_CTRL;
     }
 
diff --git a/ui/vnc.c b/ui/vnc.c
index a9fe166ee8..fc41f9b318 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1954,9 +1954,6 @@ static const char *code2name(int keycode)
 
 static void key_event(VncState *vs, int down, uint32_t sym)
 {
-    bool shift = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_SHIFT);
-    bool altgr = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_ALTGR);
-    bool ctrl  = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_CTRL);
     int keycode;
     int lsym = sym;
 
@@ -1965,7 +1962,7 @@ static void key_event(VncState *vs, int down, uint32_t sym)
     }
 
     keycode = keysym2scancode(vs->vd->kbd_layout, lsym & 0xFFFF,
-                              shift, altgr, ctrl) & SCANCODE_KEYMASK;
+                              vs->vd->kbd) & SCANCODE_KEYMASK;
     trace_vnc_key_event_map(down, sym, keycode, code2name(keycode));
     do_key_event(vs, down, keycode, sym);
 }
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [RFC PATCH v3 8/8] keymap: fix keyup mappings
  2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
                   ` (6 preceding siblings ...)
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 7/8] keymap: pass full keyboard state to keysym2scancode Gerd Hoffmann
@ 2019-01-22  9:28 ` Gerd Hoffmann
  7 siblings, 0 replies; 11+ messages in thread
From: Gerd Hoffmann @ 2019-01-22  9:28 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, Gerd Hoffmann

It is possible that the modifier state on keyup is different from the
modifier state on keydown.  In that case the keycode lookup can end up
with different keys in case multiple keysym -> keycode mappings exist,
because it picks the mapping depending on modifier state.

To fix that change the lookup logic for keyup events.  Instead of
looking at the modifier state check the key state and prefer a keycodes
where the key is in "down" state right now.

Fixes: abb4f2c965 keymap: consider modifier state when picking a mapping
Buglink: https://bugs.launchpad.net/bugs/1738283
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1658676
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 ui/keymaps.h |  2 +-
 ui/curses.c  |  2 +-
 ui/keymaps.c | 55 ++++++++++++++++++++++++++++++++++---------------------
 ui/vnc.c     |  2 +-
 4 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/ui/keymaps.h b/ui/keymaps.h
index d8652b8a5a..b6d48aac40 100644
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -56,7 +56,7 @@ typedef struct kbd_layout_t kbd_layout_t;
 kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
                                    const char *language, Error **errp);
 int keysym2scancode(kbd_layout_t *k, int keysym,
-                    QKbdState *kbd);
+                    QKbdState *kbd, bool down);
 int keycode_is_keypad(kbd_layout_t *k, int keycode);
 int keysym_is_numlock(kbd_layout_t *k, int keysym);
 
diff --git a/ui/curses.c b/ui/curses.c
index 54687589d4..6e0091c3b2 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -273,7 +273,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
             }
 
             keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK,
-                                      NULL);
+                                      NULL, false);
             if (keycode == 0)
                 continue;
 
diff --git a/ui/keymaps.c b/ui/keymaps.c
index c8b2135340..544b55c27b 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,6 +28,7 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "ui/input.h"
 
 struct keysym2code {
     uint32_t count;
@@ -188,7 +189,7 @@ kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
 
 
 int keysym2scancode(kbd_layout_t *k, int keysym,
-                    QKbdState *kbd)
+                    QKbdState *kbd, bool down)
 {
     static const uint32_t mask =
         SCANCODE_SHIFT | SCANCODE_ALTGR | SCANCODE_CTRL;
@@ -212,27 +213,39 @@ int keysym2scancode(kbd_layout_t *k, int keysym,
         return keysym2code->keycodes[0];
     }
 
-    /*
-     * We have multiple keysym -> keycode mappings.
-     *
-     * Check whenever we find one mapping where the modifier state of
-     * the mapping matches the current user interface modifier state.
-     * If so, prefer that one.
-     */
-    mods = 0;
-    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_SHIFT)) {
-        mods |= SCANCODE_SHIFT;
-    }
-    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_ALTGR)) {
-        mods |= SCANCODE_ALTGR;
-    }
-    if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_CTRL)) {
-        mods |= SCANCODE_CTRL;
-    }
+    /* We have multiple keysym -> keycode mappings. */
+    if (down) {
+        /*
+         * On keydown: Check whenever we find one mapping where the
+         * modifier state of the mapping matches the current user
+         * interface modifier state.  If so, prefer that one.
+         */
+        mods = 0;
+        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_SHIFT)) {
+            mods |= SCANCODE_SHIFT;
+        }
+        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_ALTGR)) {
+            mods |= SCANCODE_ALTGR;
+        }
+        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_CTRL)) {
+            mods |= SCANCODE_CTRL;
+        }
 
-    for (i = 0; i < keysym2code->count; i++) {
-        if ((keysym2code->keycodes[i] & mask) == mods) {
-            return keysym2code->keycodes[i];
+        for (i = 0; i < keysym2code->count; i++) {
+            if ((keysym2code->keycodes[i] & mask) == mods) {
+                return keysym2code->keycodes[i];
+            }
+        }
+    } else {
+        /*
+         * On keyup: Try find a key which is actually down.
+         */
+        for (i = 0; i < keysym2code->count; i++) {
+            QKeyCode qcode = qemu_input_key_number_to_qcode
+                (keysym2code->keycodes[i]);
+            if (kbd && qkbd_state_key_get(kbd, qcode)) {
+                return keysym2code->keycodes[i];
+            }
         }
     }
     return keysym2code->keycodes[0];
diff --git a/ui/vnc.c b/ui/vnc.c
index fc41f9b318..bc730496ae 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1962,7 +1962,7 @@ static void key_event(VncState *vs, int down, uint32_t sym)
     }
 
     keycode = keysym2scancode(vs->vd->kbd_layout, lsym & 0xFFFF,
-                              vs->vd->kbd) & SCANCODE_KEYMASK;
+                              vs->vd->kbd, down) & SCANCODE_KEYMASK;
     trace_vnc_key_event_map(down, sym, keycode, code2name(keycode));
     do_key_event(vs, down, keycode, sym);
 }
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH v3 1/8] kbd-state: add keyboard state tracker
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 1/8] kbd-state: add keyboard state tracker Gerd Hoffmann
@ 2019-01-22  9:47   ` Daniel P. Berrangé
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2019-01-22  9:47 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Tue, Jan 22, 2019 at 10:28:07AM +0100, Gerd Hoffmann wrote:
> Now that most user interfaces are using QKeyCodes it is easier to have
> common keyboard code useable by all user interfaces.
> 
> This patch adds helper code to track the state of all keyboard keys,
> using a bitmap indexed by QKeyCode.  Modifier state is tracked too,
> as separate bitmap.  That makes checking modifier state easier.
> Likewise we can easily apply special handling for capslock & numlock
> (toggles on keypress) and ctrl + shift (we have two keys for that).
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/ui/kbd-state.h |  96 +++++++++++++++++++++++++++++++++++++
>  ui/kbd-state.c         | 125 +++++++++++++++++++++++++++++++++++++++++++++++++
>  ui/Makefile.objs       |   2 +-
>  3 files changed, 222 insertions(+), 1 deletion(-)
>  create mode 100644 include/ui/kbd-state.h
>  create mode 100644 ui/kbd-state.c
> 
> diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
> new file mode 100644
> index 0000000000..277198f169
> --- /dev/null
> +++ b/include/ui/kbd-state.h
> @@ -0,0 +1,96 @@

Missing license boilerplate header, and same in the .c file.

> +#ifndef QEMU_UI_KBD_STATE_H
> +#define QEMU_UI_KBD_STATE_H 1

With license header added

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [RFC PATCH v3 6/8] kbd-state: use state tracker for vnc
  2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 6/8] kbd-state: use state tracker for vnc Gerd Hoffmann
@ 2019-01-22  9:47   ` Daniel P. Berrangé
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2019-01-22  9:47 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Tue, Jan 22, 2019 at 10:28:12AM +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(-)

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-01-22  9:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22  9:28 [Qemu-devel] [RFC PATCH v3 0/8] ui: add generic keyboard state tracker, fix keymap Gerd Hoffmann
2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 1/8] kbd-state: add keyboard state tracker Gerd Hoffmann
2019-01-22  9:47   ` Daniel P. Berrangé
2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 2/8] sdl2: remove sdl2_reset_keys() function Gerd Hoffmann
2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 3/8] kbd-state: use state tracker for sdl2 Gerd Hoffmann
2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 4/8] sdl2: use only QKeyCode in sdl2_process_key() Gerd Hoffmann
2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 5/8] kbd-state: use state tracker for gtk Gerd Hoffmann
2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 6/8] kbd-state: use state tracker for vnc Gerd Hoffmann
2019-01-22  9:47   ` Daniel P. Berrangé
2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 7/8] keymap: pass full keyboard state to keysym2scancode Gerd Hoffmann
2019-01-22  9:28 ` [Qemu-devel] [RFC PATCH v3 8/8] keymap: fix keyup mappings Gerd Hoffmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.