Thomas Huth <th...@redhat.com> writes:
> The "-display sdl" option still uses a hand-crafted parser for its
> parameters since we didn't want to drag an interface we considered
> somewhat flawed into the QAPI schema. Since the flaws are gone now,
> it's time to QAPIfy.
>
> This introduces the new "DisplaySDL" QAPI struct that is used to hold
> the parameters that are unique to the SDL display. The only specific
> parameter is currently "grab-mod" that is used to specify the required
> modifier keys to escape from the mouse grabbing mode.
>
> Signed-off-by: Thomas Huth <th...@redhat.com>
> ---
> qapi/ui.json | 26 ++++++++++++++-
> include/sysemu/sysemu.h | 2 --
> softmmu/globals.c | 2 --
> softmmu/vl.c | 70 +----------------------------------------
> ui/sdl2.c | 10 ++++++
> 5 files changed, 36 insertions(+), 74 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 11a827d10f..413371d5e8 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1295,6 +1295,29 @@
> '*swap-opt-cmd': 'bool'
> } }
>
> +##
> +# @HotKeyMod:
> +#
> +# Set of modifier keys that need to be held for shortcut key actions.
> +#
> +# Since: 7.1
> +##
> +{ 'enum' : 'HotKeyMod',
> + 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
I have a somewhat uneasy feeling about encoding what is essentially a
subset of the sets of modifier keys as an enumeration, but it's what we
have to do to QAPIfy existing grab-mod.
> +
> +##
> +# @DisplaySDL:
> +#
> +# SDL2 display options.
> +#
> +# @grab-mod: Modifier keys that should be pressed together with the
> +# "G" key to release the mouse grab.
> +#
> +# Since: 7.1
> +##
> +{ 'struct' : 'DisplaySDL',
> + 'data' : { '*grab-mod' : 'HotKeyMod' } }
> +
> ##
> # @DisplayType:
> #
> @@ -1374,7 +1397,8 @@
> 'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
> 'egl-headless': { 'type': 'DisplayEGLHeadless',
> 'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
> - 'dbus': { 'type': 'DisplayDBus', 'if': 'CONFIG_DBUS_DISPLAY' }
> + 'dbus': { 'type': 'DisplayDBus', 'if': 'CONFIG_DBUS_DISPLAY' },
> + 'sdl': { 'type': 'DisplaySDL', 'if': 'CONFIG_SDL' }
> }
> }
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index b4030acd74..812f66a31a 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -42,8 +42,6 @@ extern int graphic_depth;
> extern int display_opengl;
> extern const char *keyboard_layout;
> extern int win2k_install_hack;
> -extern int alt_grab;
> -extern int ctrl_grab;
> extern int graphic_rotate;
> extern int old_param;
> extern uint8_t *boot_splash_filedata;
> diff --git a/softmmu/globals.c b/softmmu/globals.c
> index 916bc12e2b..527edbefdd 100644
> --- a/softmmu/globals.c
> +++ b/softmmu/globals.c
> @@ -50,8 +50,6 @@ QEMUOptionRom option_rom[MAX_OPTION_ROMS];
> int nb_option_roms;
> int old_param;
> const char *qemu_name;
> -int alt_grab;
> -int ctrl_grab;
> unsigned int nb_prom_envs;
> const char *prom_envs[MAX_PROM_ENVS];
> uint8_t *boot_splash_filedata;
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 57ab9d5322..484e9d9921 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1056,75 +1056,7 @@ static void parse_display(const char *p)
> exit(0);
> }
>
> - if (strstart(p, "sdl", &opts)) {
> - /*
> - * sdl DisplayType needs hand-crafted parser instead of
> - * parse_display_qapi() due to some options not in
> - * DisplayOptions, specifically:
> - * - ctrl_grab + alt_grab
> - * They can't be moved into the QAPI since they use underscores,
> - * thus they will get replaced by "grab-mod" in the long term
> - */
> -#if defined(CONFIG_SDL)
> - dpy.type = DISPLAY_TYPE_SDL;
> - while (*opts) {
> - const char *nextopt;
> -
> - if (strstart(opts, ",grab-mod=", &nextopt)) {
> - opts = nextopt;
> - if (strstart(opts, "lshift-lctrl-lalt", &nextopt)) {
> - alt_grab = 1;
> - } else if (strstart(opts, "rctrl", &nextopt)) {
> - ctrl_grab = 1;
> - } else {
> - goto invalid_sdl_args;
> - }
> - } else if (strstart(opts, ",window-close=", &nextopt)) {
> - opts = nextopt;
> - dpy.has_window_close = true;
> - if (strstart(opts, "on", &nextopt)) {
> - dpy.window_close = true;
> - } else if (strstart(opts, "off", &nextopt)) {
> - dpy.window_close = false;
> - } else {
> - goto invalid_sdl_args;
> - }
> - } else if (strstart(opts, ",show-cursor=", &nextopt)) {
> - opts = nextopt;
> - dpy.has_show_cursor = true;
> - if (strstart(opts, "on", &nextopt)) {
> - dpy.show_cursor = true;
> - } else if (strstart(opts, "off", &nextopt)) {
> - dpy.show_cursor = false;
> - } else {
> - goto invalid_sdl_args;
> - }
> - } else if (strstart(opts, ",gl=", &nextopt)) {
> - opts = nextopt;
> - dpy.has_gl = true;
> - if (strstart(opts, "on", &nextopt)) {
> - dpy.gl = DISPLAYGL_MODE_ON;
> - } else if (strstart(opts, "core", &nextopt)) {
> - dpy.gl = DISPLAYGL_MODE_CORE;
> - } else if (strstart(opts, "es", &nextopt)) {
> - dpy.gl = DISPLAYGL_MODE_ES;
> - } else if (strstart(opts, "off", &nextopt)) {
> - dpy.gl = DISPLAYGL_MODE_OFF;
> - } else {
> - goto invalid_sdl_args;
> - }
> - } else {
> - invalid_sdl_args:
> - error_report("invalid SDL option string");
> - exit(1);
> - }
> - opts = nextopt;
> - }
> -#else
> - error_report("SDL display supported is not available in this
> binary");
> - exit(1);
> -#endif
When CONFIG_SDL is off, the error message changes from
qemu-system-x86_64: -display sdl: SDL display supported is not available in
this binary
to
qemu-system-x86_64: -display sdl: Parameter 'type' does not accept value
'sdl'
I don't mind, but I'd suggest to mention it in the commit message.
> - } else if (strstart(p, "vnc", &opts)) {
> + if (strstart(p, "vnc", &opts)) {
> /*
> * vnc isn't a (local) DisplayType but a protocol for remote
> * display access.
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index d3741f9b75..8cb77416af 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -40,6 +40,8 @@ static struct sdl2_console *sdl2_console;
>
> static SDL_Surface *guest_sprite_surface;
> static int gui_grab; /* if true, all keyboard/mouse events are grabbed */
> +static bool alt_grab;
> +static bool ctrl_grab;
I like that these become internal to sdl2.c.
>
> static int gui_saved_grab;
> static int gui_fullscreen;
> @@ -853,6 +855,14 @@ static void sdl2_display_init(DisplayState *ds,
> DisplayOptions *o)
>
> gui_fullscreen = o->has_full_screen && o->full_screen;
>
> + if (o->u.sdl.has_grab_mod) {
> + if (o->u.sdl.grab_mod == HOT_KEY_MOD_LSHIFT_LCTRL_LALT) {
> + alt_grab = true;
> + } else if (o->u.sdl.grab_mod == HOT_KEY_MOD_RCTRL) {
> + ctrl_grab = true;
> + }
> + }
> +
> for (i = 0;; i++) {
> QemuConsole *con = qemu_console_lookup_by_index(i);
> if (!con) {
Reviewed-by: Markus Armbruster <arm...@redhat.com>
