qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] cocoa: keyboard quality of life
@ 2021-04-29 23:47 gustavo
  2021-04-29 23:47 ` [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed gustavo
  2021-04-29 23:47 ` [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default gustavo
  0 siblings, 2 replies; 17+ messages in thread
From: gustavo @ 2021-04-29 23:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: 'Peter Maydell ', 'Markus Armbruster ',
	Gustavo Noronha Silva, 'Gerd Hoffmann '

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)



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-29 23:47 [PATCH 0/2] cocoa: keyboard quality of life gustavo
@ 2021-04-29 23:47 ` gustavo
  2021-04-30  7:20   ` Markus Armbruster
  2021-04-30  8:05   ` 'Gerd Hoffmann '
  2021-04-29 23:47 ` [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default gustavo
  1 sibling, 2 replies; 17+ messages in thread
From: gustavo @ 2021-04-29 23:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: 'Peter Maydell ', 'Markus Armbruster ',
	Gustavo Noronha Silva, 'Gerd Hoffmann '

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)



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default
  2021-04-29 23:47 [PATCH 0/2] cocoa: keyboard quality of life gustavo
  2021-04-29 23:47 ` [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed gustavo
@ 2021-04-29 23:47 ` gustavo
  2021-04-30  7:25   ` Markus Armbruster
  2021-04-30  8:04   ` 'Gerd Hoffmann '
  1 sibling, 2 replies; 17+ messages in thread
