From 566e176b447be367365d4312eee9bee7ae8f1dea Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Mon, 16 Sep 2024 19:17:06 -0700 Subject: [PATCH] Use `Keycode` type consistently for representing keycode It seems good to use a dedicated type for keycodes, particularly given there's two conventions for keycodes, offset by 8. So there's ambiguity about what the numberic value of the keycode actually represents. We might prefer to use the Wayland value rather than the X11 version like `xkeysym::KeyCode` uses. But it would complicate things to have two different keycode types, and the code that calls `kbd.key` already is converting from `Keycode`. --- anvil/src/input_handler.rs | 4 +- examples/seat.rs | 2 +- src/backend/input/mod.rs | 6 +- src/backend/libinput/mod.rs | 4 +- src/backend/winit/input.rs | 10 +-- src/backend/x11/input.rs | 8 +-- src/backend/x11/mod.rs | 16 +---- src/desktop/wayland/popup/grab.rs | 4 +- src/input/keyboard/mod.rs | 68 +++++++++---------- .../input_method_keyboard_grab.rs | 9 ++- src/wayland/seat/keyboard.rs | 11 ++- src/wayland/xwayland_keyboard_grab.rs | 4 +- 12 files changed, 68 insertions(+), 78 deletions(-) diff --git a/anvil/src/input_handler.rs b/anvil/src/input_handler.rs index 02ce109aaf..150f998e8a 100644 --- a/anvil/src/input_handler.rs +++ b/anvil/src/input_handler.rs @@ -148,7 +148,7 @@ impl AnvilState { fn keyboard_key_to_action(&mut self, evt: B::KeyboardKeyEvent) -> KeyAction { let keycode = evt.key_code(); let state = evt.state(); - debug!(keycode, ?state, "key"); + debug!(?keycode, ?state, "key"); let serial = SCOUNTER.next_serial(); let time = Event::time_msec(&evt); let mut suppressed_keys = self.suppressed_keys.clone(); @@ -571,7 +571,7 @@ impl AnvilState { for keycode in keyboard.pressed_keys() { keyboard.input( self, - keycode.raw(), + keycode, KeyState::Released, SCOUNTER.next_serial(), 0, diff --git a/examples/seat.rs b/examples/seat.rs index ee9512e3ef..9bc89a68e8 100644 --- a/examples/seat.rs +++ b/examples/seat.rs @@ -61,7 +61,7 @@ fn main() -> Result<(), Box> { keyboard.input( &mut state, - 1, + smithay::backend::input::Keycode::from(9u32), smithay::backend::input::KeyState::Pressed, 0.into(), 0, diff --git a/src/backend/input/mod.rs b/src/backend/input/mod.rs index e629fda998..39e41957a3 100644 --- a/src/backend/input/mod.rs +++ b/src/backend/input/mod.rs @@ -2,6 +2,8 @@ use std::path::PathBuf; +pub use xkbcommon::xkb::Keycode; + mod tablet; pub use tablet::{ @@ -102,7 +104,7 @@ pub trait KeyboardKeyEvent: Event { /// `input-event-codes.h`. /// /// [input event codes]: https://gitlab.freedesktop.org/libinput/libinput/-/blob/main/include/linux/linux/input-event-codes.h - fn key_code(&self) -> u32; + fn key_code(&self) -> Keycode; /// State of the key fn state(&self) -> KeyState; @@ -112,7 +114,7 @@ pub trait KeyboardKeyEvent: Event { } impl KeyboardKeyEvent for UnusedEvent { - fn key_code(&self) -> u32 { + fn key_code(&self) -> Keycode { match *self {} } diff --git a/src/backend/libinput/mod.rs b/src/backend/libinput/mod.rs index 5236840714..d06f9d93ec 100644 --- a/src/backend/libinput/mod.rs +++ b/src/backend/libinput/mod.rs @@ -111,9 +111,9 @@ impl backend::Event for event::keyboard::KeyboardKeyEvent } impl backend::KeyboardKeyEvent for event::keyboard::KeyboardKeyEvent { - fn key_code(&self) -> u32 { + fn key_code(&self) -> backend::Keycode { use input::event::keyboard::KeyboardEventTrait; - self.key() + (self.key() + 8).into() } fn state(&self) -> backend::KeyState { diff --git a/src/backend/winit/input.rs b/src/backend/winit/input.rs index cebec3c538..b08da9067f 100644 --- a/src/backend/winit/input.rs +++ b/src/backend/winit/input.rs @@ -7,9 +7,9 @@ use winit::{ use crate::backend::input::{ self, AbsolutePositionEvent, Axis, AxisRelativeDirection, AxisSource, ButtonState, Device, - DeviceCapability, Event, InputBackend, KeyState, KeyboardKeyEvent, PointerAxisEvent, PointerButtonEvent, - PointerMotionAbsoluteEvent, TouchCancelEvent, TouchDownEvent, TouchEvent, TouchMotionEvent, TouchSlot, - TouchUpEvent, UnusedEvent, + DeviceCapability, Event, InputBackend, KeyState, KeyboardKeyEvent, Keycode, PointerAxisEvent, + PointerButtonEvent, PointerMotionAbsoluteEvent, TouchCancelEvent, TouchDownEvent, TouchEvent, + TouchMotionEvent, TouchSlot, TouchUpEvent, UnusedEvent, }; /// Marker used to define the `InputBackend` types for the winit backend. @@ -65,8 +65,8 @@ impl Event for WinitKeyboardInputEvent { } impl KeyboardKeyEvent for WinitKeyboardInputEvent { - fn key_code(&self) -> u32 { - self.key + fn key_code(&self) -> Keycode { + (self.key + 8).into() } fn state(&self) -> KeyState { diff --git a/src/backend/x11/input.rs b/src/backend/x11/input.rs index e670ef59b8..48b7beccc8 100644 --- a/src/backend/x11/input.rs +++ b/src/backend/x11/input.rs @@ -4,8 +4,8 @@ use super::{window_inner::WindowInner, Window, WindowTemporary}; use crate::{ backend::input::{ self, AbsolutePositionEvent, Axis, AxisRelativeDirection, AxisSource, ButtonState, Device, - DeviceCapability, InputBackend, KeyState, KeyboardKeyEvent, PointerAxisEvent, PointerButtonEvent, - PointerMotionAbsoluteEvent, UnusedEvent, + DeviceCapability, InputBackend, KeyState, KeyboardKeyEvent, Keycode, PointerAxisEvent, + PointerButtonEvent, PointerMotionAbsoluteEvent, UnusedEvent, }, utils::{Logical, Size}, }; @@ -48,7 +48,7 @@ impl Device for X11VirtualDevice { #[derive(Debug, Clone)] pub struct X11KeyboardInputEvent { pub(crate) time: u32, - pub(crate) key: u32, + pub(crate) key: Keycode, pub(crate) count: u32, pub(crate) state: KeyState, pub(crate) window: Weak, @@ -74,7 +74,7 @@ impl input::Event for X11KeyboardInputEvent { } impl KeyboardKeyEvent for X11KeyboardInputEvent { - fn key_code(&self) -> u32 { + fn key_code(&self) -> Keycode { self.key } diff --git a/src/backend/x11/mod.rs b/src/backend/x11/mod.rs index 2c3368c6b9..eb25c83adb 100644 --- a/src/backend/x11/mod.rs +++ b/src/backend/x11/mod.rs @@ -82,7 +82,7 @@ use crate::{ allocator::{Allocator, Swapchain}, drm::{node::path_to_type, CreateDrmNodeError, DrmNode, NodeType}, egl::{native::X11DefaultDisplay, EGLDevice, EGLDisplay, Error as EGLError}, - input::{Axis, ButtonState, InputEvent, KeyState}, + input::{Axis, ButtonState, InputEvent, KeyState, Keycode}, }, utils::{x11rb::X11Source, Logical, Size}, }; @@ -801,12 +801,7 @@ impl X11Inner { event: InputEvent::Keyboard { event: X11KeyboardInputEvent { time: key_press.time, - // X11's keycodes are +8 relative to the libinput keycodes - // that are expected, so subtract 8 from each keycode to - // match libinput. - // - // https://github.com/freedesktop/xorg-xf86-input-libinput/blob/master/src/xf86libinput.c#L54 - key: key_press.detail as u32 - 8, + key: Keycode::from(key_press.detail), count, state: KeyState::Pressed, window, @@ -837,12 +832,7 @@ impl X11Inner { event: InputEvent::Keyboard { event: X11KeyboardInputEvent { time: key_release.time, - // X11's keycodes are +8 relative to the libinput keycodes - // that are expected, so subtract 8 from each keycode to - // match libinput. - // - // https://github.com/freedesktop/xorg-xf86-input-libinput/blob/master/src/xf86libinput.c#L54 - key: key_release.detail as u32 - 8, + key: Keycode::from(key_release.detail), count, state: KeyState::Released, window, diff --git a/src/desktop/wayland/popup/grab.rs b/src/desktop/wayland/popup/grab.rs index 74035e1d71..0eb6210a23 100644 --- a/src/desktop/wayland/popup/grab.rs +++ b/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}, + backend::input::{ButtonState, KeyState, Keycode}, input::{ keyboard::{ GrabStartData as KeyboardGrabStartData, KeyboardGrab, KeyboardHandle, KeyboardInnerHandle, @@ -441,7 +441,7 @@ where &mut self, data: &mut D, handle: &mut KeyboardInnerHandle<'_, D>, - keycode: u32, + keycode: Keycode, state: KeyState, modifiers: Option, serial: Serial, diff --git a/src/input/keyboard/mod.rs b/src/input/keyboard/mod.rs index 9cad8d26b5..6cdc2df396 100644 --- a/src/input/keyboard/mod.rs +++ b/src/input/keyboard/mod.rs @@ -133,8 +133,8 @@ impl LedState { pub(crate) struct KbdInternal { pub(crate) focus: Option<(::KeyboardFocus, Serial)>, pending_focus: Option<::KeyboardFocus>, - pub(crate) pressed_keys: HashSet, - pub(crate) forwarded_pressed_keys: HashSet, + pub(crate) pressed_keys: HashSet, + pub(crate) forwarded_pressed_keys: HashSet, pub(crate) mods_state: ModifiersState, context: xkb::Context, pub(crate) keymap: xkb::Keymap, @@ -198,7 +198,7 @@ impl KbdInternal { } // returns whether the modifiers or led state has changed - fn key_input(&mut self, keycode: u32, state: KeyState) -> (bool, bool) { + fn key_input(&mut self, keycode: Keycode, state: KeyState) -> (bool, bool) { // track pressed keys as xkbcommon does not seem to expose it :( let direction = match state { KeyState::Pressed => { @@ -214,7 +214,7 @@ impl KbdInternal { // update state // Offset the keycode by 8, as the evdev XKB rules reflect X's // broken keycode system, which starts at 8. - let state_components = self.state.update_key((keycode + 8).into(), direction); + let state_components = self.state.update_key(keycode, direction); let modifiers_changed = state_components != 0; if modifiers_changed { self.mods_state.update_with(&self.state); @@ -560,7 +560,7 @@ pub trait KeyboardGrab { &mut self, data: &mut D, handle: &mut KeyboardInnerHandle<'_, D>, - keycode: u32, + keycode: Keycode, state: KeyState, modifiers: Option, serial: Serial, @@ -726,7 +726,7 @@ impl KeyboardHandle { let mut state = xkb::State::new(&keymap); for key in &internal.pressed_keys { - state.update_key((key + 8).into(), xkb::KeyDirection::Down); + state.update_key(*key, xkb::KeyDirection::Down); } let led_mapping = LedMapping::from_keymap(&keymap); @@ -891,7 +891,7 @@ impl KeyboardHandle { pub fn input( &self, data: &mut D, - keycode: u32, + keycode: Keycode, state: KeyState, serial: Serial, time: u32, @@ -918,7 +918,13 @@ impl KeyboardHandle { /// /// Prefer using [`KeyboardHandle::input`] if this decision can be done synchronously /// in the `filter` closure. - pub fn input_intercept(&self, data: &mut D, keycode: u32, state: KeyState, filter: F) -> (T, bool) + pub fn input_intercept( + &self, + data: &mut D, + keycode: Keycode, + state: KeyState, + filter: F, + ) -> (T, bool) where F: FnOnce(&mut D, &ModifiersState, KeysymHandle<'_>) -> T, { @@ -927,9 +933,7 @@ impl KeyboardHandle { let mut guard = self.arc.internal.lock().unwrap(); let (mods_changed, leds_changed) = guard.key_input(keycode, state); let key_handle = KeysymHandle { - // Offset the keycode by 8, as the evdev XKB rules reflect X's - // broken keycode system, which starts at 8. - keycode: (keycode + 8).into(), + keycode, state: &guard.state, keymap: &guard.keymap, }; @@ -953,7 +957,7 @@ impl KeyboardHandle { pub fn input_forward( &self, data: &mut D, - keycode: u32, + keycode: Keycode, state: KeyState, serial: Serial, time: u32, @@ -1001,7 +1005,7 @@ impl KeyboardHandle { /// Return the key codes of the currently pressed keys. pub fn pressed_keys(&self) -> HashSet { let guard = self.arc.internal.lock().unwrap(); - guard.pressed_keys.iter().map(|&code| code.into()).collect() + guard.pressed_keys.clone() } /// Iterate over the keysyms of the currently pressed keys. @@ -1015,8 +1019,8 @@ impl KeyboardHandle { let handles = guard .pressed_keys .iter() - .map(|code| KeysymHandle { - keycode: (code + 8).into(), + .map(|keycode| KeysymHandle { + keycode: *keycode, state: &guard.state, keymap: &guard.keymap, }) @@ -1150,9 +1154,9 @@ impl<'a, D: SeatHandler + 'static> KeyboardInnerHandle<'a, D> { } /// Convert a given keycode as a [`KeysymHandle`] modified by this keyboards state - pub fn keysym_handle(&self, keycode: u32) -> KeysymHandle<'_> { + pub fn keysym_handle(&self, keycode: Keycode) -> KeysymHandle<'_> { KeysymHandle { - keycode: (keycode + 8).into(), + keycode, state: &self.inner.state, keymap: &self.inner.keymap, } @@ -1167,7 +1171,7 @@ impl<'a, D: SeatHandler + 'static> KeyboardInnerHandle<'a, D> { pub fn input( &mut self, data: &mut D, - keycode: u32, + keycode: Keycode, key_state: KeyState, modifiers: Option, serial: Serial, @@ -1189,7 +1193,7 @@ impl<'a, D: SeatHandler + 'static> KeyboardInnerHandle<'a, D> { // key event must be sent before modifiers event for libxkbcommon // to process them correctly let key = KeysymHandle { - keycode: (keycode + 8).into(), + keycode, state: &self.inner.state, keymap: &self.inner.keymap, }; @@ -1239,14 +1243,10 @@ impl<'a, D: SeatHandler + 'static> KeyboardInnerHandle<'a, D> { .inner .forwarded_pressed_keys .iter() - .map(|keycode| { - KeysymHandle { - // Offset the keycode by 8, as the evdev XKB rules reflect X's - // broken keycode system, which starts at 8. - keycode: (keycode + 8).into(), - state: &self.inner.state, - keymap: &self.inner.keymap, - } + .map(|keycode| KeysymHandle { + keycode: *keycode, + state: &self.inner.state, + keymap: &self.inner.keymap, }) .collect(); @@ -1258,14 +1258,10 @@ impl<'a, D: SeatHandler + 'static> KeyboardInnerHandle<'a, D> { .inner .forwarded_pressed_keys .iter() - .map(|keycode| { - KeysymHandle { - // Offset the keycode by 8, as the evdev XKB rules reflect X's - // broken keycode system, which starts at 8. - keycode: (keycode + 8).into(), - state: &self.inner.state, - keymap: &self.inner.keymap, - } + .map(|keycode| KeysymHandle { + keycode: *keycode, + state: &self.inner.state, + keymap: &self.inner.keymap, }) .collect(); @@ -1289,7 +1285,7 @@ impl KeyboardGrab for DefaultGrab { &mut self, data: &mut D, handle: &mut KeyboardInnerHandle<'_, D>, - keycode: u32, + keycode: Keycode, state: KeyState, modifiers: Option, serial: Serial, diff --git a/src/wayland/input_method/input_method_keyboard_grab.rs b/src/wayland/input_method/input_method_keyboard_grab.rs index edb56dc1db..2bdedcf3ba 100644 --- a/src/wayland/input_method/input_method_keyboard_grab.rs +++ b/src/wayland/input_method/input_method_keyboard_grab.rs @@ -17,7 +17,10 @@ use crate::input::{ SeatHandler, }; use crate::wayland::text_input::TextInputHandle; -use crate::{backend::input::KeyState, utils::Serial}; +use crate::{ + backend::input::{KeyState, Keycode}, + utils::Serial, +}; use super::InputMethodManagerState; @@ -41,7 +44,7 @@ where &mut self, _data: &mut D, _handle: &mut KeyboardInnerHandle<'_, D>, - keycode: u32, + keycode: Keycode, key_state: KeyState, modifiers: Option, serial: Serial, @@ -52,7 +55,7 @@ where inner .text_input_handle .focused_text_input_serial_or_default(serial.0, |serial| { - keyboard.key(serial, time, keycode, key_state.into()); + keyboard.key(serial, time, keycode.raw() - 8, key_state.into()); if let Some(serialized) = modifiers.map(|m| m.serialized) { keyboard.modifiers( serial, diff --git a/src/wayland/seat/keyboard.rs b/src/wayland/seat/keyboard.rs index 3d76155917..fd2d5128f1 100644 --- a/src/wayland/seat/keyboard.rs +++ b/src/wayland/seat/keyboard.rs @@ -12,7 +12,7 @@ use wayland_server::{ use super::WaylandFocus; use crate::{ - backend::input::KeyState, + backend::input::{KeyState, Keycode}, input::{ keyboard::{KeyboardHandle, KeyboardTarget, KeysymHandle, ModifiersState}, Seat, SeatHandler, SeatState, @@ -66,7 +66,7 @@ where if let Some((focused, serial)) = guard.focus.as_ref() { if focused.same_client_as(&kbd.id()) { let serialized = guard.mods_state.serialized; - let keys = serialize_pressed_keys(guard.pressed_keys.iter().cloned().collect()); + let keys = serialize_pressed_keys(guard.pressed_keys.iter().copied()); kbd.enter((*serial).into(), &focused.wl_surface().unwrap(), keys); // Modifiers must be send after enter event. kbd.modifiers( @@ -152,9 +152,8 @@ pub(crate) fn for_each_focused_kbds( } } -fn serialize_pressed_keys(keys: Vec) -> Vec { - let serialized = unsafe { ::std::slice::from_raw_parts(keys.as_ptr() as *const u8, keys.len() * 4) }; - serialized.into() +fn serialize_pressed_keys(keys: impl Iterator) -> Vec { + keys.flat_map(|key| (key.raw() - 8).to_ne_bytes()).collect() } impl KeyboardTarget for WlSurface { @@ -164,7 +163,7 @@ impl KeyboardTarget for WlSurface { kbd.enter( serial.into(), self, - serialize_pressed_keys(keys.iter().map(|h| h.raw_code().raw() - 8).collect()), + serialize_pressed_keys(keys.iter().map(|h| h.raw_code())), ) }); diff --git a/src/wayland/xwayland_keyboard_grab.rs b/src/wayland/xwayland_keyboard_grab.rs index 0dcf850eff..86a7f53360 100644 --- a/src/wayland/xwayland_keyboard_grab.rs +++ b/src/wayland/xwayland_keyboard_grab.rs @@ -47,7 +47,7 @@ use wayland_server::{ }; use crate::{ - backend::input::KeyState, + backend::input::{KeyState, Keycode}, input::{ keyboard::{self, KeyboardGrab, KeyboardInnerHandle}, Seat, SeatHandler, @@ -92,7 +92,7 @@ impl KeyboardGrab for XWaylandKeybo &mut self, data: &mut D, handle: &mut KeyboardInnerHandle<'_, D>, - keycode: u32, + keycode: Keycode, state: KeyState, modifiers: Option, serial: Serial,