diff mbox

[v5] cocoa.m: Add ability for user to specify mouse ungrab key

Message ID 20180126214731.23506-1-programmingkidx@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Programmingkid Jan. 26, 2018, 9:47 p.m. UTC
Currently the ungrab keys for the Cocoa and GTK interface are Control-Alt-g.
This combination may not be very fun for the user to have to enter, so we
now enable the user to specify their own key(s) as the ungrab key(s). The
list of keys that can be used is found in the file qapi/ui.json under QKeyCode.
The max number of keys that can be used is three.

Syntax: -ungrab <key-key-key>

Example usage:  -ungrab home
	        -ungrab shift-ctrl
		-ungrab ctrl-x
		-ungrab pgup-pgdn
		-ungrab kp_5-kp_6
		-ungrab kp_4-kp_5-kp_6
		-ungrab ctrl-alt

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
---
v5 changes:
- Removed ungrab detection code from keydown event in handleEvent.
- Removed console_ungrab_sequence_length().
- Removed ability to always use the default ctrl-alt-g ungrab key sequence.
- Added ability to actually send keys to the guest that might overlap ungrab keys. 
Example for -ungrab ctrl-alt:
down(ctrl)
down(alt)
up(ctrl)
up(alt)
..ungrab activates..

down(ctrl)
down(alt)
down(f1)
up(ctrl)
up(alt)
up(f1)
..no ungrab activates..

v4 changes:
- Removed initialization code for key_value_array.
- Added void keyword to console_ungrab_key_sequence(),
  and console_ungrab_key_string() functions.

v3 changes:
- Added the ability for any "sendkey supported" key to be used.
- Added ability for one to three key sequences to be used.

v2 changes:
- Removed the "int i" code from the for loops. 

 include/ui/console.h |   8 ++++
 qemu-options.hx      |   2 +
 ui/cocoa.m           | 109 +++++++++++++++++++++++++++++++++++++++++++++------
 ui/console.c         |  62 +++++++++++++++++++++++++++++
 vl.c                 |   3 ++
 5 files changed, 173 insertions(+), 11 deletions(-)

Comments

Gerd Hoffmann Jan. 30, 2018, 12:18 p.m. UTC | #1
On Fri, Jan 26, 2018 at 04:47:31PM -0500, John Arbuckle wrote:
> Currently the ungrab keys for the Cocoa and GTK interface are Control-Alt-g.

SDL is the same now, for consistency.

> This combination may not be very fun for the user to have to enter, so we
> now enable the user to specify their own key(s) as the ungrab key(s).

What about the other hotkeys?

There is fullscreen.  Ctrl-Alt-F for SDL and GTK.  Cmd-F for cocoa, but
it works only if the grab is not active.

Console select (Ctrl-Alt-<nr>), works for SDL and GTK.  When I read the
code correctly it should work for cocoa the same way, but it doesn't
work for me.  Dunno why.

Quit. Ctrl-Alt-Q on gtk.  Cmd-Q on cocoa, again only working without
keyboard grab.  Nothing on SDL.  Just closing the window to quit works
on GTK and SDL, both have a switch to turn it off.

[ ... list of hotkeys is incomplete, there is more, most of them working
      in some of the user interfaces only ... ]

There is the -ctrl-grab switch.  Changes all hotkeys from Ctrl-Alt-<key>
to Ctrl-<key>.  SDL only.  I want deprecate it.

There is the -alt-grab switch.  Changes all hotkeys from Ctrl-Alt-<key>
to Ctrl-Alt-Shift-<key>.  SDL only.  I want deprecate it too.


When touching this mess I want move to something more consistent.


> Syntax: -ungrab <key-key-key>

As mentioned earlier: New toplevel switch isn't going to fly.  Should be
a suboption of -display.

> Example usage:  -ungrab home
> 	        -ungrab shift-ctrl

Modifier-only hotkeys are tricky with gtk (doable, but no support for
that in the toolkit).

> 		-ungrab ctrl-x
> 		-ungrab pgup-pgdn

Really?  Two non-modifier keys?  How is that implemented?  Do you queue
up the pgup keypress, waiting to see whenever pgdn is pressed too, then
only in case that didn't happen forward the queued pgup key to the guest?

Making this work properly without unpleasent surprises in corner cases
doesn't look easy to me.  Needless to say that the gtk toolkit doesn't
support this either.


I think we should limit ourself to key combinations which have one
non-modifier key and optionally one or more modifier keys.  That should
be supportable in all user interfaces we have.  Except curses, modifier
key handling in unix terminals is a completely different world ...

When it comes to defining hotkeys I see basically two possible
ways to do it:

  (1) Have a fixed (set of) modifier keys for all hot keys,
      i.e. something like this:

         -display gtk,hotkey-modifiers=ctrl+shift,hotkey-grab=f12,hotkey-fullscreen=f11

  (2) Allow complete freedom when defining hotkeys, i.e.

         -display gtk,hotkey-grab=shift+f12,hotkey-fullscreen=ctrl+f11

Variant (1) provides a simple way to use other modifiers for all
hotkeys, simliar to the existing -alt-grab switch.  I also expect
it is easier to implement.

Another question is whenever we want allow defining different hotkeys
for the same thing.  So fullscreen could have both F11 (which is a
common hotkey in various apps, for example firefox) and Ctrl-Alt-F.
Might be useful, but also makes the implementation more complex.

