[PATCH 0/2] ui/kbd-state: QAPI'fy QKbdModifier

[PATCH 0/2] ui/kbd-state: QAPI'fy QKbdModifier

[Philippe Mathieu-Daudé]

QAPI seems designed to maintain such enums,
so convert QKbdModifier to be QAPI generated.
Besides, this is how QKeyCode is maintained.

Philippe Mathieu-Daudé (2):
  ui/kbd-state: Rename QKbdModifier enum definitions
  ui/kbd-state: QAPI'fy QKbdModifier

 include/ui/kbd-state.h | 16 ----------------
 qapi/ui.json           | 10 ++++++++++
 ui/cocoa.m             |  2 +-
 ui/kbd-state.c         | 14 +++++++-------
 ui/keymaps.c           |  6 +++---
 ui/sdl2-input.c        |  2 +-
 ui/vnc.c               | 16 ++++++++--------
 7 files changed, 30 insertions(+), 36 deletions(-)

-- 
2.38.1

[PATCH 1/2] ui/kbd-state: Rename QKbdModifier enum definitions

[Philippe Mathieu-Daudé]

In order to QAPI'fy QKbdModifier in the next commit,
rename its definitions to a QAPI-ready format.

Mechanical change doing:

  $ sed -i -e s/QKBD_MOD_/Q_KBD_MODIFIER_/g $(git grep -l QKBD_MOD_)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/ui/kbd-state.h | 16 ++++++++--------
 ui/cocoa.m             |  2 +-
 ui/kbd-state.c         | 14 +++++++-------
 ui/keymaps.c           |  6 +++---
 ui/sdl2-input.c        |  2 +-
 ui/vnc.c               | 16 ++++++++--------
 6 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
index eb9067dd53..14ee2c59e4 100644
--- a/include/ui/kbd-state.h
+++ b/include/ui/kbd-state.h
@@ -12,17 +12,17 @@
 typedef enum QKbdModifier QKbdModifier;
 
 enum QKbdModifier {
-    QKBD_MOD_NONE = 0,
+    Q_KBD_MODIFIER_NONE = 0,
 
-    QKBD_MOD_SHIFT,
-    QKBD_MOD_CTRL,
-    QKBD_MOD_ALT,
-    QKBD_MOD_ALTGR,
+    Q_KBD_MODIFIER_SHIFT,
+    Q_KBD_MODIFIER_CTRL,
+    Q_KBD_MODIFIER_ALT,
+    Q_KBD_MODIFIER_ALTGR,
 
-    QKBD_MOD_NUMLOCK,
-    QKBD_MOD_CAPSLOCK,
+    Q_KBD_MODIFIER_NUMLOCK,
+    Q_KBD_MODIFIER_CAPSLOCK,
 
-    QKBD_MOD__MAX
+    Q_KBD_MODIFIER__MAX
 };
 
 typedef struct QKbdState QKbdState;
diff --git a/ui/cocoa.m b/ui/cocoa.m
index e915c344a8..2c12fc58ea 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -840,7 +840,7 @@ - (bool) handleEventLocked:(NSEvent *)event
      *   this implementation usable enough.
      */
     if (!!(modifiers & NSEventModifierFlagCapsLock) !=
-        qkbd_state_modifier_get(kbd, QKBD_MOD_CAPSLOCK)) {
+        qkbd_state_modifier_get(kbd, Q_KBD_MODIFIER_CAPSLOCK)) {
         qkbd_state_key_event(kbd, Q_KEY_CODE_CAPS_LOCK, true);
         qkbd_state_key_event(kbd, Q_KEY_CODE_CAPS_LOCK, false);
     }
diff --git a/ui/kbd-state.c b/ui/kbd-state.c
index 62d42a7a22..cc53b5e135 100644
--- a/ui/kbd-state.c
+++ b/ui/kbd-state.c
@@ -13,7 +13,7 @@ struct QKbdState {
     QemuConsole *con;
     int key_delay_ms;
     DECLARE_BITMAP(keys, Q_KEY_CODE__MAX);
-    DECLARE_BITMAP(mods, QKBD_MOD__MAX);
+    DECLARE_BITMAP(mods, Q_KBD_MODIFIER__MAX);
 };
 
 static void qkbd_state_modifier_update(QKbdState *kbd,
@@ -67,29 +67,29 @@ void qkbd_state_key_event(QKbdState *kbd, QKeyCode qcode, bool down)
     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);
