Skip to content

Commit

Permalink
Use Keycode type consistently for representing keycode
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
ids1024 committed Sep 17, 2024
1 parent 1259a69 commit d0d7bde
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 77 deletions.
4 changes: 2 additions & 2 deletions anvil/src/input_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl<BackendData: Backend> AnvilState<BackendData> {
fn keyboard_key_to_action<B: InputBackend>(&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();
Expand Down Expand Up @@ -571,7 +571,7 @@ impl<BackendData: Backend> AnvilState<BackendData> {
for keycode in keyboard.pressed_keys() {
keyboard.input(
self,
keycode.raw(),
keycode,
KeyState::Released,
SCOUNTER.next_serial(),
0,
Expand Down
6 changes: 4 additions & 2 deletions src/backend/input/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

use std::path::PathBuf;

pub use xkbcommon::xkb::Keycode;

mod tablet;

pub use tablet::{
Expand Down Expand Up @@ -102,7 +104,7 @@ pub trait KeyboardKeyEvent<B: InputBackend>: Event<B> {
/// `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;
Expand All @@ -112,7 +114,7 @@ pub trait KeyboardKeyEvent<B: InputBackend>: Event<B> {
}

impl<B: InputBackend> KeyboardKeyEvent<B> for UnusedEvent {
fn key_code(&self) -> u32 {
fn key_code(&self) -> Keycode {
match *self {}
}

Expand Down
4 changes: 2 additions & 2 deletions src/backend/libinput/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ impl backend::Event<LibinputInputBackend> for event::keyboard::KeyboardKeyEvent
}

impl backend::KeyboardKeyEvent<LibinputInputBackend> 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 {
Expand Down
10 changes: 5 additions & 5 deletions src/backend/winit/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -65,8 +65,8 @@ impl Event<WinitInput> for WinitKeyboardInputEvent {
}

impl KeyboardKeyEvent<WinitInput> for WinitKeyboardInputEvent {
fn key_code(&self) -> u32 {
self.key
fn key_code(&self) -> Keycode {
(self.key + 8).into()
}

fn state(&self) -> KeyState {
Expand Down
8 changes: 4 additions & 4 deletions src/backend/x11/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};
Expand Down Expand Up @@ -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<WindowInner>,
Expand All @@ -74,7 +74,7 @@ impl input::Event<X11Input> for X11KeyboardInputEvent {
}

impl KeyboardKeyEvent<X11Input> for X11KeyboardInputEvent {
fn key_code(&self) -> u32 {
fn key_code(&self) -> Keycode {
self.key
}

Expand Down
16 changes: 3 additions & 13 deletions src/backend/x11/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, Keycode, KeyState},
},
utils::{x11rb::X11Source, Logical, Size},
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/desktop/wayland/popup/grab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -441,7 +441,7 @@ where
&mut self,
data: &mut D,
handle: &mut KeyboardInnerHandle<'_, D>,
keycode: u32,
keycode: Keycode,
state: KeyState,
modifiers: Option<ModifiersState>,
serial: Serial,
Expand Down
68 changes: 32 additions & 36 deletions src/input/keyboard/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ impl LedState {
pub(crate) struct KbdInternal<D: SeatHandler> {
pub(crate) focus: Option<(<D as SeatHandler>::KeyboardFocus, Serial)>,
pending_focus: Option<<D as SeatHandler>::KeyboardFocus>,
pub(crate) pressed_keys: HashSet<u32>,
pub(crate) forwarded_pressed_keys: HashSet<u32>,
pub(crate) pressed_keys: HashSet<Keycode>,
pub(crate) forwarded_pressed_keys: HashSet<Keycode>,
pub(crate) mods_state: ModifiersState,
context: xkb::Context,
pub(crate) keymap: xkb::Keymap,
Expand Down Expand Up @@ -198,7 +198,7 @@ impl<D: SeatHandler + 'static> KbdInternal<D> {
}

// 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 => {
Expand All @@ -214,7 +214,7 @@ impl<D: SeatHandler + 'static> KbdInternal<D> {
// 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);
Expand Down Expand Up @@ -560,7 +560,7 @@ pub trait KeyboardGrab<D: SeatHandler> {
&mut self,
data: &mut D,
handle: &mut KeyboardInnerHandle<'_, D>,
keycode: u32,
keycode: Keycode,
state: KeyState,
modifiers: Option<ModifiersState>,
serial: Serial,
Expand Down Expand Up @@ -726,7 +726,7 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {

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);
Expand Down Expand Up @@ -891,7 +891,7 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
pub fn input<T, F>(
&self,
data: &mut D,
keycode: u32,
keycode: Keycode,
state: KeyState,
serial: Serial,
time: u32,
Expand All @@ -918,7 +918,13 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
///
/// Prefer using [`KeyboardHandle::input`] if this decision can be done synchronously
/// in the `filter` closure.
pub fn input_intercept<T, F>(&self, data: &mut D, keycode: u32, state: KeyState, filter: F) -> (T, bool)
pub fn input_intercept<T, F>(
&self,
data: &mut D,
keycode: Keycode,
state: KeyState,
filter: F,
) -> (T, bool)
where
F: FnOnce(&mut D, &ModifiersState, KeysymHandle<'_>) -> T,
{
Expand All @@ -927,9 +933,7 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
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,
};
Expand All @@ -953,7 +957,7 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
pub fn input_forward(
&self,
data: &mut D,
keycode: u32,
keycode: Keycode,
state: KeyState,
serial: Serial,
time: u32,
Expand Down Expand Up @@ -1001,7 +1005,7 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
/// Return the key codes of the currently pressed keys.
pub fn pressed_keys(&self) -> HashSet<Keycode> {
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.
Expand All @@ -1015,8 +1019,8 @@ impl<D: SeatHandler + 'static> KeyboardHandle<D> {
let handles = guard
.pressed_keys
.iter()
.map(|code| KeysymHandle {
keycode: (code + 8).into(),
.map(|keycode| KeysymHandle {
keycode: *keycode,
state: &guard.state,
keymap: &guard.keymap,
})
Expand Down Expand Up @@ -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,
}
Expand All @@ -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<ModifiersState>,
serial: Serial,
Expand All @@ -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,
};
Expand Down Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -1289,7 +1285,7 @@ impl<D: SeatHandler + 'static> KeyboardGrab<D> for DefaultGrab {
&mut self,
data: &mut D,
handle: &mut KeyboardInnerHandle<'_, D>,
keycode: u32,
keycode: Keycode,
state: KeyState,
modifiers: Option<ModifiersState>,
serial: Serial,
Expand Down
9 changes: 6 additions & 3 deletions src/wayland/input_method/input_method_keyboard_grab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -41,7 +44,7 @@ where
&mut self,
_data: &mut D,
_handle: &mut KeyboardInnerHandle<'_, D>,
keycode: u32,
keycode: Keycode,
key_state: KeyState,
modifiers: Option<ModifiersState>,
serial: Serial,
Expand All @@ -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,
Expand Down
Loading

0 comments on commit d0d7bde

Please sign in to comment.