Patches for ui/gtk and ui/sdl

Patches for ui/gtk and ui/sdl

[Volker Rümelin]

It's rather difficult to test qemu patches in guests on Windows with important keys missing. These patches mainly fix the guest keyboard on Windows.

With best regards,
Volker

[PATCH 01/10] ui/win32-kbd-hook: handle AltGr in a hook procedure

[Volker Rümelin]

Import win32 keyboard hooking code from project spice-gtk. This
patch removes the extra left control key up/down input events
inserted by Windows for the right alt key up/down input events
with international keyboard layouts. Additionally there's some
code to grab the keyboard.

The next patches will use this code.

Only Windows needs this.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/Makefile.objs    |   1 +
 ui/win32-kbd-hook.c | 100 ++++++++++++++++++++++++++++++++++++++++++++
 ui/win32-kbd-hook.h |  14 +++++++
 3 files changed, 115 insertions(+)
 create mode 100644 ui/win32-kbd-hook.c
 create mode 100644 ui/win32-kbd-hook.h

diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index e6da6ff047..f74d72a612 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -15,6 +15,7 @@ common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
 common-obj-$(CONFIG_COCOA) += cocoa.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
 common-obj-$(call lnot,$(CONFIG_VNC)) += vnc-stubs.o
+common-obj-$(CONFIG_WIN32) += win32-kbd-hook.o
 
 # ui-sdl module
 common-obj-$(CONFIG_SDL) += sdl.mo
diff --git a/ui/win32-kbd-hook.c b/ui/win32-kbd-hook.c
new file mode 100644
index 0000000000..d558912ef2
--- /dev/null
+++ b/ui/win32-kbd-hook.c
@@ -0,0 +1,100 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.  See the COPYING file in the
+ * top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
+#include "win32-kbd-hook.h"
+
+static Notifier win32_unhook_notifier;
+static HHOOK win32_keyboard_hook;
+static HWND win32_window;
+static DWORD win32_grab;
+
+static LRESULT CALLBACK keyboard_hook_cb(int code, WPARAM wparam, LPARAM lparam)
+{
+    if  (win32_window && code == HC_ACTION && win32_window == GetFocus()) {
+        KBDLLHOOKSTRUCT *hooked = (KBDLLHOOKSTRUCT *)lparam;
+
+        if (wparam != WM_KEYUP) {
+            DWORD dwmsg = (hooked->flags << 24) |
+                          ((hooked->scanCode & 0xff) << 16) | 1;
+
+            switch (hooked->vkCode) {
+            case VK_CAPITAL:
+                /* fall through */
+            case VK_SCROLL:
+                /* fall through */
+            case VK_NUMLOCK:
+                /* fall through */
+            case VK_LSHIFT:
+                /* fall through */
+            case VK_RSHIFT:
+                /* fall through */
+            case VK_RCONTROL:
+                /* fall through */
+            case VK_LMENU:
+                /* fall through */
+            case VK_RMENU:
+                break;
+
+            case VK_LCONTROL:
+                /*
+                 * When pressing AltGr, an extra VK_LCONTROL with a special
+                 * scancode with bit 9 set is sent. Let's ignore the extra
+                 * VK_LCONTROL, as that will make AltGr misbehave.
+                 */
+                if (hooked->scanCode & 0x200) {
+                    return 1;
+                }
+                break;
+
+            default:
+                if (win32_grab) {
+                    SendMessage(win32_window, wparam, hooked->vkCode, dwmsg);
+                    return 1;
+                }
+                break;
+            }
+
+        } else {
+            switch (hooked->vkCode) {
+            case VK_LCONTROL:
+                if (hooked->scanCode & 0x200) {
+                    return 1;
+                }
+                break;
+            }
+        }
+    }
+
+    return CallNextHookEx(NULL, code, wparam, lparam);
+}
+
+static void keyboard_hook_unhook(Notifier *n, void *data)
+{
+    UnhookWindowsHookEx(win32_keyboard_hook);
+    win32_keyboard_hook = NULL;
+}
+
+void win32_kbd_set_window(HWND hwnd)
+{
+    if (hwnd && !win32_keyboard_hook) {
+        /* note: the installing thread must have a message loop */
+        win32_keyboard_hook = SetWindowsHookEx(WH_KEYBOARD_LL, keyboard_hook_cb,
+                                               GetModuleHandle(NULL), 0);
+        if (win32_keyboard_hook) {
+            win32_unhook_notifier.notify = keyboard_hook_unhook;
+            qemu_add_exit_notifier(&win32_unhook_notifier);
+        }
+    }
+
+    win32_window = hwnd;
+}
+
+void win32_kbd_set_grab(bool grab)
+{
+    win32_grab = grab;
+}
diff --git a/ui/win32-kbd-hook.h b/ui/win32-kbd-hook.h
new file mode 100644
index 0000000000..1e0e866610
--- /dev/null
+++ b/ui/win32-kbd-hook.h
@@ -0,0 +1,14 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef UI_WIN32_KBD_HOOK_H
+#define UI_WIN32_KBD_HOOK_H
+
+void win32_kbd_set_window(HWND hwnd);
+void win32_kbd_set_grab(bool grab);
+
+#endif
-- 
2.26.1

