diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 8ab83c21935..322007113ba 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -498,19 +498,8 @@ impl ContextImpl { viewport.this_pass.begin_pass(screen_rect); { - let area_order = self.memory.areas().order_map(); - let mut layers: Vec = viewport.prev_pass.widgets.layer_ids().collect(); - - layers.sort_by(|a, b| { - if a.order == b.order { - // Maybe both are windows, so respect area order: - area_order.get(a).cmp(&area_order.get(b)) - } else { - // comparing e.g. background to tooltips - a.order.cmp(&b.order) - } - }); + layers.sort_by(|&a, &b| self.memory.areas().compare_order(a, b)); viewport.hits = if let Some(pos) = viewport.input.pointer.interact_pos() { let interact_radius = self.memory.options.style().interaction.interact_radius; @@ -2248,7 +2237,8 @@ impl Context { for id in contains_pointer { let mut widget_text = format!("{id:?}"); if let Some(rect) = widget_rects.get(id) { - widget_text += &format!(" {:?} {:?}", rect.rect, rect.sense); + widget_text += + &format!(" {:?} {:?} {:?}", rect.layer_id, rect.rect, rect.sense); } if let Some(info) = widget_rects.info(id) { widget_text += &format!(" {info:?}"); @@ -2274,11 +2264,17 @@ impl Context { if self.style().debug.show_widget_hits { let hits = self.write(|ctx| ctx.viewport().hits.clone()); let WidgetHits { + close, contains_pointer, click, drag, } = hits; + if false { + for widget in &close { + paint_widget(widget, "close", Color32::from_gray(70)); + } + } if true { for widget in &contains_pointer { paint_widget(widget, "contains_pointer", Color32::BLUE); @@ -3161,28 +3157,26 @@ impl Context { self.memory_mut(|mem| *mem.areas_mut() = Default::default()); } }); - ui.indent("areas", |ui| { - ui.label("Visible areas, ordered back to front."); - ui.label("Hover to highlight"); + ui.indent("layers", |ui| { + ui.label("Layers, ordered back to front."); let layers_ids: Vec = self.memory(|mem| mem.areas().order().to_vec()); for layer_id in layers_ids { - let area = AreaState::load(self, layer_id.id); - if let Some(area) = area { + if let Some(area) = AreaState::load(self, layer_id.id) { let is_visible = self.memory(|mem| mem.areas().is_visible(&layer_id)); if !is_visible { continue; } let text = format!("{} - {:?}", layer_id.short_debug_format(), area.rect(),); // TODO(emilk): `Sense::hover_highlight()` - if ui - .add(Label::new(RichText::new(text).monospace()).sense(Sense::click())) - .hovered - && is_visible - { + let response = + ui.add(Label::new(RichText::new(text).monospace()).sense(Sense::click())); + if response.hovered && is_visible { ui.ctx() .debug_painter() .debug_rect(area.rect(), Color32::RED, ""); } + } else { + ui.monospace(layer_id.short_debug_format()); } } }); diff --git a/crates/egui/src/hit_test.rs b/crates/egui/src/hit_test.rs index 0d85621220e..8741f5f8b41 100644 --- a/crates/egui/src/hit_test.rs +++ b/crates/egui/src/hit_test.rs @@ -2,7 +2,7 @@ use ahash::HashMap; use emath::TSTransform; -use crate::{ahash, emath, LayerId, Pos2, WidgetRect, WidgetRects}; +use crate::{ahash, emath, LayerId, Pos2, Rect, WidgetRect, WidgetRects}; /// Result of a hit-test against [`WidgetRects`]. /// @@ -12,11 +12,18 @@ use crate::{ahash, emath, LayerId, Pos2, WidgetRect, WidgetRects}; /// or if we're currently already dragging something. #[derive(Clone, Debug, Default)] pub struct WidgetHits { + /// All widgets close to the pointer, back-to-front. + /// + /// This is a superset of all other widgets in this struct. + pub close: Vec, + /// All widgets that contains the pointer, back-to-front. /// - /// i.e. both a Window and the button in it can contain the pointer. + /// i.e. both a Window and the Button in it can contain the pointer. /// /// Some of these may be widgets in a layer below the top-most layer. + /// + /// This will be used for hovering. pub contains_pointer: Vec, /// If the user would start a clicking now, this is what would be clicked. @@ -63,6 +70,7 @@ pub fn hit_test( } let pos_in_layer = pos_in_layers.get(&w.layer_id).copied().unwrap_or(pos); + // TODO(emilk): we should probably do the distance testing in global space instead let dist_sq = w.interact_rect.distance_sq_to_pos(pos_in_layer); // In tie, pick last = topmost. @@ -76,51 +84,103 @@ pub fn hit_test( .copied() .collect(); - // We need to pick one single layer for the interaction. - if let Some(closest_hit) = closest_hit { - // Select the top layer, and ignore widgets in any other layer: - let top_layer = closest_hit.layer_id; - close.retain(|w| w.layer_id == top_layer); - - // If the widget is disabled, treat it as if it isn't sensing anything. - // This simplifies the code in `hit_test_on_close` so it doesn't have to check - // the `enabled` flag everywhere: - for w in &mut close { - if !w.enabled { - w.sense.click = false; - w.sense.drag = false; - } + // Transform to global coordinates: + for hit in &mut close { + if let Some(to_global) = layer_to_global.get(&hit.layer_id).copied() { + *hit = hit.transform(to_global); } + } - let pos_in_layer = pos_in_layers.get(&top_layer).copied().unwrap_or(pos); - let hits = hit_test_on_close(&close, pos_in_layer); - - if let Some(drag) = hits.drag { - debug_assert!(drag.sense.drag); - } - if let Some(click) = hits.click { - debug_assert!(click.sense.click); + // When using layer transforms it is common to stack layers close to each other. + // For instance, you may have a resize-separator on a panel, with two + // transform-layers on either side. + // The resize-separator is technically in a layer _behind_ the transform-layers, + // but the user doesn't perceive it as such. + // So how do we handle this case? + // + // If we just allow interactions with ALL close widgets, + // then we might accidentally allow clicks through windows and other bad stuff. + // + // Let's try this: + // * Set up a hit-area (based on search_radius) + // * Iterate over all hits top-to-bottom + // * Stop if any hit covers the whole hit-area, otherwise keep going + // * Collect the layers ids in a set + // * Remove all widgets not in the above layer set + // + // This will most often result in only one layer, + // but if the pointer is at the edge of a layer, we might include widgets in + // a layer behind it. + + let mut included_layers: ahash::HashSet = Default::default(); + for hit in close.iter().rev() { + included_layers.insert(hit.layer_id); + let hit_covers_search_area = contains_circle(hit.interact_rect, pos, search_radius); + if hit_covers_search_area { + break; // nothing behind this layer could ever be interacted with } + } + + close.retain(|hit| included_layers.contains(&hit.layer_id)); - hits - } else { - // No close widgets. - Default::default() + // If a widget is disabled, treat it as if it isn't sensing anything. + // This simplifies the code in `hit_test_on_close` so it doesn't have to check + // the `enabled` flag everywhere: + for w in &mut close { + if !w.enabled { + w.sense.click = false; + w.sense.drag = false; + } } -} -fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits { - #![allow(clippy::collapsible_else_if)] + let mut hits = hit_test_on_close(&close, pos); - // Only those widgets directly under the `pos`. - let hits: Vec = close + hits.contains_pointer = close .iter() .filter(|widget| widget.interact_rect.contains(pos)) .copied() .collect(); - let hit_click = hits.iter().copied().filter(|w| w.sense.click).last(); - let hit_drag = hits.iter().copied().filter(|w| w.sense.drag).last(); + hits.close = close; + + { + // Undo the to_global-transform we applied earlier, + // go back to local layer-coordinates: + + let restore_widget_rect = |w: &mut WidgetRect| { + *w = widgets.get(w.id).copied().unwrap_or(*w); + }; + + for wr in &mut hits.close { + restore_widget_rect(wr); + } + for wr in &mut hits.contains_pointer { + restore_widget_rect(wr); + } + if let Some(wr) = &mut hits.drag { + debug_assert!(wr.sense.drag); + restore_widget_rect(wr); + } + if let Some(wr) = &mut hits.click { + debug_assert!(wr.sense.click); + restore_widget_rect(wr); + } + } + + hits +} + +/// Returns true if the rectangle contains the whole circle. +fn contains_circle(interact_rect: emath::Rect, pos: Pos2, radius: f32) -> bool { + interact_rect.shrink(radius).contains(pos) +} + +fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits { + #![allow(clippy::collapsible_else_if)] + + // First find the best direct hits: + let hit_click = find_closest_within(close.iter().copied().filter(|w| w.sense.click), pos, 0.0); + let hit_drag = find_closest_within(close.iter().copied().filter(|w| w.sense.drag), pos, 0.0); match (hit_click, hit_drag) { (None, None) => { @@ -136,16 +196,16 @@ fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits { if let Some(closest) = closest { WidgetHits { - contains_pointer: hits, click: closest.sense.click.then_some(closest), drag: closest.sense.drag.then_some(closest), + ..Default::default() } } else { // Found nothing WidgetHits { - contains_pointer: hits, click: None, drag: None, + ..Default::default() } } } @@ -170,17 +230,17 @@ fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits { // This is a smaller thing on a big background - help the user hit it, // and ignore the big drag background. WidgetHits { - contains_pointer: hits, click: Some(closest_click), drag: Some(closest_click), + ..Default::default() } } else { - // The drag wiudth is separate from the click wiudth, - // so return only the drag widget + // The drag-widget is separate from the click-widget, + // so return only the drag-widget WidgetHits { - contains_pointer: hits, click: None, drag: Some(hit_drag), + ..Default::default() } } } else { @@ -194,17 +254,17 @@ fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits { // The drag widget is a big background thing (scroll area), // so returning a separate click widget should not be confusing WidgetHits { - contains_pointer: hits, click: Some(closest_click), drag: Some(hit_drag), + ..Default::default() } } else { // The two widgets are just two normal small widgets close to each other. // Highlighting both would be very confusing. WidgetHits { - contains_pointer: hits, click: None, drag: Some(hit_drag), + ..Default::default() } } } @@ -229,17 +289,17 @@ fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits { // `hit_drag` is a big background thing and `closest_drag` is something small on top of it. // Be helpful and return the small things: return WidgetHits { - contains_pointer: hits, click: None, drag: Some(closest_drag), + ..Default::default() }; } } WidgetHits { - contains_pointer: hits, click: None, drag: Some(hit_drag), + ..Default::default() } } } @@ -253,57 +313,57 @@ fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits { // where when hovering directly over a drag-widget (like a big ScrollArea), // we look for close click-widgets (e.g. buttons). // This is because big background drag-widgets (ScrollArea, Window) are common, - // but bit clickable things aren't. + // but big clickable things aren't. // Even if they were, I think it would be confusing for a user if clicking // a drag-only widget would click something _behind_ it. WidgetHits { - contains_pointer: hits, click: Some(hit_click), drag: None, + ..Default::default() } } (Some(hit_click), Some(hit_drag)) => { // We have a perfect hit on both click and drag. Which is the topmost? - let click_idx = hits.iter().position(|w| *w == hit_click).unwrap(); - let drag_idx = hits.iter().position(|w| *w == hit_drag).unwrap(); + let click_idx = close.iter().position(|w| *w == hit_click).unwrap(); + let drag_idx = close.iter().position(|w| *w == hit_drag).unwrap(); let click_is_on_top_of_drag = drag_idx < click_idx; if click_is_on_top_of_drag { if hit_click.sense.drag { // The top thing senses both clicks and drags. WidgetHits { - contains_pointer: hits, click: Some(hit_click), drag: Some(hit_click), + ..Default::default() } } else { // They are interested in different things, // and click is on top. Report both hits, // e.g. the top Button and the ScrollArea behind it. WidgetHits { - contains_pointer: hits, click: Some(hit_click), drag: Some(hit_drag), + ..Default::default() } } } else { if hit_drag.sense.click { // The top thing senses both clicks and drags. WidgetHits { - contains_pointer: hits, click: Some(hit_drag), drag: Some(hit_drag), + ..Default::default() } } else { // The top things senses only drags, // so we ignore the click-widget, because it would be confusing // if clicking a drag-widget would actually click something else below it. WidgetHits { - contains_pointer: hits, click: None, drag: Some(hit_drag), + ..Default::default() } } } @@ -312,8 +372,16 @@ fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits { } fn find_closest(widgets: impl Iterator, pos: Pos2) -> Option { - let mut closest = None; - let mut closest_dist_sq = f32::INFINITY; + find_closest_within(widgets, pos, f32::INFINITY) +} + +fn find_closest_within( + widgets: impl Iterator, + pos: Pos2, + max_dist: f32, +) -> Option { + let mut closest: Option = None; + let mut closest_dist_sq = max_dist * max_dist; for widget in widgets { if widget.interact_rect.is_negative() { continue; @@ -321,6 +389,16 @@ fn find_closest(widgets: impl Iterator, pos: Pos2) -> Option< let dist_sq = widget.interact_rect.distance_sq_to_pos(pos); + if let Some(closest) = closest { + if dist_sq == closest_dist_sq { + // It's a tie! Pick the thin candidate over the thick one. + // This makes it easier to hit a thin resize-handle, for instance: + if should_prioritizie_hits_on_back(closest.interact_rect, widget.interact_rect) { + continue; + } + } + } + // In case of a tie, take the last one = the one on top. if dist_sq <= closest_dist_sq { closest_dist_sq = dist_sq; @@ -331,6 +409,27 @@ fn find_closest(widgets: impl Iterator, pos: Pos2) -> Option< closest } +/// Should we prioritizie hits on `back` over those on `front`? +/// +/// `back` should be behind the `front` widget. +/// +/// Returns true if `back` is a small hit-target and `front` is not. +fn should_prioritizie_hits_on_back(back: Rect, front: Rect) -> bool { + if front.contains_rect(back) { + return false; // back widget is fully occluded; no way to hit it + } + + // Reduce each rect to its width or height, whichever is smaller: + let back = back.width().min(back.height()); + let front = front.width().min(front.height()); + + // These are hard-coded heuristics that could surely be improved. + let back_is_much_thinner = back <= 0.5 * front; + let back_is_thin = back <= 16.0; + + back_is_much_thinner && back_is_thin +} + #[cfg(test)] mod tests { use emath::{pos2, vec2, Rect}; diff --git a/crates/egui/src/interaction.rs b/crates/egui/src/interaction.rs index afac1602836..04f8f7dbf6f 100644 --- a/crates/egui/src/interaction.rs +++ b/crates/egui/src/interaction.rs @@ -249,7 +249,7 @@ pub(crate) fn interact( .copied() .collect() } else { - // We may be hovering a an interactive widget or two. + // We may be hovering an interactive widget or two. // We must also consider the case where non-interactive widgets // are _on top_ of an interactive widget. // For instance: a label in a draggable window. @@ -264,9 +264,9 @@ pub(crate) fn interact( // but none below it (an interactive widget stops the hover search). // // To know when to stop we need to first know the order of the widgets, - // which luckily we have in the `WidgetRects`. + // which luckily we already have in `hits.close`. - let order = |id| widgets.order(id).map(|(_layer, order)| order); // we ignore the layer, since all widgets at this point is in the same layer + let order = |id| hits.close.iter().position(|w| w.id == id); let click_order = hits.click.and_then(|w| order(w.id)).unwrap_or(0); let drag_order = hits.drag.and_then(|w| order(w.id)).unwrap_or(0); diff --git a/crates/egui/src/memory/mod.rs b/crates/egui/src/memory/mod.rs index 2d1449c1694..976ad2d95ef 100644 --- a/crates/egui/src/memory/mod.rs +++ b/crates/egui/src/memory/mod.rs @@ -1132,15 +1132,18 @@ type OrderMap = HashMap; pub struct Areas { areas: IdMap, + visible_areas_last_frame: ahash::HashSet, + visible_areas_current_frame: ahash::HashSet, + + // ---------------------------- + // Everything below this is general to all layers, not just areas. + // TODO(emilk): move this to a separate struct. /// Back-to-front, top is last. order: Vec, - /// Actual order of the layers, pre-calculated each frame. + /// Inverse of [`Self::order`], calculated at the end of the frame. order_map: OrderMap, - visible_last_frame: ahash::HashSet, - visible_current_frame: ahash::HashSet, - /// When an area wants to be on top, it is assigned here. /// This is used to reorder the layers at the end of the frame. /// If several layers want to be on top, they will keep their relative order. @@ -1148,9 +1151,9 @@ pub struct Areas { /// results in them being sent to the top and keeping their previous internal order. wants_to_be_on_top: ahash::HashSet, - /// List of sublayers for each layer. + /// The sublayers that each layer has. /// - /// When a layer has sublayers, they are moved directly above it in the ordering. + /// The parent sublayer is moved directly above the child sublayers in the ordering. sublayers: ahash::HashMap>, } @@ -1163,17 +1166,13 @@ impl Areas { self.areas.get(&id) } - /// Back-to-front, top is last. + /// All layers back-to-front, top is last. pub(crate) fn order(&self) -> &[LayerId] { &self.order } - /// For each layer, which [`Self::order`] is it in? - pub(crate) fn order_map(&self) -> &OrderMap { - &self.order_map - } - /// Compare the order of two layers, based on the order list from last frame. + /// /// May return [`std::cmp::Ordering::Equal`] if the layers are not in the order list. pub(crate) fn compare_order(&self, a: LayerId, b: LayerId) -> std::cmp::Ordering { if let (Some(a), Some(b)) = (self.order_map.get(&a), self.order_map.get(&b)) { @@ -1183,18 +1182,8 @@ impl Areas { } } - /// Calculates the order map. - fn calculate_order_map(&mut self) { - self.order_map = self - .order - .iter() - .enumerate() - .map(|(i, id)| (*id, i)) - .collect(); - } - pub(crate) fn set_state(&mut self, layer_id: LayerId, state: area::AreaState) { - self.visible_current_frame.insert(layer_id); + self.visible_areas_current_frame.insert(layer_id); self.areas.insert(layer_id.id, state); if !self.order.iter().any(|x| *x == layer_id) { self.order.push(layer_id); @@ -1227,18 +1216,19 @@ impl Areas { } pub fn visible_last_frame(&self, layer_id: &LayerId) -> bool { - self.visible_last_frame.contains(layer_id) + self.visible_areas_last_frame.contains(layer_id) } pub fn is_visible(&self, layer_id: &LayerId) -> bool { - self.visible_last_frame.contains(layer_id) || self.visible_current_frame.contains(layer_id) + self.visible_areas_last_frame.contains(layer_id) + || self.visible_areas_current_frame.contains(layer_id) } pub fn visible_layer_ids(&self) -> ahash::HashSet { - self.visible_last_frame + self.visible_areas_last_frame .iter() .copied() - .chain(self.visible_current_frame.iter().copied()) + .chain(self.visible_areas_current_frame.iter().copied()) .collect() } @@ -1251,7 +1241,7 @@ impl Areas { } pub fn move_to_top(&mut self, layer_id: LayerId) { - self.visible_current_frame.insert(layer_id); + self.visible_areas_current_frame.insert(layer_id); self.wants_to_be_on_top.insert(layer_id); if !self.order.iter().any(|x| *x == layer_id) { @@ -1266,8 +1256,21 @@ impl Areas { /// /// This currently only supports one level of nesting. If `parent` is a sublayer of another /// layer, the behavior is unspecified. + /// + /// The two layers must have the same [`LayerId::order`]. pub fn set_sublayer(&mut self, parent: LayerId, child: LayerId) { + debug_assert_eq!(parent.order, child.order, + "DEBUG ASSERT: Trying to set sublayers across layers of different order ({:?}, {:?}), which is currently undefined behavior in egui", parent.order, child.order); + self.sublayers.entry(parent).or_default().insert(child); + + // Make sure the layers are in the order list: + if !self.order.iter().any(|x| *x == parent) { + self.order.push(parent); + } + if !self.order.iter().any(|x| *x == child) { + self.order.push(child); + } } pub fn top_layer_id(&self, order: Order) -> Option { @@ -1278,26 +1281,42 @@ impl Areas { .copied() } + /// If this layer is the sublayer of another layer, return the parent. + pub fn parent_layer(&self, layer_id: LayerId) -> Option { + self.sublayers.iter().find_map(|(parent, children)| { + if children.contains(&layer_id) { + Some(*parent) + } else { + None + } + }) + } + + /// All the child layers of this layer. + pub fn child_layers(&self, layer_id: LayerId) -> impl Iterator + '_ { + self.sublayers.get(&layer_id).into_iter().flatten().copied() + } + pub(crate) fn is_sublayer(&self, layer: &LayerId) -> bool { - self.sublayers - .iter() - .any(|(_, children)| children.contains(layer)) + self.parent_layer(*layer).is_some() } pub(crate) fn end_pass(&mut self) { let Self { - visible_last_frame, - visible_current_frame, + visible_areas_last_frame, + visible_areas_current_frame, order, wants_to_be_on_top, sublayers, .. } = self; - std::mem::swap(visible_last_frame, visible_current_frame); - visible_current_frame.clear(); + std::mem::swap(visible_areas_last_frame, visible_areas_current_frame); + visible_areas_current_frame.clear(); + order.sort_by_key(|layer| (layer.order, wants_to_be_on_top.contains(layer))); wants_to_be_on_top.clear(); + // For all layers with sublayers, put the sublayers directly after the parent layer: let sublayers = std::mem::take(sublayers); for (parent, children) in sublayers { @@ -1315,7 +1334,13 @@ impl Areas { }; order.splice(parent_pos..=parent_pos, moved_layers); } - self.calculate_order_map(); + + self.order_map = self + .order + .iter() + .enumerate() + .map(|(i, id)| (*id, i)) + .collect(); } } diff --git a/crates/egui/src/widget_rect.rs b/crates/egui/src/widget_rect.rs index dd900af1f11..3725d62a7b0 100644 --- a/crates/egui/src/widget_rect.rs +++ b/crates/egui/src/widget_rect.rs @@ -20,10 +20,10 @@ pub struct WidgetRect { /// What layer the widget is on. pub layer_id: LayerId, - /// The full widget rectangle. + /// The full widget rectangle, in local layer coordinates. pub rect: Rect, - /// Where the widget is. + /// Where the widget is, in local layer coordinates. /// /// This is after clipping with the parent ui clip rect. pub interact_rect: Rect, @@ -42,6 +42,27 @@ pub struct WidgetRect { pub enabled: bool, } +impl WidgetRect { + pub fn transform(self, transform: emath::TSTransform) -> Self { + let Self { + id, + layer_id, + rect, + interact_rect, + sense, + enabled, + } = self; + Self { + id, + layer_id, + rect: transform * rect, + interact_rect: transform * interact_rect, + sense, + enabled, + } + } +} + /// Stores the [`WidgetRect`]s of all widgets generated during a single egui update/frame. /// /// All [`crate::Ui`]s have a [`WidgetRect`]. It is created in [`crate::Ui::new`] with [`Rect::NOTHING`]