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

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

v2 fixes QAPI issues pointed out by Markus and comes with his
Acked-By. 

I tried also applying Gerd's suggestion of flipping the flags
on modifiers, but turns out it is more intricate than that,
as we then also need to flip the keyCode that is used on the
switch, but we can only do that if we are handling a key event,
so things get hairy fairly quickly.

After looking at the resulting code I decided to leave the more
localized checks in place rather than going through with the
alternative.

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    |  22 ++++++++
 qemu-options.hx |   4 ++
 ui/cocoa.m      | 137 ++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 153 insertions(+), 10 deletions(-)

-- 
2.24.3 (Apple Git-128)



^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed
  2021-04-30 21:38 [PATCH v2 0/2] cocoa: keyboard quality of life gustavo
@ 2021-04-30 21:38 ` gustavo
  2021-04-30 21:38 ` [PATCH v2 2/2] ui/cocoa: add option to swap Option and Command, enable by default gustavo
  1 sibling, 0 replies; 6+ messages in thread
From: gustavo @ 2021-04-30 21:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: 'Peter Maydell ', 'Markus Armbruster ',
	'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

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      | 73 +++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 90 insertions(+), 2 deletions(-)

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 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] 6+ messages in thread
* [PATCH v2 2/2] ui/cocoa: add option to swap Option and Command, enable by default
  2021-04-30 21:38 [PATCH v2 0/2] cocoa: keyboard quality of life gustavo
  2021-04-30 21:38 ` [PATCH v2 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed gustavo
@ 2021-04-30 21:38 ` gustavo
  2021-05-01  9:39   ` BALATON Zoltan
  1 sibling, 1 reply; 6+ messages in thread
From: gustavo @ 2021-04-30 21:38 UTC (permalink / raw)
  To: qemu-devel
  Cc: 'Peter Maydell ', 'Markus Armbruster ',
	'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.

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..ffd416a474 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: on)
+#
 # 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 a77505241f..6fcb0b6aaa 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 f1e4449082..73eb5080be 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,14 +875,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;
             }
@@ -1085,6 +1120,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 +1307,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 +1996,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] 6+ messages in thread
* Re: [PATCH v2 2/2] ui/cocoa: add option to swap Option and Command, enable by default
  2021-04-30 21:38 ` [PATCH v2 2/2] ui/cocoa: add option to swap Option and Command, enable by default gustavo
@ 2021-05-01  9:39   ` BALATON Zoltan
  2021-05-01 10:47     ` Gustavo Noronha Silva
  0 siblings, 1 reply; 6+ messages in thread
From: BALATON Zoltan @ 2021-05-01  9:39 UTC (permalink / raw)
  To: gustavo
  Cc: 'Peter Maydell ', 'Gerd Hoffmann ',
	qemu-devel, 'Markus Armbruster '

On Fri, 30 Apr 2021, gustavo@noronha.eti.br wrote:
> 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.

Not sure it's a good idea to enable this by default. That mixes up the 
keyboard unexpectedly for those who don't need this and also different 
from previous behaviour so better to have this option enabled by those who 
want it than annoy everyone.

Regards,
BALATON Zoltan

> 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..ffd416a474 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: on)
> +#
> # 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 a77505241f..6fcb0b6aaa 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 f1e4449082..73eb5080be 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,14 +875,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;
>             }
> @@ -1085,6 +1120,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 +1307,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 +1996,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;
>     };
>


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] ui/cocoa: add option to swap Option and Command, enable by default
  2021-05-01  9:39   ` BALATON Zoltan
@ 2021-05-01 10:47     ` Gustavo Noronha Silva
  2021-05-04  9:20       ` 'Gerd Hoffmann '
  0 siblings, 1 reply; 6+ messages in thread
From: Gustavo Noronha Silva @ 2021-05-01 10:47 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: 'Peter Maydell ', 'Gerd Hoffmann ',
	qemu-devel, 'Markus Armbruster '

Hey there,

On Sat, May 1, 2021, at 6:39 AM, BALATON Zoltan wrote:
> > 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.
> 
> Not sure it's a good idea to enable this by default. That mixes up the 
> keyboard unexpectedly for those who don't need this and also different 
> from previous behaviour so better to have this option enabled by those who 
> want it than annoy everyone.

That is indeed a good point. I can certainly switch that off by default and enable it myself, anyone else would like to weigh in on this one?

Cheers,

Gustavo


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] ui/cocoa: add option to swap Option and Command, enable by default
  2021-05-01 10:47     ` Gustavo Noronha Silva
@ 2021-05-04  9:20       ` 'Gerd Hoffmann '
  0 siblings, 0 replies; 6+ messages in thread
From: 'Gerd Hoffmann ' @ 2021-05-04  9:20 UTC (permalink / raw)
  To: Gustavo Noronha Silva
  Cc: 'Peter Maydell ', qemu-devel, 'Markus Armbruster '

On Sat, May 01, 2021 at 07:47:10AM -0300, Gustavo Noronha Silva wrote:
> Hey there,
> 
> On Sat, May 1, 2021, at 6:39 AM, BALATON Zoltan wrote:
> > > 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.
> > 
> > Not sure it's a good idea to enable this by default. That mixes up the 
> > keyboard unexpectedly for those who don't need this and also different 
> > from previous behaviour so better to have this option enabled by those who 
> > want it than annoy everyone.
> 
> That is indeed a good point. I can certainly switch that off by
> default and enable it myself, anyone else would like to weigh in on
> this one?

Usually the default of new config option is "traditional behavior",
i.e. do what older versions without the config option did.

So, yes, please flip the default.

thanks,
  Gerd



^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-04  9:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-30 21:38 [PATCH v2 0/2] cocoa: keyboard quality of life gustavo
2021-04-30 21:38 ` [PATCH v2 1/2] ui/cocoa: capture all keys and combos when mouse is grabbed gustavo
2021-04-30 21:38 ` [PATCH v2 2/2] ui/cocoa: add option to swap Option and Command, enable by default gustavo
2021-05-01  9:39   ` BALATON Zoltan
2021-05-01 10:47     ` Gustavo Noronha Silva
2021-05-04  9:20       ` 'Gerd Hoffmann '

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