I think we should clarify those questions before working on patches.

cheers,
  Gerd
Programmingkid Jan. 30, 2018, 6 p.m. UTC | #2
> On Jan 30, 2018, at 7:18 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
> On Fri, Jan 26, 2018 at 04:47:31PM -0500, John Arbuckle wrote:
>> Currently the ungrab keys for the Cocoa and GTK interface are Control-Alt-g.
> 
> SDL is the same now, for consistency.
> 
>> This combination may not be very fun for the user to have to enter, so we
>> now enable the user to specify their own key(s) as the ungrab key(s).
> 
> What about the other hotkeys?
> 
> There is fullscreen.  Ctrl-Alt-F for SDL and GTK.  Cmd-F for cocoa, but
> it works only if the grab is not active.

If has to be that way because the meta (command) key is sent to the guest when the mouse grab is in effect. I actually suggest the SDL and GTK interfaces send the meta (windows key on PC keyboard) key to the guest when a mouse grab is in effect. This way guest operating systems like Mac OS X can use keyboard shortcuts. 

> Console select (Ctrl-Alt-<nr>), works for SDL and GTK.  When I read the
> code correctly it should work for cocoa the same way, but it doesn't
> work for me.  Dunno why. 

I know this code was broken in cocoa for a while but I made the patch that fixes this problem. Console selection does work correctly using a recent git version of QEMU.

> Quit. Ctrl-Alt-Q on gtk.  Cmd-Q on cocoa, again only working without
> keyboard grab.  Nothing on SDL.  Just closing the window to quit works
> on GTK and SDL, both have a switch to turn it off.

I know Command-Q is the standard Macintosh keyboard short for quitting an application. With GTK I would imagine it would depend on how the host operating system expects applications to be built. I don't think Windows and Linux have a standard keyboard shortcut for quitting an application. 

> 
> [ ... list of hotkeys is incomplete, there is more, most of them working
>      in some of the user interfaces only ... ]
> 
> There is the -ctrl-grab switch.  Changes all hotkeys from Ctrl-Alt-<key>
> to Ctrl-<key>.  SDL only.  I want deprecate it.
> 
> There is the -alt-grab switch.  Changes all hotkeys from Ctrl-Alt-<key>
> to Ctrl-Alt-Shift-<key>.  SDL only.  I want deprecate it too.

Sounds like a good idea. It would help to keep things consistent between the UI's.

> 
> When touching this mess I want move to something more consistent.
> 
> 
>> Syntax: -ungrab <key-key-key>
> 
> As mentioned earlier: New toplevel switch isn't going to fly.  Should be
> a suboption of -display.

Is this good?:
-display ungrab=<key>... 

<looks further down the email>. 

Ok I see what you want:

-display gtk,hotkey-modifiers=ctrl+shift,hotkey-grab=f12,hotkey-fullscreen=f11

-display cocoa,hotkey-modifiers=Command,hotkey-grab=f13,hotkey-fullscreen=f14

I assume hotkey-modifiers is used to set the meta key. Is hotkey-grab the key
used to ungrab the mouse? If it is shouldn't it be called hotkey-ungrab?


> 
>> Example usage:  -ungrab home
>> 	        -ungrab shift-ctrl
> 
> Modifier-only hotkeys are tricky with gtk (doable, but no support for
> that in the toolkit).
> 
>> 		-ungrab ctrl-x
>> 		-ungrab pgup-pgdn
> 
> Really?  Two non-modifier keys?
Yes. The ungrab sequence could be a-b-c and still allow these keys to be used in the guest.

>  How is that implemented?  Do you queue
> up the pgup keypress, waiting to see whenever pgdn is pressed too, then
> only in case that didn't happen forward the queued pgup key to the guest?

Basically there is a array that acts as a check list. It checks off keys that belong to the ungrab sequence as they are detected. Once a non-ungrab key is detected, the check list is cleared. If all the ungrab keys are detected the ungrab code is executed. This only happens on keyup events. That way if Control-ALT were the ungrab keys, sending Control-ALT-Delete to the guest is still possible because these are the keys detected on the keyup event. The Delete key would have cleared the check list. Daniel Berrange is the one I can thank for this idea. He might be able to explain it better than me.

> Making this work properly without unpleasent surprises in corner cases
> doesn't look easy to me.  Needless to say that the gtk toolkit doesn't
> support this either.

I know it sounds hard to implement but it really isn't. It is just knowing which pieces fit where. The keyup and keydown events is where most of the action takes place.

> I think we should limit ourself to key combinations which have one
> non-modifier key and optionally one or more modifier keys.  That should
> be supportable in all user interfaces we have.  Except curses, modifier
> key handling in unix terminals is a completely different world ...
> 
> When it comes to defining hotkeys I see basically two possible
> ways to do it:
> 
>  (1) Have a fixed (set of) modifier keys for all hot keys,
>      i.e. something like this:
> 
>         -display gtk,hotkey-modifiers=ctrl+shift,hotkey-grab=f12,hotkey-fullscreen=f11
> 
>  (2) Allow complete freedom when defining hotkeys, i.e.
> 
>         -display gtk,hotkey-grab=shift+f12,hotkey-fullscreen=ctrl+f11
> 
> Variant (1) provides a simple way to use other modifiers for all
> hotkeys, simliar to the existing -alt-grab switch.  I also expect
> it is easier to implement.

Choice 2 sounds really nice. It does give the user total freedom to customize QEMU
as seen fit.

