From de8582002fac1ea86ef7b2d0f54eae5db8c1b504 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 11:24:09 +0100 Subject: [PATCH 1/8] Fix bug in `InputState::button_clicked` --- crates/egui/src/input_state.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index ed29282e9508..6da8f364dd39 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -976,11 +976,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? From 1de3ba661f0c5d161ecc759df5d8ade450e97349 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 11:27:21 +0100 Subject: [PATCH 2/8] `Response::clicked_by` will no longer be true if clicking with keyboard --- crates/egui/src/context.rs | 19 ++++------ crates/egui/src/response.rs | 72 +++++++++++++++++-------------------- 2 files changed, 39 insertions(+), 52 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index c1dcd5b5be91..9728281e9b74 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/response.rs b/crates/egui/src/response.rs index ddd683c4c8f6..94b7487b0e67 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, }; // ---------------------------------------------------------------------------- @@ -69,18 +68,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 [`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 +122,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 +928,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, From b9509196401d281430de481af25b0da9e6fbf0af Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 11:28:29 +0100 Subject: [PATCH 3/8] Change the icon of the pan-and-zoom demo --- crates/egui_demo_lib/src/demo/pan_zoom.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui_demo_lib/src/demo/pan_zoom.rs b/crates/egui_demo_lib/src/demo/pan_zoom.rs index 08829d6d5e6b..40323346887e 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) { From 4388fca94662de1a093abfa746c9800376187284 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 12:02:21 +0100 Subject: [PATCH 4/8] Code cleanup regarding how multi-touches are found --- crates/egui/src/input_state.rs | 6 +----- crates/egui/src/input_state/touch_state.rs | 5 +---- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index 6da8f364dd39..7fc29518ad17 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. diff --git a/crates/egui/src/input_state/touch_state.rs b/crates/egui/src/input_state/touch_state.rs index 2ee7a91de755..2a77a4d371b9 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 From 027816366c75b36b578f021bc585753b6a9151ff Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 16:12:25 +0100 Subject: [PATCH 5/8] Add breaking changes warning --- CHANGELOG.md | 7 +++++++ crates/egui/src/response.rs | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c468fc815d0..0a074c6f11e0 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/response.rs b/crates/egui/src/response.rs index 94b7487b0e67..f98b5ce6a13e 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -14,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 withing a context-locking closures, +/// such as [`Context::input`]. #[derive(Clone, Debug)] pub struct Response { // CONTEXT: From f5e3df921002e5c4a750ae3c1d878c78358ff074 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 16:13:39 +0100 Subject: [PATCH 6/8] Fix deadlock when opening context menu --- crates/egui/src/menu.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/egui/src/menu.rs b/crates/egui/src/menu.rs index 1f7a74fbf4d9..6059236787c8 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; } } From ab406b03e0a6595d34836bc1fc65618ab9596d05 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 16:19:06 +0100 Subject: [PATCH 7/8] fix doclink --- crates/egui/src/response.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index f98b5ce6a13e..cac87d6d6dad 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -74,7 +74,7 @@ pub struct Response { /// This widget was clicked this frame. /// /// Which pointer and how many times we don't know, - /// and ask [`InputState`] about at runtime. + /// and ask [`crate::InputState`] about at runtime. /// /// This is only set to true if the widget was clicked /// by an actual mouse. From c4d6b49baca920b9265102d76dc0b4a0930cdb38 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 19 Mar 2024 16:19:32 +0100 Subject: [PATCH 8/8] Fix a typo --- crates/egui/src/response.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index cac87d6d6dad..be65b8445d6c 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -16,7 +16,7 @@ use crate::{ /// [`ui.add`] returns a [`Response`], as does [`ui.button`], and all similar shortcuts. /// /// ⚠️ The `Response` contains a clone of [`Context`], and many methods lock the `Context`. -/// It can therefor be a deadlock to use `Context` from withing a context-locking closures, +/// 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 {