Re: [PATCH 01/10] ui/win32-kbd-hook: handle AltGr in a hook procedure

[Gerd Hoffmann]

On Sun, May 10, 2020 at 08:42:55PM +0200, Volker Rümelin wrote:
> Import win32 keyboard hooking code from project spice-gtk.

> +/*
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * (at your option) any later version.  See the COPYING file in the
> + * top-level directory.
> + */

Can you add the import note to the comment too?

> +void win32_kbd_set_window(HWND hwnd);

Can we use uint32_t here (or whatever HWND actually is) so the header
can also be included on non-win32 machines?

thanks,
  Gerd

[PATCH 02/10] ui/gtk: fix handling of AltGr key on Windows

[Volker Rümelin]

Wire up the keyboard hooking code on Windows to fix the AltGr
key and improve keyboard grabbing.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/gtk.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 83f2f5d49b..68c63532b1 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -38,6 +38,10 @@
 
 #include "ui/console.h"
 #include "ui/gtk.h"
+#ifdef CONFIG_WIN32
+#include <gdk/gdkwin32.h>
+#include "win32-kbd-hook.h"
+#endif
 
 #include <glib/gi18n.h>
 #include <locale.h>
@@ -1451,6 +1455,9 @@ static void gd_grab_keyboard(VirtualConsole *vc, const char *reason)
         }
     }
 
+#ifdef CONFIG_WIN32
+    win32_kbd_set_grab(true);
+#endif
 #if GTK_CHECK_VERSION(3, 20, 0)
     gd_grab_update(vc, true, vc->s->ptr_owner == vc);
 #else
@@ -1472,6 +1479,9 @@ static void gd_ungrab_keyboard(GtkDisplayState *s)
     }
     s->kbd_owner = NULL;
 
+#ifdef CONFIG_WIN32
+    win32_kbd_set_grab(false);
+#endif
 #if GTK_CHECK_VERSION(3, 20, 0)
     gd_grab_update(vc, false, vc->s->ptr_owner == vc);
 #else
@@ -1614,12 +1624,28 @@ static gboolean gd_leave_event(GtkWidget *widget, GdkEventCrossing *crossing,
     return TRUE;
 }
 
+static gboolean gd_focus_in_event(GtkWidget *widget,
+                                  GdkEventFocus *event, gpointer opaque)
+{
+#ifdef CONFIG_WIN32
+    VirtualConsole *vc = opaque;
+    GtkDisplayState *s = vc->s;
+
+    win32_kbd_set_window(gdk_win32_window_get_impl_hwnd(
+        gtk_widget_get_window(vc->window ? vc->window : s->window)));
+#endif
+    return TRUE;
+}
+
 static gboolean gd_focus_out_event(GtkWidget *widget,
-                                   GdkEventCrossing *crossing, gpointer opaque)
+                                   GdkEventFocus *event, gpointer opaque)
 {
     VirtualConsole *vc = opaque;
     GtkDisplayState *s = vc->s;
 
+#ifdef CONFIG_WIN32
+    win32_kbd_set_window(NULL);
+#endif
     gtk_release_modifiers(s);
     return TRUE;
 }
