Improve MouseButtons based on code review.

druid-shell/src/mouse.rs

@@ -44,6 +44,9 @@ pub struct MouseEvent {
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 #[repr(u8)]
 pub enum MouseButton {
+    /// No mouse button.
+    // MUST BE FIRST (== 0)
+    None,
     /// Left mouse button.
     Left,
     /// Right mouse button.
@@ -54,8 +57,6 @@ pub enum MouseButton {
     X1,
     /// Second X button.
     X2,
-    /// No mouse button.
-    None,
 }
 
 impl MouseButton {
@@ -115,51 +116,45 @@ impl MouseButtons {
 
     /// Add the `button` to the set.
     #[inline]
-    pub fn add(&mut self, button: MouseButton) {
-        self.0 |= 1 << button as u8;
+    pub fn insert(&mut self, button: MouseButton) {
+        self.0 |= 1.min(button as u8) << button as u8;
     }
 
     /// Remove the `button` from the set.
     #[inline]
     pub fn remove(&mut self, button: MouseButton) {
-        self.0 &= !(1 << button as u8);
+        self.0 &= !(1.min(button as u8) << button as u8);
     }
 
     /// Builder-style method for adding the `button` to the set.
     #[inline]
     pub fn with(mut self, button: MouseButton) -> MouseButtons {
-        self.0 |= 1 << button as u8;
+        self.0 |= 1.min(button as u8) << button as u8;
         self
     }
 
     /// Builder-style method for removing the `button` from the set.
     #[inline]
     pub fn without(mut self, button: MouseButton) -> MouseButtons {
-        self.0 &= !(1 << button as u8);
+        self.0 &= !(1.min(button as u8) << button as u8);
         self
     }
 
     /// Returns `true` if the `button` is in the set.
     #[inline]
-    pub fn has(self, button: MouseButton) -> bool {
-        (self.0 & (1 << button as u8)) != 0
-    }
-
-    /// Returns `true` if any button is in the set.
-    #[inline]
-    pub fn has_any(self) -> bool {
-        self.0 != 0
+    pub fn contains(self, button: MouseButton) -> bool {
+        (self.0 & (1.min(button as u8) << button as u8)) != 0
     }
 
     /// Returns `true` if the set is empty.
     #[inline]
-    pub fn has_none(self) -> bool {
+    pub fn is_empty(self) -> bool {
         self.0 == 0
     }
 
     /// Returns `true` if all the `buttons` are in the set.
     #[inline]
-    pub fn has_all(self, buttons: MouseButtons) -> bool {
+    pub fn is_superset(self, buttons: MouseButtons) -> bool {
         self.0 & buttons.0 == buttons.0
     }
 
@@ -168,51 +163,47 @@ impl MouseButtons {
     /// [`MouseButton::Left`]: enum.MouseButton.html#variant.Left
     #[inline]
     pub fn has_left(self) -> bool {
-        self.has(MouseButton::Left)
+        self.contains(MouseButton::Left)
     }
 
     /// Returns `true` if [`MouseButton::Right`] is in the set.
     ///
     /// [`MouseButton::Right`]: enum.MouseButton.html#variant.Right
     #[inline]
     pub fn has_right(self) -> bool {
-        self.has(MouseButton::Right)
+        self.contains(MouseButton::Right)
     }
 
     /// Returns `true` if [`MouseButton::Middle`] is in the set.
     ///
     /// [`MouseButton::Middle`]: enum.MouseButton.html#variant.Middle
     #[inline]
     pub fn has_middle(self) -> bool {
-        self.has(MouseButton::Middle)
+        self.contains(MouseButton::Middle)
     }
 
     /// Returns `true` if [`MouseButton::X1`] is in the set.
     ///
     /// [`MouseButton::X1`]: enum.MouseButton.html#variant.X1
     #[inline]
     pub fn has_x1(self) -> bool {
-        self.has(MouseButton::X1)
+        self.contains(MouseButton::X1)
     }
 
     /// Returns `true` if [`MouseButton::X2`] is in the set.
     ///
     /// [`MouseButton::X2`]: enum.MouseButton.html#variant.X2
     #[inline]
     pub fn has_x2(self) -> bool {
-        self.has(MouseButton::X2)
+        self.contains(MouseButton::X2)
     }
 
-    /// Adds all the [`MouseButton`]s in `other` to the set.
-    ///
-    /// [`MouseButton`]: enum.MouseButton.html
-    pub fn extend(&mut self, other: MouseButtons) {
-        self.0 |= other.0;
+    /// Adds all the `buttons` to the set.
+    pub fn extend(&mut self, buttons: MouseButtons) {
+        self.0 |= buttons.0;
     }
 
     /// Returns a union of the values in `self` and `other`.
-    ///
-    /// [`MouseButton`]: enum.MouseButton.html
     #[inline]
     pub fn union(mut self, other: MouseButtons) -> MouseButtons {
         self.0 |= other.0;
@@ -228,7 +219,7 @@ impl MouseButtons {
 
 impl std::fmt::Debug for MouseButtons {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        write!(f, "MouseButtons({:06b})", self.0)
+        write!(f, "MouseButtons({:05b})", self.0 >> 1)
     }
 }
 

druid-shell/src/platform/gtk/window.rs

@@ -787,22 +787,22 @@ fn get_mouse_button(button: u32) -> Option<MouseButton> {
 fn get_mouse_buttons_from_modifiers(modifiers: gdk::ModifierType) -> MouseButtons {
     let mut buttons = MouseButtons::new();
     if modifiers.contains(ModifierType::BUTTON1_MASK) {
-        buttons.add(MouseButton::Left);
+        buttons.insert(MouseButton::Left);
     }
     if modifiers.contains(ModifierType::BUTTON2_MASK) {
-        buttons.add(MouseButton::Middle);
+        buttons.insert(MouseButton::Middle);
     }
     if modifiers.contains(ModifierType::BUTTON3_MASK) {
-        buttons.add(MouseButton::Right);
+        buttons.insert(MouseButton::Right);
     }
     // TODO: Determine X1/X2 state (do caching ourselves if needed)
     //       Checking for BUTTON4_MASK/BUTTON5_MASK does not work with GDK X,
     //       because those are wheel events instead.
     if modifiers.contains(ModifierType::BUTTON4_MASK) {
-        buttons.add(MouseButton::X1);
+        buttons.insert(MouseButton::X1);
     }
     if modifiers.contains(ModifierType::BUTTON5_MASK) {
-        buttons.add(MouseButton::X2);
+        buttons.insert(MouseButton::X2);
     }
     buttons
 }

druid-shell/src/platform/mac/window.rs

@@ -391,19 +391,19 @@ fn get_mouse_button(button: NSInteger) -> Option<MouseButton> {
 fn get_mouse_buttons(mask: NSUInteger) -> MouseButtons {
     let mut buttons = MouseButtons::new();
     if mask & 1 != 0 {
-        buttons.add(MouseButton::Left);
+        buttons.insert(MouseButton::Left);
     }
     if mask & 1 << 1 != 0 {
-        buttons.add(MouseButton::Right);
+        buttons.insert(MouseButton::Right);
     }
     if mask & 1 << 2 != 0 {
-        buttons.add(MouseButton::Middle);
+        buttons.insert(MouseButton::Middle);
     }
     if mask & 1 << 3 != 0 {
-        buttons.add(MouseButton::X1);
+        buttons.insert(MouseButton::X1);
     }
     if mask & 1 << 4 != 0 {
-        buttons.add(MouseButton::X2);
+        buttons.insert(MouseButton::X2);
     }
     buttons
 }

druid-shell/src/platform/web/window.rs

@@ -630,19 +630,19 @@ fn mouse_button(button: i16) -> Option<MouseButton> {
 fn mouse_buttons(mask: u16) -> MouseButtons {
     let mut buttons = MouseButtons::new();
     if mask & 1 != 0 {
-        buttons.add(MouseButton::Left);
+        buttons.insert(MouseButton::Left);
     }
     if mask & 1 << 1 != 0 {
-        buttons.add(MouseButton::Right);
+        buttons.insert(MouseButton::Right);
     }
     if mask & 1 << 2 != 0 {
-        buttons.add(MouseButton::Middle);
+        buttons.insert(MouseButton::Middle);
     }
     if mask & 1 << 3 != 0 {
-        buttons.add(MouseButton::X1);
+        buttons.insert(MouseButton::X1);
     }
     if mask & 1 << 4 != 0 {
-        buttons.add(MouseButton::X2);
+        buttons.insert(MouseButton::X2);
     }
     buttons
 }

druid-shell/src/platform/windows/window.rs

@@ -252,19 +252,19 @@ fn get_mod_state_win() -> bool {
 fn get_buttons(wparam: WPARAM) -> MouseButtons {
     let mut buttons = MouseButtons::new();
     if wparam & MK_LBUTTON != 0 {
-        buttons.add(MouseButton::Left);
+        buttons.insert(MouseButton::Left);
     }
     if wparam & MK_RBUTTON != 0 {
-        buttons.add(MouseButton::Right);
+        buttons.insert(MouseButton::Right);
     }
     if wparam & MK_MBUTTON != 0 {
-        buttons.add(MouseButton::Middle);
+        buttons.insert(MouseButton::Middle);
     }
     if wparam & MK_XBUTTON1 != 0 {
-        buttons.add(MouseButton::X1);
+        buttons.insert(MouseButton::X1);
     }
     if wparam & MK_XBUTTON2 != 0 {
-        buttons.add(MouseButton::X2);
+        buttons.insert(MouseButton::X2);
     }
     buttons
 }
@@ -330,17 +330,17 @@ impl WndState {
     }
 
     fn enter_mouse_capture(&mut self, hwnd: HWND, button: MouseButton) {
-        if self.captured_mouse_buttons.has_none() {
+        if self.captured_mouse_buttons.is_empty() {
             unsafe {
                 SetCapture(hwnd);
             }
         }
-        self.captured_mouse_buttons.add(button);
+        self.captured_mouse_buttons.insert(button);
     }
 
     fn exit_mouse_capture(&mut self, button: MouseButton) -> bool {
         self.captured_mouse_buttons.remove(button);
-        self.captured_mouse_buttons.has_none()
+        self.captured_mouse_buttons.is_empty()
     }
 }
 

druid-shell/src/platform/x11/window.rs

@@ -424,7 +424,6 @@ impl WindowHandle {
 
     pub fn get_dpi(&self) -> f32 {
         // TODO(x11/dpi_scaling): figure out DPI scaling
-        //log::warn!("WindowHandle::get_dpi is currently unimplemented for X11 platforms.");
         96.0
     }
 }