From d5144cba810554f3a1ed2bd1fe3999cbc3aca81f Mon Sep 17 00:00:00 2001 From: jtroo Date: Fri, 14 Jun 2024 22:49:17 -0700 Subject: [PATCH] fix(chordsv2): activate eager tap-hold-*s (#1114) --- keyberon/src/chord.rs | 18 ++++++++ keyberon/src/layout.rs | 14 +++++++ parser/src/cfg/mod.rs | 6 +++ src/tests/sim_tests/chord_sim_tests.rs | 58 +++++++++++++++++++++++--- 4 files changed, 90 insertions(+), 6 deletions(-) diff --git a/keyberon/src/chord.rs b/keyberon/src/chord.rs index 362cac499..22ccbb106 100644 --- a/keyberon/src/chord.rs +++ b/keyberon/src/chord.rs @@ -202,8 +202,26 @@ impl<'a, T> ChordsV2<'a, T> { pub(crate) fn tick_chv2(&mut self, active_layer: u16) -> SmolQueue { let mut q = SmolQueue::new(); self.queue.iter_mut().for_each(Queued::tick_qd); + let prev_active_chord_len = self.active_chords.len(); self.active_chords.iter_mut().for_each(tick_ach); self.drain_inputs(&mut q, active_layer); + if self.active_chords.len() != prev_active_chord_len { + // A chord was activated. Forward a no-op press event to potentially trigger + // HoldOnOtherKeyPress or PermissiveHold. + // FLAW: this does not associate with the actual input keys and thus cannot correctly + // trigger the early tap for *-keys variants of kanata tap-hold. + q.push_back(Queued::new_press(0, 0)); + } + if self + .active_chords + .iter() + .any(|ach| matches!(ach.status, UnreadReleased | Released)) + { + // A chord was released. Forward a no-op release event to potentially trigger + // PermissiveHold. + // FLAW: see above + q.push_back(Queued::new_release(0, 0)); + } self.clear_released_chords(&mut q); self.ticks_to_ignore_chord = self.ticks_to_ignore_chord.saturating_sub(1); q diff --git a/keyberon/src/layout.rs b/keyberon/src/layout.rs index 820f4eeb3..2b39f3a51 100644 --- a/keyberon/src/layout.rs +++ b/keyberon/src/layout.rs @@ -1020,6 +1020,20 @@ impl From for Queued { } } impl Queued { + pub(crate) fn new_press(i: u8, j: u16) -> Self { + Self { + since: 0, + event: Event::Press(i, j), + } + } + + pub(crate) fn new_release(i: u8, j: u16) -> Self { + Self { + since: 0, + event: Event::Release(i, j), + } + } + pub(crate) fn tick_qd(&mut self) { self.since = self.since.saturating_add(1); } diff --git a/parser/src/cfg/mod.rs b/parser/src/cfg/mod.rs index 82b5d9165..74df84e9f 100755 --- a/parser/src/cfg/mod.rs +++ b/parser/src/cfg/mod.rs @@ -2410,6 +2410,8 @@ fn create_defsrc_layer() -> [KanataAction; KEYS_IN_ROW] { .map(|osc| Action::KeyCode(osc.into())) .unwrap_or(Action::NoOp); } + // Ensure 0-index is no-op. + layer[0] = KanataAction::NoOp; layer } @@ -3077,6 +3079,10 @@ fn parse_layers( } } } + + // Very last thing - ensure index 0 is always no-op. This shouldn't have any way to be + // physically activated. This enable other code to rely on there always being a no-op key. + layers_cfg[layer_level][0][0] = Action::NoOp; } Ok(layers_cfg) } diff --git a/src/tests/sim_tests/chord_sim_tests.rs b/src/tests/sim_tests/chord_sim_tests.rs index f9bb0794d..d4f7a1286 100644 --- a/src/tests/sim_tests/chord_sim_tests.rs +++ b/src/tests/sim_tests/chord_sim_tests.rs @@ -19,7 +19,7 @@ fn sim_chord_basic_repeated_last_release() { d:b t:50 d:a t:50 u:b t:50 u:a t:50 ", ); assert_eq!( - "t:50ms\nout:↓C\nt:101ms\nout:↑C\nt:99ms\nout:↓C\nt:101ms\nout:↑C", + "t:50ms\nout:↓C\nt:102ms\nout:↑C\nt:98ms\nout:↓C\nt:102ms\nout:↑C", result ); } @@ -63,7 +63,7 @@ fn sim_chord_basic_repeated_first_release() { d:z t:50 d:b t:50 u:z t:50 u:b t:50 ", ); assert_eq!( - "t:50ms\nout:↓D\nt:51ms\nout:↑D\nt:149ms\nout:↓D\nt:51ms\nout:↑D", + "t:50ms\nout:↓D\nt:52ms\nout:↑D\nt:148ms\nout:↓D\nt:52ms\nout:↑D", result ); } @@ -96,7 +96,7 @@ fn sim_chord_overlapping_release() { SIMPLE_OVERLAPPING_CHORD_CFG, "d:a d:b t:100 u:a d:z t:300 u:b t:300", ); - assert_eq!("t:100ms\nout:↓C\nt:251ms\nout:↓Z\nt:50ms\nout:↑C", result); + assert_eq!("t:100ms\nout:↓C\nt:251ms\nout:↓Z\nt:51ms\nout:↑C", result); } #[test] @@ -106,7 +106,7 @@ fn sim_presses_for_old_chord_repress_into_new_chord() { "d:a d:b t:50 u:a t:50 d:z t:50 u:b t:50 d:a d:b t:50 u:a t:50", ); assert_eq!( - "t:50ms\nout:↓C\nt:101ms\nout:↑C\nt:99ms\nout:↓D\nt:7ms\nout:↑D", + "t:50ms\nout:↓C\nt:102ms\nout:↑C\nt:98ms\nout:↓D\nt:10ms\nout:↑D", result ); } @@ -117,7 +117,7 @@ fn sim_chord_activate_largest_overlapping() { SIMPLE_OVERLAPPING_CHORD_CFG, "d:a t:50 d:b t:50 d:z t:50 d:y t:50 u:b t:50", ); - assert_eq!("t:150ms\nout:↓E\nt:51ms\nout:↑E", result); + assert_eq!("t:150ms\nout:↓E\nt:52ms\nout:↑E", result); } static SIMPLE_DISABLED_LAYER_CHORD_CFG: &str = "\ @@ -237,7 +237,7 @@ fn sim_chord_into_tap_hold() { d:a t:50 d:b t:148 u:a u:b t:1000", ); assert_eq!( - "t:199ms\nout:↓Y\nt:3ms\nout:↑Y\nt:200ms\nout:↓X\nt:8ms\nout:↑X", + "t:199ms\nout:↓Y\nt:5ms\nout:↑Y\nt:198ms\nout:↓X\nt:10ms\nout:↑X", result ); } @@ -275,3 +275,49 @@ fn sim_denies_transparent() { .map(|_| ()) .expect_err("trans in defchordsv2 should error"); } + +#[test] +fn sim_chord_eager_tapholdpress_activation() { + let result = simulate( + " + (defcfg concurrent-tap-hold yes) + (defsrc caps j k bspc) + (deflayer one (tap-hold-press 0 200 esc lctl) j k bspc) + (defvirtualkeys bspc bspc) + (defchordsv2-experimental + (j k) (multi + (on-press press-vkey bspc) + (on-release release-vkey bspc) + (fork XX bspc (nop9))) 75 first-release () + ) + ", + "d:caps t:10 d:j d:k t:100 r:bspc t:10 r:bspc t:10 u:j u:k t:100 u:caps t:1000", + ) + .to_ascii(); + assert_eq!( + "t:11ms dn:LCtrl t:2ms dn:BSpace t:97ms \ + dn:BSpace t:10ms dn:BSpace t:14ms up:BSpace t:96ms up:LCtrl", + result + ); +} + +#[test] +fn sim_chord_eager_tapholdrelease_activation() { + let result = simulate( + " + (defcfg concurrent-tap-hold yes) + (defsrc caps j k bspc) + (deflayer one (tap-hold-release 0 200 esc lctl) j k bspc) + (defvirtualkeys bspc bspc) + (defchordsv2-experimental + (j k) (multi (on-press press-vkey bspc) (on-release release-vkey bspc)) 75 first-release () + ) + ", + "d:caps t:10 d:j d:k t:10 u:j u:k t:100 u:caps t:1000", + ) + .to_ascii(); + assert_eq!( + "t:20ms dn:LCtrl t:2ms dn:BSpace t:5ms up:BSpace t:93ms up:LCtrl", + result + ); +}