[Spice-devel] [PATCH] Use scancode instead of keycode names

[Daniel P. Berrangé]

On Thu, Mar 08, 2018 at 04:19:03PM +0100, Olivier Fourdan wrote:
> When running on Xwayland, the keycode mapping property is not available,
> which causes unknown keycode mapping errors and the keyboard doesn't
> work.
> 
> Use a known scancode (“XK_Page_Up”) to check against the AT scancode and
> use “xfree86” if it matches or assume “evdev” for anything else.
> 
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  src/vncdisplaykeymap.c | 33 ++++-----------------------------
>  1 file changed, 4 insertions(+), 29 deletions(-)
> 
> diff --git a/src/vncdisplaykeymap.c b/src/vncdisplaykeymap.c
> index 9ee501d..135412f 100644
> --- a/src/vncdisplaykeymap.c
> +++ b/src/vncdisplaykeymap.c
> @@ -162,8 +162,6 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow *window,
>  {
>  #ifdef GDK_WINDOWING_X11
>  	if (GDK_IS_X11_WINDOW(window)) {
> -		XkbDescPtr desc;
> -		const gchar *keycodes = NULL;
>                  GdkDisplay *dpy = gdk_window_get_display(window);
>  
>  		/* There is no easy way to determine what X11 server
> @@ -175,17 +173,6 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow *window,
>  		 */
>  
>  		Display *xdisplay = gdk_x11_display_get_xdisplay(dpy);
> -		desc = XkbGetMap(xdisplay,
> -				      XkbGBN_AllComponentsMask,
> -				      XkbUseCoreKbd);
> -		if (desc) {
> -			if (XkbGetNames(xdisplay, XkbKeycodesNameMask, desc) == Success) {
> -				keycodes = gdk_x11_get_xatom_name(desc->names->keycodes);
> -				if (!keycodes)
> -					g_warning("could not lookup keycode name");
> -			}
> -			XkbFreeKeyboard(desc, XkbGBN_AllComponentsMask, True);
> -		}
>  
>  		if (check_for_xwin(dpy)) {
>  			VNC_DEBUG("Using xwin keycode mapping");
> @@ -195,26 +182,14 @@ const guint16 *vnc_display_keymap_gdk2xtkbd_table(GdkWindow *window,
>  			VNC_DEBUG("Using xquartz keycode mapping");
>  			*maplen = G_N_ELEMENTS(keymap_xorgxquartz2xtkbd);
>  			return keymap_xorgxquartz2xtkbd;
> -		} else if (keycodes && STRPREFIX(keycodes, "evdev")) {
> -			VNC_DEBUG("Using evdev keycode mapping");
> -			*maplen = G_N_ELEMENTS(keymap_xorgevdev2xtkbd);
> -			return keymap_xorgevdev2xtkbd;
> -		} else if (keycodes && STRPREFIX(keycodes, "xfree86")) {
> +		} else if (XKeysymToKeycode(xdisplay, XK_Page_Up) == 0x63) {
>  			VNC_DEBUG("Using xfree86 keycode mapping");
>  			*maplen = G_N_ELEMENTS(keymap_xorgkbd2xtkbd);
>  			return keymap_xorgkbd2xtkbd;
>  		} else {
> -			g_warning("Unknown keycode mapping '%s'.\n"
> -				  "Please report to gtk-vnc-list at gnome.org\n"
> -				  "including the following information:\n"
> -				  "\n"
> -				  "  - Operating system\n"
> -				  "  - GDK build\n"
> -				  "  - X11 Server\n"
> -				  "  - xprop -root\n"
> -				  "  - xdpyinfo\n",
> -				  keycodes);
> -			return NULL;
> +			VNC_DEBUG("Using evdev keycode mapping");
> +			*maplen = G_N_ELEMENTS(keymap_xorgevdev2xtkbd);
> +			return keymap_xorgevdev2xtkbd;

I don't think this is desirable logic - blindly assuming everything
else is evdev is not valid - that is only true of an X server running
on a Linux host. Thus this has the effect of hiding the important
error message alerting us to use of other X servers we need to handle
and silently giving those users incorrect keymaps.

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