@@ -1878,6 +1904,8 @@ static void gd_connect_vc_gfx_signals(VirtualConsole *vc)
                          G_CALLBACK(gd_enter_event), vc);
         g_signal_connect(vc->gfx.drawing_area, "leave-notify-event",
                          G_CALLBACK(gd_leave_event), vc);
+        g_signal_connect(vc->gfx.drawing_area, "focus-in-event",
+                         G_CALLBACK(gd_focus_in_event), vc);
         g_signal_connect(vc->gfx.drawing_area, "focus-out-event",
                          G_CALLBACK(gd_focus_out_event), vc);
         g_signal_connect(vc->gfx.drawing_area, "configure-event",
-- 
2.26.1

Re: [PATCH 02/10] ui/gtk: fix handling of AltGr key on Windows

[Gerd Hoffmann]

  Hi,

> +#ifdef CONFIG_WIN32
> +    win32_kbd_set_grab(true);
> +#endif

Can we have stubs for these (see stubs/ directory for examples) so we
can avoid the #ifdefs?

thanks,
  Gerd

[PATCH 03/10] ui/gkt: release all keys on grab-broken-event

[Volker Rümelin]

There is no way to grab the Ctrl-Alt-Del key combination on
Windows. This key combination will leave all three keys in a
stuck condition. This patch uses the grab-broken-event to
release the keys.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/gtk.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 68c63532b1..5de2a75691 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1132,6 +1132,20 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
     return TRUE;
 }
 
+static gboolean gd_grab_broken_event(GtkWidget *widget,
+                                     GdkEventGrabBroken *event, void *opaque)
+{
+#ifdef CONFIG_WIN32
+    if (event->keyboard) {
+        VirtualConsole *vc = opaque;
+        GtkDisplayState *s = vc->s;
+
+        gtk_release_modifiers(s);
+    }
+#endif
+    return TRUE;
+}
+
 static gboolean gd_event(GtkWidget *widget, GdkEvent *event, void *opaque)
 {
     if (event->type == GDK_MOTION_NOTIFY) {
@@ -1910,6 +1924,8 @@ static void gd_connect_vc_gfx_signals(VirtualConsole *vc)
                          G_CALLBACK(gd_focus_out_event), vc);
         g_signal_connect(vc->gfx.drawing_area, "configure-event",
                          G_CALLBACK(gd_configure), vc);
+        g_signal_connect(vc->gfx.drawing_area, "grab-broken-event",
+                         G_CALLBACK(gd_grab_broken_event), vc);
     } else {
         g_signal_connect(vc->gfx.drawing_area, "key-press-event",
                          G_CALLBACK(gd_text_key_down), vc);
-- 
2.26.1

Re: [PATCH 03/10] ui/gkt: release all keys on grab-broken-event

[Gerd Hoffmann]

On Sun, May 10, 2020 at 08:42:57PM +0200, Volker Rümelin wrote:
> There is no way to grab the Ctrl-Alt-Del key combination on
> Windows. [ ... ]

> +#ifdef CONFIG_WIN32

A comment here would be good (so you can see the reason for this without
digging into the git commit log).

thanks,
  Gerd

[PATCH 04/10] ui/gtk: remove unused code

[Volker Rümelin]

This code was last used before commit 2ec78706d1 "ui: convert
GTK and SDL1 frontends to keycodemapdb".

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/gtk.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 5de2a75691..c70bfc2be4 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -112,15 +112,6 @@
 # define VTE_CHECK_VERSION(a, b, c) 0
 #endif
 
-/* Some older mingw versions lack this constant or have
- * it conditionally defined */
-#ifdef _WIN32
-# ifndef MAPVK_VK_TO_VSC
-#  define MAPVK_VK_TO_VSC 0
-# endif
-#endif
-
-
 #define HOTKEY_MODIFIERS        (GDK_CONTROL_MASK | GDK_MOD1_MASK)
 
 static const guint16 *keycode_map;
-- 
2.26.1

Re: [PATCH 04/10] ui/gtk: remove unused code

[Philippe Mathieu-Daudé]

On 5/10/20 8:42 PM, Volker Rümelin wrote:
> This code was last used before commit 2ec78706d1 "ui: convert
> GTK and SDL1 frontends to keycodemapdb".
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>   ui/gtk.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 5de2a75691..c70bfc2be4 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -112,15 +112,6 @@
>   # define VTE_CHECK_VERSION(a, b, c) 0
>   #endif
>   
> -/* Some older mingw versions lack this constant or have
> - * it conditionally defined */
> -#ifdef _WIN32
> -# ifndef MAPVK_VK_TO_VSC
> -#  define MAPVK_VK_TO_VSC 0
> -# endif
> -#endif
> -
> -
>   #define HOTKEY_MODIFIERS        (GDK_CONTROL_MASK | GDK_MOD1_MASK)
>   
>   static const guint16 *keycode_map;
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 05/10] ui/gtk: remove unused variable ignore_keys

