Page MenuHome

Fix for Modifier Key Detection Issues in D7229
ClosedPublic

Authored by Asher (ThatAsherGuy) on Mar 27 2020, 6:08 PM.

Details

Summary

While D7229 improves keyboard layout handling, it introduces some issues with how modifier keys are processed. Removing the GHOST_kKeyUnknown check from processKeyEvent() produces repeated unknown key events whenever a modifier key is held down, due to the way ghost uses GHOST_kKeyUnknown as a filter value for modifier key events.

This patch adds a GHOST_kKeyFilter value, so we can differentiate between unknown inputs and known-but-unwanted inputs. It fixes the modifier key issues introduced by D7229, but I haven't had a chance to test it with a wider set of keyboard layouts or input devices. Treat it like something halfway between a patch and a bug report.

Diff Detail

Repository
rB Blender

Event Timeline

Nice catch, but I would prefer a solution that does not add an enum that will not be used by other operating systems.
Perhaps making system->hardKey return a bool instead of a key. Or checking other properties.
I will analyze the code to see if I can find another way to solve it.
@Brecht Van Lommel (brecht), any suggestions?

We do something similar for OS X, returning GHOST_kKeyUnknown for a handful of volume and numpad keys. Returning GHOST_kKeyFilter for those keys might be a good idea.

Returning a boolean seems fine, makes sense to have an "ignore key" return value that is separate from the GHOST_TKey.

Here is my proposal:

