Refactor response bools to bitflags #3905
Conversation
Signed-off-by: Matt Provost <mattprovost6@gmail.com>
| @@ -6,6 +6,41 @@ use crate::{ | |||
|
|
|||
| // ---------------------------------------------------------------------------- | |||
|
|
|||
| bitflags::bitflags! { | |||
| /// The state of a widget. | |||
There was a problem hiding this comment.
I have no idea what would be good docs for this struct. Feedback would be appreciated :)
| @@ -6,6 +6,41 @@ use crate::{ | |||
|
|
|||
| // ---------------------------------------------------------------------------- | |||
|
|
|||
| bitflags::bitflags! { | |||
There was a problem hiding this comment.
Is this the right place to put this struct?
Signed-off-by: Matt Provost <mattprovost6@gmail.com>
Signed-off-by: Matt Provost <mattprovost6@gmail.com>
| if sense.click && res.hovered() && res.is_pointer_button_down_on() { | ||
| if let Some(click) = click { | ||
| let clicked = res.hovered && res.is_pointer_button_down_on; | ||
| let clicked = res.hovered() && res.is_pointer_button_down_on(); |
There was a problem hiding this comment.
Since clicked now has more complexity associated with it (performing bitwise operations to check if the bitflags contain those 2), should it be moved out of the 2 ifs and just reused? Or is the added cycles negligible to performance?
There was a problem hiding this comment.
The optimizer should take care of it for you if fn hovered is marked #[inline]
There was a problem hiding this comment.
You can always run cargo bench to check
|
Also, should |
Signed-off-by: Matt Provost <mattprovost6@gmail.com>
Yes |
| @@ -92,6 +92,7 @@ backtrace = { version = "0.3", optional = true } | |||
| ## Enable this when generating docs. | |||
| document-features = { version = "0.2", optional = true } | |||
|
|
|||
| bitflags = "2.4" | |||
There was a problem hiding this comment.
This should move up. It is currently under the #! ### Optional dependencies heading
| if sense.click && res.hovered() && res.is_pointer_button_down_on() { | ||
| if let Some(click) = click { | ||
| let clicked = res.hovered && res.is_pointer_button_down_on; | ||
| let clicked = res.hovered() && res.is_pointer_button_down_on(); |
There was a problem hiding this comment.
You can always run cargo bench to check
| @@ -6,6 +6,44 @@ use crate::{ | |||
|
|
|||
| // ---------------------------------------------------------------------------- | |||
|
|
|||
| bitflags::bitflags! { | |||
| /// The state of a widget. | |||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] | |||
There was a problem hiding this comment.
I don't think it makes sense for this to implement PartialOrd and Ord
This PR focuses primarily on adding the
bitflagsdependency and converting the immediatebools inResponseinto bitflags. I'm mainly looking for style feedback before diving into the[bool; NUM_POINTER_BUTTONS]andSensebitflags.Tested manually with some basic sanity testing as well as
cargo test.Partially fixes #3862