> Another question is whenever we want allow defining different hotkeys
> for the same thing.  So fullscreen could have both F11 (which is a
> common hotkey in various apps, for example firefox) and Ctrl-Alt-F.
> Might be useful, but also makes the implementation more complex.

Assigning two different keyboard shortcuts to the same feature might be problematic.
Keeping things to one keyboard shortcut is probably the simple thing to do.

> 
> I think we should clarify those questions before working on patches.

Agreed.

> 
> cheers,
>  Gerd

Thank you for the clarification.
Gerd Hoffmann Jan. 31, 2018, 9:03 a.m. UTC | #3
> > What about the other hotkeys?
> > 
> > There is fullscreen.  Ctrl-Alt-F for SDL and GTK.  Cmd-F for cocoa, but
> > it works only if the grab is not active.
> 
> If has to be that way because the meta (command) key is sent to the
> guest when the mouse grab is in effect. I actually suggest the SDL and
> GTK interfaces send the meta (windows key on PC keyboard) key to the
> guest when a mouse grab is in effect. This way guest operating systems
> like Mac OS X can use keyboard shortcuts. 

Well, the modifier key changes are actually sent to the guest, but
usually they have no effect there.

So, if you press Ctrl-Alt-F, the guest will see the keydown and keyup
events for ctrl and alt.  It will not see the 'f', because SDL hijacks
that.

> > Console select (Ctrl-Alt-<nr>), works for SDL and GTK.  When I read the
> > code correctly it should work for cocoa the same way, but it doesn't
> > work for me.  Dunno why. 
> 
> I know this code was broken in cocoa for a while but I made the patch
> that fixes this problem. Console selection does work correctly using a
> recent git version of QEMU.

Just pulled latest master and recompiled, still not working for me.
This is sierra in a vm.

Unrelated side note: hvf doesn't work for me too (nested on kvm).

> > Quit. Ctrl-Alt-Q on gtk.  Cmd-Q on cocoa, again only working without
> > keyboard grab.  Nothing on SDL.  Just closing the window to quit works
> > on GTK and SDL, both have a switch to turn it off.
> 
> I know Command-Q is the standard Macintosh keyboard short for quitting
> an application. With GTK I would imagine it would depend on how the
> host operating system expects applications to be built. I don't think
> Windows and Linux have a standard keyboard shortcut for quitting an
> application. 

On Linux Ctrl-Q seems to be common (maybe only in the gnome world).
On Windows Alt-F4 should be standard.

Qemu picked two-modifier hot-keys (like Ctrl-Alt-Q) to avoid overlaps
with guest hot keys, so both qemu and guest hotkeys can be used without
having to deal with keyboard/mouse grabs all the time.

> Ok I see what you want:
> 
> -display gtk,hotkey-modifiers=ctrl+shift,hotkey-grab=f12,hotkey-fullscreen=f11
> 
> -display cocoa,hotkey-modifiers=Command,hotkey-grab=f13,hotkey-fullscreen=f14

I think that would be "super" not "Command" because that is the name for
the key in the pc world so this is what the key got as QKeyCode name.

> I assume hotkey-modifiers is used to set the meta key.

Yes.

> Is hotkey-grab the key used to ungrab the mouse?

Yes (keyboard grab too).

> If it is shouldn't it be called hotkey-ungrab?

Good question.  On gtk/sdl Ctrl-Alt-G actually toggles the grab, i.e.
can do *both* grab and ungrab.  But I expect most users use it for
ungrab only, due to mouse clicks activating the grab too.

> >> Example usage:  -ungrab home
> >> 	        -ungrab shift-ctrl
> > 
> > Modifier-only hotkeys are tricky with gtk (doable, but no support for
> > that in the toolkit).
> > 
> >> 		-ungrab ctrl-x
> >> 		-ungrab pgup-pgdn
> > 
> > Really?  Two non-modifier keys?
> Yes. The ungrab sequence could be a-b-c and still allow these keys to be used in the guest.
> 
> >  How is that implemented?  Do you queue
> > up the pgup keypress, waiting to see whenever pgdn is pressed too, then
> > only in case that didn't happen forward the queued pgup key to the guest?
> 
> Basically there is a array that acts as a check list. It checks off
> keys that belong to the ungrab sequence as they are detected. Once a
> non-ungrab key is detected, the check list is cleared. If all the
> ungrab keys are detected the ungrab code is executed. This only
> happens on keyup events. That way if Control-ALT were the ungrab keys,
> sending Control-ALT-Delete to the guest is still possible because
> these are the keys detected on the keyup event. The Delete key would
> have cleared the check list. Daniel Berrange is the one I can thank
> for this idea. He might be able to explain it better than me.

Hmm, ok.

Doing the same for gtk would basically imply to not use any toolkit
support for hotkeys ...

It'll also become more difficuilt if we use that for multiple hotkeys.

But possibly we can share the code across all UIs, now that keycodemapdb
is used by qemu.  So first translate the keycode from the UI toolkit to
a QKeyCode, then feed that into shared hotkey detection code.

