Re: [Qemu-devel] [PATCH 01/42] ui/sdl2 : initial port to SDL 2.0 (v2.0)

[Stefan Weil <sw@weilnetz.de>]

Am 16.12.2013 11:48, schrieb Gerd Hoffmann:
> From: Dave Airlie <airlied@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.
>
> v1.1: fix keys in text console
> v1.2: fix shutdown, cleanups a bit of code, support ARGB cursor
>
> v2.0: merge the SDL multihead patch into this, g_new the number of consoles
> needed, wrap DCL inside per-console structure.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  configure                    |  23 +-
>  ui/Makefile.objs             |   4 +-
>  ui/sdl.c                     |   3 +
>  ui/sdl2.c                    | 981 +++++++++++++++++++++++++++++++++++++++++++
>  ui/sdl2_scancode_translate.h | 260 ++++++++++++
>  ui/sdl_keysym.h              |   3 +-
>  6 files changed, 1267 insertions(+), 7 deletions(-)
>  create mode 100644 ui/sdl2.c
>  create mode 100644 ui/sdl2_scancode_translate.h
>
> diff --git a/configure b/configure
> index edfea95..88080cc 100755
> --- a/configure
> +++ b/configure
[...]
> @@ -1789,12 +1794,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

Please replace '==' by a single '=' here. dash (and maybe other less
sophisticated shells) don't like '=='.

> +    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"
[...]


I know that sdl2.c is based on sdl.c which was coded before the
introduction of the current coding rules, but would you mind if I send a
follow-up patch which fixes the warnings from checkpatch.pl for sdl2.c?
Tools like astyle can do this automatically, and the result is pretty good.

Some of the other patches don't include a Signed-off-by, so those
patches are only ready for local testing.

I was very curious to get test results here in my environment because of
a strange effect which I had noticed recently: booting a Linux system
(Tiny Core Linux) with SDL 1.2 (1:06) takes twice as long as with GTK
(0:33) or curses (0:29). The test was run on a remote Linux x86_64
server (so X display output is slower than normal). The huge difference
is not caused by more activity but simply by delays were the QEMU
process is waiting.

I hoped that SDL 2.0 might do a better job but was disappointed: I had
to abort the test after 10 minutes, the Linux boot made progress, but
very slowly and with lots of timeout errors.

Regards
Stefan

Regards
Stefan