[PATCH 0/2] cocoa: keyboard quality of life

[PATCH 0/2] cocoa: keyboard quality of life

[gustavo]

From: Gustavo Noronha Silva <gustavo@noronha.eti.br>

This series adds two new options to the cocoa display:

 - full-grab causes it to use a global tap to steal system combos
   away from Mac OS X, so they can be handled by the VM

 - swap-option-command does what it says on the tin; while that is
   something you can do at the Mac OS X level or even supported by
   some keyboards, it is much more convenient to have qemu put
   Meta/Super and Alt where they belong if you are running a
   non-Mac VM

I propose to enable swap-option-command by default and leave full-grab
off because unfortunately it needs accessibility permissions for input
grabbing, so it requires more deliberate action by the user anyway.

Gustavo Noronha Silva (2):
  ui/cocoa: capture all keys and combos when mouse is grabbed
  ui/cocoa: add option to swap Option and Command, enable by default

 qapi/ui.json    |  20 +++++++
 qemu-options.hx |   4 ++
 ui/cocoa.m      | 139 +++++++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 151 insertions(+), 12 deletions(-)

-- 
2.24.3 (Apple Git-128)

[PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[gustavo]

From: Gustavo Noronha Silva <gustavo@noronha.eti.br>

Applications such as Gnome may use Alt-Tab and Super-Tab for different
purposes, some use Ctrl-arrows so we want to allow qemu to handle
everything when it captures the mouse/keyboard.

However, Mac OS handles some combos like Command-Tab and Ctrl-arrows
at an earlier part of the event handling chain, not letting qemu see it.

We add a global Event Tap that allows qemu to see all events when the
mouse is grabbed. Note that this requires additional permissions.

See:

https://developer.apple.com/documentation/coregraphics/1454426-cgeventtapcreate?language=objc#discussion
https://support.apple.com/en-in/guide/mac-help/mh32356/mac

Signed-off-by: Gustavo Noronha Silva <gustavo@noronha.eti.br>
---
 qapi/ui.json    | 15 ++++++++++
 qemu-options.hx |  3 ++
 ui/cocoa.m      | 73 +++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 1052ca9c38..77bc00fd0d 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1088,6 +1088,20 @@
 { 'struct'  : 'DisplayCurses',
   'data'    : { '*charset'       : 'str' } }
 
+##
+# @DisplayCocoa:
+#
+# Cocoa display options.
+#
+# @full-grab:       Capture all key presses, including system combos. This
+#                   requires accessibility permissions, since it performs
+#                   a global grab on key events. (default: off)
+#                   See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
+#
+##
+{ 'struct'  : 'DisplayCocoa',
+  'data'    : { '*full-grab'     : 'bool' } }
+
 ##
 # @DisplayType:
 #
@@ -1153,6 +1167,7 @@
                 '*gl'            : 'DisplayGLMode' },
   'discriminator' : 'type',
   'data'    : { 'gtk'            : 'DisplayGTK',
+                'cocoa'          : 'DisplayCocoa',
                 'curses'         : 'DisplayCurses',
                 'egl-headless'   : 'DisplayEGLHeadless'} }
 
diff --git a/qemu-options.hx b/qemu-options.hx
index fd21002bd6..a77505241f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1783,6 +1783,9 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 #if defined(CONFIG_CURSES)
     "-display curses[,charset=<encoding>]\n"
 #endif