1diff --git a/intern/ghost/intern/GHOST_SystemWin32.cpp b/intern/ghost/intern/GHOST_SystemWin32.cpp
2index e31186bd6a5..e4988d8a0b3 100644
3--- a/intern/ghost/intern/GHOST_SystemWin32.cpp
4+++ b/intern/ghost/intern/GHOST_SystemWin32.cpp
5@@ -620,8 +620,12 @@ GHOST_TSuccess GHOST_SystemWin32::exit()
6 return GHOST_System::exit();
7 }
8
9-GHOST_TKey GHOST_SystemWin32::hardKey(RAWINPUT const &raw, int *keyDown, char *vk)
10+GHOST_TKey GHOST_SystemWin32::hardKey(RAWINPUT const &raw,
11+ bool *r_keyDown,
12+ bool *r_is_repeated_modifier)
13 {
14+ bool is_repeated_modifier = false;
15+
16 GHOST_SystemWin32 *system = (GHOST_SystemWin32 *)getSystem();
17 GHOST_TKey key = GHOST_kKeyUnknown;
18 GHOST_ModifierKeys modifiers;
19@@ -630,7 +634,7 @@ GHOST_TKey GHOST_SystemWin32::hardKey(RAWINPUT const &raw, int *keyDown, char *v
20 // RI_KEY_BREAK doesn't work for sticky keys release, so we also
21 // check for the up message
22 unsigned int msg = raw.data.keyboard.Message;
23- *keyDown = !(raw.data.keyboard.Flags & RI_KEY_BREAK) && msg != WM_KEYUP && msg != WM_SYSKEYUP;
24+ *r_keyDown = !(raw.data.keyboard.Flags & RI_KEY_BREAK) && msg != WM_KEYUP && msg != WM_SYSKEYUP;
25
26 key = this->convertKey(raw.data.keyboard.VKey,
27 raw.data.keyboard.MakeCode,
28@@ -642,32 +646,32 @@ GHOST_TKey GHOST_SystemWin32::hardKey(RAWINPUT const &raw, int *keyDown, char *v
29 GHOST_TModifierKeyMask modifier;
30 switch (key) {
31 case GHOST_kKeyLeftShift: {
32- changed = (modifiers.get(GHOST_kModifierKeyLeftShift) != (bool)*keyDown);
33+ changed = (modifiers.get(GHOST_kModifierKeyLeftShift) != *r_keyDown);
34 modifier = GHOST_kModifierKeyLeftShift;
35 break;
36 }
37 case GHOST_kKeyRightShift: {
38- changed = (modifiers.get(GHOST_kModifierKeyRightShift) != (bool)*keyDown);
39+ changed = (modifiers.get(GHOST_kModifierKeyRightShift) != *r_keyDown);
40 modifier = GHOST_kModifierKeyRightShift;
41 break;
42 }
43 case GHOST_kKeyLeftControl: {
44- changed = (modifiers.get(GHOST_kModifierKeyLeftControl) != (bool)*keyDown);
45+ changed = (modifiers.get(GHOST_kModifierKeyLeftControl) != *r_keyDown);
46 modifier = GHOST_kModifierKeyLeftControl;
47 break;
48 }
49 case GHOST_kKeyRightControl: {
50- changed = (modifiers.get(GHOST_kModifierKeyRightControl) != (bool)*keyDown);
51+ changed = (modifiers.get(GHOST_kModifierKeyRightControl) != *r_keyDown);
52 modifier = GHOST_kModifierKeyRightControl;
53 break;
54 }
55 case GHOST_kKeyLeftAlt: {
56- changed = (modifiers.get(GHOST_kModifierKeyLeftAlt) != (bool)*keyDown);
57+ changed = (modifiers.get(GHOST_kModifierKeyLeftAlt) != *r_keyDown);
58 modifier = GHOST_kModifierKeyLeftAlt;
59 break;
60 }
61 case GHOST_kKeyRightAlt: {
62- changed = (modifiers.get(GHOST_kModifierKeyRightAlt) != (bool)*keyDown);
63+ changed = (modifiers.get(GHOST_kModifierKeyRightAlt) != *r_keyDown);
64 modifier = GHOST_kModifierKeyRightAlt;
65 break;
66 }
67@@ -676,17 +680,15 @@ GHOST_TKey GHOST_SystemWin32::hardKey(RAWINPUT const &raw, int *keyDown, char *v
68 }
69
70 if (changed) {
71- modifiers.set(modifier, (bool)*keyDown);
72+ modifiers.set(modifier, *r_keyDown);
73 system->storeModifierKeys(modifiers);
74 }
75 else {
76- key = GHOST_kKeyUnknown;
77+ is_repeated_modifier = true;
78 }
79 }
80
81- if (vk)
82- *vk = raw.data.keyboard.VKey;
83-
84+ *r_is_repeated_modifier = is_repeated_modifier;
85 return key;
86 }
87
88@@ -1038,16 +1040,17 @@ void GHOST_SystemWin32::processWheelEvent(GHOST_WindowWin32 *window, WPARAM wPar
89
90 GHOST_EventKey *GHOST_SystemWin32::processKeyEvent(GHOST_WindowWin32 *window, RAWINPUT const &raw)
91 {
92- int keyDown = 0;
93- char vk;
94+ bool keyDown = false;
95+ bool is_repeated_modifier = false;
96 GHOST_SystemWin32 *system = (GHOST_SystemWin32 *)getSystem();
97- GHOST_TKey key = system->hardKey(raw, &keyDown, &vk);
98+ GHOST_TKey key = system->hardKey(raw, &keyDown, &is_repeated_modifier);
99 GHOST_EventKey *event;
100
101 /* We used to check `if (key != GHOST_kKeyUnknown)`, but since the message
102 * values `WM_SYSKEYUP`, `WM_KEYUP` and `WM_CHAR` are ignored, we capture
103 * those events here as well. */
104- {
105+ if (!is_repeated_modifier) {
106+ char vk = raw.data.keyboard.VKey;
107 char utf8_char[6] = {0};
108 char ascii = 0;
109 bool is_repeat = false;
110@@ -1105,6 +1108,10 @@ GHOST_EventKey *GHOST_SystemWin32::processKeyEvent(GHOST_WindowWin32 *window, RA
111
112 // GHOST_PRINTF("%c\n", ascii); // we already get this info via EventPrinter
113 }
114+ else {
115+ event = NULL;
116+ }
117+
118 return event;
119 }
120
121diff --git a/intern/ghost/intern/GHOST_SystemWin32.h b/intern/ghost/intern/GHOST_SystemWin32.h
122index e624cc83427..0d9fd268d0f 100644
123--- a/intern/ghost/intern/GHOST_SystemWin32.h
124+++ b/intern/ghost/intern/GHOST_SystemWin32.h
125@@ -278,7 +278,7 @@ class GHOST_SystemWin32 : public GHOST_System {
126 * \param vk Pointer to virtual key
127 * \return The GHOST key (GHOST_kKeyUnknown if no match).
128 */
129- GHOST_TKey hardKey(RAWINPUT const &raw, int *keyDown, char *vk);
130+ GHOST_TKey hardKey(RAWINPUT const &raw, bool *r_keyDown, bool *r_is_repeated_modifier);
131
132 /**
133 * Creates mouse button event.

What do you all think?

This revision was not accepted when it landed; it landed in state Needs Review.Mar 28 2020, 1:01 AM
This revision was automatically updated to reflect the committed changes.