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

2021-05-12 Thread Gustavo Noronha Silva
Hey,

On Wed, May 12, 2021, at 12:06 AM, Akihiko Odaki wrote:
> Reviewed-by: Akihiko Odaki 
> 
> 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 2/2] ui/cocoa: add option to swap Option and Command

2021-05-11 Thread Akihiko Odaki
Reviewed-by: Akihiko Odaki 

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

2021年5月12日(水) 3:56 :
>
> From: Gustavo Noronha Silva 
>
> 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 
> Signed-off-by: Gustavo Noronha Silva 
> ---
>  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=]\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=]\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 ([s

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

2021-05-11 Thread gustavo
From: Gustavo Noronha Silva 

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 
Signed-off-by: Gustavo Noronha Silva 
---
 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=]\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=]\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: