Re: [Qemu-devel] [PATCH 1/2] Rewrite the way keycode conversions are performed

[Anthony Liguori <anthony@codemonkey.ws>]

On 01/17/2012 01:35 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange"<berrange@redhat.com>
>
> The SDL video display code needs to convert between the SDL keyboard
> event keycodes, and QEMU's internal keycode set (a variant of the
> xt coding).  Currently the SDL code is only able todo this when it is
> built for X11, and running against a Linux X11 server using evdev or
> xfree86 keymaps.  If running against an OS-X or Win32 X11 server the
> keycode mapping falls back to xfre86, which is completely wrong. There
> is no mapping at all done, if built against an SDL library with direct
> Win32 or Quartz display support.
>
> After initial creation, this QEMU code was later copied into GTK-VNC
> to deal with the same problem. GTK-VNC came across the same problem
> described above and rewrote the mapping code from scratch. Instead
> of creating two arrays for the specific conversions required, the
> GTK-VNC code created a CSV file containing data for all commonly
> known keycode sets (Linux evdev, OS-X, AT set1, AT set2, AT set3,
> XT, XT Linux KBD driver, USB HID, Win32, Xwin XT variant, Xorg
> KBD XT variant). A script is then used to generate C arrays for
> the particular conversions required.  The CSV file has since been
> reused in both the SPICE-GTK and libvirt codebases, unchanged.
>
> This patch rewrites QEMU's SDL code to use this same approach, and
> in the process adds support for 4 new targets, SDL X11 on Win32
> Xorg server, SDL X11 on OS-X Xorg server, SDL Win32 and SDL Quartz.
>
> In the near future the 'keymap-gen.pl' and 'keymaps.csv' files
> will be placed into a dedicated GIT repo which can be added to
> QEMU, libvirt, SPICE-GTK&  GTK-VNC via a git submodule, instead
> of requiring manual copying.
>
> * Makefile: Add rules for generating keymap data files
> * Makefile.objs: Replace x_keymap.o with sdl_keymap.o
> * ui/keymap-gen.pl: Script for generating keycode mapping tables
> * ui/keymaps.csv: Master datafile of keycode mappings
> * ui/sdl.c: Rewrite sdl_keyevent_to_keycode to use new
>    mapping code
> * ui/sdl_keymap.c,ui/sdl_keymap.h: Add APIs for detecting
>    suitable keymap tables&  performing keymap conversions
> * ui/x_keymap.[ch]: Remove obsolete files
> ---
>   Makefile         |   40 +++++-
>   Makefile.objs    |    2 +-
>   ui/keymap-gen.pl |  210 ++++++++++++++++++++++++
>   ui/keymaps.csv   |  464 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ui/sdl.c         |   88 ++---------
>   ui/sdl_keymap.c  |  241 ++++++++++++++++++++++++++++
>   ui/sdl_keymap.h  |   32 ++++
>   ui/x_keymap.c    |  168 --------------------
>   ui/x_keymap.h    |   32 ----
>   9 files changed, 998 insertions(+), 279 deletions(-)
>   create mode 100644 ui/keymap-gen.pl
>   create mode 100644 ui/keymaps.csv
>   create mode 100644 ui/sdl_keymap.c
>   create mode 100644 ui/sdl_keymap.h
>   delete mode 100644 ui/x_keymap.c
>   delete mode 100644 ui/x_keymap.h
>
> diff --git a/Makefile b/Makefile
> index 2bbc547..f776c30 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -116,7 +116,45 @@ QEMU_CFLAGS+=$(GLIB_CFLAGS)
>
>   ui/cocoa.o: ui/cocoa.m
>
> -ui/sdl.o audio/sdlaudio.o ui/sdl_zoom.o baum.o: QEMU_CFLAGS += $(SDL_CFLAGS)
> +ui/sdl.o audio/sdlaudio.o ui/sdl_zoom.o ui/sdl_keymap.o baum.o: QEMU_CFLAGS += $(SDL_CFLAGS)
> +
> +KEYMAP_GEN = ui/keymap-gen.pl
> +KEYMAP_CSV = ui/keymaps.csv
> +
> +SDL_KEYMAPS = \
> +        ui/sdl_keymap_xorgevdev2rfb.c \
> +        ui/sdl_keymap_xorgkbd2rfb.c \
> +        ui/sdl_keymap_xorgxquartz2rfb.c \
> +        ui/sdl_keymap_xorgxwin2rfb.c \
> +        ui/sdl_keymap_osx2rfb.c \
> +        ui/sdl_keymap_win322rfb.c
> +
> +$(SDL_KEYMAPS): $(KEYMAP_GEN) $(KEYMAP_CSV)
> +GENERATED_SOURCES += $(SDL_KEYMAPS)
> +
> +# Avoid need for perl(Text::CSV) by end users
> +# XXXX how does QEMU make file deal with this
> +#EXTRA_DIST += $(SDL_KEYMAPS)
> +
> +ui/sdl_keymap.c: $(SDL_KEYMAPS)
> +
> +ui/sdl_keymap_xorgevdev2rfb.c: $(KEYMAP_CSV)
> +	$(call quiet-command,perl $(KEYMAP_GEN) $<  xorgevdev rfb>  $@ || rm $@, "  GEN $@")

I'm not sure I'm prepared to add a perl build dependency.  That's particularly 
hard for Windows users.  We could alternatively use a python version of 
keymap-gen but then we're forking from gtk-vnc/gtk-spice.

Maybe it makes sense to create a small library for dealing with keymaps?  Then 
we could all link against it and probe for that conditionally?

Regards,

Anthony Liguori