> > I think we should limit ourself to key combinations which have one
> > non-modifier key and optionally one or more modifier keys.  That
> > should be supportable in all user interfaces we have.  Except
> > curses, modifier key handling in unix terminals is a completely
> > different world ...
> > 
> > When it comes to defining hotkeys I see basically two possible ways
> > to do it:
> > 
> >  (1) Have a fixed (set of) modifier keys for all hot keys, i.e.
> >  something like this:
> > 
> >         -display gtk,hotkey-modifiers=ctrl+shift,hotkey-grab=f12,hotkey-fullscreen=f11
> > 
> >  (2) Allow complete freedom when defining hotkeys, i.e.
> > 
> >         -display gtk,hotkey-grab=shift+f12,hotkey-fullscreen=ctrl+f11
> > 
> > Variant (1) provides a simple way to use other modifiers for all
> > hotkeys, simliar to the existing -alt-grab switch.  I also expect it
> > is easier to implement.
> 
> Choice 2 sounds really nice. It does give the user total freedom to
> customize QEMU as seen fit.

But on the other hand switching all hotkeys from ctrl-alt-<key> to
ctrl-shift-<key> is alot of work with choice (2) because you have to
redefine all hotkeys.  With choice (1) it is a simple
"hotkey-modifiers=ctrl+shift".

So both approaches have their specific advantages ...

> > Another question is whenever we want allow defining different
> > hotkeys for the same thing.  So fullscreen could have both F11
> > (which is a common hotkey in various apps, for example firefox) and
> > Ctrl-Alt-F.  Might be useful, but also makes the implementation more
> > complex.
> 
> Assigning two different keyboard shortcuts to the same feature might
> be problematic.  Keeping things to one keyboard shortcut is probably
> the simple thing to do.

Yes.  I also think the added complexity needed isn't worth the benefits.

cheers,
  Gerd
Programmingkid Jan. 31, 2018, 10:51 p.m. UTC | #4
> On Jan 31, 2018, at 4:03 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> 
>>> What about the other hotkeys?
>>> 
>>> There is fullscreen.  Ctrl-Alt-F for SDL and GTK.  Cmd-F for cocoa, but
>>> it works only if the grab is not active.
>> 
>> If has to be that way because the meta (command) key is sent to the
>> guest when the mouse grab is in effect. I actually suggest the SDL and
>> GTK interfaces send the meta (windows key on PC keyboard) key to the
>> guest when a mouse grab is in effect. This way guest operating systems
>> like Mac OS X can use keyboard shortcuts. 
> 
> Well, the modifier key changes are actually sent to the guest, but
> usually they have no effect there.
> 
> So, if you press Ctrl-Alt-F, the guest will see the keydown and keyup
> events for ctrl and alt.  It will not see the 'f', because SDL hijacks
> that.

Maybe the f could be sent to the guest directly via qemu_input_event_send_key_qcode().

> 
>>> Console select (Ctrl-Alt-<nr>), works for SDL and GTK.  When I read the
>>> code correctly it should work for cocoa the same way, but it doesn't
>>> work for me.  Dunno why. 
>> 
>> I know this code was broken in cocoa for a while but I made the patch
>> that fixes this problem. Console selection does work correctly using a
>> recent git version of QEMU.
> 
> Just pulled latest master and recompiled, still not working for me.
> This is sierra in a vm.

What software do you use to run the VM? It could be possible the VM software is interfering with the ungrab keys.

> Unrelated side note: hvf doesn't work for me too (nested on kvm).

I haven't played with this new feature yet. Maybe I will try it out one day.

> 
>>> Quit. Ctrl-Alt-Q on gtk.  Cmd-Q on cocoa, again only working without
>>> keyboard grab.  Nothing on SDL.  Just closing the window to quit works
>>> on GTK and SDL, both have a switch to turn it off.
>> 
>> I know Command-Q is the standard Macintosh keyboard short for quitting
>> an application. With GTK I would imagine it would depend on how the
>> host operating system expects applications to be built. I don't think
>> Windows and Linux have a standard keyboard shortcut for quitting an
>> application. 
> 
> On Linux Ctrl-Q seems to be common (maybe only in the gnome world).
> On Windows Alt-F4 should be standard.
> 
> Qemu picked two-modifier hot-keys (like Ctrl-Alt-Q) to avoid overlaps
> with guest hot keys, so both qemu and guest hotkeys can be used without
> having to deal with keyboard/mouse grabs all the time.
> 
>> Ok I see what you want:
>> 
>> -display gtk,hotkey-modifiers=ctrl+shift,hotkey-grab=f12,hotkey-fullscreen=f11
>> 
>> -display cocoa,hotkey-modifiers=Command,hotkey-grab=f13,hotkey-fullscreen=f14
> 
> I think that would be "super" not "Command" because that is the name for
> the key in the pc world so this is what the key got as QKeyCode name.
> 
>> I assume hotkey-modifiers is used to set the meta key.
> 
> Yes.
> 
>> Is hotkey-grab the key used to ungrab the mouse?
> 
> Yes (keyboard grab too).
> 
>> If it is shouldn't it be called hotkey-ungrab?
> 
> Good question.  On gtk/sdl Ctrl-Alt-G actually toggles the grab, i.e.
> can do *both* grab and ungrab.  But I expect most users use it for
> ungrab only, due to mouse clicks activating the grab too.
> 
>>>> Example usage:  -ungrab home
>>>> 	        -ungrab shift-ctrl
>>> 
>>> Modifier-only hotkeys are tricky with gtk (doable, but no support for
>>> that in the toolkit).
>>> 
>>>> 		-ungrab ctrl-x
>>>> 		-ungrab pgup-pgdn
>>> 
>>> Really?  Two non-modifier keys?
>> Yes. The ungrab sequence could be a-b-c and still allow these keys to be used in the guest.
>> 
>>> How is that implemented?  Do you queue
>>> up the pgup keypress, waiting to see whenever pgdn is pressed too, then
>>> only in case that didn't happen forward the queued pgup key to the guest?
>> 
>> Basically there is a array that acts as a check list. It checks off
>> keys that belong to the ungrab sequence as they are detected. Once a
>> non-ungrab key is detected, the check list is cleared. If all the
>> ungrab keys are detected the ungrab code is executed. This only
>> happens on keyup events. That way if Control-ALT were the ungrab keys,
>> sending Control-ALT-Delete to the guest is still possible because
>> these are the keys detected on the keyup event. The Delete key would
>> have cleared the check list. Daniel Berrange is the one I can thank
>> for this idea. He might be able to explain it better than me.
> 
> Hmm, ok.
> 
> Doing the same for gtk would basically imply to not use any toolkit
> support for hotkeys ...
> 
> It'll also become more difficuilt if we use that for multiple hotkeys.
> 
> But possibly we can share the code across all UIs, now that keycodemapdb
> is used by qemu.  So first translate the keycode from the UI toolkit to
> a QKeyCode, then feed that into shared hotkey detection code.

