-
Notifications
You must be signed in to change notification settings - Fork 126
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
Implement update_scrolls pass #550
Changes from all commits
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 |
---|---|---|
|
@@ -202,3 +202,31 @@ pub(crate) fn run_update_disabled_pass(root: &mut RenderRoot) { | |
let (root_widget, root_state) = root.widget_arena.get_pair_mut(root.root.id()); | ||
update_disabled_for_widget(&mut root.state, root_widget, root_state, false); | ||
} | ||
|
||
// ---------------- | ||
|
||
// This pass will update scroll positions in cases where a widget has requested to be | ||
// scrolled into view (usually a textbox getting text events). | ||
// Each parent that implements scrolling will update its scroll position to ensure the | ||
// child is visible. (If the target area is larger than the parent, the parent will try | ||
// to show the top left of that area.) | ||
pub(crate) fn run_update_scroll_pass(root: &mut RenderRoot) { | ||
let _span = info_span!("update_scroll").entered(); | ||
|
||
let scroll_request_targets = std::mem::take(&mut root.state.scroll_request_targets); | ||
for (target, rect) in scroll_request_targets { | ||
let mut target_rect = rect; | ||
|
||
run_targeted_update_pass(root, Some(target), |widget, ctx| { | ||
let event = LifeCycle::RequestPanToChild(rect); | ||
widget.lifecycle(ctx, &event); | ||
|
||
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. How will smooth scroll would be handled? The best logic I can see if to "get where the widget will be", and all the parents scroll to 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. Depends what you mean by smooth scroll. Do you mean on a touchscreen? I think the most straightforward solution is to have CSS-style animatable properties, and have scroll progress be one of them. 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. I don't mean on a touchscreen, I mean scrolling to a position over a certain amount of time. Right, yeah. This logic would have to change at that point, but it's also deliberately unaddressed at the moment. |
||
// TODO - We should run the compose method after this, so | ||
// translations are updated and the rect passed to parents | ||
// is more accurate. | ||
|
||
let state = &ctx.widget_state; | ||
target_rect = target_rect + state.translation + state.origin.to_vec2(); | ||
}); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -145,6 +145,26 @@ impl<W: Widget> Portal<W> { | |
false | ||
} | ||
} | ||
|
||
// Note - Rect is in child coordinates | ||
// TODO - Merge with pan_viewport_to | ||
// Right now these functions are just different enough to be a pain to merge. | ||
fn pan_viewport_to_raw(&mut self, portal_size: Size, content_size: Size, target: Rect) -> bool { | ||
let viewport = Rect::from_origin_size(self.viewport_pos, portal_size); | ||
|
||
let new_pos_x = compute_pan_range( | ||
viewport.min_x()..viewport.max_x(), | ||
target.min_x()..target.max_x(), | ||
) | ||
.start; | ||
let new_pos_y = compute_pan_range( | ||
viewport.min_y()..viewport.max_y(), | ||
target.min_y()..target.max_y(), | ||
) | ||
.start; | ||
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. So we prefer the 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. I added in a comment on the pass function. I'll do a more thorough documentation pass later. |
||
|
||
self.set_viewport_pos_raw(portal_size, content_size, Point::new(new_pos_x, new_pos_y)) | ||
} | ||
} | ||
|
||
// --- MARK: WIDGETMUT --- | ||
|
@@ -315,8 +335,29 @@ impl<W: Widget> Widget for Portal<W> { | |
LifeCycle::WidgetAdded => { | ||
ctx.register_as_portal(); | ||
} | ||
//TODO | ||
//LifeCycle::RequestPanToChild(target_rect) => {} | ||
LifeCycle::RequestPanToChild(target) => { | ||
let portal_size = ctx.size(); | ||
let content_size = ctx.get_raw_ref(&mut self.child).ctx().layout_rect().size(); | ||
|
||
self.pan_viewport_to_raw(portal_size, content_size, *target); | ||
ctx.request_compose(); | ||
|
||
// TODO - There's a lot of code here that's duplicated from the `MouseWheel` | ||
// event in `on_pointer_event`. | ||
// Because this code directly manipulates child widgets, it's hard to factor | ||
// it out. | ||
let mut scrollbar = ctx.get_raw_mut(&mut self.scrollbar_vertical); | ||
scrollbar.widget().cursor_progress = | ||
self.viewport_pos.y / (content_size - portal_size).height; | ||
scrollbar.ctx().request_paint(); | ||
|
||
std::mem::drop(scrollbar); | ||
|
||
let mut scrollbar = ctx.get_raw_mut(&mut self.scrollbar_horizontal); | ||
scrollbar.widget().cursor_progress = | ||
self.viewport_pos.x / (content_size - portal_size).width; | ||
scrollbar.ctx().request_paint(); | ||
} | ||
_ => {} | ||
} | ||
|
||
|
@@ -424,10 +465,6 @@ impl<W: Widget> Widget for Portal<W> { | |
} | ||
|
||
ctx.current_node().set_clips_children(); | ||
ctx.current_node() | ||
.push_child(self.scrollbar_horizontal.id().into()); | ||
ctx.current_node() | ||
.push_child(self.scrollbar_vertical.id().into()); | ||
} | ||
|
||
fn children_ids(&self) -> SmallVec<[WidgetId; 16]> { | ||
|
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.
Does the order matter here? Is it meaningful to document that?
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.
The order doesn't matter.
scroll_request_target
was initially an option, and I turned it to a Vec just to cover hypothetical cases where someone would want to pan several scroll areas at once.I guess several targets could interfere if they're in the same scroll region. I think this will be rare enough that we won't need to document it.