From cd1ed733883549bcfe52b009aed8b5c993703314 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 18:16:04 +0100 Subject: [PATCH] Change the definition of `clicked_by` (#4192) This is a refactor on the way to add support for opening context menus on touch screens via press-and-hold. This PR changes what `InputState::button_clicked` does (it was ver badly named before), and also changes `Response::clicked_by` to no longer be true if clicking with keyboard (i.e. a widget has keyboard focus and the user presses Space or Enter). --- CHANGELOG.md | 7 ++ crates/egui/src/context.rs | 19 ++---- crates/egui/src/input_state.rs | 12 ++-- crates/egui/src/input_state/touch_state.rs | 5 +- crates/egui/src/menu.rs | 7 +- crates/egui/src/response.rs | 77 ++++++++++------------ crates/egui_demo_lib/src/demo/pan_zoom.rs | 2 +- 7 files changed, 62 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c468fc815d..0a074c6f11e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ This file is updated upon each release. Changes since the last release can be found at or by running the `scripts/generate_changelog.py` script. +## Unreleased + +### ⚠️ BREAKING +* `Response::clicked*` and `Response::dragged*` may lock the `Context`, so don't call it from a `Context`-locking closure. +* `Response::clicked_by` will no longer be true if clicked with keyboard. Use `Response::clicked` instead. + + ## 0.26.2 - 2024-02-14 * Avoid interacting twice when not required [#4041](https://github.com/emilk/egui/pull/4041) (thanks [@abey79](https://github.com/abey79)!) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index c1dcd5b5be9..9728281e9b7 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -1104,9 +1104,8 @@ impl Context { contains_pointer: false, hovered: false, highlighted, - clicked: Default::default(), - double_clicked: Default::default(), - triple_clicked: Default::default(), + clicked: false, + fake_primary_click: false, drag_started: false, dragged: false, drag_stopped: false, @@ -1131,7 +1130,7 @@ impl Context { && (input.key_pressed(Key::Space) || input.key_pressed(Key::Enter)) { // Space/enter works like a primary click for e.g. selected buttons - res.clicked[PointerButton::Primary as usize] = true; + res.fake_primary_click = true; } #[cfg(feature = "accesskit")] @@ -1139,7 +1138,7 @@ impl Context { && sense.click && input.has_accesskit_action_request(id, accesskit::Action::Default) { - res.clicked[PointerButton::Primary as usize] = true; + res.fake_primary_click = true; } let interaction = memory.interaction(); @@ -1157,13 +1156,9 @@ impl Context { let clicked = Some(id) == viewport.interact_widgets.clicked; for pointer_event in &input.pointer.pointer_events { - if let PointerEvent::Released { click, button } = pointer_event { - if enabled && sense.click && clicked { - if let Some(click) = click { - res.clicked[*button as usize] = true; - res.double_clicked[*button as usize] = click.is_double(); - res.triple_clicked[*button as usize] = click.is_triple(); - } + if let PointerEvent::Released { click, .. } = pointer_event { + if enabled && sense.click && clicked && click.is_some() { + res.clicked = true; } res.is_pointer_button_down_on = false; diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index ed29282e950..7fc29518ad1 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -489,11 +489,7 @@ impl InputState { /// delivers a synthetic zoom factor based on ctrl-scroll events, as a fallback. pub fn multi_touch(&self) -> Option { // In case of multiple touch devices simply pick the touch_state of the first active device - if let Some(touch_state) = self.touch_states.values().find(|t| t.is_active()) { - touch_state.info() - } else { - None - } + self.touch_states.values().find_map(|t| t.info()) } /// True if there currently are any fingers touching egui. @@ -976,11 +972,13 @@ impl PointerState { self.pointer_events.iter().any(|event| event.is_click()) } - /// Was the button given clicked this frame? + /// Was the given pointer button given clicked this frame? + /// + /// Returns true on double- and triple- clicks too. pub fn button_clicked(&self, button: PointerButton) -> bool { self.pointer_events .iter() - .any(|event| matches!(event, &PointerEvent::Pressed { button: b, .. } if button == b)) + .any(|event| matches!(event, &PointerEvent::Released { button: b, click: Some(_) } if button == b)) } /// Was the button given double clicked this frame? diff --git a/crates/egui/src/input_state/touch_state.rs b/crates/egui/src/input_state/touch_state.rs index 2ee7a91de75..2a77a4d371b 100644 --- a/crates/egui/src/input_state/touch_state.rs +++ b/crates/egui/src/input_state/touch_state.rs @@ -163,6 +163,7 @@ impl TouchState { _ => (), } } + // This needs to be called each frame, even if there are no new touch events. // Otherwise, we would send the same old delta information multiple times: self.update_gesture(time, pointer_pos); @@ -176,10 +177,6 @@ impl TouchState { } } - pub fn is_active(&self) -> bool { - self.gesture_state.is_some() - } - pub fn info(&self) -> Option { self.gesture_state.as_ref().map(|state| { // state.previous can be `None` when the number of simultaneous touches has just diff --git a/crates/egui/src/menu.rs b/crates/egui/src/menu.rs index 1f7a74fbf4d..6059236787c 100644 --- a/crates/egui/src/menu.rs +++ b/crates/egui/src/menu.rs @@ -367,6 +367,9 @@ impl MenuRoot { /// Interaction with a context menu (secondary click). fn context_interaction(response: &Response, root: &mut Option) -> MenuResponse { let response = response.interact(Sense::click()); + let hovered = response.hovered(); + let secondary_clicked = response.secondary_clicked(); + response.ctx.input(|input| { let pointer = &input.pointer; if let Some(pos) = pointer.interact_pos() { @@ -377,9 +380,9 @@ impl MenuRoot { destroy = !in_old_menu && pointer.any_pressed() && root.id == response.id; } if !in_old_menu { - if response.hovered() && response.secondary_clicked() { + if hovered && secondary_clicked { return MenuResponse::Create(pos, response.id); - } else if (response.hovered() && pointer.primary_down()) || destroy { + } else if destroy || hovered && pointer.primary_down() { return MenuResponse::Close; } } diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index ddd683c4c8f..be65b8445d6 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -3,7 +3,6 @@ use std::{any::Any, sync::Arc}; use crate::{ emath::{Align, Pos2, Rect, Vec2}, menu, Context, CursorIcon, Id, LayerId, PointerButton, Sense, Ui, WidgetRect, WidgetText, - NUM_POINTER_BUTTONS, }; // ---------------------------------------------------------------------------- @@ -15,7 +14,10 @@ use crate::{ /// /// Whenever something gets added to a [`Ui`], a [`Response`] object is returned. /// [`ui.add`] returns a [`Response`], as does [`ui.button`], and all similar shortcuts. -// TODO(emilk): we should be using bit sets instead of so many bools +/// +/// ⚠️ The `Response` contains a clone of [`Context`], and many methods lock the `Context`. +/// It can therefor be a deadlock to use `Context` from within a context-locking closures, +/// such as [`Context::input`]. #[derive(Clone, Debug)] pub struct Response { // CONTEXT: @@ -69,18 +71,23 @@ pub struct Response { #[doc(hidden)] pub highlighted: bool, - /// The pointer clicked this thing this frame. - #[doc(hidden)] - pub clicked: [bool; NUM_POINTER_BUTTONS], - - // TODO(emilk): `released` for sliders - /// The thing was double-clicked. + /// This widget was clicked this frame. + /// + /// Which pointer and how many times we don't know, + /// and ask [`crate::InputState`] about at runtime. + /// + /// This is only set to true if the widget was clicked + /// by an actual mouse. #[doc(hidden)] - pub double_clicked: [bool; NUM_POINTER_BUTTONS], + pub clicked: bool, - /// The thing was triple-clicked. + /// This widget should act as if clicked due + /// to something else than a click. + /// + /// This is set to true if the widget has keyboard focus and + /// the user hit the Space or Enter key. #[doc(hidden)] - pub triple_clicked: [bool; NUM_POINTER_BUTTONS], + pub fake_primary_click: bool, /// The widget started being dragged this frame. #[doc(hidden)] @@ -118,55 +125,62 @@ impl Response { /// A click is registered when the mouse or touch is released within /// a certain amount of time and distance from when and where it was pressed. /// + /// This will also return true if the widget was clicked via accessibility integration, + /// or if the widget had keyboard focus and the use pressed Space/Enter. + /// /// Note that the widget must be sensing clicks with [`Sense::click`]. /// [`crate::Button`] senses clicks; [`crate::Label`] does not (unless you call [`crate::Label::sense`]). /// /// You can use [`Self::interact`] to sense more things *after* adding a widget. #[inline(always)] pub fn clicked(&self) -> bool { - self.clicked[PointerButton::Primary as usize] + self.fake_primary_click || self.clicked_by(PointerButton::Primary) } - /// Returns true if this widget was clicked this frame by the given button. + /// Returns true if this widget was clicked this frame by the given mouse button. + /// + /// This will NOT return true if the widget was "clicked" via + /// some accessibility integration, or if the widget had keyboard focus and the + /// user pressed Space/Enter. For that, use [`Self::clicked`] instead. #[inline] pub fn clicked_by(&self, button: PointerButton) -> bool { - self.clicked[button as usize] + self.clicked && self.ctx.input(|i| i.pointer.button_clicked(button)) } /// Returns true if this widget was clicked this frame by the secondary mouse button (e.g. the right mouse button). #[inline] pub fn secondary_clicked(&self) -> bool { - self.clicked[PointerButton::Secondary as usize] + self.clicked_by(PointerButton::Secondary) } /// Returns true if this widget was clicked this frame by the middle mouse button. #[inline] pub fn middle_clicked(&self) -> bool { - self.clicked[PointerButton::Middle as usize] + self.clicked_by(PointerButton::Middle) } /// Returns true if this widget was double-clicked this frame by the primary button. #[inline] pub fn double_clicked(&self) -> bool { - self.double_clicked[PointerButton::Primary as usize] + self.double_clicked_by(PointerButton::Primary) } /// Returns true if this widget was triple-clicked this frame by the primary button. #[inline] pub fn triple_clicked(&self) -> bool { - self.triple_clicked[PointerButton::Primary as usize] + self.triple_clicked_by(PointerButton::Primary) } /// Returns true if this widget was double-clicked this frame by the given button. #[inline] pub fn double_clicked_by(&self, button: PointerButton) -> bool { - self.double_clicked[button as usize] + self.clicked && self.ctx.input(|i| i.pointer.button_double_clicked(button)) } /// Returns true if this widget was triple-clicked this frame by the given button. #[inline] pub fn triple_clicked_by(&self, button: PointerButton) -> bool { - self.triple_clicked[button as usize] + self.clicked && self.ctx.input(|i| i.pointer.button_triple_clicked(button)) } /// `true` if there was a click *outside* this widget this frame. @@ -917,27 +931,8 @@ impl Response { contains_pointer: self.contains_pointer || other.contains_pointer, hovered: self.hovered || other.hovered, highlighted: self.highlighted || other.highlighted, - clicked: [ - self.clicked[0] || other.clicked[0], - self.clicked[1] || other.clicked[1], - self.clicked[2] || other.clicked[2], - self.clicked[3] || other.clicked[3], - self.clicked[4] || other.clicked[4], - ], - double_clicked: [ - self.double_clicked[0] || other.double_clicked[0], - self.double_clicked[1] || other.double_clicked[1], - self.double_clicked[2] || other.double_clicked[2], - self.double_clicked[3] || other.double_clicked[3], - self.double_clicked[4] || other.double_clicked[4], - ], - triple_clicked: [ - self.triple_clicked[0] || other.triple_clicked[0], - self.triple_clicked[1] || other.triple_clicked[1], - self.triple_clicked[2] || other.triple_clicked[2], - self.triple_clicked[3] || other.triple_clicked[3], - self.triple_clicked[4] || other.triple_clicked[4], - ], + clicked: self.clicked || other.clicked, + fake_primary_click: self.fake_primary_click || other.fake_primary_click, drag_started: self.drag_started || other.drag_started, dragged: self.dragged || other.dragged, drag_stopped: self.drag_stopped || other.drag_stopped, diff --git a/crates/egui_demo_lib/src/demo/pan_zoom.rs b/crates/egui_demo_lib/src/demo/pan_zoom.rs index 08829d6d5e6..40323346887 100644 --- a/crates/egui_demo_lib/src/demo/pan_zoom.rs +++ b/crates/egui_demo_lib/src/demo/pan_zoom.rs @@ -11,7 +11,7 @@ impl Eq for PanZoom {} impl super::Demo for PanZoom { fn name(&self) -> &'static str { - "🗖 Pan Zoom" + "🔍 Pan Zoom" } fn show(&mut self, ctx: &egui::Context, open: &mut bool) {