Fix bugs in consume_key and improve Modifiers API

Improvements and fixes following https://github.com/emilk/egui/pull/1212

egui/src/data/input.rs

@@ -254,18 +254,24 @@ pub enum PointerButton {
 pub const NUM_POINTER_BUTTONS: usize = 3;
 
 /// State of the modifier keys. These must be fed to egui.
-#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
+///
+/// The best way to compare `Modifiers` is by using [`Modifiers::matches`].
+#[derive(Clone, Copy, Debug, Default, PartialEq)]
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub struct Modifiers {
     /// Either of the alt keys are down (option ⌥ on Mac).
     pub alt: bool,
+
     /// Either of the control keys are down.
     /// When checking for keyboard shortcuts, consider using [`Self::command`] instead.
     pub ctrl: bool,
+
     /// Either of the shift keys are down.
     pub shift: bool,
+
     /// The Mac ⌘ Command key. Should always be set to `false` on other platforms.
     pub mac_cmd: bool,
+
     /// On Windows and Linux, set this to the same value as `ctrl`.
     /// On Mac, this should be set whenever one of the ⌘ Command keys are down (same as `mac_cmd`).
     /// This is so that egui can, for instance, select all text by checking for `command + A`
@@ -278,37 +284,51 @@ impl Modifiers {
         Default::default()
     }
 
-    pub fn alt(self, value: bool) -> Self {
-        Self { alt: value, ..self }
-    }
+    pub const NONE: Self = Self {
+        alt: false,
+        ctrl: false,
+        shift: false,
+        mac_cmd: false,
+        command: false,
+    };
 
-    pub fn ctrl(self, value: bool) -> Self {
-        Self {
-            ctrl: value,
-            ..self
-        }
-    }
-
-    pub fn shift(self, value: bool) -> Self {
-        Self {
-            shift: value,
-            ..self
-        }
-    }
-
-    pub fn mac_cmd(self, value: bool) -> Self {
-        Self {
-            mac_cmd: value,
-            ..self
-        }
-    }
-
-    pub fn command(self, value: bool) -> Self {
-        Self {
-            command: value,
-            ..self
-        }
-    }
+    pub const ALT: Self = Self {
+        alt: true,
+        ctrl: false,
+        shift: false,
+        mac_cmd: false,
+        command: false,
+    };
+    pub const CTRL: Self = Self {
+        alt: false,
+        ctrl: true,
+        shift: false,
+        mac_cmd: false,
+        command: false,
+    };
+    pub const SHIFT: Self = Self {
+        alt: false,
+        ctrl: false,
+        shift: true,
+        mac_cmd: false,
+        command: false,
+    };
+    /// The Mac ⌘ Command key
+    pub const MAC_CMD: Self = Self {
+        alt: false,
+        ctrl: false,
+        shift: false,
+        mac_cmd: true,
+        command: false,
+    };
+    /// On Mac: ⌘ Command key, elsewhere: Ctrl key
+    pub const COMMAND: Self = Self {
+        alt: false,
+        ctrl: false,
+        shift: false,
+        mac_cmd: false,
+        command: true,
+    };
 
     #[inline(always)]
     pub fn is_none(&self) -> bool {
@@ -320,6 +340,7 @@ impl Modifiers {
         !self.is_none()
     }
 
+    /// Is shift the only pressed button?
     #[inline(always)]
     pub fn shift_only(&self) -> bool {
         self.shift && !(self.alt || self.command)
@@ -330,6 +351,66 @@ impl Modifiers {
     pub fn command_only(&self) -> bool {
         !self.alt && !self.shift && self.command
     }
+
+    /// Check for equality but with proper handling of [`Self::command`].
+    ///
+    /// ```
+    /// # use egui::Modifiers;
+    /// assert!(Modifiers::CTRL.matches(Modifiers::CTRL));
+    /// assert!(!Modifiers::CTRL.matches(Modifiers::CTRL | Modifiers::SHIFT));
+    /// assert!(!(Modifiers::CTRL | Modifiers::SHIFT).matches(Modifiers::CTRL));
+    /// assert!((Modifiers::CTRL | Modifiers::COMMAND).matches(Modifiers::CTRL));
+    /// assert!((Modifiers::CTRL | Modifiers::COMMAND).matches(Modifiers::COMMAND));
+    /// assert!((Modifiers::MAC_CMD | Modifiers::COMMAND).matches(Modifiers::COMMAND));
+    /// assert!(!Modifiers::COMMAND.matches(Modifiers::MAC_CMD));
+    /// ```
+    pub fn matches(&self, pattern: Modifiers) -> bool {
+        // alt and shift must always match the pattern:
+        if pattern.alt != self.alt || pattern.shift != self.shift {
+            return false;
+        }
+
+        if pattern.mac_cmd {
+            // Mac-specific match:
+            if !self.mac_cmd {
+                return false;
+            }
+            if pattern.ctrl != self.ctrl {
+                return false;
+            }
+            return true;
+        }
+
+        if !pattern.ctrl && !pattern.command {
+            // the pattern explicitly doesn't want any ctrl/command:
+            return !self.ctrl && !self.command;
+        }
+
+        // if the pattern is looking for command, then `ctrl` may or may not be set depending on platform.
+        // if the pattern is looking for `ctrl`, then `command` may or may not be set depending on platform.
+
+        if pattern.ctrl && !self.ctrl {
+            return false;
+        }
+        if pattern.command && !self.command {
+            return false;
+        }
+
+        true
+    }
+}
+
+impl std::ops::BitOr for Modifiers {
+    type Output = Self;
+    fn bitor(self, rhs: Self) -> Self {
+        Self {
+            alt: self.alt | rhs.alt,
+            ctrl: self.ctrl | rhs.ctrl,
+            shift: self.shift | rhs.shift,
+            mac_cmd: self.mac_cmd | rhs.mac_cmd,
+            command: self.command | rhs.command,
+        }
+    }
 }
 
 /// Keyboard keys.

egui/src/input_state.rs

@@ -192,22 +192,26 @@ impl InputState {
         self.pointer.wants_repaint() || self.scroll_delta != Vec2::ZERO || !self.events.is_empty()
     }
 
-    /// Ignore a key if it was pressed or released this frame. Useful for hotkeys.
-    /// Matches on both key press and key release, consuming them and removing them from `self.events`.
-    /// Returns true if the key was pressed this frame (even if the key release was consumed).
+    /// Check for a key press. If found, `true` is returned and the key pressed is consumed, so that this will only return `true` once.
     pub fn consume_key(&mut self, modifiers: Modifiers, key: Key) -> bool {
+        let mut match_found = false;
+
         self.events.retain(|event| {
-            !matches!(
+            let is_match = matches!(
                 event,
                 Event::Key {
                     key: ev_key,
                     modifiers: ev_mods,
-                    ..
-                } if *ev_key == key && *ev_mods == modifiers
-            )
+                    pressed: true
+                } if *ev_key == key && ev_mods.matches(modifiers)
+            );
+
+            match_found |= is_match;
+
+            !is_match
         });
 
-        self.keys_down.remove(&key)
+        match_found
     }
 
     /// Was the given key pressed this frame?

egui_demo_lib/src/apps/demo/text_edit.rs

@@ -64,7 +64,10 @@ impl super::View for TextEdit {
             anything_selected,
             egui::Label::new("Press ctrl+T to toggle the case of selected text (cmd+T on Mac)"),
         );
-        if ui.input().modifiers.command_only() && ui.input().key_pressed(egui::Key::T) {
+        if ui
+            .input_mut()
+            .consume_key(egui::Modifiers::COMMAND, egui::Key::T)
+        {
             if let Some(text_cursor_range) = output.cursor_range {
                 use egui::TextBuffer as _;
                 let selected_chars = text_cursor_range.as_sorted_char_range();

egui_demo_lib/src/easy_mark/easy_mark_editor.rs

@@ -126,10 +126,7 @@ fn shortcuts(ui: &Ui, code: &mut dyn TextBuffer, ccursor_range: &mut CCursorRang
         (Key::S, "~"), // ~strikethrough~
         (Key::U, "_"), // _underline_
     ] {
-        if ui
-            .input_mut()
-            .consume_key(egui::Modifiers::new().command(true), key)
-        {
+        if ui.input_mut().consume_key(egui::Modifiers::COMMAND, key) {
             toggle_surrounding(code, ccursor_range, surrounding);
             any_change = true;
         };