+#if defined(CONFIG_COCOA)
+    "-display cocoa[,full_grab=on|off]\n"
+#endif
 #if defined(CONFIG_OPENGL)
     "-display egl-headless[,rendernode=<file>]\n"
 #endif
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 37e1fb52eb..f1e4449082 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -72,6 +72,7 @@
 typedef struct {
     int width;
     int height;
+    bool full_grab;
 } QEMUScreen;
 
 static void cocoa_update(DisplayChangeListener *dcl,
@@ -304,11 +305,13 @@ @interface QemuCocoaView : NSView
     BOOL isMouseGrabbed;
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
+    CFMachPortRef eventsTap;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
 - (void) ungrabMouse;
 - (void) toggleFullScreen:(id)sender;
+- (void) setFullGrab:(id)sender to:(BOOL)value;
 - (void) handleMonitorInput:(NSEvent *)event;
 - (bool) handleEvent:(NSEvent *)event;
 - (bool) handleEventLocked:(NSEvent *)event;
@@ -323,6 +326,7 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
  */
 - (BOOL) isMouseGrabbed;
 - (BOOL) isAbsoluteEnabled;
+- (BOOL) isFullGrabEnabled;
 - (float) cdx;
 - (float) cdy;
 - (QEMUScreen) gscreen;
@@ -331,6 +335,19 @@ - (void) raiseAllKeys;
 
 QemuCocoaView *cocoaView;
 
+static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEventRef cgEvent, void *userInfo)
+{
+    QemuCocoaView *cocoaView = (QemuCocoaView*) userInfo;
+    NSEvent* event = [NSEvent eventWithCGEvent:cgEvent];
+    if ([cocoaView isFullGrabEnabled] && [cocoaView isMouseGrabbed] && [cocoaView handleEvent:event]) {
+        COCOA_DEBUG("Global events tap: qemu handled the event, capturing!\n");
+        return NULL;
+    }
+    COCOA_DEBUG("Global events tap: qemu did not handle the event, letting it through...\n");
+
+    return cgEvent;
+}
+
 @implementation QemuCocoaView
 - (id)initWithFrame:(NSRect)frameRect
 {
@@ -344,6 +361,32 @@ - (id)initWithFrame:(NSRect)frameRect
         kbd = qkbd_state_init(dcl.con);
 
     }
+
+    CGEventMask mask = CGEventMaskBit(kCGEventKeyDown) | CGEventMaskBit(kCGEventKeyUp) | CGEventMaskBit(kCGEventFlagsChanged);
+    eventsTap = CGEventTapCreate(kCGHIDEventTap, kCGHeadInsertEventTap, kCGEventTapOptionDefault,
+                                 mask, handleTapEvent, self);
+    if (!eventsTap) {
+        warn_report("Could not create event tap, system key combos will not be captured.\n");
+        return self;
+    } else {
+        COCOA_DEBUG("Global events tap created! Will capture system key combos.\n");
+    }
+
+    CFRunLoopRef runLoop = CFRunLoopGetCurrent();
+    if (!runLoop) {
+        warn_report("Could not obtain current CF RunLoop, system key combos will not be captured.\n");
+        return self;
+    }
+
+    CFRunLoopSourceRef tapEventsSrc = CFMachPortCreateRunLoopSource(kCFAllocatorDefault, eventsTap, 0);
+    if (!tapEventsSrc ) {
+        warn_report("Could not obtain current CF RunLoop, system key combos will not be captured.\n");
+        return self;
+    }
+
+    CFRunLoopAddSource(runLoop, tapEventsSrc, kCFRunLoopDefaultMode);
+    CFRelease(tapEventsSrc);
+
     return self;
 }
 
@@ -356,6 +399,11 @@ - (void) dealloc
     }
 
     qkbd_state_free(kbd);
+
+    if (eventsTap) {
+        CFRelease(eventsTap);
+    }
+
     [super dealloc];
 }
 
@@ -593,6 +641,13 @@ - (void) toggleFullScreen:(id)sender
     }
 }
 
+- (void) setFullGrab:(id)sender to:(BOOL)value
+{
+    COCOA_DEBUG("QemuCocoaView: setFullGrab\n");
+
+    screen.full_grab = value;
+}
+
 - (void) toggleKey: (int)keycode {
     qkbd_state_key_event(kbd, keycode, !qkbd_state_key_get(kbd, keycode));
 }
@@ -1029,6 +1084,7 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
 }
 - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
 - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
+- (BOOL) isFullGrabEnabled {return screen.full_grab;}
 - (float) cdx {return cdx;}
 - (float) cdy {return cdy;}
 - (QEMUScreen) gscreen {return screen;}
@@ -1208,6 +1264,13 @@ - (void)toggleFullScreen:(id)sender
     [cocoaView toggleFullScreen:sender];
 }
 
