linux: Only call `on_keyboard_layout_change` when layout name changes (#32784)

Michael Sloan created

Release Notes:

- N/A

Change summary

crates/gpui/src/platform/linux/headless/client.rs |   2 
crates/gpui/src/platform/linux/keyboard.rs        |  13 +
crates/gpui/src/platform/linux/wayland/client.rs  | 107 +++++++++-------
crates/gpui/src/platform/linux/x11/client.rs      |  58 +++++----
4 files changed, 101 insertions(+), 79 deletions(-)

Detailed changes

crates/gpui/src/platform/linux/headless/client.rs 🔗

@@ -52,7 +52,7 @@ impl LinuxClient for HeadlessClient {
     }
 
     fn keyboard_layout(&self) -> Box<dyn PlatformKeyboardLayout> {
-        Box::new(LinuxKeyboardLayout::new("unknown".to_string()))
+        Box::new(LinuxKeyboardLayout::new("unknown".into()))
     }
 
     fn displays(&self) -> Vec<Rc<dyn PlatformDisplay>> {

crates/gpui/src/platform/linux/keyboard.rs 🔗

@@ -1,21 +1,22 @@
-use crate::PlatformKeyboardLayout;
+use crate::{PlatformKeyboardLayout, SharedString};
 
+#[derive(Clone)]
 pub(crate) struct LinuxKeyboardLayout {
-    id: String,
+    name: SharedString,
 }
 
 impl PlatformKeyboardLayout for LinuxKeyboardLayout {
     fn id(&self) -> &str {
-        &self.id
+        &self.name
     }
 
     fn name(&self) -> &str {
-        &self.id
+        &self.name
     }
 }
 
 impl LinuxKeyboardLayout {
-    pub(crate) fn new(id: String) -> Self {
-        Self { id }
+    pub(crate) fn new(name: SharedString) -> Self {
+        Self { name }
     }
 }

crates/gpui/src/platform/linux/wayland/client.rs 🔗

@@ -71,17 +71,6 @@ use super::{
     window::{ImeInput, WaylandWindowStatePtr},
 };
 
-use crate::platform::linux::{
-    LinuxClient, get_xkb_compose_state, is_within_click_distance, open_uri_internal, read_fd,
-    reveal_path_internal,
-    wayland::{
-        clipboard::{Clipboard, DataOffer, FILE_LIST_MIME_TYPE, TEXT_MIME_TYPES},
-        cursor::Cursor,
-        serial::{SerialKind, SerialTracker},
-        window::WaylandWindow,
-    },
-    xdg_desktop_portal::{Event as XDPEvent, XDPEventSource},
-};
 use crate::platform::{PlatformWindow, blade::BladeContext};
 use crate::{
     AnyWindowHandle, Bounds, CursorStyle, DOUBLE_CLICK_INTERVAL, DevicePixels, DisplayId,
@@ -91,10 +80,26 @@ use crate::{
     PlatformInput, PlatformKeyboardLayout, Point, SCROLL_LINES, ScaledPixels, ScreenCaptureSource,
     ScrollDelta, ScrollWheelEvent, Size, TouchPhase, WindowParams, point, px, size,
 };
+use crate::{
+    SharedString,
+    platform::linux::{
+        LinuxClient, get_xkb_compose_state, is_within_click_distance, open_uri_internal, read_fd,
+        reveal_path_internal,
+        wayland::{
+            clipboard::{Clipboard, DataOffer, FILE_LIST_MIME_TYPE, TEXT_MIME_TYPES},
+            cursor::Cursor,
+            serial::{SerialKind, SerialTracker},
+            window::WaylandWindow,
+        },
+        xdg_desktop_portal::{Event as XDPEvent, XDPEventSource},
+    },
+};
 
 /// Used to convert evdev scancode to xkb scancode
 const MIN_KEYCODE: u32 = 8;
 
+const UNKNOWN_KEYBOARD_LAYOUT_NAME: SharedString = SharedString::new_static("unknown");
+
 #[derive(Clone)]
 pub struct Globals {
     pub qh: QueueHandle<WaylandClientStatePtr>,
@@ -205,6 +210,7 @@ pub(crate) struct WaylandClientState {
     // Output to scale mapping
     outputs: HashMap<ObjectId, Output>,
     in_progress_outputs: HashMap<ObjectId, InProgressOutput>,
+    keyboard_layout: LinuxKeyboardLayout,
     keymap_state: Option<xkb::State>,
     compose_state: Option<xkb::compose::State>,
     drag: DragState,
@@ -335,6 +341,35 @@ impl WaylandClientStatePtr {
         text_input.commit();
     }
 
+    pub fn handle_keyboard_layout_change(&self) {
+        let client = self.get_client();
+        let mut state = client.borrow_mut();
+        let changed = if let Some(keymap_state) = &state.keymap_state {
+            let layout_idx = keymap_state.serialize_layout(xkbcommon::xkb::STATE_LAYOUT_EFFECTIVE);
+            let keymap = keymap_state.get_keymap();
+            let layout_name = keymap.layout_get_name(layout_idx);
+            let changed = layout_name != state.keyboard_layout.name();
+            if changed {
+                state.keyboard_layout = LinuxKeyboardLayout::new(layout_name.to_string().into());
+            }
+            changed
+        } else {
+            let changed = &UNKNOWN_KEYBOARD_LAYOUT_NAME != state.keyboard_layout.name();
+            if changed {
+                state.keyboard_layout = LinuxKeyboardLayout::new(UNKNOWN_KEYBOARD_LAYOUT_NAME);
+            }
+            changed
+        };
+        if changed {
+            if let Some(mut callback) = state.common.callbacks.keyboard_layout_change.take() {
+                drop(state);
+                callback();
+                state = client.borrow_mut();
+                state.common.callbacks.keyboard_layout_change = Some(callback);
+            }
+        }
+    }
+
     pub fn drop_window(&self, surface_id: &ObjectId) {
         let mut client = self.get_client();
         let mut state = client.borrow_mut();
@@ -533,6 +568,7 @@ impl WaylandClient {
             in_progress_outputs,
             windows: HashMap::default(),
             common,
+            keyboard_layout: LinuxKeyboardLayout::new(UNKNOWN_KEYBOARD_LAYOUT_NAME),
             keymap_state: None,
             compose_state: None,
             drag: DragState {
@@ -590,17 +626,7 @@ impl WaylandClient {
 
 impl LinuxClient for WaylandClient {
     fn keyboard_layout(&self) -> Box<dyn PlatformKeyboardLayout> {
-        let state = self.0.borrow();
-        let id = if let Some(keymap_state) = &state.keymap_state {
-            let layout_idx = keymap_state.serialize_layout(xkbcommon::xkb::STATE_LAYOUT_EFFECTIVE);
-            keymap_state
-                .get_keymap()
-                .layout_get_name(layout_idx)
-                .to_string()
-        } else {
-            "unknown".to_string()
-        };
-        Box::new(LinuxKeyboardLayout::new(id))
+        Box::new(self.0.borrow().keyboard_layout.clone())
     }
 
     fn displays(&self) -> Vec<Rc<dyn PlatformDisplay>> {
@@ -1181,13 +1207,9 @@ impl Dispatch<wl_keyboard::WlKeyboard, ()> for WaylandClientStatePtr {
                 };
                 state.keymap_state = Some(xkb::State::new(&keymap));
                 state.compose_state = get_xkb_compose_state(&xkb_context);
+                drop(state);
 
-                if let Some(mut callback) = state.common.callbacks.keyboard_layout_change.take() {
-                    drop(state);
-                    callback();
-                    state = client.borrow_mut();
-                    state.common.callbacks.keyboard_layout_change = Some(callback);
-                }
+                this.handle_keyboard_layout_change();
             }
             wl_keyboard::Event::Enter { surface, .. } => {
                 state.keyboard_focused_window = get_window(&mut state, &surface.id());
@@ -1230,26 +1252,18 @@ impl Dispatch<wl_keyboard::WlKeyboard, ()> for WaylandClientStatePtr {
                 keymap_state.update_mask(mods_depressed, mods_latched, mods_locked, 0, 0, group);
                 state.modifiers = Modifiers::from_xkb(keymap_state);
 
-                if group != old_layout {
-                    if let Some(mut callback) = state.common.callbacks.keyboard_layout_change.take()
-                    {
-                        drop(state);
-                        callback();
-                        state = client.borrow_mut();
-                        state.common.callbacks.keyboard_layout_change = Some(callback);
-                    }
-                }
-
-                let Some(focused_window) = focused_window else {
-                    return;
-                };
+                if let Some(focused_window) = focused_window {
+                    let input = PlatformInput::ModifiersChanged(ModifiersChangedEvent {
+                        modifiers: state.modifiers,
+                    });
 
-                let input = PlatformInput::ModifiersChanged(ModifiersChangedEvent {
-                    modifiers: state.modifiers,
-                });
+                    drop(state);
+                    focused_window.handle_input(input);
+                }
 
-                drop(state);
-                focused_window.handle_input(input);
+                if group != old_layout {
+                    this.handle_keyboard_layout_change();
+                }
             }
             wl_keyboard::Event::Key {
                 serial,
@@ -1374,6 +1388,7 @@ impl Dispatch<wl_keyboard::WlKeyboard, ()> for WaylandClientStatePtr {
         }
     }
 }
+
 impl Dispatch<zwp_text_input_v3::ZwpTextInputV3, ()> for WaylandClientStatePtr {
     fn event(
         this: &mut Self,

crates/gpui/src/platform/linux/x11/client.rs 🔗

@@ -198,6 +198,7 @@ pub struct X11ClientState {
     pub(crate) keyboard_focused_window: Option<xproto::Window>,
     pub(crate) xkb: xkbc::State,
     previous_xkb_state: XKBStateNotiy,
+    keyboard_layout: LinuxKeyboardLayout,
     pub(crate) ximc: Option<X11rbClient<Rc<XCBConnection>>>,
     pub(crate) xim_handler: Option<XimHandler>,
     pub modifiers: Modifiers,
@@ -401,17 +402,22 @@ impl X11Client {
             xkbc::x11::state_new_from_device(&xkb_keymap, &xcb_connection, xkb_device_id)
         };
         let compose_state = get_xkb_compose_state(&xkb_context);
-        let resource_database = x11rb::resource_manager::new_from_default(&xcb_connection).unwrap();
+        let layout_idx = xkb_state.serialize_layout(STATE_LAYOUT_EFFECTIVE);
+        let layout_name = xkb_state
+            .get_keymap()
+            .layout_get_name(layout_idx)
+            .to_string();
+        let keyboard_layout = LinuxKeyboardLayout::new(layout_name.into());
 
         let gpu_context = BladeContext::new().expect("Unable to init GPU context");
 
+        let resource_database = x11rb::resource_manager::new_from_default(&xcb_connection).unwrap();
         let scale_factor = resource_database
             .get_value("Xft.dpi", "Xft.dpi")
             .ok()
             .flatten()
             .map(|dpi: f32| dpi / 96.0)
             .unwrap_or(1.0);
-
         let cursor_handle = cursor::Handle::new(&xcb_connection, x_root_index, &resource_database)
             .unwrap()
             .reply()
@@ -489,6 +495,7 @@ impl X11Client {
             keyboard_focused_window: None,
             xkb: xkb_state,
             previous_xkb_state: XKBStateNotiy::default(),
+            keyboard_layout,
             ximc,
             xim_handler,
 
@@ -931,12 +938,8 @@ impl X11Client {
                     locked_layout,
                 };
                 state.xkb = xkb_state;
-                if let Some(mut callback) = state.common.callbacks.keyboard_layout_change.take() {
-                    drop(state);
-                    callback();
-                    state = self.0.borrow_mut();
-                    state.common.callbacks.keyboard_layout_change = Some(callback);
-                }
+                drop(state);
+                self.handle_keyboard_layout_change();
             }
             Event::XkbStateNotify(event) => {
                 let mut state = self.0.borrow_mut();
@@ -956,16 +959,6 @@ impl X11Client {
                     locked_layout: event.locked_group.into(),
                 };
 
-                if new_layout != old_layout {
-                    if let Some(mut callback) = state.common.callbacks.keyboard_layout_change.take()
-                    {
-                        drop(state);
-                        callback();
-                        state = self.0.borrow_mut();
-                        state.common.callbacks.keyboard_layout_change = Some(callback);
-                    }
-                }
-
                 let modifiers = Modifiers::from_xkb(&state.xkb);
                 if state.last_modifiers_changed_event == modifiers {
                     drop(state);
@@ -980,6 +973,10 @@ impl X11Client {
                         ModifiersChangedEvent { modifiers },
                     ));
                 }
+
+                if new_layout != old_layout {
+                    self.handle_keyboard_layout_change();
+                }
             }
             Event::KeyPress(event) => {
                 let window = self.get_window(event.event)?;
@@ -1368,6 +1365,22 @@ impl X11Client {
         drop(state);
         Some(())
     }
+
+    fn handle_keyboard_layout_change(&self) {
+        let mut state = self.0.borrow_mut();
+        let layout_idx = state.xkb.serialize_layout(STATE_LAYOUT_EFFECTIVE);
+        let keymap = state.xkb.get_keymap();
+        let layout_name = keymap.layout_get_name(layout_idx);
+        if layout_name != state.keyboard_layout.name() {
+            state.keyboard_layout = LinuxKeyboardLayout::new(layout_name.to_string().into());
+            if let Some(mut callback) = state.common.callbacks.keyboard_layout_change.take() {
+                drop(state);
+                callback();
+                state = self.0.borrow_mut();
+                state.common.callbacks.keyboard_layout_change = Some(callback);
+            }
+        }
+    }
 }
 
 impl LinuxClient for X11Client {
@@ -1381,14 +1394,7 @@ impl LinuxClient for X11Client {
 
     fn keyboard_layout(&self) -> Box<dyn PlatformKeyboardLayout> {
         let state = self.0.borrow();
-        let layout_idx = state.xkb.serialize_layout(STATE_LAYOUT_EFFECTIVE);
-        Box::new(LinuxKeyboardLayout::new(
-            state
-                .xkb
-                .get_keymap()
-                .layout_get_name(layout_idx)
-                .to_string(),
-        ))
+        Box::new(state.keyboard_layout.clone())
     }
 
     fn displays(&self) -> Vec<Rc<dyn PlatformDisplay>> {