Make the modifiers simple booleans in the key event

This will give a nicer API to expose to .60. If the struct weren't
repr(C) then the booleans would be nicely packed, but alas that's not
happening. On the other hand we're not keeping many instances of them
around.

api/sixtyfps-rs/lib.rs

@@ -199,8 +199,6 @@ pub mod re_exports {
     };
     pub use sixtyfps_corelib::input::{
         FocusEvent, InputEventResult, KeyEvent, KeyEventResult, KeyboardModifiers, MouseEvent,
-        ALT_MODIFIER, CONTROL_MODIFIER, COPY_PASTE_MODIFIER, LOGO_MODIFIER, NO_MODIFIER,
-        SHIFT_MODIFIER,
     };
     pub use sixtyfps_corelib::item_tree::{
         item_offset, visit_item_tree, ItemTreeNode, ItemVisitorRefMut, ItemVisitorVTable,

sixtyfps_runtime/corelib/input.rs

@@ -144,97 +144,37 @@ impl InternalKeyCode {
     }
 }
 
-/// KeyboardModifiers wraps a u32 that reserves a single bit for each
-/// possible modifier key on a keyboard, such as Shift, Control, etc.
+/// KeyboardModifier provides booleans to indicate possible modifier keys
+/// on a keyboard, such as Shift, Control, etc.
 ///
 /// On macOS, the command key is mapped to the logo modifier.
 ///
 /// On Windows, the windows key is mapped to the logo modifier.
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
 #[repr(C)]