+- (void) setFullGrab:(id)sender to:(BOOL)value
+{
+    COCOA_DEBUG("QemuCocoaAppController: setFullGrab\n");
+
+    [cocoaView setFullGrab:sender to:value];
+}
+
 /* Tries to find then open the specified filename */
 - (void) openDocumentation: (NSString *) filename
 {
@@ -1877,16 +1940,22 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
     qemu_sem_wait(&app_started_sem);
     COCOA_DEBUG("cocoa_display_init: app start completed\n");
 
+    QemuCocoaAppController* controller = (QemuCocoaAppController *)[NSApplication sharedApplication];
     /* if fullscreen mode is to be used */
     if (opts->has_full_screen && opts->full_screen) {
         dispatch_async(dispatch_get_main_queue(), ^{
             [NSApp activateIgnoringOtherApps: YES];
-            [(QemuCocoaAppController *)[[NSApplication sharedApplication] delegate] toggleFullScreen: nil];
+            [[controller delegate] toggleFullScreen: nil];
+        });
+    }
+    if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
+        dispatch_async(dispatch_get_main_queue(), ^{
+            [[controller delegate] setFullGrab: nil to:opts->u.cocoa.full_grab];
         });
     }
     if (opts->has_show_cursor && opts->show_cursor) {
         cursor_hide = 0;
-    }
+    };
 
     // register vga output callbacks
     register_displaychangelistener(&dcl);
-- 
2.24.3 (Apple Git-128)

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[Markus Armbruster]

gustavo@noronha.eti.br writes:

> From: Gustavo Noronha Silva <gustavo@noronha.eti.br>
>
> Applications such as Gnome may use Alt-Tab and Super-Tab for different
> purposes, some use Ctrl-arrows so we want to allow qemu to handle
> everything when it captures the mouse/keyboard.
>
> However, Mac OS handles some combos like Command-Tab and Ctrl-arrows
> at an earlier part of the event handling chain, not letting qemu see it.
>
> We add a global Event Tap that allows qemu to see all events when the
> mouse is grabbed. Note that this requires additional permissions.
>
> See:
>
> https://developer.apple.com/documentation/coregraphics/1454426-cgeventtapcreate?language=objc#discussion
> https://support.apple.com/en-in/guide/mac-help/mh32356/mac
>
> Signed-off-by: Gustavo Noronha Silva <gustavo@noronha.eti.br>
> ---
>  qapi/ui.json    | 15 ++++++++++
>  qemu-options.hx |  3 ++
>  ui/cocoa.m      | 73 +++++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 89 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 1052ca9c38..77bc00fd0d 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1088,6 +1088,20 @@
>  { 'struct'  : 'DisplayCurses',
>    'data'    : { '*charset'       : 'str' } }
>  
> +##
> +# @DisplayCocoa:
> +#
> +# Cocoa display options.
> +#
> +# @full-grab:       Capture all key presses, including system combos. This
> +#                   requires accessibility permissions, since it performs
> +#                   a global grab on key events. (default: off)
> +#                   See https://support.apple.com/en-in/guide/mac-help/mh32356/mac

Please indent like this

   # @full-grab: Capture all key presses, including system combos. This
   #             requires accessibility permissions, since it performs
   #             a global grab on key events. (default: off)
   #             See https://support.apple.com/en-in/guide/mac-help/mh32356/mac

I hope the link is permanent.

> +#
> +##
> +{ 'struct'  : 'DisplayCocoa',
> +  'data'    : { '*full-grab'     : 'bool' } }
> +
>  ##
>  # @DisplayType:
>  #
> @@ -1153,6 +1167,7 @@
>                  '*gl'            : 'DisplayGLMode' },
>    'discriminator' : 'type',
>    'data'    : { 'gtk'            : 'DisplayGTK',
> +                'cocoa'          : 'DisplayCocoa',
>                  'curses'         : 'DisplayCurses',
>                  'egl-headless'   : 'DisplayEGLHeadless'} }
>  

With indentation tidied up, QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[Gustavo Noronha Silva]

Hey Markus,

On Fri, Apr 30, 2021, at 4:20 AM, Markus Armbruster wrote:
> Please indent like this
> 
>    # @full-grab: Capture all key presses, including system combos. This
>    #             requires accessibility permissions, since it performs
>    #             a global grab on key events. (default: off)
>    #             See https://support.apple.com/en-in/guide/mac-help/mh32356/mac

Will do, but just to be sure, the surrounding comments have the second to last lines lined up to the C in Capture there, I assume that is what you mean?
 
> I hope the link is permanent.

So do I. I have found Apple to be fairly consistent in providing at least proper redirection to newer versions of the documentation while learning about Cocoa and the Mac itself (first timer, just interested in the M1 to be quite honest), so I'm fairly confident it will survive.

