From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:56486) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RnaPa-00058q-Ce for qemu-devel@nongnu.org; Wed, 18 Jan 2012 13:39:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RnaPV-0002sI-EE for qemu-devel@nongnu.org; Wed, 18 Jan 2012 13:39:06 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:57225) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RnaPV-0002sB-3n for qemu-devel@nongnu.org; Wed, 18 Jan 2012 13:39:01 -0500 Received: by iaeo4 with SMTP id o4so13136769iae.4 for ; Wed, 18 Jan 2012 10:39:00 -0800 (PST) Message-ID: <4F1711C0.9020407@codemonkey.ws> Date: Wed, 18 Jan 2012 12:38:56 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1326828907-13822-1-git-send-email-berrange@redhat.com> <1326828907-13822-2-git-send-email-berrange@redhat.com> <4F16F925.2000809@codemonkey.ws> <20120118170743.GG24642@redhat.com> <4F1705A0.7020206@codemonkey.ws> <20120118183153.GH24642@redhat.com> In-Reply-To: <20120118183153.GH24642@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] Rewrite the way keycode conversions are performed List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel@nongnu.org On 01/18/2012 12:31 PM, Daniel P. Berrange wrote: > 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" >>>>> >>>>> 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 Okay, let me know, and I can help with the submodule bits. Thanks, Anthony Liguori > > Daniel