[Volker Rümelin]

Since the removal of GTK2 code the code around ignore_keys is
unused. See commit 1a01716a30 "gtk: Avoid accel key leakage
into guest on console switch" why it was needed before.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/gtk.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index c70bfc2be4..5a25e3fa4c 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -168,8 +168,6 @@ struct GtkDisplayState {
 
     bool external_pause_update;
 
-    bool ignore_keys;
-
     DisplayOptions *opts;
 };
 
@@ -1085,14 +1083,8 @@ static gboolean gd_text_key_down(GtkWidget *widget,
 static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
 {
     VirtualConsole *vc = opaque;
-    GtkDisplayState *s = vc->s;
     int qcode;
 
-    if (s->ignore_keys) {
-        s->ignore_keys = (key->type == GDK_KEY_PRESS);
-        return TRUE;
-    }
-
 #ifdef WIN32
     /* on windows, we ought to ignore the reserved key event? */
     if (key->hardware_keycode == 0xff)
@@ -1189,7 +1181,6 @@ static void gd_menu_switch_vc(GtkMenuItem *item, void *opaque)
         gtk_notebook_set_current_page(nb, page);
         gtk_widget_grab_focus(vc->focus);
     }
-    s->ignore_keys = false;
 }
 
 static void gd_accel_switch_vc(void *opaque)
-- 
2.26.1

Re: [PATCH 05/10] ui/gtk: remove unused variable ignore_keys

[Philippe Mathieu-Daudé]

On 5/10/20 8:42 PM, Volker Rümelin wrote:
> Since the removal of GTK2 code 

"... in commit 89d85cde7 ..."

> the code around ignore_keys is
> unused. See commit 1a01716a30 "gtk: Avoid accel key leakage
> into guest on console switch" why it was needed before.
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>

With description updated:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   ui/gtk.c | 9 ---------
>   1 file changed, 9 deletions(-)
> 
> diff --git a/ui/gtk.c b/ui/gtk.c
> index c70bfc2be4..5a25e3fa4c 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -168,8 +168,6 @@ struct GtkDisplayState {
>   
>       bool external_pause_update;
>   
> -    bool ignore_keys;
> -
>       DisplayOptions *opts;
>   };
>   
> @@ -1085,14 +1083,8 @@ static gboolean gd_text_key_down(GtkWidget *widget,
>   static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
>   {
>       VirtualConsole *vc = opaque;
> -    GtkDisplayState *s = vc->s;
>       int qcode;
>   
> -    if (s->ignore_keys) {
> -        s->ignore_keys = (key->type == GDK_KEY_PRESS);
> -        return TRUE;
> -    }
> -
>   #ifdef WIN32
>       /* on windows, we ought to ignore the reserved key event? */
>       if (key->hardware_keycode == 0xff)
> @@ -1189,7 +1181,6 @@ static void gd_menu_switch_vc(GtkMenuItem *item, void *opaque)
>           gtk_notebook_set_current_page(nb, page);
>           gtk_widget_grab_focus(vc->focus);
>       }
> -    s->ignore_keys = false;
>   }
>   
>   static void gd_accel_switch_vc(void *opaque)
> 

[PATCH 06/10] ui/sdl2: fix handling of AltGr key on Windows

[Volker Rümelin]

Wire up the keyboard hooking code on Windows to fix the AltGr
key and improve keyboard grabbing.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/sdl2.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 3c9424eb42..ec1cb8131f 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -30,6 +30,9 @@
 #include "ui/sdl2.h"
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
+#ifdef CONFIG_WIN32
+#include "win32-kbd-hook.h"
+#endif
 
 static int sdl2_num_outputs;
 static struct sdl2_console *sdl2_console;
