From 651ba9f236162b2649bec0417c409aafeb0079ca Mon Sep 17 00:00:00 2001 From: Ross Date: Wed, 26 Oct 2022 15:34:12 +1000 Subject: [PATCH] feat(windows): convert wm keyevent and to private wm user message wm_keyman_keyevent and wm_keyman_modifierevent have been convert to use the WM_USER message range and no longer global message --- windows/src/engine/keyman32/globals.h | 4 +- windows/src/engine/keyman32/k32_dbg.cpp | 5 +- windows/src/engine/keyman32/k32_globals.cpp | 1 - .../keyman32/k32_lowlevelkeyboardhook.cpp | 2 +- windows/src/engine/keyman32/keyman32.cpp | 22 ++++--- .../src/engine/keyman32/kmhook_getmessage.cpp | 3 +- .../engine/keyman32/serialkeyeventcommon.h | 7 +++ .../engine/keyman32/serialkeyeventserver.cpp | 59 +++++++++++-------- 8 files changed, 57 insertions(+), 46 deletions(-) diff --git a/windows/src/engine/keyman32/globals.h b/windows/src/engine/keyman32/globals.h index 222b15e431..f7d791ab1e 100644 --- a/windows/src/engine/keyman32/globals.h +++ b/windows/src/engine/keyman32/globals.h @@ -278,9 +278,7 @@ extern UINT wm_keymanshift, wm_keymanim_close, wm_keymanim_contextchanged, - wm_test_keyman_functioning, - wm_keyman_keyevent; // for serialized input - + wm_test_keyman_functioning; extern BOOL flag_ShouldSerializeInput; diff --git a/windows/src/engine/keyman32/k32_dbg.cpp b/windows/src/engine/keyman32/k32_dbg.cpp index dedb263bf2..39c87da96d 100644 --- a/windows/src/engine/keyman32/k32_dbg.cpp +++ b/windows/src/engine/keyman32/k32_dbg.cpp @@ -44,6 +44,7 @@ 09 Aug 2015 - mcdurdin - I4843 - Log reported modifier state as well as Keyman current modifier state */ #include "pch.h" +#include "serialkeyeventcommon.h" #include #include #include @@ -206,8 +207,8 @@ void DebugMessage(LPMSG msg, WPARAM wParam) // I2908 else if(msg->message == wm_keymankeyup) wsprintf(ds, "DebugMessage(%x, wm_keymankeyup: %s lParam: %X) [message flags: %x time: %d]", PtrToInt(msg->hwnd), Debug_VirtualKey((WORD) msg->wParam), (unsigned int) msg->lParam, wParam, (int) msg->time); - else if (msg->message == wm_keyman_keyevent) - wsprintf(ds, "DebugMessage(%x, wm_keyman_keyevent: %s lParam: %X) [message flags: %x time: %d]", PtrToInt(msg->hwnd), + else if (msg->message == WM_KEYMAN_KEY_EVENT) + wsprintf(ds, "DebugMessage(%x, WM_KEYMAN_KEYEVENT: %s lParam: %X) [message flags: %x time: %d]", PtrToInt(msg->hwnd), Debug_VirtualKey((WORD)msg->wParam), (unsigned int) msg->lParam, wParam, (int) msg->time); else if(msg->message == WM_KEYDOWN || msg->message == WM_KEYUP || msg->message == WM_SYSKEYDOWN || msg->message == WM_SYSKEYUP) wsprintf(ds, "DebugMessage(%x, %s, wParam: %s, lParam: %X) [message flags: %x time: %d extra: %x]", diff --git a/windows/src/engine/keyman32/k32_globals.cpp b/windows/src/engine/keyman32/k32_globals.cpp index 5bdf1b23c6..054945b812 100644 --- a/windows/src/engine/keyman32/k32_globals.cpp +++ b/windows/src/engine/keyman32/k32_globals.cpp @@ -72,7 +72,6 @@ UINT //TODO: consolidate these messages -- they are probably not all required now wm_keyman = 0, // user message - ignore msg // I3594 - wm_keyman_keyevent = 0, // for serialized input wm_kmdebug = 0, // " " " " - debugging wm_keymankeydown = 0, diff --git a/windows/src/engine/keyman32/k32_lowlevelkeyboardhook.cpp b/windows/src/engine/keyman32/k32_lowlevelkeyboardhook.cpp index 1408704f46..379b79e404 100644 --- a/windows/src/engine/keyman32/k32_lowlevelkeyboardhook.cpp +++ b/windows/src/engine/keyman32/k32_lowlevelkeyboardhook.cpp @@ -275,7 +275,7 @@ LRESULT _kmnLowLevelKeyboardProc( HWND hwnd = gui.hwndFocus ? gui.hwndFocus : gui.hwndActive; if (!IsConsoleWindow(hwnd)) { - PostMessage(ISerialKeyEventServer::GetServer()->GetWindow(), wm_keyman_keyevent, hs->vkCode, LLKHFFlagstoWMKeymanKeyEventFlags(hs)); + PostMessage(ISerialKeyEventServer::GetServer()->GetWindow(), WM_KEYMAN_KEY_EVENT, hs->vkCode, LLKHFFlagstoWMKeymanKeyEventFlags(hs)); return 1; } //else SendDebugMessageFormat(0, sdmGlobal, 0, "LowLevelHook: console window, not serializing"); // too noisy diff --git a/windows/src/engine/keyman32/keyman32.cpp b/windows/src/engine/keyman32/keyman32.cpp index d8c3ac44a0..14536c4556 100644 --- a/windows/src/engine/keyman32/keyman32.cpp +++ b/windows/src/engine/keyman32/keyman32.cpp @@ -208,17 +208,16 @@ void DoChangeWindowMessageFilter() } DoCWMF(wm_keyman); // I3594 - DoCWMF(wm_keyman_keyevent); - DoCWMF(wm_keymankeydown); - DoCWMF(wm_keymankeyup); - DoCWMF(wm_keyman_grabwindowproc); - DoCWMF(wm_keyman_refresh); - DoCWMF(wm_kmgetactivekeymanid); - DoCWMF(wm_keymanim_close); - DoCWMF(wm_keymanim_contextchanged); - DoCWMF(wm_keymanshift); - DoCWMF(wm_keyman_control); // I4714 - DoCWMF(wm_keyman_control_internal); // I4714 + DoCWMF(wm_keymankeydown); + DoCWMF(wm_keymankeyup); + DoCWMF(wm_keyman_grabwindowproc); + DoCWMF(wm_keyman_refresh); + DoCWMF(wm_kmgetactivekeymanid); + DoCWMF(wm_keymanim_close); + DoCWMF(wm_keymanim_contextchanged); + DoCWMF(wm_keymanshift); + DoCWMF(wm_keyman_control); // I4714 + DoCWMF(wm_keyman_control_internal); // I4714 FreeLibrary(hUser32); } @@ -300,7 +299,6 @@ BOOL InitialiseProcess(HWND hwnd) Initialise_Flag_ShouldSerializeInput(); wm_keyman = RegisterWindowMessage(RWM_KEYMAN); - wm_keyman_keyevent = RegisterWindowMessage("WM_KEYMAN_KEYEVENT"); wm_keymankeydown = RegisterWindowMessage("WM_KEYMANKEYDOWN"); wm_keymankeyup = RegisterWindowMessage("WM_KEYMANKEYUP"); wm_keyman_grabwindowproc = RegisterWindowMessage("WM_KEYMAN_GRABWINDOWPROC"); diff --git a/windows/src/engine/keyman32/kmhook_getmessage.cpp b/windows/src/engine/keyman32/kmhook_getmessage.cpp index 30785df7bb..f6a56340f9 100644 --- a/windows/src/engine/keyman32/kmhook_getmessage.cpp +++ b/windows/src/engine/keyman32/kmhook_getmessage.cpp @@ -63,6 +63,7 @@ */ // I3583 // I4287 #include "pch.h" +#include "serialkeyeventcommon.h" void ProcessWMKeymanControlInternal(HWND hwnd, WPARAM wParam, LPARAM lParam); void ProcessWMKeymanControl(WPARAM wParam, LPARAM lParam); @@ -141,7 +142,7 @@ LRESULT _kmnGetMessageProc(int nCode, WPARAM wParam, LPARAM lParam) } if (((mp->message >= WM_KEYFIRST && mp->message <= WM_KEYLAST) || mp->message == wm_keymankeydown || mp->message == wm_keymankeyup || - mp->message == wm_keyman_keyevent) + mp->message == WM_KEYMAN_KEY_EVENT) && ShouldDebug(sdmMessage)) { DebugMessage(mp, wParam); } diff --git a/windows/src/engine/keyman32/serialkeyeventcommon.h b/windows/src/engine/keyman32/serialkeyeventcommon.h index dae5070e2c..d48bcff131 100644 --- a/windows/src/engine/keyman32/serialkeyeventcommon.h +++ b/windows/src/engine/keyman32/serialkeyeventcommon.h @@ -16,6 +16,11 @@ #define GLOBAL_KEY_EVENT_NAME "KeymanEngine_KeyEvent" #define GLOBAL_KEY_MUTEX_NAME "KeymanEngine_KeyMutex" +/** +WM_USER private messages +*/ +#define WM_KEYMAN_KEY_EVENT (WM_USER + 1) +#define WM_KEYMAN_MODIFIER_EVENT (WM_USER + 2) /** The INPUT structure and the KEYBDINPUT structure both vary in size between x86 and x64 because of the presence of the ULONG_PTR member dwExtraInfo. Thus we need to maintain an @@ -34,3 +39,5 @@ struct SerialKeyEventSharedData { DWORD nInputs; CSDINPUT inputs[MAX_KEYEVENT_INPUTS]; }; + + diff --git a/windows/src/engine/keyman32/serialkeyeventserver.cpp b/windows/src/engine/keyman32/serialkeyeventserver.cpp index bf10694176..04659d1d63 100644 --- a/windows/src/engine/keyman32/serialkeyeventserver.cpp +++ b/windows/src/engine/keyman32/serialkeyeventserver.cpp @@ -11,21 +11,21 @@ due to restricted permissions, all SendInput must be done from this thread, which runs in the Keyman main process. - NOTE: Postponing writing architecture technical note because of change to architecture + NOTE: Postponing writing architecture technical note because of change to architecture below... - TODO: For simplicity of proof-of-concept data sharing, we ran two copies of the key event - sender thread: one in the 32 bit space, and one in the 64 bit space. This means that we + TODO: For simplicity of proof-of-concept data sharing, we ran two copies of the key event + sender thread: one in the 32 bit space, and one in the 64 bit space. This means that we can still have a race condition because we lose serialization guarantees. Input is first processed in the Low Level Keyboard Hook which runs in the keyman.exe 32 bit space. This then gets forwarded to the target application with the necessary flags on the message to - tell Keyman to not reprocess it. However, after keystroke processing, the target - application fills the shared data structure and signals the key event sender thread in its - own bitness space (32 or 64 bit). The key event sender thread then takes the final shared - data and sends it to the target app. And that breaks the serialization guarantee because + tell Keyman to not reprocess it. However, after keystroke processing, the target + application fills the shared data structure and signals the key event sender thread in its + own bitness space (32 or 64 bit). The key event sender thread then takes the final shared + data and sends it to the target app. And that breaks the serialization guarantee because the 64 bit apps are not serialized with the original 32 bit captured input. - The fix is to redesign the shared data to use a memory mapped file, which can be shared + The fix is to redesign the shared data to use a memory mapped file, which can be shared across the 32-64 boundary. Must tweak the permissions on this file, of course. TODO: Console apps still not working @@ -73,7 +73,7 @@ public: m_pInputs = NULL; m_pSharedData = NULL; - // We create the file mapping and global data on the main thread but release it on the + // We create the file mapping and global data on the main thread but release it on the // local thread. This ensures that these objects are available for other processes to // open even if we haven't completed startup of the local thread. if (!InitSharedData()) { @@ -117,7 +117,7 @@ public: // Normally, this is cleaned up by thread termination, but this // handles error conditions better - CloseSharedData(); + CloseSharedData(); } virtual HWND GetWindow() const { @@ -295,7 +295,7 @@ private: /** Main message loop for thread. Terminates on error or when - m_hThreadExitEvent is signaled. Sleeps until either a + m_hThreadExitEvent is signaled. Sleeps until either a window message is received or a key event is signaled from a client app. */ @@ -332,7 +332,7 @@ private: HANDLE handles[2] = { m_hThreadExitEvent, m_hKeyMutex }; // - // Wait for access to the shared data (must also watch out for + // Wait for access to the shared data (must also watch out for // shutdown event so we don't stall forever here) // switch (WaitForMultipleObjects(2, handles, FALSE, INFINITE)) { @@ -377,7 +377,7 @@ private: } /** - Add modifier state adjustment events and then copy the new input + Add modifier state adjustment events and then copy the new input events from the shared buffer */ void PrepareInjectedInput() { @@ -406,7 +406,7 @@ private: if (server == NULL) { return DefWindowProc(hwnd, msg, wParam, lParam); } - + return server->WndProc(hwnd, msg, wParam, lParam); } @@ -436,15 +436,15 @@ private: You can disable this flag with flag_ShouldSerializeInput. */ - if (msg == wm_keyman_keyevent && flag_ShouldSerializeInput /*&& _td->lpActiveKeyboard*/) { + if ((msg == WM_KEYMAN_KEY_EVENT || msg == WM_KEYMAN_MODIFIER_EVENT) && flag_ShouldSerializeInput /*&& _td->lpActiveKeyboard*/) { if (wParam == VK_RMENU && (lParam & (KEYEVENTF_EXTENDEDKEY | KEYEVENTF_KEYUP)) == (KEYEVENTF_EXTENDEDKEY | KEYEVENTF_KEYUP) && GetKeyState(VK_LCONTROL) < 0) { - /* - When Windows has a European layout that uses AltGr installed, it can emit an additional LCtrl down via software + /* + When Windows has a European layout that uses AltGr installed, it can emit an additional LCtrl down via software when RAlt is pressed. However, the corresponding LCtrl up is never received, seemingly because when Keyman re-emits the LCtrl+RAlt, there are subtle differences in the event flags which we cannot duplicate -- specifically - the flag that emits WM_SYSKEYDOWN for the VK_LCONTROL, even though it is received before the VK_RALT event. It - appears that Windows figures this out by giving this VK_LCONTROL the scan code 0x21D instead of 0x1D. But we + the flag that emits WM_SYSKEYDOWN for the VK_LCONTROL, even though it is received before the VK_RALT event. It + appears that Windows figures this out by giving this VK_LCONTROL the scan code 0x21D instead of 0x1D. But we are unable to emit that scan code: Windows truncates the scan code sent through SendInput so that we can only send 0x1D. @@ -485,8 +485,13 @@ private: input[1].ki.dwExtraInfo = EXTRAINFO_FLAG_SERIALIZED_USER_KEY_EVENT; input[1].ki.dwFlags = lParam & 0xFFFF; - if (!SendInput(2, input, sizeof(INPUT))) { - DebugLastError("SendInput"); + if (msg == WM_KEYMAN_KEY_EVENT) { + // We track changes to modifiers with WM_KEYMAN_MODIFIER_EVENT, but only ever + // pass them on to the app when we receive them with the WM_KEYMAN_KEYEVENT + // message. + if (!SendInput(2, input, sizeof(INPUT))) { + DebugLastError("SendInput"); + } } UpdateLocalModifierState( @@ -510,8 +515,10 @@ private: input.ki.dwExtraInfo = EXTRAINFO_FLAG_SERIALIZED_USER_KEY_EVENT; input.ki.dwFlags = lParam & 0xFFFF; - if (!SendInput(1, &input, sizeof(INPUT))) { - DebugLastError("SendInput"); + if (msg == WM_KEYMAN_KEY_EVENT){ + if (!SendInput(1, &input, sizeof(INPUT))) { + DebugLastError("SendInput"); + } } UpdateLocalModifierState( @@ -527,10 +534,10 @@ private: } /** - When a physical key event is received by the serializer, we know that this will - reflect the key state that the app sees at the time that the input is sent. + When a physical key event is received by the serializer, we know that this will + reflect the key state that the app sees at the time that the input is sent. We maintain a local modifier state here rather than using GetKeyState because that - ensures that we are keeping the keyboard state consistent with our version of + ensures that we are keeping the keyboard state consistent with our version of reality. */ void UpdateLocalModifierState(BYTE bVk, BOOL fIsExtendedKey, BYTE bScan, BOOL fIsUp) {