From a95ebc5ee6a085140c57735aab44bcfff7f839ea Mon Sep 17 00:00:00 2001 From: Murarth Date: Fri, 22 Nov 2019 17:11:30 -0700 Subject: [PATCH] X11: Fix incorrect modifiers when events are missed (#1279) * X11: Fix incorrect modifiers when events are missed * Syncs modifier state with state data in X key/button/motion events. * Fixes modifier state in XWayland, as xinput2 raw input events will not be received when a window does not have focus. * Removes `impl From<_> for ModifiersState` on X11/Wayland API types, replacing them with `pub(crate)` methods. * Cleanup modifier state update using a macro * Remove keys from modifier state when updating --- src/platform_impl/linux/wayland/keyboard.rs | 6 +- .../linux/x11/event_processor.rs | 35 +++++++- src/platform_impl/linux/x11/util/input.rs | 19 +++-- src/platform_impl/linux/x11/util/modifiers.rs | 82 ++++++++++++++----- 4 files changed, 105 insertions(+), 37 deletions(-) diff --git a/src/platform_impl/linux/wayland/keyboard.rs b/src/platform_impl/linux/wayland/keyboard.rs index 1b46f81f..31811422 100644 --- a/src/platform_impl/linux/wayland/keyboard.rs +++ b/src/platform_impl/linux/wayland/keyboard.rs @@ -97,7 +97,7 @@ pub fn init_keyboard( KbEvent::Modifiers { modifiers: event_modifiers, } => { - let modifiers = event_modifiers.into(); + let modifiers = ModifiersState::from_wayland(event_modifiers); *modifiers_tracker.lock().unwrap() = modifiers; @@ -399,8 +399,8 @@ fn keysym_to_vkey(keysym: u32) -> Option { } } -impl From for ModifiersState { - fn from(mods: keyboard::ModifiersState) -> ModifiersState { +impl ModifiersState { + pub(crate) fn from_wayland(mods: keyboard::ModifiersState) -> ModifiersState { ModifiersState { shift: mods.shift, ctrl: mods.ctrl, diff --git a/src/platform_impl/linux/x11/event_processor.rs b/src/platform_impl/linux/x11/event_processor.rs index 9c74e74b..2758aa35 100644 --- a/src/platform_impl/linux/x11/event_processor.rs +++ b/src/platform_impl/linux/x11/event_processor.rs @@ -120,6 +120,26 @@ impl EventProcessor { return; } + // We can't call a `&mut self` method because of the above borrow, + // so we use this macro for repeated modifier state updates. + macro_rules! update_modifiers { + ( $state:expr , $modifier:expr ) => {{ + match ($state, $modifier) { + (state, modifier) => { + if let Some(modifiers) = + self.device_mod_state.update_state(&state, modifier) + { + let device_id = mkdid(util::VIRTUAL_CORE_KEYBOARD); + callback(Event::DeviceEvent { + device_id, + event: DeviceEvent::ModifiersChanged { modifiers }, + }); + } + } + } + }}; + } + let event_type = xev.get_type(); match event_type { ffi::MappingNotify => { @@ -136,7 +156,7 @@ impl EventProcessor { .expect("Failed to call XRefreshKeyboardMapping"); self.mod_keymap.reset_from_x_connection(&wt.xconn); - self.device_mod_state.update(&self.mod_keymap); + self.device_mod_state.update_keymap(&self.mod_keymap); } } @@ -553,6 +573,11 @@ impl EventProcessor { }; let virtual_keycode = events::keysym_to_element(keysym as c_uint); + update_modifiers!( + ModifiersState::from_x11_mask(xkev.state), + self.mod_keymap.get_modifier(xkev.keycode as ffi::KeyCode) + ); + let modifiers = self.device_mod_state.modifiers(); callback(Event::WindowEvent { @@ -618,7 +643,8 @@ impl EventProcessor { return; } - let modifiers = ModifiersState::from(xev.mods); + let modifiers = ModifiersState::from_x11(&xev.mods); + update_modifiers!(modifiers, None); let state = if xev.evtype == ffi::XI_ButtonPress { Pressed @@ -694,7 +720,8 @@ impl EventProcessor { let window_id = mkwid(xev.event); let new_cursor_pos = (xev.event_x, xev.event_y); - let modifiers = ModifiersState::from(xev.mods); + let modifiers = ModifiersState::from_x11(&xev.mods); + update_modifiers!(modifiers, None); let cursor_moved = self.with_window(xev.event, |window| { let mut shared_state_lock = window.shared_state.lock(); @@ -897,7 +924,7 @@ impl EventProcessor { event: CursorMoved { device_id: mkdid(pointer_id), position, - modifiers: ModifiersState::from(xev.mods), + modifiers: ModifiersState::from_x11(&xev.mods), }, }); } diff --git a/src/platform_impl/linux/x11/util/input.rs b/src/platform_impl/linux/x11/util/input.rs index 1f02090d..2fbabe1e 100644 --- a/src/platform_impl/linux/x11/util/input.rs +++ b/src/platform_impl/linux/x11/util/input.rs @@ -11,14 +11,17 @@ pub const VIRTUAL_CORE_KEYBOARD: c_int = 3; // To test if `lookup_utf8` works correctly, set this to 1. const TEXT_BUFFER_SIZE: usize = 1024; -impl From for ModifiersState { - fn from(mods: ffi::XIModifierState) -> Self { - let state = mods.effective as c_uint; +impl ModifiersState { + pub(crate) fn from_x11(state: &ffi::XIModifierState) -> Self { + ModifiersState::from_x11_mask(state.effective as c_uint) + } + + pub(crate) fn from_x11_mask(mask: c_uint) -> Self { ModifiersState { - alt: state & ffi::Mod1Mask != 0, - shift: state & ffi::ShiftMask != 0, - ctrl: state & ffi::ControlMask != 0, - logo: state & ffi::Mod4Mask != 0, + alt: mask & ffi::Mod1Mask != 0, + shift: mask & ffi::ShiftMask != 0, + ctrl: mask & ffi::ControlMask != 0, + logo: mask & ffi::Mod4Mask != 0, } } } @@ -40,7 +43,7 @@ pub struct PointerState<'a> { impl<'a> PointerState<'a> { pub fn get_modifier_state(&self) -> ModifiersState { - self.modifiers.into() + ModifiersState::from_x11(&self.modifiers) } } diff --git a/src/platform_impl/linux/x11/util/modifiers.rs b/src/platform_impl/linux/x11/util/modifiers.rs index 9b8da4b7..9bdc6e99 100644 --- a/src/platform_impl/linux/x11/util/modifiers.rs +++ b/src/platform_impl/linux/x11/util/modifiers.rs @@ -35,6 +35,7 @@ pub struct ModifierKeymap { pub struct ModifierKeyState { // Contains currently pressed modifier keys and their corresponding modifiers keys: HashMap, + state: ModifiersState, } impl ModifierKeymap { @@ -94,15 +95,7 @@ impl ModifierKeymap { } impl ModifierKeyState { - pub fn clear(&mut self) { - self.keys.clear(); - } - - pub fn is_empty(&self) -> bool { - self.keys.is_empty() - } - - pub fn update(&mut self, mods: &ModifierKeymap) { + pub fn update_keymap(&mut self, mods: &ModifierKeymap) { self.keys.retain(|k, v| { if let Some(m) = mods.get_modifier(*k) { *v = m; @@ -111,16 +104,36 @@ impl ModifierKeyState { false } }); + + self.reset_state(); + } + + pub fn update_state( + &mut self, + state: &ModifiersState, + except: Option, + ) -> Option { + let mut new_state = *state; + + match except { + Some(Modifier::Alt) => new_state.alt = self.state.alt, + Some(Modifier::Ctrl) => new_state.ctrl = self.state.ctrl, + Some(Modifier::Shift) => new_state.shift = self.state.shift, + Some(Modifier::Logo) => new_state.logo = self.state.logo, + None => (), + } + + if self.state == new_state { + None + } else { + self.keys.retain(|_k, v| get_modifier(&new_state, *v)); + self.state = new_state; + Some(new_state) + } } pub fn modifiers(&self) -> ModifiersState { - let mut state = ModifiersState::default(); - - for &m in self.keys.values() { - set_modifier(&mut state, m); - } - - state + self.state } pub fn key_event(&mut self, state: ElementState, keycode: ffi::KeyCode, modifier: Modifier) { @@ -132,18 +145,43 @@ impl ModifierKeyState { pub fn key_press(&mut self, keycode: ffi::KeyCode, modifier: Modifier) { self.keys.insert(keycode, modifier); + + set_modifier(&mut self.state, modifier, true); } pub fn key_release(&mut self, keycode: ffi::KeyCode) { - self.keys.remove(&keycode); + if let Some(modifier) = self.keys.remove(&keycode) { + if self.keys.values().find(|&&m| m == modifier).is_none() { + set_modifier(&mut self.state, modifier, false); + } + } + } + + fn reset_state(&mut self) { + let mut new_state = ModifiersState::default(); + + for &m in self.keys.values() { + set_modifier(&mut new_state, m, true); + } + + self.state = new_state; } } -fn set_modifier(state: &mut ModifiersState, modifier: Modifier) { +fn get_modifier(state: &ModifiersState, modifier: Modifier) -> bool { match modifier { - Modifier::Alt => state.alt = true, - Modifier::Ctrl => state.ctrl = true, - Modifier::Shift => state.shift = true, - Modifier::Logo => state.logo = true, + Modifier::Alt => state.alt, + Modifier::Ctrl => state.ctrl, + Modifier::Shift => state.shift, + Modifier::Logo => state.logo, + } +} + +fn set_modifier(state: &mut ModifiersState, modifier: Modifier, value: bool) { + match modifier { + Modifier::Alt => state.alt = value, + Modifier::Ctrl => state.ctrl = value, + Modifier::Shift => state.shift = value, + Modifier::Logo => state.logo = value, } }