[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ui/sdl2 : initial port to SDL 2.0
From: |
Dave Airlie |
Subject: |
Re: [Qemu-devel] [PATCH] ui/sdl2 : initial port to SDL 2.0 |
Date: |
Mon, 11 Nov 2013 19:10:16 +1000 |
On Mon, Nov 11, 2013 at 2:02 PM, Anthony Liguori <address@hidden> wrote:
> On Sun, Nov 10, 2013 at 3:15 PM, Dave Airlie <address@hidden> wrote:
>> From: Dave Airlie <address@hidden>
>>
>> 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 <address@hidden>
>> ---
>> 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.