qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ui/cocoa: Clear modifiers whenever possible
@ 2021-03-05 12:19 Akihiko Odaki
  2021-03-09 11:24 ` Gerd Hoffmann
  0 siblings, 1 reply; 2+ messages in thread
From: Akihiko Odaki @ 2021-03-05 12:19 UTC (permalink / raw)
  Cc: Peter Maydell, qemu-devel, Akihiko Odaki, Gerd Hoffmann

ui/cocoa does not receive NSEventTypeFlagsChanged when it is not active,
and the modifier state can be desynchronized in such a situation.

[NSEvent -modifierFlags] tells whether a modifier is *not* pressed, so
check it whenever receiving an event and clear the modifier if it is not
pressed.

Note that [NSEvent -modifierFlags] does not tell if a certain modifier
*is* pressed because the documented mask for [NSEvent -modifierFlags]
generalizes left shift and right shift, for example. CapsLock is the
only exception. The pressed state is synchronized only with
NSEventTypeFlagsChanged.

This change also removes modifier keys from keycode map. If they
are input with NSEventTypeKeyDown or NSEventTypeKeyUp, it leads to
desynchronization. Although such a situation is not observed, they are
removed just in case.

Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
 ui/cocoa.m | 142 ++++++++++++++++++++++++++++++++---------------------
 1 file changed, 86 insertions(+), 56 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index f27beb30e6e..940d8b83e31 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -189,14 +189,6 @@ static bool bool_with_iothread_lock(BoolCodeBlock block)
     [kVK_ANSI_Comma] = Q_KEY_CODE_COMMA,
     [kVK_ANSI_Period] = Q_KEY_CODE_DOT,
     [kVK_ANSI_Slash] = Q_KEY_CODE_SLASH,
-    [kVK_Shift] = Q_KEY_CODE_SHIFT,
-    [kVK_RightShift] = Q_KEY_CODE_SHIFT_R,
-    [kVK_Control] = Q_KEY_CODE_CTRL,
-    [kVK_RightControl] = Q_KEY_CODE_CTRL_R,
-    [kVK_Option] = Q_KEY_CODE_ALT,
-    [kVK_RightOption] = Q_KEY_CODE_ALT_R,
-    [kVK_Command] = Q_KEY_CODE_META_L,
-    [0x36] = Q_KEY_CODE_META_R, /* There is no kVK_RightCommand */
     [kVK_Space] = Q_KEY_CODE_SPC,
 
     [kVK_ANSI_Keypad0] = Q_KEY_CODE_KP_0,
@@ -615,9 +607,24 @@ - (void) toggleModifier: (int)keycode {
     qemu_input_event_send_key_qcode(dcl.con, keycode, modifiers_state[keycode]);
 }
 
-- (void) toggleStatefulModifier: (int)keycode {
+- (void) clearModifier: (int)keycode {
+    if (!modifiers_state[keycode]) {
+        return;
+    }
+
+    // Clear the stored state.
+    modifiers_state[keycode] = NO;
+    // Send a keyup.
+    qemu_input_event_send_key_qcode(dcl.con, keycode, false);
+}
+
+- (void) setStatefulModifier: (int)keycode down:(BOOL)down {
+    if (down == modifiers_state[keycode]) {
+        return;
+    }
+
     // Toggle the stored state.
-    modifiers_state[keycode] = !modifiers_state[keycode];
+    modifiers_state[keycode] = down;
     // Generate keydown and keyup.
     qemu_input_event_send_key_qcode(dcl.con, keycode, true);
     qemu_input_event_send_key_qcode(dcl.con, keycode, false);
@@ -714,57 +721,80 @@ - (bool) handleEventLocked:(NSEvent *)event
     static bool switched_to_fullscreen = false;
     // Location of event in virtual screen coordinates
     NSPoint p = [self screenLocationOfEvent:event];
+    NSUInteger modifiers = [event modifierFlags];
 
-    switch ([event type]) {
-        case NSEventTypeFlagsChanged:
-            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)) {
-                    NSUInteger modifiers = [event modifierFlags];
+    // emulate caps lock keydown and keyup
+    [self setStatefulModifier:Q_KEY_CODE_CAPS_LOCK down:!!(modifiers & NSEventModifierFlagCapsLock)];
 
-                    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) {
-              /* Don't pass command key changes to guest unless mouse is grabbed */
-              keycode = 0;
-            }
+    if (qemu_console_is_graphic(NULL)) {
+        if (!(modifiers & NSEventModifierFlagShift)) {
+            [self clearModifier:Q_KEY_CODE_SHIFT];
+            [self clearModifier:Q_KEY_CODE_SHIFT_R];
+        }
+        if (!(modifiers & NSEventModifierFlagControl)) {
+            [self clearModifier:Q_KEY_CODE_CTRL];
+            [self clearModifier:Q_KEY_CODE_CTRL_R];
+        }
+        if (!(modifiers & NSEventModifierFlagOption)) {
+            [self clearModifier:Q_KEY_CODE_ALT];
+            [self clearModifier:Q_KEY_CODE_ALT_R];
+        }
+        if (!(modifiers & NSEventModifierFlagCommand)) {
+            [self clearModifier:Q_KEY_CODE_META_L];
+            [self clearModifier:Q_KEY_CODE_META_R];
+        }
+    }
 
-            if (keycode) {
-                // emulate caps lock and num lock keydown and keyup
-                if (keycode == Q_KEY_CODE_CAPS_LOCK ||
-                    keycode == Q_KEY_CODE_NUM_LOCK) {
-                    [self toggleStatefulModifier:keycode];
-                } else if (qemu_console_is_graphic(NULL)) {
-                    if (switched_to_fullscreen) {
-                        switched_to_fullscreen = false;
-                    } else {
-                        [self toggleModifier:keycode];
-                    }
+    switch ([event type]) {
+        case NSEventTypeFlagsChanged:
+            if (qemu_console_is_graphic(NULL)) {
+                switch ([event keyCode]) {
+                    case kVK_Shift:
+                        if (!!(modifiers & NSEventModifierFlagShift)) {
+                            [self toggleModifier:Q_KEY_CODE_SHIFT];
+                        }
+                        break;
+
+                    case kVK_RightShift:
+                        if (!!(modifiers & NSEventModifierFlagShift)) {
+                            [self toggleModifier:Q_KEY_CODE_SHIFT_R];
+                        }
+                        break;
+
+                    case kVK_Control:
+                        if (!!(modifiers & NSEventModifierFlagControl)) {
+                            [self toggleModifier:Q_KEY_CODE_CTRL];
+                        }
+                        break;
+
+                    case kVK_Option:
+                        if (!!(modifiers & NSEventModifierFlagOption)) {
+                            [self toggleModifier:Q_KEY_CODE_ALT];
+                        }
+                        break;
+
+                    case kVK_RightOption:
+                        if (!!(modifiers & NSEventModifierFlagOption)) {
+                            [self toggleModifier:Q_KEY_CODE_ALT_R];
+                        }
+                        break;
+
+                    /* Don't pass command key changes to guest unless mouse is grabbed */
+                    case kVK_Command:
+                        if (isMouseGrabbed &&
+                            !!(modifiers & NSEventModifierFlagCommand)) {
+                            [self toggleModifier:Q_KEY_CODE_META_L];
+                        }
+                        break;
+
+                    case kVK_RightCommand:
+                        if (isMouseGrabbed &&
+                            !!(modifiers & NSEventModifierFlagCommand)) {
+                            [self toggleModifier:Q_KEY_CODE_META_R];
+                        }
+                        break;
                 }
             }
-
             break;
         case NSEventTypeKeyDown:
             keycode = cocoa_keycode_to_qemu([event keyCode]);
-- 
2.24.3 (Apple Git-128)



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

* Re: [PATCH] ui/cocoa: Clear modifiers whenever possible
  2021-03-05 12:19 [PATCH] ui/cocoa: Clear modifiers whenever possible Akihiko Odaki
@ 2021-03-09 11:24 ` Gerd Hoffmann
  0 siblings, 0 replies; 2+ messages in thread
