Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

masonry: reimplement Widget::get_child_at_pos #565

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions masonry/src/contexts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, TextFieldRegistration};
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,
};
Expand Down Expand Up @@ -51,6 +51,15 @@ pub struct MutateCtx<'a> {
pub(crate) widget_children: ArenaMutChildren<'a, Box<dyn Widget>>,
}

/// 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<dyn Widget>>,
}

/// A context provided to event handling methods of widgets.
///
/// Widgets should call [`request_paint`](Self::request_paint) whenever an event causes a change
Expand Down Expand Up @@ -118,6 +127,7 @@ pub struct AccessCtx<'a> {
// Methods for all context types
impl_context_method!(
MutateCtx<'_>,
QueryCtx<'_>,
EventCtx<'_>,
LifeCycleCtx<'_>,
LayoutCtx<'_>,
Expand Down Expand Up @@ -194,6 +204,7 @@ impl_context_method!(
// These methods access layout info calculated during the layout pass.
impl_context_method!(
MutateCtx<'_>,
QueryCtx<'_>,
EventCtx<'_>,
LifeCycleCtx<'_>,
ComposeCtx<'_>,
Expand Down Expand Up @@ -237,6 +248,7 @@ impl_context_method!(
// Access status information (hot/pointer captured/disabled/etc).
impl_context_method!(
MutateCtx<'_>,
QueryCtx<'_>,
EventCtx<'_>,
LifeCycleCtx<'_>,
ComposeCtx<'_>,
Expand Down Expand Up @@ -403,6 +415,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> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably take WidgetPod for consistency with other context methods.

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<'_>, {
Expand Down
2 changes: 1 addition & 1 deletion masonry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
QueryCtx, RawWrapper, RawWrapperMut,
};
pub use event::{
AccessEvent, InternalLifeCycle, LifeCycle, PointerButton, PointerEvent, PointerState,
Expand Down
37 changes: 32 additions & 5 deletions masonry/src/render_root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ use crate::tree_arena::TreeArena;
use crate::widget::WidgetArena;
use crate::widget::{WidgetMut, WidgetRef, WidgetState};
use crate::{
AccessEvent, Action, BoxConstraints, CursorIcon, Handled, InternalLifeCycle, LifeCycle, Widget,
WidgetId, WidgetPod,
AccessEvent, Action, BoxConstraints, CursorIcon, Handled, InternalLifeCycle, LifeCycle,
QueryCtx, Widget, WidgetId, WidgetPod,
};

// --- MARK: STRUCTS ---
Expand Down Expand Up @@ -305,6 +305,7 @@ impl RenderRoot {
}

// --- MARK: ACCESS WIDGETS---
/// Get a [`WidgetRef`] to the root widget.
pub fn get_root_widget(&self) -> WidgetRef<dyn Widget> {
let root_state_token = self.widget_arena.widget_states.root_token();
let root_widget_token = self.widget_arena.widgets.root_token();
Expand All @@ -324,12 +325,38 @@ impl RenderRoot {
.downcast_ref::<Box<dyn Widget>>()
.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<WidgetRef<dyn Widget>> {
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> -> &dyn Widget
// Without this step, the type of `WidgetRef::widget` would be
// `&Box<dyn Widget> 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.
Expand Down
8 changes: 3 additions & 5 deletions masonry/src/testing/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<WidgetRef<'_, dyn Widget>> {
self.render_root.widget_arena.try_get_widget_ref(id)
self.render_root.get_widget(id)
}

// TODO - link to focus documentation.
Expand All @@ -476,8 +475,7 @@ impl TestHarness {
// TODO - Multiple pointers
pub fn pointer_capture_target(&self) -> Option<WidgetRef<'_, dyn Widget>> {
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
Expand Down
61 changes: 48 additions & 13 deletions masonry/src/widget/widget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, QueryCtx, Size,
};

/// A unique identifier for a single [`Widget`].
Expand Down Expand Up @@ -165,20 +167,45 @@ 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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should implementations overriding the default uphold certain conditions?

Custom implementations should probably uphold the guarantee that pos is actually inside the returned child's layout rect. Must custom custom implementations also uphold the guarantee that pos is contained in the clip path, and that in case of overlap the last child in "z-order" is chosen?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes to all of these.

Ideally we should even implement some best-effort checks in debug_assertions mode to enforce that these conditions are upheld.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The low effort version is to check the child contains the position in its layout rect and clip path.

The conditions uniquely identify a child, so we could do the default search alongside Widget::get_child_at_pos and check the same children are returned. For widgets with many children—where custom implementations are most likely—this would be slow. Valid options are to accept that cost or to limit this exhaustive check to widgets with N children for some value of N.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, most of that is beyond the scope of this PR.

We could probably add a TODO comment and keep it to that for now.

Valid options are to accept that cost or to limit this exhaustive check to widgets with N children for some value of N.

Sure. I was thinking we'd probably need to add a child_count() method for cases where building a child array is needlessly expensive (eg a Flex with thousands of children).

/// 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<WidgetRef<'_, dyn Widget>> {
// 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,
tomcur marked this conversation as resolved.
Show resolved Hide resolved
) -> Option<WidgetRef<'c, dyn Widget>> {
let relative_pos = pos - ctx.widget_state.window_origin().to_vec2();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QueryCtx::widget_state is pub(crate), we can access it here but user code currently can't.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, same with the clip field access.

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.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.
Expand Down Expand Up @@ -354,6 +381,14 @@ impl Widget for Box<dyn Widget> {
self.deref().get_cursor()
}

fn get_child_at_pos<'c>(
&self,
ctx: QueryCtx<'c>,
pos: Point,
) -> Option<WidgetRef<'c, dyn Widget>> {
self.deref().get_child_at_pos(ctx, pos)
}

fn as_any(&self) -> &dyn Any {
self.deref().as_any()
}
Expand Down
22 changes: 0 additions & 22 deletions masonry/src/widget/widget_arena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<WidgetRef<dyn Widget>> {
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> -> &dyn Widget
// Without this step, the type of `WidgetRef::widget` would be
// `&Box<dyn Widget> 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,
})
}
}
Loading