From f5692305ff96bef87cf005d4665aa0747c58b0db Mon Sep 17 00:00:00 2001
From: Thomas Churchman <thomas@kepow.org>
Date: Thu, 29 Aug 2024 13:53:52 +0200
Subject: [PATCH] masonry: reimplement `Widget::get_child_at_pos`

---
 masonry/src/widget/widget.rs     | 37 ++++++++++++++++++++++++--------
 masonry/src/widget/widget_ref.rs | 21 +++++++++---------
 2 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/masonry/src/widget/widget.rs b/masonry/src/widget/widget.rs
index 52065f7ec..bb2afec15 100644
--- a/masonry/src/widget/widget.rs
+++ b/masonry/src/widget/widget.rs
@@ -14,8 +14,10 @@ use vello::Scene;
 
 use crate::contexts::ComposeCtx;
 use crate::event::{AccessEvent, PointerEvent, StatusChange, TextEvent};
+use crate::widget::WidgetRef;
 use crate::{
-    AccessCtx, AsAny, BoxConstraints, EventCtx, LayoutCtx, LifeCycle, LifeCycleCtx, PaintCtx, Size,
+    AccessCtx, AsAny, BoxConstraints, EventCtx, LayoutCtx, LifeCycle, LifeCycleCtx, PaintCtx,
+    Point, Size,
 };
 
 /// A unique identifier for a single [`Widget`].
@@ -165,20 +167,29 @@ pub trait Widget: AsAny {
 
     // --- Auto-generated implementations ---
 
-    // FIXME
-    #[cfg(FALSE)]
     /// Return which child, if any, has the given `pos` in its layout rect.
     ///
-    /// The child return is a direct child, not eg a grand-child. The position is in
+    /// The child returned is a direct child, not eg a grand-child. The position is in
     /// relative coordinates. (Eg `(0,0)` is the top-left corner of `self`).
     ///
     /// Has a default implementation, that can be overridden to search children more
     /// efficiently.
-    fn get_child_at_pos(&self, pos: Point) -> Option<WidgetRef<'_, dyn Widget>> {
-        // layout_rect() is in parent coordinate space
-        self.children()
-            .into_iter()
-            .find(|child| child.state().layout_rect().contains(pos))
+    ///
+    /// The child widget references in `children` are in the same order as returned by
+    /// [`Self::children_ids`].
+    fn get_child_at_pos<'w>(
+        &self,
+        children: &[WidgetRef<'w, dyn Widget>],
+        pos: Point,
+    ) -> Option<WidgetRef<'w, dyn Widget>> {
+        children
+            .iter()
+            // TODO: currently assumes `Self::children_ids` is in increasing "z-order". Picks the
+            // last child in case of overlapping children. Is this the desired behavior?
+            .rev()
+            // A child's layout_rect() is in the parent's coordinate space.
+            .find(|child| !child.widget.skip_pointer() && child.state().layout_rect().contains(pos))
+            .copied()
     }
 
     /// Get the (verbose) type name of the widget for debugging purposes.
@@ -354,6 +365,14 @@ impl Widget for Box<dyn Widget> {
         self.deref().get_cursor()
     }
 
+    fn get_child_at_pos<'w>(
+        &self,
+        children: &[WidgetRef<'w, dyn Widget>],
+        pos: Point,
+    ) -> Option<WidgetRef<'w, dyn Widget>> {
+        self.deref().get_child_at_pos(children, pos)
+    }
+
     fn as_any(&self) -> &dyn Any {
         self.deref().as_any()
     }
diff --git a/masonry/src/widget/widget_ref.rs b/masonry/src/widget/widget_ref.rs
index 2f85ea5ba..5481471ac 100644
--- a/masonry/src/widget/widget_ref.rs
+++ b/masonry/src/widget/widget_ref.rs
@@ -172,31 +172,32 @@ impl<'w> WidgetRef<'w, dyn Widget> {
     ///
     /// **pos** - the position in local coordinates (zero being the top-left of the
     /// inner widget).
-    pub fn find_widget_at_pos(&self, pos: Point) -> Option<WidgetRef<'w, dyn Widget>> {
+    pub fn find_widget_at_pos(&self, mut pos: Point) -> Option<WidgetRef<'w, dyn Widget>> {
         let mut innermost_widget: WidgetRef<'w, dyn Widget> = *self;
 
         if !self.state().layout_rect().contains(pos) {
             return None;
         }
 
-        // TODO - Rewrite more elegantly
         loop {
             if let Some(clip) = innermost_widget.state().clip {
-                let relative_pos = pos.to_vec2() - innermost_widget.state().window_origin.to_vec2();
-                // If the widget has a clip, the point must be inside
-                // else we don't iterate over children.
-                if !clip.contains(relative_pos.to_point()) {
+                // If the widget has a clip, the point must be inside, else we don't iterate over
+                // children.
+                if !clip.contains(pos) {
                     break;
                 }
             }
-            // TODO - Use Widget::get_child_at_pos method
-            if let Some(child) = innermost_widget.children().into_iter().find(|child| {
-                !child.widget.skip_pointer() && child.state().window_layout_rect().contains(pos)
-            }) {
+
+            if let Some(child) = innermost_widget
+                .widget
+                .get_child_at_pos(&innermost_widget.children(), pos)
+            {
                 innermost_widget = child;
             } else {
                 break;
             }
+
+            pos -= innermost_widget.state().layout_rect().origin().to_vec2();
         }
 
         Some(innermost_widget)