Implement compositor-side key repeating

Author: dcz

anvil/src/focus.rs

@@ -3,14 +3,20 @@ use std::borrow::Cow;
 #[cfg(feature = "xwayland")]
 use smithay::xwayland::X11Surface;
 pub use smithay::{
-    backend::input::KeyState,
     desktop::{LayerSurface, PopupKind},
     input::{
         keyboard::{KeyboardTarget, KeysymHandle, ModifiersState},
         pointer::{AxisFrame, ButtonEvent, MotionEvent, PointerTarget, RelativeMotionEvent},
         Seat,
     },
-    reexports::wayland_server::{backend::ObjectId, protocol::wl_surface::WlSurface, Resource},
+    reexports::wayland_server::{
+        backend::ObjectId,
+        protocol::{
+            wl_keyboard::KeyState,
+            wl_surface::WlSurface,
+        },
+        Resource,
+    },
     utils::{IsAlive, Serial},
     wayland::seat::WaylandFocus,
 };

anvil/src/input_handler.rs

@@ -35,7 +35,7 @@ use smithay::{
     },
     output::Output,
     reexports::{
-        calloop::{generic::Generic, Interest, LoopHandle, Mode, PostAction},
+        calloop::{self, generic::Generic, Interest, LoopHandle, Mode, PostAction, RegistrationToken},
         wayland_protocols::xdg::decoration::{
             self as xdg_decoration, zv1::server::zxdg_toplevel_decoration_v1::Mode as DecorationMode,
         },
@@ -301,6 +301,27 @@ impl<BackendData: Backend> SeatHandler for AnvilState<BackendData> {
     fn led_state_changed(&mut self, _seat: &Seat<Self>, led_state: LedState) {
         self.backend_data.update_led_state(led_state)
     }
+
+    fn set_timeout(
+        &mut self,
+        duration: std::time::Duration,
+        mut callback: impl FnMut(&mut Self) + 'static,
+    ) -> RegistrationToken {
+        self.handle.insert_source(
+            calloop::timer::Timer::from_duration(duration),
+            move |_, _, state| {
+                callback(state);
+                calloop::timer::TimeoutAction::ToDuration(duration)
+            },
+        ).unwrap()
+    }
+    
+    fn clear_timeout(
+        &mut self,
+        token: RegistrationToken,
+    ) {
+        self.handle.remove(token);
+    }
 }
 delegate_seat!(@<BackendData: Backend + 'static> AnvilState<BackendData>);
 

anvil/src/state.rs

@@ -96,6 +96,8 @@ pub enum KeyState {
     Pressed,
 }
 
+pub use wayland_server::protocol::wl_keyboard::KeyState as KeyEvent;
+
 /// Trait for keyboard event
 pub trait KeyboardKeyEvent<B: InputBackend>: Event<B> {
     /// Returns the numerical button code of the keyboard button.

src/backend/input/mod.rs

@@ -6,7 +6,7 @@ use std::{
 use wayland_server::{protocol::wl_surface::WlSurface, Resource};
 
 use crate::{
-    backend::input::{ButtonState, KeyState, Keycode},
+    backend::input::{ButtonState, KeyEvent, Keycode},
     input::{
         keyboard::{
             GrabStartData as KeyboardGrabStartData, KeyboardGrab, KeyboardHandle, KeyboardInnerHandle,
@@ -434,7 +434,7 @@ where
         data: &mut D,
         handle: &mut KeyboardInnerHandle<'_, D>,
         keycode: Keycode,
-        state: KeyState,
+        state: KeyEvent,
         modifiers: Option<ModifiersState>,
         serial: Serial,
         time: u32,

src/desktop/wayland/popup/grab.rs

@@ -1,9 +1,12 @@
 //! Keyboard-related types for smithay's input abstraction
 
-use crate::backend::input::KeyState;
+use crate::backend::input::{Event, InputBackend, KeyEvent, KeyState, KeyboardKeyEvent};
+use crate::reexports::calloop::LoopHandle;
 use crate::utils::{IsAlive, Serial, SERIAL_COUNTER};
+use calloop::RegistrationToken;
 use downcast_rs::{impl_downcast, Downcast};
 use std::collections::HashSet;
+use std::time::Duration;
 #[cfg(feature = "wayland_frontend")]
 use std::sync::RwLock;
 use std::{
@@ -12,7 +15,7 @@ use std::{
     sync::{Arc, Mutex},
 };
 use thiserror::Error;
-use tracing::{debug, error, info, info_span, instrument, trace};
+use tracing::{debug, error, info, info_span, instrument, trace, warn};
 
 use xkbcommon::xkb::ffi::XKB_STATE_LAYOUT_EFFECTIVE;
 pub use xkbcommon::xkb::{self, keysyms, Keycode, Keysym};
@@ -47,7 +50,7 @@ where
         seat: &Seat<D>,
         data: &mut D,
         key: KeysymHandle<'_>,
-        state: KeyState,
+        state: KeyEvent,
         serial: Serial,
         time: u32,
     );
@@ -215,6 +218,17 @@ pub(crate) struct KbdInternal<D: SeatHandler> {
     led_mapping: LedMapping,
     pub(crate) led_state: LedState,
     grab: GrabStatus<dyn KeyboardGrab<D>>,
+    /// Warning: the token cannot unregister itself when the object is dropped, because it can't hold a reference to the event loop. This object is Send, but the event loop is not.
+    pub(crate) key_repeat_timer: Option<RegistrationToken>,
+}
+
+#[cfg(feature = "wayland_frontend")]
+impl<D: SeatHandler> Drop for KbdInternal<D> {
+    fn drop(&mut self) {
+        if self.key_repeat_timer.is_some() {
+            error!("A keyboard was dropped without unregistering a repeat handler. This is a bug in smithay or in the compositor.");
+        }
+    }
 }
 
 // focus_hook does not implement debug, so we have to impl Debug manually
@@ -229,6 +243,7 @@ impl<D: SeatHandler> fmt::Debug for KbdInternal<D> {
             .field("xkb", &self.xkb)
             .field("repeat_rate", &self.repeat_rate)
             .field("repeat_delay", &self.repeat_delay)
+            .field("key_repeat_timer", &self.key_repeat_timer)
             .finish()
     }
 }
@@ -266,21 +281,25 @@ impl<D: SeatHandler + 'static> KbdInternal<D> {
             led_mapping,
             led_state,
             grab: GrabStatus::None,
+            key_repeat_timer: None,
         })
     }
 
     // returns whether the modifiers or led state has changed
-    fn key_input(&mut self, keycode: Keycode, state: KeyState) -> (bool, bool) {
+    fn key_input(&mut self, keycode: Keycode, state: KeyEvent) -> (bool, bool) {
         // track pressed keys as xkbcommon does not seem to expose it :(
         let direction = match state {
-            KeyState::Pressed => {
+            KeyEvent::Pressed => {
                 self.pressed_keys.insert(keycode);
                 xkb::KeyDirection::Down
             }
-            KeyState::Released => {
+            KeyEvent::Released => {
                 self.pressed_keys.remove(&keycode);
                 xkb::KeyDirection::Up
             }
+            KeyEvent::Repeated | _ => {
+                return (false, false);
+            }
         };
 
         // update state
@@ -602,10 +621,10 @@ pub trait KeyboardGrab<D: SeatHandler>: Downcast {
         data: &mut D,
         handle: &mut KeyboardInnerHandle<'_, D>,
         keycode: Keycode,
-        state: KeyState,
+        event: KeyEvent,
         modifiers: Option<ModifiersState>,
         serial: Serial,
-        time: u32,
+        time_ms: u32,
     );
 
     /// A focus change was requested.
@@ -931,6 +950,91 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
         }
     }
 
+    /// Processes the keyboard event, starting or stopping key repeat.
+    /// If this method is used, it must receive all keyboard events.
+    /// The `on_event` argument will be called with all events: the ones received directly and the generated repeats.
+    pub fn key_register_repeat<B: InputBackend>(
+        &self,
+        data: &mut D,
+        get_handle: impl Fn(&D) -> &LoopHandle<'static, D> + 'static,
+        event: B::KeyboardKeyEvent,
+        // event, timeout, code.
+        // This is Clone because there are two closures in here...
+        on_event: impl Fn(&mut D, KeyEvent, u32, Keycode) + Clone + 'static,
+    ) {
+        let time_ms = event.time_msec();
+        let keycode = event.key_code();
+        let state = event.state();
+    
+        // Forward initial hardware event as logical event
+        on_event(data, state.into(), time_ms, keycode);
+
+        // Unregister preexisting repeating
+        // Releasing a key press obviously stops the repeat.
+        // But also, pressing another key stops the repeat of the previous key and starts it for the newly pressed key.
+        // TODO: this may had odd consequences when a modifier is pressed as the second key. But is that worth worrying about?
+        self.key_stop_repeat(data, &get_handle);
+        
+        // Register repeating
+        match event.state() {
+            KeyState::Pressed => {
+                let mut guard = self.arc.internal.lock().unwrap();
+                let delay = guard.repeat_delay;
+                let rate = guard.repeat_rate;
+                let mut time_ms = time_ms;
+
+                // This closure-in-closure business is somewhat ugly.
+                // The reason is that there are two timers needed: first, the delay timer, and after the delay, the repeat timer. Both of them receive different tokens for cancelling, so we have to swap the token after the delay.
+                // The only comparable alternative I can think of is to wrap the key_repeat_timer in an Mutex<Arc<>> and change the token when delay turns into repeat. But locks are worse than nesting.
+                let kbd = self.arc.clone();
+                let duration = Duration::from_millis(delay as _);
+                let handle = get_handle(data);
+                guard.key_repeat_timer = Some(handle.insert_source(
+                    calloop::timer::Timer::from_duration(duration),
+                    move |_, _, data| {
+                        time_ms += delay as u32;
+                        on_event(data, KeyEvent::Repeated, time_ms, keycode);
+                        let mut guard = kbd.internal.lock().unwrap();
+
+                        match guard.key_repeat_timer {
+                            Some(token) => data.clear_timeout(token),
+                            None => debug!("Key starts repeating but there is no delay timer. Was repeat already cancelled?"),
+                        };
+
+                        // This implementation doesn't take into account changes to the repeat rate after repeating begins.
+                        let handle = get_handle(data);
+                        let on_event = on_event.clone();
+                        let duration = Duration::from_millis(rate as _);
+                        guard.key_repeat_timer = Some(handle.insert_source(
+                            calloop::timer::Timer::from_duration(duration),
+                            move |_, _, data| {
+                                time_ms += rate as u32;
+                                on_event(data, KeyEvent::Repeated, time_ms, keycode);
+                                calloop::timer::TimeoutAction::ToDuration(duration)
+                            },
+                        ).unwrap());
+                        calloop::timer::TimeoutAction::Drop
+                    }
+                ).unwrap());
+            },
+            KeyState::Released => {
+                // Nothing to do; timer is released for both in the common path.
+            }
+        }
+    }
+    
+    /// Cancels any ongoing key repeat
+    pub fn key_stop_repeat(&self,
+        data: &mut D,
+        get_handle: impl Fn(&D) -> &LoopHandle<'static, D>,
+    ) {
+        let mut guard = self.arc.internal.lock().unwrap();
+        if let Some(token) = guard.key_repeat_timer.take() {
+            let handle = get_handle(data);
+            handle.remove(token);
+        };
+    }
+
     /// Handle a keystroke
     ///
     /// All keystrokes from the input backend should be fed _in order_ to this method of the
@@ -949,7 +1053,7 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
         &self,
         data: &mut D,
         keycode: Keycode,
-        state: KeyState,
+        state: KeyEvent,
         serial: Serial,
         time: u32,
         filter: F,
@@ -979,13 +1083,13 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
         &self,
         data: &mut D,
         keycode: Keycode,
-        state: KeyState,
+        state: KeyEvent,
         filter: F,
     ) -> (T, bool)
     where
         F: FnOnce(&mut D, &ModifiersState, KeysymHandle<'_>) -> T,
     {
-        trace!("Handling keystroke");
+        trace!("Handling key event");
 
         let mut guard = self.arc.internal.lock().unwrap();
         let (mods_changed, leds_changed) = guard.key_input(keycode, state);
@@ -1014,26 +1118,20 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
         &self,
         data: &mut D,
         keycode: Keycode,
-        state: KeyState,
+        state: KeyEvent,
         serial: Serial,
-        time: u32,
+        time_ms: u32,
         mods_changed: bool,
     ) {
         let mut guard = self.arc.internal.lock().unwrap();
-        match state {
-            KeyState::Pressed => {
-                guard.forwarded_pressed_keys.insert(keycode);
-            }
-            KeyState::Released => {
-                guard.forwarded_pressed_keys.remove(&keycode);
-            }
-        };
+        
+        let event = state.into();
 
         // forward to client if no keybinding is triggered
         let seat = self.get_seat(data);
         let modifiers = mods_changed.then_some(guard.mods_state);
         guard.with_grab(data, &seat, |data, handle, grab| {
-            grab.input(data, handle, keycode, state, modifiers, serial, time);
+            grab.input(data, handle, keycode, event, modifiers, serial, time_ms);
         });
         if guard.focus.is_some() {
             trace!("Input forwarded to client");
@@ -1151,6 +1249,11 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
                 continue;
             };
             if kbd.version() >= 4 {
+                let rate = if kbd.version() >= 10 {
+                    0 // Enables compositor-side key repeat. See wl_keyboard key event
+                } else {
+                    rate
+                };
                 kbd.repeat_info(rate, delay);
             }
         }
@@ -1266,7 +1369,7 @@ impl<D: SeatHandler + 'static> KeyboardInnerHandle<'_, D> {
         &mut self,
         data: &mut D,
         keycode: Keycode,
-        key_state: KeyState,
+        key_state: KeyEvent,
         modifiers: Option<ModifiersState>,
         serial: Serial,
         time: u32,
@@ -1377,7 +1480,7 @@ impl<D: SeatHandler + 'static> KeyboardGrab<D> for DefaultGrab {
         data: &mut D,
         handle: &mut KeyboardInnerHandle<'_, D>,
         keycode: Keycode,
-        state: KeyState,
+        state: KeyEvent,
         modifiers: Option<ModifiersState>,
         serial: Serial,
         time: u32,

src/input/keyboard/mod.rs

@@ -117,7 +117,7 @@
 //! These methods return handles that can be cloned and sent across thread, so you can keep one around
 //! in your event-handling code to forward inputs to your clients.
 //!
-
+use calloop::RegistrationToken;
 use std::{
     fmt,
     hash::Hash,
@@ -161,6 +161,19 @@ pub trait SeatHandler: Sized {
 
     /// Callback that will be notified whenever the keyboard led state changes.
     fn led_state_changed(&mut self, _seat: &Seat<Self>, _led_state: LedState) {}
+    
+    /// Register a timeout for key repeat.
+    fn set_timeout(
+        &mut self,
+        duration: std::time::Duration,
+        callback: impl FnMut(&mut Self) + 'static,
+    ) -> RegistrationToken;
+    
+    /// Cleare a nonrepeating timeout for key repeat.
+    fn clear_timeout(
+        &mut self,
+        token: RegistrationToken,
+    );
 }
 /// Delegate type for all [Seat] globals.
 ///