+                                   Q_KBD_MODIFIER_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);
+                                   Q_KBD_MODIFIER_CTRL);
         break;
     case Q_KEY_CODE_ALT:
         qkbd_state_modifier_update(kbd, Q_KEY_CODE_ALT, Q_KEY_CODE_ALT,
-                                   QKBD_MOD_ALT);
+                                   Q_KBD_MODIFIER_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);
+                                   Q_KBD_MODIFIER_ALTGR);
         break;
     case Q_KEY_CODE_CAPS_LOCK:
         if (down) {
-            change_bit(QKBD_MOD_CAPSLOCK, kbd->mods);
+            change_bit(Q_KBD_MODIFIER_CAPSLOCK, kbd->mods);
         }
         break;
     case Q_KEY_CODE_NUM_LOCK:
         if (down) {
-            change_bit(QKBD_MOD_NUMLOCK, kbd->mods);
+            change_bit(Q_KBD_MODIFIER_NUMLOCK, kbd->mods);
         }
         break;
     default:
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 6ceaa97085..c8b99eaf4a 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -222,13 +222,13 @@ int keysym2scancode(kbd_layout_t *k, int keysym,
          * interface modifier state.  If so, prefer that one.
          */
         mods = 0;
-        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_SHIFT)) {
+        if (kbd && qkbd_state_modifier_get(kbd, Q_KBD_MODIFIER_SHIFT)) {
             mods |= SCANCODE_SHIFT;
         }
-        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_ALTGR)) {
+        if (kbd && qkbd_state_modifier_get(kbd, Q_KBD_MODIFIER_ALTGR)) {
             mods |= SCANCODE_ALTGR;
         }
-        if (kbd && qkbd_state_modifier_get(kbd, QKBD_MOD_CTRL)) {
+        if (kbd && qkbd_state_modifier_get(kbd, Q_KBD_MODIFIER_CTRL)) {
             mods |= SCANCODE_CTRL;
         }
 
diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index f068382209..24c920e3af 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -44,7 +44,7 @@ void sdl2_process_key(struct sdl2_console *scon,
     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);
+        bool ctrl = qkbd_state_modifier_get(scon->kbd, Q_KBD_MODIFIER_CTRL);
         if (ev->type == SDL_KEYDOWN) {
             switch (qcode) {
             case Q_KEY_CODE_RET:
diff --git a/ui/vnc.c b/ui/vnc.c
index d9eacad759..a10d0b7b20 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1877,8 +1877,8 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
     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)) {
+            qkbd_state_modifier_get(vs->vd->kbd, Q_KBD_MODIFIER_CTRL) &&
+            qkbd_state_modifier_get(vs->vd->kbd, Q_KBD_MODIFIER_ALT)) {
             /* Reset the modifiers sent to the current console */
             qkbd_state_lift_all_keys(vs->vd->kbd);
             console_select(qcode - Q_KEY_CODE_1);
@@ -1899,12 +1899,12 @@ 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 (!qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_NUMLOCK)) {
+            if (!qkbd_state_modifier_get(vs->vd->kbd, Q_KBD_MODIFIER_NUMLOCK)) {
                 trace_vnc_key_sync_numlock(true);
                 press_key(vs, Q_KEY_CODE_NUM_LOCK);
             }
         } else {
-            if (qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_NUMLOCK)) {
+            if (qkbd_state_modifier_get(vs->vd->kbd, Q_KBD_MODIFIER_NUMLOCK)) {
                 trace_vnc_key_sync_numlock(false);
                 press_key(vs, Q_KEY_CODE_NUM_LOCK);
             }
@@ -1919,8 +1919,8 @@ 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');
-        bool shift = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_SHIFT);
-        bool capslock = qkbd_state_modifier_get(vs->vd->kbd, QKBD_MOD_CAPSLOCK);
+        bool shift = qkbd_state_modifier_get(vs->vd->kbd, Q_KBD_MODIFIER_SHIFT);
+        bool capslock = qkbd_state_modifier_get(vs->vd->kbd, Q_KBD_MODIFIER_CAPSLOCK);
         if (capslock) {
             if (uppercase == shift) {
                 trace_vnc_key_sync_capslock(false);
@@ -1936,8 +1936,8 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
 
     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);
+        bool numlock = qkbd_state_modifier_get(vs->vd->kbd, Q_KBD_MODIFIER_NUMLOCK);
+        bool control = qkbd_state_modifier_get(vs->vd->kbd, Q_KBD_MODIFIER_CTRL);
         /* QEMU console emulation */
         if (down) {
             switch (keycode) {
-- 
2.38.1

[PATCH 2/2] ui/kbd-state: QAPI'fy QKbdModifier

[Philippe Mathieu-Daudé]

Let QAPI maintain the QKbdModifier enum values.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/ui/kbd-state.h | 16 ----------------
 qapi/ui.json           | 10 ++++++++++
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
index 14ee2c59e4..83e869bc09 100644
--- a/include/ui/kbd-state.h
+++ b/include/ui/kbd-state.h
@@ -9,22 +9,6 @@
 
 #include "qapi/qapi-types-ui.h"
 
-typedef enum QKbdModifier QKbdModifier;
-
-enum QKbdModifier {
-    Q_KBD_MODIFIER_NONE = 0,
-
-    Q_KBD_MODIFIER_SHIFT,
-    Q_KBD_MODIFIER_CTRL,
-    Q_KBD_MODIFIER_ALT,
-    Q_KBD_MODIFIER_ALTGR,
-
-    Q_KBD_MODIFIER_NUMLOCK,
-    Q_KBD_MODIFIER_CAPSLOCK,
-
-    Q_KBD_MODIFIER__MAX
-};
-
 typedef struct QKbdState QKbdState;
 
 /**
diff --git a/qapi/ui.json b/qapi/ui.json
index 0abba3e930..d3230a6bc7 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -920,6 +920,16 @@
             'ac_home', 'ac_back', 'ac_forward', 'ac_refresh', 'ac_bookmarks',
             'lang1', 'lang2' ] }
 
+##
+# @QKbdModifier:
+#
+# An enumeration of keyboard modifier name.
+#
+# Since: 8.0
+##
+{ 'enum': 'QKbdModifier',
+  'data': [ 'none', 'shift', 'ctrl', 'alt', 'altgr', 'numlock', 'capslock' ] }
+
 ##
 # @KeyValueKind:
 #
-- 
2.38.1

Re: [PATCH 0/2] ui/kbd-state: QAPI'fy QKbdModifier

[Akihiko Odaki]

On 2023/02/24 20:01, Philippe Mathieu-Daudé wrote:
> QAPI seems designed to maintain such enums,
> so convert QKbdModifier to be QAPI generated.
> Besides, this is how QKeyCode is maintained.

I recognize QkbdModifier as more like an internal detail of displays so 
I'm not convinced it should be converted to QAPI.

The interface of QEMU's input subsystem is so simple: send key up or key 
down events for QKeyCode. The modifiers are not exceptions. However, 
some display backends (cocoa, sdl2, and vnc) are not designed this way, 
and has internal states for modifiers. For such displays, QkbdState 
maintains the states to help them convert their internal key state 
representation to key up/down events of QKeyCode.

QKbdModifier is used by displays only to query these internal states 
QkbdState holds. As such, the definition of QKbdModifier is very 
dependent of the internal working of displays. It is particularly 
designed to match the needs of vnc, and I even wonder if the modifier 
state tracking should be moved away from the common code of QkbdState to 
vnc.

Regards,
Akihiko Odaki

> 
> Philippe Mathieu-Daudé (2):
>    ui/kbd-state: Rename QKbdModifier enum definitions
>    ui/kbd-state: QAPI'fy QKbdModifier
> 
>   include/ui/kbd-state.h | 16 ----------------
>   qapi/ui.json           | 10 ++++++++++
>   ui/cocoa.m             |  2 +-
>   ui/kbd-state.c         | 14 +++++++-------
>   ui/keymaps.c           |  6 +++---
>   ui/sdl2-input.c        |  2 +-
>   ui/vnc.c               | 16 ++++++++--------
>   7 files changed, 30 insertions(+), 36 deletions(-)
>