Improve hit-test of thin widgets, and widgets across layers (#5468)

When there are multiple layers (e.g. with custom transforms) close to
each other, the hit test code used to only consider widgets in the layer
directly under the mouse. This can make it difficult to hit thin widgets
just on the outside of a transform layer.

This PR fixes that.

It also prioritizes thin widgets, so that if there is both a thin widget
and a thick widget under the mouse cursor, you will always hit the thin
widgets, even if the thin widgets is layered behind the thick one. This
makes it easier to hit thin resize-handles.

In theory this should allow us to make `resize_grab_radius_side` and
`resize_grab_radius_corner` smaller in a future PR, if we want to.
This commit is contained in:
Emil Ernerfeldt 2024-12-16 09:33:25 +01:00 committed by GitHub
parent 3af907919b
commit f7efb2186d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 261 additions and 122 deletions

View File

@ -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<LayerId> = 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<LayerId> = 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());
}
}
});

View File

@ -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<WidgetRect>,
/// 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<WidgetRect>,
/// 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);
}
hits
} else {
// No close widgets.
Default::default()
}
}
fn hit_test_on_close(close: &[WidgetRect], pos: Pos2) -> WidgetHits {
#![allow(clippy::collapsible_else_if)]
// 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.
// Only those widgets directly under the `pos`.
let hits: Vec<WidgetRect> = close
let mut included_layers: ahash::HashSet<LayerId> = 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));
// 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;
}
}
let mut hits = hit_test_on_close(&close, pos);
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<Item = WidgetRect>, pos: Pos2) -> Option<WidgetRect> {
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<Item = WidgetRect>,
pos: Pos2,
max_dist: f32,
) -> Option<WidgetRect> {
let mut closest: Option<WidgetRect> = 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<Item = WidgetRect>, 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<Item = WidgetRect>, 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};

View File

@ -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);

View File

@ -1132,15 +1132,18 @@ type OrderMap = HashMap<LayerId, usize>;
pub struct Areas {
areas: IdMap<area::AreaState>,
visible_areas_last_frame: ahash::HashSet<LayerId>,
visible_areas_current_frame: ahash::HashSet<LayerId>,
// ----------------------------
// 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<LayerId>,
/// 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<LayerId>,
visible_current_frame: ahash::HashSet<LayerId>,
/// 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<LayerId>,
/// 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<LayerId, HashSet<LayerId>>,
}
@ -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<LayerId> {
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<LayerId> {
@ -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<LayerId> {
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<Item = LayerId> + '_ {
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();
}
}

View File

@ -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`]