-pub struct KeyboardModifiers(u32);
-/// KeyboardModifier wraps a u32 that has a single bit set to represent
-/// a modifier key such as shift on a keyboard. Convenience constants such as
-/// [`NO_MODIFIER`], [`SHIFT_MODIFIER`], [`CONTROL_MODIFIER`], [`ALT_MODIFIER`]
-/// and [`LOGO_MODIFIER`] are provided.
-#[derive(Copy, Clone, Debug)]
-#[repr(C)]
-pub struct KeyboardModifier(u32);
-/// Convenience constant that indicates no modifier key being pressed on a keyboard.
-pub const NO_MODIFIER: KeyboardModifier = KeyboardModifier(0);
-/// Convenience constant that indicates the shift key being pressed on a keyboard.
-pub const SHIFT_MODIFIER: KeyboardModifier = KeyboardModifier(1);
-/// Convenience constant that indicates the control key being pressed on a keyboard.
-pub const CONTROL_MODIFIER: KeyboardModifier = KeyboardModifier(2);
-/// Convenience constant that indicates the control key being pressed on a keyboard.
-pub const ALT_MODIFIER: KeyboardModifier = KeyboardModifier(4);
-/// Convenience constant that on macOS indicates the command key and on Windows the
-/// windows key being pressed on a keyboard.
-pub const LOGO_MODIFIER: KeyboardModifier = KeyboardModifier(8);
-
-/// Convenience constant that is used to detect copy & paste related shortcuts, where
-/// on macOS the modifier is the command key (aka LOGO_MODIFIER) and on Linux and Windows
-/// it is control.
-pub const COPY_PASTE_MODIFIER: KeyboardModifier =
-    if cfg!(target_os = "macos") { LOGO_MODIFIER } else { CONTROL_MODIFIER };
+pub struct KeyboardModifiers {
+    /// Indicates the alt key on a keyboard.
+    pub alt: bool,
+    /// Indicates the control key on a keyboard.
+    pub control: bool,
+    /// Indicates the shift key on a keyboard.
+    pub shift: bool,
+    /// Indicates the logo key on macOS and the windows key on Windows.
+    pub logo: bool,
+}
 
 impl KeyboardModifiers {
-    /// Returns true if this set of keyboard modifiers includes the given modifier; false otherwise.
-    ///
-    /// Arguments:
-    /// * `modifier`: The keyboard modifier to test for, usually one of the provided convenience
-    ///               constants such as [`SHIFT_MODIFIER`].
-    pub fn test(&self, modifier: KeyboardModifier) -> bool {
-        self.0 & modifier.0 != 0
+    /// Convenience function that is used to detect copy & paste related shortcuts, where
+    /// on macOS the modifier is the command key (aka logo) and on Linux and Windows
+    /// it is control.
+    pub fn is_copy_paste_modifier(&self) -> bool {
+        match (self.alt, self.control, self.shift, self.logo) {
+            #[cfg(target_os = "macos")]
+            (false, false, false, true) => true,
+            #[cfg(not(target_os = "macos"))]
+            (false, true, false, false) => true,
+            _ => false,
         }
-
-    /// Returns true if this set of keyboard modifiers consists of exactly the one specified
-    /// modifier; false otherwise.
-    ///
-    /// Arguments:
-    /// * `modifier`: The only modifier that is allowed to be in this modifier set, in order
-    //                for this function to return true;
-    pub fn test_exclusive(&self, modifier: KeyboardModifier) -> bool {
-        self.0 == modifier.0
-    }
-
-    /// Returns true if the shift key is part of this set of keyboard modifiers.
-    pub fn shift(&self) -> bool {
-        self.test(SHIFT_MODIFIER)
-    }
-
-    /// Returns true if the control key is part of this set of keyboard modifiers.
-    pub fn control(&self) -> bool {
-        self.test(CONTROL_MODIFIER)
-    }
-
-    /// Returns true if the alt key is part of this set of keyboard modifiers.
-    pub fn alt(&self) -> bool {
-        self.test(ALT_MODIFIER)
-    }
-
-    /// Returns true if on macOS the command key and on Windows the Windows key is part of this
-    /// set of keyboard modifiers.
-    pub fn logo(&self) -> bool {
-        self.test(LOGO_MODIFIER)
-    }
-}
-
-impl Default for KeyboardModifiers {
-    fn default() -> Self {
-        Self(NO_MODIFIER.0)
-    }
-}
-
-impl From<KeyboardModifier> for KeyboardModifiers {
-    fn from(modifier: KeyboardModifier) -> Self {
-        Self(modifier.0)
-    }
-}
-
-impl core::ops::BitOrAssign<KeyboardModifier> for KeyboardModifiers {
-    fn bitor_assign(&mut self, rhs: KeyboardModifier) {
-        self.0 |= rhs.0;
     }
 }
 

sixtyfps_runtime/corelib/items/text.rs

@@ -298,7 +298,7 @@ impl Item for TextInput {
                 // Only insert/interpreter non-control character strings
                 if !event_text.is_empty() && event_text.as_str().chars().all(|ch| !ch.is_control()) =>
             {
-                if modifiers.test_exclusive(crate::input::COPY_PASTE_MODIFIER) {
+                if modifiers.is_copy_paste_modifier() {
                     if event_text == "c" {
                         self.copy();
                         return KeyEventResult::EventAccepted;
@@ -385,7 +385,7 @@ enum AnchorMode {
 
 impl From<KeyboardModifiers> for AnchorMode {
     fn from(modifiers: KeyboardModifiers) -> Self {
-        if modifiers.shift() {
+        if modifiers.shift {
             Self::KeepAnchor
         } else {
             Self::MoveAnchor

sixtyfps_runtime/corelib/tests.rs

@@ -10,7 +10,7 @@ LICENSE END */
 //! Functions usefull for testing
 #![warn(missing_docs)]
 
-use crate::input::{KeyEvent, KeyboardModifiers, MouseEvent, MouseEventType, SHIFT_MODIFIER};
+use crate::input::{KeyEvent, KeyboardModifiers, MouseEvent, MouseEventType};
 use crate::window::ComponentWindow;
 use crate::SharedString;
 
@@ -71,7 +71,7 @@ pub extern "C" fn send_keyboard_string_sequence(
     for ch in sequence.chars() {
         let mut modifiers = modifiers;
         if ch.is_ascii_uppercase() {
-            modifiers |= SHIFT_MODIFIER;
+            modifiers.shift = true;
         }
         let text: SharedString = ch.to_string().into();
 

sixtyfps_runtime/rendering_backends/gl/eventloop.rs

@@ -15,9 +15,9 @@ LICENSE END */
 */
 use sixtyfps_corelib as corelib;
 
+use corelib::graphics::Point;
 use corelib::input::{InternalKeyCode, KeyEvent, KeyboardModifiers, MouseEventType};
 use corelib::window::*;
-use corelib::{graphics::Point, input::NO_MODIFIER};
 use std::cell::RefCell;
 use std::rc::{Rc, Weak};
 
@@ -355,7 +355,7 @@ pub fn run() {
                             {
                                 let modifiers = window.current_keyboard_modifiers();
 
-                                if !modifiers.control() && !modifiers.alt() && !modifiers.logo() {
+                                if !modifiers.control && !modifiers.alt && !modifiers.logo {
                                     let key_event = KeyEvent::KeyReleased {
                                         text: ch.to_string().into(),
                                         modifiers,
@@ -382,19 +382,12 @@ pub fn run() {
                         if let Some(Some(window)) =
                             windows.borrow().get(&window_id).map(|weakref| weakref.upgrade())
                         {
-                            let mut modifiers: KeyboardModifiers = NO_MODIFIER.into();
-                            if state.shift() {
-                                modifiers |= corelib::input::SHIFT_MODIFIER;
-                            }
-                            if state.alt() {
-                                modifiers |= corelib::input::ALT_MODIFIER;
-                            }
-                            if state.ctrl() {
-                                modifiers |= corelib::input::CONTROL_MODIFIER;
-                            }
-                            if state.logo() {
-                                modifiers |= corelib::input::LOGO_MODIFIER;
-                            }
+                            let modifiers = KeyboardModifiers {
+                                shift: state.shift(),
+                                alt: state.alt(),
+                                control: state.ctrl(),
+                                logo: state.logo(),
+                            };
                             window.set_current_keyboard_modifiers(modifiers);
                         }
                     });

sixtyfps_runtime/rendering_backends/qt/qt_window.rs

@@ -647,19 +647,12 @@ impl QtWindow {
     fn key_event(&self, key: i32, text: qttypes::QString, modif: u32, released: bool) {
         sixtyfps_corelib::animations::update_animations();
         let text: String = text.into();
-        let mut modifiers = sixtyfps_corelib::input::KeyboardModifiers::default();
-        if modif & key_generated::Qt_KeyboardModifier_ControlModifier != 0 {
-            modifiers |= sixtyfps_corelib::input::CONTROL_MODIFIER
-        }
-        if modif & key_generated::Qt_KeyboardModifier_AltModifier != 0 {
-            modifiers |= sixtyfps_corelib::input::ALT_MODIFIER
-        }
-        if modif & key_generated::Qt_KeyboardModifier_ShiftModifier != 0 {
-            modifiers |= sixtyfps_corelib::input::SHIFT_MODIFIER
-        }
-        if modif & key_generated::Qt_KeyboardModifier_MetaModifier != 0 {
-            modifiers |= sixtyfps_corelib::input::LOGO_MODIFIER
-        }
+        let modifiers = sixtyfps_corelib::input::KeyboardModifiers {
+            control: modif & key_generated::Qt_KeyboardModifier_ControlModifier != 0,
+            alt: modif & key_generated::Qt_KeyboardModifier_AltModifier != 0,
+            shift: modif & key_generated::Qt_KeyboardModifier_ShiftModifier != 0,
+            logo: modif & key_generated::Qt_KeyboardModifier_MetaModifier != 0,
+        };
 
         let text = match key as key_generated::Qt_Key {
             key_generated::Qt_Key_Key_Left => Some(InternalKeyCode::Left),

tests/cases/text/cursor_move.60

@@ -27,6 +27,11 @@ const HOME_CODE: char = '\u{0002}'; // start of text
 const END_CODE: char = '\u{0003}'; // end of text
 const BACK_CODE: char = '\u{0007}'; // backspace \b
 
+let shift_modifier = sixtyfps::re_exports::KeyboardModifiers {
+    shift: true,
+    ..Default::default()
+};
+
 let instance = TestCase::new();
 sixtyfps::testing::send_mouse_click(&instance, 50., 50.);
 assert!(instance.get_input_focused());
@@ -35,9 +40,9 @@ sixtyfps::testing::send_keyboard_string_sequence(&instance, "Test");
 assert_eq!(instance.get_test_text(), "Test");
 assert!(!instance.get_has_selection());
 
-sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::SHIFT_MODIFIER.into());
+sixtyfps::testing::set_current_keyboard_modifiers(&instance, shift_modifier);
 sixtyfps::testing::send_keyboard_string_sequence(&instance, &LEFT_CODE.to_string());
-sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::NO_MODIFIER.into());
+sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::KeyboardModifiers::default());
 assert!(instance.get_has_selection());
 sixtyfps::testing::send_keyboard_string_sequence(&instance, &BACK_CODE.to_string());
 assert!(!instance.get_has_selection());
@@ -57,9 +62,9 @@ sixtyfps::testing::send_keyboard_string_sequence(&instance, &LEFT_CODE.to_string
 sixtyfps::testing::send_keyboard_string_sequence(&instance, &LEFT_CODE.to_string());
 assert_eq!(instance.get_test_cursor_pos(), 0);
 
-sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::SHIFT_MODIFIER.into());
+sixtyfps::testing::set_current_keyboard_modifiers(&instance, shift_modifier);
 sixtyfps::testing::send_keyboard_string_sequence(&instance, &END_CODE.to_string());
-sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::NO_MODIFIER.into());
+sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::KeyboardModifiers::default());
 assert!(instance.get_has_selection());
 assert_eq!(instance.get_test_cursor_pos(), 2);
 assert_eq!(instance.get_test_anchor_pos(), 0);
@@ -67,9 +72,9 @@ assert_eq!(instance.get_test_anchor_pos(), 0);
 sixtyfps::testing::send_keyboard_string_sequence(&instance, &LEFT_CODE.to_string());
 assert!(!instance.get_has_selection());
 
-sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::SHIFT_MODIFIER.into());
+sixtyfps::testing::set_current_keyboard_modifiers(&instance, shift_modifier);
 sixtyfps::testing::send_keyboard_string_sequence(&instance, &HOME_CODE.to_string());
-sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::NO_MODIFIER.into());
+sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::KeyboardModifiers::default());
 assert!(instance.get_has_selection());
 assert_eq!(instance.get_test_cursor_pos(), 0);
 assert_eq!(instance.get_test_anchor_pos(), 1);

tests/cases/text/surrogate_cursor.60

@@ -24,6 +24,11 @@ TestCase := TextInput {
 const LEFT_CODE: char = '\u{000E}'; // shift out
 const BACK_CODE: char = '\u{0007}'; // backspace \b
 
+let shift_modifier = sixtyfps::re_exports::KeyboardModifiers {
+    shift: true,
+    ..Default::default()
+};
+
 let instance = TestCase::new();
 sixtyfps::testing::send_mouse_click(&instance, 50., 50.);
 assert!(instance.get_input_focused());
@@ -32,7 +37,7 @@ sixtyfps::testing::send_keyboard_string_sequence(&instance, "😍");
 assert_eq!(instance.get_test_text(), "😍");
 assert!(!instance.get_has_selection());
 
-sixtyfps::testing::set_current_keyboard_modifiers(&instance, sixtyfps::re_exports::SHIFT_MODIFIER.into());
+sixtyfps::testing::set_current_keyboard_modifiers(&instance, shift_modifier);
 sixtyfps::testing::send_keyboard_string_sequence(&instance, &LEFT_CODE.to_string());
 assert!(instance.get_has_selection());
 assert_eq!(instance.get_test_cursor_pos(), 0);