Thanks for the review! I'll send a v2 later today.

Cheers,

Gustavo

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[Gustavo Noronha Silva]

Oh by the way,

On Fri, Apr 30, 2021, at 7:02 AM, Gustavo Noronha Silva wrote:
> >    # @full-grab: Capture all key presses, including system combos. This
> >    #             requires accessibility permissions, since it performs
> >    #             a global grab on key events. (default: off)
> >    #             See https://support.apple.com/en-in/guide/mac-help/mh32356/mac

I did not add a Since: here because I wasn't sure how that is handled. Should I add something or is that taken care of at the time of release somehow?

Thanks again,

Gustavo

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[Markus Armbruster]

"Gustavo Noronha Silva" <gustavo@noronha.eti.br> writes:

> Oh by the way,
>
> On Fri, Apr 30, 2021, at 7:02 AM, Gustavo Noronha Silva wrote:
>> >    # @full-grab: Capture all key presses, including system combos. This
>> >    #             requires accessibility permissions, since it performs
>> >    #             a global grab on key events. (default: off)
>> >    #             See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
>
> I did not add a Since: here because I wasn't sure how that is handled. Should I add something or is that taken care of at the time of release somehow?

You should add (since 6.1) at the end, like this

# @full-grab: Capture all key presses, including system combos. This
#             requires accessibility permissions, since it performs
#             a global grab on key events. (default: off)
#             See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
#             (Since 6.1)

Same for @swap-option-command in the next patch.

Glad you asked, I'm quite prone to not noticing missing these in
review...

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[Gustavo Noronha Silva]

Hey,

On Fri, Apr 30, 2021, at 7:58 AM, Markus Armbruster wrote:
> > I did not add a Since: here because I wasn't sure how that is handled. Should I add something or is that taken care of at the time of release somehow?
> 
> You should add (since 6.1) at the end, like this
> 
> # @full-grab: Capture all key presses, including system combos. This
> #             requires accessibility permissions, since it performs
> #             a global grab on key events. (default: off)
> #             See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
> #             (Since 6.1)
> 
> Same for @swap-option-command in the next patch.
> 
> Glad you asked, I'm quite prone to not noticing missing these in
> review...

One last question, please bear with me =). Looking at the other options I see that some have a single Since tag for the whole thing, I assume because they were all added in one go. For instance, @DisplayGLMode has a single Since: 3.0 at the bottom and not one for each of the options. Should I do that as well considering I'm adding @DisplayCocoa, or is the per-option Since still preferred?

Thanks again,

Gustavo

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[Markus Armbruster]

"Gustavo Noronha Silva" <gustavo@noronha.eti.br> writes:

> Hey,
>
> On Fri, Apr 30, 2021, at 7:58 AM, Markus Armbruster wrote:
>> > I did not add a Since: here because I wasn't sure how that is handled. Should I add something or is that taken care of at the time of release somehow?
>> 
>> You should add (since 6.1) at the end, like this
>> 
>> # @full-grab: Capture all key presses, including system combos. This
>> #             requires accessibility permissions, since it performs
>> #             a global grab on key events. (default: off)
>> #             See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
>> #             (Since 6.1)
>> 
>> Same for @swap-option-command in the next patch.
>> 
>> Glad you asked, I'm quite prone to not noticing missing these in
>> review...
>
> One last question, please bear with me =). Looking at the other options I see that some have a single Since tag for the whole thing, I assume because they were all added in one go. For instance, @DisplayGLMode has a single Since: 3.0 at the bottom and not one for each of the options. Should I do that as well considering I'm adding @DisplayCocoa, or is the per-option Since still preferred?

You're right.

The

    # Since: X.Y

lines apply to the whole definition.  Since you're adding one, that's
what you should use.

Only when you change a definition later do you add (Since X.Y) to the
appropriate doc string part.

I blame Friday for my negligence.  Thanks for paying attention!

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[Markus Armbruster]

"Gustavo Noronha Silva" <gustavo@noronha.eti.br> writes:

> Hey Markus,
>
> On Fri, Apr 30, 2021, at 4:20 AM, Markus Armbruster wrote:
>> Please indent like this
>> 
>>    # @full-grab: Capture all key presses, including system combos. This
>>    #             requires accessibility permissions, since it performs
>>    #             a global grab on key events. (default: off)
>>    #             See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
>
> Will do, but just to be sure, the surrounding comments have the second to last lines lined up to the C in Capture there, I assume that is what you mean?

