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

Refactor response bools to bitflags #3905

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/egui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Owner

Choose a reason for hiding this comment

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

This should move up. It is currently under the #! ### Optional dependencies heading

log = { version = "0.4", optional = true, features = ["std"] }
puffin = { workspace = true, optional = true }
ron = { version = "0.8", optional = true }
Expand Down
66 changes: 35 additions & 31 deletions crates/egui/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -956,26 +956,24 @@ impl Context {
enabled: bool,
contains_pointer: bool,
) -> Response {
let mut state = ResponseState::empty();
state.set(ResponseState::ENABLED, enabled);
state.set(ResponseState::CONTAINS_POINTER, contains_pointer);
state.set(ResponseState::HOVERED, contains_pointer && enabled);
state.set(ResponseState::HIGHLIGHTED, self.frame_state(|fs| fs.highlight_this_frame.contains(&id)));

// This is the start - we'll fill in the fields below:
let mut res = Response {
ctx: self.clone(),
layer_id,
id,
rect,
sense,
enabled,
contains_pointer,
hovered: contains_pointer && enabled,
highlighted: self.frame_state(|fs| fs.highlight_this_frame.contains(&id)),
state,
clicked: Default::default(),
double_clicked: Default::default(),
triple_clicked: Default::default(),
drag_started: false,
dragged: false,
drag_released: false,
is_pointer_button_down_on: false,
interact_pointer_pos: None,
changed: false, // must be set by the widget itself
};

if !enabled || !sense.focusable || !layer_id.allow_interaction() {
Expand Down Expand Up @@ -1024,22 +1022,21 @@ impl Context {
interaction.click_interest |= contains_pointer && sense.click;
interaction.drag_interest |= contains_pointer && sense.drag;

res.is_pointer_button_down_on =
interaction.click_id == Some(id) || interaction.drag_id == Some(id);
res.state.set(ResponseState::IS_POINTER_BUTTON_DOWN_ON, interaction.click_id == Some(id) || interaction.drag_id == Some(id));

if sense.click && sense.drag {
// This widget is sensitive to both clicks and drags.
// When the mouse first is pressed, it could be either,
// so we postpone the decision until we know.
res.dragged =
interaction.drag_id == Some(id) && input.pointer.is_decidedly_dragging();
res.drag_started = res.dragged && input.pointer.started_decidedly_dragging;
res.state.set(ResponseState::DRAGGED, interaction.drag_id == Some(id) && input.pointer.is_decidedly_dragging());
res.state.set(ResponseState::DRAG_STARTED, res.dragged() && input.pointer.started_decidedly_dragging);
} else if sense.drag {
// We are just sensitive to drags, so we can mark ourself as dragged right away:
res.dragged = interaction.drag_id == Some(id);
res.state.set(ResponseState::DRAGGED, interaction.drag_id == Some(id));
// res.drag_started will be filled below if applicable
}


for pointer_event in &input.pointer.pointer_events {
match pointer_event {
PointerEvent::Moved(_) => {}
Expand All @@ -1051,7 +1048,9 @@ impl Context {
if sense.click && interaction.click_id.is_none() {
// potential start of a click
interaction.click_id = Some(id);
res.is_pointer_button_down_on = true;
res
.state
.insert(ResponseState::IS_POINTER_BUTTON_DOWN_ON);
}

// HACK: windows have low priority on dragging.
Expand All @@ -1066,28 +1065,31 @@ impl Context {
interaction.drag_id = Some(id);
interaction.drag_is_window = false;
memory.set_window_interaction(None); // HACK: stop moving windows (if any)

res.is_pointer_button_down_on = true;
res
.state
.insert(ResponseState::IS_POINTER_BUTTON_DOWN_ON);

// Again, only if we are ONLY sensitive to drags can we decide that this is a drag now.
if sense.click {
res.dragged = false;
res.drag_started = false;
res.state.remove(ResponseState::DRAGGED);
res.state.remove(ResponseState::DRAG_STARTED);
} else {
res.dragged = true;
res.drag_started = true;
res.state.insert(ResponseState::DRAGGED);
res.state.insert(ResponseState::DRAG_STARTED);
}
}
}
}

PointerEvent::Released { click, button } => {
res.drag_released = res.dragged;
res.dragged = false;
res
.state
.set(ResponseState::DRAG_RELEASED, res.dragged());
res.state.remove(ResponseState::DRAGGED);

if sense.click && res.hovered && res.is_pointer_button_down_on {
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();
Comment on lines +1097 to +1099
Copy link
Author

Choose a reason for hiding this comment

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

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?

Copy link
Owner

Choose a reason for hiding this comment

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

The optimizer should take care of it for you if fn hovered is marked #[inline]

Copy link
Owner

Choose a reason for hiding this comment

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

You can always run cargo bench to check

res.clicked[*button as usize] = clicked;
res.double_clicked[*button as usize] =
clicked && click.is_double();
Expand All @@ -1096,19 +1098,21 @@ impl Context {
}
}

res.is_pointer_button_down_on = false;
res
.state
.remove(ResponseState::IS_POINTER_BUTTON_DOWN_ON);
}
}
}
}

if res.is_pointer_button_down_on {
if res.is_pointer_button_down_on() {
res.interact_pointer_pos = input.pointer.interact_pos();
}

if input.pointer.any_down() && !res.is_pointer_button_down_on {
if input.pointer.any_down() && !res.is_pointer_button_down_on() {
// We don't hover widgets while interacting with *other* widgets:
res.hovered = false;
res.state.remove(ResponseState::HOVERED);
}

if memory.has_focus(res.id) && clicked_elsewhere {
Expand Down Expand Up @@ -2491,7 +2495,7 @@ impl Context {
// TODO(emilk): `Sense::hover_highlight()`
if ui
.add(Label::new(RichText::new(text).monospace()).sense(Sense::click()))
.hovered
.hovered()
&& is_visible
{
ui.ctx()
Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ pub use {
load::SizeHint,
memory::{Memory, Options},
painter::Painter,
response::{InnerResponse, Response},
response::{InnerResponse, Response, ResponseState},
sense::Sense,
style::{FontSelection, Margin, Style, TextStyle, Visuals},
text::{Galley, TextFormat},
Expand Down
Loading