From 37b76cd1caed209e20e0e353671abf6ba4346b42 Mon Sep 17 00:00:00 2001 From: rszyma Date: Mon, 1 Jan 2024 18:29:51 +0100 Subject: [PATCH] keybinds: fix keys getting stuck + minor refactor & optimizations to keybind handling (#4304) --- src/config/ConfigManager.cpp | 2 +- src/managers/KeybindManager.cpp | 160 ++++++++++++++++++++++---------- src/managers/KeybindManager.hpp | 53 ++++++----- 3 files changed, 142 insertions(+), 73 deletions(-) diff --git a/src/config/ConfigManager.cpp b/src/config/ConfigManager.cpp index ee198ded..28959128 100644 --- a/src/config/ConfigManager.cpp +++ b/src/config/ConfigManager.cpp @@ -935,7 +935,7 @@ void CConfigManager::handleBind(const std::string& command, const std::string& v g_pKeybindManager->addKeybind( SKeybind{"", std::stoi(KEY.substr(5)), MOD, HANDLER, COMMAND, locked, m_szCurrentSubmap, release, repeat, mouse, nonConsuming, transparent, ignoreMods}); else - g_pKeybindManager->addKeybind(SKeybind{KEY, -1, MOD, HANDLER, COMMAND, locked, m_szCurrentSubmap, release, repeat, mouse, nonConsuming, transparent, ignoreMods}); + g_pKeybindManager->addKeybind(SKeybind{KEY, 0, MOD, HANDLER, COMMAND, locked, m_szCurrentSubmap, release, repeat, mouse, nonConsuming, transparent, ignoreMods}); } } diff --git a/src/managers/KeybindManager.cpp b/src/managers/KeybindManager.cpp index 899003d8..d379a408 100644 --- a/src/managers/KeybindManager.cpp +++ b/src/managers/KeybindManager.cpp @@ -7,6 +7,7 @@ #include #include +#include #if defined(__linux__) #include #elif defined(__NetBSD__) || defined(__OpenBSD__) @@ -95,7 +96,7 @@ void CKeybindManager::addKeybind(SKeybind kb) { void CKeybindManager::removeKeybind(uint32_t mod, const std::string& key) { for (auto it = m_lKeybinds.begin(); it != m_lKeybinds.end(); ++it) { if (isNumber(key) && std::stoi(key) > 9) { - const auto KEYNUM = std::stoi(key); + const uint32_t KEYNUM = std::stoi(key); if (it->modmask == mod && it->keycode == KEYNUM) { it = m_lKeybinds.erase(it); @@ -137,6 +138,22 @@ uint32_t CKeybindManager::stringToModMask(std::string mods) { return modMask; } +uint32_t CKeybindManager::keycodeToModifier(xkb_keycode_t keycode) { + switch (keycode - 8) { + case KEY_LEFTMETA: return WLR_MODIFIER_LOGO; + case KEY_RIGHTMETA: return WLR_MODIFIER_LOGO; + case KEY_LEFTSHIFT: return WLR_MODIFIER_SHIFT; + case KEY_RIGHTSHIFT: return WLR_MODIFIER_SHIFT; + case KEY_LEFTCTRL: return WLR_MODIFIER_CTRL; + case KEY_RIGHTCTRL: return WLR_MODIFIER_CTRL; + case KEY_LEFTALT: return WLR_MODIFIER_ALT; + case KEY_RIGHTALT: return WLR_MODIFIER_ALT; + case KEY_CAPSLOCK: return WLR_MODIFIER_CAPS; + case KEY_NUMLOCK: return WLR_MODIFIER_MOD2; + default: return 0; + } +} + void CKeybindManager::updateXKBTranslationState() { if (m_pXKBTranslationState) { xkb_keymap_unref(xkb_state_get_keymap(m_pXKBTranslationState)); @@ -261,8 +278,7 @@ void CKeybindManager::switchToWindow(CWindow* PWINDOWTOCHANGETO) { bool CKeybindManager::onKeyEvent(wlr_keyboard_key_event* e, SKeyboard* pKeyboard) { if (!g_pCompositor->m_bSessionActive || g_pCompositor->m_bUnsafeState) { - m_dPressedKeycodes.clear(); - m_dPressedKeysyms.clear(); + m_dPressedKeys.clear(); return true; } @@ -291,7 +307,13 @@ bool CKeybindManager::onKeyEvent(wlr_keyboard_key_event* e, SKeyboard* pKeyboard m_uLastCode = KEYCODE; m_uLastMouseCode = 0; - bool mouseBindWasActive = ensureMouseBindState(); + bool mouseBindWasActive = ensureMouseBindState(); + + const auto KEY = SPressedKeyWithMods{ + .keysym = keysym, + .keycode = KEYCODE, + .modmaskAtPressTime = MODS, + }; bool found = false; if (e->state == WL_KEYBOARD_KEY_STATE_PRESSED) { @@ -302,16 +324,13 @@ bool CKeybindManager::onKeyEvent(wlr_keyboard_key_event* e, SKeyboard* pKeyboard m_pActiveKeybind = nullptr; } - m_dPressedKeycodes.push_back(KEYCODE); - m_dPressedKeysyms.push_back(keysym); + m_dPressedKeys.push_back(KEY); - found = handleKeybinds(MODS, "", keysym, 0, true, e->time_msec) || found; - - found = handleKeybinds(MODS, "", 0, KEYCODE, true, e->time_msec) || found; + found = handleKeybinds(MODS, KEY, true); if (found) shadowKeybinds(keysym, KEYCODE); - } else if (e->state == WL_KEYBOARD_KEY_STATE_RELEASED) { + } else { // key release // clean repeat if (m_pActiveKeybindEventSource) { wl_event_source_remove(m_pActiveKeybindEventSource); @@ -319,12 +338,23 @@ bool CKeybindManager::onKeyEvent(wlr_keyboard_key_event* e, SKeyboard* pKeyboard m_pActiveKeybind = nullptr; } - m_dPressedKeycodes.erase(std::remove(m_dPressedKeycodes.begin(), m_dPressedKeycodes.end(), KEYCODE), m_dPressedKeycodes.end()); - m_dPressedKeysyms.erase(std::remove(m_dPressedKeysyms.begin(), m_dPressedKeysyms.end(), keysym), m_dPressedKeysyms.end()); - - found = handleKeybinds(MODS, "", keysym, 0, false, e->time_msec) || found; - - found = handleKeybinds(MODS, "", 0, KEYCODE, false, e->time_msec) || found; + bool foundInPressedKeys = false; + for (auto it = m_dPressedKeys.begin(); it != m_dPressedKeys.end();) { + if (it->keycode == KEYCODE) { + if (!foundInPressedKeys) { + found = handleKeybinds(MODS, *it, false); + foundInPressedKeys = true; + } + it = m_dPressedKeys.erase(it); + } else { + ++it; + } + } + if (!foundInPressedKeys) { + Debug::log(ERR, "BUG THIS: key not found in m_dPressedKeys"); + // fallback with wrong `KEY.modmaskAtPressTime`, this can be buggy + found = handleKeybinds(MODS, KEY, false); + } shadowKeybinds(); } @@ -347,14 +377,14 @@ bool CKeybindManager::onAxisEvent(wlr_pointer_axis_event* e) { bool found = false; if (e->source == WLR_AXIS_SOURCE_WHEEL && e->orientation == WLR_AXIS_ORIENTATION_VERTICAL) { if (e->delta < 0) - found = handleKeybinds(MODS, "mouse_down", 0, 0, true, 0); + found = handleKeybinds(MODS, SPressedKeyWithMods{.keyName = "mouse_down"}, true); else - found = handleKeybinds(MODS, "mouse_up", 0, 0, true, 0); + found = handleKeybinds(MODS, SPressedKeyWithMods{.keyName = "mouse_up"}, true); } else if (e->source == WLR_AXIS_SOURCE_WHEEL && e->orientation == WLR_AXIS_ORIENTATION_HORIZONTAL) { if (e->delta < 0) - found = handleKeybinds(MODS, "mouse_left", 0, 0, true, 0); + found = handleKeybinds(MODS, SPressedKeyWithMods{.keyName = "mouse_left"}, true); else - found = handleKeybinds(MODS, "mouse_right", 0, 0, true, 0); + found = handleKeybinds(MODS, SPressedKeyWithMods{.keyName = "mouse_right"}, true); } if (found) @@ -372,15 +402,40 @@ bool CKeybindManager::onMouseEvent(wlr_pointer_button_event* e) { m_uLastCode = 0; m_uTimeLastMs = e->time_msec; - bool mouseBindWasActive = ensureMouseBindState(); + bool mouseBindWasActive = ensureMouseBindState(); + + const auto KEY_NAME = "mouse:" + std::to_string(e->button); + + const auto KEY = SPressedKeyWithMods{ + .keyName = KEY_NAME, + .modmaskAtPressTime = MODS, + }; if (e->state == WLR_BUTTON_PRESSED) { - found = handleKeybinds(MODS, "mouse:" + std::to_string(e->button), 0, 0, true, 0); + m_dPressedKeys.push_back(KEY); + + found = handleKeybinds(MODS, KEY, true); if (found) shadowKeybinds(); } else { - found = handleKeybinds(MODS, "mouse:" + std::to_string(e->button), 0, 0, false, 0); + bool foundInPressedKeys = false; + for (auto it = m_dPressedKeys.begin(); it != m_dPressedKeys.end();) { + if (it->keyName == KEY_NAME) { + if (!foundInPressedKeys) { + found = handleKeybinds(MODS, *it, false); + foundInPressedKeys = true; + } + it = m_dPressedKeys.erase(it); + } else { + ++it; + } + } + if (!foundInPressedKeys) { + Debug::log(ERR, "BUG THIS: key not found in m_dPressedKeys (2)"); + // fallback with wrong `KEY.modmaskAtPressTime`, this can be buggy + found = handleKeybinds(MODS, KEY, false); + } shadowKeybinds(); } @@ -397,15 +452,15 @@ void CKeybindManager::resizeWithBorder(wlr_pointer_button_event* e) { } void CKeybindManager::onSwitchEvent(const std::string& switchName) { - handleKeybinds(0, "switch:" + switchName, 0, 0, true, 0); + handleKeybinds(0, SPressedKeyWithMods{.keyName = "switch:" + switchName}, true); } void CKeybindManager::onSwitchOnEvent(const std::string& switchName) { - handleKeybinds(0, "switch:on:" + switchName, 0, 0, true, 0); + handleKeybinds(0, SPressedKeyWithMods{.keyName = "switch:on:" + switchName}, true); } void CKeybindManager::onSwitchOffEvent(const std::string& switchName) { - handleKeybinds(0, "switch:off:" + switchName, 0, 0, true, 0); + handleKeybinds(0, SPressedKeyWithMods{.keyName = "switch:off:" + switchName}, true); } int repeatKeyHandler(void* data) { @@ -424,7 +479,7 @@ int repeatKeyHandler(void* data) { return 0; } -bool CKeybindManager::handleKeybinds(const uint32_t& modmask, const std::string& key, const xkb_keysym_t& keysym, const int& keycode, bool pressed, uint32_t time) { +bool CKeybindManager::handleKeybinds(const uint32_t modmask, const SPressedKeyWithMods& key, bool pressed) { bool found = false; if (g_pCompositor->m_sSeat.exclusiveClient) @@ -441,22 +496,19 @@ bool CKeybindManager::handleKeybinds(const uint32_t& modmask, const std::string& ((modmask != k.modmask && !k.ignoreMods) || (g_pCompositor->m_sSeat.exclusiveClient && !k.locked) || k.submap != m_szCurrentSelectedSubmap || k.shadowed)) continue; - if (!key.empty()) { - if (key != k.key) + if (!key.keyName.empty()) { + if (key.keyName != k.key) continue; - } else if (k.keycode != -1) { - if (keycode != k.keycode) + } else if (k.keycode != 0) { + if (key.keycode != k.keycode) continue; } else { - if (keysym == 0) - continue; // this is a keycode check run - // oMg such performance hit!!11! // this little maneouver is gonna cost us 4µs const auto KBKEY = xkb_keysym_from_name(k.key.c_str(), XKB_KEYSYM_CASE_INSENSITIVE); const auto KBKEYUPPER = xkb_keysym_to_upper(KBKEY); - if (keysym != KBKEY && keysym != KBKEYUPPER) + if (key.keysym != KBKEY && key.keysym != KBKEYUPPER) continue; } @@ -468,12 +520,24 @@ bool CKeybindManager::handleKeybinds(const uint32_t& modmask, const std::string& continue; } - if (!pressed && !k.release && !SPECIALDISPATCHER) { - if (k.nonConsuming) - continue; + if (!pressed) { + // Require mods to be matching when the key was first pressed. + if (key.modmaskAtPressTime != modmask) { + // Handle properly `bindr` where a key is itself a bind mod for example: + // "bindr = SUPER, SUPER_L, exec, $launcher". + // This needs to be handled separately for the above case, because `key.modmaskAtPressTime` is set + // from currently pressed keys as programs see them, but it doesn't yet include the currently + // pressed mod key, which is still being handled internally. + if (keycodeToModifier(key.keycode) == key.modmaskAtPressTime) + continue; - found = true; // suppress the event - continue; + } else if (!k.release && !SPECIALDISPATCHER) { + if (k.nonConsuming) + continue; + + found = true; // suppress the event + continue; + } } const auto DISPATCHER = m_mDispatchers.find(k.mouse ? "mouse" : k.handler); @@ -488,7 +552,7 @@ bool CKeybindManager::handleKeybinds(const uint32_t& modmask, const std::string& Debug::log(ERR, "Invalid handler in a keybind! (handler {} does not exist)", k.handler); } else { // call the dispatcher - Debug::log(LOG, "Keybind triggered, calling dispatcher ({}, {}, {})", modmask, key, keysym); + Debug::log(LOG, "Keybind triggered, calling dispatcher ({}, {}, {})", modmask, key.keyName, key.keysym); m_iPassPressed = (int)pressed; @@ -521,7 +585,7 @@ bool CKeybindManager::handleKeybinds(const uint32_t& modmask, const std::string& return found; } -void CKeybindManager::shadowKeybinds(const xkb_keysym_t& doesntHave, const int& doesntHaveCode) { +void CKeybindManager::shadowKeybinds(const xkb_keysym_t& doesntHave, const uint32_t doesntHaveCode) { // shadow disables keybinds after one has been triggered for (auto& k : m_lKeybinds) { @@ -534,22 +598,20 @@ void CKeybindManager::shadowKeybinds(const xkb_keysym_t& doesntHave, const int& const auto KBKEY = xkb_keysym_from_name(k.key.c_str(), XKB_KEYSYM_CASE_INSENSITIVE); const auto KBKEYUPPER = xkb_keysym_to_upper(KBKEY); - for (auto& pk : m_dPressedKeysyms) { - if ((pk == KBKEY || pk == KBKEYUPPER)) { + for (auto& pk : m_dPressedKeys) { + if ((pk.keysym != 0 && (pk.keysym == KBKEY || pk.keysym == KBKEYUPPER))) { shadow = true; - if (pk == doesntHave && doesntHave != 0) { + if (pk.keysym == doesntHave && doesntHave != 0) { shadow = false; break; } } - } - for (auto& pk : m_dPressedKeycodes) { - if (pk == k.keycode) { + if (pk.keycode != 0 && pk.keycode == k.keycode) { shadow = true; - if (pk == doesntHaveCode && doesntHaveCode != 0 && doesntHaveCode != -1) { + if (pk.keycode == doesntHaveCode && doesntHaveCode != 0) { shadow = false; break; } diff --git a/src/managers/KeybindManager.hpp b/src/managers/KeybindManager.hpp index 73997dd6..631b0147 100644 --- a/src/managers/KeybindManager.hpp +++ b/src/managers/KeybindManager.hpp @@ -12,7 +12,7 @@ class CPluginSystem; struct SKeybind { std::string key = ""; - int keycode = -1; + uint32_t keycode = 0; uint32_t modmask = 0; std::string handler = ""; std::string arg = ""; @@ -36,6 +36,13 @@ enum eFocusWindowMode { MODE_PID }; +struct SPressedKeyWithMods { + std::string keyName = ""; + xkb_keysym_t keysym = 0; + uint32_t keycode = 0; + uint32_t modmaskAtPressTime = 0; +}; + class CKeybindManager { public: CKeybindManager(); @@ -51,8 +58,9 @@ class CKeybindManager { void addKeybind(SKeybind); void removeKeybind(uint32_t, const std::string&); uint32_t stringToModMask(std::string); + uint32_t keycodeToModifier(xkb_keycode_t); void clearKeybinds(); - void shadowKeybinds(const xkb_keysym_t& doesntHave = 0, const int& doesntHaveCode = 0); + void shadowKeybinds(const xkb_keysym_t& doesntHave = 0, const uint32_t doesntHaveCode = 0); std::unordered_map> m_mDispatchers; @@ -63,38 +71,37 @@ class CKeybindManager { std::list m_lKeybinds; private: - std::deque m_dPressedKeysyms; - std::deque m_dPressedKeycodes; + std::deque m_dPressedKeys; - inline static std::string m_szCurrentSelectedSubmap = ""; + inline static std::string m_szCurrentSelectedSubmap = ""; - SKeybind* m_pActiveKeybind = nullptr; + SKeybind* m_pActiveKeybind = nullptr; - uint32_t m_uTimeLastMs = 0; - uint32_t m_uLastCode = 0; - uint32_t m_uLastMouseCode = 0; + uint32_t m_uTimeLastMs = 0; + uint32_t m_uLastCode = 0; + uint32_t m_uLastMouseCode = 0; - bool m_bIsMouseBindActive = false; - std::vector m_vPressedSpecialBinds; + bool m_bIsMouseBindActive = false; + std::vector m_vPressedSpecialBinds; - int m_iPassPressed = -1; // used for pass + int m_iPassPressed = -1; // used for pass - CTimer m_tScrollTimer; + CTimer m_tScrollTimer; - bool handleKeybinds(const uint32_t&, const std::string&, const xkb_keysym_t&, const int&, bool, uint32_t); + bool handleKeybinds(const uint32_t, const SPressedKeyWithMods&, bool); - bool handleInternalKeybinds(xkb_keysym_t); - bool handleVT(xkb_keysym_t); + bool handleInternalKeybinds(xkb_keysym_t); + bool handleVT(xkb_keysym_t); - xkb_state* m_pXKBTranslationState = nullptr; + xkb_state* m_pXKBTranslationState = nullptr; - void updateXKBTranslationState(); - bool ensureMouseBindState(); + void updateXKBTranslationState(); + bool ensureMouseBindState(); - static bool tryMoveFocusToMonitor(CMonitor* monitor); - static void moveWindowOutOfGroup(CWindow* pWindow, const std::string& dir = ""); - static void moveWindowIntoGroup(CWindow* pWindow, CWindow* pWindowInDirection); - static void switchToWindow(CWindow* PWINDOWTOCHANGETO); + static bool tryMoveFocusToMonitor(CMonitor* monitor); + static void moveWindowOutOfGroup(CWindow* pWindow, const std::string& dir = ""); + static void moveWindowIntoGroup(CWindow* pWindow, CWindow* pWindowInDirection); + static void switchToWindow(CWindow* PWINDOWTOCHANGETO); // -------------- Dispatchers -------------- // static void killActive(std::string);