From bbb9b4a3578ab298932c525d7475bd6bcdb59e43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Fri, 1 Dec 2023 20:01:53 +0100 Subject: [PATCH 01/13] feat: Implemented modifiers detection in Wayland Co-authored-by: Wez Furlong --- wezterm-input-types/src/lib.rs | 33 ++- window/src/os/wayland/connection.rs | 114 ++++++++- window/src/os/x11/connection.rs | 4 +- window/src/os/x11/keyboard.rs | 88 +++++-- window/src/os/x11/mod.rs | 1 + window/src/os/x11/modifiers.rs | 354 ++++++++++++++++++++++++++++ window/src/os/xkeysyms.rs | 4 + 7 files changed, 564 insertions(+), 34 deletions(-) create mode 100644 window/src/os/x11/modifiers.rs diff --git a/wezterm-input-types/src/lib.rs b/wezterm-input-types/src/lib.rs index b3298bad6aa..1cd3dbbcf5f 100644 --- a/wezterm-input-types/src/lib.rs +++ b/wezterm-input-types/src/lib.rs @@ -480,21 +480,27 @@ bitflags! { #[cfg_attr(feature="serde", derive(Serialize, Deserialize))] #[derive(Default, FromDynamic, ToDynamic)] #[dynamic(into="String", try_from="String")] - pub struct Modifiers: u16 { + pub struct Modifiers: u32 { const NONE = 0; + const SHIFT = 1<<1; + const LEFT_SHIFT = 1<<10; + const RIGHT_SHIFT = 1<<11; const ALT = 1<<2; - const CTRL = 1<<3; - const SUPER = 1<<4; const LEFT_ALT = 1<<5; const RIGHT_ALT = 1<<6; - /// This is a virtual modifier used by wezterm - const LEADER = 1<<7; + const CTRL = 1<<3; const LEFT_CTRL = 1<<8; const RIGHT_CTRL = 1<<9; - const LEFT_SHIFT = 1<<10; - const RIGHT_SHIFT = 1<<11; + const SUPER = 1<<4; + /// This is a virtual modifier used by wezterm + const LEADER = 1<<7; const ENHANCED_KEY = 1<<12; + + const META = 1<<13; + const HYPER = 1<<14; + const CAPS_LOCK = 1<<15; + const NUM_LOCK = 1<<16; } } @@ -1674,13 +1680,16 @@ impl KeyEvent { if raw_modifiers.contains(Modifiers::SUPER) { modifiers |= 8; } - // TODO: Hyper and Meta are not handled yet. - // We should somehow detect this? - // See: https://github.com/wez/wezterm/pull/4605#issuecomment-1823604708 - if self.leds.contains(KeyboardLedStatus::CAPS_LOCK) { + if raw_modifiers.contains(Modifiers::HYPER) { + modifiers |= 16; + } + if raw_modifiers.contains(Modifiers::META) { + modifiers |= 32; + } + if raw_modifiers.contains(Modifiers::CAPS_LOCK) { modifiers |= 64; } - if self.leds.contains(KeyboardLedStatus::NUM_LOCK) { + if raw_modifiers.contains(Modifiers::NUM_LOCK) { modifiers |= 128; } modifiers += 1; diff --git a/window/src/os/wayland/connection.rs b/window/src/os/wayland/connection.rs index d657374b10f..908ee22d550 100644 --- a/window/src/os/wayland/connection.rs +++ b/window/src/os/wayland/connection.rs @@ -43,7 +43,119 @@ impl WaylandConnection { wayland_state: RefCell::new(wayland_state), }; - Ok(wayland_connection) + fn keyboard_event( + &self, + keyboard: Main, + event: WlKeyboardEvent, + ) -> anyhow::Result<()> { + match &event { + WlKeyboardEvent::Enter { + serial, surface, .. + } => { + // update global active surface id + *self.active_surface_id.borrow_mut() = surface.as_ref().id(); + + *self.last_serial.borrow_mut() = *serial; + if let Some(&window_id) = self + .surface_to_window_id + .borrow() + .get(&surface.as_ref().id()) + { + self.keyboard_window_id.borrow_mut().replace(window_id); + self.environment.with_inner(|env| { + if let Some(input) = + env.input_handler.get_text_input_for_keyboard(&keyboard) + { + input.enable(); + input.commit(); + } + env.input_handler.advise_surface(&surface, &keyboard); + }); + } else { + log::warn!("{:?}, no known surface", event); + } + } + WlKeyboardEvent::Leave { serial, .. } => { + if let Some(input) = self + .environment + .with_inner(|env| env.input_handler.get_text_input_for_keyboard(&keyboard)) + { + input.disable(); + input.commit(); + } + *self.last_serial.borrow_mut() = *serial; + } + WlKeyboardEvent::Key { serial, .. } | WlKeyboardEvent::Modifiers { serial, .. } => { + *self.last_serial.borrow_mut() = *serial; + } + WlKeyboardEvent::RepeatInfo { rate, delay } => { + *self.key_repeat_rate.borrow_mut() = *rate; + *self.key_repeat_delay.borrow_mut() = *delay; + } + WlKeyboardEvent::Keymap { format, fd, size } => { + let mut file = unsafe { std::fs::File::from_raw_fd(*fd) }; + match format { + KeymapFormat::XkbV1 => { + let mut data = vec![0u8; *size as usize]; + // If we weren't passed a pipe, be sure to explicitly + // read from the start of the file + match file.read_exact_at(&mut data, 0) { + Ok(_) => {} + Err(err) => { + // ideally: we check for: + // err.kind() == std::io::ErrorKind::NotSeekable + // but that is not yet in stable rust + if err.raw_os_error() == Some(libc::ESPIPE) { + // It's a pipe, which cannot be seeked, so we + // just try reading from the current pipe position + file.read(&mut data).context("read from Keymap fd/pipe")?; + } else { + return Err(err).context("read_exact_at from Keymap fd"); + } + } + } + // Dance around CString panicing on the NUL terminator + // in the xkbcommon crate + while let Some(0) = data.last() { + data.pop(); + } + let s = String::from_utf8(data)?; + match KeyboardWithFallback::new_from_string(s, true) { + Ok(k) => { + self.keyboard_mapper.replace(Some(k)); + } + Err(err) => { + log::error!("Error processing keymap change: {:#}", err); + } + } + } + _ => {} + } + } + _ => {} + } + if let Some(&window_id) = self.keyboard_window_id.borrow().as_ref() { + if let Some(win) = self.window_by_id(window_id) { + let mut inner = win.borrow_mut(); + inner.keyboard_event(event); + } + } + + Ok(()) + } + + pub(crate) fn dispatch_to_focused_window(&self, event: WindowEvent) { + if let Some(&window_id) = self.keyboard_window_id.borrow().as_ref() { + if let Some(win) = self.window_by_id(window_id) { + let mut inner = win.borrow_mut(); + inner.events.dispatch(event); + } + } + } + + pub(crate) fn next_window_id(&self) -> usize { + self.next_window_id + .fetch_add(1, ::std::sync::atomic::Ordering::Relaxed) } pub(crate) fn advise_of_appearance_change(&self, appearance: crate::Appearance) { diff --git a/window/src/os/x11/connection.rs b/window/src/os/x11/connection.rs index 3f078b82f08..0a9f57a8a6d 100644 --- a/window/src/os/x11/connection.rs +++ b/window/src/os/x11/connection.rs @@ -712,8 +712,8 @@ impl XConnection { visual.green_mask(), visual.blue_mask() ); - let (keyboard, kbd_ev) = Keyboard::new(&conn)?; - let keyboard = KeyboardWithFallback::new(keyboard)?; + let (keyboard, kbd_ev) = Keyboard::new(&conn, false)?; + let keyboard = KeyboardWithFallback::new(keyboard, false)?; let cursor_font_id = conn.generate_id(); let cursor_font_name = "cursor"; diff --git a/window/src/os/x11/keyboard.rs b/window/src/os/x11/keyboard.rs index 8fd3edeee6e..85c1d3acf5b 100644 --- a/window/src/os/x11/keyboard.rs +++ b/window/src/os/x11/keyboard.rs @@ -1,4 +1,7 @@ use crate::os::xkeysyms::keysym_to_keycode; +use crate::x11::modifiers::{ + init_modifier_table_wayland, init_modifier_table_x11, ModifierIndex, ModifierMap, +}; use crate::{ DeadKeyStatus, Handled, KeyCode, KeyEvent, Modifiers, RawKeyEvent, WindowEvent, WindowEventSender, WindowKeyEvent, @@ -12,8 +15,7 @@ use std::os::unix::ffi::OsStrExt; use wezterm_input_types::{KeyboardLedStatus, PhysKeyCode}; use xcb::x::KeyButMask; use xkb::compose::Status as ComposeStatus; -use xkbcommon::xkb; -use xkbcommon::xkb::{LayoutIndex, ModMask}; +use xkbcommon::xkb::{self}; pub struct Keyboard { context: xkb::Context, @@ -22,6 +24,7 @@ pub struct Keyboard { state: RefCell, compose_state: RefCell, + mod_map: ModifierMap, phys_code_map: RefCell>, mods_leds: RefCell<(Modifiers, KeyboardLedStatus)>, last_xcb_state: RefCell, @@ -168,16 +171,16 @@ fn default_keymap(context: &xkb::Context) -> Option { } impl KeyboardWithFallback { - pub fn new(selected: Keyboard) -> anyhow::Result { + pub fn new(selected: Keyboard, using_wayland: bool) -> anyhow::Result { Ok(Self { selected, - fallback: Keyboard::new_default()?, + fallback: Keyboard::new_default(using_wayland)?, }) } - pub fn new_from_string(s: String) -> anyhow::Result { - let selected = Keyboard::new_from_string(s)?; - Self::new(selected) + pub fn new_from_string(s: String, using_wayland: bool) -> anyhow::Result { + let selected = Keyboard::new_from_string(s, using_wayland)?; + Self::new(selected, using_wayland) } pub fn process_wayland_key( @@ -457,12 +460,16 @@ impl KeyboardWithFallback { } } - fn mod_is_active(&self, modifier: &str) -> bool { + fn mod_is_active(&self, modifier: ModifierIndex) -> bool { // [TODO] consider state Depressed & consumed mods + if modifier.idx == xkb::MOD_INVALID { + return false; + } + self.selected .state .borrow() - .mod_name_is_active(modifier, xkb::STATE_MODS_EFFECTIVE) + .mod_index_is_active(modifier.idx, xkb::STATE_MODS_EFFECTIVE) } fn led_is_active(&self, led: &str) -> bool { self.selected.state.borrow().led_name_is_active(led) @@ -484,20 +491,39 @@ impl KeyboardWithFallback { pub fn get_key_modifiers(&self) -> Modifiers { let mut res = Modifiers::default(); - if self.mod_is_active(xkb::MOD_NAME_SHIFT) { + if self.mod_is_active(self.selected.mod_map.shift) { res |= Modifiers::SHIFT; } - if self.mod_is_active(xkb::MOD_NAME_CTRL) { + + if self.mod_is_active(self.selected.mod_map.ctrl) { res |= Modifiers::CTRL; } - if self.mod_is_active(xkb::MOD_NAME_ALT) { - // Mod1 + + if self.mod_is_active(self.selected.mod_map.alt) { res |= Modifiers::ALT; } - if self.mod_is_active(xkb::MOD_NAME_LOGO) { - // Mod4 + + if self.mod_is_active(self.selected.mod_map.meta) { + res |= Modifiers::META; + } + + if self.mod_is_active(self.selected.mod_map.supr) { res |= Modifiers::SUPER; } + + if self.mod_is_active(self.selected.mod_map.hyper) { + res |= Modifiers::HYPER; + } + + if self.mod_is_active(self.selected.mod_map.caps_lock) { + res |= Modifiers::CAPS_LOCK; + } + + if self.mod_is_active(self.selected.mod_map.num_lock) { + res |= Modifiers::NUM_LOCK; + } + + log::debug!("Modifiers detected: {:?}", res); res } @@ -563,7 +589,7 @@ impl KeyboardWithFallback { } impl Keyboard { - pub fn new_default() -> anyhow::Result { + pub fn new_default(using_wayland: bool) -> anyhow::Result { let context = xkb::Context::new(xkb::CONTEXT_NO_FLAGS); let keymap = default_keymap(&context) .ok_or_else(|| anyhow!("Failed to load system default keymap"))?; @@ -579,6 +605,12 @@ impl Keyboard { let phys_code_map = build_physkeycode_map(&keymap); let label = "fallback"; + let mod_map = if using_wayland { + init_modifier_table_wayland(&keymap) + } else { + init_modifier_table_x11(&keymap) + }; + Ok(Self { context, device_id: -1, @@ -589,6 +621,7 @@ impl Keyboard { composition: String::new(), label, }), + mod_map, phys_code_map: RefCell::new(phys_code_map), mods_leds: RefCell::new(Default::default()), last_xcb_state: RefCell::new(Default::default()), @@ -596,7 +629,7 @@ impl Keyboard { }) } - pub fn new_from_string(s: String) -> anyhow::Result { + pub fn new_from_string(s: String, using_wayland: bool) -> anyhow::Result { let context = xkb::Context::new(xkb::CONTEXT_NO_FLAGS); let keymap = xkb::Keymap::new_from_string( &context, @@ -617,6 +650,12 @@ impl Keyboard { let phys_code_map = build_physkeycode_map(&keymap); let label = "selected"; + let mod_map = if using_wayland { + init_modifier_table_wayland(&keymap) + } else { + init_modifier_table_x11(&keymap) + }; + Ok(Self { context, device_id: -1, @@ -627,6 +666,7 @@ impl Keyboard { composition: String::new(), label, }), + mod_map, phys_code_map: RefCell::new(phys_code_map), mods_leds: RefCell::new(Default::default()), last_xcb_state: RefCell::new(Default::default()), @@ -634,7 +674,10 @@ impl Keyboard { }) } - pub fn new(connection: &xcb::Connection) -> anyhow::Result<(Keyboard, u8)> { + pub fn new( + connection: &xcb::Connection, + using_wayland: bool, + ) -> anyhow::Result<(Keyboard, u8)> { let first_ev = xcb::xkb::get_extension_data(connection) .ok_or_else(|| anyhow!("could not get xkb extension data"))? .first_event; @@ -687,7 +730,13 @@ impl Keyboard { let phys_code_map = build_physkeycode_map(&keymap); let label = "selected"; - let kbd = Keyboard { + let mod_map = if using_wayland { + init_modifier_table_wayland(&keymap) + } else { + init_modifier_table_x11(&keymap) + }; + + let kbd = Self { context, device_id, keymap: RefCell::new(keymap), @@ -697,6 +746,7 @@ impl Keyboard { composition: String::new(), label, }), + mod_map, phys_code_map: RefCell::new(phys_code_map), mods_leds: RefCell::new(Default::default()), last_xcb_state: RefCell::new(Default::default()), diff --git a/window/src/os/x11/mod.rs b/window/src/os/x11/mod.rs index 862b9cadf77..937f706a1a9 100644 --- a/window/src/os/x11/mod.rs +++ b/window/src/os/x11/mod.rs @@ -2,6 +2,7 @@ pub mod connection; pub mod cursor; pub mod keyboard; +mod modifiers; pub mod window; pub mod xcb_util; pub mod xrm; diff --git a/window/src/os/x11/modifiers.rs b/window/src/os/x11/modifiers.rs new file mode 100644 index 00000000000..3bcfa1a498c --- /dev/null +++ b/window/src/os/x11/modifiers.rs @@ -0,0 +1,354 @@ +use xkbcommon::xkb::{self}; + +#[derive(Debug, Clone, Copy)] +pub struct ModifierIndex { + pub idx: xkb::ModIndex, + pub mask: u32, +} + +impl Default for ModifierIndex { + fn default() -> Self { + return Self { + idx: xkb::MOD_INVALID, + mask: 0, + }; + } +} + +#[derive(Default, Debug, Clone, Copy)] +pub struct ModifierMap { + pub ctrl: ModifierIndex, + pub meta: ModifierIndex, + pub alt: ModifierIndex, + pub shift: ModifierIndex, + pub caps_lock: ModifierIndex, + pub num_lock: ModifierIndex, + pub supr: ModifierIndex, + pub hyper: ModifierIndex, +} + +#[derive(Debug, Clone, Copy)] +struct Algorithm { + failed: bool, + + try_shift: bool, + shift_keycode: u32, + + shift: u32, + ctrl: u32, + alt: u32, + meta: u32, + caps_lock: u32, + num_lock: u32, + supr: u32, + hyper: u32, +} + +impl Default for Algorithm { + fn default() -> Algorithm { + return Algorithm { + failed: false, + try_shift: false, + shift_keycode: 0, + + shift: 0, + ctrl: 0, + alt: 0, + meta: 0, + caps_lock: 0, + num_lock: 0, + supr: 0, + hyper: 0, + }; + } +} + +// Algorithm for mapping virtual modifiers to real modifiers: +// 1. create new state +// 2. for each key in keymap +// a. send key down to state +// b. if it affected exactly one bit in modifier map +// i) get keysym +// ii) if keysym matches one of the known modifiers, save it for that modifier +// iii) if modifier is latched, send key up and key down to toggle again +// c. send key up to reset the state +// 3. if shift key found in step 2, run step 2 with all shift+key for each key +// 4. if shift, control, alt and super are not all found, declare failure +// 5. if failure, use static mapping from xkbcommon-names.h +// +// Step 3 is needed because many popular keymaps map meta to alt+shift. +// +// We could do better by constructing a system of linear equations, but it should not be +// needed in any sane system. We could also use this algorithm with X11, but X11 +// provides XkbVirtualModsToReal which is guaranteed to be accurate, while this +// algorithm is only a heuristic. +// +// We don't touch level3 or level5 modifiers. +// +fn get_mapper_algo<'a>( + algo: &'a mut Algorithm, + state: &'a mut xkb::State, +) -> impl FnMut(&xkb::Keymap, xkb::Keycode) + 'a { + return move |_: &xkb::Keymap, key: xkb::Keycode| { + log::debug!("Key: {:?}", key); + + if algo.failed { + return; + } + + if algo.try_shift { + if key == algo.shift_keycode { + return; + } + + state.update_key(algo.shift_keycode, xkb::KeyDirection::Down); + } + + let changed_type = state.update_key(key, xkb::KeyDirection::Down); + + if changed_type + & (xkb::STATE_MODS_DEPRESSED | xkb::STATE_MODS_LATCHED | xkb::STATE_MODS_LOCKED) + != 0 + { + let mods = state.serialize_mods(if algo.try_shift { + xkb::STATE_MODS_EFFECTIVE + } else { + xkb::STATE_MODS_DEPRESSED | xkb::STATE_MODS_LATCHED | xkb::STATE_MODS_LOCKED + }); + + let keysyms = state.key_get_syms(key); + log::debug!("Keysym: {:x?}", keysyms); + + macro_rules! assign_left_right { + ($key_l:expr, $key_r:expr, $mod:ident) => { + if keysyms[0] == $key_l || keysyms[0] == $key_r { + if algo.$mod == 0 { + log::debug!( + "Keycode {:?} triggered keysym '{:x?}' for {:?}' modifier.", + key, + keysyms[0], + stringify!($mod) + ); + algo.$mod = mods; + } else if algo.$mod != mods { + log::debug!( + "Keycode {:?} triggered again keysym '{:x?}' for {:?}' modifier.", + key, + keysyms[0], + stringify!($mod) + ); + algo.failed = true; + } + } + }; + } + + macro_rules! assign { + ($key:expr, $mod:ident) => { + if keysyms[0] == $key && algo.$mod == 0 { + log::debug!( + "Key {:?} triggered keysym '{:?}' for '{:?}' modifier.", + key, + keysyms[0], + stringify!($mod) + ); + algo.$mod = mods; + } + }; + } + + // We can handle exactly one keysym with exactly one bit set in the implementation + // below; with a lot more gymnastics, we could set up an 8x8 linear system and solve + // for each modifier in case there are some modifiers that are only present in + // combination with others, but it is not worth the effort. + if keysyms.len() == 1 && mods.is_power_of_two() { + assign_left_right!(xkb::KEY_Shift_L, xkb::KEY_Shift_R, shift); + assign_left_right!(xkb::KEY_Control_L, xkb::KEY_Control_R, ctrl); + assign!(xkb::KEY_Caps_Lock, caps_lock); + assign!(xkb::KEY_Shift_Lock, num_lock); + assign_left_right!(xkb::KEY_Alt_L, xkb::KEY_Alt_R, alt); + assign_left_right!(xkb::KEY_Meta_L, xkb::KEY_Meta_R, meta); + assign_left_right!(xkb::KEY_Super_L, xkb::KEY_Super_R, supr); + assign_left_right!(xkb::KEY_Hyper_L, xkb::KEY_Hyper_R, hyper); + } + + if algo.shift_keycode == 0 + && (keysyms[0] == xkb::KEY_Shift_L || keysyms[0] == xkb::KEY_Shift_R) + { + log::debug!("Found shift keycode."); + algo.shift_keycode = key; + } + + // If this is a lock, then up and down to remove lock state + if changed_type & xkb::STATE_MODS_LOCKED != 0 { + log::debug!("Found lock state. Set up/down to remove lock state."); + state.update_key(key, xkb::KeyDirection::Up); + state.update_key(key, xkb::KeyDirection::Down); + } + } + + state.update_key(key, xkb::KeyDirection::Up); + + if algo.try_shift { + state.update_key(algo.shift_keycode, xkb::KeyDirection::Up); + } + }; +} + +/// This function initializes wezterm internal modifiers depending +/// on a default mapping. +/// This function simply queries the index for the xkb modifiers +/// `Control`, `Lock`, `Shift`, `Mod1`, `Mod2`, `Mod4` +/// and treats them as default (assumption) to +/// `Ctrl`, `Caps_Lock`, `Shift`, `Alt`, `Num_Lock`, `Super` +/// +/// Modifiers `Hyper` and `Meta` are not detected. +fn init_modifier_table_fallback(keymap: &xkb::Keymap) -> ModifierMap { + let mut mod_map = ModifierMap::default(); + + macro_rules! assign { + ($mod:ident, $n:expr) => { + let idx = keymap.mod_get_index($n); + mod_map.$mod = ModifierIndex { + idx: idx, + mask: 1 << idx, + }; + }; + } + + assign!(ctrl, xkb::MOD_NAME_CTRL); + assign!(shift, xkb::MOD_NAME_SHIFT); + assign!(alt, xkb::MOD_NAME_ALT); + assign!(caps_lock, xkb::MOD_NAME_CAPS); + assign!(num_lock, xkb::MOD_NAME_NUM); + assign!(supr, xkb::MOD_NAME_LOGO); + + return mod_map; +} + +/// This function initializes wezterm internal modifiers +/// by looking up virtual modifiers (e.g. run `xmodmap -pm`) +/// and +/// This function initializes `xkb` modifier indices for +/// all modifiers +/// `Ctrl`, `Shift`, `Alt`, `Num_Lock`, `Caps_Lock`, `Super`, +/// `Hyper`, `Meta`. +pub fn init_modifier_table_x11(keymap: &xkb::Keymap) -> ModifierMap { + // TODO: This implementation needs to be done with + // https://github.com/kovidgoyal/kitty/blob/0248edbdb98cc3ae80d98bf5ad17fbf497a24a43/glfw/xkb_glfw.c#L321 return init_modifier_table_fallback(keymap); + return init_modifier_table_fallback(keymap); +} + +/// This function initializes wezterm internal modifiers +/// by probing the keyboard state for each keypress. +/// +/// This is a workaround because under Wayland the code in +/// [init_modifier_table_x11](init_modifier_table_x11) +/// does not work. +/// +/// This function tries to initialize `xkb` modifier indices for +/// all modifiers +/// `Ctrl`, `Shift`, `Alt`, `Num_Lock`, `Caps_Lock`, `Super`, +/// `Hyper`, `Meta` and if it fails it uses the fallback method +/// [init_modifier_table_fallback](init_modifier_table_fallback). +/// +/// Implementation is taken from Kitty: +/// https://github.com/kovidgoyal/kitty/blob/0248edbdb98cc3ae80d98bf5ad17fbf497a24a43/glfw/xkb_glfw.c#L523 +pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { + // + // This is a client side hack for wayland, once + // https://github.com/xkbcommon/libxkbcommon/pull/36 + // is implemented or some other solution exists + // this code iterates through key presses to find the + // activated modifiers. + // Check: https://github.com/wez/wezterm/issues/4626 + + log::info!("Detect modifiers on Wayland [with key press iterations]."); + + let mut algo = Algorithm::default(); + let mut state: xkb::State = xkb::State::new(keymap); + let mut mod_map = ModifierMap::default(); + + keymap.key_for_each(get_mapper_algo(&mut algo, &mut state)); + + if algo.shift_keycode == 0 { + algo.failed = true; + log::debug!("Did not found shift keycode.") + } + + if (algo.ctrl == 0 + || algo.alt == 0 + || algo.meta == 0 + || algo.shift == 0 + || algo.supr == 0 + || algo.hyper == 0) + && !algo.failed + { + algo.try_shift = true; + log::debug!("Detect modifiers on Wayland [with Shift+key press iterations]."); + keymap.key_for_each(get_mapper_algo(&mut algo, &mut state)); + } + + // We must have found a least those 4 modifiers. + if !algo.failed && (algo.ctrl == 0 || algo.shift == 0 || algo.alt == 0 || algo.supr == 0) { + log::debug!("Some of essential modifiers (ctrl, shift, alt, supr) have not been found."); + algo.failed = true; + } + + if !algo.failed { + let mut shifted: u32 = 1; + let mut used_bits: u32 = 0; + + macro_rules! assign { + ($mod:ident, $i:ident) => { + if mod_map.$mod.idx == xkb::MOD_INVALID + && (used_bits & shifted == 0) + && algo.$mod == shifted + { + mod_map.$mod = ModifierIndex { + idx: $i, + mask: shifted, + }; + used_bits |= shifted; + } + }; + } + + for i in 0..32 { + assign!(ctrl, i); + assign!(shift, i); + assign!(alt, i); + assign!(meta, i); + assign!(caps_lock, i); + assign!(num_lock, i); + assign!(supr, i); + assign!(hyper, i); + shifted <<= 1; + } + } else { + log::warn!("Detect modifiers on Wayland failed. Using default mapping."); + mod_map = init_modifier_table_fallback(keymap); + } + + log::info!( + "Modifier map:\n + - ctrl: {:?} + - shift: {:?} + - alt: {:?} + - meta: {:?} + - caps_lock: {:?} + - num_lock: {:?} + - super: {:?} + - hyper: {:?}", + mod_map.ctrl, + mod_map.shift, + mod_map.alt, + mod_map.meta, + mod_map.caps_lock, + mod_map.num_lock, + mod_map.supr, + mod_map.hyper, + ); + + return mod_map; +} diff --git a/window/src/os/xkeysyms.rs b/window/src/os/xkeysyms.rs index 25e4ceb673d..1ebf9a4e80c 100644 --- a/window/src/os/xkeysyms.rs +++ b/window/src/os/xkeysyms.rs @@ -74,8 +74,12 @@ pub fn keysym_to_keycode(keysym: u32) -> Option { KEY_Caps_Lock => KeyCode::CapsLock, KEY_Num_Lock => KeyCode::NumLock, KEY_Scroll_Lock => KeyCode::ScrollLock, + KEY_Meta_L => KeyCode::Meta, + KEY_Meta_R => KeyCode::Meta, KEY_Super_L => KeyCode::Super, KEY_Super_R => KeyCode::Super, + KEY_Hyper_R => KeyCode::Hyper, + KEY_Hyper_L => KeyCode::Hyper, KEY_Menu => KeyCode::Applications, KEY_Help => KeyCode::Help, From 0add6dece353449dcbd972c9dae9c3cb1d99d61c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Sat, 2 Dec 2023 21:28:08 +0100 Subject: [PATCH 02/13] fix: Remove mask Co-authored-by: Wez Furlong --- window/src/os/x11/modifiers.rs | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/window/src/os/x11/modifiers.rs b/window/src/os/x11/modifiers.rs index 3bcfa1a498c..91e492edc36 100644 --- a/window/src/os/x11/modifiers.rs +++ b/window/src/os/x11/modifiers.rs @@ -3,14 +3,12 @@ use xkbcommon::xkb::{self}; #[derive(Debug, Clone, Copy)] pub struct ModifierIndex { pub idx: xkb::ModIndex, - pub mask: u32, } impl Default for ModifierIndex { fn default() -> Self { return Self { idx: xkb::MOD_INVALID, - mask: 0, }; } } @@ -305,10 +303,7 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { && (used_bits & shifted == 0) && algo.$mod == shifted { - mod_map.$mod = ModifierIndex { - idx: $i, - mask: shifted, - }; + mod_map.$mod = ModifierIndex { idx: $i }; used_bits |= shifted; } }; @@ -331,23 +326,7 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { } log::info!( - "Modifier map:\n - - ctrl: {:?} - - shift: {:?} - - alt: {:?} - - meta: {:?} - - caps_lock: {:?} - - num_lock: {:?} - - super: {:?} - - hyper: {:?}", - mod_map.ctrl, - mod_map.shift, - mod_map.alt, - mod_map.meta, - mod_map.caps_lock, - mod_map.num_lock, - mod_map.supr, - mod_map.hyper, + "Modifier map {mod_map:#?}" ); return mod_map; From 653a262fdb9cb0b6a8bf4c04c4e469ff988bf8a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Sat, 2 Dec 2023 21:59:07 +0100 Subject: [PATCH 03/13] fix: Apply visitor pattern for keymap probing --- window/src/os/x11/modifiers.rs | 130 +++++++++++++++++---------------- 1 file changed, 67 insertions(+), 63 deletions(-) diff --git a/window/src/os/x11/modifiers.rs b/window/src/os/x11/modifiers.rs index 91e492edc36..ec1d05e504a 100644 --- a/window/src/os/x11/modifiers.rs +++ b/window/src/os/x11/modifiers.rs @@ -25,8 +25,7 @@ pub struct ModifierMap { pub hyper: ModifierIndex, } -#[derive(Debug, Clone, Copy)] -struct Algorithm { +struct Algorithm<'a> { failed: bool, try_shift: bool, @@ -40,10 +39,12 @@ struct Algorithm { num_lock: u32, supr: u32, hyper: u32, + + state: &'a mut xkb::State, } -impl Default for Algorithm { - fn default() -> Algorithm { +impl<'a> Algorithm<'a> { + fn new(state: &'a mut xkb::State) -> Algorithm<'a> { return Algorithm { failed: false, try_shift: false, @@ -57,85 +58,85 @@ impl Default for Algorithm { num_lock: 0, supr: 0, hyper: 0, + + state, }; } } -// Algorithm for mapping virtual modifiers to real modifiers: -// 1. create new state -// 2. for each key in keymap -// a. send key down to state -// b. if it affected exactly one bit in modifier map -// i) get keysym -// ii) if keysym matches one of the known modifiers, save it for that modifier -// iii) if modifier is latched, send key up and key down to toggle again -// c. send key up to reset the state -// 3. if shift key found in step 2, run step 2 with all shift+key for each key -// 4. if shift, control, alt and super are not all found, declare failure -// 5. if failure, use static mapping from xkbcommon-names.h -// -// Step 3 is needed because many popular keymaps map meta to alt+shift. -// -// We could do better by constructing a system of linear equations, but it should not be -// needed in any sane system. We could also use this algorithm with X11, but X11 -// provides XkbVirtualModsToReal which is guaranteed to be accurate, while this -// algorithm is only a heuristic. -// -// We don't touch level3 or level5 modifiers. -// -fn get_mapper_algo<'a>( - algo: &'a mut Algorithm, - state: &'a mut xkb::State, -) -> impl FnMut(&xkb::Keymap, xkb::Keycode) + 'a { - return move |_: &xkb::Keymap, key: xkb::Keycode| { +impl<'a> Algorithm<'a> { + /// Algorithm for mapping virtual modifiers to real modifiers: + /// 1. create new state + /// 2. for each key in keymap + /// a. send key down to state + /// b. if it affected exactly one bit in modifier map + /// i) get keysym + /// ii) if keysym matches one of the known modifiers, save it for that modifier + /// iii) if modifier is latched, send key up and key down to toggle again + /// c. send key up to reset the state + /// 3. if shift key found in step 2, run step 2 with all shift+key for each key + /// 4. if shift, control, alt and super are not all found, declare failure + /// 5. if failure, use static mapping from xkbcommon-names.h + /// + /// Step 3 is needed because many popular keymaps map meta to alt+shift. + /// + /// We could do better by constructing a system of linear equations, but it should not be + /// needed in any sane system. We could also use this algorithm with X11, but X11 + /// provides XkbVirtualModsToReal which is guaranteed to be accurate, while this + /// algorithm is only a heuristic. + /// + /// We don't touch level3 or level5 modifiers. + /// + fn visit_key(&mut self, key: xkb::Keycode) { log::debug!("Key: {:?}", key); - if algo.failed { + if self.failed { return; } - if algo.try_shift { - if key == algo.shift_keycode { + if self.try_shift { + if key == self.shift_keycode { return; } - state.update_key(algo.shift_keycode, xkb::KeyDirection::Down); + self.state + .update_key(self.shift_keycode, xkb::KeyDirection::Down); } - let changed_type = state.update_key(key, xkb::KeyDirection::Down); + let changed_type = self.state.update_key(key, xkb::KeyDirection::Down); if changed_type & (xkb::STATE_MODS_DEPRESSED | xkb::STATE_MODS_LATCHED | xkb::STATE_MODS_LOCKED) != 0 { - let mods = state.serialize_mods(if algo.try_shift { + let mods = self.state.serialize_mods(if self.try_shift { xkb::STATE_MODS_EFFECTIVE } else { xkb::STATE_MODS_DEPRESSED | xkb::STATE_MODS_LATCHED | xkb::STATE_MODS_LOCKED }); - let keysyms = state.key_get_syms(key); + let keysyms = self.state.key_get_syms(key); log::debug!("Keysym: {:x?}", keysyms); macro_rules! assign_left_right { ($key_l:expr, $key_r:expr, $mod:ident) => { if keysyms[0] == $key_l || keysyms[0] == $key_r { - if algo.$mod == 0 { + if self.$mod == 0 { log::debug!( "Keycode {:?} triggered keysym '{:x?}' for {:?}' modifier.", key, keysyms[0], stringify!($mod) ); - algo.$mod = mods; - } else if algo.$mod != mods { + self.$mod = mods; + } else if self.$mod != mods { log::debug!( "Keycode {:?} triggered again keysym '{:x?}' for {:?}' modifier.", key, keysyms[0], stringify!($mod) ); - algo.failed = true; + self.failed = true; } } }; @@ -143,14 +144,14 @@ fn get_mapper_algo<'a>( macro_rules! assign { ($key:expr, $mod:ident) => { - if keysyms[0] == $key && algo.$mod == 0 { + if keysyms[0] == $key && self.$mod == 0 { log::debug!( "Key {:?} triggered keysym '{:?}' for '{:?}' modifier.", key, keysyms[0], stringify!($mod) ); - algo.$mod = mods; + self.$mod = mods; } }; } @@ -170,27 +171,34 @@ fn get_mapper_algo<'a>( assign_left_right!(xkb::KEY_Hyper_L, xkb::KEY_Hyper_R, hyper); } - if algo.shift_keycode == 0 + if self.shift_keycode == 0 && (keysyms[0] == xkb::KEY_Shift_L || keysyms[0] == xkb::KEY_Shift_R) { log::debug!("Found shift keycode."); - algo.shift_keycode = key; + self.shift_keycode = key; } // If this is a lock, then up and down to remove lock state if changed_type & xkb::STATE_MODS_LOCKED != 0 { - log::debug!("Found lock state. Set up/down to remove lock state."); - state.update_key(key, xkb::KeyDirection::Up); - state.update_key(key, xkb::KeyDirection::Down); + log::debug!("Found lock self.state. Set up/down to remove lock self.state."); + self.state.update_key(key, xkb::KeyDirection::Up); + self.state.update_key(key, xkb::KeyDirection::Down); } } - state.update_key(key, xkb::KeyDirection::Up); + self.state.update_key(key, xkb::KeyDirection::Up); - if algo.try_shift { - state.update_key(algo.shift_keycode, xkb::KeyDirection::Up); + if self.try_shift { + self.state + .update_key(self.shift_keycode, xkb::KeyDirection::Up); } - }; + } + + fn visit_keymap(&mut self, keymap: &xkb::Keymap) { + keymap.key_for_each(|_, key| { + self.visit_key(key); + }); + } } /// This function initializes wezterm internal modifiers depending @@ -207,10 +215,7 @@ fn init_modifier_table_fallback(keymap: &xkb::Keymap) -> ModifierMap { macro_rules! assign { ($mod:ident, $n:expr) => { let idx = keymap.mod_get_index($n); - mod_map.$mod = ModifierIndex { - idx: idx, - mask: 1 << idx, - }; + mod_map.$mod = ModifierIndex { idx: idx }; }; } @@ -263,11 +268,11 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { log::info!("Detect modifiers on Wayland [with key press iterations]."); - let mut algo = Algorithm::default(); let mut state: xkb::State = xkb::State::new(keymap); + let mut algo = Algorithm::new(&mut state); let mut mod_map = ModifierMap::default(); - keymap.key_for_each(get_mapper_algo(&mut algo, &mut state)); + algo.visit_keymap(&keymap); if algo.shift_keycode == 0 { algo.failed = true; @@ -283,8 +288,9 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { && !algo.failed { algo.try_shift = true; + log::debug!("Detect modifiers on Wayland [with Shift+key press iterations]."); - keymap.key_for_each(get_mapper_algo(&mut algo, &mut state)); + algo.visit_keymap(&keymap); } // We must have found a least those 4 modifiers. @@ -325,9 +331,7 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { mod_map = init_modifier_table_fallback(keymap); } - log::info!( - "Modifier map {mod_map:#?}" - ); + log::info!("Modifier map {mod_map:#?}"); return mod_map; } From 83772f1f395d1bcc7829d68ed3a0c888ee032e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Sat, 2 Dec 2023 22:08:44 +0100 Subject: [PATCH 04/13] fix: Add comment and better macro name --- window/src/os/x11/modifiers.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/window/src/os/x11/modifiers.rs b/window/src/os/x11/modifiers.rs index ec1d05e504a..f02b824e054 100644 --- a/window/src/os/x11/modifiers.rs +++ b/window/src/os/x11/modifiers.rs @@ -303,7 +303,9 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { let mut shifted: u32 = 1; let mut used_bits: u32 = 0; - macro_rules! assign { + // Algorithm can detect the same index for multiple modifiers + // but we only want one assigned to each of our modifiers. + macro_rules! try_assign_unique_index { ($mod:ident, $i:ident) => { if mod_map.$mod.idx == xkb::MOD_INVALID && (used_bits & shifted == 0) @@ -316,14 +318,14 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { } for i in 0..32 { - assign!(ctrl, i); - assign!(shift, i); - assign!(alt, i); - assign!(meta, i); - assign!(caps_lock, i); - assign!(num_lock, i); - assign!(supr, i); - assign!(hyper, i); + try_assign_unique_index!(ctrl, i); + try_assign_unique_index!(shift, i); + try_assign_unique_index!(alt, i); + try_assign_unique_index!(meta, i); + try_assign_unique_index!(caps_lock, i); + try_assign_unique_index!(num_lock, i); + try_assign_unique_index!(supr, i); + try_assign_unique_index!(hyper, i); shifted <<= 1; } } else { From 6acc36d61072269afefa6c36eadfd1fdbb35bcbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Sat, 2 Dec 2023 22:22:28 +0100 Subject: [PATCH 05/13] fix: Wrong order mods --- wezterm-input-types/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/wezterm-input-types/src/lib.rs b/wezterm-input-types/src/lib.rs index 1cd3dbbcf5f..a1ae14c7f99 100644 --- a/wezterm-input-types/src/lib.rs +++ b/wezterm-input-types/src/lib.rs @@ -482,19 +482,18 @@ bitflags! { #[dynamic(into="String", try_from="String")] pub struct Modifiers: u32 { const NONE = 0; - const SHIFT = 1<<1; - const LEFT_SHIFT = 1<<10; - const RIGHT_SHIFT = 1<<11; const ALT = 1<<2; - const LEFT_ALT = 1<<5; - const RIGHT_ALT = 1<<6; const CTRL = 1<<3; - const LEFT_CTRL = 1<<8; - const RIGHT_CTRL = 1<<9; const SUPER = 1<<4; + const LEFT_ALT = 1<<5; + const RIGHT_ALT = 1<<6; /// This is a virtual modifier used by wezterm const LEADER = 1<<7; + const LEFT_CTRL = 1<<8; + const RIGHT_CTRL = 1<<9; + const LEFT_SHIFT = 1<<10; + const RIGHT_SHIFT = 1<<11; const ENHANCED_KEY = 1<<12; const META = 1<<13; From 3ed3800c137a3e62c4036bb814025b7b915c43da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Sat, 2 Dec 2023 22:41:43 +0100 Subject: [PATCH 06/13] fix: Make better enum instead of bool --- window/src/os/wayland/connection.rs | 17 +++++++++++- window/src/os/x11/connection.rs | 6 ++-- window/src/os/x11/keyboard.rs | 43 +++++++++++++++-------------- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/window/src/os/wayland/connection.rs b/window/src/os/wayland/connection.rs index 908ee22d550..5bd2c0d43f6 100644 --- a/window/src/os/wayland/connection.rs +++ b/window/src/os/wayland/connection.rs @@ -1,3 +1,18 @@ +#![allow(dead_code)] +use super::pointer::*; +use super::window::*; +use crate::connection::ConnectionOps; +use crate::os::wayland::inputhandler::InputHandler; +use crate::os::wayland::output::OutputHandler; +use crate::os::x11::keyboard::KeyboardWithFallback; +use crate::screen::{ScreenInfo, Screens}; +use crate::spawn::*; +use crate::x11::ModifierInit; +use crate::{Appearance, Connection, ScreenRect, WindowEvent}; +use anyhow::{bail, Context}; +use mio::unix::SourceFd; +use mio::{Events, Interest, Poll, Token}; +use smithay_client_toolkit as toolkit; use std::cell::RefCell; use std::collections::HashMap; use std::os::fd::AsRawFd; @@ -120,7 +135,7 @@ impl WaylandConnection { data.pop(); } let s = String::from_utf8(data)?; - match KeyboardWithFallback::new_from_string(s, true) { + match KeyboardWithFallback::new_from_string(s, ModifierInit::Wayland) { Ok(k) => { self.keyboard_mapper.replace(Some(k)); } diff --git a/window/src/os/x11/connection.rs b/window/src/os/x11/connection.rs index 0a9f57a8a6d..9b99cafcadd 100644 --- a/window/src/os/x11/connection.rs +++ b/window/src/os/x11/connection.rs @@ -1,4 +1,4 @@ -use super::keyboard::{Keyboard, KeyboardWithFallback}; +use super::keyboard::{Keyboard, KeyboardWithFallback, ModifierInit}; use crate::connection::ConnectionOps; use crate::os::x11::window::XWindowInner; use crate::os::x11::xsettings::*; @@ -712,8 +712,8 @@ impl XConnection { visual.green_mask(), visual.blue_mask() ); - let (keyboard, kbd_ev) = Keyboard::new(&conn, false)?; - let keyboard = KeyboardWithFallback::new(keyboard, false)?; + let (keyboard, kbd_ev) = Keyboard::new(&conn, ModifierInit::X11)?; + let keyboard = KeyboardWithFallback::new(keyboard, ModifierInit::X11)?; let cursor_font_id = conn.generate_id(); let cursor_font_name = "cursor"; diff --git a/window/src/os/x11/keyboard.rs b/window/src/os/x11/keyboard.rs index 85c1d3acf5b..de7fef7dc18 100644 --- a/window/src/os/x11/keyboard.rs +++ b/window/src/os/x11/keyboard.rs @@ -41,6 +41,12 @@ struct StateFromXcbStateNotify { locked_layout: LayoutIndex, } +#[derive(Clone, Copy)] +pub enum ModifierInit { + X11, + Wayland, +} + pub struct KeyboardWithFallback { selected: Keyboard, fallback: Keyboard, @@ -171,16 +177,16 @@ fn default_keymap(context: &xkb::Context) -> Option { } impl KeyboardWithFallback { - pub fn new(selected: Keyboard, using_wayland: bool) -> anyhow::Result { + pub fn new(selected: Keyboard, modifier_init: ModifierInit) -> anyhow::Result { Ok(Self { selected, - fallback: Keyboard::new_default(using_wayland)?, + fallback: Keyboard::new_default(modifier_init)?, }) } - pub fn new_from_string(s: String, using_wayland: bool) -> anyhow::Result { - let selected = Keyboard::new_from_string(s, using_wayland)?; - Self::new(selected, using_wayland) + pub fn new_from_string(s: String, modifier_init: ModifierInit) -> anyhow::Result { + let selected = Keyboard::new_from_string(s, modifier_init)?; + Self::new(selected, modifier_init) } pub fn process_wayland_key( @@ -589,7 +595,7 @@ impl KeyboardWithFallback { } impl Keyboard { - pub fn new_default(using_wayland: bool) -> anyhow::Result { + pub fn new_default(modifier_init: ModifierInit) -> anyhow::Result { let context = xkb::Context::new(xkb::CONTEXT_NO_FLAGS); let keymap = default_keymap(&context) .ok_or_else(|| anyhow!("Failed to load system default keymap"))?; @@ -605,10 +611,9 @@ impl Keyboard { let phys_code_map = build_physkeycode_map(&keymap); let label = "fallback"; - let mod_map = if using_wayland { - init_modifier_table_wayland(&keymap) - } else { - init_modifier_table_x11(&keymap) + let mod_map = match modifier_init { + ModifierInit::Wayland => init_modifier_table_wayland(&keymap), + ModifierInit::X11 => init_modifier_table_x11(&keymap), }; Ok(Self { @@ -629,7 +634,7 @@ impl Keyboard { }) } - pub fn new_from_string(s: String, using_wayland: bool) -> anyhow::Result { + pub fn new_from_string(s: String, modifier_init: ModifierInit) -> anyhow::Result { let context = xkb::Context::new(xkb::CONTEXT_NO_FLAGS); let keymap = xkb::Keymap::new_from_string( &context, @@ -650,10 +655,9 @@ impl Keyboard { let phys_code_map = build_physkeycode_map(&keymap); let label = "selected"; - let mod_map = if using_wayland { - init_modifier_table_wayland(&keymap) - } else { - init_modifier_table_x11(&keymap) + let mod_map = match modifier_init { + ModifierInit::Wayland => init_modifier_table_wayland(&keymap), + ModifierInit::X11 => init_modifier_table_x11(&keymap), }; Ok(Self { @@ -676,7 +680,7 @@ impl Keyboard { pub fn new( connection: &xcb::Connection, - using_wayland: bool, + modifier_init: ModifierInit, ) -> anyhow::Result<(Keyboard, u8)> { let first_ev = xcb::xkb::get_extension_data(connection) .ok_or_else(|| anyhow!("could not get xkb extension data"))? @@ -730,10 +734,9 @@ impl Keyboard { let phys_code_map = build_physkeycode_map(&keymap); let label = "selected"; - let mod_map = if using_wayland { - init_modifier_table_wayland(&keymap) - } else { - init_modifier_table_x11(&keymap) + let mod_map = match modifier_init { + ModifierInit::Wayland => init_modifier_table_wayland(&keymap), + ModifierInit::X11 => init_modifier_table_x11(&keymap), }; let kbd = Self { From 592ddde3067b9b7ac11c0257ed69f139bbbe7bf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Mon, 4 Dec 2023 20:11:00 +0100 Subject: [PATCH 07/13] fix: Implement modifier init for X11 --- Cargo.lock | 1 + window/Cargo.toml | 1 + window/src/os/x11/connection.rs | 4 +- window/src/os/x11/keyboard.rs | 10 +- window/src/os/x11/modifiers.rs | 180 ++++++++++++++++++++++++++++++-- 5 files changed, 181 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3ee07bae9f9..1c8634efd55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6752,6 +6752,7 @@ dependencies = [ "promise", "raw-window-handle", "resize", + "scopeguard", "serde", "shared_library", "shlex", diff --git a/window/Cargo.toml b/window/Cargo.toml index 3cfa26dc9b6..86a9653561f 100644 --- a/window/Cargo.toml +++ b/window/Cargo.toml @@ -46,6 +46,7 @@ wezterm-bidi = { path = "../bidi" } wezterm-color-types = { path = "../color-types" } wezterm-font = { path = "../wezterm-font" } wezterm-input-types = { path = "../wezterm-input-types" } +scopeguard = "1.2.0" [target."cfg(windows)".dependencies] clipboard-win = "2.2" diff --git a/window/src/os/x11/connection.rs b/window/src/os/x11/connection.rs index 9b99cafcadd..66e1fab9b92 100644 --- a/window/src/os/x11/connection.rs +++ b/window/src/os/x11/connection.rs @@ -712,8 +712,8 @@ impl XConnection { visual.green_mask(), visual.blue_mask() ); - let (keyboard, kbd_ev) = Keyboard::new(&conn, ModifierInit::X11)?; - let keyboard = KeyboardWithFallback::new(keyboard, ModifierInit::X11)?; + let (keyboard, kbd_ev) = Keyboard::new(&conn, ModifierInit::X11(&conn))?; + let keyboard = KeyboardWithFallback::new(keyboard, ModifierInit::X11(&conn))?; let cursor_font_id = conn.generate_id(); let cursor_font_name = "cursor"; diff --git a/window/src/os/x11/keyboard.rs b/window/src/os/x11/keyboard.rs index de7fef7dc18..7b0b5bdd8b1 100644 --- a/window/src/os/x11/keyboard.rs +++ b/window/src/os/x11/keyboard.rs @@ -42,8 +42,8 @@ struct StateFromXcbStateNotify { } #[derive(Clone, Copy)] -pub enum ModifierInit { - X11, +pub enum ModifierInit<'a> { + X11(&'a xcb::Connection), Wayland, } @@ -613,7 +613,7 @@ impl Keyboard { let mod_map = match modifier_init { ModifierInit::Wayland => init_modifier_table_wayland(&keymap), - ModifierInit::X11 => init_modifier_table_x11(&keymap), + ModifierInit::X11(conn) => init_modifier_table_x11(&conn, &keymap), }; Ok(Self { @@ -657,7 +657,7 @@ impl Keyboard { let mod_map = match modifier_init { ModifierInit::Wayland => init_modifier_table_wayland(&keymap), - ModifierInit::X11 => init_modifier_table_x11(&keymap), + ModifierInit::X11(conn) => init_modifier_table_x11(&conn, &keymap), }; Ok(Self { @@ -736,7 +736,7 @@ impl Keyboard { let mod_map = match modifier_init { ModifierInit::Wayland => init_modifier_table_wayland(&keymap), - ModifierInit::X11 => init_modifier_table_x11(&keymap), + ModifierInit::X11(conn) => init_modifier_table_x11(&conn, &keymap), }; let kbd = Self { diff --git a/window/src/os/x11/modifiers.rs b/window/src/os/x11/modifiers.rs index f02b824e054..9574d8ac5a3 100644 --- a/window/src/os/x11/modifiers.rs +++ b/window/src/os/x11/modifiers.rs @@ -1,3 +1,9 @@ +use libc::c_uint; +use scopeguard::defer; +use std::ffi::CStr; +use std::os::raw::{c_char, c_int}; +use x11::xlib; +use xcb; use xkbcommon::xkb::{self}; #[derive(Debug, Clone, Copy)] @@ -214,8 +220,9 @@ fn init_modifier_table_fallback(keymap: &xkb::Keymap) -> ModifierMap { macro_rules! assign { ($mod:ident, $n:expr) => { - let idx = keymap.mod_get_index($n); - mod_map.$mod = ModifierIndex { idx: idx }; + mod_map.$mod = ModifierIndex { + idx: keymap.mod_get_index($n), + }; }; } @@ -236,10 +243,166 @@ fn init_modifier_table_fallback(keymap: &xkb::Keymap) -> ModifierMap { /// all modifiers /// `Ctrl`, `Shift`, `Alt`, `Num_Lock`, `Caps_Lock`, `Super`, /// `Hyper`, `Meta`. -pub fn init_modifier_table_x11(keymap: &xkb::Keymap) -> ModifierMap { +pub fn init_modifier_table_x11(connection: &xcb::Connection, keymap: &xkb::Keymap) -> ModifierMap { // TODO: This implementation needs to be done with - // https://github.com/kovidgoyal/kitty/blob/0248edbdb98cc3ae80d98bf5ad17fbf497a24a43/glfw/xkb_glfw.c#L321 return init_modifier_table_fallback(keymap); - return init_modifier_table_fallback(keymap); + // https://github.com/kovidgoyal/kitty/blob/0248edbdb98cc3ae80d98bf5ad17fbf497a24a43/glfw/xkb_glfw.c#L321 + + let mut mod_map = ModifierMap::default(); + + // Keeping track of used real modifiers (bits). + let mut used_real_mod_bits: u32 = 0; + + // Shift, Ctrl, CapsLock are special they cannot be identified reliably on X11 + macro_rules! assign { + ($mod:ident, $n:expr) => {{ + mod_map.$mod = ModifierIndex { + idx: keymap.mod_get_index($n), + }; + used_real_mod_bits |= 1 << mod_map.$mod.idx; + }}; + } + assign!(ctrl, xkb::MOD_NAME_CTRL); + assign!(shift, xkb::MOD_NAME_SHIFT); + assign!(caps_lock, xkb::MOD_NAME_CAPS); + + // Mask variables for all modifiers not yet set. + // The value stored here represents the same as `used_real_mod_bits` + // Each bit is a real modifier (only 8 modifiers). + let mut alt: u32 = 0; + let mut supr: u32 = 0; + let mut num_lock: u32 = 0; + let mut meta: u32 = 0; + let mut hyper: u32 = 0; + + let success = || -> bool { + let xkb_ptr = unsafe { + xlib::XkbGetMap( + connection.get_raw_dpy(), + (xcb::xkb::MapPart::VIRTUAL_MODS | xcb::xkb::MapPart::VIRTUAL_MOD_MAP).bits(), + xcb::xkb::Id::UseCoreKbd as c_uint, + ) + }; + + if xkb_ptr.is_null() { + log::warn!("Could not initialize moidifiers from virtual modifiers table."); + return false; + } + + defer! { + unsafe { xlib::XkbFreeKeyboard(xkb_ptr, 0, xlib::True); } + } + + let status = unsafe { + xlib::XkbGetNames( + connection.get_raw_dpy(), + xcb::xkb::NameDetail::VIRTUAL_MOD_NAMES.bits(), + xkb_ptr, + ) + }; + + if status != xlib::Success as c_int { + return false; + } + + defer! { + unsafe { + xlib::XkbFreeNames( + xkb_ptr, + xcb::xkb::NameDetail::VIRTUAL_MOD_NAMES.bits() as c_uint, + xlib::True, + )}; + } + + // Iterate over all virtual modifiers and get its map to the real modifiers + // (variable `mask`) and check if the virtual modifier name + // matches a wezterm internal modifier we handle. + for i in 0..xlib::XkbNumVirtualMods { + let atom = unsafe { (*(*xkb_ptr).names).vmods[i] }; + + if atom == 0 { + continue; + } + + let mut mask: c_uint = 0; + let mask_ptr: *mut c_uint = &mut mask; + + if unsafe { xlib::XkbVirtualModsToReal(xkb_ptr, (1 << i) as c_uint, mask_ptr) } + == xlib::Success as c_int + { + log::debug!("Could not map virtual modifier index '{i}' to real modifier mask."); + continue; + } + + if used_real_mod_bits & mask != 0 { + // The variable `mask` (multiple bits can be set, which means + // this virtual modifier maps to multiple real modifiers) and overlaps + // already used bits. + continue; + } + + let name: *mut c_char = unsafe { xlib::XGetAtomName(connection.get_raw_dpy(), atom) }; + let virtual_modifier_name = match unsafe { CStr::from_ptr(name).to_str() } { + Ok(n) => n, + Err(_) => continue, + }; + + match virtual_modifier_name { + // Note that the order matters here; earlier is higher priority. + "Alt" => { + alt = mask; + used_real_mod_bits |= mask; + } + "Super" => { + supr = mask; + used_real_mod_bits |= mask; + } + "NumLock" => { + num_lock = mask; + used_real_mod_bits |= mask; + } + "Meta" => { + meta = mask; + used_real_mod_bits |= mask; + } + "Hyper" => { + hyper = mask; + used_real_mod_bits |= mask; + } + _ => (), + } + } + + return true; + }(); + + if !success || used_real_mod_bits == 0 { + return init_modifier_table_fallback(keymap); + } + + macro_rules! assign { + ($mod:ident, $i:ident) => { + if $mod & ($i << 1) != 0 { + mod_map.$mod = ModifierIndex { idx: $i }; + } + }; + } + + // We could assign the bit index also above in the match statement, but + // this is more efficient and we can do it together by traversing + // the used bits. + let mut idx = 0; + while used_real_mod_bits != 0 { + assign!(alt, idx); + assign!(supr, idx); + assign!(hyper, idx); + assign!(meta, idx); + assign!(num_lock, idx); + + idx += 1; + used_real_mod_bits >>= 1; + } + + return mod_map; } /// This function initializes wezterm internal modifiers @@ -261,6 +424,7 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { // // This is a client side hack for wayland, once // https://github.com/xkbcommon/libxkbcommon/pull/36 + // or https://github.com/xkbcommon/libxkbcommon/pull/353 // is implemented or some other solution exists // this code iterates through key presses to find the // activated modifiers. @@ -301,18 +465,18 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { if !algo.failed { let mut shifted: u32 = 1; - let mut used_bits: u32 = 0; + let mut used_real_mod_bits: u32 = 0; // Algorithm can detect the same index for multiple modifiers // but we only want one assigned to each of our modifiers. macro_rules! try_assign_unique_index { ($mod:ident, $i:ident) => { if mod_map.$mod.idx == xkb::MOD_INVALID - && (used_bits & shifted == 0) + && (used_real_mod_bits & shifted == 0) && algo.$mod == shifted { mod_map.$mod = ModifierIndex { idx: $i }; - used_bits |= shifted; + used_real_mod_bits |= shifted; } }; } From 82cfe32d7b8a9dee3b1d54226c9b4991a0dd4397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Mon, 4 Dec 2023 20:45:01 +0100 Subject: [PATCH 08/13] fix: Documentation --- window/src/os/x11/modifiers.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/window/src/os/x11/modifiers.rs b/window/src/os/x11/modifiers.rs index 9574d8ac5a3..250cc85323c 100644 --- a/window/src/os/x11/modifiers.rs +++ b/window/src/os/x11/modifiers.rs @@ -31,7 +31,7 @@ pub struct ModifierMap { pub hyper: ModifierIndex, } -struct Algorithm<'a> { +struct KeyProbingAlgorithm<'a> { failed: bool, try_shift: bool, @@ -49,9 +49,9 @@ struct Algorithm<'a> { state: &'a mut xkb::State, } -impl<'a> Algorithm<'a> { - fn new(state: &'a mut xkb::State) -> Algorithm<'a> { - return Algorithm { +impl<'a> KeyProbingAlgorithm<'a> { + fn new(state: &'a mut xkb::State) -> KeyProbingAlgorithm<'a> { + return KeyProbingAlgorithm { failed: false, try_shift: false, shift_keycode: 0, @@ -70,7 +70,7 @@ impl<'a> Algorithm<'a> { } } -impl<'a> Algorithm<'a> { +impl<'a> KeyProbingAlgorithm<'a> { /// Algorithm for mapping virtual modifiers to real modifiers: /// 1. create new state /// 2. for each key in keymap @@ -209,6 +209,7 @@ impl<'a> Algorithm<'a> { /// This function initializes wezterm internal modifiers depending /// on a default mapping. +/// /// This function simply queries the index for the xkb modifiers /// `Control`, `Lock`, `Shift`, `Mod1`, `Mod2`, `Mod4` /// and treats them as default (assumption) to @@ -238,11 +239,13 @@ fn init_modifier_table_fallback(keymap: &xkb::Keymap) -> ModifierMap { /// This function initializes wezterm internal modifiers /// by looking up virtual modifiers (e.g. run `xmodmap -pm`) -/// and -/// This function initializes `xkb` modifier indices for +/// over the functionality exposed by X11. +/// This function tries to initialize `xkb` modifier indices for /// all modifiers /// `Ctrl`, `Shift`, `Alt`, `Num_Lock`, `Caps_Lock`, `Super`, /// `Hyper`, `Meta`. +/// If not successful it uses [init_modifier_table_fallback](init_modifier_table_fallback) +/// to initialize the default. pub fn init_modifier_table_x11(connection: &xcb::Connection, keymap: &xkb::Keymap) -> ModifierMap { // TODO: This implementation needs to be done with // https://github.com/kovidgoyal/kitty/blob/0248edbdb98cc3ae80d98bf5ad17fbf497a24a43/glfw/xkb_glfw.c#L321 @@ -433,7 +436,7 @@ pub fn init_modifier_table_wayland(keymap: &xkb::Keymap) -> ModifierMap { log::info!("Detect modifiers on Wayland [with key press iterations]."); let mut state: xkb::State = xkb::State::new(keymap); - let mut algo = Algorithm::new(&mut state); + let mut algo = KeyProbingAlgorithm::new(&mut state); let mut mod_map = ModifierMap::default(); algo.visit_keymap(&keymap); From c7f5b4dfe3ee4b4fe5d2a492abcb2157b45d8167 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Thu, 7 Dec 2023 09:09:03 +0100 Subject: [PATCH 09/13] fix: Add modifier map init also on update keymap --- window/src/os/x11/keyboard.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/window/src/os/x11/keyboard.rs b/window/src/os/x11/keyboard.rs index 7b0b5bdd8b1..deabdda96af 100644 --- a/window/src/os/x11/keyboard.rs +++ b/window/src/os/x11/keyboard.rs @@ -866,9 +866,13 @@ impl Keyboard { ensure!(!new_state.get_raw_ptr().is_null(), "problem with new state"); let phys_code_map = build_physkeycode_map(&new_keymap); + // This function is only called from X11. + init_modifier_table_x11(&connection, &new_keymap); + self.state.replace(new_state); self.keymap.replace(new_keymap); self.phys_code_map.replace(phys_code_map); + Ok(()) } } From 2196c9ab491755ea69c48a54a55dcff034de1f17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Thu, 7 Dec 2023 14:20:52 +0100 Subject: [PATCH 10/13] fix: Add capslock on macOS to modifier state --- window/src/os/macos/window.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/window/src/os/macos/window.rs b/window/src/os/macos/window.rs index 7576e72b637..fbb537f3639 100644 --- a/window/src/os/macos/window.rs +++ b/window/src/os/macos/window.rs @@ -1754,6 +1754,9 @@ fn key_modifiers(flags: NSEventModifierFlags) -> Modifiers { if flags.contains(NSEventModifierFlags::NSCommandKeyMask) { mods |= Modifiers::SUPER; } + if flags.contains(NSEventModifierFlags::NSAlphaShiftKeyMask) { + mods |= Modifiers::CAPS_LOCK; + } mods } From 45b4baae19d45b77b9047e2923a70f50a7217d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Thu, 7 Dec 2023 14:25:34 +0100 Subject: [PATCH 11/13] fix: Add capslock and numlock on Windows --- window/src/os/windows/window.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/window/src/os/windows/window.rs b/window/src/os/windows/window.rs index 58053344991..5068eb0d584 100644 --- a/window/src/os/windows/window.rs +++ b/window/src/os/windows/window.rs @@ -2592,9 +2592,11 @@ unsafe fn key(hwnd: HWND, msg: UINT, wparam: WPARAM, lparam: LPARAM) -> Option Date: Thu, 7 Dec 2023 18:17:40 +0100 Subject: [PATCH 12/13] fix: Emit correct codes for latched keys... - Caps and NumLock should also be correctly encoded if the they are latched. This was ensured by checking the led status. Continue doing this. --- wezterm-input-types/src/lib.rs | 52 +++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/wezterm-input-types/src/lib.rs b/wezterm-input-types/src/lib.rs index a1ae14c7f99..635d8e57161 100644 --- a/wezterm-input-types/src/lib.rs +++ b/wezterm-input-types/src/lib.rs @@ -1685,10 +1685,14 @@ impl KeyEvent { if raw_modifiers.contains(Modifiers::META) { modifiers |= 32; } - if raw_modifiers.contains(Modifiers::CAPS_LOCK) { + if raw_modifiers.contains(Modifiers::CAPS_LOCK) + || self.leds.contains(KeyboardLedStatus::CAPS_LOCK) + { modifiers |= 64; } - if raw_modifiers.contains(Modifiers::NUM_LOCK) { + if raw_modifiers.contains(Modifiers::NUM_LOCK) + || self.leds.contains(KeyboardLedStatus::NUM_LOCK) + { modifiers |= 128; } modifiers += 1; @@ -1757,7 +1761,11 @@ impl KeyEvent { }); if let Some(numpad) = is_numpad { - let code = match (numpad, self.leds.contains(KeyboardLedStatus::NUM_LOCK)) { + let code = match ( + numpad, + self.leds.contains(KeyboardLedStatus::NUM_LOCK) + || raw_modifiers.contains(Modifiers::NUM_LOCK), + ) { (PhysKeyCode::Keypad0, true) => 57399, (PhysKeyCode::Keypad0, false) => 57425, (PhysKeyCode::Keypad1, true) => 57400, @@ -1822,26 +1830,25 @@ impl KeyEvent { && event_type.is_empty() && is_legacy_key && !(flags.contains(KittyKeyboardFlags::DISAMBIGUATE_ESCAPE_CODES) - && (self.modifiers.contains(Modifiers::CTRL) - || self.modifiers.contains(Modifiers::ALT))) - && !self.modifiers.intersects( - Modifiers::SUPER, /* TODO: Hyper and Meta should be added here. */ + && (raw_modifiers.contains(Modifiers::CTRL) + || raw_modifiers.contains(Modifiers::ALT))) + && !raw_modifiers.intersects( + Modifiers::SUPER + | Modifiers::META + | Modifiers::NUM_LOCK + | Modifiers::CAPS_LOCK, ); if use_legacy { // Legacy text key // https://sw.kovidgoyal.net/kitty/keyboard-protocol/#legacy-text-keys let mut output = String::new(); - if self.modifiers.contains(Modifiers::ALT) { + if raw_modifiers.contains(Modifiers::ALT) { output.push('\x1b'); } - if self.modifiers.contains(Modifiers::CTRL) { - csi_u_encode( - &mut output, - shifted_key.to_ascii_uppercase(), - self.modifiers, - ); + if raw_modifiers.contains(Modifiers::CTRL) { + csi_u_encode(&mut output, shifted_key.to_ascii_uppercase(), raw_modifiers); } else { output.push(shifted_key); } @@ -2708,6 +2715,23 @@ mod test { .encode_kitty(flags), "\u{1b}[57399;129u".to_string() ); + assert_eq!( + make_event_with_raw( + KeyEvent { + key: KeyCode::Numpad(0), + modifiers: Modifiers::NUM_LOCK, + leds: KeyboardLedStatus::empty(), + repeat_count: 1, + key_is_down: true, + raw: None, + #[cfg(windows)] + win32_uni_char: None, + }, + Some(PhysKeyCode::Keypad0) + ) + .encode_kitty(flags), + "\u{1b}[57399;129u".to_string() + ); assert_eq!( make_event_with_raw( KeyEvent { From 9018bdcf99fbed7b4990832fdfd5e5aeea007ce1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20N=C3=BCtzi?= Date: Mon, 11 Dec 2023 15:59:20 +0100 Subject: [PATCH 13/13] fix: Small test fix because of modifiers size change --- termwiz/src/escape/csi.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/termwiz/src/escape/csi.rs b/termwiz/src/escape/csi.rs index 810f99f1f3f..d5748ad3f37 100644 --- a/termwiz/src/escape/csi.rs +++ b/termwiz/src/escape/csi.rs @@ -46,7 +46,7 @@ fn csi_size() { assert_eq!(std::mem::size_of::(), 12); assert_eq!(std::mem::size_of::(), 8); assert_eq!(std::mem::size_of::(), 24); - assert_eq!(std::mem::size_of::(), 8); + assert_eq!(std::mem::size_of::(), 12); assert_eq!(std::mem::size_of::(), 40); assert_eq!(std::mem::size_of::(), 8); assert_eq!(std::mem::size_of::(), 32);