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

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

[gustavo]

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

v4 moves the code to add the tap to the set function, which
not only sidesteps the issue pointed out by Akihiko Odaki
but simply makes sense - full-grab is not a runtime toggle
and there is no reason to have the tap set up if it's not
turned on.

It also fixes the commit message for the swap-option-command
option to no longer lie about it being turned on by default,
as pointed out by Gerd.

-----

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

Both are off by default. For full-grab in particular, it is off also
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

 qapi/ui.json    |  22 +++++++++
 qemu-options.hx |   4 ++
 ui/cocoa.m      | 129 ++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 146 insertions(+), 9 deletions(-)

-- 
2.30.1 (Apple Git-130)

[PATCH v4 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

Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gustavo Noronha Silva <gustavo@noronha.eti.br>
---
 qapi/ui.json    | 16 ++++++++++++
 qemu-options.hx |  3 +++
 ui/cocoa.m      | 65 ++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 1052ca9c38..4ccfae4bbb 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1088,6 +1088,21 @@
 { '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
+#
+# Since: 6.1
+##
+{ 'struct'  : 'DisplayCocoa',
+  'data'    : { '*full-grab'     : 'bool' } }
+
 ##
 # @DisplayType:
 #
@@ -1153,6 +1168,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 7c825f81fc..eeaa5c73e9 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..236352deac 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -304,11 +304,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;
 - (void) handleMonitorInput:(NSEvent *)event;
 - (bool) handleEvent:(NSEvent *)event;
 - (bool) handleEventLocked:(NSEvent *)event;
@@ -331,6 +333,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 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
 {
@@ -356,6 +371,11 @@ - (void) dealloc
     }
 
     qkbd_state_free(kbd);
+
+    if (eventsTap) {
+        CFRelease(eventsTap);
+    }
+
     [super dealloc];
 }
 
@@ -593,6 +613,36 @@ - (void) toggleFullScreen:(id)sender
     }
 }
 
+- (void) setFullGrab:(id)sender
+{
+    COCOA_DEBUG("QemuCocoaView: setFullGrab\n");
+
+    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;
+    } 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;
+    }
+
+    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;
+    }
+
+    CFRunLoopAddSource(runLoop, tapEventsSrc, kCFRunLoopDefaultMode);
+    CFRelease(tapEventsSrc);
+}
+
 - (void) toggleKey: (int)keycode {
     qkbd_state_key_event(kbd, keycode, !qkbd_state_key_get(kbd, keycode));
 }
@@ -1208,6 +1258,13 @@ - (void)toggleFullScreen:(id)sender
     [cocoaView toggleFullScreen:sender];
 }
 
+- (void) setFullGrab:(id)sender
+{
+    COCOA_DEBUG("QemuCocoaAppController: setFullGrab\n");
+
+    [cocoaView setFullGrab:sender];
+}
+
 /* Tries to find then open the specified filename */
 - (void) openDocumentation: (NSString *) filename
 {
@@ -1877,11 +1934,17 @@ 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];
         });
     }
     if (opts->has_show_cursor && opts->show_cursor) {
-- 
2.30.1 (Apple Git-130)

[PATCH v4 2/2] ui/cocoa: add option to swap Option and Command

[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, disabled by default.

Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gustavo Noronha Silva <gustavo@noronha.eti.br>
---
 qapi/ui.json    |  8 ++++++-
 qemu-options.hx |  3 ++-
 ui/cocoa.m      | 64 ++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 4ccfae4bbb..ee6fde46d5 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1098,10 +1098,16 @@
 #             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: off)
+#
 # Since: 6.1
 ##
 { '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 eeaa5c73e9..e0e93724b1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1784,7 +1784,8 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
     "-display curses[,charset=<encoding>]\n"
 #endif
 #if defined(CONFIG_COCOA)
-    "-display cocoa[,full_grab=on|off]\n"
+    "-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 236352deac..995301502b 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -72,6 +72,7 @@
 typedef struct {
     int width;
     int height;
+    bool swap_option_command;
 } QEMUScreen;
 
 static void cocoa_update(DisplayChangeListener *dcl,
@@ -325,6 +326,7 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
  */
 - (BOOL) isMouseGrabbed;
 - (BOOL) isAbsoluteEnabled;
+- (BOOL) isSwapOptionCommandEnabled;
 - (float) cdx;
 - (float) cdy;
 - (QEMUScreen) gscreen;
@@ -643,6 +645,13 @@ - (void) setFullGrab:(id)sender
     CFRelease(tapEventsSrc);
 }
 
+- (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));
 }
@@ -792,12 +801,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]) {
@@ -829,13 +848,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;
 
@@ -843,14 +870,22 @@ - (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;
             }
@@ -1079,6 +1114,7 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
 }
 - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
 - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
+- (BOOL) isSwapOptionCommandEnabled {return screen.swap_option_command;}
 - (float) cdx {return cdx;}
 - (float) cdy {return cdy;}
 - (QEMUScreen) gscreen {return screen;}
@@ -1265,6 +1301,13 @@ - (void) setFullGrab:(id)sender
     [cocoaView setFullGrab:sender];
 }
 
+- (void) setSwapOptionCommand:(id)sender
+{
+    COCOA_DEBUG("QemuCocoaAppController: setSwapOptionCommand\n");
+
+    [cocoaView setSwapOptionCommand:sender];
+}
+
 /* Tries to find then open the specified filename */
 - (void) openDocumentation: (NSString *) filename
 {
@@ -1947,6 +1990,11 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
             [[controller delegate] setFullGrab: nil];
         });
     }
