Changeset View
Standalone View
ghost/intern/GHOST_SystemWin32.cpp
| Context not available. | |||||
| typedef BOOL(API *GHOST_WIN32_EnableNonClientDpiScaling)(HWND); | typedef BOOL(API *GHOST_WIN32_EnableNonClientDpiScaling)(HWND); | ||||
| GHOST_SystemWin32::GHOST_SystemWin32() | GHOST_SystemWin32::GHOST_SystemWin32() | ||||
| : m_hasPerformanceCounter(false), m_freq(0), m_start(0), m_lfstart(0) | : m_keycode_last_repeat_key(0), | ||||
| m_hasPerformanceCounter(false), m_freq(0), m_start(0), m_lfstart(0) | |||||
| { | { | ||||
| m_displayManager = new GHOST_DisplayManagerWin32(); | m_displayManager = new GHOST_DisplayManagerWin32(); | ||||
| GHOST_ASSERT(m_displayManager, "GHOST_SystemWin32::GHOST_SystemWin32(): m_displayManager==0\n"); | GHOST_ASSERT(m_displayManager, "GHOST_SystemWin32::GHOST_SystemWin32(): m_displayManager==0\n"); | ||||
| Context not available. | |||||
| system->m_wheelDeltaAccum = acc * direction; | system->m_wheelDeltaAccum = acc * direction; | ||||
| } | } | ||||
| GHOST_EventKey *GHOST_SystemWin32::processKeyEvent(GHOST_WindowWin32 *window, RAWINPUT const &raw) | GHOST_EventKey *GHOST_SystemWin32::processRawKeyEvent(GHOST_WindowWin32 *window, RAWINPUT const &raw) | ||||
| { | { | ||||
| bool keyDown = false; | bool keyDown = false; | ||||
| bool is_repeated_modifier = false; | bool is_repeated_modifier = false; | ||||
| GHOST_SystemWin32 *system = (GHOST_SystemWin32 *)getSystem(); | GHOST_SystemWin32 *system = (GHOST_SystemWin32 *)getSystem(); | ||||
| GHOST_TKey key = system->hardKey(raw, &keyDown, &is_repeated_modifier); | GHOST_TKey key = system->hardKey(raw, &keyDown, &is_repeated_modifier); | ||||
| GHOST_EventKey *event; | |||||
| /* We used to check `if (key != GHOST_kKeyUnknown)`, but since the message | /* We used to check `if (key != GHOST_kKeyUnknown)`, but since the message | ||||
| * values `WM_SYSKEYUP`, `WM_KEYUP` and `WM_CHAR` are ignored, we capture | * values `WM_SYSKEYUP`, `WM_KEYUP` and `WM_CHAR` are ignored for hardware | ||||
nicholas_rishel: No longer ignored, needs explanation. | |||||
Done Inline Actions
nicholas_rishel: > No longer ignored, needs explanation.
| |||||
| * those events here as well. */ | * keys, we capture those events here as well. */ | ||||
mano-wiiUnsubmitted Not Done Inline ActionsFor me that description needs a little more clarification.
(I'm not sure who maintains this area, so whoever is going to revise this patch may have to study it first). mano-wii: For me that description needs a little more clarification.
- Where are these messages ignored? | |||||
samhocevarAuthorUnsubmitted Done Inline ActionsI removed mention of the “hardware keys”. Do you think the new wording in the comment clarifies what happens, or does it need more explanations? I could add more details about where the messages are ignored (in the large switch/case around line 1600) but that information was already missing before my patch. I believe a seasoned Windows programmer will immediately know where to look for when reading about WM_-something being ignored. samhocevar: I removed mention of the “hardware keys”. Do you think the new wording in the comment clarifies… | |||||
| if (!is_repeated_modifier) { | if (!is_repeated_modifier) { | ||||
| char vk = raw.data.keyboard.VKey; | int vk = raw.data.keyboard.VKey; | ||||
| char utf8_char[6] = {0}; | int scanCode = raw.data.keyboard.MakeCode; | ||||
| char ascii = 0; | return processKeyEvent(window, keyDown, key, vk, scanCode); | ||||
| bool is_repeat = false; | } else { | ||||
| return NULL; | |||||
| /* Unlike on Linux, not all keys can send repeat events. E.g. modifier keys don't. */ | } | ||||
| if (keyDown) { | } | ||||
| if (system->m_keycode_last_repeat_key == vk) { | |||||
| is_repeat = true; | GHOST_EventKey *GHOST_SystemWin32::processKeyEvent(GHOST_WindowWin32 *window, | ||||
| } | bool keyDown, | ||||
| system->m_keycode_last_repeat_key = vk; | GHOST_TKey key, | ||||
| unsigned short vk, | |||||
| unsigned short scanCode) | |||||
Done Inline ActionsShould these be unsigned shorts? nicholas_rishel: Should these be unsigned shorts? | |||||
Done Inline ActionsTo clarify this question: I'd appreciate if you could review when types are being converted to types of different length or signed/unsigned and either explain why it was intentional (in inline comments in this issue), or rewrite them to be consistent and explain what was changed and for what reasons. Right now there are a few implicit conversions between different sized types which may be perfectly fine, but reviewing each takes time when I suspect the type conversion are unnecessary and would be made consistent after you go through them. nicholas_rishel: To clarify this question: I'd appreciate if you could review when types are being converted to… | |||||
Done Inline ActionsYou are right, going from unsigned short to int and back to unsigned short is unnecessary. I overlooked those. samhocevar: You are right, going from `unsigned short` to `int` and back to `unsigned short` is unnecessary. | |||||
| { | |||||
| GHOST_SystemWin32 *system = (GHOST_SystemWin32 *)getSystem(); | |||||
| GHOST_EventKey *event; | |||||
| char utf8_char[6] = {0}; | |||||
| char ascii = 0; | |||||
| bool is_repeat = false; | |||||
| /* Unlike on Linux, not all keys can send repeat events. E.g. modifier keys don't. */ | |||||
| if (keyDown) { | |||||
| if (system->m_keycode_last_repeat_key == vk) { | |||||
| is_repeat = true; | |||||
| } | } | ||||
| else { | system->m_keycode_last_repeat_key = vk; | ||||
| if (system->m_keycode_last_repeat_key == vk) { | } | ||||
| system->m_keycode_last_repeat_key = 0; | else { | ||||
| } | if (system->m_keycode_last_repeat_key == vk) { | ||||
| system->m_keycode_last_repeat_key = 0; | |||||
| } | } | ||||
| } | |||||
| wchar_t utf16[3] = {0}; | wchar_t utf16[3] = {0}; | ||||
| BYTE state[256] = {0}; | BYTE state[256] = {0}; | ||||
| int r; | int r; | ||||
| GetKeyboardState((PBYTE)state); | GetKeyboardState((PBYTE)state); | ||||
| bool ctrl_pressed = state[VK_CONTROL] & 0x80; | bool ctrl_pressed = state[VK_CONTROL] & 0x80; | ||||
| bool alt_pressed = state[VK_MENU] & 0x80; | bool alt_pressed = state[VK_MENU] & 0x80; | ||||
| /* No text with control key pressed (Alt can be used to insert special characters though!). */ | /* No text with control key pressed (Alt can be used to insert special characters though!). */ | ||||
| if (ctrl_pressed && !alt_pressed) { | if (ctrl_pressed && !alt_pressed) { | ||||
| utf8_char[0] = '\0'; | utf8_char[0] = '\0'; | ||||
| } | } | ||||
| // Don't call ToUnicodeEx on dead keys as it clears the buffer and so won't allow diacritical | // Don't call ToUnicodeEx on dead keys as it clears the buffer and so won't allow diacritical | ||||
| // composition. | // composition. | ||||
| else if (MapVirtualKeyW(vk, 2) != 0) { | else if (vk == VK_PACKET || MapVirtualKeyW(vk, MAPVK_VK_TO_CHAR) != 0) { | ||||
Not Done Inline ActionsJust to be sure, MapVirtualKeyW does not return the expected value if vk is VK_PACKET? mano-wii: Just to be sure, `MapVirtualKeyW` does not return the expected value if `vk` is `VK_PACKET`? | |||||
Done Inline ActionsIndeed, it will return 0. That function relies on the current keyboard layout (this is not documented but can be confirmed experimentally and is a well known fact), not the current input state (contrary to ToUnicodeEx which is used later in the function), so it cannot map VK_PACKET to multiple different Unicode characters. samhocevar: Indeed, it will return `0`. That function relies on the current keyboard layout (this is not… | |||||
Not Done Inline Actions@Germano Cavalcante (mano-wii) VK_PACKET indicates the message contains a Unicode character but doesn't embed what that Unicode character is, whereas most (all?) other virtual keys meaningfully map to a single character for a given keyboard layout. @Sam Hocevar (samhocevar) I'd suggest reordering this to MapVirtualKeyW(vk, MAPVK_VK_TO_CHAR) != 0 || vk == VK_PACKET as the former is the common case. nicholas_rishel: @mano-wii VK_PACKET indicates the message contains a Unicode character but doesn't embed what… | |||||
| // todo: ToUnicodeEx can respond with up to 4 utf16 chars (only 2 here). | // todo: ToUnicodeEx can respond with up to 4 utf16 chars (only 2 here). | ||||
| // Could be up to 24 utf8 bytes. | // Could be up to 24 utf8 bytes. | ||||
| if ((r = ToUnicodeEx( | if ((r = ToUnicodeEx(vk, scanCode, state, utf16, 2, 0, system->m_keylayout))) { | ||||
| vk, raw.data.keyboard.MakeCode, state, utf16, 2, 0, system->m_keylayout))) { | if ((r > 0 && r < 3)) { | ||||
| if ((r > 0 && r < 3)) { | utf16[r] = 0; | ||||
| utf16[r] = 0; | conv_utf_16_to_8(utf16, utf8_char, 6); | ||||
| conv_utf_16_to_8(utf16, utf8_char, 6); | } | ||||
| } | else if (r == -1) { | ||||
| else if (r == -1) { | utf8_char[0] = '\0'; | ||||
| utf8_char[0] = '\0'; | |||||
| } | |||||
| } | } | ||||
| } | } | ||||
| } | |||||
| if (!keyDown) { | if (!keyDown) { | ||||
| utf8_char[0] = '\0'; | utf8_char[0] = '\0'; | ||||
| ascii = '\0'; | ascii = '\0'; | ||||
| } | |||||
| else { | |||||
| ascii = utf8_char[0] & 0x80 ? '?' : utf8_char[0]; | |||||
| } | |||||
| event = new GHOST_EventKey(system->getMilliSeconds(), | |||||
| keyDown ? GHOST_kEventKeyDown : GHOST_kEventKeyUp, | |||||
| window, | |||||
| key, | |||||
| ascii, | |||||
| utf8_char, | |||||
| is_repeat); | |||||
| // GHOST_PRINTF("%c\n", ascii); // we already get this info via EventPrinter | |||||
| } | } | ||||
| else { | else { | ||||
| event = NULL; | ascii = utf8_char[0] & 0x80 ? '?' : utf8_char[0]; | ||||
| } | } | ||||
| event = new GHOST_EventKey(system->getMilliSeconds(), | |||||
| keyDown ? GHOST_kEventKeyDown : GHOST_kEventKeyUp, | |||||
| window, | |||||
| key, | |||||
| ascii, | |||||
| utf8_char, | |||||
| is_repeat); | |||||
| // GHOST_PRINTF("%c\n", ascii); // we already get this info via EventPrinter | |||||
| return event; | return event; | ||||
| } | } | ||||
| Context not available. | |||||
| switch (raw.header.dwType) { | switch (raw.header.dwType) { | ||||
| case RIM_TYPEKEYBOARD: | case RIM_TYPEKEYBOARD: | ||||
| event = processKeyEvent(window, raw); | event = processRawKeyEvent(window, raw); | ||||
| if (!event) { | if (!event) { | ||||
| GHOST_PRINT("GHOST_SystemWin32::wndProc: key event "); | GHOST_PRINT("GHOST_SystemWin32::wndProc: key event "); | ||||
| GHOST_PRINT(msg); | GHOST_PRINT(msg); | ||||
| Context not available. | |||||
| } | } | ||||
| #endif /* WITH_INPUT_IME */ | #endif /* WITH_INPUT_IME */ | ||||
| //////////////////////////////////////////////////////////////////////// | //////////////////////////////////////////////////////////////////////// | ||||
| // Keyboard events, ignored | // Keyboard events, processed if synthesized | ||||
| //////////////////////////////////////////////////////////////////////// | //////////////////////////////////////////////////////////////////////// | ||||
| case WM_KEYDOWN: | case WM_KEYDOWN: | ||||
| case WM_SYSKEYDOWN: | |||||
| case WM_KEYUP: | case WM_KEYUP: | ||||
| if (wParam == VK_PACKET) { | |||||
| /* The VK_PACKET virtual key is a synthesised event, most likely from third-party | |||||
| * input software (AutoHotKey, WinCompose), that will not be received by the WM_INPUT | |||||
| * handler, so it is best to handle it here. | |||||
| */ | |||||
| bool keyDown = msg == WM_KEYDOWN; | |||||
| unsigned short scanCode = HIWORD(lParam) & 0xff; | |||||
Done Inline ActionsCould you link me to the MSDN which explains this shift? From https://docs.microsoft.com/en-us/windows/win32/inputdev/wm-keydown it looks like only bits 16-23 are valid for the scan code. nicholas_rishel: Could you link me to the MSDN which explains this shift? From https://docs.microsoft.com/en… | |||||
Done Inline ActionsYou’re right, this isn’t good. I got confused by GTK+ which does the same, I guess they’re looking for trouble, too. Fixed. samhocevar: You’re right, this isn’t good. I got confused by GTK+ which does the same, I guess they’re… | |||||
| event = processKeyEvent(window, keyDown, GHOST_kKeyUnknown, VK_PACKET, scanCode); | |||||
Not Done Inline Actionshttps://docs.microsoft.com/en-us/windows/win32/inputdev/virtual-key-codes
Do you have an intuition for what this means? I'm guessing it just means the virtual key is the low word of wParam in keydown/up, which is true for all virtual keys? nicholas_rishel: https://docs.microsoft.com/en-us/windows/win32/inputdev/virtual-key-codes
> The VK_PACKET key… | |||||
| } | |||||
| break; | |||||
Done Inline ActionsThese should be moved out of the Keyboard events, ignored commented section. nicholas_rishel: These should be moved out of the `Keyboard events, ignored` commented section. | |||||
| //////////////////////////////////////////////////////////////////////// | |||||
| // Keyboard events, ignored | |||||
| //////////////////////////////////////////////////////////////////////// | |||||
| case WM_SYSKEYDOWN: | |||||
| case WM_SYSKEYUP: | case WM_SYSKEYUP: | ||||
| /* These functions were replaced by WM_INPUT*/ | /* These functions were replaced by WM_INPUT*/ | ||||
Done Inline ActionsFind and replace error? nicholas_rishel: Find and replace error? | |||||
Done Inline ActionsI just thought the sentence wasn’t very clear here. MSDN says both “The WM_CHAR message contains the character code of the key” and “wParam — The character code of the key” and I thought stating the first part without clarifying with the second part was omitting valuable information. samhocevar: I just thought the sentence wasn’t very clear here. [MSDN](https://docs.microsoft.com/en… | |||||
| case WM_CHAR: | case WM_CHAR: | ||||
| /* The WM_CHAR message is posted to the window with the keyboard focus when | /* The WM_CHAR message is posted to the window with the keyboard focus when | ||||
| * a WM_KEYDOWN message is translated by the TranslateMessage function. WM_CHAR | * a WM_KEYDOWN message is translated by the TranslateMessage function. wParam | ||||
| * contains the character code of the key that was pressed. | * contains the character code of the key that was pressed. | ||||
| */ | */ | ||||
| case WM_DEADCHAR: | case WM_DEADCHAR: | ||||
| Context not available. | |||||
No longer ignored, needs explanation.