From d8c232dc7c3e4d693db1f4bed08bd9029519c7d2 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Tue, 30 Jan 2024 14:07:55 +1000 Subject: [PATCH 01/10] feat(windows): refactor to use km_core_actions struct --- .../src/engine/keyman32/kmprocessactions.cpp | 233 +++++++----------- 1 file changed, 93 insertions(+), 140 deletions(-) diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index 1eecfa6807..08694e9004 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -6,56 +6,77 @@ */ #include "pch.h" -static BOOL processUnicodeChar(AITIP* app, const km_core_action_item* actionItem) { - if (Uni_IsSMP(actionItem->character)) { - app->QueueAction(QIT_CHAR, (Uni_UTF32ToSurrogate1(actionItem->character))); - app->QueueAction(QIT_CHAR, (Uni_UTF32ToSurrogate2(actionItem->character))); - } - else { - app->QueueAction(QIT_CHAR, actionItem->character); - } - return TRUE; -} - -static BOOL processMarker(AITIP* app, const km_core_action_item* actionItem) { - app->QueueAction(QIT_DEADKEY, (DWORD)actionItem->marker); - return TRUE; -} - -static BOOL processAlert(AITIP* app) { - app->QueueAction(QIT_BELL, 0); - return TRUE; -} - -static BOOL processBack(AITIP* app, const km_core_action_item* actionItem) { - if (actionItem->backspace.expected_type == KM_CORE_BT_MARKER) { - app->QueueAction(QIT_BACK, BK_DEADKEY); - } else if(actionItem->backspace.expected_type == KM_CORE_BT_CHAR) { - // If this is a TSF-aware app we need to set the BK_SURROGATE flag to delete - // both parts of the surrogate pair. Legacy apps receive a BKSP WM_KEYDOWN event - // which results in deleting both parts in one action. - if (!app->IsLegacy() && Uni_IsSMP(actionItem->backspace.expected_value)) { - app->QueueAction(QIT_BACK, BK_DEFAULT | BK_SURROGATE); +static void +processUnicodeChar(AITIP* app, const km_core_usv* actionItem) { + while (*actionItem) { + if (Uni_IsSMP(*actionItem)) { + app->QueueAction(QIT_CHAR, (Uni_UTF32ToSurrogate1(*actionItem))); + app->QueueAction(QIT_CHAR, (Uni_UTF32ToSurrogate2(*actionItem))); } else { - app->QueueAction(QIT_BACK, BK_DEFAULT); + app->QueueAction(QIT_CHAR, *actionItem); } - } else { // KM_CORE_BT_UNKNOWN - app->QueueAction(QIT_BACK, BK_DEFAULT); + actionItem++; } - return TRUE; } -static BOOL processPersistOpt( - const km_core_action_item* actionItem, +static void processAlert(AITIP* app) { + app->QueueAction(QIT_BELL, 0); +} + +static BOOL +processBack(AITIP* app, const unsigned int code_points_to_delete) { + + if (app->IsLegacy()) { + for (unsigned int i = 0; i < code_points_to_delete; i++) { + app->QueueAction(QIT_BACK, BK_DEFAULT); + } + return TRUE; + } + + if (!app->IsLegacy()) { + WCHAR application_context[MAXCONTEXT]; + DWORD cp_to_delete = code_points_to_delete; + if (!app->ReadContext(application_context)) { + SendDebugMessageFormat(0, sdmGlobal, 0, "processBack: Error reading context from application."); + } else { + // Find the length of the string + int length = 0; + while (application_context[length] != L'\0') { + length++; + } + // Read each character starting from caret + for (int i = length - 1; i >= 0 && cp_to_delete > 0; i--) { + // Need to consider malformed context where only one surrogate is present, at either the start or end of the context. + // In both these scenarios just perform one backspace one the same as if it was a non surrogate pair. + if ((i > 0) && (Uni_IsSurrogate1(application_context[i - 1]) && Uni_IsSurrogate2(application_context[i]))) { + app->QueueAction(QIT_BACK, BK_DEFAULT | BK_SURROGATE); + i--; + cp_to_delete--; + } + else { + app->QueueAction(QIT_BACK, BK_DEFAULT); + cp_to_delete--; + } + } + } + return TRUE; + } + return FALSE; +} + +static void +processPersistOpt(km_core_actions const* actions, km_core_state* keyboardState, LPINTKEYBOARDINFO activeKeyboard ) { - if (actionItem->option != NULL) - { + + + for (auto option = actions->persist_options; option->key; option++) { + // TODO: Do we need really to write the option back to the core? // Allocate for 1 option plus 1 pad struct of 0's for KM_CORE_IT_END km_core_option_item keyboardOpts[2] = { 0 }; - keyboardOpts[0].key = actionItem->option->key; - keyboardOpts[0].value = actionItem->option->value; + keyboardOpts[0].key = option->key; + keyboardOpts[0].value = option->value; km_core_status eventStatus = (km_core_status_codes)km_core_state_options_update(keyboardState, keyboardOpts); if (eventStatus != KM_CORE_STATUS_OK) { @@ -64,36 +85,26 @@ static BOOL processPersistOpt( } // Put the keyboard option into Windows Registry - if (actionItem->option != NULL && actionItem->option->key != NULL && - actionItem->option->value != NULL) - { - // log"Saving keyboard option to registry"); - SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessHook: Saving option to registry for keyboard [%s].", activeKeyboard->Name); - LPWSTR value = new WCHAR[sizeof(actionItem->option->value) + 1]; - wcscpy_s(value, sizeof(actionItem->option->value) + 1, reinterpret_cast(actionItem->option->value)); - SaveKeyboardOptionCoretoRegistry(activeKeyboard, reinterpret_cast(actionItem->option->key), value); - } + // log"Saving keyboard option to registry"); + SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessHook: Saving option to registry for keyboard [%s].", activeKeyboard->Name); + size_t value_length = wcslen(reinterpret_cast(option->value)); + LPWSTR value = new WCHAR[value_length + 1]; + wcscpy_s(value, value_length + 1, reinterpret_cast(option->value)); + SaveKeyboardOptionCoretoRegistry(activeKeyboard, reinterpret_cast(option->key), value); + delete[] value; + } - return TRUE; } -static BOOL processInvalidateContext( - AITIP* app -) { - app->ResetContext(); - return TRUE; -} - -static BOOL -processCapsLock(const km_core_action_item* actionItem, BOOL isUp, BOOL Updateable, BOOL externalEvent) { +static void processCapsLock(const km_core_caps_state caps_lock_state, BOOL isUp, BOOL Updateable, BOOL externalEvent) { // We only want to process the Caps Lock key event once -- // in the first pass (!Updateable). if (Updateable){ - return TRUE; + return; } - if (actionItem->capsLock) { + if (caps_lock_state == KM_CORE_CAPS_ON) { // This case would occur for the keyboard system store setting `store(&CapsOnOnly) '1'` if (isUp && !IsCapsLockOn()) { // I267 - 24/11/2006 invert GetKeyState test SendDebugMessageFormat(0, sdmGlobal, 0, "processCapsLock: TURN CAPS ON: FIsUp=%d CapsState=%d", isUp, IsCapsLockOn()); @@ -109,8 +120,7 @@ processCapsLock(const km_core_action_item* actionItem, BOOL isUp, BOOL Updateabl keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, KEYEVENTF_KEYUP, 0); keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, 0, 0); } - } - else { + } else if (caps_lock_state == KM_CORE_CAPS_OFF) { // This case would occur for the keyboard system store setting `store(&ShiftFreesCaps) '1'` // OR selecting a keyboard with CAPs always off rule if ((!isUp && IsCapsLockOn()) || (externalEvent && IsCapsLockOn())) { @@ -119,8 +129,6 @@ processCapsLock(const km_core_action_item* actionItem, BOOL isUp, BOOL Updateabl keybd_event(VK_CAPITAL, SCAN_FLAG_KEYMAN_KEY_EVENT, KEYEVENTF_KEYUP, 0); } } - - return TRUE; } BOOL ProcessActions(BOOL* emitKeyStroke) @@ -131,46 +139,20 @@ BOOL ProcessActions(BOOL* emitKeyStroke) _td->CoreProcessEventRun = FALSE; // Process the action items from the core. This actions will modify the windows context (AppContext). // Therefore it is not required to copy the context from the core to the windows context. + km_core_actions const* actions = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); - for (auto act = km_core_state_action_items(_td->lpActiveKeyboard->lpCoreKeyboardState, nullptr); act->type != KM_CORE_IT_END; act++) { - BOOL continueProcessingActions = TRUE; - SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActions : act->type=%d", act->type); - switch (act->type) { - case KM_CORE_IT_CHAR: - continueProcessingActions = processUnicodeChar(_td->app, act); - break; - case KM_CORE_IT_MARKER: - continueProcessingActions = processMarker(_td->app, act); - break; - case KM_CORE_IT_ALERT: - continueProcessingActions = processAlert(_td->app); - break; - case KM_CORE_IT_BACK: - continueProcessingActions = processBack(_td->app, act); - break; - case KM_CORE_IT_PERSIST_OPT: - continueProcessingActions = processPersistOpt(act, _td->lpActiveKeyboard->lpCoreKeyboardState, _td->lpActiveKeyboard); - break; - case KM_CORE_IT_EMIT_KEYSTROKE: - *emitKeyStroke = TRUE; - continueProcessingActions = TRUE; - break; - case KM_CORE_IT_INVALIDATE_CONTEXT: - continueProcessingActions = processInvalidateContext(_td->app); - break; - case KM_CORE_IT_CAPSLOCK: - continueProcessingActions = processCapsLock(act, !_td->state.isDown, _td->TIPFUpdateable, FALSE); - break; - case KM_CORE_IT_END: - // fallthrough - default: - assert(false); // NOT SUPPORTED - break; - } - if (!continueProcessingActions) { - return FALSE; - } + processBack(_td->app, actions->code_points_to_delete); + processUnicodeChar(_td->app, actions->output); + if (actions->persist_options != NULL) { + processPersistOpt(actions, _td->lpActiveKeyboard->lpCoreKeyboardState, _td->lpActiveKeyboard); } + if (actions->do_alert) { + processAlert(_td->app); + } + if (actions->emit_keystroke) { + *emitKeyStroke = TRUE; + } + processCapsLock(actions->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); return TRUE; } @@ -184,28 +166,14 @@ ProcessActionsNonUpdatableParse(BOOL* emitKeyStroke) { if (_td->TIPFUpdateable) { // ensure only run when not updateable return FALSE; } - _td->CoreProcessEventRun = TRUE; - BOOL continueProcessingActions = TRUE; - for (auto act = km_core_state_action_items(_td->lpActiveKeyboard->lpCoreKeyboardState, nullptr); act->type != KM_CORE_IT_END; act++) { - switch (act->type) { - case KM_CORE_IT_EMIT_KEYSTROKE: - *emitKeyStroke = TRUE; - SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActionsNonUpdatableParse EMIT_KEYSTROKE: act->type=[%d]", act->type); - continueProcessingActions = TRUE; - _td->CoreProcessEventRun = FALSE; // If we emit the key stroke on this parse we don't need the second parse - break; - case KM_CORE_IT_CAPSLOCK: - continueProcessingActions = processCapsLock(act, !_td->state.isDown, _td->TIPFUpdateable, FALSE); - break; - case KM_CORE_IT_INVALIDATE_CONTEXT: - continueProcessingActions = processInvalidateContext(_td->app); - break; - } - if (!continueProcessingActions) { - return FALSE; - } + km_core_actions const* acts = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); + processCapsLock(acts->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); + if (acts->emit_keystroke) { + *emitKeyStroke = TRUE; + SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActionsNonUpdatableParse EMIT_KEYSTROKE"); + _td->CoreProcessEventRun = FALSE; // If we emit the key stroke on this parse we don't need the second parse } return TRUE; } @@ -216,22 +184,7 @@ ProcessActionsExternalEvent() { if (!_td) { return FALSE; } - // Currently only a subset of actions are handled. - // Other actions will be added when needed. - BOOL continueProcessingActions = TRUE; - for (auto act = km_core_state_action_items(_td->lpActiveKeyboard->lpCoreKeyboardState, nullptr); act->type != KM_CORE_IT_END; - act++) { - switch (act->type) { - case KM_CORE_IT_CAPSLOCK: - continueProcessingActions = processCapsLock(act, !_td->state.isDown, FALSE, TRUE); - break; - case KM_CORE_IT_INVALIDATE_CONTEXT: - continueProcessingActions = processInvalidateContext(_td->app); - break; - } - if (!continueProcessingActions) { - return FALSE; - } - } + km_core_actions const* acts = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); + processCapsLock(acts->new_caps_lock_state, !_td->state.isDown, FALSE, TRUE); return TRUE; } From 1e554c8804418ec82acb1cfed0ed02ae7d0b7890 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Tue, 30 Jan 2024 15:42:17 +1000 Subject: [PATCH 02/10] feat(windows): don't write persisted option back to core When the action from the core is to persist an option there is no need to write that value back to the core as it has already been updated, by the core. --- .../src/engine/keyman32/kmprocessactions.cpp | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index 08694e9004..1e2c0c8709 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -65,25 +65,9 @@ processBack(AITIP* app, const unsigned int code_points_to_delete) { } static void -processPersistOpt(km_core_actions const* actions, - km_core_state* keyboardState, - LPINTKEYBOARDINFO activeKeyboard +processPersistOpt(km_core_actions const* actions, LPINTKEYBOARDINFO activeKeyboard ) { - - for (auto option = actions->persist_options; option->key; option++) { - // TODO: Do we need really to write the option back to the core? - // Allocate for 1 option plus 1 pad struct of 0's for KM_CORE_IT_END - km_core_option_item keyboardOpts[2] = { 0 }; - keyboardOpts[0].key = option->key; - keyboardOpts[0].value = option->value; - km_core_status eventStatus = (km_core_status_codes)km_core_state_options_update(keyboardState, keyboardOpts); - if (eventStatus != KM_CORE_STATUS_OK) - { - // log warning "problem saving option for km_core_keyboard"); - SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessHook: Error %d saving option for keyboard [%s].", eventStatus, activeKeyboard->Name); - } - // Put the keyboard option into Windows Registry // log"Saving keyboard option to registry"); SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessHook: Saving option to registry for keyboard [%s].", activeKeyboard->Name); @@ -92,7 +76,6 @@ processPersistOpt(km_core_actions const* actions, wcscpy_s(value, value_length + 1, reinterpret_cast(option->value)); SaveKeyboardOptionCoretoRegistry(activeKeyboard, reinterpret_cast(option->key), value); delete[] value; - } } @@ -144,7 +127,7 @@ BOOL ProcessActions(BOOL* emitKeyStroke) processBack(_td->app, actions->code_points_to_delete); processUnicodeChar(_td->app, actions->output); if (actions->persist_options != NULL) { - processPersistOpt(actions, _td->lpActiveKeyboard->lpCoreKeyboardState, _td->lpActiveKeyboard); + processPersistOpt(actions, _td->lpActiveKeyboard); } if (actions->do_alert) { processAlert(_td->app); From 118466f6b28741cb3a0c584547b4bcf1832b3e4d Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:31:59 +1000 Subject: [PATCH 03/10] feat(windows): use delete-context for backspace Use the delete_context memember of the action struct to check for surrogates and determine the code units to delete. --- .../src/engine/keyman32/kmprocessactions.cpp | 37 +++++-------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index 1e2c0c8709..b21942fc38 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -23,41 +23,24 @@ static void processAlert(AITIP* app) { app->QueueAction(QIT_BELL, 0); } -static BOOL -processBack(AITIP* app, const unsigned int code_points_to_delete) { +static BOOL +processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_usv* delete_context) { if (app->IsLegacy()) { for (unsigned int i = 0; i < code_points_to_delete; i++) { - app->QueueAction(QIT_BACK, BK_DEFAULT); + app->QueueAction(QIT_BACK, BK_DEFAULT); } return TRUE; } - if (!app->IsLegacy()) { - WCHAR application_context[MAXCONTEXT]; - DWORD cp_to_delete = code_points_to_delete; - if (!app->ReadContext(application_context)) { - SendDebugMessageFormat(0, sdmGlobal, 0, "processBack: Error reading context from application."); - } else { - // Find the length of the string - int length = 0; - while (application_context[length] != L'\0') { - length++; + while (*delete_context) { + if (Uni_IsSMP(*delete_context)) { + app->QueueAction(QIT_BACK, BK_DEFAULT | BK_SURROGATE); } - // Read each character starting from caret - for (int i = length - 1; i >= 0 && cp_to_delete > 0; i--) { - // Need to consider malformed context where only one surrogate is present, at either the start or end of the context. - // In both these scenarios just perform one backspace one the same as if it was a non surrogate pair. - if ((i > 0) && (Uni_IsSurrogate1(application_context[i - 1]) && Uni_IsSurrogate2(application_context[i]))) { - app->QueueAction(QIT_BACK, BK_DEFAULT | BK_SURROGATE); - i--; - cp_to_delete--; - } - else { - app->QueueAction(QIT_BACK, BK_DEFAULT); - cp_to_delete--; - } + else { + app->QueueAction(QIT_BACK, BK_DEFAULT); } + delete_context++; } return TRUE; } @@ -124,7 +107,7 @@ BOOL ProcessActions(BOOL* emitKeyStroke) // Therefore it is not required to copy the context from the core to the windows context. km_core_actions const* actions = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); - processBack(_td->app, actions->code_points_to_delete); + processBack(_td->app, actions->code_points_to_delete, actions->deleted_context); processUnicodeChar(_td->app, actions->output); if (actions->persist_options != NULL) { processPersistOpt(actions, _td->lpActiveKeyboard); From 2b0950617d5645816e2711e4ea830b6dbdf4f7ee Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Thu, 1 Feb 2024 15:11:23 +1000 Subject: [PATCH 04/10] feat(windows): add cached actions --- windows/src/engine/keyman32/globals.h | 2 + windows/src/engine/keyman32/kmprocess.cpp | 2 + .../src/engine/keyman32/kmprocessactions.cpp | 130 +++++++++++++++--- 3 files changed, 115 insertions(+), 19 deletions(-) diff --git a/windows/src/engine/keyman32/globals.h b/windows/src/engine/keyman32/globals.h index 8f1dfc4c90..e0f86df07c 100644 --- a/windows/src/engine/keyman32/globals.h +++ b/windows/src/engine/keyman32/globals.h @@ -212,6 +212,8 @@ typedef struct tagKEYMAN64THREADDATA BOOL TIPFUpdateable, TIPFPreserved; // I4290 BOOL CoreProcessEventRun; // True if core process event has been run + // TODO: #10583 remove core_actions cache + km_core_actions const *core_actions; BOOL FInRefreshKeyboards; BOOL RefreshRequired; diff --git a/windows/src/engine/keyman32/kmprocess.cpp b/windows/src/engine/keyman32/kmprocess.cpp index c95c7ce5fd..b0c77d376d 100644 --- a/windows/src/engine/keyman32/kmprocess.cpp +++ b/windows/src/engine/keyman32/kmprocess.cpp @@ -83,6 +83,8 @@ Process_Event_Core(PKEYMAN64THREADDATA _td) { if (_td->app->ReadContext(application_context)) { km_core_context_status result; result = km_core_state_context_set_if_needed(_td->lpActiveKeyboard->lpCoreKeyboardState, reinterpret_cast(application_context)); + // TODO remove debuging set if needed + SendDebugMessageFormat(0, sdmGlobal, 0, "Process_Event_Core: km_core_state_context_set_if_needed returned [%d]", result); if (result == KM_CORE_CONTEXT_STATUS_ERROR || result == KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT) { SendDebugMessageFormat(0, sdmGlobal, 0, "Process_Event_Core: km_core_state_context_set_if_needed returned [%d]", result); } diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index b21942fc38..f37d0827c6 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -23,37 +23,105 @@ static void processAlert(AITIP* app) { app->QueueAction(QIT_BELL, 0); } +// TODO: #10583 +// Remove extra debuging code +static void +processDebugDeleteContext(const km_core_usv* delete_context) { + km_core_usv const* delete_context_ptr = delete_context; + while (*delete_context_ptr) { + delete_context_ptr++; + } + delete_context_ptr--; + for (; delete_context_ptr >= delete_context; delete_context_ptr--) { + if (Uni_IsSMP(*delete_context_ptr)) { + SendDebugMessageFormat(0, sdmGlobal, 0, "processDebugContext: s1 U+%04x", Uni_UTF32ToSurrogate1(*delete_context_ptr)); + SendDebugMessageFormat(0, sdmGlobal, 0, "processDebugContext: s2 U+%04x", Uni_UTF32ToSurrogate2(*delete_context_ptr)); + } else { + SendDebugMessageFormat(0, sdmGlobal, 0, "processDebugContext: U+%04x", *delete_context_ptr); + } + } + +} static BOOL processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_usv* delete_context) { if (app->IsLegacy()) { + SendDebugMessageFormat(0, sdmGlobal, 0, "processBack: Legacy app cptd [%d].", code_points_to_delete); for (unsigned int i = 0; i < code_points_to_delete; i++) { app->QueueAction(QIT_BACK, BK_DEFAULT); } return TRUE; } if (!app->IsLegacy()) { - while (*delete_context) { - if (Uni_IsSMP(*delete_context)) { + SendDebugMessageFormat(0, sdmGlobal, 0, "processBack: TSF app cptd [%d].", code_points_to_delete); + km_core_usv const* delete_context_ptr = delete_context; + while (*delete_context_ptr) { + delete_context_ptr++; + } + delete_context_ptr--; + for (; delete_context_ptr >= delete_context; delete_context_ptr--) { + if (Uni_IsSMP(*delete_context_ptr)) { app->QueueAction(QIT_BACK, BK_DEFAULT | BK_SURROGATE); } else { app->QueueAction(QIT_BACK, BK_DEFAULT); } - delete_context++; } return TRUE; } return FALSE; } +// TODO remove reading the applications context +static BOOL +processBack2(AITIP* app, const unsigned int code_points_to_delete) { + if (app->IsLegacy()) { + SendDebugMessageFormat(0, sdmGlobal, 0, "processBack2: Legacy app cptd [%d].", code_points_to_delete); + for (unsigned int i = 0; i < code_points_to_delete; i++) { + //app->QueueAction(QIT_BACK, BK_DEFAULT); + } + return TRUE; + } + + if (!app->IsLegacy()) { + SendDebugMessageFormat(0, sdmGlobal, 0, "processBack2: TSF app cptd [%d].", code_points_to_delete); + WCHAR application_context[MAXCONTEXT]; + DWORD cp_to_delete = code_points_to_delete; + if (!app->ReadContext(application_context)) { + SendDebugMessageFormat(0, sdmGlobal, 0, "processBack2: Error reading context from application."); + } else { + // Find the length of the string + int length = 0; + while (application_context[length] != L'\0') { + length++; + } + // Read each character starting from caret + for (int i = length - 1; i >= 0 && cp_to_delete > 0; i--) { + // Need to consider malformed context where only one surrogate is present, at either the start or end of the context. + // In both these scenarios just perform one backspace one the same as if it was a non surrogate pair. + if ((i > 0) && (Uni_IsSurrogate1(application_context[i - 1]) && Uni_IsSurrogate2(application_context[i]))) { + //app->QueueAction(QIT_BACK, BK_DEFAULT | BK_SURROGATE); + i--; + cp_to_delete--; + } else { + //app->QueueAction(QIT_BACK, BK_DEFAULT); + cp_to_delete--; + } + } + } + return TRUE; + } + + return FALSE; +} + static void processPersistOpt(km_core_actions const* actions, LPINTKEYBOARDINFO activeKeyboard ) { for (auto option = actions->persist_options; option->key; option++) { // Put the keyboard option into Windows Registry // log"Saving keyboard option to registry"); - SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessHook: Saving option to registry for keyboard [%s].", activeKeyboard->Name); + SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessPersistOpt: Saving option to registry for keyboard [%s].", activeKeyboard->Name); size_t value_length = wcslen(reinterpret_cast(option->value)); LPWSTR value = new WCHAR[value_length + 1]; wcscpy_s(value, value_length + 1, reinterpret_cast(option->value)); @@ -101,24 +169,38 @@ BOOL ProcessActions(BOOL* emitKeyStroke) { PKEYMAN64THREADDATA _td = ThreadGlobals(); if (!_td) return FALSE; + SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActions: Enter"); + + // TODO: #10583 Remove caching action_struct + if (!_td->core_actions) { + SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActions: core_actions not set"); + _td->core_actions = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); + } _td->CoreProcessEventRun = FALSE; + + + // Process the action items from the core. This actions will modify the windows context (AppContext). // Therefore it is not required to copy the context from the core to the windows context. - km_core_actions const* actions = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); - - processBack(_td->app, actions->code_points_to_delete, actions->deleted_context); - processUnicodeChar(_td->app, actions->output); - if (actions->persist_options != NULL) { - processPersistOpt(actions, _td->lpActiveKeyboard); + processDebugDeleteContext(_td->core_actions->deleted_context); + processBack(_td->app, _td->core_actions->code_points_to_delete, _td->core_actions->deleted_context); + processBack2(_td->app, _td->core_actions->code_points_to_delete); + processUnicodeChar(_td->app, _td->core_actions->output); + if (_td->core_actions->persist_options != NULL) { + processPersistOpt(_td->core_actions, _td->lpActiveKeyboard); } - if (actions->do_alert) { + if (_td->core_actions->do_alert) { processAlert(_td->app); } - if (actions->emit_keystroke) { + if (_td->core_actions->emit_keystroke) { *emitKeyStroke = TRUE; } - processCapsLock(actions->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); + processCapsLock(_td->core_actions->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); + // TODO: #10583 remove dispose + km_core_actions_dispose(_td->core_actions); + _td->core_actions = nullptr; + return TRUE; } @@ -128,15 +210,21 @@ ProcessActionsNonUpdatableParse(BOOL* emitKeyStroke) { if (!_td) { return FALSE; } - + SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActionsNonUpdatableParse: Enter"); if (_td->TIPFUpdateable) { // ensure only run when not updateable return FALSE; } _td->CoreProcessEventRun = TRUE; + // TODO: #10583 remove dispose + if (_td->core_actions) { + km_core_actions_dispose(_td->core_actions); + _td->core_actions = nullptr; + } - km_core_actions const* acts = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); - processCapsLock(acts->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); - if (acts->emit_keystroke) { + _td->core_actions = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); + SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActionsNonUpdatableParse: km_core_state_get_actions"); + processCapsLock(_td->core_actions->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); + if (_td->core_actions->emit_keystroke) { *emitKeyStroke = TRUE; SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActionsNonUpdatableParse EMIT_KEYSTROKE"); _td->CoreProcessEventRun = FALSE; // If we emit the key stroke on this parse we don't need the second parse @@ -150,7 +238,11 @@ ProcessActionsExternalEvent() { if (!_td) { return FALSE; } - km_core_actions const* acts = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); - processCapsLock(acts->new_caps_lock_state, !_td->state.isDown, FALSE, TRUE); + if (!_td->core_actions) { // when ideponent we will not need this + return FALSE; + } + // TODO: #10583 remove dispose + //km_core_actions const* acts = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); + processCapsLock(_td->core_actions->new_caps_lock_state, !_td->state.isDown, FALSE, TRUE); return TRUE; } From d34ba7e5277c04e3489bba9b89ea0c253903b8c5 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Fri, 2 Feb 2024 16:48:09 +1000 Subject: [PATCH 05/10] feat(windows): remove extra debugging --- windows/src/engine/keyman32/kmprocess.cpp | 3 +- .../src/engine/keyman32/kmprocessactions.cpp | 65 ------------------- 2 files changed, 1 insertion(+), 67 deletions(-) diff --git a/windows/src/engine/keyman32/kmprocess.cpp b/windows/src/engine/keyman32/kmprocess.cpp index b0c77d376d..5aa12d47fe 100644 --- a/windows/src/engine/keyman32/kmprocess.cpp +++ b/windows/src/engine/keyman32/kmprocess.cpp @@ -83,8 +83,7 @@ Process_Event_Core(PKEYMAN64THREADDATA _td) { if (_td->app->ReadContext(application_context)) { km_core_context_status result; result = km_core_state_context_set_if_needed(_td->lpActiveKeyboard->lpCoreKeyboardState, reinterpret_cast(application_context)); - // TODO remove debuging set if needed - SendDebugMessageFormat(0, sdmGlobal, 0, "Process_Event_Core: km_core_state_context_set_if_needed returned [%d]", result); + if (result == KM_CORE_CONTEXT_STATUS_ERROR || result == KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT) { SendDebugMessageFormat(0, sdmGlobal, 0, "Process_Event_Core: km_core_state_context_set_if_needed returned [%d]", result); } diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index f37d0827c6..3beb04669f 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -23,26 +23,6 @@ static void processAlert(AITIP* app) { app->QueueAction(QIT_BELL, 0); } -// TODO: #10583 -// Remove extra debuging code -static void -processDebugDeleteContext(const km_core_usv* delete_context) { - km_core_usv const* delete_context_ptr = delete_context; - while (*delete_context_ptr) { - delete_context_ptr++; - } - delete_context_ptr--; - for (; delete_context_ptr >= delete_context; delete_context_ptr--) { - if (Uni_IsSMP(*delete_context_ptr)) { - - SendDebugMessageFormat(0, sdmGlobal, 0, "processDebugContext: s1 U+%04x", Uni_UTF32ToSurrogate1(*delete_context_ptr)); - SendDebugMessageFormat(0, sdmGlobal, 0, "processDebugContext: s2 U+%04x", Uni_UTF32ToSurrogate2(*delete_context_ptr)); - } else { - SendDebugMessageFormat(0, sdmGlobal, 0, "processDebugContext: U+%04x", *delete_context_ptr); - } - } - -} static BOOL processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_usv* delete_context) { if (app->IsLegacy()) { @@ -72,48 +52,7 @@ processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_ return FALSE; } -// TODO remove reading the applications context -static BOOL -processBack2(AITIP* app, const unsigned int code_points_to_delete) { - if (app->IsLegacy()) { - SendDebugMessageFormat(0, sdmGlobal, 0, "processBack2: Legacy app cptd [%d].", code_points_to_delete); - for (unsigned int i = 0; i < code_points_to_delete; i++) { - //app->QueueAction(QIT_BACK, BK_DEFAULT); - } - return TRUE; - } - if (!app->IsLegacy()) { - SendDebugMessageFormat(0, sdmGlobal, 0, "processBack2: TSF app cptd [%d].", code_points_to_delete); - WCHAR application_context[MAXCONTEXT]; - DWORD cp_to_delete = code_points_to_delete; - if (!app->ReadContext(application_context)) { - SendDebugMessageFormat(0, sdmGlobal, 0, "processBack2: Error reading context from application."); - } else { - // Find the length of the string - int length = 0; - while (application_context[length] != L'\0') { - length++; - } - // Read each character starting from caret - for (int i = length - 1; i >= 0 && cp_to_delete > 0; i--) { - // Need to consider malformed context where only one surrogate is present, at either the start or end of the context. - // In both these scenarios just perform one backspace one the same as if it was a non surrogate pair. - if ((i > 0) && (Uni_IsSurrogate1(application_context[i - 1]) && Uni_IsSurrogate2(application_context[i]))) { - //app->QueueAction(QIT_BACK, BK_DEFAULT | BK_SURROGATE); - i--; - cp_to_delete--; - } else { - //app->QueueAction(QIT_BACK, BK_DEFAULT); - cp_to_delete--; - } - } - } - return TRUE; - } - - return FALSE; -} static void processPersistOpt(km_core_actions const* actions, LPINTKEYBOARDINFO activeKeyboard @@ -181,11 +120,7 @@ BOOL ProcessActions(BOOL* emitKeyStroke) - // Process the action items from the core. This actions will modify the windows context (AppContext). - // Therefore it is not required to copy the context from the core to the windows context. - processDebugDeleteContext(_td->core_actions->deleted_context); processBack(_td->app, _td->core_actions->code_points_to_delete, _td->core_actions->deleted_context); - processBack2(_td->app, _td->core_actions->code_points_to_delete); processUnicodeChar(_td->app, _td->core_actions->output); if (_td->core_actions->persist_options != NULL) { processPersistOpt(_td->core_actions, _td->lpActiveKeyboard); From 8f2e0a76eb3d335be5fb5c05f34d21d770b97e20 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Fri, 2 Feb 2024 16:55:28 +1000 Subject: [PATCH 06/10] feat(windows): code comment --- windows/src/desktop/kmshell/kmshell.res | Bin 7036 -> 7036 bytes .../src/engine/keyman32/kmprocessactions.cpp | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/windows/src/desktop/kmshell/kmshell.res b/windows/src/desktop/kmshell/kmshell.res index cfcbd309f137d6a5f5221aba48a1c759c0d56682..187b7bdad8c0c6e35543c45f7322290a4c74cfdc 100644 GIT binary patch delta 15 Wcmexk_Qz~O3Cs1n9!eW4Sfl|zV+PXz delta 15 Xcmexk_Qz~O3Cr{S4;406ut);{LY)U( diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index 3beb04669f..f1f2677a28 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -176,7 +176,7 @@ ProcessActionsExternalEvent() { if (!_td->core_actions) { // when ideponent we will not need this return FALSE; } - // TODO: #10583 remove dispose + // TODO: #10583 remove and call get actions here directly //km_core_actions const* acts = km_core_state_get_actions(_td->lpActiveKeyboard->lpCoreKeyboardState); processCapsLock(_td->core_actions->new_caps_lock_state, !_td->state.isDown, FALSE, TRUE); return TRUE; From 50b5d197b3895f2612ff1b88fcf1e294a327bdd6 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Mon, 5 Feb 2024 11:00:27 +1000 Subject: [PATCH 07/10] feat(windows): remove extra debug messages --- windows/src/engine/keyman32/kmprocess.cpp | 1 - windows/src/engine/keyman32/kmprocessactions.cpp | 15 ++++----------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/windows/src/engine/keyman32/kmprocess.cpp b/windows/src/engine/keyman32/kmprocess.cpp index 5aa12d47fe..c95c7ce5fd 100644 --- a/windows/src/engine/keyman32/kmprocess.cpp +++ b/windows/src/engine/keyman32/kmprocess.cpp @@ -83,7 +83,6 @@ Process_Event_Core(PKEYMAN64THREADDATA _td) { if (_td->app->ReadContext(application_context)) { km_core_context_status result; result = km_core_state_context_set_if_needed(_td->lpActiveKeyboard->lpCoreKeyboardState, reinterpret_cast(application_context)); - if (result == KM_CORE_CONTEXT_STATUS_ERROR || result == KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT) { SendDebugMessageFormat(0, sdmGlobal, 0, "Process_Event_Core: km_core_state_context_set_if_needed returned [%d]", result); } diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index f1f2677a28..b1408b98ff 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -23,17 +23,15 @@ static void processAlert(AITIP* app) { app->QueueAction(QIT_BELL, 0); } -static BOOL +static void processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_usv* delete_context) { if (app->IsLegacy()) { - SendDebugMessageFormat(0, sdmGlobal, 0, "processBack: Legacy app cptd [%d].", code_points_to_delete); for (unsigned int i = 0; i < code_points_to_delete; i++) { app->QueueAction(QIT_BACK, BK_DEFAULT); } - return TRUE; + return; } if (!app->IsLegacy()) { - SendDebugMessageFormat(0, sdmGlobal, 0, "processBack: TSF app cptd [%d].", code_points_to_delete); km_core_usv const* delete_context_ptr = delete_context; while (*delete_context_ptr) { delete_context_ptr++; @@ -47,9 +45,8 @@ processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_ app->QueueAction(QIT_BACK, BK_DEFAULT); } } - return TRUE; + return; } - return FALSE; } @@ -58,8 +55,6 @@ static void processPersistOpt(km_core_actions const* actions, LPINTKEYBOARDINFO activeKeyboard ) { for (auto option = actions->persist_options; option->key; option++) { - // Put the keyboard option into Windows Registry - // log"Saving keyboard option to registry"); SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessPersistOpt: Saving option to registry for keyboard [%s].", activeKeyboard->Name); size_t value_length = wcslen(reinterpret_cast(option->value)); LPWSTR value = new WCHAR[value_length + 1]; @@ -108,8 +103,6 @@ BOOL ProcessActions(BOOL* emitKeyStroke) { PKEYMAN64THREADDATA _td = ThreadGlobals(); if (!_td) return FALSE; - SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActions: Enter"); - // TODO: #10583 Remove caching action_struct if (!_td->core_actions) { SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActions: core_actions not set"); @@ -145,7 +138,7 @@ ProcessActionsNonUpdatableParse(BOOL* emitKeyStroke) { if (!_td) { return FALSE; } - SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActionsNonUpdatableParse: Enter"); + if (_td->TIPFUpdateable) { // ensure only run when not updateable return FALSE; } From 7ad81f94404e3adad3045f820d101fc593f2c6c8 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Mon, 5 Feb 2024 11:03:31 +1000 Subject: [PATCH 08/10] feat(windows): revert kmshell.res --- windows/src/desktop/kmshell/kmshell.res | Bin 7036 -> 7036 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/windows/src/desktop/kmshell/kmshell.res b/windows/src/desktop/kmshell/kmshell.res index 187b7bdad8c0c6e35543c45f7322290a4c74cfdc..cfcbd309f137d6a5f5221aba48a1c759c0d56682 100644 GIT binary patch delta 15 Xcmexk_Qz~O3Cr{S4;406ut);{LY)U( delta 15 Wcmexk_Qz~O3Cs1n9!eW4Sfl|zV+PXz From eecc6cd7bd0bc1bba08fa4d20ca454e6d75288e8 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Tue, 6 Feb 2024 08:25:08 +1000 Subject: [PATCH 09/10] fix(windows): address review comments Co-authored-by: Marc Durdin --- windows/src/engine/keyman32/kmprocessactions.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index b1408b98ff..ed817d754e 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -29,9 +29,8 @@ processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_ for (unsigned int i = 0; i < code_points_to_delete; i++) { app->QueueAction(QIT_BACK, BK_DEFAULT); } - return; } - if (!app->IsLegacy()) { + else { km_core_usv const* delete_context_ptr = delete_context; while (*delete_context_ptr) { delete_context_ptr++; @@ -45,7 +44,6 @@ processBack(AITIP* app, const unsigned int code_points_to_delete, const km_core_ app->QueueAction(QIT_BACK, BK_DEFAULT); } } - return; } } @@ -122,7 +120,7 @@ BOOL ProcessActions(BOOL* emitKeyStroke) processAlert(_td->app); } if (_td->core_actions->emit_keystroke) { - *emitKeyStroke = TRUE; + *emitKeystroke = TRUE; } processCapsLock(_td->core_actions->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); // TODO: #10583 remove dispose From 6d4b287d7dfab2fe0efe7ee8343b0a0211a36ba5 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Tue, 6 Feb 2024 08:47:22 +1000 Subject: [PATCH 10/10] feat(windows): variable name change --- windows/src/engine/keyman32/kmprocessactions.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/windows/src/engine/keyman32/kmprocessactions.cpp b/windows/src/engine/keyman32/kmprocessactions.cpp index ed817d754e..7e089fb286 100644 --- a/windows/src/engine/keyman32/kmprocessactions.cpp +++ b/windows/src/engine/keyman32/kmprocessactions.cpp @@ -97,7 +97,7 @@ static void processCapsLock(const km_core_caps_state caps_lock_state, BOOL isUp, } } -BOOL ProcessActions(BOOL* emitKeyStroke) +BOOL ProcessActions(BOOL* emitKeystroke) { PKEYMAN64THREADDATA _td = ThreadGlobals(); if (!_td) return FALSE; @@ -131,7 +131,7 @@ BOOL ProcessActions(BOOL* emitKeyStroke) } BOOL -ProcessActionsNonUpdatableParse(BOOL* emitKeyStroke) { +ProcessActionsNonUpdatableParse(BOOL* emitKeystroke) { PKEYMAN64THREADDATA _td = ThreadGlobals(); if (!_td) { return FALSE; @@ -151,7 +151,7 @@ ProcessActionsNonUpdatableParse(BOOL* emitKeyStroke) { SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActionsNonUpdatableParse: km_core_state_get_actions"); processCapsLock(_td->core_actions->new_caps_lock_state, !_td->state.isDown, _td->TIPFUpdateable, FALSE); if (_td->core_actions->emit_keystroke) { - *emitKeyStroke = TRUE; + *emitKeystroke = TRUE; SendDebugMessageFormat(0, sdmGlobal, 0, "ProcessActionsNonUpdatableParse EMIT_KEYSTROKE"); _td->CoreProcessEventRun = FALSE; // If we emit the key stroke on this parse we don't need the second parse }