-
Notifications
You must be signed in to change notification settings - Fork 109
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
base: main
Are you sure you want to change the base?
Changes from all commits
098c0bf
ee01ea8
b77bb9f
a17447b
cb2dcaa
79348ac
f0eb0fc
499242c
6f88715
7aebe37
8c8aa7d
1eac8bc
f7589ee
719d9ce
0b5528d
cfe929e
9591a3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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`]. | ||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Sure. I was thinking we'd probably need to add a |
||
/// 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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, same with the |
||
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. | ||
|
@@ -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() | ||
} | ||
|
There was a problem hiding this comment.
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.