From 510d4a55843a8f5a22d88992cd7132baa23df1d1 Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Sat, 4 Jul 2020 11:25:43 -0700 Subject: [PATCH 1/4] Fix gtk key repeat logic Keep track of which keys are pressed and use that to determine whether a key is a repeat. The previous logic of comparing the keyval to the last value fails in the case of multiple keys. Also add lock modifiers, a followup from #1079. Fixes #1080 --- CHANGELOG.md | 2 ++ druid-shell/src/platform/gtk/window.rs | 16 +++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e769df73d2..44b0db2c56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ You can find its changes [documented below](#060---2020-06-01). - `Scale::from_scale` to `Scale::new`, and `Scale` methods `scale_x` / `scale_y` to `x` / `y`. ([#1042] by [@xStrom]) - Major rework of keyboard event handling ([#1049] by [@raphlinus]) - `Container::rounded` takes `KeyOrValue` instead of `f64`. ([#1054] by [@binomial0]) +- Fix Gtk key repeat logic ([#1081] by [@raphlinus]) ### Deprecated @@ -348,6 +349,7 @@ Last release without a changelog :( [#1054]: https://github.com/linebender/druid/pull/1054 [#1058]: https://github.com/linebender/druid/pull/1058 [#1062]: https://github.com/linebender/druid/pull/1062 +[#1081]: https://github.com/linebender/druid/pull/1081 [Unreleased]: https://github.com/linebender/druid/compare/v0.6.0...master [0.6.0]: https://github.com/linebender/druid/compare/v0.5.0...v0.6.0 diff --git a/druid-shell/src/platform/gtk/window.rs b/druid-shell/src/platform/gtk/window.rs index cd6ffd5e4a..62d92e3daf 100644 --- a/druid-shell/src/platform/gtk/window.rs +++ b/druid-shell/src/platform/gtk/window.rs @@ -16,6 +16,7 @@ use std::any::Any; use std::cell::{Cell, RefCell}; +use std::collections::HashSet; use std::convert::TryFrom; use std::ffi::c_void; use std::ffi::OsString; @@ -119,7 +120,7 @@ pub(crate) struct WindowState { drawing_area: DrawingArea, pub(crate) handler: RefCell>, idle_queue: Arc>>, - current_keyval: RefCell>, + pressed_keys: RefCell>, } impl WindowBuilder { @@ -201,7 +202,7 @@ impl WindowBuilder { drawing_area, handler: RefCell::new(handler), idle_queue: Arc::new(Mutex::new(vec![])), - current_keyval: RefCell::new(None), + pressed_keys: Default::default(), }); self.app @@ -475,10 +476,7 @@ impl WindowBuilder { win_state.drawing_area.connect_key_press_event(clone!(handle => move |_widget, key| { if let Some(state) = handle.state.upgrade() { - let mut current_keyval = state.current_keyval.borrow_mut(); - let repeat = *current_keyval == Some(key.get_keyval()); - - *current_keyval = Some(key.get_keyval()); + let repeat = !state.pressed_keys.borrow_mut().insert(key.get_hardware_keycode()); if let Ok(mut handler) = state.handler.try_borrow_mut() { handler.key_down(make_key_event(key, repeat, KeyState::Down)); @@ -493,7 +491,7 @@ impl WindowBuilder { win_state.drawing_area.connect_key_release_event(clone!(handle => move |_widget, key| { if let Some(state) = handle.state.upgrade() { - *(state.current_keyval.borrow_mut()) = None; + state.pressed_keys.borrow_mut().remove(&key.get_hardware_keycode()); if let Ok(mut handler) = state.handler.try_borrow_mut() { handler.key_up(make_key_event(key, false, KeyState::Up)); @@ -839,6 +837,10 @@ const MODIFIER_MAP: &[(gdk::ModifierType, Modifiers)] = &[ (ModifierType::MOD1_MASK, Modifiers::ALT), (ModifierType::CONTROL_MASK, Modifiers::CONTROL), (ModifierType::META_MASK, Modifiers::META), + (ModifierType::LOCK_MASK, Modifiers::CAPS_LOCK), + // Note: this is the usual value on X11, not sure how consistent it is. + // Possibly we should use `Keymap::get_num_lock_state()` instead. + (ModifierType::MOD2_MASK, Modifiers::NUM_LOCK), ]; fn get_modifiers(modifiers: gdk::ModifierType) -> Modifiers { From d9af8de53a1fca941b9e0f10c2dddefc85ebd9ac Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Sat, 4 Jul 2020 12:22:17 -0700 Subject: [PATCH 2/4] Adapt mozilla logic for detecting repeats Use single optional keycode rather than hashset as it's more robust against a key getting "stuck" due to focus or whatnot. --- druid-shell/src/platform/gtk/window.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/druid-shell/src/platform/gtk/window.rs b/druid-shell/src/platform/gtk/window.rs index 62d92e3daf..9e822d124d 100644 --- a/druid-shell/src/platform/gtk/window.rs +++ b/druid-shell/src/platform/gtk/window.rs @@ -16,7 +16,6 @@ use std::any::Any; use std::cell::{Cell, RefCell}; -use std::collections::HashSet; use std::convert::TryFrom; use std::ffi::c_void; use std::ffi::OsString; @@ -120,7 +119,7 @@ pub(crate) struct WindowState { drawing_area: DrawingArea, pub(crate) handler: RefCell>, idle_queue: Arc>>, - pressed_keys: RefCell>, + current_keycode: RefCell>, } impl WindowBuilder { @@ -202,7 +201,7 @@ impl WindowBuilder { drawing_area, handler: RefCell::new(handler), idle_queue: Arc::new(Mutex::new(vec![])), - pressed_keys: Default::default(), + current_keycode: RefCell::new(None), }); self.app @@ -476,7 +475,11 @@ impl WindowBuilder { win_state.drawing_area.connect_key_press_event(clone!(handle => move |_widget, key| { if let Some(state) = handle.state.upgrade() { - let repeat = !state.pressed_keys.borrow_mut().insert(key.get_hardware_keycode()); + let mut current_keycode = state.current_keycode.borrow_mut(); + let hw_keycode = key.get_hardware_keycode(); + let repeat = *current_keycode == Some(hw_keycode); + + *current_keycode = Some(hw_keycode); if let Ok(mut handler) = state.handler.try_borrow_mut() { handler.key_down(make_key_event(key, repeat, KeyState::Down)); @@ -491,7 +494,10 @@ impl WindowBuilder { win_state.drawing_area.connect_key_release_event(clone!(handle => move |_widget, key| { if let Some(state) = handle.state.upgrade() { - state.pressed_keys.borrow_mut().remove(&key.get_hardware_keycode()); + let mut current_keycode = state.current_keycode.borrow_mut(); + if *current_keycode == Some(key.get_hardware_keycode()) { + *current_keycode = None; + } if let Ok(mut handler) = state.handler.try_borrow_mut() { handler.key_up(make_key_event(key, false, KeyState::Up)); From 8b886ac110c22d79d7347407eece0354a0466d35 Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Sun, 5 Jul 2020 07:33:27 -0700 Subject: [PATCH 3/4] Move changelog entry to "Fixed" section --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44b0db2c56..e8307e2d22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,6 @@ You can find its changes [documented below](#060---2020-06-01). - `Scale::from_scale` to `Scale::new`, and `Scale` methods `scale_x` / `scale_y` to `x` / `y`. ([#1042] by [@xStrom]) - Major rework of keyboard event handling ([#1049] by [@raphlinus]) - `Container::rounded` takes `KeyOrValue` instead of `f64`. ([#1054] by [@binomial0]) -- Fix Gtk key repeat logic ([#1081] by [@raphlinus]) ### Deprecated @@ -30,6 +29,7 @@ You can find its changes [documented below](#060---2020-06-01). - GTK: Don't crash when receiving an external command while a file dialog is visible. ([#1043] by [@jneem]) - Fix derive `Data` when type param bounds are defined ([#1058] by [@chris-zen]) - Ensure that `update` is called after all commands. ([#1062] by [@jneem]) +- Fix Gtk key repeat logic ([#1081] by [@raphlinus]) ### Visual From 2a45eff76320bda064a7bb8b0dad1fc4f1e1da93 Mon Sep 17 00:00:00 2001 From: Raph Levien Date: Sun, 5 Jul 2020 08:06:14 -0700 Subject: [PATCH 4/4] Apply Finnerale's suggestion for changelog wording --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8307e2d22..4cf94f3fdf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,7 @@ You can find its changes [documented below](#060---2020-06-01). - GTK: Don't crash when receiving an external command while a file dialog is visible. ([#1043] by [@jneem]) - Fix derive `Data` when type param bounds are defined ([#1058] by [@chris-zen]) - Ensure that `update` is called after all commands. ([#1062] by [@jneem]) -- Fix Gtk key repeat logic ([#1081] by [@raphlinus]) +- GTK: Don't interrupt `KeyEvent.repeat` when releasing another key. ([#1081] by [@raphlinus]) ### Visual