On Mon, Nov 11, 2013 at 2:02 PM, Anthony Liguori <anth...@codemonkey.ws> wrote: > On Sun, Nov 10, 2013 at 3:15 PM, Dave Airlie <airl...@gmail.com> wrote: >> From: Dave Airlie <airl...@redhat.com> >> >> I've ported the SDL1.2 code over, and rewritten it to use the SDL2 interface. >> >> The biggest changes were in the input handling, where SDL2 has done a major >> overhaul, and I've had to include a generated translation file to get from >> SDL2 codes back to qemu compatible ones. I'm still not sure how the keyboard >> layout code works in qemu, so there may be further work if someone can point >> me a test case that works with SDL1.2 and doesn't with SDL2. >> >> Some SDL env vars we used to set are no longer used by SDL2, >> Windows, OSX support is untested, >> >> I don't think we can link to SDL1.2 and SDL2 at the same time, so I felt >> using --with-sdlabi=2.0 to select the new code should be fine, like how >> gtk does it. >> >> Signed-off-by: Dave Airlie <airl...@redhat.com> >> --- >> configure | 23 +- >> ui/Makefile.objs | 4 +- >> ui/sdl.c | 3 + >> ui/sdl2.c | 889 >> +++++++++++++++++++++++++++++++++++++++++++ > > Can we refactor this to not duplicate everything and instead have > function hooks or even #ifdefs for the things that are different? We > try to guess the right SDL to use in configure. See how we handle > GTK2 vs. GTK3. > > It's very hard to review ATM due to the split.
No I talked to enough people at kvmforum and everyone said I should split this into a separate file, please don't make me undo that now, I originally did it with ifdefs and just spent a few days redoing it the other way! > > Regarding the keycodes, danpb has a great write up on his blog: > > https://www.berrange.com/posts/2010/07/04/a-summary-of-scan-code-key-codes-sets-used-in-the-pc-virtualization-stack/ Okay I'll read that later, > > Internally, we use a variant of raw XT scancodes. We have a > keymapping routine that translates from a symbolic key (i.e. "capital > A") to the appropriate XT scancode. > > SDL 1.x at least lets you get at raw X11 scancodes which are either > evdev or PS/2 codes depending on the version of X11. So for SDL 1.x > we have two translations mechanisms 1) X11 scancodes to XT scancodes > and 2) SDL keysyms to internal QEMU keysyms. > > From what I can tell, SDL2 has moved from just passing through raw X11 > scancodes (which like I said, can be different depending on your X > configuration) to having an intermediate translation layer. See > comments inline: > >> ui/sdl2_scancode_translate.h | 260 +++++++++++++ >> ui/sdl_keysym.h | 3 +- >> 6 files changed, 1175 insertions(+), 7 deletions(-) >> create mode 100644 ui/sdl2.c >> create mode 100644 ui/sdl2_scancode_translate.h >> >> diff --git a/configure b/configure >> index 9addff1..bf3be37 100755 >> --- a/configure >> +++ b/configure >> @@ -158,6 +158,7 @@ docs="" >> fdt="" >> pixman="" >> sdl="" >> +sdlabi="1.2" >> virtfs="" >> vnc="yes" >> sparse="no" >> @@ -310,6 +311,7 @@ query_pkg_config() { >> } >> pkg_config=query_pkg_config >> sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}" >> +sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}" >> >> # default flags for all hosts >> QEMU_CFLAGS="-fno-strict-aliasing $QEMU_CFLAGS" >> @@ -710,6 +712,8 @@ for opt do >> ;; >> --enable-sdl) sdl="yes" >> ;; >> + --with-sdlabi=*) sdlabi="$optarg" >> + ;; >> --disable-qom-cast-debug) qom_cast_debug="no" >> ;; >> --enable-qom-cast-debug) qom_cast_debug="yes" >> @@ -1092,6 +1096,7 @@ echo " --disable-strip disable stripping >> binaries" >> echo " --disable-werror disable compilation abort on warning" >> echo " --disable-sdl disable SDL" >> echo " --enable-sdl enable SDL" >> +echo " --with-sdlabi select preferred SDL ABI 1.2 or 2.0" >> echo " --disable-gtk disable gtk UI" >> echo " --enable-gtk enable gtk UI" >> echo " --disable-virtfs disable VirtFS" >> @@ -1751,12 +1756,22 @@ fi >> >> # Look for sdl configuration program (pkg-config or sdl-config). Try >> # sdl-config even without cross prefix, and favour pkg-config over >> sdl-config. >> -if test "`basename $sdl_config`" != sdl-config && ! has ${sdl_config}; then >> - sdl_config=sdl-config >> + >> +if test $sdlabi == "2.0"; then >> + sdl_config=$sdl2_config >> + sdlname=sdl2 >> + sdlconfigname=sdl2_config >> +else >> + sdlname=sdl >> + sdlconfigname=sdl_config >> +fi >> + >> +if test "`basename $sdl_config`" != $sdlconfigname && ! has ${sdl_config}; >> then >> + sdl_config=$sdlconfigname >> fi >> >> -if $pkg_config sdl --exists; then >> - sdlconfig="$pkg_config sdl" >> +if $pkg_config $sdlname --exists; then >> + sdlconfig="$pkg_config $sdlname" >> _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'` >> elif has ${sdl_config}; then >> sdlconfig="$sdl_config" >> diff --git a/ui/Makefile.objs b/ui/Makefile.objs >> index f33be47..721ad37 100644 >> --- a/ui/Makefile.objs >> +++ b/ui/Makefile.objs >> @@ -9,12 +9,12 @@ vnc-obj-y += vnc-jobs.o >> >> common-obj-y += keymaps.o console.o cursor.o input.o qemu-pixman.o >> common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o >> -common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o >> +common-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o sdl2.o >> common-obj-$(CONFIG_COCOA) += cocoa.o >> common-obj-$(CONFIG_CURSES) += curses.o >> common-obj-$(CONFIG_VNC) += $(vnc-obj-y) >> common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o >> >> -$(obj)/sdl.o $(obj)/sdl_zoom.o: QEMU_CFLAGS += $(SDL_CFLAGS) >> +$(obj)/sdl.o $(obj)/sdl_zoom.o $(obj)/sdl2.o: QEMU_CFLAGS += $(SDL_CFLAGS) >> >> $(obj)/gtk.o: QEMU_CFLAGS += $(GTK_CFLAGS) $(VTE_CFLAGS) >> diff --git a/ui/sdl.c b/ui/sdl.c >> index 9d8583c..736bb95 100644 >> --- a/ui/sdl.c >> +++ b/ui/sdl.c >> @@ -26,6 +26,8 @@ >> #undef WIN32_LEAN_AND_MEAN >> >> #include <SDL.h> >> + >> +#if SDL_MAJOR_VERSION == 1 >> #include <SDL_syswm.h> >> >> #include "qemu-common.h" >> @@ -966,3 +968,4 @@ void sdl_display_init(DisplayState *ds, int full_screen, >> int no_frame) >> >> atexit(sdl_cleanup); >> } >> +#endif >> diff --git a/ui/sdl2.c b/ui/sdl2.c >> new file mode 100644 >> index 0000000..1a71f59 >> --- /dev/null >> +++ b/ui/sdl2.c >> @@ -0,0 +1,889 @@ >> +/* >> + * QEMU SDL display driver >> + * >> + * Copyright (c) 2003 Fabrice Bellard >> + * >> + * Permission is hereby granted, free of charge, to any person obtaining a >> copy >> + * of this software and associated documentation files (the "Software"), to >> deal >> + * in the Software without restriction, including without limitation the >> rights >> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell >> + * copies of the Software, and to permit persons to whom the Software is >> + * furnished to do so, subject to the following conditions: >> + * >> + * The above copyright notice and this permission notice shall be included >> in >> + * all copies or substantial portions of the Software. >> + * >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS >> OR >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >> OTHER >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING >> FROM, >> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN >> + * THE SOFTWARE. >> + */ >> +/* Ported SDL 1.2 code to 2.0 by Dave Airlie. */ >> + >> +/* Avoid compiler warning because macro is redefined in SDL_syswm.h. */ >> +#undef WIN32_LEAN_AND_MEAN >> + >> +#include <SDL.h> >> + >> +#if SDL_MAJOR_VERSION == 2 >> +#include <SDL_syswm.h> >> + >> +#include "qemu-common.h" >> +#include "ui/console.h" >> +#include "sysemu/sysemu.h" >> +#include "x_keymap.h" >> +#include "sdl_zoom.h" >> + >> +#include "sdl2_scancode_translate.h" >> + >> +static DisplayChangeListener *dcl; >> +static DisplaySurface *surface; >> +static SDL_Window *real_window; >> +static SDL_Renderer *real_renderer; >> +static SDL_Texture *guest_texture; >> + >> +static int gui_grab; /* if true, all keyboard/mouse events are grabbed */ >> +static int last_vm_running; >> +static bool gui_saved_scaling; >> +static int gui_saved_width; >> +static int gui_saved_height; >> +static int gui_saved_grab; >> +static int gui_fullscreen; >> +static int gui_noframe; >> +static int gui_key_modifier_pressed; >> +static int gui_keysym; >> +static int gui_grab_code = KMOD_LALT | KMOD_LCTRL; >> +static uint8_t modifiers_state[256]; >> +static SDL_Cursor *sdl_cursor_normal; >> +static SDL_Cursor *sdl_cursor_hidden; >> +static int absolute_enabled = 0; >> +static int guest_cursor = 0; >> +static int guest_x, guest_y; >> +static SDL_Cursor *guest_sprite = NULL; >> +static int scaling_active = 0; >> +static Notifier mouse_mode_notifier; >> + >> +static void sdl_update_caption(void); >> + >> +static void sdl_update(DisplayChangeListener *dcl, >> + int x, int y, int w, int h) >> +{ >> + if (!surface) >> + return; >> + >> + SDL_UpdateTexture(guest_texture, NULL, surface_data(surface), >> + surface_stride(surface)); >> + SDL_RenderCopy(real_renderer, guest_texture, NULL, NULL); >> + SDL_RenderPresent(real_renderer); >> +} >> + >> +static void do_sdl_resize(int width, int height, int bpp) >> +{ >> + int flags; >> + >> + if (real_window) { >> + SDL_SetWindowSize(real_window, width, height); >> + SDL_RenderSetLogicalSize(real_renderer, width, height); >> + } else { >> + flags = 0; >> + if (gui_fullscreen) >> + flags |= SDL_WINDOW_FULLSCREEN; >> + else >> + flags |= SDL_WINDOW_RESIZABLE; >> + >> + real_window = SDL_CreateWindow("", SDL_WINDOWPOS_UNDEFINED, >> + SDL_WINDOWPOS_UNDEFINED, >> + width, height, flags); >> + real_renderer = SDL_CreateRenderer(real_window, -1, 0); >> + sdl_update_caption(); >> + } >> +} >> + >> +static void sdl_switch(DisplayChangeListener *dcl, >> + DisplaySurface *new_surface) >> +{ >> + int format = 0; >> + /* temporary hack: allows to call sdl_switch to handle scaling changes >> */ >> + if (new_surface) { >> + surface = new_surface; >> + } >> + >> + if (!scaling_active) { >> + do_sdl_resize(surface_width(surface), surface_height(surface), 0); >> + } else >> + do_sdl_resize(surface_width(surface), surface_height(surface), 0); >> + >> + if (guest_texture) >> + SDL_DestroyTexture(guest_texture); >> + >> + if (surface_bits_per_pixel(surface) == 16) >> + format = SDL_PIXELFORMAT_RGB565; >> + else if (surface_bits_per_pixel(surface) == 32) >> + format = SDL_PIXELFORMAT_ARGB8888; >> + >> + if (!format) >> + exit(1); >> + guest_texture = SDL_CreateTexture(real_renderer, format, >> + SDL_TEXTUREACCESS_STREAMING, >> + surface_width(surface), >> surface_height(surface)); >> + SDL_RenderSetLogicalSize(real_renderer, surface_width(surface), >> surface_height(surface)); >> +} >> + >> +/* generic keyboard conversion */ >> + >> +#include "sdl_keysym.h" >> + >> +static kbd_layout_t *kbd_layout = NULL; >> + >> +static uint8_t sdl_keyevent_to_keycode_generic(const SDL_KeyboardEvent *ev) >> +{ >> + int keysym; >> + /* workaround for X11+SDL bug with AltGR */ >> + keysym = ev->keysym.sym; >> + if (keysym == 0 && ev->keysym.scancode == 113) >> + keysym = SDLK_MODE; >> + /* For Japanese key '\' and '|' */ >> + if (keysym == 92 && ev->keysym.scancode == 133) { >> + keysym = 0xa5; >> + } >> + return keysym2scancode(kbd_layout, keysym) & SCANCODE_KEYMASK; >> +} >> + >> +/* specific keyboard conversions from scan codes */ >> + >> +#if defined(_WIN32) >> + >> +static uint8_t sdl_keyevent_to_keycode(const SDL_KeyboardEvent *ev) >> +{ >> + int keycode; >> + >> + keycode = ev->keysym.scancode; >> + keycode = sdl2_scancode_to_keycode[keycode]; >> + return keycode; >> +} >> + >> +#else >> + >> +static uint8_t sdl_keyevent_to_keycode(const SDL_KeyboardEvent *ev) >> +{ >> + int keycode; >> + >> + keycode = ev->keysym.scancode; >> + >> + keycode = sdl2_scancode_to_keycode[keycode]; >> + if (keycode >= 89 && keycode < 150) { >> + keycode = translate_evdev_keycode(keycode - 89); >> + } > > This doesn't seem right to me. It just so happened that the low value > scan codes are the same for both X11, evdev, and SDL 1.x. That's why > we only used the evdev/x11 translation tables for higher codes. It > was just a table size optimization. > > Presumably, sdl2 doesn't do this though, or does it? If it does, > don't we need to probe for evdev instead of assuming it's there? The table I wrote translates to evdev codes, then we use the evdev translator to translate to qemu ones, so its perfectly fine as is, SDL2 hides everything in its own table. Dave.