Sounds like a good idea. I will be more than happy to help. My cocoa code
uses sets to implement keeping track of key presses. A set is a collection of data 
that only allows for unique values. So no two items in a set can be the same
value. My patch uses cocoa's NSSet so the shared code would probably not be
able to use it. I don't currently know of an implementation of a set that is
in C and available for us to use, so we may have to implement it ourselves. 

> 
>>> I think we should limit ourself to key combinations which have one
>>> non-modifier key and optionally one or more modifier keys.  That
>>> should be supportable in all user interfaces we have.  Except
>>> curses, modifier key handling in unix terminals is a completely
>>> different world ...
>>> 
>>> When it comes to defining hotkeys I see basically two possible ways
>>> to do it:
>>> 
>>> (1) Have a fixed (set of) modifier keys for all hot keys, i.e.
>>> something like this:
>>> 
>>>        -display gtk,hotkey-modifiers=ctrl+shift,hotkey-grab=f12,hotkey-fullscreen=f11
>>> 
>>> (2) Allow complete freedom when defining hotkeys, i.e.
>>> 
>>>        -display gtk,hotkey-grab=shift+f12,hotkey-fullscreen=ctrl+f11
>>> 
>>> Variant (1) provides a simple way to use other modifiers for all
>>> hotkeys, simliar to the existing -alt-grab switch.  I also expect it
>>> is easier to implement.
>> 
>> Choice 2 sounds really nice. It does give the user total freedom to
>> customize QEMU as seen fit.
> 
> But on the other hand switching all hotkeys from ctrl-alt-<key> to
> ctrl-shift-<key> is alot of work with choice (2) because you have to
> redefine all hotkeys.  With choice (1) it is a simple
> "hotkey-modifiers=ctrl+shift".
> 
> So both approaches have their specific advantages ...
> 
>>> Another question is whenever we want allow defining different
>>> hotkeys for the same thing.  So fullscreen could have both F11
>>> (which is a common hotkey in various apps, for example firefox) and
>>> Ctrl-Alt-F.  Might be useful, but also makes the implementation more
>>> complex.
>> 
>> Assigning two different keyboard shortcuts to the same feature might
>> be problematic.  Keeping things to one keyboard shortcut is probably
>> the simple thing to do.
> 
> Yes.  I also think the added complexity needed isn't worth the benefits.
> 
> cheers,
>  Gerd
>
Gerd Hoffmann Feb. 1, 2018, 8:08 a.m. UTC | #5
Hi,

> > Well, the modifier key changes are actually sent to the guest, but
> > usually they have no effect there.
> > 
> > So, if you press Ctrl-Alt-F, the guest will see the keydown and keyup
> > events for ctrl and alt.  It will not see the 'f', because SDL hijacks
> > that.
> 
> Maybe the f could be sent to the guest directly via qemu_input_event_send_key_qcode().

Why?  If the user presses the ctrl-alt-f hotkey it expects qemu react on
it (and toggle fullscreen mode), so it the 'f' keydown is
*intentionally* not sent to the guest.  But the guest still can see the
modifier (ctrl and alt) key events.

> > Just pulled latest master and recompiled, still not working for me.
> > This is sierra in a vm.
> 
> What software do you use to run the VM? It could be possible the VM
> software is interfering with the ungrab keys.

qemu, with vnc and virt-viewer.  Ahem, right, the qemu vnc server reacts
on ctrl-alt-<nr> too, so that is probably the reason.  The joy of nested
virtualization ...

