diff --git a/src/modules/keyboardmanager/common/KeyboardManagerState.cpp b/src/modules/keyboardmanager/common/KeyboardManagerState.cpp index cad7247299..7a91c84d45 100644 --- a/src/modules/keyboardmanager/common/KeyboardManagerState.cpp +++ b/src/modules/keyboardmanager/common/KeyboardManagerState.cpp @@ -8,7 +8,7 @@ // Constructor KeyboardManagerState::KeyboardManagerState() : - uiState(KeyboardManagerUIState::Deactivated), currentUIWindow(nullptr), currentShortcutUI1(nullptr), currentShortcutUI2(nullptr), currentSingleKeyUI(nullptr), detectedRemapKey(NULL) + uiState(KeyboardManagerUIState::Deactivated), currentUIWindow(nullptr), currentShortcutUI1(nullptr), currentShortcutUI2(nullptr), currentSingleKeyUI(nullptr), detectedRemapKey(NULL), remappingsEnabled(true) { configFile_mutex = CreateMutex( NULL, // default security descriptor @@ -104,7 +104,6 @@ void KeyboardManagerState::ResetUIState() // Function to clear the OS Level shortcut remapping table void KeyboardManagerState::ClearOSLevelShortcuts() { - std::lock_guard lock(osLevelShortcutReMap_mutex); osLevelShortcutReMap.clear(); osLevelShortcutReMapSortedKeys.clear(); } @@ -112,23 +111,19 @@ void KeyboardManagerState::ClearOSLevelShortcuts() // Function to clear the Keys remapping table. void KeyboardManagerState::ClearSingleKeyRemaps() { - std::lock_guard lock(singleKeyReMap_mutex); singleKeyReMap.clear(); } // Function to clear the App specific shortcut remapping table void KeyboardManagerState::ClearAppSpecificShortcuts() { - std::lock_guard lock(appSpecificShortcutReMap_mutex); appSpecificShortcutReMap.clear(); appSpecificShortcutReMapSortedKeys.clear(); } // Function to add a new OS level shortcut remapping -bool KeyboardManagerState::AddOSLevelShortcut(const Shortcut& originalSC, const std::variant& newSC) +bool KeyboardManagerState::AddOSLevelShortcut(const Shortcut& originalSC, const KeyShortcutUnion& newSC) { - std::lock_guard lock(osLevelShortcutReMap_mutex); - // Check if the shortcut is already remapped auto it = osLevelShortcutReMap.find(originalSC); if (it != osLevelShortcutReMap.end()) @@ -144,10 +139,8 @@ bool KeyboardManagerState::AddOSLevelShortcut(const Shortcut& originalSC, const } // Function to add a new single key to key/shortcut remapping -bool KeyboardManagerState::AddSingleKeyRemap(const DWORD& originalKey, const std::variant& newRemapKey) +bool KeyboardManagerState::AddSingleKeyRemap(const DWORD& originalKey, const KeyShortcutUnion& newRemapKey) { - std::lock_guard lock(singleKeyReMap_mutex); - // Check if the key is already remapped auto it = singleKeyReMap.find(originalKey); if (it != singleKeyReMap.end()) @@ -160,10 +153,8 @@ bool KeyboardManagerState::AddSingleKeyRemap(const DWORD& originalKey, const std } // Function to add a new App specific shortcut remapping -bool KeyboardManagerState::AddAppSpecificShortcut(const std::wstring& app, const Shortcut& originalSC, const std::variant& newSC) +bool KeyboardManagerState::AddAppSpecificShortcut(const std::wstring& app, const Shortcut& originalSC, const KeyShortcutUnion& newSC) { - std::lock_guard lock(appSpecificShortcutReMap_mutex); - // Convert app name to lower case std::wstring process_name; process_name.resize(app.length()); @@ -191,6 +182,54 @@ bool KeyboardManagerState::AddAppSpecificShortcut(const std::wstring& app, const return true; } +// Function to get the iterator of a single key remap given the source key. Returns nullopt if it isn't remapped +std::optional KeyboardManagerState::GetSingleKeyRemap(const DWORD& originalKey) +{ + auto it = singleKeyReMap.find(originalKey); + if (it != singleKeyReMap.end()) + { + return it; + } + + return std::nullopt; +} + +bool KeyboardManagerState::CheckShortcutRemapInvoked(const std::optional& appName) +{ + // Assumes appName exists in the app-specific remap table + ShortcutRemapTable& currentRemapTable = appName ? appSpecificShortcutReMap[*appName] : osLevelShortcutReMap; + for (auto& it : currentRemapTable) + { + if (it.second.isShortcutInvoked) + { + return true; + } + } + + return false; +} + +std::vector& KeyboardManagerState::GetSortedShortcutRemapVector(const std::optional& appName) +{ + // Assumes appName exists in the app-specific remap table + return appName ? appSpecificShortcutReMapSortedKeys[*appName] : osLevelShortcutReMapSortedKeys; +} + +// Function to get the source and target of a shortcut remap given the source shortcut. Returns nullopt if it isn't remapped +ShortcutRemapTable& KeyboardManagerState::GetShortcutRemapTable(const std::optional& appName) +{ + if (appName) + { + auto itTable = appSpecificShortcutReMap.find(*appName); + if (itTable != appSpecificShortcutReMap.end()) + { + return itTable->second; + } + } + + return osLevelShortcutReMap; +} + // Function to set the textblock of the detect shortcut UI so that it can be accessed by the hook void KeyboardManagerState::ConfigureDetectShortcutUI(const StackPanel& textBlock1, const StackPanel& textBlock2) { @@ -468,7 +507,6 @@ bool KeyboardManagerState::SaveConfigToFile() json::JsonArray inProcessRemapKeysArray; json::JsonArray appSpecificRemapShortcutsArray; json::JsonArray globalRemapShortcutsArray; - std::unique_lock lockSingleKeyReMap(singleKeyReMap_mutex); for (const auto& it : singleKeyReMap) { json::JsonObject keys; @@ -488,9 +526,7 @@ bool KeyboardManagerState::SaveConfigToFile() inProcessRemapKeysArray.Append(keys); } - lockSingleKeyReMap.unlock(); - std::unique_lock lockOsLevelShortcutReMap(osLevelShortcutReMap_mutex); for (const auto& it : osLevelShortcutReMap) { json::JsonObject keys; @@ -510,9 +546,7 @@ bool KeyboardManagerState::SaveConfigToFile() globalRemapShortcutsArray.Append(keys); } - lockOsLevelShortcutReMap.unlock(); - std::unique_lock lockAppSpecificShortcutReMap(appSpecificShortcutReMap_mutex); for (const auto& itApp : appSpecificShortcutReMap) { // Iterate over apps @@ -538,7 +572,6 @@ bool KeyboardManagerState::SaveConfigToFile() appSpecificRemapShortcutsArray.Append(keys); } } - lockAppSpecificShortcutReMap.unlock(); remapShortcuts.SetNamedValue(KeyboardManagerConstants::GlobalRemapShortcutsSettingName, globalRemapShortcutsArray); remapShortcuts.SetNamedValue(KeyboardManagerConstants::AppSpecificRemapShortcutsSettingName, appSpecificRemapShortcutsArray); @@ -596,3 +629,20 @@ std::wstring KeyboardManagerState::GetActivatedApp() { return activatedAppSpecificShortcutTarget; } + +bool KeyboardManagerState::AreRemappingsEnabled() +{ + return remappingsEnabled; +} + +void KeyboardManagerState::RemappingsDisabledWrapper(std::function method) +{ + // Disable keyboard remappings + remappingsEnabled = false; + + // Run the method which requires the remappings to be disabled + method(); + + // Re-enable the keyboard remappings + remappingsEnabled = true; +} diff --git a/src/modules/keyboardmanager/common/KeyboardManagerState.h b/src/modules/keyboardmanager/common/KeyboardManagerState.h index 6a4f4f4482..85a0467249 100644 --- a/src/modules/keyboardmanager/common/KeyboardManagerState.h +++ b/src/modules/keyboardmanager/common/KeyboardManagerState.h @@ -20,6 +20,10 @@ namespace winrt::Windows::UI::Xaml::Controls struct StackPanel; } +using SingleKeyRemapTable = std::unordered_map; +using ShortcutRemapTable = std::map; +using AppSpecificShortcutRemapTable = std::map; + // Enum type to store different states of the UI enum class KeyboardManagerUIState { @@ -84,6 +88,9 @@ private: // Stores the activated target application in app-specfic shortcut std::wstring activatedAppSpecificShortcutTarget; + // Thread safe boolean value to check if remappings are currently enabled. This is used to disable remappings while the remap tables are being updated by the UI thread + std::atomic_bool remappingsEnabled; + // Display a key by appending a border Control as a child of the panel. void AddKeyToLayout(const winrt::Windows::UI::Xaml::Controls::StackPanel& panel, const winrt::hstring& key); @@ -91,22 +98,21 @@ public: // The map members and their mutexes are left as public since the maps are used extensively in dllmain.cpp. // Maps which store the remappings for each of the features. The bool fields should be initialized to false. They are used to check the current state of the shortcut (i.e is that particular shortcut currently pressed down or not). // Stores single key remappings - std::unordered_map> singleKeyReMap; - std::mutex singleKeyReMap_mutex; + std::unordered_map singleKeyReMap; + /* This feature has not been enabled (code from proof of concept stage) + * // Stores keys which need to be changed from toggle behavior to modifier behavior. Eg. Caps Lock std::unordered_map singleKeyToggleToMod; - std::mutex singleKeyToggleToMod_mutex; + */ // Stores the os level shortcut remappings - std::map osLevelShortcutReMap; + ShortcutRemapTable osLevelShortcutReMap; std::vector osLevelShortcutReMapSortedKeys; - std::mutex osLevelShortcutReMap_mutex; // Stores the app-specific shortcut remappings. Maps application name to the shortcut map - std::map> appSpecificShortcutReMap; + AppSpecificShortcutRemapTable appSpecificShortcutReMap; std::map> appSpecificShortcutReMapSortedKeys; - std::mutex appSpecificShortcutReMap_mutex; // Stores the keyboard layout LayoutMap keyboardMap; @@ -139,13 +145,23 @@ public: void ClearAppSpecificShortcuts(); // Function to add a new single key to key remapping - bool AddSingleKeyRemap(const DWORD& originalKey, const std::variant& newRemapKey); + bool AddSingleKeyRemap(const DWORD& originalKey, const KeyShortcutUnion& newRemapKey); // Function to add a new OS level shortcut remapping - bool AddOSLevelShortcut(const Shortcut& originalSC, const std::variant& newSC); + bool AddOSLevelShortcut(const Shortcut& originalSC, const KeyShortcutUnion& newSC); // Function to add a new App specific level shortcut remapping - bool AddAppSpecificShortcut(const std::wstring& app, const Shortcut& originalSC, const std::variant& newSC); + bool AddAppSpecificShortcut(const std::wstring& app, const Shortcut& originalSC, const KeyShortcutUnion& newSC); + + // Function to get the iterator of a single key remap given the source key. Returns nullopt if it isn't remapped + std::optional GetSingleKeyRemap(const DWORD& originalKey); + + bool CheckShortcutRemapInvoked(const std::optional& appName); + + std::vector& GetSortedShortcutRemapVector(const std::optional& appName); + + // Function to get the source and target of a shortcut remap given the source shortcut. Returns nullopt if it isn't remapped + ShortcutRemapTable& GetShortcutRemapTable(const std::optional& appName); // Function to set the textblock of the detect shortcut UI so that it can be accessed by the hook void ConfigureDetectShortcutUI(const winrt::Windows::UI::Xaml::Controls::StackPanel& textBlock1, const winrt::Windows::UI::Xaml::Controls::StackPanel& textBlock2); @@ -214,4 +230,8 @@ public: // Gets the activated target application in app-specfic shortcut std::wstring GetActivatedApp(); + + bool AreRemappingsEnabled(); + + void RemappingsDisabledWrapper(std::function method); }; diff --git a/src/modules/keyboardmanager/common/RemapShortcut.h b/src/modules/keyboardmanager/common/RemapShortcut.h index f2f5e66ed6..a22ef83b45 100644 --- a/src/modules/keyboardmanager/common/RemapShortcut.h +++ b/src/modules/keyboardmanager/common/RemapShortcut.h @@ -6,11 +6,11 @@ class RemapShortcut { public: - std::variant targetShortcut; + KeyShortcutUnion targetShortcut; bool isShortcutInvoked; ModifierKey winKeyInvoked; - RemapShortcut(const std::variant& sc) : + RemapShortcut(const KeyShortcutUnion& sc) : targetShortcut(sc), isShortcutInvoked(false), winKeyInvoked(ModifierKey::Disabled) { } diff --git a/src/modules/keyboardmanager/common/Shortcut.h b/src/modules/keyboardmanager/common/Shortcut.h index fd3885dce2..a86628c4c2 100644 --- a/src/modules/keyboardmanager/common/Shortcut.h +++ b/src/modules/keyboardmanager/common/Shortcut.h @@ -171,6 +171,7 @@ public: KeyboardManagerHelper::ErrorType IsShortcutIllegal() const; }; -using RemapBufferItem = std::vector>; +using KeyShortcutUnion = std::variant; +using RemapBufferItem = std::vector; using RemapBufferRow = std::pair; using RemapBuffer = std::vector; diff --git a/src/modules/keyboardmanager/dll/KeyboardEventHandlers.cpp b/src/modules/keyboardmanager/dll/KeyboardEventHandlers.cpp index e09dc133a7..5fc123ce65 100644 --- a/src/modules/keyboardmanager/dll/KeyboardEventHandlers.cpp +++ b/src/modules/keyboardmanager/dll/KeyboardEventHandlers.cpp @@ -16,11 +16,11 @@ namespace KeyboardEventHandlers // Check if the key event was generated by KeyboardManager to avoid remapping events generated by us. if (!(data->lParam->dwExtraInfo & CommonSharedConstants::KEYBOARDMANAGER_INJECTED_FLAG)) { - // The mutex should be unlocked before SendInput is called to avoid re-entry into the same mutex. More details can be found at https://github.com/microsoft/PowerToys/pull/1789#issuecomment-607555837 - std::unique_lock lock(keyboardManagerState.singleKeyReMap_mutex); - auto it = keyboardManagerState.singleKeyReMap.find(data->lParam->vkCode); - if (it != keyboardManagerState.singleKeyReMap.end()) + auto& remapping = keyboardManagerState.GetSingleKeyRemap(data->lParam->vkCode); + if (remapping) { + auto it = remapping.value(); + // Check if the remap is to a key or a shortcut bool remapToKey = (it->second.index() == 0); @@ -95,7 +95,6 @@ namespace KeyboardEventHandlers } } - lock.unlock(); UINT res = ii.SendVirtualInput(key_count, keyEventList, sizeof(INPUT)); delete[] keyEventList; @@ -126,6 +125,8 @@ namespace KeyboardEventHandlers return 0; } + /* This feature has not been enabled (code from proof of concept stage) + * // Function to a change a key's behavior from toggle to modifier __declspec(dllexport) intptr_t HandleSingleKeyToggleToModEvent(InputInterface& ii, LowlevelKeyboardEvent* data, KeyboardManagerState& keyboardManagerState) noexcept { @@ -174,26 +175,19 @@ namespace KeyboardEventHandlers return 0; } + */ // Function to a handle a shortcut remap - __declspec(dllexport) intptr_t HandleShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, std::map& reMap, std::vector& sortedReMapKeys, std::mutex& map_mutex, KeyboardManagerState& keyboardManagerState, const std::wstring& activatedApp) noexcept + __declspec(dllexport) intptr_t HandleShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, KeyboardManagerState& keyboardManagerState, const std::optional& activatedApp) noexcept { - // The mutex should be unlocked before SendInput is called to avoid re-entry into the same mutex. More details can be found at https://github.com/microsoft/PowerToys/pull/1789#issuecomment-607555837 - std::unique_lock lock(map_mutex); - // Check if any shortcut is currently in the invoked state - bool isShortcutInvoked = false; - for (auto& it : reMap) - { - if (it.second.isShortcutInvoked) - { - isShortcutInvoked = true; - break; - } - } + bool isShortcutInvoked = keyboardManagerState.CheckShortcutRemapInvoked(activatedApp); + + // Get shortcut table for given activatedApp + ShortcutRemapTable& reMap = keyboardManagerState.GetShortcutRemapTable(activatedApp); // Iterate through the shortcut remaps and apply whichever has been pressed - for (auto& itShortcut : sortedReMapKeys) + for (auto& itShortcut : keyboardManagerState.GetSortedShortcutRemapVector(activatedApp)) { auto& it = reMap.find(itShortcut); @@ -317,16 +311,16 @@ namespace KeyboardEventHandlers it->second.isShortcutInvoked = true; // If app specific shortcut is invoked, store the target application - if (activatedApp != KeyboardManagerConstants::NoActivatedApp) + if (activatedApp) { - keyboardManagerState.SetActivatedApp(activatedApp); + keyboardManagerState.SetActivatedApp(*activatedApp); } - lock.unlock(); + UINT res = ii.SendVirtualInput((UINT)key_count, keyEventList, sizeof(INPUT)); delete[] keyEventList; // Log telemetry event when shortcut remap is invoked - Trace::ShortcutRemapInvoked(remapToShortcut, activatedApp != KeyboardManagerConstants::NoActivatedApp); + Trace::ShortcutRemapInvoked(remapToShortcut, activatedApp.has_value()); return 1; } @@ -415,11 +409,10 @@ namespace KeyboardEventHandlers it->second.isShortcutInvoked = false; it->second.winKeyInvoked = ModifierKey::Disabled; // If app specific shortcut has finished invoking, reset the target application - if (activatedApp != KeyboardManagerConstants::NoActivatedApp) + if (activatedApp) { keyboardManagerState.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp); } - lock.unlock(); // key count can be 0 if both shortcuts have same modifiers and the action key is not held down. delete will throw an error if keyEventList is empty if (key_count > 0) @@ -455,7 +448,6 @@ namespace KeyboardEventHandlers } it->second.isShortcutInvoked = true; - lock.unlock(); UINT res = ii.SendVirtualInput((UINT)key_count, keyEventList, sizeof(INPUT)); delete[] keyEventList; return 1; @@ -507,13 +499,12 @@ namespace KeyboardEventHandlers it->second.isShortcutInvoked = false; it->second.winKeyInvoked = ModifierKey::Disabled; // If app specific shortcut has finished invoking, reset the target application - if (activatedApp != KeyboardManagerConstants::NoActivatedApp) + if (activatedApp) { keyboardManagerState.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp); } } - lock.unlock(); UINT res = ii.SendVirtualInput((UINT)key_count, keyEventList, sizeof(INPUT)); delete[] keyEventList; return 1; @@ -639,11 +630,11 @@ namespace KeyboardEventHandlers it->second.isShortcutInvoked = false; it->second.winKeyInvoked = ModifierKey::Disabled; // If app specific shortcut has finished invoking, reset the target application - if (activatedApp != KeyboardManagerConstants::NoActivatedApp) + if (activatedApp) { keyboardManagerState.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp); } - lock.unlock(); + UINT res = ii.SendVirtualInput((UINT)key_count, keyEventList, sizeof(INPUT)); delete[] keyEventList; return 1; @@ -680,7 +671,7 @@ namespace KeyboardEventHandlers { keyboardManagerState.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp); } - lock.unlock(); + UINT res = ii.SendVirtualInput((UINT)key_count, keyEventList, sizeof(INPUT)); delete[] keyEventList; return 1; @@ -701,7 +692,7 @@ namespace KeyboardEventHandlers // Check if the key event was generated by KeyboardManager to avoid remapping events generated by us. if (data->lParam->dwExtraInfo != KeyboardManagerConstants::KEYBOARDMANAGER_SHORTCUT_FLAG) { - bool result = HandleShortcutRemapEvent(ii, data, keyboardManagerState.osLevelShortcutReMap, keyboardManagerState.osLevelShortcutReMapSortedKeys, keyboardManagerState.osLevelShortcutReMap_mutex, keyboardManagerState); + bool result = HandleShortcutRemapEvent(ii, data, keyboardManagerState); return result; } @@ -731,10 +722,9 @@ namespace KeyboardEventHandlers // Convert process name to lower case std::transform(process_name.begin(), process_name.end(), process_name.begin(), towlower); - std::unique_lock lock(keyboardManagerState.appSpecificShortcutReMap_mutex); std::wstring query_string; - std::map>::iterator it; + AppSpecificShortcutRemapTable::iterator it; // Check if an app-specific shortcut is already activated if (keyboardManagerState.GetActivatedApp() == KeyboardManagerConstants::NoActivatedApp) { @@ -758,8 +748,7 @@ namespace KeyboardEventHandlers if (it != keyboardManagerState.appSpecificShortcutReMap.end()) { - lock.unlock(); - bool result = HandleShortcutRemapEvent(ii, data, keyboardManagerState.appSpecificShortcutReMap[query_string], keyboardManagerState.appSpecificShortcutReMapSortedKeys[query_string], keyboardManagerState.appSpecificShortcutReMap_mutex, keyboardManagerState, query_string); + bool result = HandleShortcutRemapEvent(ii, data, keyboardManagerState, query_string); return result; } } diff --git a/src/modules/keyboardmanager/dll/KeyboardEventHandlers.h b/src/modules/keyboardmanager/dll/KeyboardEventHandlers.h index 0708c64a4e..8780899651 100644 --- a/src/modules/keyboardmanager/dll/KeyboardEventHandlers.h +++ b/src/modules/keyboardmanager/dll/KeyboardEventHandlers.h @@ -15,11 +15,14 @@ namespace KeyboardEventHandlers // Function to a handle a single key remap __declspec(dllexport) intptr_t HandleSingleKeyRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, KeyboardManagerState& keyboardManagerState) noexcept; + /* This feature has not been enabled (code from proof of concept stage) + * // Function to a change a key's behavior from toggle to modifier __declspec(dllexport) intptr_t HandleSingleKeyToggleToModEvent(InputInterface& ii, LowlevelKeyboardEvent* data, KeyboardManagerState& keyboardManagerState) noexcept; + */ // Function to a handle a shortcut remap - __declspec(dllexport) intptr_t HandleShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, std::map& reMap, std::vector& sortedReMapKeys, std::mutex& map_mutex, KeyboardManagerState& keyboardManagerState, const std::wstring& activatedApp = KeyboardManagerConstants::NoActivatedApp) noexcept; + __declspec(dllexport) intptr_t HandleShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, KeyboardManagerState& keyboardManagerState, const std::optional& activatedApp = std::nullopt) noexcept; // Function to a handle an os-level shortcut remap __declspec(dllexport) intptr_t HandleOSLevelShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, KeyboardManagerState& keyboardManagerState) noexcept; diff --git a/src/modules/keyboardmanager/dll/dllmain.cpp b/src/modules/keyboardmanager/dll/dllmain.cpp index bfc3ba383f..554f5ff3cd 100644 --- a/src/modules/keyboardmanager/dll/dllmain.cpp +++ b/src/modules/keyboardmanager/dll/dllmain.cpp @@ -384,6 +384,12 @@ public: // Function called by the hook procedure to handle the events. This is the starting point function for remapping intptr_t HandleKeyboardHookEvent(LowlevelKeyboardEvent* data) noexcept { + // If remappings are disabled (due to the remap tables getting updated) skip the rest of the hook + if (!keyboardManagerState.AreRemappingsEnabled()) + { + return 0; + } + // If key has suppress flag, then suppress it if (data->lParam->dwExtraInfo == KeyboardManagerConstants::KEYBOARDMANAGER_SUPPRESS_FLAG) { @@ -432,8 +438,11 @@ public: return 0; } + /* This feature has not been enabled (code from proof of concept stage) + * //// Remap a key to behave like a modifier instead of a toggle //intptr_t SingleKeyToggleToModResult = KeyboardEventHandlers::HandleSingleKeyToggleToModEvent(inputHandler, data, keyboardManagerState); + */ // Handle an app-specific shortcut remapping intptr_t AppSpecificShortcutRemapResult = KeyboardEventHandlers::HandleAppSpecificShortcutRemapEvent(inputHandler, data, keyboardManagerState); diff --git a/src/modules/keyboardmanager/test/LoadingAndSavingRemappingTests.cpp b/src/modules/keyboardmanager/test/LoadingAndSavingRemappingTests.cpp index ccc2c15ecd..c30bf03838 100644 --- a/src/modules/keyboardmanager/test/LoadingAndSavingRemappingTests.cpp +++ b/src/modules/keyboardmanager/test/LoadingAndSavingRemappingTests.cpp @@ -298,7 +298,7 @@ namespace RemappingUITests // Test if the PreProcessRemapTable method combines all the modifier pairs when the left and right modifiers are remapped to the same target TEST_METHOD (PreProcessRemapTable_ShouldCombineAllPairs_OnPassingLeftAndRightModifiersRemappedToTheSameTarget) { - std::unordered_map> remapTable; + SingleKeyRemapTable remapTable; // Remap LCtrl and RCtrl to A, LAlt and RAlt to B, LShift and RShift to C, LWin and RWin to D remapTable[VK_LCONTROL] = 0x41; @@ -314,7 +314,7 @@ namespace RemappingUITests LoadingAndSavingRemappingHelper::PreProcessRemapTable(remapTable); // Expected Ctrl remapped to A, Alt to B, Shift to C, Win to D - std::unordered_map> expectedTable; + SingleKeyRemapTable expectedTable; expectedTable[VK_CONTROL] = 0x41; expectedTable[VK_MENU] = 0x42; expectedTable[VK_SHIFT] = 0x43; @@ -327,7 +327,7 @@ namespace RemappingUITests // Test if the PreProcessRemapTable method does not combines any of the modifier pairs when the left and right modifiers are remapped to different targets TEST_METHOD (PreProcessRemapTable_ShouldNotCombineAnyPairs_OnPassingLeftAndRightModifiersRemappedToTheDifferentTargets) { - std::unordered_map> remapTable; + SingleKeyRemapTable remapTable; // Remap left modifiers to A and right modifiers to B remapTable[VK_LCONTROL] = 0x41; @@ -343,7 +343,7 @@ namespace RemappingUITests LoadingAndSavingRemappingHelper::PreProcessRemapTable(remapTable); // Expected unchanged table - std::unordered_map> expectedTable; + SingleKeyRemapTable expectedTable; expectedTable[VK_LCONTROL] = 0x41; expectedTable[VK_RCONTROL] = 0x42; expectedTable[VK_LMENU] = 0x41; @@ -394,7 +394,7 @@ namespace RemappingUITests LoadingAndSavingRemappingHelper::ApplySingleKeyRemappings(testState, remapBuffer, false); // Expected A remapped to B, B remapped to Ctrl+V - std::unordered_map> expectedTable; + SingleKeyRemapTable expectedTable; expectedTable[0x41] = 0x42; expectedTable[0x42] = s1; @@ -418,7 +418,7 @@ namespace RemappingUITests LoadingAndSavingRemappingHelper::ApplySingleKeyRemappings(testState, remapBuffer, false); // Expected LCtrl/RCtrl remapped to A, LAlt/RAlt to B, LShift/RShift to C, LWin/RWin to D - std::unordered_map> expectedTable; + SingleKeyRemapTable expectedTable; expectedTable[VK_LCONTROL] = 0x41; expectedTable[VK_RCONTROL] = 0x41; expectedTable[VK_LMENU] = 0x42; @@ -503,12 +503,12 @@ namespace RemappingUITests LoadingAndSavingRemappingHelper::ApplyShortcutRemappings(testState, remapBuffer, false); // Ctrl+A->Ctrl+B and Ctrl+C->Alt+V - std::map expectedOSLevelTable; + ShortcutRemapTable expectedOSLevelTable; expectedOSLevelTable[src1] = RemapShortcut(dest1); expectedOSLevelTable[src2] = RemapShortcut(dest2); // Ctrl+F->Alt+V and Ctrl+G->Ctrl+B for testApp1 - std::map> expectedAppSpecificLevelTable; + AppSpecificShortcutRemapTable expectedAppSpecificLevelTable; expectedAppSpecificLevelTable[testApp1][src3] = RemapShortcut(dest2); expectedAppSpecificLevelTable[testApp1][src4] = RemapShortcut(dest1); diff --git a/src/modules/keyboardmanager/ui/BufferValidationHelpers.cpp b/src/modules/keyboardmanager/ui/BufferValidationHelpers.cpp index a8ead4fc9f..34d600e691 100644 --- a/src/modules/keyboardmanager/ui/BufferValidationHelpers.cpp +++ b/src/modules/keyboardmanager/ui/BufferValidationHelpers.cpp @@ -204,7 +204,7 @@ namespace BufferValidationHelpers // After validating the shortcut, now for errors like remap to same shortcut, remap shortcut more than once, Win L and Ctrl Alt Del if (errorType == KeyboardManagerHelper::ErrorType::NoError) { - std::variant tempShortcut; + KeyShortcutUnion tempShortcut; if (isHybridControl && selectedKeyCodes.size() == 1) { tempShortcut = selectedKeyCodes[0]; diff --git a/src/modules/keyboardmanager/ui/EditKeyboardWindow.cpp b/src/modules/keyboardmanager/ui/EditKeyboardWindow.cpp index 1c2b5119c5..72e1871760 100644 --- a/src/modules/keyboardmanager/ui/EditKeyboardWindow.cpp +++ b/src/modules/keyboardmanager/ui/EditKeyboardWindow.cpp @@ -252,9 +252,8 @@ void createEditKeyboardWindow(HINSTANCE hInst, KeyboardManagerState& keyboardMan keyboardManagerState.SetUIState(KeyboardManagerUIState::EditKeyboardWindowActivated, _hWndEditKeyboardWindow); // Load existing remaps into UI - std::unique_lock lock(keyboardManagerState.singleKeyReMap_mutex); - std::unordered_map> singleKeyRemapCopy = keyboardManagerState.singleKeyReMap; - lock.unlock(); + SingleKeyRemapTable singleKeyRemapCopy = keyboardManagerState.singleKeyReMap; + LoadingAndSavingRemappingHelper::PreProcessRemapTable(singleKeyRemapCopy); for (const auto& it : singleKeyRemapCopy) @@ -272,9 +271,13 @@ void createEditKeyboardWindow(HINSTANCE hInst, KeyboardManagerState& keyboardMan header.SetLeftOf(applyButton, cancelButton); auto ApplyRemappings = [&keyboardManagerState, _hWndEditKeyboardWindow]() { - LoadingAndSavingRemappingHelper::ApplySingleKeyRemappings(keyboardManagerState, SingleKeyRemapControl::singleKeyRemapBuffer, true); - // Save the updated shortcuts remaps to file. - bool saveResult = keyboardManagerState.SaveConfigToFile(); + // Disable the remappings while the remapping table is updated + keyboardManagerState.RemappingsDisabledWrapper( + [&keyboardManagerState]() { + LoadingAndSavingRemappingHelper::ApplySingleKeyRemappings(keyboardManagerState, SingleKeyRemapControl::singleKeyRemapBuffer, true); + // Save the updated shortcuts remaps to file. + bool saveResult = keyboardManagerState.SaveConfigToFile(); + }); PostMessage(_hWndEditKeyboardWindow, WM_CLOSE, 0, 0); }; diff --git a/src/modules/keyboardmanager/ui/EditShortcutsWindow.cpp b/src/modules/keyboardmanager/ui/EditShortcutsWindow.cpp index bb8cde6ece..88d2ac6079 100644 --- a/src/modules/keyboardmanager/ui/EditShortcutsWindow.cpp +++ b/src/modules/keyboardmanager/ui/EditShortcutsWindow.cpp @@ -226,17 +226,20 @@ void createEditShortcutsWindow(HINSTANCE hInst, KeyboardManagerState& keyboardMa keyboardManagerState.SetUIState(KeyboardManagerUIState::EditShortcutsWindowActivated, _hWndEditShortcutsWindow); // Load existing os level shortcuts into UI - std::unique_lock lockOSLevel(keyboardManagerState.osLevelShortcutReMap_mutex); - for (const auto& it : keyboardManagerState.osLevelShortcutReMap) + // Create copy of the remaps to avoid concurrent access + ShortcutRemapTable osLevelShortcutReMapCopy = keyboardManagerState.osLevelShortcutReMap; + + for (const auto& it : osLevelShortcutReMapCopy) { ShortcutControl::AddNewShortcutControlRow(shortcutTable, keyboardRemapControlObjects, it.first, it.second.targetShortcut); } - lockOSLevel.unlock(); // Load existing app-specific shortcuts into UI - std::unique_lock lockAppSpecific(keyboardManagerState.appSpecificShortcutReMap_mutex); + // Create copy of the remaps to avoid concurrent access + AppSpecificShortcutRemapTable appSpecificShortcutReMapCopy = keyboardManagerState.appSpecificShortcutReMap; + // Iterate through all the apps - for (const auto& itApp : keyboardManagerState.appSpecificShortcutReMap) + for (const auto& itApp : appSpecificShortcutReMapCopy) { // Iterate through shortcuts for each app for (const auto& itShortcut : itApp.second) @@ -244,7 +247,6 @@ void createEditShortcutsWindow(HINSTANCE hInst, KeyboardManagerState& keyboardMa ShortcutControl::AddNewShortcutControlRow(shortcutTable, keyboardRemapControlObjects, itShortcut.first, itShortcut.second.targetShortcut, itApp.first); } } - lockAppSpecific.unlock(); // Apply button Button applyButton; @@ -256,9 +258,13 @@ void createEditShortcutsWindow(HINSTANCE hInst, KeyboardManagerState& keyboardMa header.SetLeftOf(applyButton, cancelButton); auto ApplyRemappings = [&keyboardManagerState, _hWndEditShortcutsWindow]() { - LoadingAndSavingRemappingHelper::ApplyShortcutRemappings(keyboardManagerState, ShortcutControl::shortcutRemapBuffer, true); - // Save the updated key remaps to file. - bool saveResult = keyboardManagerState.SaveConfigToFile(); + // Disable the remappings while the remapping table is updated + keyboardManagerState.RemappingsDisabledWrapper( + [&keyboardManagerState]() { + LoadingAndSavingRemappingHelper::ApplyShortcutRemappings(keyboardManagerState, ShortcutControl::shortcutRemapBuffer, true); + // Save the updated key remaps to file. + bool saveResult = keyboardManagerState.SaveConfigToFile(); + }); PostMessage(_hWndEditShortcutsWindow, WM_CLOSE, 0, 0); }; diff --git a/src/modules/keyboardmanager/ui/KeyDropDownControl.cpp b/src/modules/keyboardmanager/ui/KeyDropDownControl.cpp index 5948ff59a7..ff4eae2260 100644 --- a/src/modules/keyboardmanager/ui/KeyDropDownControl.cpp +++ b/src/modules/keyboardmanager/ui/KeyDropDownControl.cpp @@ -341,7 +341,7 @@ void KeyDropDownControl::ValidateShortcutFromDropDownList(Grid table, StackPanel { // Check for errors only if the current selection is a valid shortcut std::vector selectedKeyCodes = KeyboardManagerHelper::GetKeyCodesFromSelectedIndices(keyDropDownControlObjects[i]->GetSelectedIndicesFromStackPanel(parent), GetKeyCodeList(true, colIndex == 1)); - std::variant currentShortcut; + KeyShortcutUnion currentShortcut; if (selectedKeyCodes.size() == 1 && isHybridControl) { currentShortcut = selectedKeyCodes[0]; diff --git a/src/modules/keyboardmanager/ui/LoadingAndSavingRemappingHelper.cpp b/src/modules/keyboardmanager/ui/LoadingAndSavingRemappingHelper.cpp index b814d94355..690f946726 100644 --- a/src/modules/keyboardmanager/ui/LoadingAndSavingRemappingHelper.cpp +++ b/src/modules/keyboardmanager/ui/LoadingAndSavingRemappingHelper.cpp @@ -11,11 +11,11 @@ namespace LoadingAndSavingRemappingHelper KeyboardManagerHelper::ErrorType CheckIfRemappingsAreValid(const RemapBuffer& remappings) { KeyboardManagerHelper::ErrorType isSuccess = KeyboardManagerHelper::ErrorType::NoError; - std::map>> ogKeys; + std::map> ogKeys; for (int i = 0; i < remappings.size(); i++) { - std::variant ogKey = remappings[i].first[0]; - std::variant newKey = remappings[i].first[1]; + KeyShortcutUnion ogKey = remappings[i].first[0]; + KeyShortcutUnion newKey = remappings[i].first[1]; std::wstring appName = remappings[i].second; bool ogKeyValidity = (ogKey.index() == 0 && std::get(ogKey) != NULL) || (ogKey.index() == 1 && std::get(ogKey).IsValidShortcut()); @@ -24,7 +24,7 @@ namespace LoadingAndSavingRemappingHelper // Add new set for a new target app name if (ogKeys.find(appName) == ogKeys.end()) { - ogKeys[appName] = std::set>(); + ogKeys[appName] = std::set(); } if (ogKeyValidity && newKeyValidity && ogKeys[appName].find(ogKey) == ogKeys[appName].end()) @@ -52,7 +52,7 @@ namespace LoadingAndSavingRemappingHelper for (int i = 0; i < remappings.size(); i++) { DWORD ogKey = std::get(remappings[i].first[0]); - std::variant newKey = remappings[i].first[1]; + KeyShortcutUnion newKey = remappings[i].first[1]; if (ogKey != NULL && ((newKey.index() == 0 && std::get(newKey) != 0) || (newKey.index() == 1 && std::get(newKey).IsValidShortcut()))) { @@ -75,7 +75,7 @@ namespace LoadingAndSavingRemappingHelper } // Function to combine remappings if the L and R version of the modifier is mapped to the same key - void CombineRemappings(std::unordered_map>& table, DWORD leftKey, DWORD rightKey, DWORD combinedKey) + void CombineRemappings(SingleKeyRemapTable& table, DWORD leftKey, DWORD rightKey, DWORD combinedKey) { if (table.find(leftKey) != table.end() && table.find(rightKey) != table.end()) { @@ -90,7 +90,7 @@ namespace LoadingAndSavingRemappingHelper } // Function to pre process the remap table before loading it into the UI - void PreProcessRemapTable(std::unordered_map>& table) + void PreProcessRemapTable(SingleKeyRemapTable& table) { // Pre process the table to combine L and R versions of Ctrl/Alt/Shift/Win that are mapped to the same key CombineRemappings(table, VK_LCONTROL, VK_RCONTROL, VK_CONTROL); @@ -109,7 +109,7 @@ namespace LoadingAndSavingRemappingHelper for (int i = 0; i < remappings.size(); i++) { DWORD originalKey = std::get(remappings[i].first[0]); - std::variant newKey = remappings[i].first[1]; + KeyShortcutUnion newKey = remappings[i].first[1]; if (originalKey != NULL && !(newKey.index() == 0 && std::get(newKey) == NULL) && !(newKey.index() == 1 && !std::get(newKey).IsValidShortcut())) { @@ -177,7 +177,7 @@ namespace LoadingAndSavingRemappingHelper for (int i = 0; i < remappings.size(); i++) { Shortcut originalShortcut = std::get(remappings[i].first[0]); - std::variant newShortcut = remappings[i].first[1]; + KeyShortcutUnion newShortcut = remappings[i].first[1]; if (originalShortcut.IsValidShortcut() && ((newShortcut.index() == 0 && std::get(newShortcut) != NULL) || (newShortcut.index() == 1 && std::get(newShortcut).IsValidShortcut()))) { diff --git a/src/modules/keyboardmanager/ui/LoadingAndSavingRemappingHelper.h b/src/modules/keyboardmanager/ui/LoadingAndSavingRemappingHelper.h index 5604e2c907..d715145eae 100644 --- a/src/modules/keyboardmanager/ui/LoadingAndSavingRemappingHelper.h +++ b/src/modules/keyboardmanager/ui/LoadingAndSavingRemappingHelper.h @@ -14,10 +14,10 @@ namespace LoadingAndSavingRemappingHelper std::vector GetOrphanedKeys(const RemapBuffer& remappings); // Function to combine remappings if the L and R version of the modifier is mapped to the same key - void CombineRemappings(std::unordered_map>& table, DWORD leftKey, DWORD rightKey, DWORD combinedKey); + void CombineRemappings(std::unordered_map& table, DWORD leftKey, DWORD rightKey, DWORD combinedKey); // Function to pre process the remap table before loading it into the UI - void PreProcessRemapTable(std::unordered_map>& table); + void PreProcessRemapTable(std::unordered_map& table); // Function to apply the single key remappings from the buffer to the KeyboardManagerState variable void ApplySingleKeyRemappings(KeyboardManagerState& keyboardManagerState, const RemapBuffer& remappings, bool isTelemetryRequired); diff --git a/src/modules/keyboardmanager/ui/ShortcutControl.cpp b/src/modules/keyboardmanager/ui/ShortcutControl.cpp index 52c2e61972..b4782752fd 100644 --- a/src/modules/keyboardmanager/ui/ShortcutControl.cpp +++ b/src/modules/keyboardmanager/ui/ShortcutControl.cpp @@ -65,7 +65,7 @@ void ShortcutControl::UpdateAccessibleNames(StackPanel sourceColumn, StackPanel } // Function to add a new row to the shortcut table. If the originalKeys and newKeys args are provided, then the displayed shortcuts are set to those values. -void ShortcutControl::AddNewShortcutControlRow(Grid& parent, std::vector>>& keyboardRemapControlObjects, const Shortcut& originalKeys, const std::variant& newKeys, const std::wstring& targetAppName) +void ShortcutControl::AddNewShortcutControlRow(Grid& parent, std::vector>>& keyboardRemapControlObjects, const Shortcut& originalKeys, const KeyShortcutUnion& newKeys, const std::wstring& targetAppName) { // Textbox for target application TextBox targetAppTextBox; diff --git a/src/modules/keyboardmanager/ui/ShortcutControl.h b/src/modules/keyboardmanager/ui/ShortcutControl.h index 6730fe8ecf..77618ca2cc 100644 --- a/src/modules/keyboardmanager/ui/ShortcutControl.h +++ b/src/modules/keyboardmanager/ui/ShortcutControl.h @@ -48,7 +48,7 @@ public: ShortcutControl(Grid table, const int colIndex, TextBox targetApp); // Function to add a new row to the shortcut table. If the originalKeys and newKeys args are provided, then the displayed shortcuts are set to those values. - static void AddNewShortcutControlRow(Grid& parent, std::vector>>& keyboardRemapControlObjects, const Shortcut& originalKeys = Shortcut(), const std::variant& newKeys = Shortcut(), const std::wstring& targetAppName = L""); + static void AddNewShortcutControlRow(Grid& parent, std::vector>>& keyboardRemapControlObjects, const Shortcut& originalKeys = Shortcut(), const KeyShortcutUnion& newKeys = Shortcut(), const std::wstring& targetAppName = L""); // Function to return the stack panel element of the ShortcutControl. This is the externally visible UI element which can be used to add it to other layouts StackPanel getShortcutControl(); diff --git a/src/modules/keyboardmanager/ui/SingleKeyRemapControl.cpp b/src/modules/keyboardmanager/ui/SingleKeyRemapControl.cpp index 6c39e4a7ac..2cdf31d201 100644 --- a/src/modules/keyboardmanager/ui/SingleKeyRemapControl.cpp +++ b/src/modules/keyboardmanager/ui/SingleKeyRemapControl.cpp @@ -72,7 +72,7 @@ void SingleKeyRemapControl::UpdateAccessibleNames(StackPanel sourceColumn, Stack } // Function to add a new row to the remap keys table. If the originalKey and newKey args are provided, then the displayed remap keys are set to those values. -void SingleKeyRemapControl::AddNewControlKeyRemapRow(Grid& parent, std::vector>>& keyboardRemapControlObjects, const DWORD originalKey, const std::variant newKey) +void SingleKeyRemapControl::AddNewControlKeyRemapRow(Grid& parent, std::vector>>& keyboardRemapControlObjects, const DWORD originalKey, const KeyShortcutUnion newKey) { // Create new SingleKeyRemapControl objects dynamically so that we does not get destructed std::vector> newrow; diff --git a/src/modules/keyboardmanager/ui/SingleKeyRemapControl.h b/src/modules/keyboardmanager/ui/SingleKeyRemapControl.h index 9e70f28a23..0961df4008 100644 --- a/src/modules/keyboardmanager/ui/SingleKeyRemapControl.h +++ b/src/modules/keyboardmanager/ui/SingleKeyRemapControl.h @@ -43,7 +43,7 @@ public: SingleKeyRemapControl(Grid table, const int colIndex); // Function to add a new row to the remap keys table. If the originalKey and newKey args are provided, then the displayed remap keys are set to those values. - static void AddNewControlKeyRemapRow(winrt::Windows::UI::Xaml::Controls::Grid& parent, std::vector>>& keyboardRemapControlObjects, const DWORD originalKey = NULL, const std::variant newKey = NULL); + static void AddNewControlKeyRemapRow(winrt::Windows::UI::Xaml::Controls::Grid& parent, std::vector>>& keyboardRemapControlObjects, const DWORD originalKey = NULL, const KeyShortcutUnion newKey = NULL); // Function to return the stack panel element of the SingleKeyRemapControl. This is the externally visible UI element which can be used to add it to other layouts winrt::Windows::UI::Xaml::Controls::StackPanel getSingleKeyRemapControl();