From 88bd95db8e60deb7f9663fb40669d3ae7b8ee130 Mon Sep 17 00:00:00 2001 From: Jan Tache Date: Sun, 24 Jul 2022 00:54:43 -0700 Subject: [PATCH] feat(altgr): add altgr bug mitigation cfgs for Windows This commit adds an optional configuration entry when running on Windows: `windows-altgr`. This configuration item can either make kanata cancel lctl presses that occur alongside ralt presses or make kanata send a lctl release when ralt is released. --- cfg_samples/kanata.kbd | 13 +++ src/kanata.rs | 178 ++++++++++++++++++++++++++++++++++++++++- src/keys/windows.rs | 2 +- src/main.rs | 11 ++- src/oskbd/windows.rs | 9 ++- 5 files changed, 202 insertions(+), 11 deletions(-) diff --git a/cfg_samples/kanata.kbd b/cfg_samples/kanata.kbd index afa91e9e2..6966ad465 100644 --- a/cfg_samples/kanata.kbd +++ b/cfg_samples/kanata.kbd @@ -23,6 +23,19 @@ ;; Windows doesn't need any input/output configuration entries; however, there ;; must still be a defcfg entry. + ;; + ;; There is an optional configuration entry for Windows to help mitigate strange + ;; behaviour of AltGr if your layout uses that. Uncomment one of the items below + ;; to change what kanata does with the key. + ;; + ;; For more context, see: https://github.com/jtroo/kanata/issues/55. + ;; + ;; windows-altgr cancel-lctl-press ;; remove the lctl press that comes as a combo with ralt + ;; windows-altgr add-lctl-release ;; add an lctl release when ralt is released + ;; + ;; NOTE: even with these workarounds, putting lctl+ralt in your defsrc may + ;; not work too well with other applications that use WH_KEYBOARD_LL. + ;; Known applications with issues: GWSL/VcXsrv ;; Optional confguration: enable kanata to execute commands. ;; It is also not enabled in this sample configuration. diff --git a/src/kanata.rs b/src/kanata.rs index 9457ad1b2..a2a68cc3c 100644 --- a/src/kanata.rs +++ b/src/kanata.rs @@ -24,6 +24,14 @@ use crate::{cfg, ValidatedArgs}; use kanata_keyberon::key_code::*; use kanata_keyberon::layout::*; +#[cfg(target_os = "windows")] +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum AltGrBehaviour { + DoNothing, + CancelLctlPress, + AddLctlRelease, +} + pub struct Kanata { pub kbd_in_path: PathBuf, pub kbd_out: KbdOut, @@ -44,6 +52,10 @@ static MAPPED_KEYS: Lazy> = Lazy::new(|| Mutex::new([fals #[cfg(target_os = "windows")] static PRESSED_KEYS: Lazy>> = Lazy::new(|| Mutex::new(HashSet::new())); +#[cfg(target_os = "windows")] +static ALTGR_BEHAVIOUR: Lazy> = + Lazy::new(|| Mutex::new(AltGrBehaviour::DoNothing)); + impl Kanata { /// Create a new configuration from a file. pub fn new(args: &ValidatedArgs) -> Result { @@ -74,6 +86,8 @@ impl Kanata { } } + set_altgr_behaviour(&cfg)?; + Ok(Self { kbd_in_path, kbd_out, @@ -229,6 +243,10 @@ impl Kanata { log::error!("Could not reload configuration:\n{}", e); } Ok(cfg) => { + if let Err(e) = set_altgr_behaviour(&cfg) { + log::error!("{}", e); + return; + } self.layout = cfg.layout; let mut mapped_keys = MAPPED_KEYS.lock(); *mapped_keys = cfg.mapped_keys; @@ -411,6 +429,7 @@ impl Kanata { loop { let in_event = kbd_in.read()?; + log::trace!("{in_event:?}"); // Pass-through non-key events let key_event = match KeyEvent::try_from(in_event) { @@ -455,6 +474,9 @@ impl Kanata { *mapped_keys = kanata.lock().mapped_keys; } + let (preprocess_tx, preprocess_rx) = crossbeam_channel::bounded(10); + start_event_preprocessor(preprocess_rx, tx); + // This callback should return `false` if the input event is **not** handled by the // callback and `true` if the input event **is** handled by the callback. Returning false // informs the callback caller that the input event should be handed back to the OS for @@ -495,9 +517,7 @@ impl Kanata { // getting full, assuming regular operation of the program and some other bug isn't the // problem. I've tried to crash the program by pressing as many keys on my keyboard at // the same time as I could, but was unable to. - if let Err(e) = tx.try_send(key_event) { - panic!("failed to send on channel: {:?}", e) - } + try_send_panic(&preprocess_tx, key_event); true }); @@ -507,6 +527,134 @@ impl Kanata { } } +#[cfg(target_os = "windows")] +fn try_send_panic(tx: &Sender, kev: KeyEvent) { + if let Err(e) = tx.try_send(kev) { + panic!("failed to send on channel: {:?}", e) + } +} + +#[cfg(target_os = "windows")] +fn start_event_preprocessor(preprocess_rx: Receiver, process_tx: Sender) { + #[derive(Debug, Clone, Copy, PartialEq)] + enum LctlState { + Pressed, + Released, + Pending, + PendingReleased, + None, + } + + std::thread::spawn(move || { + let mut lctl_state = LctlState::None; + loop { + match preprocess_rx.try_recv() { + Ok(kev) => match (*ALTGR_BEHAVIOUR.lock(), kev) { + (AltGrBehaviour::DoNothing, _) => try_send_panic(&process_tx, kev), + ( + AltGrBehaviour::AddLctlRelease, + KeyEvent { + value: KeyValue::Release, + code: OsCode::KEY_RIGHTALT, + .. + }, + ) => { + log::debug!("altgr add: adding lctl release"); + try_send_panic(&process_tx, kev); + try_send_panic( + &process_tx, + KeyEvent::new(OsCode::KEY_LEFTCTRL, KeyValue::Release), + ); + PRESSED_KEYS.lock().remove(&OsCode::KEY_LEFTCTRL); + } + ( + AltGrBehaviour::CancelLctlPress, + KeyEvent { + value: KeyValue::Press, + code: OsCode::KEY_LEFTCTRL, + .. + }, + ) => { + log::debug!("altgr cancel: lctl state->pressed"); + lctl_state = LctlState::Pressed; + } + ( + AltGrBehaviour::CancelLctlPress, + KeyEvent { + value: KeyValue::Release, + code: OsCode::KEY_LEFTCTRL, + .. + }, + ) => match lctl_state { + LctlState::Pressed => { + log::debug!("altgr cancel: lctl state->released"); + lctl_state = LctlState::Released; + } + LctlState::Pending => { + log::debug!("altgr cancel: lctl state->pending-released"); + lctl_state = LctlState::PendingReleased; + } + LctlState::None => try_send_panic(&process_tx, kev), + _ => {} + }, + ( + AltGrBehaviour::CancelLctlPress, + KeyEvent { + value: KeyValue::Press, + code: OsCode::KEY_RIGHTALT, + .. + }, + ) => { + log::debug!("altgr cancel: lctl state->none"); + lctl_state = LctlState::None; + try_send_panic(&process_tx, kev); + } + (_, _) => try_send_panic(&process_tx, kev), + }, + Err(TryRecvError::Empty) => { + if *ALTGR_BEHAVIOUR.lock() == AltGrBehaviour::CancelLctlPress { + match lctl_state { + LctlState::Pressed => { + log::debug!("altgr cancel: lctl state->pending"); + lctl_state = LctlState::Pending; + } + LctlState::Released => { + log::debug!("altgr cancel: lctl state->pending-released"); + lctl_state = LctlState::PendingReleased; + } + LctlState::Pending => { + log::debug!("altgr cancel: lctl state->send"); + try_send_panic( + &process_tx, + KeyEvent::new(OsCode::KEY_LEFTCTRL, KeyValue::Press), + ); + lctl_state = LctlState::None; + } + LctlState::PendingReleased => { + log::debug!("altgr cancel: lctl state->send+release"); + try_send_panic( + &process_tx, + KeyEvent::new(OsCode::KEY_LEFTCTRL, KeyValue::Press), + ); + try_send_panic( + &process_tx, + KeyEvent::new(OsCode::KEY_LEFTCTRL, KeyValue::Release), + ); + lctl_state = LctlState::None; + } + _ => {} + } + } + std::thread::sleep(time::Duration::from_millis(1)); + } + Err(TryRecvError::Disconnected) => { + panic!("channel disconnected") + } + } + } + }); +} + #[cfg(feature = "cmd")] fn run_cmd(cmd_and_args: &'static [String]) -> std::thread::JoinHandle<()> { std::thread::spawn(move || { @@ -540,6 +688,30 @@ fn run_cmd(cmd_and_args: &'static [String]) -> std::thread::JoinHandle<()> { }) } +fn set_altgr_behaviour(_cfg: &cfg::Cfg) -> Result<()> { + #[cfg(target_os = "windows")] + { + *ALTGR_BEHAVIOUR.lock() = { + const CANCEL: &str = "cancel-lctl-press"; + const ADD: &str = "add-lctl-release"; + match _cfg.items.get("windows-altgr") { + None => AltGrBehaviour::DoNothing, + Some(cfg_val) => match cfg_val.as_str() { + CANCEL => AltGrBehaviour::CancelLctlPress, + ADD => AltGrBehaviour::AddLctlRelease, + _ => bail!( + "Invalid value for windows-altgr: {}. Valid values are {},{}", + cfg_val, + CANCEL, + ADD + ), + }, + } + }; + } + Ok(()) +} + #[cfg(feature = "cmd")] fn run_multi_cmd(cmds: &'static [&'static [String]]) { let cmds = <&[&[String]]>::clone(&cmds); diff --git a/src/keys/windows.rs b/src/keys/windows.rs index a6f378c34..17067a5a3 100644 --- a/src/keys/windows.rs +++ b/src/keys/windows.rs @@ -1426,7 +1426,7 @@ impl From for bool { } } -#[derive(Debug)] +#[derive(Debug, Clone, Copy)] pub struct KeyEvent { pub code: OsCode, pub value: KeyValue, diff --git a/src/main.rs b/src/main.rs index 6fc26768a..010296f11 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,6 +36,10 @@ struct Args { /// Enable debug logging #[clap(short, long)] debug: bool, + + /// Enable trace logging + #[clap(short, long)] + trace: bool, } /// Parse CLI arguments and initialize logging. @@ -44,9 +48,10 @@ fn cli_init() -> Result { let cfg_path = Path::new(&args.cfg); - let log_lvl = match args.debug { - true => LevelFilter::Debug, - _ => LevelFilter::Info, + let log_lvl = match (args.debug, args.trace) { + (_, true) => LevelFilter::Trace, + (true, false) => LevelFilter::Debug, + (false, false) => LevelFilter::Info, }; CombinedLogger::init(vec![TermLogger::new( diff --git a/src/oskbd/windows.rs b/src/oskbd/windows.rs index 2f18c99b4..653f9e97c 100644 --- a/src/oskbd/windows.rs +++ b/src/oskbd/windows.rs @@ -105,18 +105,19 @@ impl InputEvent { /// The actual WinAPI compatible callback. unsafe extern "system" fn hook_proc(code: c_int, wparam: WPARAM, lparam: LPARAM) -> LRESULT { + let hook_lparam = &*(lparam as *const KBDLLHOOKSTRUCT); + let is_injected = hook_lparam.flags & LLKHF_INJECTED != 0; + log::trace!("{code}, {wparam:?}, {is_injected}"); if code != HC_ACTION { return CallNextHookEx(ptr::null_mut(), code, wparam, lparam); } - let hook_lparam = &*(lparam as *const KBDLLHOOKSTRUCT); let key_event = InputEvent::from_hook_lparam(hook_lparam); - let injected = hook_lparam.flags & LLKHF_INJECTED != 0; // `SendInput()` internally calls the hook function. Filter out injected events // to prevent recursion and potential stack overflows if our remapping logic // sent the injected event. - if injected { + if is_injected { return CallNextHookEx(ptr::null_mut(), code, wparam, lparam); } @@ -131,7 +132,7 @@ unsafe extern "system" fn hook_proc(code: c_int, wparam: WPARAM, lparam: LPARAM) }); if handled { - -1 + 1 } else { CallNextHookEx(ptr::null_mut(), code, wparam, lparam) }