Yes.

qapi-gen will complain when you indent subsequent lines too little.
Indenting them too much will be (mis-)interpreted as a definition list.

[...]

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

['Gerd Hoffmann ']

On Thu, Apr 29, 2021 at 08:47:04PM -0300, gustavo@noronha.eti.br wrote:
> From: Gustavo Noronha Silva <gustavo@noronha.eti.br>
> 
> Applications such as Gnome may use Alt-Tab and Super-Tab for different
> purposes, some use Ctrl-arrows so we want to allow qemu to handle
> everything when it captures the mouse/keyboard.
> 
> However, Mac OS handles some combos like Command-Tab and Ctrl-arrows
> at an earlier part of the event handling chain, not letting qemu see it.
> 
> We add a global Event Tap that allows qemu to see all events when the
> mouse is grabbed. Note that this requires additional permissions.
> 
> See:
> 
> https://developer.apple.com/documentation/coregraphics/1454426-cgeventtapcreate?language=objc#discussion
> https://support.apple.com/en-in/guide/mac-help/mh32356/mac

Looks all sensible to me, I'd like to have the opinion from the MacOS
experts for this one though.

thanks,
  Gerd

Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed

[Gustavo Noronha Silva]

Hey Gerd,

On Fri, Apr 30, 2021, at 5:05 AM, 'Gerd Hoffmann ' wrote:
> Looks all sensible to me, I'd like to have the opinion from the MacOS
> experts for this one though.

Thanks for the reviews, I'll send a v2 later today! Are there any other people I should CC for the MacOS expertise? It's the firs time I contribute to qemu (the first time I use the send-email process even), so if you have any suggestions on that front I'd appreciate as well =)

Cheers,

Gustavo

[PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default

[gustavo]

From: Gustavo Noronha Silva <gustavo@noronha.eti.br>

On Mac OS X the Option key maps to Alt and Command to Super/Meta. This change
swaps them around so that Alt is the key closer to the space bar and Meta/Super
is between Control and Alt, like on non-Mac keyboards.

It is a cocoa display option, enabled by default.

Signed-off-by: Gustavo Noronha Silva <gustavo@noronha.eti.br>
---
 qapi/ui.json    |  7 +++++-
 qemu-options.hx |  1 +
 ui/cocoa.m      | 66 +++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 77bc00fd0d..02db684251 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1098,9 +1098,14 @@
 #                   a global grab on key events. (default: off)
 #                   See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
 #
+# @swap-option-command: Swaps the Option and Command keys so that their key codes
+#                       match their position on non-Mac keyboards and you can use
+#                       Meta/Super and Alt where you expect them. (default: on)
+#
 ##
 { 'struct'  : 'DisplayCocoa',
-  'data'    : { '*full-grab'     : 'bool' } }
+  'data'    : { '*full-grab'           : 'bool',
+                '*swap-option-command' : 'bool' } }
 
 ##
 # @DisplayType:
diff --git a/qemu-options.hx b/qemu-options.hx
index a77505241f..d6137eedac 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1785,6 +1785,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 #endif
 #if defined(CONFIG_COCOA)
     "-display cocoa[,full_grab=on|off]\n"
+    "              [,swap_option_command=on|off]\n"
 #endif
 #if defined(CONFIG_OPENGL)
     "-display egl-headless[,rendernode=<file>]\n"
diff --git a/ui/cocoa.m b/ui/cocoa.m
index f1e4449082..879e568a9d 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -73,6 +73,7 @@
     int width;
     int height;
     bool full_grab;
+    bool swap_option_command;
 } QEMUScreen;
 
 static void cocoa_update(DisplayChangeListener *dcl,
@@ -327,6 +328,7 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
 - (BOOL) isMouseGrabbed;
 - (BOOL) isAbsoluteEnabled;
 - (BOOL) isFullGrabEnabled;
+- (BOOL) isSwapOptionCommandEnabled;
 - (float) cdx;
 - (float) cdy;
 - (QEMUScreen) gscreen;
@@ -648,6 +650,13 @@ - (void) setFullGrab:(id)sender to:(BOOL)value
     screen.full_grab = value;
 }
 
