From 653874e1030a659ce63092346466a1660d2556f4 Mon Sep 17 00:00:00 2001 From: Tom Churchman Date: Thu, 26 Sep 2024 16:37:05 +0200 Subject: [PATCH] masonry: reimplement `Widget::get_child_at_pos` (#565) The `Widget::get_child_at_pos` has a default linear search implementation, upholding some conditions (e.g., picking the last child in "z-order" in case of overlap). Widgets can override this with a custom implementation (e.g., a widget with many children that maintains a quadtree to search through). Custom implementations should uphold the same conditions, we can add some debug assertions to check this in a later PR. This introduces a `QueryCtx`, allowing widget methods access to the global state, widget state, and widget subtree. `QueryCtx` is similar to the other (mutable) context types, but can be shared. This also changes `WidgetRef` to consist of a `Widget` and a `QueryCtx`, similar to `WidgetMut` consisting of a `Widget` and `MutateCtx`. This required changing where `WidgetRef` can be constructed. This resolves code comment https://github.com/linebender/xilem/blob/ac95f2524ac33fccd02f444b8a75ae0daaf98c3c/masonry/src/widget/widget_ref.rs#L192-L199 --- masonry/src/contexts.rs | 44 +++++++++++++++- masonry/src/lib.rs | 2 +- masonry/src/render_root.rs | 35 +++++++++++-- masonry/src/testing/harness.rs | 8 ++- masonry/src/widget/widget.rs | 63 ++++++++++++++++++----- masonry/src/widget/widget_arena.rs | 22 -------- masonry/src/widget/widget_ref.rs | 82 +++++++++++------------------- 7 files changed, 158 insertions(+), 98 deletions(-) diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 8f49ad4f8..888ec92bb 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -15,8 +15,8 @@ use crate::passes::layout::run_layout_on; use crate::render_root::{MutateCallback, RenderRootSignal, RenderRootState}; use crate::text::TextBrush; use crate::text_helpers::ImeChangeSignal; -use crate::tree_arena::ArenaMutChildren; -use crate::widget::{WidgetMut, WidgetState}; +use crate::tree_arena::{ArenaMutChildren, ArenaRefChildren}; +use crate::widget::{WidgetMut, WidgetRef, WidgetState}; use crate::{ AllowRawMut, BoxConstraints, CursorIcon, Insets, Point, Rect, Size, Widget, WidgetId, WidgetPod, }; @@ -51,6 +51,15 @@ pub struct MutateCtx<'a> { pub(crate) widget_children: ArenaMutChildren<'a, Box>, } +/// A context provided to methods of widgets requiring shared, read-only access. +#[derive(Clone, Copy)] +pub struct QueryCtx<'a> { + pub(crate) global_state: &'a RenderRootState, + pub(crate) widget_state: &'a WidgetState, + pub(crate) widget_state_children: ArenaRefChildren<'a, WidgetState>, + pub(crate) widget_children: ArenaRefChildren<'a, Box>, +} + /// A context provided to event handling methods of widgets. /// /// Widgets should call [`request_paint`](Self::request_paint) whenever an event causes a change @@ -124,6 +133,7 @@ pub struct AccessCtx<'a> { // Methods for all context types impl_context_method!( MutateCtx<'_>, + QueryCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, LayoutCtx<'_>, @@ -200,6 +210,7 @@ impl_context_method!( // These methods access layout info calculated during the layout pass. impl_context_method!( MutateCtx<'_>, + QueryCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, ComposeCtx<'_>, @@ -243,6 +254,7 @@ impl_context_method!( // Access status information (hot/pointer captured/disabled/etc). impl_context_method!( MutateCtx<'_>, + QueryCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, ComposeCtx<'_>, @@ -413,6 +425,34 @@ impl<'a> MutateCtx<'a> { } } +// --- MARK: WIDGET_REF --- +// Methods to get a child WidgetRef from a parent. +impl<'w> QueryCtx<'w> { + /// Return a [`WidgetRef`] to a child widget. + pub fn get(self, child: WidgetId) -> WidgetRef<'w, dyn Widget> { + let child_state = self + .widget_state_children + .into_child(child.to_raw()) + .expect("get: child not found"); + let child = self + .widget_children + .into_child(child.to_raw()) + .expect("get: child not found"); + + let ctx = QueryCtx { + global_state: self.global_state, + widget_state_children: child_state.children, + widget_children: child.children, + widget_state: child_state.item, + }; + + WidgetRef { + ctx, + widget: child.item, + } + } +} + // --- MARK: UPDATE FLAGS --- // Methods on MutateCtx, EventCtx, and LifeCycleCtx impl_context_method!(MutateCtx<'_>, EventCtx<'_>, LifeCycleCtx<'_>, { diff --git a/masonry/src/lib.rs b/masonry/src/lib.rs index 861f7f3e6..32b71b7a2 100644 --- a/masonry/src/lib.rs +++ b/masonry/src/lib.rs @@ -131,7 +131,7 @@ pub use action::Action; pub use box_constraints::BoxConstraints; pub use contexts::{ AccessCtx, ComposeCtx, EventCtx, IsContext, LayoutCtx, LifeCycleCtx, MutateCtx, PaintCtx, - RawWrapper, RawWrapperMut, RegisterCtx, + QueryCtx, RawWrapper, RawWrapperMut, RegisterCtx, }; pub use event::{ AccessEvent, LifeCycle, PointerButton, PointerEvent, PointerState, StatusChange, TextEvent, diff --git a/masonry/src/render_root.rs b/masonry/src/render_root.rs index 5d487c743..6ecffa6d9 100644 --- a/masonry/src/render_root.rs +++ b/masonry/src/render_root.rs @@ -34,7 +34,7 @@ use crate::tree_arena::TreeArena; use crate::widget::WidgetArena; use crate::widget::{WidgetMut, WidgetRef, WidgetState}; use crate::{ - AccessEvent, Action, BoxConstraints, CursorIcon, Handled, Widget, WidgetId, WidgetPod, + AccessEvent, Action, BoxConstraints, CursorIcon, Handled, QueryCtx, Widget, WidgetId, WidgetPod, }; // --- MARK: STRUCTS --- @@ -306,6 +306,7 @@ impl RenderRoot { } // --- MARK: ACCESS WIDGETS--- + /// Get a [`WidgetRef`] to the root widget. pub fn get_root_widget(&self) -> WidgetRef { let root_state_token = self.widget_arena.widget_states.root_token(); let root_widget_token = self.widget_arena.widgets.root_token(); @@ -325,12 +326,38 @@ impl RenderRoot { .downcast_ref::>() .unwrap(); - WidgetRef { + let ctx = QueryCtx { + global_state: &self.state, widget_state_children: state_ref.children, widget_children: widget_ref.children, widget_state: state_ref.item, - widget, - } + }; + + WidgetRef { ctx, widget } + } + + /// Get a [`WidgetRef`] to a specific widget. + pub fn get_widget(&self, id: WidgetId) -> Option> { + let state_ref = self.widget_arena.widget_states.find(id.to_raw())?; + let widget_ref = self + .widget_arena + .widgets + .find(id.to_raw()) + .expect("found state but not widget"); + + // Box -> &dyn Widget + // Without this step, the type of `WidgetRef::widget` would be + // `&Box as &dyn Widget`, which would be an additional layer + // of indirection. + let widget = widget_ref.item; + let widget: &dyn Widget = &**widget; + let ctx = QueryCtx { + global_state: &self.state, + widget_state_children: state_ref.children, + widget_children: widget_ref.children, + widget_state: state_ref.item, + }; + Some(WidgetRef { ctx, widget }) } /// Get a [`WidgetMut`] to the root widget. diff --git a/masonry/src/testing/harness.rs b/masonry/src/testing/harness.rs index 8c97fe95a..c9163726b 100644 --- a/masonry/src/testing/harness.rs +++ b/masonry/src/testing/harness.rs @@ -456,14 +456,13 @@ impl TestHarness { #[track_caller] pub fn get_widget(&self, id: WidgetId) -> WidgetRef<'_, dyn Widget> { self.render_root - .widget_arena - .try_get_widget_ref(id) + .get_widget(id) .unwrap_or_else(|| panic!("could not find widget #{}", id.to_raw())) } /// Try to return the widget with the given id. pub fn try_get_widget(&self, id: WidgetId) -> Option> { - self.render_root.widget_arena.try_get_widget_ref(id) + self.render_root.get_widget(id) } // TODO - link to focus documentation. @@ -476,8 +475,7 @@ impl TestHarness { // TODO - Multiple pointers pub fn pointer_capture_target(&self) -> Option> { self.render_root - .widget_arena - .try_get_widget_ref(self.render_root.state.pointer_capture_target?) + .get_widget(self.render_root.state.pointer_capture_target?) } // TODO - This is kinda redundant with the above diff --git a/masonry/src/widget/widget.rs b/masonry/src/widget/widget.rs index 0e8317461..9104a8e76 100644 --- a/masonry/src/widget/widget.rs +++ b/masonry/src/widget/widget.rs @@ -14,9 +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, - RegisterCtx, Size, + Point, QueryCtx, RegisterCtx, Size, }; /// A unique identifier for a single [`Widget`]. @@ -177,20 +178,48 @@ pub trait Widget: AsAny { // --- Auto-generated implementations --- - // FIXME - #[cfg(FALSE)] - /// Return which child, if any, has the given `pos` in its layout rect. + /// Return which child, if any, has the given `pos` in its layout rect. In case of overlapping + /// children, the last child as determined by [`Widget::children_ids`] is chosen. No child is + /// returned if `pos` is outside the widget's clip path. + /// + /// The child returned is a direct child, not e.g. a grand-child. /// - /// The child return 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. + /// Custom implementations must uphold the conditions outlined above. /// - /// Has a default implementation, that can be overridden to search children more - /// efficiently. - fn get_child_at_pos(&self, pos: Point) -> Option> { - // layout_rect() is in parent coordinate space - self.children() - .into_iter() - .find(|child| child.state().layout_rect().contains(pos)) + /// **pos** - the position in global coordinates (e.g. `(0,0)` is the top-left corner of the + /// window). + fn get_child_at_pos<'c>( + &self, + ctx: QueryCtx<'c>, + pos: Point, + ) -> Option> { + let relative_pos = pos - ctx.widget_state.window_origin().to_vec2(); + if !ctx + .widget_state + .clip + .map_or(true, |clip| clip.contains(relative_pos)) + { + return None; + } + + // Assumes `Self::children_ids` is in increasing "z-order", picking the last child in case + // of overlapping children. + for child_id in self.children_ids().iter().rev() { + let child = ctx.get(*child_id); + + let relative_pos = pos - child.state().window_origin().to_vec2(); + // The position must be inside the child's layout and inside the child's clip path (if + // any). + if !child.state().is_stashed + && !child.widget.skip_pointer() + && child.state().window_layout_rect().contains(pos) + { + return Some(child); + } + } + + None } /// Get the (verbose) type name of the widget for debugging purposes. @@ -370,6 +399,14 @@ impl Widget for Box { self.deref().get_cursor() } + fn get_child_at_pos<'c>( + &self, + ctx: QueryCtx<'c>, + pos: Point, + ) -> Option> { + self.deref().get_child_at_pos(ctx, pos) + } + fn as_any(&self) -> &dyn Any { self.deref().as_any() } diff --git a/masonry/src/widget/widget_arena.rs b/masonry/src/widget/widget_arena.rs index 4b2f858df..e5534c8f0 100644 --- a/masonry/src/widget/widget_arena.rs +++ b/masonry/src/widget/widget_arena.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::tree_arena::{ArenaMut, ArenaRef, TreeArena}; -use crate::widget::WidgetRef; use crate::{Widget, WidgetId, WidgetState}; pub(crate) struct WidgetArena { @@ -87,25 +86,4 @@ impl WidgetArena { .find_mut(widget_id.to_raw()) .expect("get_state_mut: widget state not in widget tree") } - - pub fn try_get_widget_ref(&self, id: WidgetId) -> Option> { - let state_ref = self.widget_states.find(id.to_raw())?; - let widget_ref = self - .widgets - .find(id.to_raw()) - .expect("found state but not widget"); - - // Box -> &dyn Widget - // Without this step, the type of `WidgetRef::widget` would be - // `&Box as &dyn Widget`, which would be an additional layer - // of indirection. - let widget = widget_ref.item; - let widget: &dyn Widget = &**widget; - Some(WidgetRef { - widget_state_children: state_ref.children, - widget_children: widget_ref.children, - widget_state: state_ref.item, - widget, - }) - } } diff --git a/masonry/src/widget/widget_ref.rs b/masonry/src/widget/widget_ref.rs index 7bb0db119..f9473446f 100644 --- a/masonry/src/widget/widget_ref.rs +++ b/masonry/src/widget/widget_ref.rs @@ -6,8 +6,7 @@ use std::ops::Deref; use smallvec::SmallVec; use vello::kurbo::Point; -use crate::tree_arena::ArenaRefChildren; -use crate::{Widget, WidgetId, WidgetState}; +use crate::{QueryCtx, Widget, WidgetId, WidgetState}; /// A rich reference to a [`Widget`]. /// @@ -23,9 +22,7 @@ use crate::{Widget, WidgetId, WidgetState}; /// This is only for shared access to widgets. For widget mutation, see [`WidgetMut`](crate::widget::WidgetMut). pub struct WidgetRef<'w, W: Widget + ?Sized> { - pub(crate) widget_state_children: ArenaRefChildren<'w, WidgetState>, - pub(crate) widget_children: ArenaRefChildren<'w, Box>, - pub(crate) widget_state: &'w WidgetState, + pub(crate) ctx: QueryCtx<'w>, pub(crate) widget: &'w W, } @@ -35,9 +32,7 @@ pub struct WidgetRef<'w, W: Widget + ?Sized> { impl<'w, W: Widget + ?Sized> Clone for WidgetRef<'w, W> { fn clone(&self) -> Self { Self { - widget_state_children: self.widget_state_children, - widget_children: self.widget_children, - widget_state: self.widget_state, + ctx: self.ctx, widget: self.widget, } } @@ -82,7 +77,7 @@ impl<'w, W: Widget + ?Sized> WidgetRef<'w, W> { // TODO - Replace with individual methods from WidgetState /// Get the [`WidgetState`] of the current widget. pub fn state(self) -> &'w WidgetState { - self.widget_state + self.ctx.widget_state } /// Get the actual referenced `Widget`. @@ -92,34 +87,32 @@ impl<'w, W: Widget + ?Sized> WidgetRef<'w, W> { /// Get the [`WidgetId`] of the current widget. pub fn id(&self) -> WidgetId { - self.widget_state.id + self.ctx.widget_state.id } /// Attempt to downcast to `WidgetRef` of concrete Widget type. pub fn downcast(&self) -> Option> { Some(WidgetRef { - widget_state_children: self.widget_state_children, - widget_children: self.widget_children, - widget_state: self.widget_state, + ctx: self.ctx, widget: self.widget.as_any().downcast_ref()?, }) } /// Return widget's children. pub fn children(&self) -> SmallVec<[WidgetRef<'w, dyn Widget>; 16]> { - let parent_id = self.widget_state.id.to_raw(); + let parent_id = self.ctx.widget_state.id.to_raw(); self.widget .children_ids() .iter() .map(|id| { let id = id.to_raw(); - let Some(state_ref) = self.widget_state_children.into_child(id) else { + let Some(state_ref) = self.ctx.widget_state_children.into_child(id) else { panic!( "Error in '{}' #{parent_id}: child #{id} has not been added to tree", self.widget.short_type_name() ); }; - let Some(widget_ref) = self.widget_children.into_child(id) else { + let Some(widget_ref) = self.ctx.widget_children.into_child(id) else { panic!( "Error in '{}' #{parent_id}: child #{id} has not been added to tree", self.widget.short_type_name() @@ -133,12 +126,14 @@ impl<'w, W: Widget + ?Sized> WidgetRef<'w, W> { let widget = widget_ref.item; let widget: &dyn Widget = &**widget; - WidgetRef { + let ctx = QueryCtx { + global_state: self.ctx.global_state, widget_state_children: state_ref.children, widget_children: widget_ref.children, widget_state: state_ref.item, - widget, - } + }; + + WidgetRef { ctx, widget } }) .collect() } @@ -148,9 +143,7 @@ impl<'w, W: Widget> WidgetRef<'w, W> { /// Return a type-erased `WidgetRef`. pub fn as_dyn(&self) -> WidgetRef<'w, dyn Widget> { WidgetRef { - widget_state_children: self.widget_state_children, - widget_children: self.widget_children, - widget_state: self.widget_state, + ctx: self.ctx, widget: self.widget, } } @@ -168,40 +161,27 @@ impl<'w> WidgetRef<'w, dyn Widget> { } } - /// Recursively find innermost widget at given position. - /// - /// If multiple overlapping children of a widget contain the given position in their layout - /// boxes, the last child as determined by [`Widget::children_ids`] is chosen. + /// Recursively find the innermost widget at the given position, using + /// [`Widget::get_child_at_pos`] to descend the widget tree. If `self` does not contain the + /// given position in its layout rect or clip path, this returns `None`. /// - /// **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> { - let mut innermost_widget: WidgetRef<'w, dyn Widget> = *self; + /// **pos** - the position in global coordinates (e.g. `(0,0)` is the top-left corner of the + /// window). + pub fn find_widget_at_pos(&self, pos: Point) -> Option> { + let mut innermost_widget = *self; - if !self.state().layout_rect().contains(pos) { + if !self.state().window_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()) { - break; - } - } - // TODO - Use Widget::get_child_at_pos method - if let Some(child) = innermost_widget.children().into_iter().rev().find(|child| { - !child.state().is_stashed - && !child.widget.skip_pointer() - && child.state().window_layout_rect().contains(pos) - }) { - innermost_widget = child; - } else { - break; - } + // TODO: add debug assertion to check whether the child returned by + // `Widget::get_child_at_pos` upholds the conditions of that method. See + // https://github.com/linebender/xilem/pull/565#discussion_r1756536870 + while let Some(child) = innermost_widget + .widget + .get_child_at_pos(innermost_widget.ctx, pos) + { + innermost_widget = child; } Some(innermost_widget)