Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for more mouse buttons and tracking holding. #843

Merged
merged 1 commit into from
Apr 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ While some features like the clipboard, menus or file dialogs are not yet availa
- Window title can be any `LabelText` (such as a simple `String`). ([#869] by [@cmyr])
- `Label::with_font` and `set_font`. ([#785] by [@thecodewarrior])
- `InternalEvent::RouteTimer` to route timer events. ([#831] by [@sjoshid])
- `MouseButtons` to `MouseEvent` to track which buttons are being held down during an event. ([#843] by [@xStrom])

### Changed

Expand Down Expand Up @@ -65,6 +66,7 @@ While some features like the clipboard, menus or file dialogs are not yet availa
- Supply correct `LifeCycleCtx` to `Event::FocusChanged`. ([#878] by [@cmyr])
- Windows: Termiate app when all windows have closed. ([#763] by [@xStrom])
- macOS: `Application::quit` now quits the run loop instead of killing the process. ([#763] by [@xStrom])
- macOS/GTK/web: `MouseButton::X1` and `MouseButton::X2` clicks are now recognized. ([#843] by [@xStrom])

### Visual

Expand Down Expand Up @@ -123,6 +125,7 @@ While some features like the clipboard, menus or file dialogs are not yet availa
[#839]: https://github.com/xi-editor/druid/pull/839
[#840]: https://github.com/xi-editor/druid/pull/840
[#841]: https://github.com/xi-editor/druid/pull/841
[#843]: https://github.com/xi-editor/druid/pull/843
[#845]: https://github.com/xi-editor/druid/pull/845
[#847]: https://github.com/xi-editor/druid/pull/847
[#850]: https://github.com/xi-editor/druid/pull/850
Expand Down
2 changes: 1 addition & 1 deletion druid-shell/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub use hotkey::{HotKey, KeyCompare, RawMods, SysMods};
pub use keyboard::{KeyEvent, KeyModifiers};
pub use keycodes::KeyCode;
pub use menu::Menu;
pub use mouse::{Cursor, MouseButton, MouseEvent};
pub use mouse::{Cursor, MouseButton, MouseButtons, MouseEvent};
pub use window::{
IdleHandle, IdleToken, Text, TimerToken, WinHandler, WindowBuilder, WindowHandle,
};
185 changes: 172 additions & 13 deletions druid-shell/src/mouse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,50 +18,209 @@ use crate::kurbo::Point;

use crate::keyboard::KeyModifiers;

/// The state of the mouse for a click, mouse-up, or move event.
/// Information about the mouse event.
#[derive(Debug, Clone, PartialEq)]
pub struct MouseEvent {
/// The location of the mouse in the current window.
///
/// This is in px units, that is, adjusted for hi-dpi.
/// This is in px units not device pixels, that is, adjusted for hi-dpi.
pub pos: Point,
/// Keyboard modifiers at the time of the mouse event.
/// Mouse buttons being held down during a move or after a click event.
/// Thus it will contain the `button` that triggered a mouse-down event,
/// and it will not contain the `button` that triggered a mouse-up event.
pub buttons: MouseButtons,
/// Keyboard modifiers at the time of the event.
pub mods: KeyModifiers,
/// The number of mouse clicks associated with this event. This will always
/// be `0` for a mouse-up event.
pub count: u32,
/// The currently pressed button in the case of a move or click event,
/// or the released button in the case of a mouse-up event.
/// be `0` for a mouse-up and mouse-move events.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just discussion, but I wonder if it makes more sense for, in the case of mouse-up, for count to be the same as the count of the corresponding mouse-down.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but it's not a trivial change and so will come with #859.

pub count: u8,
/// The button that was pressed down in the case of mouse-down,
/// or the button that was released in the case of mouse-up.
/// This will always be `MouseButton::None` in the case of mouse-move.
pub button: MouseButton,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, thinking about the API:

I think that separating out MouseButtons from MouseButton solves most of our issue here, and shows a viable way to have a single type cover these three events without any clear inconsistency. There will be two fields, button and count, that are not used in MouseMove, and I think that's fine.

count is actually a funny one; the docstring says "will always be 0 on mouse-up", but I'm not sure I actually like that as an implementation; this is something we're going to have to revisit at some point, in that some platforms might provide their own double/triple click counting, and on some platforms we might need to do it ourselves. We probably do want to trust the platform, though, so that we are respecting whatever settings the user has on that platform.

My gut tells me that mouse-up should include count, though? (but this is unrelated to current PR).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MouseMove would also have a useless focused that will be introduced in #842, however even so I'm also thinking a single event is probably the way to go.

As for count, I have additional thoughts on it that I describe in #859. The short of it is that yes I think count should be there for MouseUp but it's not there right now on any platfrom besides macOS so I just wrote the docs to reflect that reality and made macOS compatible with that. This would be a short-lived change in light of #859.

}

/// An indicator of which mouse button was pressed.
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
#[repr(u8)]
pub enum MouseButton {
/// No mouse button.
// MUST BE FIRST (== 0)
None,
/// Left mouse button.
Left,
/// Middle mouse button.
Middle,
/// Right mouse button.
Right,
/// Middle mouse button.
Middle,
/// First X button.
X1,
/// Second X button.
X2,
}

impl MouseButton {
/// Returns `true` if this is the left mouse button.
#[inline(always)]
/// Returns `true` if this is [`MouseButton::Left`].
///
/// [`MouseButton::Left`]: #variant.Left
#[inline]
pub fn is_left(self) -> bool {
self == MouseButton::Left
}

/// Returns `true` if this is the right mouse button.
#[inline(always)]
/// Returns `true` if this is [`MouseButton::Right`].
///
/// [`MouseButton::Right`]: #variant.Right
#[inline]
pub fn is_right(self) -> bool {
self == MouseButton::Right
}

/// Returns `true` if this is [`MouseButton::Middle`].
///
/// [`MouseButton::Middle`]: #variant.Middle
#[inline]
pub fn is_middle(self) -> bool {
self == MouseButton::Middle
}

/// Returns `true` if this is [`MouseButton::X1`].
///
/// [`MouseButton::X1`]: #variant.X1
#[inline]
pub fn is_x1(self) -> bool {
self == MouseButton::X1
}

/// Returns `true` if this is [`MouseButton::X2`].
///
/// [`MouseButton::X2`]: #variant.X2
#[inline]
pub fn is_x2(self) -> bool {
self == MouseButton::X2
}
}

/// A set of [`MouseButton`]s.
///
/// [`MouseButton`]: enum.MouseButton.html
#[derive(PartialEq, Eq, Clone, Copy, Default)]
pub struct MouseButtons(u8);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably have more than 8 bits of possible buttons; I would just make this a u32 now, that should be enough for anybody.


impl MouseButtons {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just sort of riffing on this type; if the idea is to construct it from MouseButtons, one possible thing to do is implement FromIterator<MouseButton>, and then you could write, say,

let buttons: MouseButtons = [MouseButton::Left, MouseButton::X1].iter().copied().collect();

That doesn't feel great really; probably simpler to have a constructor that just takes a &[MouseButton].

In any case I'm just daydreaming, this isn't necessary or anything.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following one liner isn't too bad either:

let buttons = MouseButtons::new().with(MouseButton::Left).with(MouseButton::Right);

There can be value in adding more methods, however I think I would leave these slice/iterator constructors out of this PR.

/// Create a new empty set.
#[inline]
pub fn new() -> MouseButtons {
MouseButtons(0)
}

/// Add the `button` to the set.
#[inline]
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.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.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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting some YAGNI vibes here; it isn't clear when or where these would be used? Are they just by us, or do we expect external users to need to synthesize mouse buttons?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this specific without method is used in the GTK platform code already.

More generally these functions can be used by external users. Yes they might synthesize MouseButtons from some sort of hotkey configuration file and then do a check during a mouse event against that.

self.0 &= !(1.min(button as u8) << button as u8);
self
}

/// Returns `true` if the `button` is in the set.
#[inline]
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 is_empty(self) -> bool {
self.0 == 0
}

/// Returns `true` if all the `buttons` are in the set.
#[inline]
pub fn is_superset(self, buttons: MouseButtons) -> bool {
self.0 & buttons.0 == buttons.0
}

/// Returns `true` if [`MouseButton::Left`] is in the set.
///
/// [`MouseButton::Left`]: enum.MouseButton.html#variant.Left
#[inline]
pub fn has_left(self) -> bool {
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.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.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.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.contains(MouseButton::X2)
}

/// 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`.
#[inline]
pub fn union(mut self, other: MouseButtons) -> MouseButtons {
self.0 |= other.0;
self
}

/// Clear the set.
#[inline]
pub fn clear(&mut self) {
self.0 = 0;
}
}

impl std::fmt::Debug for MouseButtons {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "MouseButtons({:05b})", self.0 >> 1)
}
}

//NOTE: this currently only contains cursors that are included by default on
Expand Down
Loading