Page MenuHome
Differential D10248 Diff 33378 ghost/intern/GHOST_SystemWin32.cpp

Changeset View

Standalone View

View Options

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, we capture
nicholas_rishelUnsubmitted
Done
Inline Actions

No longer ignored, needs explanation.

nicholas_rishel: No longer ignored, needs explanation.
nicholas_rishelUnsubmitted
Done
Inline Actions

No longer ignored, needs explanation.

nicholas_rishel: > No longer ignored, needs explanation.
* those events here as well. */ * those events here as well. */
mano-wiiUnsubmitted
Not Done
Inline Actions

For me that description needs a little more clarification.

  • Where are these messages ignored?
  • What are hardware keys? Are they the physical keys on a keyboard? So what are the other types of keys that are not "ignored"?

(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 Actions

I 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,
int vk,
int scanCode)
nicholas_rishelUnsubmitted
Done
Inline Actions

Should these be unsigned shorts?

nicholas_rishel: Should these be unsigned shorts?
nicholas_rishelUnsubmitted
Done
Inline Actions

To 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…
samhocevarAuthorUnsubmitted
Done
Inline Actions

You 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) {
// 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).
mano-wiiUnsubmitted
Not Done
Inline Actions

Just 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`?
samhocevarAuthorUnsubmitted
Done
Inline Actions

Indeed, 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…
nicholas_rishelUnsubmitted
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…
// 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.
// Keyboard events, ignored // Keyboard events, ignored
//////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////
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;
int scanCode = lParam >> 16;
nicholas_rishelUnsubmitted
Done
Inline Actions

Could 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…
samhocevarAuthorUnsubmitted
Done
Inline Actions

You’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);
}
nicholas_rishelUnsubmitted
Not Done
Inline Actions

https://docs.microsoft.com/en-us/windows/win32/inputdev/virtual-key-codes

The VK_PACKET key is the low word of a 32-bit Virtual Key value used for non-keyboard input methods.

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;
nicholas_rishelUnsubmitted
Done
Inline Actions

These 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.
case WM_SYSKEYDOWN:
case WM_SYSKEYUP: case WM_SYSKEYUP:
/* These functions were replaced by WM_INPUT*/ /* These functions were replaced by WM_INPUT*/
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
nicholas_rishelUnsubmitted
Done
Inline Actions

Find and replace error?

nicholas_rishel: Find and replace error?
samhocevarAuthorUnsubmitted
Done
Inline Actions

I 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…
* 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.