Ignore extra SHIFT and ALT when matching modifiers (#3769)

* Closes #3626

Basically, egui now ignores extra SHIFT and ALT pressed when matching
keyboard shortcuts.
This is because SHIFT and ALT are often requires to produce some logical
keys.
For instance, typing `+` on an English keyboard requires pressing `SHIFT
=`,
so the keyboard shortcut looking for `CTRL +` should ignore the SHIFT
key.

@abey79 You reported problem using `Cmd +` and `Cmd -` to zoom - does
this fix it for you?

You can run with `RUST_LOG=egui_winit=trace cargo run` to see a printout
of how winit reports the logical and physical keys, and how egui
interprets them.

Weirdly, on Mac winit reports `SHIFT =` as `+`, but `CMD SHIFT =` as `=`
(on an English keyboard) so things are… difficult.

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) {
@@ -1064,9 +1073,8 @@ fn key_from_key_code(key: winit::keyboard::KeyCode) -> Option<egui::Key> {
 
         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,

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 {
@@ -897,8 +968,11 @@ pub enum Key {
     /// `.`
     Period,
 
-    /// The for the Plus/Equals key.
-    PlusEquals,
+    /// `+`
+    Plus,
+
+    /// `=`
+    Equals,
 
     /// `;`
     Semicolon,
@@ -1017,7 +1091,8 @@ impl Key {
         Self::Comma,
         Self::Minus,
         Self::Period,
-        Self::PlusEquals,
+        Self::Plus,
+        Self::Equals,
         Self::Semicolon,
         // Digits:
         Self::Num0,
@@ -1117,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,
@@ -1194,7 +1270,8 @@ impl Key {
             Key::ArrowRight => "⏵",
             Key::ArrowUp => "⏶",
             Key::Minus => crate::MINUS_CHAR_STR,
-            Key::PlusEquals => "+",
+            Key::Plus => "+",
+            Key::Equals => "=",
             _ => self.name(),
         }
     }
@@ -1228,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",
@@ -1327,12 +1405,16 @@ fn test_key_from_name() {
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub struct KeyboardShortcut {
     pub modifiers: Modifiers,
-    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 {
@@ -1341,9 +1423,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
     }

crates/egui/src/gui_zoom.rs

@@ -6,9 +6,22 @@ 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` + `=`,
+    /// but it is annoying to have to press shift.
+    /// So most browsers also allow `Cmd` + `=` for zooming in.
+    /// We do the same.
+    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 +34,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)) {

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;
@@ -314,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.
@@ -324,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?

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