> >> Basically there is a array that acts as a check list. It checks off
> >> keys that belong to the ungrab sequence as they are detected. Once a
> >> non-ungrab key is detected, the check list is cleared. If all the
> >> ungrab keys are detected the ungrab code is executed. This only
> >> happens on keyup events. That way if Control-ALT were the ungrab keys,
> >> sending Control-ALT-Delete to the guest is still possible because
> >> these are the keys detected on the keyup event. The Delete key would
> >> have cleared the check list. Daniel Berrange is the one I can thank
> >> for this idea. He might be able to explain it better than me.
> > 
> > Hmm, ok.
> > 
> > Doing the same for gtk would basically imply to not use any toolkit
> > support for hotkeys ...
> > 
> > It'll also become more difficuilt if we use that for multiple hotkeys.
> > 
> > But possibly we can share the code across all UIs, now that keycodemapdb
> > is used by qemu.  So first translate the keycode from the UI toolkit to
> > a QKeyCode, then feed that into shared hotkey detection code.
> 
> Sounds like a good idea. I will be more than happy to help. My cocoa code
> uses sets to implement keeping track of key presses. A set is a collection of data 
> that only allows for unique values. So no two items in a set can be the same
> value. My patch uses cocoa's NSSet so the shared code would probably not be
> able to use it. I don't currently know of an implementation of a set that is
> in C and available for us to use, so we may have to implement it ourselves. 

Yes, probably.  Using cocoa (or objc) features for shared code isn't
going to work.  Also I'm not sure a set is a good coice here, it looses
the ordering of the keydown events.  Which doesn't matter in case the
hotkey is pressed, but in case it turns out it wasn't the hotkey and we
have to forward the recorded events to the guest we should not re-order
them.

cheers,
  Gerd
BALATON Zoltan Feb. 1, 2018, 12:16 p.m. UTC | #6
On Wed, 31 Jan 2018, Programmingkid wrote:
>>> Basically there is a array that acts as a check list. It checks off
>>> keys that belong to the ungrab sequence as they are detected. Once a
>>> non-ungrab key is detected, the check list is cleared. If all the
>>> ungrab keys are detected the ungrab code is executed. This only
>>> happens on keyup events. That way if Control-ALT were the ungrab keys,
>>> sending Control-ALT-Delete to the guest is still possible because
>>> these are the keys detected on the keyup event. The Delete key would
>>> have cleared the check list. Daniel Berrange is the one I can thank
>>> for this idea. He might be able to explain it better than me.
>>
>> Hmm, ok.
>>
>> Doing the same for gtk would basically imply to not use any toolkit
>> support for hotkeys ...
>>
>> It'll also become more difficuilt if we use that for multiple hotkeys.
>>
>> But possibly we can share the code across all UIs, now that keycodemapdb
>> is used by qemu.  So first translate the keycode from the UI toolkit to
>> a QKeyCode, then feed that into shared hotkey detection code.
>
> Sounds like a good idea. I will be more than happy to help. My cocoa code
> uses sets to implement keeping track of key presses. A set is a collection of data
> that only allows for unique values. So no two items in a set can be the same
> value. My patch uses cocoa's NSSet so the shared code would probably not be
> able to use it. I don't currently know of an implementation of a set that is
> in C and available for us to use, so we may have to implement it ourselves.

I think we already depend on glib so maybe you could find something useful 
for this here:

https://developer.gnome.org/glib/stable/glib-data-types.html

Regards,
BALATON Zoltan
Programmingkid Feb. 1, 2018, 1:11 p.m. UTC | #7
> On Feb 1, 2018, at 7:16 AM, BALATON Zoltan <balaton@eik.bme.hu> wrote:
> 
> On Wed, 31 Jan 2018, Programmingkid wrote:
>>>> Basically there is a array that acts as a check list. It checks off
>>>> keys that belong to the ungrab sequence as they are detected. Once a
>>>> non-ungrab key is detected, the check list is cleared. If all the
>>>> ungrab keys are detected the ungrab code is executed. This only
>>>> happens on keyup events. That way if Control-ALT were the ungrab keys,
>>>> sending Control-ALT-Delete to the guest is still possible because
>>>> these are the keys detected on the keyup event. The Delete key would
>>>> have cleared the check list. Daniel Berrange is the one I can thank
>>>> for this idea. He might be able to explain it better than me.
>>> 
>>> Hmm, ok.
>>> 
>>> Doing the same for gtk would basically imply to not use any toolkit
>>> support for hotkeys ...
>>> 
>>> It'll also become more difficuilt if we use that for multiple hotkeys.
>>> 
>>> But possibly we can share the code across all UIs, now that keycodemapdb
>>> is used by qemu.  So first translate the keycode from the UI toolkit to
>>> a QKeyCode, then feed that into shared hotkey detection code.
>> 
>> Sounds like a good idea. I will be more than happy to help. My cocoa code
>> uses sets to implement keeping track of key presses. A set is a collection of data
>> that only allows for unique values. So no two items in a set can be the same
>> value. My patch uses cocoa's NSSet so the shared code would probably not be
>> able to use it. I don't currently know of an implementation of a set that is
>> in C and available for us to use, so we may have to implement it ourselves.
> 
> I think we already depend on glib so maybe you could find something useful for this here:
> 
> https://developer.gnome.org/glib/stable/glib-data-types.html
> 
> Regards,
> BALATON Zoltan

Thanks BALATON for the help, but I didn't see anything that was a good fit for the patch.
diff mbox

Patch

diff --git a/include/ui/console.h b/include/ui/console.h
index 580dfc57ee..f346cb05d0 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -534,4 +534,12 @@  static inline void early_gtk_display_init(int opengl)
 /* egl-headless.c */
 void egl_headless_init(void);
 
+/* console.c */
+/* max number of keys that can be used as the ungrab keys */
+#define MAX_UNGRAB_KEYS 3
+void set_ungrab_seq(const char *new_seq);
+int *console_ungrab_key_sequence(void);
+const char *console_ungrab_key_string(void);
+void use_default_ungrab_keys(void);
+void init_ungrab_keys(void);
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index 678181c599..5ba66905f5 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4251,6 +4251,8 @@  contents of @code{iv.b64} to the second secret
 
 ETEXI
 
