Re: [PATCH] ui: Remove deprecated parameters of -display sdl and switch to QAPI parser

[Thomas Huth]

On 11/05/2022 14.40, Daniel P. Berrangé wrote:
> On Wed, May 11, 2022 at 02:17:25PM +0200, Thomas Huth wrote:
> > The "-display sdl" option still used a hand-crafted parser for its
> > parameters since some of them used underscores which is forbidden
> > in QAPI. Now that they've been deprecated and the deprecation period
> > is over, we can remove the problematic parameters and switch to use
> > the QAPI parser instead.
>
> I'd say the removal of deprecated bits ought to be separate
> from the addition of new QAPI bits.

Ok, agreed, that likely makes sense.

> > diff --git a/qapi/ui.json b/qapi/ui.json
> > index 059302a5ef..7d5097808a 100644
> > --- a/qapi/ui.json
> > +++ b/qapi/ui.json
> > @@ -1309,6 +1309,19 @@
> >         '*swap-opt-cmd': 'bool'
> >     } }
> >   
> > +##
> > +# @DisplaySDL:
> > +#
> > +# SDL2 display options.
> > +#
> > +# @grab-mod:  Modifier keys that should be pressed together
> > +#             with "G" to release the mouse grab.
> > +#
> > +# Since: 7.1
> > +##
> > +{ 'struct'  : 'DisplaySDL',
> > +  'data'    : { '*grab-mod'   : 'str' } }
>
> So any arbitrary string here, but...
>
> > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > index d3741f9b75..18c63e1fc9 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;
> >   
> >   static int gui_saved_grab;
> >   static int gui_fullscreen;
> > @@ -853,6 +855,17 @@ 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 (g_str_equal(o->u.sdl.grab_mod, "lshift-lctrl-lalt")) {
> > +            alt_grab = true;
> > +        } else if (g_str_equal(o->u.sdl.grab_mod, "rctrl")) {
> > +            ctrl_grab = true;
> > +        } else {
> > +            error_report("Unsupported grab-mod: %s", o->u.sdl.grab_mod);
> > +            exit(1);
> > +        }
> > +    }
>
> We're treating this more like an enum here. It does leave the door
> open for adding a generic parsing of arbitrary grab mods later
> I guess

Yes, that's the basic idea - so one day, the user might specify an arbitrary 
string here consisting of their favorite modifiers.

 Thomas