+- (void) setSwapOptionCommand:(id)sender
+{
+    COCOA_DEBUG("QemuCocoaView: setSwapOptionCommand\n");
+
+    screen.swap_option_command = true;
+}
+
 - (void) toggleKey: (int)keycode {
     qkbd_state_key_event(kbd, keycode, !qkbd_state_key_get(kbd, keycode));
 }
@@ -797,12 +806,22 @@ - (bool) handleEventLocked:(NSEvent *)event
         qkbd_state_key_event(kbd, Q_KEY_CODE_CTRL_R, false);
     }
     if (!(modifiers & NSEventModifierFlagOption)) {
-        qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
-        qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
+        if ([self isSwapOptionCommandEnabled]) {
+            qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
+            qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
+        } else {
+            qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
+            qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
+        }
     }
     if (!(modifiers & NSEventModifierFlagCommand)) {
-        qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
-        qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
+        if ([self isSwapOptionCommandEnabled]) {
+            qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
+            qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
+        } else {
+            qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
+            qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
+        }
     }
 
     switch ([event type]) {
@@ -834,13 +853,21 @@ - (bool) handleEventLocked:(NSEvent *)event
 
                 case kVK_Option:
                     if (!!(modifiers & NSEventModifierFlagOption)) {
-                        [self toggleKey:Q_KEY_CODE_ALT];
+                        if ([self isSwapOptionCommandEnabled]) {
+                            [self toggleKey:Q_KEY_CODE_META_L];
+                        } else {
+                            [self toggleKey:Q_KEY_CODE_ALT];
+                        }
                     }
                     break;
 
                 case kVK_RightOption:
                     if (!!(modifiers & NSEventModifierFlagOption)) {
-                        [self toggleKey:Q_KEY_CODE_ALT_R];
+                        if ([self isSwapOptionCommandEnabled]) {
+                            [self toggleKey:Q_KEY_CODE_META_R];
+                        } else {
+                            [self toggleKey:Q_KEY_CODE_ALT_R];
+                        }
                     }
                     break;
 
@@ -848,15 +875,21 @@ - (bool) handleEventLocked:(NSEvent *)event
                 case kVK_Command:
                     if (isMouseGrabbed &&
                         !!(modifiers & NSEventModifierFlagCommand)) {
-                        [self toggleKey:Q_KEY_CODE_META_L];
-                    }
+                        if ([self isSwapOptionCommandEnabled]) {
+                            [self toggleKey:Q_KEY_CODE_ALT];
+                        } else {
+                            [self toggleKey:Q_KEY_CODE_META_L];
+                        }                    }
                     break;
 
                 case kVK_RightCommand:
                     if (isMouseGrabbed &&
                         !!(modifiers & NSEventModifierFlagCommand)) {
-                        [self toggleKey:Q_KEY_CODE_META_R];
-                    }
+                        if ([self isSwapOptionCommandEnabled]) {
+                            [self toggleKey:Q_KEY_CODE_ALT_R];
+                        } else {
+                            [self toggleKey:Q_KEY_CODE_META_R];
+                        }                    }
                     break;
             }
             break;
@@ -1085,6 +1118,7 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
 - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
 - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
 - (BOOL) isFullGrabEnabled {return screen.full_grab;}
+- (BOOL) isSwapOptionCommandEnabled {return screen.swap_option_command;}
 - (float) cdx {return cdx;}
 - (float) cdy {return cdy;}
 - (QEMUScreen) gscreen {return screen;}
@@ -1271,6 +1305,13 @@ - (void) setFullGrab:(id)sender to:(BOOL)value
     [cocoaView setFullGrab:sender to:value];
 }
 
+- (void) setSwapOptionCommand:(id)sender
+{
+    COCOA_DEBUG("QemuCocoaAppController: setSwapOptionCommand\n");
+
+    [cocoaView setSwapOptionCommand:sender];
+}
+
 /* Tries to find then open the specified filename */
 - (void) openDocumentation: (NSString *) filename
 {
@@ -1953,6 +1994,11 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
             [[controller delegate] setFullGrab: nil to:opts->u.cocoa.full_grab];
         });
     }
