[Qemu-devel] [PATCH] Improve Cocoa modifier key handling

[Qemu-devel] [PATCH] Improve Cocoa modifier key handling

[Ian McKellar]

I had two problems with QEMU on macOS:
 1) Sometimes when alt-tabbing to QEMU it would act as if the 'a' key
    was pressed so I'd get 'aaaaaaaaa....'.
 2) Using Sikuli to programatically send keys to the QEMU window text
    like "foo_bar" would come out as "fooa-bar".

They looked similar and after much digging the problem turned out to be
the same. When QEMU's ui/cocoa.m received an NSFlagsChanged NSEvent it
looked at the keyCode to determine what modifier key changed. This
usually works fine but sometimes the keyCode is 0 and the app should
instead be looking at the modifierFlags bitmask. Key code 0 is the 'a'
key.

I added code that handles keyCode == 0 differently. It checks the
modifierFlags and if they differ from QEMU's idea of which modifier
keys are currently pressed it toggles those changed keys.

This fixes my problems and seems work fine.

Signed-off-by: Ian McKellar <ianloic@google.com>
---
 ui/cocoa.m | 60 ++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 12 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 207555edf7..e89020929b 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -52,6 +52,8 @@
 /* macOS 10.12 deprecated many constants, #define the new names for older SDKs */
 #if MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_X_VERSION_10_12
 #define NSEventMaskAny                  NSAnyEventMask
+#define NSEventModifierFlagCapsLock     NSAlphaShiftKeyMask
+#define NSEventModifierFlagShift        NSShiftKeyMask
 #define NSEventModifierFlagCommand      NSCommandKeyMask
 #define NSEventModifierFlagControl      NSControlKeyMask
 #define NSEventModifierFlagOption       NSAlternateKeyMask
@@ -268,7 +270,7 @@ static void handleAnyDeviceErrors(Error * err)
     NSWindow *fullScreenWindow;
     float cx,cy,cw,ch,cdx,cdy;
     CGDataProviderRef dataProviderRef;
-    int modifiers_state[256];
+    BOOL modifiers_state[256];
     BOOL isMouseGrabbed;
     BOOL isFullscreen;
     BOOL isAbsoluteEnabled;
@@ -536,18 +538,59 @@ QemuCocoaView *cocoaView;
     }
 }
 
+- (void) toggleModifier: (int)keycode {
+    // Toggle the stored state.
+    modifiers_state[keycode] = !modifiers_state[keycode];
+    // Send a keyup or keydown depending on the state.
+    qemu_input_event_send_key_qcode(dcl->con, keycode, modifiers_state[keycode]);
+}
+
+- (void) toggleStatefulModifier: (int)keycode {
+    // Toggle the stored state.
+    modifiers_state[keycode] = !modifiers_state[keycode];
+    // Generate keydown and keyup.
+    qemu_input_event_send_key_qcode(dcl->con, keycode, true);
+    qemu_input_event_send_key_qcode(dcl->con, keycode, false);
+}
+
 - (void) handleEvent:(NSEvent *)event
 {
     COCOA_DEBUG("QemuCocoaView: handleEvent\n");
 
     int buttons = 0;
-    int keycode;
+    int keycode = 0;
     bool mouse_event = false;
     NSPoint p = [event locationInWindow];
 
     switch ([event type]) {
         case NSEventTypeFlagsChanged:
-            keycode = cocoa_keycode_to_qemu([event keyCode]);
+            if ([event keyCode] == 0) {
+                // When the Cocoa keyCode is zero that means keys should be
+                // synthesized based on the values in in the eventModifiers
+                // bitmask.
+
+                if (qemu_console_is_graphic(NULL)) {
+                    NSEventModifierFlags modifiers = [event modifierFlags];
+
+                    if (!!(modifiers & NSEventModifierFlagCapsLock) != !!modifiers_state[Q_KEY_CODE_CAPS_LOCK]) {
+                        [self toggleStatefulModifier:Q_KEY_CODE_CAPS_LOCK];
+                    }
+                    if (!!(modifiers & NSEventModifierFlagShift) != !!modifiers_state[Q_KEY_CODE_SHIFT]) {
+                        [self toggleModifier:Q_KEY_CODE_SHIFT];
+                    }
+                    if (!!(modifiers & NSEventModifierFlagControl) != !!modifiers_state[Q_KEY_CODE_CTRL]) {
+                        [self toggleModifier:Q_KEY_CODE_CTRL];
+                    }
+                    if (!!(modifiers & NSEventModifierFlagOption) != !!modifiers_state[Q_KEY_CODE_ALT]) {
+                        [self toggleModifier:Q_KEY_CODE_ALT];
+                    }
+                    if (!!(modifiers & NSEventModifierFlagCommand) != !!modifiers_state[Q_KEY_CODE_META_L]) {
+                        [self toggleModifier:Q_KEY_CODE_META_L];
+                    }
+                }
+            } else {
+                keycode = cocoa_keycode_to_qemu([event keyCode]);
+            }
 
             if ((keycode == Q_KEY_CODE_META_L || keycode == Q_KEY_CODE_META_R)
                && !isMouseGrabbed) {
@@ -559,16 +602,9 @@ QemuCocoaView *cocoaView;
                 // emulate caps lock and num lock keydown and keyup
                 if (keycode == Q_KEY_CODE_CAPS_LOCK ||
                     keycode == Q_KEY_CODE_NUM_LOCK) {
-                    qemu_input_event_send_key_qcode(dcl->con, keycode, true);
-                    qemu_input_event_send_key_qcode(dcl->con, keycode, false);
+                    [self toggleStatefulModifier:keycode];
                 } else if (qemu_console_is_graphic(NULL)) {
-                    if (modifiers_state[keycode] == 0) { // keydown
-                        qemu_input_event_send_key_qcode(dcl->con, keycode, true);
-                        modifiers_state[keycode] = 1;
-                    } else { // keyup
-                        qemu_input_event_send_key_qcode(dcl->con, keycode, false);
-                        modifiers_state[keycode] = 0;
-                    }
+                  [self toggleModifier:keycode];
                 }
             }
 
-- 
2.13.0.rc1.294.g07d810a77f-goog

Re: [Qemu-devel] [PATCH] Improve Cocoa modifier key handling

[Gerd Hoffmann]

On Fri, 2017-05-26 at 16:38 -0700, Ian McKellar wrote:
> I had two problems with QEMU on macOS:
>  1) Sometimes when alt-tabbing to QEMU it would act as if the 'a' key
>     was pressed so I'd get 'aaaaaaaaa....'.
>  2) Using Sikuli to programatically send keys to the QEMU window text
>     like "foo_bar" would come out as "fooa-bar".
> 
> They looked similar and after much digging the problem turned out to
> be
> the same. When QEMU's ui/cocoa.m received an NSFlagsChanged NSEvent
> it
> looked at the keyCode to determine what modifier key changed. This
> usually works fine but sometimes the keyCode is 0 and the app should
> instead be looking at the modifierFlags bitmask. Key code 0 is the
> 'a'
> key.
> 
> I added code that handles keyCode == 0 differently. It checks the
> modifierFlags and if they differ from QEMU's idea of which modifier
> keys are currently pressed it toggles those changed keys.
> 
> This fixes my problems and seems work fine.