@@ -220,6 +223,9 @@ static void sdl_grab_start(struct sdl2_console *scon)
     }
     SDL_SetWindowGrab(scon->real_window, SDL_TRUE);
     gui_grab = 1;
+#ifdef CONFIG_WIN32
+    win32_kbd_set_grab(true);
+#endif
     sdl_update_caption(scon);
 }
 
@@ -227,6 +233,9 @@ static void sdl_grab_end(struct sdl2_console *scon)
 {
     SDL_SetWindowGrab(scon->real_window, SDL_FALSE);
     gui_grab = 0;
+#ifdef CONFIG_WIN32
+    win32_kbd_set_grab(false);
+#endif
     sdl_show_cursor(scon);
     sdl_update_caption(scon);
 }
@@ -532,6 +541,18 @@ static void handle_windowevent(SDL_Event *ev)
         sdl2_redraw(scon);
         break;
     case SDL_WINDOWEVENT_FOCUS_GAINED:
+#ifdef CONFIG_WIN32
+        if (qemu_console_is_graphic(scon->dcl.con)) {
+            SDL_SysWMinfo info;
+
+            SDL_VERSION(&info.version);
+            if (SDL_GetWindowWMInfo(scon->real_window, &info)) {
+                win32_kbd_set_grab(gui_grab);
+                win32_kbd_set_window(info.info.win.window);
+            }
+        }
+#endif
+        /* fall through */
     case SDL_WINDOWEVENT_ENTER:
         if (!gui_grab && (qemu_input_is_absolute() || absolute_enabled)) {
             absolute_mouse_grab(scon);
@@ -546,6 +567,11 @@ static void handle_windowevent(SDL_Event *ev)
         scon->ignore_hotkeys = get_mod_state();
         break;
     case SDL_WINDOWEVENT_FOCUS_LOST:
+#ifdef CONFIG_WIN32
+        if (qemu_console_is_graphic(scon->dcl.con)) {
+            win32_kbd_set_window(NULL);
+        }
+#endif
         if (gui_grab && !gui_fullscreen) {
             sdl_grab_end(scon);
         }
-- 
2.26.1

[PATCH 07/10] ui/sdl2: start in full screen with grab enabled

[Volker Rümelin]

To do it's work the sdl_grab_start() function needs a pointer
to a sdl2_console structure.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/sdl2.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index ec1cb8131f..c88ac97a79 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -871,17 +871,16 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
         SDL_SetWindowIcon(sdl2_console[0].real_window, icon);
     }
 
-    gui_grab = 0;
-    if (gui_fullscreen) {
-        sdl_grab_start(0);
-    }
-
     mouse_mode_notifier.notify = sdl_mouse_mode_change;
     qemu_add_mouse_mode_change_notifier(&mouse_mode_notifier);
 
     sdl_cursor_hidden = SDL_CreateCursor(&data, &data, 8, 1, 0, 0);
     sdl_cursor_normal = SDL_GetCursor();
 
+    if (gui_fullscreen) {
+        sdl_grab_start(&sdl2_console[0]);
+    }
+
     atexit(sdl_cleanup);
 }
 
-- 
2.26.1

[PATCH 08/10] ui/sdl2-input: use trace-events to debug key events

[Volker Rümelin]

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/sdl2-input.c | 3 +++
 ui/trace-events | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 1f9fe831b3..f068382209 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -27,6 +27,7 @@
 #include "ui/console.h"
 #include "ui/input.h"
 #include "ui/sdl2.h"
+#include "trace.h"
 
 void sdl2_process_key(struct sdl2_console *scon,
                       SDL_KeyboardEvent *ev)
@@ -38,6 +39,8 @@ void sdl2_process_key(struct sdl2_console *scon,
         return;
     }
     qcode = qemu_input_map_usb_to_qcode[ev->keysym.scancode];