From: Gerd Hoffmann @ 2021-03-09 11:24 UTC (permalink / raw)
  To: Akihiko Odaki; +Cc: Peter Maydell, qemu-devel

On Fri, Mar 05, 2021 at 09:19:09PM +0900, Akihiko Odaki wrote:
> ui/cocoa does not receive NSEventTypeFlagsChanged when it is not active,
> and the modifier state can be desynchronized in such a situation.
> 
> [NSEvent -modifierFlags] tells whether a modifier is *not* pressed, so
> check it whenever receiving an event and clear the modifier if it is not
> pressed.
> 
> Note that [NSEvent -modifierFlags] does not tell if a certain modifier
> *is* pressed because the documented mask for [NSEvent -modifierFlags]
> generalizes left shift and right shift, for example. CapsLock is the
> only exception. The pressed state is synchronized only with
> NSEventTypeFlagsChanged.
> 
> This change also removes modifier keys from keycode map. If they
> are input with NSEventTypeKeyDown or NSEventTypeKeyUp, it leads to
> desynchronization. Although such a situation is not observed, they are
> removed just in case.

Have you noticed / looked at ui/kbd-state.c?

This provides a bunch of helper functions to track key and modifier
state.  The other UIs used to have their own keystate tracking too,
but most switched over meanwhile.  Nobody did it for cocoa yet, but
looking at your changes I suspect at the end it might be easier to
switch over cocoa to the shared too instead of trying to fix the
private implementation ...

take care,
  Gerd



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

end of thread, other threads:[~2021-03-09 11:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 12:19 [PATCH] ui/cocoa: Clear modifiers whenever possible Akihiko Odaki
2021-03-09 11:24 ` 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).