+    if (!opts->u.cocoa.has_swap_option_command || opts->u.cocoa.swap_option_command) {
+        dispatch_async(dispatch_get_main_queue(), ^{
+            [[controller delegate] setSwapOptionCommand: nil];
+        });
+    }
     if (opts->has_show_cursor && opts->show_cursor) {
         cursor_hide = 0;
     };
-- 
2.24.3 (Apple Git-128)

Re: [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default

[Markus Armbruster]

gustavo@noronha.eti.br writes:

> From: Gustavo Noronha Silva <gustavo@noronha.eti.br>
>
> On Mac OS X the Option key maps to Alt and Command to Super/Meta. This change
> swaps them around so that Alt is the key closer to the space bar and Meta/Super
> is between Control and Alt, like on non-Mac keyboards.
>
> It is a cocoa display option, enabled by default.
>
> Signed-off-by: Gustavo Noronha Silva <gustavo@noronha.eti.br>
> ---
>  qapi/ui.json    |  7 +++++-
>  qemu-options.hx |  1 +
>  ui/cocoa.m      | 66 +++++++++++++++++++++++++++++++++++++++++--------
>  3 files changed, 63 insertions(+), 11 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 77bc00fd0d..02db684251 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1098,9 +1098,14 @@
   ##
   # @DisplayCocoa:
   #
   # Cocoa display options.
   #
   # @full-grab:       Capture all key presses, including system combos. This
   #                   requires accessibility permissions, since it performs
>  #                   a global grab on key events. (default: off)
>  #                   See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
>  #
> +# @swap-option-command: Swaps the Option and Command keys so that their key codes

Please use imperative mood consistently: "Swap", like "Capture" above.

> +#                       match their position on non-Mac keyboards and you can use
> +#                       Meta/Super and Alt where you expect them. (default: on)
> +#

Drop the blank comment line, and break your lines a bit earlier.

>  ##

Like this:

   ##
   # @DisplayCocoa:
   #
   # Cocoa display options.
   #
   # @full-grab:       Capture all key presses, including system combos. This
   #                   requires accessibility permissions, since it performs
   #                   a global grab on key events. (default: off)
   #                   See https://support.apple.com/en-in/guide/mac-help/mh32356/mac
   #
   # @swap-option-command: Swap the Option and Command keys so that their key
   #                       codes match their position on non-Mac keyboards and
   #                       you can use Meta/Super and Alt where you expect
   #                       them. (default: on)
   ##


>  { 'struct'  : 'DisplayCocoa',
> -  'data'    : { '*full-grab'     : 'bool' } }
> +  'data'    : { '*full-grab'           : 'bool',
> +                '*swap-option-command' : 'bool' } }
>  
>  ##
>  # @DisplayType:

With the doc comment tidied up, QAPI schema
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default

['Gerd Hoffmann ']

> @@ -797,12 +806,22 @@ - (bool) handleEventLocked:(NSEvent *)event
>          qkbd_state_key_event(kbd, Q_KEY_CODE_CTRL_R, false);
>      }
>      if (!(modifiers & NSEventModifierFlagOption)) {
> -        qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
> -        qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
> +        if ([self isSwapOptionCommandEnabled]) {
> +            qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
> +            qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
> +        } else {
> +            qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
> +            qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
> +        }
>      }
>      if (!(modifiers & NSEventModifierFlagCommand)) {
> -        qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
> -        qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
> +        if ([self isSwapOptionCommandEnabled]) {
> +            qkbd_state_key_event(kbd, Q_KEY_CODE_ALT, false);
> +            qkbd_state_key_event(kbd, Q_KEY_CODE_ALT_R, false);
> +        } else {
> +            qkbd_state_key_event(kbd, Q_KEY_CODE_META_L, false);
> +            qkbd_state_key_event(kbd, Q_KEY_CODE_META_R, false);
> +        }
>      }

Wouldn't it be easier to swap the bits in the modifiers variable once
instead of having lots of isSwapOptionCommandEnabled checks in the code?

take care,
  Gerd

Re: [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default

[Gustavo Noronha Silva]

Hey again,

On Fri, Apr 30, 2021, at 5:04 AM, 'Gerd Hoffmann ' wrote:
> Wouldn't it be easier to swap the bits in the modifiers variable once
> instead of having lots of isSwapOptionCommandEnabled checks in the code?

Good point, since a local variable is used it's definitely easier. I'll do the change and test, will be in v2.

Cheers,

Gustavo