From 00396145d13fb191aa401d4b120df1858e710f8c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 29 May 2024 21:18:08 +0200 Subject: [PATCH] Improve tooltip positioning (#4579) This simplifies and improves the tooltip positioning * Closes https://github.com/emilk/egui/issues/4568 ### For a follow-up PR * [ ] Test if it closes https://github.com/emilk/egui/issues/4471 * [ ] Add an API to close https://github.com/emilk/egui/issues/890 --- crates/egui/src/containers/area.rs | 11 +- crates/egui/src/containers/popup.rs | 287 +++++++++++----------------- crates/egui/src/frame_state.rs | 29 ++- 3 files changed, 136 insertions(+), 191 deletions(-) diff --git a/crates/egui/src/containers/area.rs b/crates/egui/src/containers/area.rs index 9d4ed44de..df9e9db86 100644 --- a/crates/egui/src/containers/area.rs +++ b/crates/egui/src/containers/area.rs @@ -525,20 +525,11 @@ impl Prepared { enabled: _, constrain: _, constrain_rect: _, - sizing_pass, + sizing_pass: _, } = self; state.size = content_ui.min_size(); - if sizing_pass { - // If during the sizing pass we measure our width to `123.45` and - // then try to wrap to exactly that next frame, - // we may accidentally wrap the last letter of some text. - // We only do this after the initial sizing pass though; - // otherwise we could end up with for-ever expanding areas. - state.size = state.size.ceil(); - } - ctx.memory_mut(|m| m.areas_mut().set_state(layer_id, state)); move_response diff --git a/crates/egui/src/containers/popup.rs b/crates/egui/src/containers/popup.rs index f0c828197..ed2a15989 100644 --- a/crates/egui/src/containers/popup.rs +++ b/crates/egui/src/containers/popup.rs @@ -1,52 +1,11 @@ //! Show popup windows, tooltips, context menus etc. +use frame_state::PerWidgetTooltipState; + use crate::*; // ---------------------------------------------------------------------------- -/// Same state for all tooltips. -#[derive(Clone, Debug, Default)] -pub(crate) struct TooltipState { - last_common_id: Option, - individual_ids_and_sizes: ahash::HashMap, -} - -impl TooltipState { - pub fn load(ctx: &Context) -> Option { - ctx.data_mut(|d| d.get_temp(Id::NULL)) - } - - fn store(self, ctx: &Context) { - ctx.data_mut(|d| d.insert_temp(Id::NULL, self)); - } - - fn individual_tooltip_size(&self, common_id: Id, index: usize) -> Option { - if self.last_common_id == Some(common_id) { - Some(self.individual_ids_and_sizes.get(&index)?.1) - } else { - None - } - } - - fn set_individual_tooltip( - &mut self, - common_id: Id, - index: usize, - individual_id: Id, - size: Vec2, - ) { - if self.last_common_id != Some(common_id) { - self.last_common_id = Some(common_id); - self.individual_ids_and_sizes.clear(); - } - - self.individual_ids_and_sizes - .insert(index, (individual_id, size)); - } -} - -// ---------------------------------------------------------------------------- - /// Show a tooltip at the current pointer position (if any). /// /// Most of the time it is easier to use [`Response::on_hover_ui`]. @@ -94,10 +53,8 @@ pub fn show_tooltip_at_pointer( id: Id, add_contents: impl FnOnce(&mut Ui) -> R, ) -> Option { - let suggested_pos = ctx - .input(|i| i.pointer.hover_pos()) - .map(|pointer_pos| pointer_pos + vec2(16.0, 16.0)); - show_tooltip_at(ctx, id, suggested_pos, add_contents) + ctx.input(|i| i.pointer.hover_pos()) + .map(|pointer_pos| show_tooltip_at(ctx, id, pointer_pos + vec2(16.0, 16.0), add_contents)) } /// Show a tooltip under the given area. @@ -106,21 +63,16 @@ pub fn show_tooltip_at_pointer( pub fn show_tooltip_for( ctx: &Context, id: Id, - rect: &Rect, + widget_rect: &Rect, add_contents: impl FnOnce(&mut Ui) -> R, -) -> Option { - let expanded_rect = rect.expand2(vec2(2.0, 4.0)); - let (above, position) = if ctx.input(|i| i.any_touches()) { - (true, expanded_rect.left_top()) - } else { - (false, expanded_rect.left_bottom()) - }; +) -> R { + let is_touch_screen = ctx.input(|i| i.any_touches()); + let allow_placing_below = !is_touch_screen; // There is a finger below. show_tooltip_at_avoid_dyn( ctx, id, - Some(position), - above, - expanded_rect, + allow_placing_below, + widget_rect, Box::new(add_contents), ) } @@ -131,101 +83,119 @@ pub fn show_tooltip_for( pub fn show_tooltip_at( ctx: &Context, id: Id, - suggested_position: Option, + suggested_position: Pos2, add_contents: impl FnOnce(&mut Ui) -> R, -) -> Option { - let above = false; - show_tooltip_at_avoid_dyn( - ctx, - id, - suggested_position, - above, - Rect::NOTHING, - Box::new(add_contents), - ) +) -> R { + let allow_placing_below = true; + let rect = Rect::from_center_size(suggested_position, Vec2::ZERO); + show_tooltip_at_avoid_dyn(ctx, id, allow_placing_below, &rect, Box::new(add_contents)) } fn show_tooltip_at_avoid_dyn<'c, R>( ctx: &Context, - individual_id: Id, - suggested_position: Option, - above: bool, - mut avoid_rect: Rect, + widget_id: Id, + allow_placing_below: bool, + widget_rect: &Rect, add_contents: Box R + 'c>, -) -> Option { +) -> R { + // if there are multiple tooltips open they should use the same common_id for the `tooltip_size` caching to work. + let mut state = ctx.frame_state(|fs| { + fs.tooltip_state + .widget_tooltips + .get(&widget_id) + .copied() + .unwrap_or(PerWidgetTooltipState { + bounding_rect: *widget_rect, + tooltip_count: 0, + }) + }); + + let tooltip_area_id = tooltip_id(widget_id, state.tooltip_count); + let expected_tooltip_size = + AreaState::load(ctx, tooltip_area_id).map_or(vec2(64.0, 32.0), |area| area.size); + + let screen_rect = ctx.screen_rect(); + + let (pivot, anchor) = find_tooltip_position( + screen_rect, + state.bounding_rect, + allow_placing_below, + expected_tooltip_size, + ); + + let InnerResponse { inner, response } = Area::new(tooltip_area_id) + .order(Order::Tooltip) + .pivot(pivot) + .fixed_pos(anchor) + .default_width(ctx.style().spacing.tooltip_width) + .constrain_to(screen_rect) + .interactable(false) + .show(ctx, |ui| { + Frame::popup(&ctx.style()).show_dyn(ui, add_contents).inner + }); + + state.tooltip_count += 1; + state.bounding_rect = state.bounding_rect.union(response.rect); + ctx.frame_state_mut(|fs| fs.tooltip_state.widget_tooltips.insert(widget_id, state)); + + inner +} + +fn tooltip_id(widget_id: Id, tooltip_count: usize) -> Id { + widget_id.with(tooltip_count) +} + +/// Returns `(PIVOT, POS)` to mean: put the `PIVOT` corner of the tooltip at `POS`. +/// +/// Note: the position might need to be constrained to the screen, +/// (e.g. moved sideways if shown under the widget) +/// but the `Area` will take care of that. +fn find_tooltip_position( + screen_rect: Rect, + widget_rect: Rect, + allow_placing_below: bool, + tooltip_size: Vec2, +) -> (Align2, Pos2) { let spacing = 4.0; - // if there are multiple tooltips open they should use the same common_id for the `tooltip_size` caching to work. - let mut frame_state = - ctx.frame_state(|fs| fs.tooltip_state) - .unwrap_or(crate::frame_state::TooltipFrameState { - common_id: individual_id, - rect: Rect::NOTHING, - count: 0, - }); - - let mut position = if frame_state.rect.is_positive() { - avoid_rect = avoid_rect.union(frame_state.rect); - if above { - frame_state.rect.left_top() - spacing * Vec2::Y - } else { - frame_state.rect.left_bottom() + spacing * Vec2::Y - } - } else if let Some(position) = suggested_position { - position - } else if ctx.memory(|mem| mem.everything_is_visible()) { - Pos2::ZERO - } else { - return None; // No good place for a tooltip :( - }; - - let mut long_state = TooltipState::load(ctx).unwrap_or_default(); - let expected_size = - long_state.individual_tooltip_size(frame_state.common_id, frame_state.count); - let expected_size = expected_size.unwrap_or_else(|| vec2(64.0, 32.0)); - - if above { - position.y -= expected_size.y; - } - - position = position.at_most(ctx.screen_rect().max - expected_size); - - // check if we intersect the avoid_rect + // Does it fit below? + if allow_placing_below + && widget_rect.bottom() + spacing + tooltip_size.y <= screen_rect.bottom() { - let new_rect = Rect::from_min_size(position, expected_size); - - // Note: We use shrink so that we don't get false positives when the rects just touch - if new_rect.shrink(1.0).intersects(avoid_rect) { - if above { - // place below instead: - position = avoid_rect.left_bottom() + spacing * Vec2::Y; - } else { - // place above instead: - position = Pos2::new(position.x, avoid_rect.min.y - expected_size.y - spacing); - } - } + return ( + Align2::LEFT_TOP, + widget_rect.left_bottom() + spacing * Vec2::DOWN, + ); } - let position = position.at_least(ctx.screen_rect().min); + // Does it fit above? + if screen_rect.top() + tooltip_size.y + spacing <= widget_rect.top() { + return ( + Align2::LEFT_BOTTOM, + widget_rect.left_top() + spacing * Vec2::UP, + ); + } - let area_id = frame_state.common_id.with(frame_state.count); + // Does it fit to the right? + if widget_rect.right() + spacing + tooltip_size.x <= screen_rect.right() { + return ( + Align2::LEFT_TOP, + widget_rect.right_top() + spacing * Vec2::RIGHT, + ); + } - let InnerResponse { inner, response } = - show_tooltip_area_dyn(ctx, area_id, position, add_contents); + // Does it fit to the left? + if screen_rect.left() + tooltip_size.x + spacing <= widget_rect.left() { + return ( + Align2::RIGHT_TOP, + widget_rect.left_top() + spacing * Vec2::LEFT, + ); + } - long_state.set_individual_tooltip( - frame_state.common_id, - frame_state.count, - individual_id, - response.rect.size(), - ); - long_state.store(ctx); + // It doesn't fit anywhere :( - frame_state.count += 1; - frame_state.rect = frame_state.rect.union(response.rect); - ctx.frame_state_mut(|fs| fs.tooltip_state = Some(frame_state)); - - Some(inner) + // Just show it anyway: + (Align2::LEFT_TOP, screen_rect.left_top()) } /// Show some text at the current pointer position (if any). @@ -249,42 +219,13 @@ pub fn show_tooltip_text(ctx: &Context, id: Id, text: impl Into) -> }) } -/// Show a pop-over window. -fn show_tooltip_area_dyn<'c, R>( - ctx: &Context, - area_id: Id, - window_pos: Pos2, - add_contents: Box R + 'c>, -) -> InnerResponse { - use containers::*; - Area::new(area_id) - .order(Order::Tooltip) - .fixed_pos(window_pos) - .default_width(ctx.style().spacing.tooltip_width) - .constrain_to(ctx.screen_rect()) - .interactable(false) - .show(ctx, |ui| { - Frame::popup(&ctx.style()).show_dyn(ui, add_contents).inner - }) -} - /// Was this popup visible last frame? -pub fn was_tooltip_open_last_frame(ctx: &Context, tooltip_id: Id) -> bool { - if let Some(state) = TooltipState::load(ctx) { - if let Some(common_id) = state.last_common_id { - for (count, (individual_id, _size)) in &state.individual_ids_and_sizes { - if *individual_id == tooltip_id { - let area_id = common_id.with(count); - let layer_id = LayerId::new(Order::Tooltip, area_id); - if ctx.memory(|mem| mem.areas().visible_last_frame(&layer_id)) { - return true; - } - } - } - } - } - - false +pub fn was_tooltip_open_last_frame(ctx: &Context, widget_id: Id) -> bool { + let primary_tooltip_area_id = tooltip_id(widget_id, 0); + ctx.memory(|mem| { + mem.areas() + .visible_last_frame(&LayerId::new(Order::Tooltip, primary_tooltip_area_id)) + }) } /// Helper for [`popup_above_or_below_widget`]. diff --git a/crates/egui/src/frame_state.rs b/crates/egui/src/frame_state.rs index e1414bba1..28c8c364b 100644 --- a/crates/egui/src/frame_state.rs +++ b/crates/egui/src/frame_state.rs @@ -1,10 +1,23 @@ use crate::{id::IdSet, *}; -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Debug, Default)] pub(crate) struct TooltipFrameState { - pub common_id: Id, - pub rect: Rect, - pub count: usize, + pub widget_tooltips: IdMap, +} + +impl TooltipFrameState { + pub(crate) fn clear(&mut self) { + self.widget_tooltips.clear(); + } +} + +#[derive(Clone, Copy, Debug)] +pub(crate) struct PerWidgetTooltipState { + /// Bounding rectangle for all widget and all previous tooltips. + pub bounding_rect: Rect, + + /// How many tooltips have been shown for this widget this frame? + pub tooltip_count: usize, } #[cfg(feature = "accesskit")] @@ -35,8 +48,8 @@ pub(crate) struct FrameState { /// If a tooltip has been shown this frame, where was it? /// This is used to prevent multiple tooltips to cover each other. - /// Initialized to `None` at the start of each frame. - pub(crate) tooltip_state: Option, + /// Reset at the start of each frame. + pub(crate) tooltip_state: TooltipFrameState, /// The current scroll area should scroll to this range (horizontal, vertical). pub(crate) scroll_target: [Option<(Rangef, Option)>; 2], @@ -72,7 +85,7 @@ impl Default for FrameState { available_rect: Rect::NAN, unused_rect: Rect::NAN, used_by_panels: Rect::NAN, - tooltip_state: None, + tooltip_state: Default::default(), scroll_target: [None, None], scroll_delta: Vec2::default(), #[cfg(feature = "accesskit")] @@ -110,7 +123,7 @@ impl FrameState { *available_rect = screen_rect; *unused_rect = screen_rect; *used_by_panels = Rect::NOTHING; - *tooltip_state = None; + tooltip_state.clear(); *scroll_target = [None, None]; *scroll_delta = Vec2::default();