+DEF("ungrab", HAS_ARG, QEMU_OPTION_ungrab, \
+    "-ungrab <key sequence>", QEMU_ARCH_ALL)
 
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 330ccebf90..1dc9b23941 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -105,6 +105,8 @@ 
 bool stretch_video;
 NSTextField *pauseLabel;
 NSArray * supportedImageFileTypes;
+NSMutableSet *key_set, *ungrab_set;
+int ungrab_sequence_length;
 
 // Mac to QKeyCode conversion
 const int mac_to_qkeycode_map[] = {
@@ -303,6 +305,7 @@  - (float) cdx;
 - (float) cdy;
 - (QEMUScreen) gscreen;
 - (void) raiseAllKeys;
+- (BOOL) is_ungrab_seq;
 @end
 
 QemuCocoaView *cocoaView;
@@ -488,8 +491,6 @@  - (void) switchSurface:(DisplaySurface *)surface
         [[fullScreenWindow contentView] setFrame:[[NSScreen mainScreen] frame]];
         [normalWindow setFrame:NSMakeRect([normalWindow frame].origin.x, [normalWindow frame].origin.y - h + oldh, w, h + [normalWindow frame].size.height - oldh) display:NO animate:NO];
     } else {
-        if (qemu_name)
-            [normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s", qemu_name]];
         [normalWindow setFrame:NSMakeRect([normalWindow frame].origin.x, [normalWindow frame].origin.y - h + oldh, w, h + [normalWindow frame].size.height - oldh) display:YES animate:NO];
     }
 
@@ -669,14 +670,29 @@  - (void) handleEvent:(NSEvent *)event
                 if (keycode == Q_KEY_CODE_CAPS_LOCK ||
                     keycode == Q_KEY_CODE_NUM_LOCK) {
                     [self toggleStatefulModifier:keycode];
-                } else if (qemu_console_is_graphic(NULL)) {
+                } else {
                   [self toggleModifier:keycode];
                 }
             }
 
+            /*
+             * This code has to be here because the user might use a modifier
+             * key like shift as an ungrab key.
+             */
+            if (modifiers_state[keycode] == YES) { // if the key is down
+                [self check_key: keycode];
+            } else {                               // if the key is up
+                if ([self is_ungrab_seq] == YES) {
+                    [self ungrabMouse];
+                    [self clear_ungrab_array];
+                    return;
+                }
+                [self remove_key_from_ungrab_array: keycode];
+            }
             break;
         case NSEventTypeKeyDown:
             keycode = cocoa_keycode_to_qemu([event keyCode]);
+            [self check_key: keycode];
 
             // forward command key combos to the host UI unless the mouse is grabbed
             if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
@@ -686,7 +702,7 @@  - (void) handleEvent:(NSEvent *)event
 
             // default
 
-            // handle control + alt Key Combos (ctrl+alt+[1..9,g] is reserved for QEMU)
+            // handle control + alt Key Combos (ctrl+alt+[1..9] is reserved for QEMU)
             if (([event modifierFlags] & NSEventModifierFlagControl) && ([event modifierFlags] & NSEventModifierFlagOption)) {
                 NSString *keychar = [event charactersIgnoringModifiers];
                 if ([keychar length] == 1) {
@@ -697,11 +713,6 @@  - (void) handleEvent:(NSEvent *)event
                         case '1' ... '9':
                             console_select(key - '0' - 1); /* ascii math */
                             return;
-
-                        // release the mouse grab
-                        case 'g':
-                            [self ungrabMouse];
-                            return;
                     }
                 }
             }
@@ -715,6 +726,13 @@  - (void) handleEvent:(NSEvent *)event
         case NSEventTypeKeyUp:
             keycode = cocoa_keycode_to_qemu([event keyCode]);
 
+            if ([self is_ungrab_seq] == YES) {
+                [self ungrabMouse];
+                [self clear_ungrab_array];
+                return;
+            }
+            [self remove_key_from_ungrab_array: keycode];
+
             // don't pass the guest a spurious key-up if we treated this
             // command-key combo as a host UI action
             if (!isMouseGrabbed && ([event modifierFlags] & NSEventModifierFlagCommand)) {
@@ -842,10 +860,13 @@  - (void) grabMouse
     COCOA_DEBUG("QemuCocoaView: grabMouse\n");
 
     if (!isFullscreen) {
+        NSString * message_string;
+        message_string = [NSString stringWithFormat: @"- (Press %s to release Mouse)", console_ungrab_key_string()];
+
         if (qemu_name)
-            [normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s - (Press ctrl + alt + g to release Mouse)", qemu_name]];
+            [normalWindow setTitle:[NSString stringWithFormat: @"QEMU %s %@", qemu_name, message_string]];
         else
-            [normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release Mouse)"];
+            [normalWindow setTitle:[NSString stringWithFormat: @"QEMU %@", message_string]];
     }
     [self hideCursor];
     if (!isAbsoluteEnabled) {
@@ -898,6 +919,47 @@  - (void) raiseAllKeys
        }
    }
 }