Added to ui queue.

thanks,
  Gerd

Re: [Qemu-devel] [PATCH] Improve Cocoa modifier key handling

[Programmingkid]

This patch is incompatible with anything below Mac OS 10.10. We support Mac OS 10.5 and up. I was able to make this patch work on Mac OS 10.6 by changing this line:

NSEventModifierFlags modifiers = [event modifierFlags];

to this:

NSUInteger modifiers = [event modifierFlags];

This wouldn't be enough. The [event modifierFlags] code would have to be replaced because the modifierFlags function is not available on Mac OS 10.5.

Re: [Qemu-devel] [PATCH] Improve Cocoa modifier key handling

[Peter Maydell]

On 23 June 2017 at 21:05, Programmingkid <programmingkidx@gmail.com> wrote:
> This patch is incompatible with anything below Mac OS 10.10.

Oops. Thanks for the report.

> We support Mac OS 10.5 and up. I was able to make this patch work
> on Mac OS 10.6 by changing this line:
>
> NSEventModifierFlags modifiers = [event modifierFlags];
>
> to this:
>
> NSUInteger modifiers = [event modifierFlags];
>
> This wouldn't be enough. The [event modifierFlags] code would have to
> be replaced because the modifierFlags function is not available on
> Mac OS 10.5.

Do you still use 10.5? It's a decade old now, and I notice that
in the QEMU 2.5 changelog we actually said we were going to
remove 10.5 support. So one option would be to simply remove
10.5 support entirely, if nobody needs it any more.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] Improve Cocoa modifier key handling

[G 3]

On Jun 24, 2017, at 8:14 AM, Peter Maydell wrote:

> On 23 June 2017 at 21:05, Programmingkid  
> <programmingkidx@gmail.com> wrote:
>> This patch is incompatible with anything below Mac OS 10.10.
>
> Oops. Thanks for the report.
>
>> We support Mac OS 10.5 and up. I was able to make this patch work
>> on Mac OS 10.6 by changing this line:
>>
>> NSEventModifierFlags modifiers = [event modifierFlags];
>>
>> to this:
>>
>> NSUInteger modifiers = [event modifierFlags];
>>
>> This wouldn't be enough. The [event modifierFlags] code would have to
>> be replaced because the modifierFlags function is not available on
>> Mac OS 10.5.

Please disregard what I said about the [event modifierFlags] code. It  
turns out it has been available since Mac OS 10.0. Sorry for the  
mistake.


> Do you still use 10.5? It's a decade old now, and I notice that
> in the QEMU 2.5 changelog we actually said we were going to
> remove 10.5 support. So one option would be to simply remove
> 10.5 support entirely, if nobody needs it any more.

I do have a computer with 10.5 on it. I will be happy to help support  
Mac OS 10.5.