On Nov 11, 2013 1:10 AM, "Dave Airlie" <airl...@gmail.com> wrote: > > 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!
Perhaps whoever you spoke with should speal up then. Forking sdl.c seems like a pretty bad idea to me. > > > > > 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. Why not translate to XT directly? The intermediate evdev step seems unnecessary. Regards, Anthony Liguori > > Dave.