Fix for Modifier Key Detection Issues in D7229
ClosedPublic
Actions

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

Details

Reviewers

Brecht Van Lommel (brecht)
Germano Cavalcante (mano-wii)

Commits

rBe0030d53bc4e: Fix key detection issues introduced by D7229

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

Asher (ThatAsherGuy) created this revision.Mar 27 2020, 6:08 PM

Asher (ThatAsherGuy) mentioned this in T75126: Broken Selection while holding Shift or Ctrl.Mar 27 2020, 6:16 PM

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:

P1316 Proposed for D7257 (repeated modifier)

1	diff --git a/intern/ghost/intern/GHOST_SystemWin32.cpp b/intern/ghost/intern/GHOST_SystemWin32.cpp
2	index 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
121	diff --git a/intern/ghost/intern/GHOST_SystemWin32.h b/intern/ghost/intern/GHOST_SystemWin32.h
122	index 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.