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

[Daniel P. Berrange]

On Wed, Jan 18, 2012 at 11:47:12AM -0600, Anthony Liguori wrote:
> On 01/18/2012 11:07 AM, Daniel P. Berrange wrote:
> >On Wed, Jan 18, 2012 at 10:53:57AM -0600, Anthony Liguori wrote:
> >>On 01/17/2012 01:35 PM, Daniel P. Berrange wrote:
> >>>From: "Daniel P. Berrange"<address@hidden>
> >>>
> >>>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.
> >
> >I expected as much, which is why I sent the 2nd patch which actually
> >adds the generated files to GIT, so Perl is not required unless
> >someone decides to update the keymaps.csv file.
> >
> >>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?
> >
> >I don't think it is worth doing a library just for the keymap
> >tables. It could be worth creating a set of add-on libraries
> >(eg libsdl-keymap, libgdk-keymap, etc) for doing the keymap
> >hueristics.  In the immediate future though, my intent is to
> >request a GIT repository on freedesktop.org to host the keymaps.csv
> >file and a generator script, which could just be just included in
> >apps using a GIT submodule approach.
> 
> Ah, we already do git submodules so I'd much prefer we wait to do
> this.  Is this something that you are going to do in the very near
> future or only sort of near future?

I can probably get it done in the next week or two

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|