From ca6ce2dd9264cb6821ec07dc7e71074ca936a6d2 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 22 Jan 2024 14:37:53 +0100 Subject: [PATCH] Always set `response.hovered` to false when dragging another widget (#3860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Closes https://github.com/emilk/egui/issues/3841 ⚠️ Breaking change! This defines `response.hovered()` to never be true if any other widget is being dragged. If you want to detect if a widget is being hovered while another widget is being dragged, use the new `response.contains_pointer` instead. This is necessaert for things like drag-and-drop targets. Overall, this removes special-casing of non-interactive widgets. --- crates/egui/src/context.rs | 247 +++++++++++++++++++++--------------- crates/egui/src/response.rs | 42 ++++-- crates/egui/src/sense.rs | 14 +- crates/egui/src/ui.rs | 17 ++- 4 files changed, 205 insertions(+), 115 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 9382dad035e..9b7b5220487 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -318,7 +318,13 @@ impl ContextImpl { .native_pixels_per_point .unwrap_or(1.0); - viewport.layer_rects_prev_frame = std::mem::take(&mut viewport.layer_rects_this_frame); + { + std::mem::swap( + &mut viewport.layer_rects_prev_frame, + &mut viewport.layer_rects_this_frame, + ); + viewport.layer_rects_this_frame.clear(); + } let all_viewport_ids: ViewportIdSet = self.all_viewport_ids(); @@ -865,91 +871,25 @@ impl Context { .at_most(Vec2::splat(5.0)), ); - // Respect clip rectangle when interacting + // Respect clip rectangle when interacting: let interact_rect = clip_rect.intersect(interact_rect); - let mut hovered = self.rect_contains_pointer(layer_id, interact_rect); - - // This solves the problem of overlapping widgets. - // Whichever widget is added LAST (=on top) gets the input: - if interact_rect.is_positive() && sense.interactive() { - #[cfg(debug_assertions)] - if self.style().debug.show_interactive_widgets { - Self::layer_painter(self, LayerId::debug()).rect( - interact_rect, - 0.0, - Color32::YELLOW.additive().linear_multiply(0.005), - Stroke::new(1.0, Color32::YELLOW.additive().linear_multiply(0.05)), - ); - } - #[cfg(debug_assertions)] - let mut show_blocking_widget = None; + let contains_pointer = self.widget_contains_pointer(layer_id, id, sense, interact_rect); - self.write(|ctx| { - let viewport = ctx.viewport(); - - viewport - .layer_rects_this_frame - .entry(layer_id) - .or_default() - .push(WidgetRect { - id, - rect: interact_rect, - sense, - }); - - if hovered { - let pointer_pos = viewport.input.pointer.interact_pos(); - if let Some(pointer_pos) = pointer_pos { - if let Some(rects) = viewport.layer_rects_prev_frame.get(&layer_id) { - for &WidgetRect { - id: prev_id, - rect: prev_rect, - sense: prev_sense, - } in rects.iter().rev() - { - if prev_id == id { - break; // there is no other interactive widget covering us at the pointer position. - } - // We don't want a click-only button to block drag-events to a `ScrollArea`: - let has_conflicting_sense = (prev_sense.click && sense.click) - || (prev_sense.drag && sense.drag); - if prev_rect.contains(pointer_pos) && has_conflicting_sense { - // Another interactive widget is covering us at the pointer position, - // so we aren't hovered. - - #[cfg(debug_assertions)] - if ctx.memory.options.style.debug.show_blocking_widget { - // Store the rects to use them outside the write() call to - // avoid deadlock - show_blocking_widget = Some((interact_rect, prev_rect)); - } - - hovered = false; - break; - } - } - } - } - } - }); - - #[cfg(debug_assertions)] - if let Some((interact_rect, prev_rect)) = show_blocking_widget { - Self::layer_painter(self, LayerId::debug()).debug_rect( - interact_rect, - Color32::GREEN, - "Covered", - ); - Self::layer_painter(self, LayerId::debug()).debug_rect( - prev_rect, - Color32::LIGHT_BLUE, - "On top", - ); - } + #[cfg(debug_assertions)] + if sense.interactive() + && interact_rect.is_positive() + && self.style().debug.show_interactive_widgets + { + Self::layer_painter(self, LayerId::debug()).rect( + interact_rect, + 0.0, + Color32::YELLOW.additive().linear_multiply(0.005), + Stroke::new(1.0, Color32::YELLOW.additive().linear_multiply(0.05)), + ); } - self.interact_with_hovered(layer_id, id, rect, sense, enabled, hovered) + self.interact_with_hovered(layer_id, id, rect, sense, enabled, contains_pointer) } /// You specify if a thing is hovered, and the function gives a [`Response`]. @@ -960,9 +900,9 @@ impl Context { rect: Rect, sense: Sense, enabled: bool, - hovered: bool, + contains_pointer: bool, ) -> Response { - let hovered = hovered && enabled; // can't even hover disabled widgets + let hovered = contains_pointer && enabled; // can't even hover disabled widgets let highlighted = self.frame_state(|fs| fs.highlight_this_frame.contains(&id)); @@ -973,6 +913,7 @@ impl Context { rect, sense, enabled, + contains_pointer, hovered, highlighted, clicked: Default::default(), @@ -988,10 +929,11 @@ impl Context { if !enabled || !sense.focusable || !layer_id.allow_interaction() { // Not interested or allowed input: self.memory_mut(|mem| mem.surrender_focus(id)); - return response; } - self.check_for_id_clash(id, rect, "widget"); + if sense.interactive() || sense.focusable { + self.check_for_id_clash(id, rect, "widget"); + } #[cfg(feature = "accesskit")] if sense.focusable { @@ -1019,12 +961,10 @@ impl Context { } #[cfg(feature = "accesskit")] + if sense.click + && input.has_accesskit_action_request(response.id, accesskit::Action::Default) { - if sense.click - && input.has_accesskit_action_request(response.id, accesskit::Action::Default) - { - response.clicked[PointerButton::Primary as usize] = true; - } + response.clicked[PointerButton::Primary as usize] = true; } if sense.click || sense.drag { @@ -1091,8 +1031,9 @@ impl Context { response.interact_pointer_pos = input.pointer.interact_pos(); } - if input.pointer.any_down() { - response.hovered &= response.is_pointer_button_down_on; // we don't hover widgets while interacting with *other* widgets + if input.pointer.any_down() && !response.is_pointer_button_down_on { + // We don't hover widgets while interacting with *other* widgets: + response.hovered = false; } if memory.has_focus(response.id) && clicked_elsewhere { @@ -2028,15 +1969,123 @@ impl Context { self.memory(|mem| mem.areas().top_layer_id(Order::Middle)) } - pub(crate) fn rect_contains_pointer(&self, layer_id: LayerId, rect: Rect) -> bool { - rect.is_positive() && { - let pointer_pos = self.input(|i| i.pointer.interact_pos()); - if let Some(pointer_pos) = pointer_pos { - rect.contains(pointer_pos) && self.layer_id_at(pointer_pos) == Some(layer_id) - } else { - false + /// Does the given rectangle contain the mouse pointer? + /// + /// Will return false if some other area is covering the given layer. + /// + /// See also [`Response::contains_pointer`]. + pub fn rect_contains_pointer(&self, layer_id: LayerId, rect: Rect) -> bool { + if !rect.is_positive() { + return false; + } + + let pointer_pos = self.input(|i| i.pointer.interact_pos()); + let Some(pointer_pos) = pointer_pos else { + return false; + }; + + if !rect.contains(pointer_pos) { + return false; + } + + if self.layer_id_at(pointer_pos) != Some(layer_id) { + return false; + } + + true + } + + /// Does the given widget contain the mouse pointer? + /// + /// Will return false if some other area is covering the given layer. + /// + /// If another widget is covering us and is listening for the same input (click and/or drag), + /// this will return false. + /// + /// See also [`Response::contains_pointer`]. + pub fn widget_contains_pointer( + &self, + layer_id: LayerId, + id: Id, + sense: Sense, + rect: Rect, + ) -> bool { + let contains_pointer = self.rect_contains_pointer(layer_id, rect); + + let mut blocking_widget = None; + + self.write(|ctx| { + let viewport = ctx.viewport(); + + // We add all widgets here, even non-interactive ones, + // because we need this list not only for checking for blocking widgets, + // but also to know when we have reach the widget we are checking for cover. + viewport + .layer_rects_this_frame + .entry(layer_id) + .or_default() + .push(WidgetRect { id, rect, sense }); + + // Check if any other widget is covering us. + // Whichever widget is added LAST (=on top) gets the input. + if contains_pointer { + let pointer_pos = viewport.input.pointer.interact_pos(); + if let Some(pointer_pos) = pointer_pos { + if let Some(rects) = viewport.layer_rects_prev_frame.get(&layer_id) { + for blocking in rects.iter().rev() { + if blocking.id == id { + // There are no earlier widgets before this one, + // which means there are no widgets covering us. + break; + } + if !blocking.rect.contains(pointer_pos) { + continue; + } + if sense.interactive() && !blocking.sense.interactive() { + // Only interactive widgets can block other interactive widgets. + continue; + } + + // The `prev` widget is covering us - do we care? + // We don't want a click-only button to block drag-events to a `ScrollArea`: + + let sense_only_drags = sense.drag && !sense.click; + if sense_only_drags && !blocking.sense.drag { + continue; + } + let sense_only_clicks = sense.click && !sense.drag; + if sense_only_clicks && !blocking.sense.click { + continue; + } + + if blocking.sense.interactive() { + // Another widget is covering us at the pointer position + blocking_widget = Some(blocking.rect); + break; + } + } + } + } + } + }); + + #[cfg(debug_assertions)] + if let Some(blocking_rect) = blocking_widget { + if sense.interactive() && self.memory(|m| m.options.style.debug.show_blocking_widget) { + Self::layer_painter(self, LayerId::debug()).debug_rect( + rect, + Color32::GREEN, + "Covered", + ); + Self::layer_painter(self, LayerId::debug()).debug_rect( + blocking_rect, + Color32::LIGHT_BLUE, + "On top", + ); } } + + contains_pointer && blocking_widget.is_none() } // --------------------------------------------------------------------- diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 605cf95f66a..02d908d9723 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -39,6 +39,10 @@ pub struct Response { pub enabled: bool, // OUT: + /// The pointer is hovering above this widget. + #[doc(hidden)] + pub contains_pointer: bool, + /// The pointer is hovering above this widget or the widget was clicked/tapped this frame. #[doc(hidden)] pub hovered: bool, @@ -95,6 +99,7 @@ impl std::fmt::Debug for Response { rect, sense, enabled, + contains_pointer, hovered, highlighted, clicked, @@ -112,6 +117,7 @@ impl std::fmt::Debug for Response { .field("rect", rect) .field("sense", sense) .field("enabled", enabled) + .field("contains_pointer", contains_pointer) .field("hovered", hovered) .field("highlighted", highlighted) .field("clicked", clicked) @@ -142,36 +148,43 @@ impl Response { } /// Returns true if this widget was clicked this frame by the given button. + #[inline] pub fn clicked_by(&self, button: PointerButton) -> bool { self.clicked[button as usize] } /// 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] } /// 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] } /// 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] } /// 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] } /// 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] } /// 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] } @@ -212,12 +225,8 @@ impl Response { /// The pointer is hovering above this widget or the widget was clicked/tapped this frame. /// - /// This will be `false` whenever some other widget is being dragged. - /// - /// Note that this is slightly different from [`Self::contains_pointer`]. - /// For one, the hover rectangle is slightly larger, by half of the current item spacing - /// (to make it easier to click things). But `hovered` also checks that no other area - /// is covering this response rectangle. + /// In contrast to [`Self::contains_pointer`], this will be `false` whenever some other widget is being dragged. + /// `hovered` is always `false` for disabled widgets. #[inline(always)] pub fn hovered(&self) -> bool { self.hovered @@ -227,15 +236,19 @@ impl Response { /// /// In contrast to [`Self::hovered`], this can be `true` even if some other widget is being dragged. /// This means it is useful for styling things like drag-and-drop targets. + /// `contains_pointer` can also be `true` for disabled widgets. /// - /// In contrast to [`Self::hovered`], this is true even when dragging some other widget - /// onto this one. + /// This is slightly different from [`Ui::rect_contains_pointer`] and [`Context::rect_contains_pointer`], + /// The rectangle used here is slightly larger, by half of the current item spacing. + /// [`Self::contains_pointer`] also checks that no other widget is covering this response rectangle. + #[inline(always)] pub fn contains_pointer(&self) -> bool { - self.ctx.rect_contains_pointer(self.layer_id, self.rect) + self.contains_pointer } /// The widget is highlighted via a call to [`Self::highlight`] or [`Context::highlight_widget`]. #[doc(hidden)] + #[inline(always)] pub fn highlighted(&self) -> bool { self.highlighted } @@ -296,21 +309,25 @@ impl Response { self.dragged } + #[inline] pub fn dragged_by(&self, button: PointerButton) -> bool { self.dragged() && self.ctx.input(|i| i.pointer.button_down(button)) } /// Did a drag on this widgets begin this frame? + #[inline] pub fn drag_started(&self) -> bool { self.dragged && self.ctx.input(|i| i.pointer.any_pressed()) } /// Did a drag on this widgets by the button begin this frame? + #[inline] pub fn drag_started_by(&self, button: PointerButton) -> bool { self.drag_started() && self.ctx.input(|i| i.pointer.button_pressed(button)) } /// The widget was being dragged, but now it has been released. + #[inline] pub fn drag_released(&self) -> bool { self.drag_released } @@ -321,6 +338,7 @@ impl Response { } /// If dragged, how many points were we dragged and in what direction? + #[inline] pub fn drag_delta(&self) -> Vec2 { if self.dragged() { self.ctx.input(|i| i.pointer.delta()) @@ -331,12 +349,14 @@ impl Response { /// Where the pointer (mouse/touch) were when when this widget was clicked or dragged. /// `None` if the widget is not being interacted with. + #[inline] pub fn interact_pointer_pos(&self) -> Option { self.interact_pointer_pos } /// If it is a good idea to show a tooltip, where is pointer? /// None if the pointer is outside the response area. + #[inline] pub fn hover_pos(&self) -> Option { if self.hovered() { self.ctx.input(|i| i.pointer.hover_pos()) @@ -510,6 +530,7 @@ impl Response { } /// When hovered, use this icon for the mouse cursor. + #[inline] pub fn on_hover_cursor(self, cursor: CursorIcon) -> Self { if self.hovered() { self.ctx.set_cursor_icon(cursor); @@ -518,6 +539,7 @@ impl Response { } /// When hovered or dragged, use this icon for the mouse cursor. + #[inline] pub fn on_hover_and_drag_cursor(self, cursor: CursorIcon) -> Self { if self.hovered() || self.dragged() { self.ctx.set_cursor_icon(cursor); @@ -739,6 +761,7 @@ impl Response { rect: self.rect.union(other.rect), sense: self.sense.union(other.sense), enabled: self.enabled || other.enabled, + contains_pointer: self.contains_pointer || other.contains_pointer, hovered: self.hovered || other.hovered, highlighted: self.highlighted || other.highlighted, clicked: [ @@ -774,6 +797,7 @@ impl Response { impl Response { /// Returns a response with a modified [`Self::rect`]. + #[inline] pub fn with_new_rect(self, rect: Rect) -> Self { Self { rect, ..self } } diff --git a/crates/egui/src/sense.rs b/crates/egui/src/sense.rs index 06c3a2b06c0..236437d8873 100644 --- a/crates/egui/src/sense.rs +++ b/crates/egui/src/sense.rs @@ -2,13 +2,14 @@ #[derive(Clone, Copy, Debug, Eq, PartialEq)] // #[cfg_attr(feature = "serde", derive(serde::Serialize))] pub struct Sense { - /// buttons, sliders, windows, … + /// Buttons, sliders, windows, … pub click: bool, - /// sliders, windows, scroll bars, scroll areas, … + /// Sliders, windows, scroll bars, scroll areas, … pub drag: bool, - /// this widgets want focus. + /// This widget wants focus. + /// /// Anything interactive + labels that can be focused /// for the benefit of screen readers. pub focusable: bool, @@ -17,6 +18,7 @@ pub struct Sense { impl Sense { /// Senses no clicks or drags. Only senses mouse hover. #[doc(alias = "none")] + #[inline] pub fn hover() -> Self { Self { click: false, @@ -27,6 +29,7 @@ impl Sense { /// Senses no clicks or drags, but can be focused with the keyboard. /// Used for labels that can be focused for the benefit of screen readers. + #[inline] pub fn focusable_noninteractive() -> Self { Self { click: false, @@ -36,6 +39,7 @@ impl Sense { } /// Sense clicks and hover, but not drags. + #[inline] pub fn click() -> Self { Self { click: true, @@ -45,6 +49,7 @@ impl Sense { } /// Sense drags and hover, but not clicks. + #[inline] pub fn drag() -> Self { Self { click: false, @@ -54,6 +59,7 @@ impl Sense { } /// Sense both clicks, drags and hover (e.g. a slider or window). + #[inline] pub fn click_and_drag() -> Self { Self { click: true, @@ -64,6 +70,7 @@ impl Sense { /// The logical "or" of two [`Sense`]s. #[must_use] + #[inline] pub fn union(self, other: Self) -> Self { Self { click: self.click | other.click, @@ -73,6 +80,7 @@ impl Sense { } /// Returns true if we sense either clicks or drags. + #[inline] pub fn interactive(&self) -> bool { self.click || self.drag } diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index cbf352fb363..cd255505b99 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -640,23 +640,32 @@ impl Ui { /// Check for clicks, and drags on a specific region that is hovered. /// This can be used once you have checked that some shape you are painting has been hovered, /// and want to check for clicks and drags on hovered items this frame. + /// /// The given [`Rect`] should approximately be where the thing is, - /// as it is just where warnings will be painted if there is an [`Id`] clash. + /// as will be the rectangle for the returned [`Response::rect`]. pub fn interact_with_hovered( &self, rect: Rect, - hovered: bool, + contains_pointer: bool, id: Id, sense: Sense, ) -> Response { - self.ctx() - .interact_with_hovered(self.layer_id(), id, rect, sense, self.enabled, hovered) + self.ctx().interact_with_hovered( + self.layer_id(), + id, + rect, + sense, + self.enabled, + contains_pointer, + ) } /// Is the pointer (mouse/touch) above this rectangle in this [`Ui`]? /// /// The `clip_rect` and layer of this [`Ui`] will be respected, so, for instance, /// if this [`Ui`] is behind some other window, this will always return `false`. + /// + /// However, this will NOT check if any other _widget_ in the same layer is covering this widget. For that, use [`Response::contains_pointer`] instead. pub fn rect_contains_pointer(&self, rect: Rect) -> bool { self.ctx() .rect_contains_pointer(self.layer_id(), self.clip_rect().intersect(rect))