+
+/* Determines if a key is one of the mouse ungrab keys */
+- (BOOL) is_an_ungrab_key: (int) keycode
+{
+    return [ungrab_set containsObject: [NSNumber numberWithInt: keycode]];
+}
+
+/* Adds an ungrab key to an array that tracks if the ungrab keys are pushed */
+- (void) add_key_to_ungrab_array: (int) keycode
+{
+    [key_set addObject: [NSNumber numberWithInt: keycode]];
+}
+
+/* Removes a key from ungrab key tracking */
+- (void) remove_key_from_ungrab_array: (int) keycode
+{
+    [key_set removeObject: [NSNumber numberWithInt: keycode]];
+}
+
+/* Clears the array used to track the ungrab keys */
+- (void) clear_ungrab_array
+{
+    [key_set removeAllObjects];
+}
+
+/* Check the keycode to see if it one of the ungrab keys */
+- (void) check_key: (int) keycode
+{
+    if ([self is_an_ungrab_key: keycode]) {
+        [self add_key_to_ungrab_array: keycode];
+    } else {
+        [self clear_ungrab_array];
+    }
+}
+
+/* Determines if the user specified ungrab sequence is being used */
+- (BOOL) is_ungrab_seq
+{
+    return [ungrab_set isEqualToSet: key_set];
+}
+
 @end
 
 
@@ -1671,6 +1733,29 @@  static void addRemovableDevicesMenuItems(void)
     qapi_free_BlockInfoList(pointerToFree);
 }
 
+/* initializes the mouse ungrab system */
+static void ungrab_init(void)
+{
+    key_set = [NSMutableSet new];
+    ungrab_set = [NSMutableSet new];
+    init_ungrab_keys();
+
+    /* determine length of the mouse ungrab sequence */
+    int index, *ungrab_seq;
+    ungrab_sequence_length = 0;
+    ungrab_seq = console_ungrab_key_sequence();
+    for (index = 0; index < MAX_UNGRAB_KEYS; index++) {
+        if (ungrab_seq[index] != 0) {
+            ungrab_sequence_length++;
+        }
+    }
+
+    /* make the ungrab set */
+    for (index = 0; index < ungrab_sequence_length; index++) {
+        [ungrab_set addObject: [NSNumber numberWithInt: ungrab_seq[index]]];
+    }
+}
+
 void cocoa_display_init(DisplayState *ds, int full_screen)
 {
     COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n");
@@ -1700,4 +1785,6 @@  void cocoa_display_init(DisplayState *ds, int full_screen)
      * find out what removable devices it has.
      */
     addRemovableDevicesMenuItems();
+
+    ungrab_init();
 }
diff --git a/ui/console.c b/ui/console.c
index c4c95abed7..9ff599706d 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -63,6 +63,12 @@  typedef struct QEMUFIFO {
     int count, wptr, rptr;
 } QEMUFIFO;
 
+/* stores the ungrab keys' values */
+static int key_value_array[MAX_UNGRAB_KEYS + 1];
+
+/* stores the string that is returned by console_ungrab_key_string */
+static char *ungrab_key_string;
+
 static int qemu_fifo_write(QEMUFIFO *f, const uint8_t *buf, int len1)
 {
     int l, len;
@@ -2239,4 +2245,60 @@  static void register_types(void)
     type_register_static(&qemu_console_info);
 }
 
+/* Sets the mouse ungrab key sequence to what the user wants */
+void set_ungrab_seq(const char *new_seq)
+{
+    const char *separator = "-";  /* What the user places between keys */
+    gchar **key_array;
+    int key_value, count;
+
+    count = 0;
+    key_array = g_strsplit(new_seq, separator, -1);
+    ungrab_key_string = g_strdup(new_seq);
+
+    for (; *key_array; key_array++) {
+        key_value = index_from_key(*key_array, strlen(*key_array));
+        if (key_value == Q_KEY_CODE__MAX) {
+            printf("-ungrab: unknown key: %s\n", *key_array);
+            exit(EXIT_FAILURE);
+        }
+        key_value_array[count] = key_value;
+        count++;
+    }
+}
+
+/* Returns the user specified ungrab key sequence */
+int *console_ungrab_key_sequence(void)
+{
+    return key_value_array;
+}
+
+/* Returns the name of the user specified ungrab keys */
+const char *console_ungrab_key_string(void)
+{
+    return ungrab_key_string;
+}
+
+/* Sets the UI to use the default ungrab key sequence */
+void use_default_ungrab_keys(void)
+{
+    /* Default ungrab keys: Control Alt g */
+    ungrab_key_string = (char *) malloc(sizeof(char) * 14);
+    sprintf(ungrab_key_string, "%s", "ctrl-alt-g");
+    key_value_array[0] = Q_KEY_CODE_CTRL;
+    key_value_array[1] = Q_KEY_CODE_ALT;
+    key_value_array[2] = Q_KEY_CODE_G;
+}
+
+/*
+ * Initializes the ungrab key settings - should be called by the front-end on
+ * startup.
+ */
+void init_ungrab_keys(void)
+{
+    if (console_ungrab_key_string() == NULL) {
+        use_default_ungrab_keys();
+    }
+}
+
 type_init(register_types);
diff --git a/vl.c b/vl.c
index 444b7507da..bfe61ad2fa 100644
--- a/vl.c
+++ b/vl.c
@@ -4073,6 +4073,9 @@  int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
+            case QEMU_OPTION_ungrab:
+                set_ungrab_seq(optarg);
+                break;
             default:
                 os_parse_cmd_args(popt->index, optarg);
             }