Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
Hi, > Hmm, ok ... but maybe I should call the new enum HotKeyMod instead of > GrabMod to make it more obvious that it is something different? Looks good to me. take care, Gerd
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On 19/05/2022 12.48, Gerd Hoffmann wrote: Hi, Oh well, I just noticed that we already have a GrabToggleKeys enum in qapi/common.json ... I wonder whether I should try to use that instead? It seems to be used in a slightly different context, though, if I get that right ...? qemu/ui/input-linux.c Those switch the input routing between host and guest, and they act on their own (i.e. by default both control keys without anything else). For SDL it defines the modifiers to press in addition to the hotkeys (i.e. ctrl + shift + 'G' for grab release, IIRC there a are more, 'F' for fullscreen?). So I don't think it makes sense to merge them. Hmm, ok ... but maybe I should call the new enum HotKeyMod instead of GrabMod to make it more obvious that it is something different? Thomas
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
Hi, > Oh well, I just noticed that we already have a GrabToggleKeys enum in > qapi/common.json ... I wonder whether I should try to use that instead? It > seems to be used in a slightly different context, though, if I get that > right ...? qemu/ui/input-linux.c Those switch the input routing between host and guest, and they act on their own (i.e. by default both control keys without anything else). For SDL it defines the modifiers to press in addition to the hotkeys (i.e. ctrl + shift + 'G' for grab release, IIRC there a are more, 'F' for fullscreen?). So I don't think it makes sense to merge them. take care, Gerd
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On 19/05/2022 09.51, Daniel P. Berrangé wrote:
On Thu, May 19, 2022 at 09:27:08AM +0200, Thomas Huth wrote:
On 19/05/2022 09.08, Thomas Huth wrote:
On 19/05/2022 08.39, Thomas Huth wrote:
On 18/05/2022 17.08, Markus Armbruster wrote:
Thomas Huth 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
---
qapi/ui.json | 27 +++-
include/sysemu/sysemu.h | 2 --
softmmu/globals.c | 2 --
softmmu/vl.c | 70 +
ui/sdl2.c | 10 ++
5 files changed, 37 insertions(+), 74 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 11a827d10f..a244e26e0f 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1295,6 +1295,30 @@
'*swap-opt-cmd': 'bool'
} }
+##
+# @GrabMod:
+#
+# Set of modifier keys that need to be hold for shortcut key actions.
+#
+# Since: 7.1
+##
+{ 'enum' : 'GrabMod',
+ 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
This is fine now. If we ever generalize to "arbitrary set of modifier
keys", it'll become somewhat awkward. No objection from me.
Oh well, I just noticed that we already have a GrabToggleKeys enum in
qapi/common.json ... I wonder whether I should try to use that instead? It
seems to be used in a slightly different context, though, if I get that
right ...?
It also doesn't distinguish left & right control/alt/shift keys
for some reason. So you would end up having to add more enum
entries for SDL, none of which overlap with existing enum entries.
We could also extend the SDL code to work with the other combos from
GrabToggleKeys, I guess.
Rather a pity, as the consistency would have been nice
I wonder which way would cause less "WTF?" situations in the future ... if
we have one enum with slightly different naming between the entries, or if
we have two enums that seems to be there for the same or at least very
similar things, which still have still inconsistent namings between the
entries...
I'm slightly inclined to go for the unified GrabToggleKeys enum, I think...
Thomas
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On 19/05/2022 10.57, Markus Armbruster wrote:
Daniel P. Berrangé writes:
On Thu, May 19, 2022 at 09:27:08AM +0200, Thomas Huth wrote:
On 19/05/2022 09.08, Thomas Huth wrote:
On 19/05/2022 08.39, Thomas Huth wrote:
On 18/05/2022 17.08, Markus Armbruster wrote:
Thomas Huth 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
---
qapi/ui.json | 27 +++-
include/sysemu/sysemu.h | 2 --
softmmu/globals.c | 2 --
softmmu/vl.c | 70 +
ui/sdl2.c | 10 ++
5 files changed, 37 insertions(+), 74 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 11a827d10f..a244e26e0f 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1295,6 +1295,30 @@
'*swap-opt-cmd': 'bool'
} }
+##
+# @GrabMod:
+#
+# Set of modifier keys that need to be hold for shortcut key actions.
+#
+# Since: 7.1
+##
+{ 'enum' : 'GrabMod',
+ 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
This is fine now. If we ever generalize to "arbitrary set of modifier
keys", it'll become somewhat awkward. No objection from me.
Oh well, I just noticed that we already have a GrabToggleKeys enum in
qapi/common.json ... I wonder whether I should try to use that instead? It
seems to be used in a slightly different context, though, if I get that
right ...?
It also doesn't distinguish left & right control/alt/shift keys
for some reason. So you would end up having to add more enum
entries for SDL, none of which overlap with existing enum entries.
Rather a pity, as the consistency would have been nice
Speaking of consistency: stick to the key names we use in QKeyCode?
Sadly, they contain '_'.
The "grab-mod" with the current name already exists, so if we want to switch
to those names, we need to deprecate the current ones again ... and the
enums would really look ugly in the code, too, e.g. CTRL_L_ALT_L ... hard to
tell where the "L" really belongs to ... so I'd rather stick with the
current names, I think.
Thomas
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
Daniel P. Berrangé writes:
> On Thu, May 19, 2022 at 09:27:08AM +0200, Thomas Huth wrote:
>> On 19/05/2022 09.08, Thomas Huth wrote:
>> > On 19/05/2022 08.39, Thomas Huth wrote:
>> > > On 18/05/2022 17.08, Markus Armbruster wrote:
>> > > > Thomas Huth 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
>> > > > > ---
>> > > > > qapi/ui.json | 27 +++-
>> > > > > include/sysemu/sysemu.h | 2 --
>> > > > > softmmu/globals.c | 2 --
>> > > > > softmmu/vl.c | 70
>> > > > > +
>> > > > > ui/sdl2.c | 10 ++
>> > > > > 5 files changed, 37 insertions(+), 74 deletions(-)
>> > > > >
>> > > > > diff --git a/qapi/ui.json b/qapi/ui.json
>> > > > > index 11a827d10f..a244e26e0f 100644
>> > > > > --- a/qapi/ui.json
>> > > > > +++ b/qapi/ui.json
>> > > > > @@ -1295,6 +1295,30 @@
>> > > > > '*swap-opt-cmd': 'bool'
>> > > > > } }
>> > > > > +##
>> > > > > +# @GrabMod:
>> > > > > +#
>> > > > > +# Set of modifier keys that need to be hold for shortcut key
>> > > > > actions.
>> > > > > +#
>> > > > > +# Since: 7.1
>> > > > > +##
>> > > > > +{ 'enum' : 'GrabMod',
>> > > > > + 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
>> > > >
>> > > > This is fine now. If we ever generalize to "arbitrary set of modifier
>> > > > keys", it'll become somewhat awkward. No objection from me.
>>
>> Oh well, I just noticed that we already have a GrabToggleKeys enum in
>> qapi/common.json ... I wonder whether I should try to use that instead? It
>> seems to be used in a slightly different context, though, if I get that
>> right ...?
>
> It also doesn't distinguish left & right control/alt/shift keys
> for some reason. So you would end up having to add more enum
> entries for SDL, none of which overlap with existing enum entries.
> Rather a pity, as the consistency would have been nice
Speaking of consistency: stick to the key names we use in QKeyCode?
Sadly, they contain '_'.
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On Thu, May 19, 2022 at 09:27:08AM +0200, Thomas Huth wrote:
> On 19/05/2022 09.08, Thomas Huth wrote:
> > On 19/05/2022 08.39, Thomas Huth wrote:
> > > On 18/05/2022 17.08, Markus Armbruster wrote:
> > > > Thomas Huth 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
> > > > > ---
> > > > > qapi/ui.json | 27 +++-
> > > > > include/sysemu/sysemu.h | 2 --
> > > > > softmmu/globals.c | 2 --
> > > > > softmmu/vl.c | 70
> > > > > +
> > > > > ui/sdl2.c | 10 ++
> > > > > 5 files changed, 37 insertions(+), 74 deletions(-)
> > > > >
> > > > > diff --git a/qapi/ui.json b/qapi/ui.json
> > > > > index 11a827d10f..a244e26e0f 100644
> > > > > --- a/qapi/ui.json
> > > > > +++ b/qapi/ui.json
> > > > > @@ -1295,6 +1295,30 @@
> > > > > '*swap-opt-cmd': 'bool'
> > > > > } }
> > > > > +##
> > > > > +# @GrabMod:
> > > > > +#
> > > > > +# Set of modifier keys that need to be hold for shortcut key actions.
> > > > > +#
> > > > > +# Since: 7.1
> > > > > +##
> > > > > +{ 'enum' : 'GrabMod',
> > > > > + 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
> > > >
> > > > This is fine now. If we ever generalize to "arbitrary set of modifier
> > > > keys", it'll become somewhat awkward. No objection from me.
>
> Oh well, I just noticed that we already have a GrabToggleKeys enum in
> qapi/common.json ... I wonder whether I should try to use that instead? It
> seems to be used in a slightly different context, though, if I get that
> right ...?
It also doesn't distinguish left & right control/alt/shift keys
for some reason. So you would end up having to add more enum
entries for SDL, none of which overlap with existing enum entries.
Rather a pity, as the consistency would have been nice
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
Thomas Huth writes:
> On 18/05/2022 17.08, Markus Armbruster wrote:
>> Thomas Huth 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
>>> ---
>>> qapi/ui.json| 27 +++-
>>> include/sysemu/sysemu.h | 2 --
>>> softmmu/globals.c | 2 --
>>> softmmu/vl.c| 70 +
>>> ui/sdl2.c | 10 ++
>>> 5 files changed, 37 insertions(+), 74 deletions(-)
>>>
>>> diff --git a/qapi/ui.json b/qapi/ui.json
>>> index 11a827d10f..a244e26e0f 100644
>>> --- a/qapi/ui.json
>>> +++ b/qapi/ui.json
>>> @@ -1295,6 +1295,30 @@
>>> '*swap-opt-cmd': 'bool'
>>> } }
>>>
>>> +##
>>> +# @GrabMod:
>>> +#
>>> +# Set of modifier keys that need to be hold for shortcut key actions.
>>> +#
>>> +# Since: 7.1
>>> +##
>>> +{ 'enum' : 'GrabMod',
>>> + 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
>>
>> This is fine now. If we ever generalize to "arbitrary set of modifier
>> keys", it'll become somewhat awkward. No objection from me.
>>
>>> +
>>> +##
>>> +# @DisplaySDL:
>>> +#
>>> +# SDL2 display options.
>>> +#
>>> +# @grab-mod: String with modifier keys that should be pressed together
>>> with
>>
>> s/String with modifier keys/Modifier keys/
>>
>>> +# the "G" key to release the mouse grab. Only
>>> "lshift-lctrl-lalt"
>>> +# and "rctrl" are currently supported.
>>
>> Why do we define lctrl-lalt if it's not supported?
>
> It's the default value if you don't specify the grab-mod parameter. So it's
> supported, you get this mode if you use "grab-mod=lctrl-lalt" or no grab-mod
> parameter at all.
That's not how I understood the comment...
Suggest
# @grab-mod: Modifier keys that should be pressed together with the "G"
#key to release the mouse grab (default "lctrl-lalt")
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On 19/05/2022 09.08, Thomas Huth wrote:
On 19/05/2022 08.39, Thomas Huth wrote:
On 18/05/2022 17.08, Markus Armbruster wrote:
Thomas Huth 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
---
qapi/ui.json | 27 +++-
include/sysemu/sysemu.h | 2 --
softmmu/globals.c | 2 --
softmmu/vl.c | 70 +
ui/sdl2.c | 10 ++
5 files changed, 37 insertions(+), 74 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 11a827d10f..a244e26e0f 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1295,6 +1295,30 @@
'*swap-opt-cmd': 'bool'
} }
+##
+# @GrabMod:
+#
+# Set of modifier keys that need to be hold for shortcut key actions.
+#
+# Since: 7.1
+##
+{ 'enum' : 'GrabMod',
+ 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
This is fine now. If we ever generalize to "arbitrary set of modifier
keys", it'll become somewhat awkward. No objection from me.
Oh well, I just noticed that we already have a GrabToggleKeys enum in
qapi/common.json ... I wonder whether I should try to use that instead? It
seems to be used in a slightly different context, though, if I get that
right ...?
Thomas
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On 19/05/2022 08.39, Thomas Huth wrote:
On 18/05/2022 17.08, Markus Armbruster wrote:
Thomas Huth 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
---
qapi/ui.json | 27 +++-
include/sysemu/sysemu.h | 2 --
softmmu/globals.c | 2 --
softmmu/vl.c | 70 +
ui/sdl2.c | 10 ++
5 files changed, 37 insertions(+), 74 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 11a827d10f..a244e26e0f 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1295,6 +1295,30 @@
'*swap-opt-cmd': 'bool'
} }
+##
+# @GrabMod:
+#
+# Set of modifier keys that need to be hold for shortcut key actions.
+#
+# Since: 7.1
+##
+{ 'enum' : 'GrabMod',
+ 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
This is fine now. If we ever generalize to "arbitrary set of modifier
keys", it'll become somewhat awkward. No objection from me.
+
+##
+# @DisplaySDL:
+#
+# SDL2 display options.
+#
+# @grab-mod: String with modifier keys that should be pressed together
with
s/String with modifier keys/Modifier keys/
+# the "G" key to release the mouse grab. Only
"lshift-lctrl-lalt"
+# and "rctrl" are currently supported.
Why do we define lctrl-lalt if it's not supported?
It's the default value if you don't specify the grab-mod parameter. So it's
supported, you get this mode if you use "grab-mod=lctrl-lalt" or no grab-mod
parameter at all.
Forgot to mention: I'll drop that sentence in v3. Since there is the enum
now, people can look up the valid combinations there instead.
Thomas
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On 18/05/2022 17.41, Eric Blake wrote: On Wed, May 18, 2022 at 03:44:45PM +0200, Thomas Huth wrote: 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 --- +++ b/qapi/ui.json @@ -1295,6 +1295,30 @@ '*swap-opt-cmd': 'bool' } } +## +# @GrabMod: +# +# Set of modifier keys that need to be hold for shortcut key actions. s/hold/held/ Thanks, I'll send a v3 with this (and the nits found by Markus) fixed. Thomas
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On 18/05/2022 17.08, Markus Armbruster wrote:
Thomas Huth 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
---
qapi/ui.json| 27 +++-
include/sysemu/sysemu.h | 2 --
softmmu/globals.c | 2 --
softmmu/vl.c| 70 +
ui/sdl2.c | 10 ++
5 files changed, 37 insertions(+), 74 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 11a827d10f..a244e26e0f 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1295,6 +1295,30 @@
'*swap-opt-cmd': 'bool'
} }
+##
+# @GrabMod:
+#
+# Set of modifier keys that need to be hold for shortcut key actions.
+#
+# Since: 7.1
+##
+{ 'enum' : 'GrabMod',
+ 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
This is fine now. If we ever generalize to "arbitrary set of modifier
keys", it'll become somewhat awkward. No objection from me.
+
+##
+# @DisplaySDL:
+#
+# SDL2 display options.
+#
+# @grab-mod: String with modifier keys that should be pressed together with
s/String with modifier keys/Modifier keys/
+# the "G" key to release the mouse grab. Only "lshift-lctrl-lalt"
+# and "rctrl" are currently supported.
Why do we define lctrl-lalt if it's not supported?
It's the default value if you don't specify the grab-mod parameter. So it's
supported, you get this mode if you use "grab-mod=lctrl-lalt" or no grab-mod
parameter at all.
Thomas
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
On Wed, May 18, 2022 at 03:44:45PM +0200, Thomas Huth wrote: > 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 > --- > +++ b/qapi/ui.json > @@ -1295,6 +1295,30 @@ >'*swap-opt-cmd': 'bool' >} } > > +## > +# @GrabMod: > +# > +# Set of modifier keys that need to be hold for shortcut key actions. s/hold/held/ -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
Thomas Huth 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
> ---
> qapi/ui.json| 27 +++-
> include/sysemu/sysemu.h | 2 --
> softmmu/globals.c | 2 --
> softmmu/vl.c| 70 +
> ui/sdl2.c | 10 ++
> 5 files changed, 37 insertions(+), 74 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 11a827d10f..a244e26e0f 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1295,6 +1295,30 @@
>'*swap-opt-cmd': 'bool'
>} }
>
> +##
> +# @GrabMod:
> +#
> +# Set of modifier keys that need to be hold for shortcut key actions.
> +#
> +# Since: 7.1
> +##
> +{ 'enum' : 'GrabMod',
> + 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
This is fine now. If we ever generalize to "arbitrary set of modifier
keys", it'll become somewhat awkward. No objection from me.
> +
> +##
> +# @DisplaySDL:
> +#
> +# SDL2 display options.
> +#
> +# @grab-mod: String with modifier keys that should be pressed together with
s/String with modifier keys/Modifier keys/
> +# the "G" key to release the mouse grab. Only "lshift-lctrl-lalt"
> +# and "rctrl" are currently supported.
Why do we define lctrl-lalt if it's not supported?
> +#
> +# Since: 7.1
> +##
> +{ 'struct' : 'DisplaySDL',
> + 'data': { '*grab-mod' : 'GrabMod' } }
> +
> ##
> # @DisplayType:
> #
> @@ -1374,7 +1398,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' }
>}
> }
[...]
[PATCH v2 2/3] ui: Switch "-display sdl" to use the QAPI parser
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
---
qapi/ui.json| 27 +++-
include/sysemu/sysemu.h | 2 --
softmmu/globals.c | 2 --
softmmu/vl.c| 70 +
ui/sdl2.c | 10 ++
5 files changed, 37 insertions(+), 74 deletions(-)
diff --git a/qapi/ui.json b/qapi/ui.json
index 11a827d10f..a244e26e0f 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1295,6 +1295,30 @@
'*swap-opt-cmd': 'bool'
} }
+##
+# @GrabMod:
+#
+# Set of modifier keys that need to be hold for shortcut key actions.
+#
+# Since: 7.1
+##
+{ 'enum' : 'GrabMod',
+ 'data' : [ 'lctrl-lalt', 'lshift-lctrl-lalt', 'rctrl' ] }
+
+##
+# @DisplaySDL:
+#
+# SDL2 display options.
+#
+# @grab-mod: String with modifier keys that should be pressed together with
+# the "G" key to release the mouse grab. Only "lshift-lctrl-lalt"
+# and "rctrl" are currently supported.
+#
+# Since: 7.1
+##
+{ 'struct' : 'DisplaySDL',
+ 'data': { '*grab-mod' : 'GrabMod' } }
+
##
# @DisplayType:
#
@@ -1374,7 +1398,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", )) {
-/*
- * 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=", )) {
-opts = nextopt;
-if (strstart(opts, "lshift-lctrl-lalt", )) {
-alt_grab = 1;
-} else if (strstart(opts, "rctrl", )) {
-ctrl_grab = 1;
-} else {
-goto invalid_sdl_args;
-}
-} else if (strstart(opts, ",window-close=", )) {
-opts = nextopt;
-dpy.has_window_close = true;
-if (strstart(opts, "on", )) {
-dpy.window_close = true;
-} else if (strstart(opts, "off", )) {
-dpy.window_close = false;
-} else {
-goto invalid_sdl_args;
-}
-} else if (strstart(opts, ",show-cursor=", )) {
-opts = nextopt;
-dpy.has_show_cursor = true;
-if (strstart(opts, "on", )) {
-dpy.show_cursor = true;
-} else if (strstart(opts, "off", )) {
-dpy.show_cursor = false;
-} else {
-goto invalid_sdl_args;
-}
-} else if (strstart(opts, ",gl=", )) {
-opts = nextopt;
-dpy.has_gl = true;
-if (strstart(opts, "on", )) {
-dpy.gl = DISPLAYGL_MODE_ON;
-
