From 1970cdec1c67818b27c281c5066a8e9f0853c86c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 4 Jan 2024 22:33:16 +0100 Subject: [PATCH 1/5] Add trace logging of pressed physical and logical keys --- crates/egui-winit/src/lib.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 607afdbac2b..330adf3fc03 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -689,6 +689,15 @@ impl State { let logical_key = key_from_winit_key(logical_key); + // Helpful logging to enable when adding new key support + log::trace!( + "logical {:?} -> {:?}, physical {:?} -> {:?}", + event.logical_key, + logical_key, + event.physical_key, + physical_key + ); + if let Some(logical_key) = logical_key { if pressed { if is_cut_command(self.egui_input.modifiers, logical_key) { From de17dbe04c5597eb23ee702d08a9a4b047c6116f Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 4 Jan 2024 22:34:12 +0100 Subject: [PATCH 2/5] Add `Modifiers::match_logically` --- crates/egui/src/data/input.rs | 79 +++++++++++++++++++- crates/egui/src/input_state.rs | 10 ++- crates/egui/src/widgets/text_edit/builder.rs | 7 +- 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index 71b77d97020..566b5b2bcba 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -655,13 +655,68 @@ impl Modifiers { !self.alt && !self.shift && self.command } + /// Checks that the `ctrl/cmd` matches, and that the `shift/alt` of the argument is a subset + /// of the pressed ksey (`self`). + /// + /// This means that if the pattern has not set `shift`, then `self` can have `shift` set or not. + /// + /// The reason is that many logical keys require `shift` or `alt` on some keyboard layouts. + /// For instance, in order to press `+` on an English keyboard, you need to press `shift` and `=`, + /// but a Swedish keyboard has dedicated `+` key. + /// So if you want to make a [`KeyboardShortcut`] looking for `Cmd` + `+`, it makes sense + /// to ignore the shift key. + /// Similarly, the `Alt` key is sometimes used to type special characters. + /// + /// However, if the pattern (the argument) explicitly requires the `shift` or `alt` keys + /// to be pressed, then they must be pressed. + /// + /// # Example: + /// ``` + /// # use egui::Modifiers; + /// # let pressed_modifiers = Modifiers::default(); + /// if pressed_modifiers.matches(Modifiers::ALT | Modifiers::SHIFT) { + /// // Alt and Shift are pressed, and nothing else + /// } + /// ``` + /// + /// ## Behavior: + /// ``` + /// # use egui::Modifiers; + /// assert!(Modifiers::CTRL.matches_logically(Modifiers::CTRL)); + /// assert!(!Modifiers::CTRL.matches_logically(Modifiers::CTRL | Modifiers::SHIFT)); + /// assert!((Modifiers::CTRL | Modifiers::SHIFT).matches_logically(Modifiers::CTRL)); + /// assert!((Modifiers::CTRL | Modifiers::COMMAND).matches_logically(Modifiers::CTRL)); + /// assert!((Modifiers::CTRL | Modifiers::COMMAND).matches_logically(Modifiers::COMMAND)); + /// assert!((Modifiers::MAC_CMD | Modifiers::COMMAND).matches_logically(Modifiers::COMMAND)); + /// assert!(!Modifiers::COMMAND.matches_logically(Modifiers::MAC_CMD)); + /// ``` + pub fn matches_logically(&self, pattern: Modifiers) -> bool { + if pattern.alt && !self.alt { + return false; + } + if pattern.shift && !self.shift { + return false; + } + + self.cmd_ctrl_matches(pattern) + } + /// Check for equality but with proper handling of [`Self::command`]. /// + /// `self` here are the currently pressed modifiers, + /// and the argument the pattern we are testing for. + /// + /// Note that this will require the `shift` and `alt` keys match, even though + /// these modifiers are sometimes required to produce some logical keys. + /// For instance, to press `+` on an English keyboard, you need to press `shift` and `=`, + /// but on a Swedish keyboard you can press the dedicated `+` key. + /// Therefore, you often want to use [`Self::matches_logically`] instead. + /// /// # Example: /// ``` /// # use egui::Modifiers; - /// # let current_modifiers = Modifiers::default(); - /// if current_modifiers.matches(Modifiers::ALT | Modifiers::SHIFT) { + /// # let pressed_modifiers = Modifiers::default(); + /// if pressed_modifiers.matches(Modifiers::ALT | Modifiers::SHIFT) { /// // Alt and Shift are pressed, and nothing else /// } /// ``` @@ -677,12 +732,28 @@ impl Modifiers { /// assert!((Modifiers::MAC_CMD | Modifiers::COMMAND).matches(Modifiers::COMMAND)); /// assert!(!Modifiers::COMMAND.matches(Modifiers::MAC_CMD)); /// ``` - pub fn matches(&self, pattern: Modifiers) -> bool { + pub fn matches_exact(&self, pattern: Modifiers) -> bool { // alt and shift must always match the pattern: if pattern.alt != self.alt || pattern.shift != self.shift { return false; } + self.cmd_ctrl_matches(pattern) + } + + #[deprecated = "Renamed `matches_exact`, but maybe you want to use `matches_logically` instead"] + pub fn matches(&self, pattern: Modifiers) -> bool { + self.matches_exact(pattern) + } + + /// Checks only cmd/ctrl, not alt/shift. + /// + /// `self` here are the currently pressed modifiers, + /// and the argument the pattern we are testing for. + /// + /// This takes care to properly handle the difference between + /// [`Self::ctrl`], [`Self::command`] and [`Self::mac_cmd`]. + pub fn cmd_ctrl_matches(&self, pattern: Modifiers) -> bool { if pattern.mac_cmd { // Mac-specific match: if !self.mac_cmd { @@ -1327,6 +1398,8 @@ fn test_key_from_name() { #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] pub struct KeyboardShortcut { pub modifiers: Modifiers, + + /// The logical key. pub key: Key, } diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index 693f6c56ee2..a044dd8c7b3 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -289,7 +289,13 @@ impl InputState { /// Count presses of a key. If non-zero, the presses are consumed, so that this will only return non-zero once. /// /// Includes key-repeat events. - pub fn count_and_consume_key(&mut self, modifiers: Modifiers, key: Key) -> usize { + /// + /// This uses [`Modifiers::matches_logically`] to match modifiers. + /// This means that e.g. the shortcut `Ctrl` + `Key::Plus` will be matched + /// as long as `Ctrl` and `Plus` are pressed, ignoring if + /// `Shift` or `Alt` are also pressed (because those modifiers might + /// be required to produce the logical `Key::Plus`). + pub fn count_and_consume_key(&mut self, modifiers: Modifiers, logical_key: Key) -> usize { let mut count = 0usize; self.events.retain(|event| { @@ -300,7 +306,7 @@ impl InputState { modifiers: ev_mods, pressed: true, .. - } if *ev_key == key && ev_mods.matches(modifiers) + } if *ev_key == logical_key && ev_mods.matches_logically(modifiers) ); count += is_match as usize; diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index 1a2243a92e7..ded1f335d57 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -990,7 +990,7 @@ fn events( pressed: true, modifiers, .. - } if modifiers.matches(Modifiers::COMMAND) => { + } if modifiers.matches_logically(Modifiers::COMMAND) => { if let Some((undo_ccursor_range, undo_txt)) = state .undoer .lock() @@ -1007,8 +1007,9 @@ fn events( pressed: true, modifiers, .. - } if (modifiers.matches(Modifiers::COMMAND) && *key == Key::Y) - || (modifiers.matches(Modifiers::SHIFT | Modifiers::COMMAND) && *key == Key::Z) => + } if (modifiers.matches_logically(Modifiers::COMMAND) && *key == Key::Y) + || (modifiers.matches_logically(Modifiers::SHIFT | Modifiers::COMMAND) + && *key == Key::Z) => { if let Some((redo_ccursor_range, redo_txt)) = state .undoer From 83e6a944c5610df9d9a418c56cdb5072924285c4 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 4 Jan 2024 22:35:00 +0100 Subject: [PATCH 3/5] Use `logical_key` for explicitness --- crates/egui/src/data/input.rs | 14 ++++++++------ crates/egui/src/input_state.rs | 11 +++++++---- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index 566b5b2bcba..17b04151ec7 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -1399,13 +1399,15 @@ fn test_key_from_name() { pub struct KeyboardShortcut { pub modifiers: Modifiers, - /// The logical key. - pub key: Key, + pub logical_key: Key, } impl KeyboardShortcut { - pub const fn new(modifiers: Modifiers, key: Key) -> Self { - Self { modifiers, key } + pub const fn new(modifiers: Modifiers, logical_key: Key) -> Self { + Self { + modifiers, + logical_key, + } } pub fn format(&self, names: &ModifierNames<'_>, is_mac: bool) -> String { @@ -1414,9 +1416,9 @@ impl KeyboardShortcut { s += names.concat; } if names.is_short { - s += self.key.symbol_or_name(); + s += self.logical_key.symbol_or_name(); } else { - s += self.key.name(); + s += self.logical_key.name(); } s } diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index a044dd8c7b3..b95303ec819 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -320,8 +320,8 @@ impl InputState { /// Check for a key press. If found, `true` is returned and the key pressed is consumed, so that this will only return `true` once. /// /// Includes key-repeat events. - pub fn consume_key(&mut self, modifiers: Modifiers, key: Key) -> bool { - self.count_and_consume_key(modifiers, key) > 0 + pub fn consume_key(&mut self, modifiers: Modifiers, logical_key: Key) -> bool { + self.count_and_consume_key(modifiers, logical_key) > 0 } /// Check if the given shortcut has been pressed. @@ -330,8 +330,11 @@ impl InputState { /// /// Includes key-repeat events. pub fn consume_shortcut(&mut self, shortcut: &KeyboardShortcut) -> bool { - let KeyboardShortcut { modifiers, key } = *shortcut; - self.consume_key(modifiers, key) + let KeyboardShortcut { + modifiers, + logical_key, + } = *shortcut; + self.consume_key(modifiers, logical_key) } /// Was the given key pressed this frame? From f3fd11e79d330fec3602978a55d3446256941140 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 4 Jan 2024 22:46:12 +0100 Subject: [PATCH 4/5] Split `Key::PluEquals` into `Plus` and `Equals` --- crates/egui-winit/src/lib.rs | 5 ++--- crates/egui/src/data/input.rs | 19 +++++++++++++------ crates/egui/src/gui_zoom.rs | 23 ++++++++++++++++++++--- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/crates/egui-winit/src/lib.rs b/crates/egui-winit/src/lib.rs index 330adf3fc03..c7123648b1b 100644 --- a/crates/egui-winit/src/lib.rs +++ b/crates/egui-winit/src/lib.rs @@ -1073,9 +1073,8 @@ fn key_from_key_code(key: winit::keyboard::KeyCode) -> Option { KeyCode::Minus | KeyCode::NumpadSubtract => Key::Minus, - // Using Mac the key with the Plus sign on it is reported as the Equals key - // (with both English and Swedish keyboard). - KeyCode::Equal | KeyCode::NumpadAdd => Key::PlusEquals, + KeyCode::NumpadAdd => Key::Plus, + KeyCode::Equal => Key::Equals, KeyCode::Digit0 | KeyCode::Numpad0 => Key::Num0, KeyCode::Digit1 | KeyCode::Numpad1 => Key::Num1, diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index 17b04151ec7..451e7f4e6da 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -968,8 +968,11 @@ pub enum Key { /// `.` Period, - /// The for the Plus/Equals key. - PlusEquals, + /// `+` + Plus, + + /// `=` + Equals, /// `;` Semicolon, @@ -1088,7 +1091,8 @@ impl Key { Self::Comma, Self::Minus, Self::Period, - Self::PlusEquals, + Self::Plus, + Self::Equals, Self::Semicolon, // Digits: Self::Num0, @@ -1188,7 +1192,8 @@ impl Key { "Comma" | "," => Self::Comma, "Minus" | "-" | "−" => Self::Minus, "Period" | "." => Self::Period, - "Plus" | "+" | "Equals" | "=" => Self::PlusEquals, + "Plus" | "+" => Self::Plus, + "Equals" | "=" => Self::Equals, "Semicolon" | ";" => Self::Semicolon, "0" => Self::Num0, @@ -1265,7 +1270,8 @@ impl Key { Key::ArrowRight => "⏵", Key::ArrowUp => "⏶", Key::Minus => crate::MINUS_CHAR_STR, - Key::PlusEquals => "+", + Key::Plus => "+", + Key::Equals => "=", _ => self.name(), } } @@ -1299,7 +1305,8 @@ impl Key { Key::Comma => "Comma", Key::Minus => "Minus", Key::Period => "Period", - Key::PlusEquals => "Plus", + Key::Plus => "Plus", + Key::Equals => "Equals", Key::Semicolon => "Semicolon", Key::Num0 => "0", diff --git a/crates/egui/src/gui_zoom.rs b/crates/egui/src/gui_zoom.rs index fb4f9bd11ed..1a4387c295f 100644 --- a/crates/egui/src/gui_zoom.rs +++ b/crates/egui/src/gui_zoom.rs @@ -6,9 +6,24 @@ use crate::*; pub mod kb_shortcuts { use super::*; - pub const ZOOM_IN: KeyboardShortcut = - KeyboardShortcut::new(Modifiers::COMMAND, Key::PlusEquals); + /// Primary keyboard shortcut for zooming in (`Cmd` + `+`). + pub const ZOOM_IN: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Plus); + + /// Secondary keyboard shortcut for zooming in (`Cmd` + `=`). + /// + /// On an English keyboard `+` is `Shift` + `=` on, + /// but it is annoying to have to press shift. + /// So most browsers also allow `Cmd` + `=` for zooming in. + /// + /// Also, on Mac winit reports `SHIFT =` as `+`, but `CMD SHIFT =` as `=`, + /// so we really need to check this. + pub const ZOOM_IN_SECONDARY: KeyboardShortcut = + KeyboardShortcut::new(Modifiers::COMMAND, Key::Equals); + + /// Keyboard shortcut for zooming in (`Cmd` + `-`). pub const ZOOM_OUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Minus); + + /// Keyboard shortcut for resetting zoom in (`Cmd` + `0`). pub const ZOOM_RESET: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Num0); } @@ -21,7 +36,9 @@ pub(crate) fn zoom_with_keyboard(ctx: &Context) { if ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_RESET)) { ctx.set_zoom_factor(1.0); } else { - if ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_IN)) { + if ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_IN)) + || ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_IN_SECONDARY)) + { zoom_in(ctx); } if ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_OUT)) { From 012c18fa267bd72ef12bf0ffb17f33af7da789ab Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 5 Jan 2024 09:35:20 +0100 Subject: [PATCH 5/5] Update comment --- crates/egui/src/gui_zoom.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/egui/src/gui_zoom.rs b/crates/egui/src/gui_zoom.rs index 1a4387c295f..c4b5deb5ed6 100644 --- a/crates/egui/src/gui_zoom.rs +++ b/crates/egui/src/gui_zoom.rs @@ -11,12 +11,10 @@ pub mod kb_shortcuts { /// Secondary keyboard shortcut for zooming in (`Cmd` + `=`). /// - /// On an English keyboard `+` is `Shift` + `=` on, + /// On an English keyboard `+` is `Shift` + `=`, /// but it is annoying to have to press shift. /// So most browsers also allow `Cmd` + `=` for zooming in. - /// - /// Also, on Mac winit reports `SHIFT =` as `+`, but `CMD SHIFT =` as `=`, - /// so we really need to check this. + /// We do the same. pub const ZOOM_IN_SECONDARY: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Equals);