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/state.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>);
 

src/backend/input/mod.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/desktop/wayland/popup/grab.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/input/keyboard/mod.rs

@@ -1,9 +1,12 @@
 //! Keyboard-related types for smithay's input abstraction
 
-use crate::backend::input::KeyState;
+use crate::backend::input::{KeyEvent, KeyState};
+use crate::input::WeakSeat;
 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,6 +281,7 @@ impl<D: SeatHandler + 'static> KbdInternal<D> {
             led_mapping,
             led_state,
             grab: GrabStatus::None,
+            key_repeat_timer: None,
         })
     }
 
@@ -602,10 +618,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.
@@ -1016,10 +1032,74 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
         keycode: Keycode,
         state: KeyState,
         serial: Serial,
-        time: u32,
+        time_ms: u32,
         mods_changed: bool,
     ) {
         let mut guard = self.arc.internal.lock().unwrap();
+        if let Some(token) = guard.key_repeat_timer.take() {
+            // 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?
+            data.clear_timeout(token);
+        };
+        match state {
+            KeyState::Pressed => {
+                let seat = self.get_seat(data);
+                let delay = guard.repeat_delay;
+                let rate = guard.repeat_rate;
+                let mut time_ms = time_ms;
+                
+                fn issue_repeat<D: SeatHandler + 'static>(data: &mut D, time_ms: u32, kbd: &Arc<KbdRc<D>>, mods_changed: bool, weak_seat: &WeakSeat<D>, keycode: Keycode, serial: Serial) {
+                    let mut guard = kbd.internal.lock().unwrap();
+                    let modifiers = mods_changed.then_some(guard.mods_state);
+                    let seat = weak_seat.upgrade().unwrap();
+                    guard.with_grab(data, &seat, |data, handle, grab| {
+                        grab.input(data, handle, keycode, KeyEvent::Repeated, modifiers, serial, time_ms);
+                    });
+                    if guard.focus.is_some() {
+                        trace!("Input forwarded to client");
+                    } else {
+                        trace!("No client currently focused");
+                    }
+                }
+
+                // 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 weak_seat = seat.downgrade();
+                guard.key_repeat_timer = Some(data.set_timeout(
+                    Duration::from_millis(delay as _),
+                    move |data| {
+                        time_ms += delay as u32;
+                        issue_repeat(data, time_ms, &kbd, mods_changed, &weak_seat, keycode, serial);
+                        let mut guard = kbd.internal.lock().unwrap();
+
+                        match guard.key_repeat_timer {
+                            // Can the repeat timer fire again if this handler is stalled before the cancel?
+                            Some(token) => data.clear_timeout(token),
+                            // This is a bug currently where all repeat cancelling is handled on key press/release. This might not be a bug in the future when other events can cancel key repeat.
+                            None => warn!("Key starts repeating but there is no delay timer. This might be a bug."),
+                        };
+
+                        // This implementation doesn't take into account changes to the repeat rate after repeating begins.
+                        let kbd = kbd.clone();
+                        let weak_seat = weak_seat.clone();
+                        guard.key_repeat_timer = Some(data.set_timeout(
+                            Duration::from_millis(rate as _),
+                            move |data| {
+                                time_ms += rate as u32;
+                                issue_repeat(data, time_ms, &kbd, mods_changed, &weak_seat, keycode, serial);
+                            },
+                        ));
+                    },
+                ));
+            },
+            KeyState::Released => {
+                // Nothing to do; timer is released for both in the common path.
+            }
+        }
+
         match state {
             KeyState::Pressed => {
                 guard.forwarded_pressed_keys.insert(keycode);
@@ -1028,12 +1108,14 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
                 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 +1233,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 +1353,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 +1464,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/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.
 ///

src/wayland/input_method/input_method_keyboard_grab.rs

@@ -18,7 +18,7 @@ use crate::input::{
 };
 use crate::wayland::text_input::TextInputHandle;
 use crate::{
-    backend::input::{KeyState, Keycode},
+    backend::input::{KeyEvent, Keycode},
     utils::Serial,
 };
 
@@ -45,7 +45,7 @@ where
         _data: &mut D,
         _handle: &mut KeyboardInnerHandle<'_, D>,
         keycode: Keycode,
-        key_state: KeyState,
+        key_state: KeyEvent,
         modifiers: Option<ModifiersState>,
         serial: Serial,
         time: u32,

src/wayland/seat/keyboard.rs

@@ -68,7 +68,12 @@ where
 
         let guard = self.arc.internal.lock().unwrap();
         if kbd.version() >= 4 {
-            kbd.repeat_info(guard.repeat_rate, guard.repeat_delay);
+            let rate = if kbd.version() >= 10 {
+                0 // Enables compositor-side key repeat. See wl_keyboard key event
+            } else {
+                guard.repeat_rate
+            };
+            kbd.repeat_info(rate, guard.repeat_delay);
         }
         if let Some((focused, serial)) = guard.focus.as_ref() {
             if focused.same_client_as(&kbd.id()) {
@@ -211,7 +216,7 @@ impl<D: SeatHandler + 'static> KeyboardTarget<D> for WlSurface {
         seat: &Seat<D>,
         _data: &mut D,
         key: KeysymHandle<'_>,
-        state: KeyState,
+        state: WlKeyState,
         serial: Serial,
         time: u32,
     ) {

src/wayland/seat/mod.rs

@@ -173,7 +173,7 @@ impl<D: SeatHandler + 'static> SeatState<D> {
     {
         let Seat { arc } = self.new_seat(name);
 
-        let global_id = display.create_global::<D, _, _>(9, SeatGlobalData { arc: arc.clone() });
+        let global_id = display.create_global::<D, _, _>(10, SeatGlobalData { arc: arc.clone() });
         arc.inner.lock().unwrap().global = Some(global_id);
 
         Seat { arc }

src/wayland/xwayland_keyboard_grab.rs

@@ -47,7 +47,7 @@ use wayland_server::{
 };
 
 use crate::{
-    backend::input::{KeyState, Keycode},
+    backend::input::{KeyEvent, Keycode},
     input::{
         keyboard::{self, KeyboardGrab, KeyboardInnerHandle},
         Seat, SeatHandler,
@@ -109,7 +109,7 @@ impl<D: XWaylandKeyboardGrabHandler + 'static> KeyboardGrab<D> for XWaylandKeybo
         data: &mut D,
         handle: &mut KeyboardInnerHandle<'_, D>,
         keycode: Keycode,
-        state: KeyState,
+        state: KeyEvent,
         modifiers: Option<keyboard::ModifiersState>,
         serial: Serial,
         time: u32,