From: gustavo @ 2021-04-29 23:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: 'Peter Maydell ', 'Markus Armbruster ',
	Gustavo Noronha Silva, 'Gerd Hoffmann '

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)



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-29 23:47 ` [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed gustavo
@ 2021-04-30  7:20   ` Markus Armbruster
  2021-04-30 10:02     ` Gustavo Noronha Silva
  2021-04-30  8:05   ` 'Gerd Hoffmann '
  1 sibling, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2021-04-30  7:20 UTC (permalink / raw)
  To: gustavo; +Cc: 'Peter Maydell ', qemu-devel, 'Gerd Hoffmann '

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>



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default
  2021-04-29 23:47 ` [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default gustavo
@ 2021-04-30  7:25   ` Markus Armbruster
  2021-04-30  8:04   ` 'Gerd Hoffmann '
  1 sibling, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2021-04-30  7:25 UTC (permalink / raw)
  To: gustavo; +Cc: 'Peter Maydell ', qemu-devel, 'Gerd Hoffmann '

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>



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default
  2021-04-29 23:47 ` [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default gustavo
  2021-04-30  7:25   ` Markus Armbruster
@ 2021-04-30  8:04   ` 'Gerd Hoffmann '
  2021-04-30 10:36     ` Gustavo Noronha Silva
  1 sibling, 1 reply; 17+ messages in thread
From: 'Gerd Hoffmann ' @ 2021-04-30  8:04 UTC (permalink / raw)
  To: gustavo
  Cc: 'Peter Maydell ', qemu-devel, 'Markus Armbruster '

> @@ -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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-29 23:47 ` [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed gustavo
  2021-04-30  7:20   ` Markus Armbruster
@ 2021-04-30  8:05   ` 'Gerd Hoffmann '
  2021-04-30  9:43     ` Gustavo Noronha Silva
  1 sibling, 1 reply; 17+ messages in thread
From: 'Gerd Hoffmann ' @ 2021-04-30  8:05 UTC (permalink / raw)
  To: gustavo
  Cc: 'Peter Maydell ', qemu-devel, 'Markus Armbruster '

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



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-30  8:05   ` 'Gerd Hoffmann '
@ 2021-04-30  9:43     ` Gustavo Noronha Silva
  0 siblings, 0 replies; 17+ messages in thread
From: Gustavo Noronha Silva @ 2021-04-30  9:43 UTC (permalink / raw)
  To: 'Gerd Hoffmann '
  Cc: 'Peter Maydell ', qemu-devel, 'Markus Armbruster '

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-30  7:20   ` Markus Armbruster
@ 2021-04-30 10:02     ` Gustavo Noronha Silva
  2021-04-30 10:10       ` Gustavo Noronha Silva
  2021-04-30 10:54       ` Markus Armbruster
  0 siblings, 2 replies; 17+ messages in thread
From: Gustavo Noronha Silva @ 2021-04-30 10:02 UTC (permalink / raw)
  To: 'Markus Armbruster '
  Cc: 'Peter Maydell ', qemu-devel, 'Gerd Hoffmann '

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-30 10:02     ` Gustavo Noronha Silva
@ 2021-04-30 10:10       ` Gustavo Noronha Silva
  2021-04-30 10:58         ` Markus Armbruster
  2021-04-30 10:54       ` Markus Armbruster
  1 sibling, 1 reply; 17+ messages in thread
From: Gustavo Noronha Silva @ 2021-04-30 10:10 UTC (permalink / raw)
  To: 'Markus Armbruster '
  Cc: 'Peter Maydell ', qemu-devel, 'Gerd Hoffmann '

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default
  2021-04-30  8:04   ` 'Gerd Hoffmann '
@ 2021-04-30 10:36     ` Gustavo Noronha Silva
  0 siblings, 0 replies; 17+ messages in thread
From: Gustavo Noronha Silva @ 2021-04-30 10:36 UTC (permalink / raw)
  To: 'Gerd Hoffmann '
  Cc: 'Peter Maydell ', qemu-devel, 'Markus Armbruster '

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-30 10:02     ` Gustavo Noronha Silva
  2021-04-30 10:10       ` Gustavo Noronha Silva
@ 2021-04-30 10:54       ` Markus Armbruster
  1 sibling, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2021-04-30 10:54 UTC (permalink / raw)
  To: Gustavo Noronha Silva
  Cc: 'Peter Maydell ', qemu-devel, 'Gerd Hoffmann '

"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.

[...]



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-30 10:10       ` Gustavo Noronha Silva
@ 2021-04-30 10:58         ` Markus Armbruster
  2021-04-30 12:02           ` Gustavo Noronha Silva
  0 siblings, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2021-04-30 10:58 UTC (permalink / raw)
  To: Gustavo Noronha Silva
  Cc: 'Peter Maydell ', qemu-devel, 'Gerd Hoffmann '

"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...



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-30 10:58         ` Markus Armbruster
@ 2021-04-30 12:02           ` Gustavo Noronha Silva
  2021-04-30 14:36             ` Markus Armbruster
  0 siblings, 1 reply; 17+ messages in thread
From: Gustavo Noronha Silva @ 2021-04-30 12:02 UTC (permalink / raw)
  To: 'Markus Armbruster '
  Cc: 'Peter Maydell ', qemu-devel, 'Gerd Hoffmann '

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-30 12:02           ` Gustavo Noronha Silva
@ 2021-04-30 14:36             ` Markus Armbruster
  0 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2021-04-30 14:36 UTC (permalink / raw)
  To: Gustavo Noronha Silva
  Cc: 'Peter Maydell ', qemu-devel, 'Gerd Hoffmann '

"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!



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2022-03-06 11:11 ` [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed Akihiko Odaki
@ 2022-03-06 11:28   ` BALATON Zoltan
  0 siblings, 0 replies; 17+ messages in thread
From: BALATON Zoltan @ 2022-03-06 11:28 UTC (permalink / raw)
  To: Akihiko Odaki
  Cc: Peter Maydell, Markus Armbruster, qemu-devel, Gerd Hoffmann,
	Eric Blake, Gustavo Noronha Silva

On Sun, 6 Mar 2022, Akihiko Odaki wrote:
> From: Gustavo Noronha Silva <gustavo@noronha.dev.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.dev.br>
> Message-Id: <20210713213200.2547-2-gustavo@noronha.dev.br>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
> 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 4a13f883a30..1e9893419fe 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -1260,6 +1260,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:
> #
> @@ -1338,6 +1353,7 @@
>   'discriminator' : 'type',
>   'data'    : {
>       'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
> +      'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
>       'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
>       'egl-headless': { 'type': 'DisplayEGLHeadless',
>                         'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 094a6c1d7c2..4df9ccc3446 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1916,6 +1916,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 8ab9ab5e84d..c41bc615d33 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -308,11 +308,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;
> @@ -335,6 +337,19 @@ - (void) raiseAllKeys;
>
> QemuCocoaView *cocoaView;
>
> +static CGEventRef handleTapEvent(CGEventTapProxy proxy, CGEventType type, CGEventRef cgEvent, void *userInfo)
> +{
> +    QemuCocoaView *cocoaView = (QemuCocoaView*) userInfo;

Extra space between cast and userinfo? Do you need explicit cast at all? 
In C at least void * does not need explicit cast, not sure about 
Objective-C but if it's not needed you could just remove the cast.

Regards,
BALATON Zoltan

> +    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
> {
> @@ -360,6 +375,11 @@ - (void) dealloc
>     }
>
>     qkbd_state_free(kbd);
> +
> +    if (eventsTap) {
> +        CFRelease(eventsTap);
> +    }
> +
>     [super dealloc];
> }
>
> @@ -654,6 +674,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));
> }
> @@ -1281,6 +1331,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
> {
> @@ -2057,11 +2114,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] delegate];
>     /* 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 toggleFullScreen: nil];
> +        });
> +    }
> +    if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
> +        dispatch_async(dispatch_get_main_queue(), ^{
> +            [controller setFullGrab: nil];
>         });
>     }
>     if (opts->has_show_cursor && opts->show_cursor) {
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2022-03-06 11:11 [PATCH 0/2] cocoa: keyboard quality of life Akihiko Odaki
@ 2022-03-06 11:11 ` Akihiko Odaki
  2022-03-06 11:28   ` BALATON Zoltan
  0 siblings, 1 reply; 17+ messages in thread
From: Akihiko Odaki @ 2022-03-06 11:11 UTC (permalink / raw)
  Cc: Peter Maydell, Markus Armbruster, qemu-devel, Gerd Hoffmann,
	Akihiko Odaki, Eric Blake, Gustavo Noronha Silva

From: Gustavo Noronha Silva <gustavo@noronha.dev.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.dev.br>
Message-Id: <20210713213200.2547-2-gustavo@noronha.dev.br>
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 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 4a13f883a30..1e9893419fe 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1260,6 +1260,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:
 #
@@ -1338,6 +1353,7 @@
   'discriminator' : 'type',
   'data'    : {
       'gtk': { 'type': 'DisplayGTK', 'if': 'CONFIG_GTK' },
+      'cocoa': { 'type': 'DisplayCocoa', 'if': 'CONFIG_COCOA' },
       'curses': { 'type': 'DisplayCurses', 'if': 'CONFIG_CURSES' },
       'egl-headless': { 'type': 'DisplayEGLHeadless',
                         'if': { 'all': ['CONFIG_OPENGL', 'CONFIG_GBM'] } },
diff --git a/qemu-options.hx b/qemu-options.hx
index 094a6c1d7c2..4df9ccc3446 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1916,6 +1916,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 8ab9ab5e84d..c41bc615d33 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -308,11 +308,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;
@@ -335,6 +337,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
 {
@@ -360,6 +375,11 @@ - (void) dealloc
     }
 
     qkbd_state_free(kbd);
+
+    if (eventsTap) {
+        CFRelease(eventsTap);
+    }
+
     [super dealloc];
 }
 
@@ -654,6 +674,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));
 }
@@ -1281,6 +1331,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
 {
@@ -2057,11 +2114,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] delegate];
     /* 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 toggleFullScreen: nil];
+        });
+    }
+    if (opts->u.cocoa.has_full_grab && opts->u.cocoa.full_grab) {
+        dispatch_async(dispatch_get_main_queue(), ^{
+            [controller setFullGrab: nil];
         });
     }
     if (opts->has_show_cursor && opts->show_cursor) {
-- 
2.32.0 (Apple Git-132)



^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2022-03-06 11:31 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-29 23:47 [PATCH 0/2] cocoa: keyboard quality of life gustavo
2021-04-29 23:47 ` [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed gustavo
2021-04-30  7:20   ` Markus Armbruster
2021-04-30 10:02     ` Gustavo Noronha Silva
2021-04-30 10:10       ` Gustavo Noronha Silva
2021-04-30 10:58         ` Markus Armbruster
2021-04-30 12:02           ` Gustavo Noronha Silva
2021-04-30 14:36             ` Markus Armbruster
2021-04-30 10:54       ` Markus Armbruster
2021-04-30  8:05   ` 'Gerd Hoffmann '
2021-04-30  9:43     ` Gustavo Noronha Silva
2021-04-29 23:47 ` [PATCH 2/2] ui/cocoa: add option to swap Option and Command, enable by default gustavo
2021-04-30  7:25   ` Markus Armbruster
2021-04-30  8:04   ` 'Gerd Hoffmann '
2021-04-30 10:36     ` Gustavo Noronha Silva
2022-03-06 11:11 [PATCH 0/2] cocoa: keyboard quality of life Akihiko Odaki
2022-03-06 11:11 ` [PATCH 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed Akihiko Odaki
2022-03-06 11:28   ` BALATON Zoltan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).