Skip to content

Commit

Permalink
On X11, avoid false positive key repeats
Browse files Browse the repository at this point in the history
Instead of a single `bool` indicating that a key press has occured and
no key has been released since then, we store the scancode of the last
pressed key (if it is a key that repeats when held). This fixes a bug
where pressing a new key while one is already held down will be flagged
as a repeat even though it is obviously not a repeat.
  • Loading branch information
Imberflur authored Jul 9, 2023
1 parent bca57ed commit bd890e6
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ And please only add new entries to the top of this list, right below the `# Unre

# Unreleased

- On X11, fix false positive flagging of key repeats when pressing different keys with no release
between presses.
- Implement `PartialOrd` and `Ord` for `KeyCode` and `NativeKeyCode`.

# 0.29.0-beta.0
Expand Down
1 change: 0 additions & 1 deletion src/platform_impl/linux/common/xkb_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,6 @@ impl KbdState {
self.post_init(state, keymap);
}

#[cfg(feature = "wayland")]
pub fn key_repeats(&mut self, keycode: ffi::xkb_keycode_t) -> bool {
unsafe { (XKBH.xkb_keymap_key_repeats)(self.xkb_keymap, keycode) == 1 }
}
Expand Down
44 changes: 38 additions & 6 deletions src/platform_impl/linux/x11/event_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ pub(super) struct EventProcessor<T: 'static> {
pub(super) kb_state: KbdState,
// Number of touch events currently in progress
pub(super) num_touch: u32,
// Whether we've got a key release for the key press.
pub(super) got_key_release: bool,
// This is the last pressed key that is repeatable (if it hasn't been
// released).
//
// Used to detect key repeats.
pub(super) held_key_press: Option<u32>,
pub(super) first_touch: Option<u64>,
// Currently focused window belonging to this process
pub(super) active_window: Option<ffi::Window>,
Expand Down Expand Up @@ -548,13 +551,39 @@ impl<T: 'static> EventProcessor<T> {
let device_id = mkdid(util::VIRTUAL_CORE_KEYBOARD);

let keycode = xkev.keycode as _;
let repeat = ty == ffi::KeyPress && !self.got_key_release;
// Update state after the repeat setting.

// Update state to track key repeats and determine whether this key was a repeat.
//
// Note, when a key is held before focusing on this window the first
// (non-synthetic) event will not be flagged as a repeat (also note that the
// synthetic press event that is generated before this when the window gains focus
// will also not be flagged as a repeat).
//
// Only keys that can repeat should change the held_key_press state since a
// continuously held repeatable key may continue repeating after the press of a
// non-repeatable key.
let repeat = if self.kb_state.key_repeats(keycode) {
let is_latest_held = self.held_key_press == Some(keycode);

if ty == ffi::KeyPress {
self.held_key_press = Some(keycode);
is_latest_held
} else {
// Check that the released key is the latest repeatable key that has been
// pressed, since repeats will continue for the latest key press if a
// different previously pressed key is released.
if is_latest_held {
self.held_key_press = None;
}
false
}
} else {
false
};

let state = if ty == ffi::KeyPress {
self.got_key_release = false;
ElementState::Pressed
} else {
self.got_key_release = true;
ElementState::Released
};

Expand Down Expand Up @@ -926,6 +955,9 @@ impl<T: 'static> EventProcessor<T> {
&mut self.kb_state,
&mut callback,
);
// Clear this so detecting key repeats is consistently handled when the
// window regains focus.
self.held_key_press = None;

callback(Event::WindowEvent {
window_id,
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/linux/x11/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,10 @@ impl<T: 'static> EventLoop<T> {
xkbext,
kb_state,
num_touch: 0,
held_key_press: None,
first_touch: None,
active_window: None,
is_composing: false,
got_key_release: true,
};

// Register for device hotplug events
Expand Down

0 comments on commit bd890e6

Please sign in to comment.