+    trace_sdl2_process_key(ev->keysym.scancode, qcode,
+                           ev->type == SDL_KEYDOWN ? "down" : "up");
     qkbd_state_key_event(scon->kbd, qcode, ev->type == SDL_KEYDOWN);
 
     if (!qemu_console_is_graphic(con)) {
diff --git a/ui/trace-events b/ui/trace-events
index 0dcda393c1..5367fd3f16 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -75,6 +75,9 @@ input_event_abs(int conidx, const char *axis, int value) "con %d, axis %s, value
 input_event_sync(void) ""
 input_mouse_mode(int absolute) "absolute %d"
 
+# sdl2-input.c
+sdl2_process_key(int sdl_scancode, int qcode, const char *action) "translated SDL scancode %d to QKeyCode %d (%s)"
+
 # spice-display.c
 qemu_spice_add_memslot(int qid, uint32_t slot_id, unsigned long virt_start, unsigned long virt_end, int async) "%d %u: host virt 0x%lx - 0x%lx async=%d"
 qemu_spice_del_memslot(int qid, uint32_t gid, uint32_t slot_id) "%d gid=%u sid=%u"
-- 
2.26.1

Re: [PATCH 08/10] ui/sdl2-input: use trace-events to debug key events

[Philippe Mathieu-Daudé]

On 5/10/20 8:43 PM, Volker Rümelin wrote:
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>   ui/sdl2-input.c | 3 +++
>   ui/trace-events | 3 +++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
> index 1f9fe831b3..f068382209 100644
> --- a/ui/sdl2-input.c
> +++ b/ui/sdl2-input.c
> @@ -27,6 +27,7 @@
>   #include "ui/console.h"
>   #include "ui/input.h"
>   #include "ui/sdl2.h"
> +#include "trace.h"
>   
>   void sdl2_process_key(struct sdl2_console *scon,
>                         SDL_KeyboardEvent *ev)
> @@ -38,6 +39,8 @@ void sdl2_process_key(struct sdl2_console *scon,
>           return;
>       }
>       qcode = qemu_input_map_usb_to_qcode[ev->keysym.scancode];
> +    trace_sdl2_process_key(ev->keysym.scancode, qcode,
> +                           ev->type == SDL_KEYDOWN ? "down" : "up");
>       qkbd_state_key_event(scon->kbd, qcode, ev->type == SDL_KEYDOWN);
>   
>       if (!qemu_console_is_graphic(con)) {
> diff --git a/ui/trace-events b/ui/trace-events
> index 0dcda393c1..5367fd3f16 100644
> --- a/ui/trace-events
> +++ b/ui/trace-events
> @@ -75,6 +75,9 @@ input_event_abs(int conidx, const char *axis, int value) "con %d, axis %s, value
>   input_event_sync(void) ""
>   input_mouse_mode(int absolute) "absolute %d"
>   
> +# sdl2-input.c
> +sdl2_process_key(int sdl_scancode, int qcode, const char *action) "translated SDL scancode %d to QKeyCode %d (%s)"
> +
>   # spice-display.c
>   qemu_spice_add_memslot(int qid, uint32_t slot_id, unsigned long virt_start, unsigned long virt_end, int async) "%d %u: host virt 0x%lx - 0x%lx async=%d"
>   qemu_spice_del_memslot(int qid, uint32_t gid, uint32_t slot_id) "%d gid=%u sid=%u"
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 09/10] ui/gtk: don't pass on win keys without keyboard grab

[Volker Rümelin]

This patch applies commit c68f74b02e "win32: do not handle win
keys when the keyboard is not grabbed" from project spice-gtk
to ui/gtk.c.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/gtk.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index 5a25e3fa4c..a43fddc57f 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1085,10 +1085,17 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
     VirtualConsole *vc = opaque;
     int qcode;
 
-#ifdef WIN32
+#ifdef G_OS_WIN32
     /* on windows, we ought to ignore the reserved key event? */
     if (key->hardware_keycode == 0xff)
         return false;
+
+    if (!vc->s->kbd_owner) {
+        if (key->hardware_keycode == VK_LWIN ||
+            key->hardware_keycode == VK_RWIN) {
+            return FALSE;
+        }
+    }
 #endif
 
     if (key->keyval == GDK_KEY_Pause
-- 
2.26.1

Re: [PATCH 09/10] ui/gtk: don't pass on win keys without keyboard grab

[Gerd Hoffmann]

On Sun, May 10, 2020 at 08:43:03PM +0200, Volker Rümelin wrote:
> This patch applies commit c68f74b02e "win32: do not handle win
> keys when the keyboard is not grabbed" from project spice-gtk
> to ui/gtk.c.

Which issue does this solve?

thanks,
  Gerd

[PATCH 10/10] ui/gtk: use native keyboard scancodes on Windows

[Volker Rümelin]

Since GTK 3.22 the function gdk_event_get_scancode() is
available. On Windows this function returns keyboard scancodes
and some extended flags. These raw keyboard scancodes are much
better suited for this use case than the half-cooked win32
virtual-key codes because scancodes report the key position on
the keyboard and the positions are independent of national
language settings.

Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
---
 ui/gtk.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index a43fddc57f..242b378bf1 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1016,8 +1016,13 @@ static const guint16 *gd_get_keymap(size_t *maplen)
 #ifdef GDK_WINDOWING_WIN32
     if (GDK_IS_WIN32_DISPLAY(dpy)) {
         trace_gd_keymap_windowing("win32");
+#if GTK_CHECK_VERSION(3, 22, 0)
+        *maplen = qemu_input_map_atset1_to_qcode_len;
+        return qemu_input_map_atset1_to_qcode;
+#else
         *maplen = qemu_input_map_win32_to_qcode_len;
         return qemu_input_map_win32_to_qcode;
+#endif
     }
 #endif
 
@@ -1063,6 +1068,25 @@ static int gd_map_keycode(int scancode)
     return keycode_map[scancode];
 }
 
+static int gd_get_keycode(GdkEventKey *key)
+{
+#if defined G_OS_WIN32 && GTK_CHECK_VERSION(3, 22, 0)
+    int scancode = gdk_event_get_scancode((GdkEvent *)key);
+
+    /* translate Windows native scan codes to atset1 keycodes */
+    switch (scancode & (KF_EXTENDED | 0xff)) {
+    case 0x145:     /* NUMLOCK */
+        return scancode & 0xff;
+    }
+
+    return scancode & KF_EXTENDED ?
+        0xe000 | (scancode & 0xff) : scancode & 0xff;
+
+#else
+    return key->hardware_keycode;
+#endif
+}
+
 static gboolean gd_text_key_down(GtkWidget *widget,
                                  GdkEventKey *key, void *opaque)
 {
@@ -1074,7 +1098,7 @@ static gboolean gd_text_key_down(GtkWidget *widget,
     } else if (key->length) {
         kbd_put_string_console(con, key->string, key->length);
     } else {
-        int qcode = gd_map_keycode(key->hardware_keycode);
+        int qcode = gd_map_keycode(gd_get_keycode(key));
         kbd_put_qcode_console(con, qcode, false);
     }
     return TRUE;
@@ -1083,7 +1107,7 @@ static gboolean gd_text_key_down(GtkWidget *widget,
 static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
 {
     VirtualConsole *vc = opaque;
-    int qcode;
+    int keycode, qcode;
 
 #ifdef G_OS_WIN32
     /* on windows, we ought to ignore the reserved key event? */
@@ -1111,9 +1135,10 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
         return TRUE;
     }
 
-    qcode = gd_map_keycode(key->hardware_keycode);
+    keycode = gd_get_keycode(key);
+    qcode = gd_map_keycode(keycode);
 
-    trace_gd_key_event(vc->label, key->hardware_keycode, qcode,
+    trace_gd_key_event(vc->label, keycode, qcode,
                        (key->type == GDK_KEY_PRESS) ? "down" : "up");
 
     qkbd_state_key_event(vc->gfx.kbd, qcode,
-- 
2.26.1

Re: [PATCH 10/10] ui/gtk: use native keyboard scancodes on Windows

[Daniel P. Berrangé]

On Sun, May 10, 2020 at 08:43:04PM +0200, Volker Rümelin wrote:
> Since GTK 3.22 the function gdk_event_get_scancode() is
> available. On Windows this function returns keyboard scancodes
> and some extended flags. These raw keyboard scancodes are much
> better suited for this use case than the half-cooked win32
> virtual-key codes because scancodes report the key position on
> the keyboard and the positions are independent of national
> language settings.
> 
> Signed-off-by: Volker Rümelin <vr_qemu@t-online.de>
> ---
>  ui/gtk.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/ui/gtk.c b/ui/gtk.c
> index a43fddc57f..242b378bf1 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -1016,8 +1016,13 @@ static const guint16 *gd_get_keymap(size_t *maplen)
>  #ifdef GDK_WINDOWING_WIN32
>      if (GDK_IS_WIN32_DISPLAY(dpy)) {
>          trace_gd_keymap_windowing("win32");
> +#if GTK_CHECK_VERSION(3, 22, 0)
> +        *maplen = qemu_input_map_atset1_to_qcode_len;
> +        return qemu_input_map_atset1_to_qcode;
> +#else
>          *maplen = qemu_input_map_win32_to_qcode_len;
>          return qemu_input_map_win32_to_qcode;
> +#endif

Our current min GTK is 3.14, which I picked here:

commit 58296cb61866195297510e946a51acc5f0b9639e
Author: Daniel P. Berrangé <berrange@redhat.com>
Date:   Wed Aug 22 14:15:53 2018 +0100

    ui: increase min required GTK3 version to 3.14.0
    
    Per supported platforms doc[1], the various min GTK3 on relevant distros is:
    
      RHEL-7.0: 3.8.8
      RHEL-7.2: 3.14.13
      RHEL-7.4: 3.22.10
      RHEL-7.5: 3.22.26
      Debian (Stretch): 3.22.11
      Debian (Jessie): 3.14.5
      OpenBSD (Ports): 3.22.30
      FreeBSD (Ports): 3.22.29
      OpenSUSE Leap 15: 3.22.30
      SLE12-SP2: Unknown
      Ubuntu (Xenial): 3.18.9
      macOS (Homebrew): 3.22.30
    
    This suggests that a minimum GTK3 of 3.14.0 is a reasonable target,
    as users are unlikely to be stuck on RHEL-7.0/7.1 still
    


since that time, we no longer support Debian Jessie, since Debian Buster
is now released. We also no longer support Ubuntu Xenial (16.04), since
we now only need Ubuntu Bionic (18.04) and Focal (20.04).

So we can justify moving the minium Gtk in QEMU to 3.22 at this time.

This will avoid you needing to do versioned ifdef for this new functionality.


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: [PATCH 10/10] ui/gtk: use native keyboard scancodes on Windows

[Volker Rümelin]

> since that time, we no longer support Debian Jessie, since Debian Buster
> is now released. We also no longer support Ubuntu Xenial (16.04), since
> we now only need Ubuntu Bionic (18.04) and Focal (20.04).
>
> So we can justify moving the minium Gtk in QEMU to 3.22 at this time.
>
> This will avoid you needing to do versioned ifdef for this new functionality.

Hi Daniel,

I noticed there are already seven versioned ifdefs in ui/gtk.c. I would prefer to leave my patch as it is at the moment and send in a separate follow up patch which increases the minimum GTK version in configure to 3.22 and removes all versioned code in ui/gtk.c. Just like your patch from 2018. I think this is easier to revert if someone complains about the removal.

With best regards,
Volker

Re: Patches for ui/gtk and ui/sdl

[Howard Spoelstra]

[-- Attachment #1: Type: text/plain, Size: 975 bytes --]

On Sun, May 10, 2020 at 8:42 PM Volker Rümelin <vr_qemu@t-online.de> wrote:

> It's rather difficult to test qemu patches in guests on Windows with
> important keys missing. These patches mainly fix the guest keyboard on
> Windows.
>
> With best regards,
> Volker
>

Hi Volker,

Excellent patch! I tested this on Windows with qemu-system-ppc running Mac
OS 9.2 with both SDL and GTK GUIs. Finally no more popping up of that
windows menu. Key combos are sent correctly into the guest. Also right alt
does no longer send left ctrl+alt into the guest.

A mere cosmetic difference between using a real mac keyboard and a pc
keyboard is that alt and windows key seem to have traded places. However,
the mac keyboard does have the alt where the windows key is on the pc
keyboard. The keys are, however, functionally correct.

The GTK GUI itself was and is unusable for Mac OS 9 guests in WIndows. That
OS has no tablet driver available.

Thanks,
Howard

[-- Attachment #2: Type: text/html, Size: 1405 bytes --]