Skip to content

Commit

Permalink
fix(linux): fire release event even if the modifiers have been rele…
Browse files Browse the repository at this point in the history
…ased

closes #39
  • Loading branch information
amrbashir committed Nov 24, 2023
1 parent 860a449 commit 782aeb1
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 37 deletions.
5 changes: 5 additions & 0 deletions .changes/linux-press-release-order.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"global-hotkey": "patch"
---

On Linux, fix hotkey `press/release` events order and sometimes missing `release` event when the modifiers have been already released before the key itself has been released.
6 changes: 3 additions & 3 deletions src/platform_impl/macos/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{collections::HashMap, ffi::c_void, sync::Mutex};
use std::{collections::BTreeMap, ffi::c_void, sync::Mutex};

use keyboard_types::{Code, Modifiers};

Expand All @@ -15,7 +15,7 @@ mod ffi;

pub struct GlobalHotKeyManager {
event_handler_ptr: EventHandlerRef,
hotkeys: Mutex<HashMap<u32, HotKeyWrapper>>,
hotkeys: Mutex<BTreeMap<u32, HotKeyWrapper>>,
}

unsafe impl Send for GlobalHotKeyManager {}
Expand Down Expand Up @@ -54,7 +54,7 @@ impl GlobalHotKeyManager {

Ok(Self {
event_handler_ptr: ptr,
hotkeys: Mutex::new(HashMap::new()),
hotkeys: Mutex::new(BTreeMap::new()),
})
}

Expand Down
75 changes: 41 additions & 34 deletions src/platform_impl/x11/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-License-Identifier: MIT

use std::{
collections::{hash_map::Entry, HashMap},
ptr,
};
use std::{collections::BTreeMap, ptr};

use crossbeam_channel::{unbounded, Receiver, Sender};
use keyboard_types::{Code, Modifiers};
Expand Down Expand Up @@ -109,7 +106,7 @@ fn register_hotkey(
xlib: &Xlib,
display: *mut _XDisplay,
root: u64,
hotkeys: &mut HashMap<(u32, u32), (u32, bool)>,
hotkeys: &mut BTreeMap<u32, Vec<(u32, u32, bool)>>,
hotkey: HotKey,
) -> crate::Result<()> {
let (modifiers, key) = (
Expand Down Expand Up @@ -142,27 +139,28 @@ fn register_hotkey(
}
}

if let Entry::Vacant(e) = hotkeys.entry((modifiers, keycode as _)) {
e.insert((hotkey.id(), false));
} else {
return Err(crate::Error::AlreadyRegistered(hotkey));
let entry = hotkeys.entry(keycode as _).or_default();
match entry.iter().find(|e| e.1 == modifiers) {
None => {
entry.push((hotkey.id(), modifiers, false));
Ok(())
}
Some(_) => Err(crate::Error::AlreadyRegistered(hotkey)),
}
} else {
return Err(crate::Error::FailedToRegister(format!(
Err(crate::Error::FailedToRegister(format!(
"Unable to register accelerator (unknown scancode for this key: {}).",
hotkey.key
)));
)))
}

Ok(())
}

#[inline]
fn unregister_hotkey(
xlib: &Xlib,
display: *mut _XDisplay,
root: u64,
hotkeys: &mut HashMap<(u32, u32), (u32, bool)>,
hotkeys: &mut BTreeMap<u32, Vec<(u32, u32, bool)>>,
hotkey: HotKey,
) -> crate::Result<()> {
let (modifiers, key) = (
Expand All @@ -177,16 +175,17 @@ fn unregister_hotkey(
unsafe { (xlib.XUngrabKey)(display, keycode as _, modifiers | m, root) };
}

hotkeys.remove(&(modifiers, keycode as _));
let entry = hotkeys.entry(keycode as _).or_default();
entry.retain(|k| k.1 != modifiers);
Ok(())
} else {
Err(crate::Error::FailedToUnRegister(hotkey))
}
}

fn events_processor(thread_rx: Receiver<ThreadMessage>) {
// mods, key id, repeating
let mut hotkeys = HashMap::<(u32, u32), (u32, bool)>::new();
// key id, mods, pressed
let mut hotkeys = BTreeMap::<u32, Vec<(u32, u32, bool)>>::new();
if let Ok(xlib) = xlib::Xlib::open() {
unsafe {
let display = (xlib.XOpenDisplay)(ptr::null());
Expand All @@ -203,30 +202,38 @@ fn events_processor(thread_rx: Receiver<ThreadMessage>) {
if (xlib.XPending)(display) > 0 {
(xlib.XNextEvent)(display, &mut event);
match event.get_type() {
e if matches!(e, xlib::KeyPress | xlib::KeyRelease) => {
e @ xlib::KeyPress | e @ xlib::KeyRelease => {
let keycode = event.key.keycode;
// X11 sends masks for Lock keys also and we only care about the 4 below
let modifiers = event.key.state
let event_mods = event.key.state
& (xlib::ControlMask
| xlib::ShiftMask
| xlib::Mod4Mask
| xlib::Mod1Mask);

if let Some((id, repeating)) = hotkeys.get_mut(&(modifiers, keycode)) {
match (e, *repeating) {
(xlib::KeyPress, false) => {
GlobalHotKeyEvent::send(GlobalHotKeyEvent {
id: *id,
state: crate::HotKeyState::Pressed,
});
*repeating = true;
if let Some(entry) = hotkeys.get_mut(&keycode) {
match e {
xlib::KeyPress => {
for (id, mods, pressed) in entry {
if event_mods == *mods && !*pressed {
GlobalHotKeyEvent::send(GlobalHotKeyEvent {
id: *id,
state: crate::HotKeyState::Pressed,
});
*pressed = true;
}
}
}
(xlib::KeyRelease, true) => {
GlobalHotKeyEvent::send(GlobalHotKeyEvent {
id: *id,
state: crate::HotKeyState::Released,
});
*repeating = false;
xlib::KeyRelease => {
for (id, _, pressed) in entry {
if *pressed {
GlobalHotKeyEvent::send(GlobalHotKeyEvent {
id: *id,
state: crate::HotKeyState::Released,
});
*pressed = false;
}
}
}
_ => {}
}
Expand Down Expand Up @@ -258,7 +265,7 @@ fn events_processor(thread_rx: Receiver<ThreadMessage>) {
let _ = tx.send(Ok(()));
}
ThreadMessage::UnRegisterHotKey(hotkey, tx) => {
let _ = tx.send(register_hotkey(
let _ = tx.send(unregister_hotkey(
&xlib,
display,
root,
Expand Down

0 comments on commit 782aeb1

Please sign in to comment.