+    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.30.1 (Apple Git-130)

Re: [PATCH v4 2/2] ui/cocoa: add option to swap Option and Command

[Akihiko Odaki]

Reviewed-by: Akihiko Odaki <akihiko.odaki@gmail.com>

I applied them to my personal tree:
https://github.com/akihikodaki/qemu/tree/macos

2021年5月12日(水) 3:56 <gustavo@noronha.eti.br>:
>
> 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, disabled by default.
>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Gustavo Noronha Silva <gustavo@noronha.eti.br>
> ---
>  qapi/ui.json    |  8 ++++++-
>  qemu-options.hx |  3 ++-
>  ui/cocoa.m      | 64 ++++++++++++++++++++++++++++++++++++++++++-------
>  3 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 4ccfae4bbb..ee6fde46d5 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1098,10 +1098,16 @@
>  #             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: off)
> +#
>  # Since: 6.1
>  ##
>  { '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 eeaa5c73e9..e0e93724b1 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1784,7 +1784,8 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
>      "-display curses[,charset=<encoding>]\n"
>  #endif
>  #if defined(CONFIG_COCOA)
> -    "-display cocoa[,full_grab=on|off]\n"
> +    "-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 236352deac..995301502b 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -72,6 +72,7 @@
>  typedef struct {
>      int width;
>      int height;
> +    bool swap_option_command;
>  } QEMUScreen;
>
>  static void cocoa_update(DisplayChangeListener *dcl,
> @@ -325,6 +326,7 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled;
>   */
>  - (BOOL) isMouseGrabbed;
>  - (BOOL) isAbsoluteEnabled;
> +- (BOOL) isSwapOptionCommandEnabled;
>  - (float) cdx;
>  - (float) cdy;
>  - (QEMUScreen) gscreen;
> @@ -643,6 +645,13 @@ - (void) setFullGrab:(id)sender
>      CFRelease(tapEventsSrc);
>  }
>
> +- (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));
>  }
> @@ -792,12 +801,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]) {
> @@ -829,13 +848,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;
>
> @@ -843,14 +870,22 @@ - (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;
>              }
> @@ -1079,6 +1114,7 @@ - (void) setAbsoluteEnabled:(BOOL)tIsAbsoluteEnabled {
>  }
>  - (BOOL) isMouseGrabbed {return isMouseGrabbed;}
>  - (BOOL) isAbsoluteEnabled {return isAbsoluteEnabled;}
> +- (BOOL) isSwapOptionCommandEnabled {return screen.swap_option_command;}
>  - (float) cdx {return cdx;}
>  - (float) cdy {return cdy;}
>  - (QEMUScreen) gscreen {return screen;}
> @@ -1265,6 +1301,13 @@ - (void) setFullGrab:(id)sender
>      [cocoaView setFullGrab:sender];
>  }
>
> +- (void) setSwapOptionCommand:(id)sender
> +{
> +    COCOA_DEBUG("QemuCocoaAppController: setSwapOptionCommand\n");
> +
> +    [cocoaView setSwapOptionCommand:sender];
> +}
> +
>  /* Tries to find then open the specified filename */
>  - (void) openDocumentation: (NSString *) filename
>  {
> @@ -1947,6 +1990,11 @@ static void cocoa_display_init(DisplayState *ds, DisplayOptions *opts)
>              [[controller delegate] setFullGrab: nil];
>          });
>      }
> +    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.30.1 (Apple Git-130)
>

Re: [PATCH v4 2/2] ui/cocoa: add option to swap Option and Command

[Gustavo Noronha Silva]

Hey,

On Wed, May 12, 2021, at 12:06 AM, Akihiko Odaki wrote:
> Reviewed-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> 
> I applied them to my personal tree:
> https://github.com/akihikodaki/qemu/tree/macos

Thank you for the review! That top one I am still to submit here, by the way. Will do once this series is through.

Cheers,

Gustavo

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

['Gerd Hoffmann ']

On Tue, May 11, 2021 at 03:55:37PM -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

Ping.  Reviews from cocoa guys please?

thanks,
  Gerd

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

[Gustavo Noronha Silva]

On Tue, May 18, 2021, at 7:14 AM, 'Gerd Hoffmann ' wrote:
> On Tue, May 11, 2021 at 03:55:37PM -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
> 
> Ping.  Reviews from cocoa guys please?

Ping 2? =)

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

[Gustavo Noronha Silva]

Ping 3? =)

On Wed, May 26, 2021, at 12:13 PM, Gustavo Noronha Silva wrote:
> On Tue, May 18, 2021, at 7:14 AM, 'Gerd Hoffmann ' wrote:
> > On Tue, May 11, 2021 at 03:55:37PM -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
> > 
> > Ping.  Reviews from cocoa guys please?
> 
> Ping 2? =)

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

[Akihiko Odaki]

2021年5月12日(水) 3:56 <gustavo@noronha.eti.br>:
> @@ -1877,11 +1934,17 @@ 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];
>          });
>      }
>      if (opts->has_show_cursor && opts->show_cursor) {
> --
> 2.30.1 (Apple Git-130)
>

Hi,

I found a problem which I didn't notice when I reviewed this patch
before. [NSApplication sharedApplication] is casted to
QemuCocoaAppController *, but its type is QemuApplication. Perhaps it
should be:
QemuCocoaAppController* controller = (QemuCocoaAppController
*)[[NSApplication sharedApplication] delegate];
and the following [controller delegate] should be replaced with controller.

Regards,
Akihiko Odaki