From 49e222c273315a3d0a5859b464e1fbb5360d9638 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Mon, 18 May 2026 21:07:30 +1000 Subject: [PATCH 1/4] fix(windows): actions list copy constructor --- core/src/mock/mock_processor.cpp | 11 +++++++- core/src/state.cpp | 19 +++++++++++++ core/src/state.hpp | 4 +++ core/tests/unit/kmnkbd/state_api.tests.cpp | 32 ++++++++++++++++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/core/src/mock/mock_processor.cpp b/core/src/mock/mock_processor.cpp index 570553264d..d014716c87 100644 --- a/core/src/mock/mock_processor.cpp +++ b/core/src/mock/mock_processor.cpp @@ -125,8 +125,17 @@ namespace km { { assert(state); assert(action_item); - if ((!state) || (!action_item)) + if ((!state) || (!action_item)){ return false; + } + // For this mock processor we only support queuing PERSIST_OPT action items. + if (action_item->type == KM_CORE_IT_PERSIST_OPT && action_item->option) { + state->actions().push_persist(update_option(static_cast(action_item->option->scope), action_item->option->key, action_item->option->value)); + return true; + } + else { + return false; + } return false; } diff --git a/core/src/state.cpp b/core/src/state.cpp index 8eb99d1b0a..c354d3434b 100644 --- a/core/src/state.cpp +++ b/core/src/state.cpp @@ -36,6 +36,25 @@ void actions::push_capslock(bool turnOn) { emplace_back(std::move(ai)); } +actions::actions(actions const &other) +: std::vector(other) +, _option_items_stack(other._option_items_stack) +{ + // Update all option pointers to point to the new stack + for (auto &item : *this) { + if (item.type == KM_CORE_IT_PERSIST_OPT && item.option) { + // Find the corresponding option in the new stack + // The pointers in the original point to positions in other._option_items_stack + // We need to find the equivalent position in our _option_items_stack + auto original_ptr = item.option; + auto original_base = reinterpret_cast(other._option_items_stack.data()); + auto offset = original_ptr - original_base; + if (offset >= 0 && static_cast(offset) < _option_items_stack.size()) { + item.option = &_option_items_stack[offset]; + } + } + } +} state::state(km::core::abstract_processor & ap, km_core_option_item const *env) : _processor(ap) diff --git a/core/src/state.hpp b/core/src/state.hpp index da2cf6bb75..098aad93da 100644 --- a/core/src/state.hpp +++ b/core/src/state.hpp @@ -35,6 +35,10 @@ class actions : public std::vector public: template actions(Args&&... args); + actions(actions const &other); + // If the operator is needed in the future, it shall be implemented + // in like the deep copy constructor. Blocking accidental use of it. + actions &operator=(actions const &) = delete; void push_character(km_core_usv usv); void push_marker(uint32_t marker); diff --git a/core/tests/unit/kmnkbd/state_api.tests.cpp b/core/tests/unit/kmnkbd/state_api.tests.cpp index 9b46e2fea8..8def80f967 100644 --- a/core/tests/unit/kmnkbd/state_api.tests.cpp +++ b/core/tests/unit/kmnkbd/state_api.tests.cpp @@ -251,6 +251,12 @@ constexpr km_core_option_item const expected_persist_opt = { KM_CORE_OPT_KEYBOARD }; +//constexpr km_core_option_item const clone_persist_opt = { +// u"__test_clone", +// u"Not in original", +// KM_CORE_OPT_KEYBOARD +//}; + extern "C" { uint8_t test_imx_callback(km_core_state *state, uint32_t imx_id, void *callback_object){ @@ -270,6 +276,7 @@ int main(int argc, char * argv[]) km_core_keyboard * test_kb = nullptr; km_core_state * test_state = nullptr, * test_clone = nullptr; +// * test_clone2 = nullptr; test_kb = (km_core_keyboard *)new km::core::mock_processor(km::core::path("dummy.mock")); // Simple sanity tests. @@ -393,9 +400,34 @@ int main(int argc, char * argv[]) clone_state_deleted_text )); +// Need to test the option pointer is pointing to the correct the correct _option_items_stack. +// This could not be tested like this as the way commit() works adding KM_CORE_IT_END to the +// end of the actions list. This means when calling km_core_state_queue_action_items +// I get an assert in push_persist "empty() || back().type != KM_CORE_IT_END" it is a protection +// against adding a action to an already commited list. +// To test this change I need to do mocking closer to the integration of that actions list. + + + +// km_core_action_item actions_test[] = { +// { KM_CORE_IT_PERSIST_OPT, {0,}, }, +// { KM_CORE_IT_END, {0,}, } +// }; + +// actions_test[0].option = &clone_persist_opt; + +//try_status(km_core_state_create(test_kb, test_env_opts, &test_clone2)); + + +//km_core_state_queue_action_items(test_clone2, actions_test); +//test_assert(action_items(test_clone2, actions_test)); + +//test_assert(action_items(test_clone2, {action_clone, {KM_CORE_IT_END}})); + // Destroy them km_core_state_dispose(test_state); km_core_state_dispose(test_clone); + //km_core_state_dispose(test_clone2); km_core_keyboard_dispose(test_kb); return 0; From a07fff8be3e30fc39aa40ed75c17fd445f9e8101 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Thu, 23 Jul 2026 21:52:32 +1000 Subject: [PATCH 2/4] fix(windows): change option_stack to deque when vectors are resized the memory is realocated so all the option pointers become invalid. Using a deque avoids this it has all the same methods so it is straight replacement. fixes: #15961 --- core/src/mock/mock_processor.cpp | 16 ++++- core/src/state.cpp | 19 +++--- core/src/state.hpp | 3 +- core/tests/unit/kmnkbd/state_api.tests.cpp | 71 ++++++++++++++-------- 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/core/src/mock/mock_processor.cpp b/core/src/mock/mock_processor.cpp index d014716c87..c56f1f40dd 100644 --- a/core/src/mock/mock_processor.cpp +++ b/core/src/mock/mock_processor.cpp @@ -130,7 +130,9 @@ namespace km { } // For this mock processor we only support queuing PERSIST_OPT action items. if (action_item->type == KM_CORE_IT_PERSIST_OPT && action_item->option) { - state->actions().push_persist(update_option(static_cast(action_item->option->scope), action_item->option->key, action_item->option->value)); + state->actions().push_persist(update_option(static_cast(action_item->option->scope), + action_item->option->key, + action_item->option->value)); return true; } else { @@ -175,6 +177,18 @@ namespace km { u"F2 pressed test save.")); break; } + case KM_CORE_VKEY_F3: + { + state->actions().push_persist( + update_option(KM_CORE_OPT_KEYBOARD, + u"__test_point_3", + u"F3 pressed test save 1.")); + state->actions().push_persist( + update_option(KM_CORE_OPT_KEYBOARD, + u"__test_point_4", + u"F3 pressed test save 2.")); + break; + } case KM_CORE_VKEY_F4: state->context().push_marker(KM_CORE_VKEY_QUOTE); diff --git a/core/src/state.cpp b/core/src/state.cpp index c354d3434b..cf5a00c1da 100644 --- a/core/src/state.cpp +++ b/core/src/state.cpp @@ -40,17 +40,16 @@ actions::actions(actions const &other) : std::vector(other) , _option_items_stack(other._option_items_stack) { - // Update all option pointers to point to the new stack + // Update all option pointers to point to the new stack. + + size_t opt_index = 0; for (auto &item : *this) { - if (item.type == KM_CORE_IT_PERSIST_OPT && item.option) { - // Find the corresponding option in the new stack - // The pointers in the original point to positions in other._option_items_stack - // We need to find the equivalent position in our _option_items_stack - auto original_ptr = item.option; - auto original_base = reinterpret_cast(other._option_items_stack.data()); - auto offset = original_ptr - original_base; - if (offset >= 0 && static_cast(offset) < _option_items_stack.size()) { - item.option = &_option_items_stack[offset]; + if (item.type == KM_CORE_IT_PERSIST_OPT) { + if (opt_index < _option_items_stack.size()) { + item.option = &_option_items_stack[opt_index++]; + } else { + // no matching item in the stack; clear pointer. + item.option = nullptr; } } } diff --git a/core/src/state.hpp b/core/src/state.hpp index 098aad93da..0cd452e7fc 100644 --- a/core/src/state.hpp +++ b/core/src/state.hpp @@ -10,6 +10,7 @@ #include #include +#include #include "keyman_core.h" @@ -27,7 +28,7 @@ using action = km_core_action_item; class actions : public std::vector { - std::vector