* [PATCH v3] ui/cocoa: Clear modifiers whenever possible
@ 2021-03-10 14:46 Akihiko Odaki
2021-03-11 8:53 ` Gerd Hoffmann
0 siblings, 1 reply; 2+ messages in thread
From: Akihiko Odaki @ 2021-03-10 14:46 UTC (permalink / raw)
Cc: Peter Maydell, Konstantin Nazarov, 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.
Moreover, QKbdState is introduced for automatic key state tracking.
Thanks to Konstantin Nazarov for testing and finding a bug in this
change:
https://gist.github.com/akihikodaki/87df4149e7ca87f18dc56807ec5a1bc5#gistcomment-3659419
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
ui/cocoa.m | 148 +++++++++++++++++++++++++++--------------------------
1 file changed, 76 insertions(+), 72 deletions(-)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index f27beb30e6e..035f96aab04 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -30,6 +30,7 @@
#include "qemu-common.h"
#include "ui/console.h"
#include "ui/input.h"
+#include "ui/kbd-state.h"
#include "sysemu/sysemu.h"
#include "sysemu/runstate.h"
#include "sysemu/cpu-throttle.h"
@@ -189,14 +190,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,
@@ -306,7 +299,7 @@ @interface QemuCocoaView : NSView
NSWindow *fullScreenWindow;
float cx,cy,cw,ch,cdx,cdy;
pixman_image_t *pixman_image;
- BOOL modifiers_state[256];
+ QKbdState *kbd;
BOOL isMouseGrabbed;
BOOL isFullscreen;
BOOL isAbsoluteEnabled;
@@ -353,6 +346,7 @@ - (id)initWithFrame:(NSRect)frameRect
screen.width = frameRect.size.width;
screen.height = frameRect.size.height;
+ kbd = qkbd_state_init(dcl.con);
}
return self;
@@ -366,6 +360,7 @@ - (void) dealloc
pixman_image_unref(pixman_image);
}
+ qkbd_state_free(kbd);
[super dealloc];
}
@@ -608,19 +603,8 @@ - (void) toggleFullScreen:(id)sender
}
}
-- (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) toggleKey: (int)keycode {
+ qkbd_state_key_event(kbd, keycode, !qkbd_state_key_get(kbd, keycode));
}
// Does the work of sending input to the monitor
@@ -714,57 +698,86 @@ - (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];
+
+ // emulate caps lock keydown and keyup
+ if (!!(modifiers & NSEventModifierFlagCapsLock) !=
+ qkbd_state_modifier_get(kbd, QKBD_MOD_CAPSLOCK)) {
+ qkbd_state_key_event(kbd, Q_KEY_CODE_CAPS_LOCK, true);
+ qkbd_state_key_event(kbd, Q_KEY_CODE_CAPS_LOCK, false);
+ }
+
+ if (!(modifiers & NSEventModifierFlagShift)) {
+ qkbd_state_key_event(kbd, Q_KEY_CODE_SHIFT, false);
+ qkbd_state_key_event(kbd, Q_KEY_CODE_SHIFT_R, false);
+ }
+ if (!(modifiers & NSEventModifierFlagControl)) {
+ qkbd_state_key_event(kbd, Q_KEY_CODE_CTRL, false);
+ 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 (!(modifiers & NSEventModifierFlagCommand)) {
+ 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]) {
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];
+ switch ([event keyCode]) {
+ case kVK_Shift:
+ if (!!(modifiers & NSEventModifierFlagShift)) {
+ [self toggleKey:Q_KEY_CODE_SHIFT];
+ }
+ break;
- if (!!(modifiers & NSEventModifierFlagCapsLock) != !!modifiers_state[Q_KEY_CODE_CAPS_LOCK]) {
- [self toggleStatefulModifier:Q_KEY_CODE_CAPS_LOCK];
+ case kVK_RightShift:
+ if (!!(modifiers & NSEventModifierFlagShift)) {
+ [self toggleKey:Q_KEY_CODE_SHIFT_R];
}
- if (!!(modifiers & NSEventModifierFlagShift) != !!modifiers_state[Q_KEY_CODE_SHIFT]) {
- [self toggleModifier:Q_KEY_CODE_SHIFT];
+ break;
+
+ case kVK_Control:
+ if (!!(modifiers & NSEventModifierFlagControl)) {
+ [self toggleKey:Q_KEY_CODE_CTRL];
}
- if (!!(modifiers & NSEventModifierFlagControl) != !!modifiers_state[Q_KEY_CODE_CTRL]) {
- [self toggleModifier:Q_KEY_CODE_CTRL];
+ break;
+
+ case kVK_RightControl:
+ if (!!(modifiers & NSEventModifierFlagControl)) {
+ [self toggleKey:Q_KEY_CODE_CTRL_R];
}
- if (!!(modifiers & NSEventModifierFlagOption) != !!modifiers_state[Q_KEY_CODE_ALT]) {
- [self toggleModifier:Q_KEY_CODE_ALT];
+ break;
+
+ case kVK_Option:
+ if (!!(modifiers & NSEventModifierFlagOption)) {
+ [self toggleKey:Q_KEY_CODE_ALT];
}
- if (!!(modifiers & NSEventModifierFlagCommand) != !!modifiers_state[Q_KEY_CODE_META_L]) {
- [self toggleModifier:Q_KEY_CODE_META_L];
+ break;
+
+ case kVK_RightOption:
+ if (!!(modifiers & NSEventModifierFlagOption)) {
+ [self toggleKey:Q_KEY_CODE_ALT_R];
}
- }
- } else {
- keycode = cocoa_keycode_to_qemu([event keyCode]);
- }
+ break;
- 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;
- }
+ /* Don't pass command key changes to guest unless mouse is grabbed */
+ case kVK_Command:
+ if (isMouseGrabbed &&
+ !!(modifiers & NSEventModifierFlagCommand)) {
+ [self toggleKey:Q_KEY_CODE_META_L];
+ }
+ break;
- 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];
+ case kVK_RightCommand:
+ if (isMouseGrabbed &&
+ !!(modifiers & NSEventModifierFlagCommand)) {
+ [self toggleKey:Q_KEY_CODE_META_R];
}
- }
+ break;
}
-
break;
case NSEventTypeKeyDown:
keycode = cocoa_keycode_to_qemu([event keyCode]);
@@ -804,7 +817,7 @@ - (bool) handleEventLocked:(NSEvent *)event
}
if (qemu_console_is_graphic(NULL)) {
- qemu_input_event_send_key_qcode(dcl.con, keycode, true);
+ qkbd_state_key_event(kbd, keycode, true);
} else {
[self handleMonitorInput: event];
}
@@ -819,7 +832,7 @@ - (bool) handleEventLocked:(NSEvent *)event
}
if (qemu_console_is_graphic(NULL)) {
- qemu_input_event_send_key_qcode(dcl.con, keycode, false);
+ qkbd_state_key_event(kbd, keycode, false);
}
break;
case NSEventTypeMouseMoved:
@@ -1003,17 +1016,8 @@ - (QEMUScreen) gscreen {return screen;}
*/
- (void) raiseAllKeys
{
- const int max_index = ARRAY_SIZE(modifiers_state);
-
with_iothread_lock(^{
- int index;
-
- for (index = 0; index < max_index; index++) {
- if (modifiers_state[index]) {
- modifiers_state[index] = 0;
- qemu_input_event_send_key_qcode(dcl.con, index, false);
- }
- }
+ qkbd_state_lift_all_keys(kbd);
});
}
@end
--
2.24.3 (Apple Git-128)
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] ui/cocoa: Clear modifiers whenever possible
2021-03-10 14:46 [PATCH v3] ui/cocoa: Clear modifiers whenever possible Akihiko Odaki
@ 2021-03-11 8:53 ` Gerd Hoffmann
0 siblings, 0 replies; 2+ messages in thread
From: Gerd Hoffmann @ 2021-03-11 8:53 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: Peter Maydell, Konstantin Nazarov, qemu-devel
Hi,
> + switch ([event keyCode]) {
> + case kVK_Shift:
> + if (!!(modifiers & NSEventModifierFlagShift)) {
> + [self toggleKey:Q_KEY_CODE_SHIFT];
> + }
Ah, nice, this should get left-shift toggles while holding right-shift
right.
A comment describing the NSEventTypeFlagsChanged quirks would be nice to
make it easier to understand the logic of this code. Can go as separate
patch, I'll add this to the UI queue.
thanks,
Gerd
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-03-11 8:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-10 14:46 [PATCH v3] ui/cocoa: Clear modifiers whenever possible Akihiko Odaki
2021-03-11 8:53 ` 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).