From d4110fd2ab680364b265704cba9165d29208c33b Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Wed, 16 Jul 2025 19:25:13 +0530 Subject: [PATCH] linux: Fix spacebar not working with multiple keyboard layouts (#34514) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #26468 #16667 This PR fixes the spacebar not working with multiple keyboard layouts on Linux X11. I have tested this with Czech, Russian, German, German Neo 2, etc. It seems to work correctly. `XkbStateNotify` events correctly update XKB state with complete modifier info (depressed/latched/locked), but `KeyPress/KeyRelease` events immediately overwrite that state using `update_mask()` with only raw X11 modifier bits. This breaks xkb state as we reset `latched_mods` and `locked_mods` to 0, as well as we might not correctly handle cases where this new xkb state needs to change. Previous logic is flawed because `KeyPress/KeyRelease` event only gives you depressed modifiers (`event.state`) and not others, which we try to fill in from `previous_xkb_state`. This patch was introduced to fix capitalization issue with Neo 2 (https://github.com/zed-industries/zed/pull/14466) and later to fix wrong keys with German layout (https://github.com/zed-industries/zed/pull/31193), both of which I have tested this PR with. Now, instead of manually managing XKB state, we use the `update_key` method, which internally handles modifier states and other cases we might have missed. From `update_key` docs: > Update the keyboard state to reflect a given key being pressed or released. > > This entry point is intended for programs which track the keyboard state explictly (like an evdev client). If the state is serialized to you by a master process (like a Wayland compositor) using functions like `xkb_state_serialize_mods()`, you should use `xkb_state_update_mask()` instead. **_The two functins should not generally be used together._** > > A series of calls to this function should be consistent; that is, a call with `xkb::KEY_DOWN` for a key should be matched by an `xkb::KEY_UP`; if a key is pressed twice, it should be released twice; etc. Otherwise (e.g. due to missed input events), situations like "stuck modifiers" may occur. > > This function is often used in conjunction with the function `xkb_state_key_get_syms()` (or `xkb_state_key_get_one_sym()`), for example, when handling a key event. In this case, you should prefer to get the keysyms *before* updating the key, such that the keysyms reported for the key event are not affected by the event itself. This is the conventional behavior. Release Notes: - Fix the issue where the spacebar doesn’t work with multiple keyboard layouts on Linux X11. --- crates/gpui/src/platform/linux/x11/client.rs | 70 +++++--------------- 1 file changed, 18 insertions(+), 52 deletions(-) diff --git a/crates/gpui/src/platform/linux/x11/client.rs b/crates/gpui/src/platform/linux/x11/client.rs index 6cff977128ec594d683085e5f2cc24683c9e9ba7..0606f619c6fb808e4be42abe07f51d1e124a69f4 100644 --- a/crates/gpui/src/platform/linux/x11/client.rs +++ b/crates/gpui/src/platform/linux/x11/client.rs @@ -1,23 +1,22 @@ use crate::{Capslock, xcb_flush}; -use core::str; -use std::{ - cell::RefCell, - collections::{BTreeMap, HashSet}, - ops::Deref, - path::PathBuf, - rc::{Rc, Weak}, - time::{Duration, Instant}, -}; - use anyhow::{Context as _, anyhow}; use calloop::{ EventLoop, LoopHandle, RegistrationToken, generic::{FdWrapper, Generic}, }; use collections::HashMap; +use core::str; use http_client::Url; use log::Level; use smallvec::SmallVec; +use std::{ + cell::RefCell, + collections::{BTreeMap, HashSet}, + ops::Deref, + path::PathBuf, + rc::{Rc, Weak}, + time::{Duration, Instant}, +}; use util::ResultExt; use x11rb::{ @@ -38,7 +37,7 @@ use x11rb::{ }; use xim::{AttributeName, Client, InputStyle, x11rb::X11rbClient}; use xkbc::x11::ffi::{XKB_X11_MIN_MAJOR_XKB_VERSION, XKB_X11_MIN_MINOR_XKB_VERSION}; -use xkbcommon::xkb::{self as xkbc, LayoutIndex, ModMask, STATE_LAYOUT_EFFECTIVE}; +use xkbcommon::xkb::{self as xkbc, STATE_LAYOUT_EFFECTIVE}; use super::{ ButtonOrScroll, ScrollDirection, X11Display, X11WindowStatePtr, XcbAtoms, XimCallbackEvent, @@ -141,13 +140,6 @@ impl From for EventHandlerError { } } -#[derive(Debug, Default, Clone)] -struct XKBStateNotiy { - depressed_layout: LayoutIndex, - latched_layout: LayoutIndex, - locked_layout: LayoutIndex, -} - #[derive(Debug, Default)] pub struct Xdnd { other_window: xproto::Window, @@ -200,7 +192,6 @@ pub struct X11ClientState { pub(crate) mouse_focused_window: Option, pub(crate) keyboard_focused_window: Option, pub(crate) xkb: xkbc::State, - previous_xkb_state: XKBStateNotiy, keyboard_layout: LinuxKeyboardLayout, pub(crate) ximc: Option>>, pub(crate) xim_handler: Option, @@ -507,7 +498,6 @@ impl X11Client { mouse_focused_window: None, keyboard_focused_window: None, xkb: xkb_state, - previous_xkb_state: XKBStateNotiy::default(), keyboard_layout, ximc, xim_handler, @@ -959,14 +949,6 @@ impl X11Client { state.xkb_device_id, ) }; - let depressed_layout = xkb_state.serialize_layout(xkbc::STATE_LAYOUT_DEPRESSED); - let latched_layout = xkb_state.serialize_layout(xkbc::STATE_LAYOUT_LATCHED); - let locked_layout = xkb_state.serialize_layout(xkbc::ffi::XKB_STATE_LAYOUT_LOCKED); - state.previous_xkb_state = XKBStateNotiy { - depressed_layout, - latched_layout, - locked_layout, - }; state.xkb = xkb_state; drop(state); self.handle_keyboard_layout_change(); @@ -983,12 +965,6 @@ impl X11Client { event.latched_group as u32, event.locked_group.into(), ); - state.previous_xkb_state = XKBStateNotiy { - depressed_layout: event.base_group as u32, - latched_layout: event.latched_group as u32, - locked_layout: event.locked_group.into(), - }; - let modifiers = Modifiers::from_xkb(&state.xkb); let capslock = Capslock::from_xkb(&state.xkb); if state.last_modifiers_changed_event == modifiers @@ -1025,17 +1001,12 @@ impl X11Client { state.pre_key_char_down.take(); let keystroke = { let code = event.detail.into(); - let xkb_state = state.previous_xkb_state.clone(); - state.xkb.update_mask( - event.state.bits() as ModMask, - 0, - 0, - xkb_state.depressed_layout, - xkb_state.latched_layout, - xkb_state.locked_layout, - ); let mut keystroke = crate::Keystroke::from_xkb(&state.xkb, modifiers, code); let keysym = state.xkb.key_get_one_sym(code); + + // should be called after key_get_one_sym + state.xkb.update_key(code, xkbc::KeyDirection::Down); + if keysym.is_modifier_key() { return Some(()); } @@ -1093,17 +1064,12 @@ impl X11Client { let keystroke = { let code = event.detail.into(); - let xkb_state = state.previous_xkb_state.clone(); - state.xkb.update_mask( - event.state.bits() as ModMask, - 0, - 0, - xkb_state.depressed_layout, - xkb_state.latched_layout, - xkb_state.locked_layout, - ); let keystroke = crate::Keystroke::from_xkb(&state.xkb, modifiers, code); let keysym = state.xkb.key_get_one_sym(code); + + // should be called after key_get_one_sym + state.xkb.update_key(code, xkbc::KeyDirection::Up); + if keysym.is_modifier_key() { return Some(()); }