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

@@ -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<VirtualKeyCode> {
     }
 }
 
-impl From<keyboard::ModifiersState> for ModifiersState {
+impl ModifiersState {
-    fn from(mods: keyboard::ModifiersState) -> ModifiersState {
+    pub(crate) fn from_wayland(mods: keyboard::ModifiersState) -> ModifiersState {
         ModifiersState {
             shift: mods.shift,
             ctrl: mods.ctrl,

src/platform_impl/linux/x11/event_processor.rs

@@ -120,6 +120,26 @@ impl<T: 'static> EventProcessor<T> {
             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<T: 'static> EventProcessor<T> {
                         .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<T: 'static> EventProcessor<T> {
                     };
                     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<T: 'static> EventProcessor<T> {
                             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<T: 'static> EventProcessor<T> {
                         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<T: 'static> EventProcessor<T> {
                             event: CursorMoved {
                                 device_id: mkdid(pointer_id),
                                 position,
-                                modifiers: ModifiersState::from(xev.mods),
+                                modifiers: ModifiersState::from_x11(&xev.mods),
                             },
                         });
                     }

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<ffi::XIModifierState> for ModifiersState {
+impl ModifiersState {
-    fn from(mods: ffi::XIModifierState) -> Self {
+    pub(crate) fn from_x11(state: &ffi::XIModifierState) -> Self {
-        let state = mods.effective as c_uint;
+        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,
+            alt: mask & ffi::Mod1Mask != 0,
-            shift: state & ffi::ShiftMask != 0,
+            shift: mask & ffi::ShiftMask != 0,
-            ctrl: state & ffi::ControlMask != 0,
+            ctrl: mask & ffi::ControlMask != 0,
-            logo: state & ffi::Mod4Mask != 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)
     }
 }
 

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<ffi::KeyCode, Modifier>,
+    state: ModifiersState,
 }
 
 impl ModifierKeymap {
@@ -94,15 +95,7 @@ impl ModifierKeymap {
 }
 
 impl ModifierKeyState {
-    pub fn clear(&mut self) {
+    pub fn update_keymap(&mut self, mods: &ModifierKeymap) {
-        self.keys.clear();
-    }
-
-    pub fn is_empty(&self) -> bool {
-        self.keys.is_empty()
-    }
-
-    pub fn update(&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<Modifier>,
+    ) -> Option<ModifiersState> {
+        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();
+        self.state
-
-        for &m in self.keys.values() {
-            set_modifier(&mut state, m);
-        }
-
-        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::Alt => state.alt,
-        Modifier::Ctrl => state.ctrl = true,
+        Modifier::Ctrl => state.ctrl,
-        Modifier::Shift => state.shift = true,
+        Modifier::Shift => state.shift,
-        Modifier::Logo => state.logo = true,
+        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,
     }
 }