From 535f2cb15fa7a8d6a828e43787effd934232805c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 21:37:41 +0000 Subject: [PATCH 1/4] Initial plan From 6a87c988781fc39b2fbed63e34557585916f8c58 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 22:14:10 +0000 Subject: [PATCH 2/4] Fix altwin:swap_alt_win modifier tracking by querying XKB state Instead of manually tracking modifiers based on scan codes, query XKB's modifier state after each key event. This ensures XKB options like altwin:swap_alt_win are properly respected. - Add XKB modifier index caching in XkbMappingState constructor - Implement update_modifier_from_xkb_state() to query XKB for active modifiers - Track pressed scancodes to determine left/right modifier distinction - Remove reliance on modifier_from_xkb_scan_code() for modifier tracking Co-authored-by: mattkae <25062299+mattkae@users.noreply.github.com> --- include/common/mir/input/xkb_mapper.h | 10 +++ src/common/input/xkb_mapper.cpp | 115 ++++++++++++++++++++++---- 2 files changed, 111 insertions(+), 14 deletions(-) diff --git a/include/common/mir/input/xkb_mapper.h b/include/common/mir/input/xkb_mapper.h index 7592a3ec213..93d38b11cf6 100644 --- a/include/common/mir/input/xkb_mapper.h +++ b/include/common/mir/input/xkb_mapper.h @@ -97,11 +97,21 @@ class XKBMapper : public KeyMapper std::string& text) -> std::pair; void press_modifier(MirInputEventModifiers mod); void release_modifier(MirInputEventModifiers mod); + void update_modifier_from_xkb_state(); std::shared_ptr const keymap; std::shared_ptr const compiled_keymap; XKBStatePtr state; MirInputEventModifiers modifier_state{0}; + std::unordered_set pressed_scancodes; + + // Cached XKB modifier indices + xkb_mod_index_t mod_index_shift{XKB_MOD_INVALID}; + xkb_mod_index_t mod_index_ctrl{XKB_MOD_INVALID}; + xkb_mod_index_t mod_index_alt{XKB_MOD_INVALID}; + xkb_mod_index_t mod_index_logo{XKB_MOD_INVALID}; + xkb_mod_index_t mod_index_caps{XKB_MOD_INVALID}; + xkb_mod_index_t mod_index_num{XKB_MOD_INVALID}; }; XkbMappingState* get_keymapping_state(MirInputDeviceId id); diff --git a/src/common/input/xkb_mapper.cpp b/src/common/input/xkb_mapper.cpp index 0bbebcf0ecb..0c0d4a5d9ba 100644 --- a/src/common/input/xkb_mapper.cpp +++ b/src/common/input/xkb_mapper.cpp @@ -300,6 +300,13 @@ mircv::XKBMapper::XkbMappingState::XkbMappingState( compiled_keymap{std::move(compiled_keymap)}, state{make_unique_state(this->compiled_keymap.get())} { + // Cache XKB modifier indices for efficient lookup + mod_index_shift = xkb_keymap_mod_get_index(this->compiled_keymap.get(), XKB_MOD_NAME_SHIFT); + mod_index_ctrl = xkb_keymap_mod_get_index(this->compiled_keymap.get(), XKB_MOD_NAME_CTRL); + mod_index_alt = xkb_keymap_mod_get_index(this->compiled_keymap.get(), XKB_MOD_NAME_ALT); + mod_index_logo = xkb_keymap_mod_get_index(this->compiled_keymap.get(), XKB_MOD_NAME_LOGO); + mod_index_caps = xkb_keymap_mod_get_index(this->compiled_keymap.get(), XKB_MOD_NAME_CAPS); + mod_index_num = xkb_keymap_mod_get_index(this->compiled_keymap.get(), XKB_MOD_NAME_NUM); } void mircv::XKBMapper::XkbMappingState::set_key_state(std::vector const& key_state) @@ -365,18 +372,10 @@ auto mircv::XKBMapper::XkbMappingState::update_state( -> std::pair { auto keysym = xkb_state_key_get_one_sym(state.get(), scan_code); - auto const mod_change = modifier_from_xkb_scan_code(scan_code); - - // Occasionally, we see XKB_KEY_Meta_L where XKB_KEY_Alt_L is correct - if (mod_change == mir_input_event_modifier_alt_left) - { - keysym = XKB_KEY_Alt_L; - } if(action == mir_keyboard_action_down || action == mir_keyboard_action_repeat) { char buffer[7]; - // scan code? really? not keysym? xkb_state_key_get_utf8(state.get(), scan_code, buffer, sizeof(buffer)); text = buffer; } @@ -388,16 +387,16 @@ auto mircv::XKBMapper::XkbMappingState::update_state( if (action == mir_keyboard_action_up) { mask = xkb_state_update_key(state.get(), scan_code, XKB_KEY_UP); - // TODO get the modifier state from xkbcommon and apply it - // for all other modifiers manually track them here: - release_modifier(mod_change); + pressed_scancodes.erase(scan_code); + // Query XKB for the actual modifier state after key update + update_modifier_from_xkb_state(); } else if (action == mir_keyboard_action_down) { mask = xkb_state_update_key(state.get(), scan_code, XKB_KEY_DOWN); - // TODO get the modifier state from xkbcommon and apply it - // for all other modifiers manually track them here: - press_modifier(mod_change); + pressed_scancodes.insert(scan_code); + // Query XKB for the actual modifier state after key update + update_modifier_from_xkb_state(); } bool const xkb_modifiers_changed = @@ -494,6 +493,94 @@ xkb_keysym_t mircv::XKBMapper::ComposeState::update_state(xkb_keysym_t mapped_ke return mapped_key; } +void mircv::XKBMapper::XkbMappingState::update_modifier_from_xkb_state() +{ + // Query XKB for the actual modifier state instead of tracking manually by scan codes + // This ensures that XKB options like altwin:swap_alt_win are properly respected + MirInputEventModifiers new_modifier_state = mir_input_event_modifier_none; + + // Check shift modifier and determine left/right based on which physical keys are pressed + if (mod_index_shift != XKB_MOD_INVALID && + xkb_state_mod_index_is_active(state.get(), mod_index_shift, XKB_STATE_MODS_EFFECTIVE)) + { + new_modifier_state |= mir_input_event_modifier_shift; + // Check which physical shift keys are pressed + if (pressed_scancodes.count(to_xkb_scan_code(KEY_LEFTSHIFT))) + new_modifier_state |= mir_input_event_modifier_shift_left; + if (pressed_scancodes.count(to_xkb_scan_code(KEY_RIGHTSHIFT))) + new_modifier_state |= mir_input_event_modifier_shift_right; + } + + // Check ctrl modifier + if (mod_index_ctrl != XKB_MOD_INVALID && + xkb_state_mod_index_is_active(state.get(), mod_index_ctrl, XKB_STATE_MODS_EFFECTIVE)) + { + new_modifier_state |= mir_input_event_modifier_ctrl; + // Check which physical ctrl keys are pressed + if (pressed_scancodes.count(to_xkb_scan_code(KEY_LEFTCTRL))) + new_modifier_state |= mir_input_event_modifier_ctrl_left; + if (pressed_scancodes.count(to_xkb_scan_code(KEY_RIGHTCTRL))) + new_modifier_state |= mir_input_event_modifier_ctrl_right; + } + + // Check alt modifier - this is where altwin:swap_alt_win affects behavior + // XKB tells us if Alt is active, we determine left/right based on which keys are pressed + if (mod_index_alt != XKB_MOD_INVALID && + xkb_state_mod_index_is_active(state.get(), mod_index_alt, XKB_STATE_MODS_EFFECTIVE)) + { + new_modifier_state |= mir_input_event_modifier_alt; + // With altwin:swap_alt_win, the physical Win keys will activate Alt + // Check all keys that could activate Alt modifier + for (auto scan_code : pressed_scancodes) + { + auto keysym = xkb_state_key_get_one_sym(state.get(), scan_code); + if (keysym == XKB_KEY_Alt_L) + { + new_modifier_state |= mir_input_event_modifier_alt_left; + } + else if (keysym == XKB_KEY_Alt_R) + { + new_modifier_state |= mir_input_event_modifier_alt_right; + } + } + } + + // Check logo/meta/super modifier (Win key) - also affected by altwin:swap_alt_win + if (mod_index_logo != XKB_MOD_INVALID && + xkb_state_mod_index_is_active(state.get(), mod_index_logo, XKB_STATE_MODS_EFFECTIVE)) + { + new_modifier_state |= mir_input_event_modifier_meta; + // With altwin:swap_alt_win, the physical Alt keys will activate Super/Meta + for (auto scan_code : pressed_scancodes) + { + auto keysym = xkb_state_key_get_one_sym(state.get(), scan_code); + if (keysym == XKB_KEY_Super_L || keysym == XKB_KEY_Meta_L) + { + new_modifier_state |= mir_input_event_modifier_meta_left; + } + else if (keysym == XKB_KEY_Super_R || keysym == XKB_KEY_Meta_R) + { + new_modifier_state |= mir_input_event_modifier_meta_right; + } + } + } + + // Check caps lock + if (mod_index_caps != XKB_MOD_INVALID && + xkb_state_mod_index_is_active(state.get(), mod_index_caps, XKB_STATE_MODS_EFFECTIVE)) + { + new_modifier_state |= mir_input_event_modifier_caps_lock; + } + + // Check num lock + if (mod_index_num != XKB_MOD_INVALID && + xkb_state_mod_index_is_active(state.get(), mod_index_num, XKB_STATE_MODS_EFFECTIVE)) + { + new_modifier_state |= mir_input_event_modifier_num_lock; + } + + modifier_state = new_modifier_state; +} void mircv::XKBMapper::XkbMappingState::press_modifier(MirInputEventModifiers mod) { From 1e884e337197d8343bfcf69d279bacbf8dce56f9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 22:19:51 +0000 Subject: [PATCH 3/4] Address code review: Add documentation and optimize modifier loops - Add comprehensive documentation for pressed_scancodes member - Expand comment for cached XKB modifier indices - Combine Alt and Meta modifier loops into single iteration for better performance - Pre-check if Alt/Meta are active before iterating pressed scancodes Co-authored-by: mattkae <25062299+mattkae@users.noreply.github.com> --- include/common/mir/input/xkb_mapper.h | 7 +++- src/common/input/xkb_mapper.cpp | 56 +++++++++++++-------------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/include/common/mir/input/xkb_mapper.h b/include/common/mir/input/xkb_mapper.h index 93d38b11cf6..1b9047548b9 100644 --- a/include/common/mir/input/xkb_mapper.h +++ b/include/common/mir/input/xkb_mapper.h @@ -103,9 +103,14 @@ class XKBMapper : public KeyMapper std::shared_ptr const compiled_keymap; XKBStatePtr state; MirInputEventModifiers modifier_state{0}; + + // Tracks physically pressed keys (as XKB scan codes) to determine which physical keys + // are pressed when querying XKB state. Used to distinguish left/right modifier variants. std::unordered_set pressed_scancodes; - // Cached XKB modifier indices + // Cached XKB modifier indices for performance. These are looked up once during + // construction from standard XKB modifier names and used to efficiently query + // XKB state for active modifiers. xkb_mod_index_t mod_index_shift{XKB_MOD_INVALID}; xkb_mod_index_t mod_index_ctrl{XKB_MOD_INVALID}; xkb_mod_index_t mod_index_alt{XKB_MOD_INVALID}; diff --git a/src/common/input/xkb_mapper.cpp b/src/common/input/xkb_mapper.cpp index 0c0d4a5d9ba..861ab3e17f8 100644 --- a/src/common/input/xkb_mapper.cpp +++ b/src/common/input/xkb_mapper.cpp @@ -523,44 +523,40 @@ void mircv::XKBMapper::XkbMappingState::update_modifier_from_xkb_state() new_modifier_state |= mir_input_event_modifier_ctrl_right; } - // Check alt modifier - this is where altwin:swap_alt_win affects behavior - // XKB tells us if Alt is active, we determine left/right based on which keys are pressed - if (mod_index_alt != XKB_MOD_INVALID && - xkb_state_mod_index_is_active(state.get(), mod_index_alt, XKB_STATE_MODS_EFFECTIVE)) - { - new_modifier_state |= mir_input_event_modifier_alt; - // With altwin:swap_alt_win, the physical Win keys will activate Alt - // Check all keys that could activate Alt modifier - for (auto scan_code : pressed_scancodes) - { - auto keysym = xkb_state_key_get_one_sym(state.get(), scan_code); - if (keysym == XKB_KEY_Alt_L) - { - new_modifier_state |= mir_input_event_modifier_alt_left; - } - else if (keysym == XKB_KEY_Alt_R) - { - new_modifier_state |= mir_input_event_modifier_alt_right; - } - } - } + // Check alt and meta modifiers - these are where altwin:swap_alt_win affects behavior + // XKB tells us if Alt/Meta are active, we determine left/right based on keysyms of pressed keys + bool const alt_active = mod_index_alt != XKB_MOD_INVALID && + xkb_state_mod_index_is_active(state.get(), mod_index_alt, XKB_STATE_MODS_EFFECTIVE); + bool const meta_active = mod_index_logo != XKB_MOD_INVALID && + xkb_state_mod_index_is_active(state.get(), mod_index_logo, XKB_STATE_MODS_EFFECTIVE); - // Check logo/meta/super modifier (Win key) - also affected by altwin:swap_alt_win - if (mod_index_logo != XKB_MOD_INVALID && - xkb_state_mod_index_is_active(state.get(), mod_index_logo, XKB_STATE_MODS_EFFECTIVE)) + if (alt_active || meta_active) { - new_modifier_state |= mir_input_event_modifier_meta; - // With altwin:swap_alt_win, the physical Alt keys will activate Super/Meta + if (alt_active) + new_modifier_state |= mir_input_event_modifier_alt; + if (meta_active) + new_modifier_state |= mir_input_event_modifier_meta; + + // Single iteration to check all pressed keys for both Alt and Meta modifiers + // With altwin:swap_alt_win, physical Win keys produce Alt keysyms, physical Alt keys produce Meta keysyms for (auto scan_code : pressed_scancodes) { auto keysym = xkb_state_key_get_one_sym(state.get(), scan_code); - if (keysym == XKB_KEY_Super_L || keysym == XKB_KEY_Meta_L) + + if (alt_active) { - new_modifier_state |= mir_input_event_modifier_meta_left; + if (keysym == XKB_KEY_Alt_L) + new_modifier_state |= mir_input_event_modifier_alt_left; + else if (keysym == XKB_KEY_Alt_R) + new_modifier_state |= mir_input_event_modifier_alt_right; } - else if (keysym == XKB_KEY_Super_R || keysym == XKB_KEY_Meta_R) + + if (meta_active) { - new_modifier_state |= mir_input_event_modifier_meta_right; + if (keysym == XKB_KEY_Super_L || keysym == XKB_KEY_Meta_L) + new_modifier_state |= mir_input_event_modifier_meta_left; + else if (keysym == XKB_KEY_Super_R || keysym == XKB_KEY_Meta_R) + new_modifier_state |= mir_input_event_modifier_meta_right; } } } From fc9352864283c1bce5b8749c64c31384690cc682 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 22:22:03 +0000 Subject: [PATCH 4/4] Polish: Use find() instead of count() and add clarifying comment - Use find() instead of count() for existence checks (more efficient) - Add detailed comment explaining why we query keysyms for left/right detection Co-authored-by: mattkae <25062299+mattkae@users.noreply.github.com> --- src/common/input/xkb_mapper.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/common/input/xkb_mapper.cpp b/src/common/input/xkb_mapper.cpp index 861ab3e17f8..17abe2c3fe3 100644 --- a/src/common/input/xkb_mapper.cpp +++ b/src/common/input/xkb_mapper.cpp @@ -505,9 +505,9 @@ void mircv::XKBMapper::XkbMappingState::update_modifier_from_xkb_state() { new_modifier_state |= mir_input_event_modifier_shift; // Check which physical shift keys are pressed - if (pressed_scancodes.count(to_xkb_scan_code(KEY_LEFTSHIFT))) + if (pressed_scancodes.find(to_xkb_scan_code(KEY_LEFTSHIFT)) != pressed_scancodes.end()) new_modifier_state |= mir_input_event_modifier_shift_left; - if (pressed_scancodes.count(to_xkb_scan_code(KEY_RIGHTSHIFT))) + if (pressed_scancodes.find(to_xkb_scan_code(KEY_RIGHTSHIFT)) != pressed_scancodes.end()) new_modifier_state |= mir_input_event_modifier_shift_right; } @@ -517,9 +517,9 @@ void mircv::XKBMapper::XkbMappingState::update_modifier_from_xkb_state() { new_modifier_state |= mir_input_event_modifier_ctrl; // Check which physical ctrl keys are pressed - if (pressed_scancodes.count(to_xkb_scan_code(KEY_LEFTCTRL))) + if (pressed_scancodes.find(to_xkb_scan_code(KEY_LEFTCTRL)) != pressed_scancodes.end()) new_modifier_state |= mir_input_event_modifier_ctrl_left; - if (pressed_scancodes.count(to_xkb_scan_code(KEY_RIGHTCTRL))) + if (pressed_scancodes.find(to_xkb_scan_code(KEY_RIGHTCTRL)) != pressed_scancodes.end()) new_modifier_state |= mir_input_event_modifier_ctrl_right; } @@ -537,8 +537,10 @@ void mircv::XKBMapper::XkbMappingState::update_modifier_from_xkb_state() if (meta_active) new_modifier_state |= mir_input_event_modifier_meta; - // Single iteration to check all pressed keys for both Alt and Meta modifiers - // With altwin:swap_alt_win, physical Win keys produce Alt keysyms, physical Alt keys produce Meta keysyms + // Single iteration to check all pressed keys for both Alt and Meta modifiers. + // We query the keysym of each pressed key because XKB options like altwin:swap_alt_win + // change which keysyms are produced (e.g., physical Win key produces Alt_L keysym). + // This allows us to correctly determine left/right variants after key remapping. for (auto scan_code : pressed_scancodes) { auto keysym = xkb_state_key_get_one_sym(state.get(), scan_code);