From 9a3c8e308c3f46f15bb1a5bd48b9119f0178ad98 Mon Sep 17 00:00:00 2001 From: Tom Churchman Date: Tue, 10 Sep 2024 13:55:51 +0200 Subject: [PATCH] masonry: replace `set_active` and `is_active` with pointer capture (#564) Also improve documentation of pointer capture. Continuation of 59ee615651266421ffe65dadee6d16663f056c9d (https://github.com/linebender/xilem/pull/488). Makes `has_pointer_capture` available on all context types except `LayoutCtx`, like `is_active` used to be. --- masonry/examples/calc_masonry.rs | 5 +- masonry/src/contexts.rs | 70 ++++++++++++------- masonry/src/passes/event.rs | 10 +++ masonry/src/widget/button.rs | 12 +--- masonry/src/widget/checkbox.rs | 5 +- masonry/src/widget/prose.rs | 12 ++-- masonry/src/widget/scroll_bar.rs | 3 +- masonry/src/widget/split.rs | 7 +- masonry/src/widget/tests/status_change.rs | 83 +++++++++++++++++++---- masonry/src/widget/textbox.rs | 10 +-- 10 files changed, 145 insertions(+), 72 deletions(-) diff --git a/masonry/examples/calc_masonry.rs b/masonry/examples/calc_masonry.rs index 022707be..b490c867 100644 --- a/masonry/examples/calc_masonry.rs +++ b/masonry/examples/calc_masonry.rs @@ -150,12 +150,12 @@ impl Widget for CalcButton { ctx.mutate_later(&mut self.inner, move |mut inner| { inner.set_background(color); }); - ctx.set_active(true); + ctx.capture_pointer(); trace!("CalcButton {:?} pressed", ctx.widget_id()); } } PointerEvent::PointerUp(_, _) => { - if ctx.is_active() && !ctx.is_disabled() { + if ctx.has_pointer_capture() && !ctx.is_disabled() { let color = self.base_color; // See on_status_change for why we use `mutate_later` here. ctx.mutate_later(&mut self.inner, move |mut inner| { @@ -164,7 +164,6 @@ impl Widget for CalcButton { ctx.submit_action(Action::Other(Box::new(self.action))); trace!("CalcButton {:?} released", ctx.widget_id()); } - ctx.set_active(false); } _ => (), } diff --git a/masonry/src/contexts.rs b/masonry/src/contexts.rs index 69a8bfa3..bacceb62 100644 --- a/masonry/src/contexts.rs +++ b/masonry/src/contexts.rs @@ -234,7 +234,7 @@ impl_context_method!( // --- MARK: GET STATUS --- // Methods on all context types except LayoutCtx -// Access status information (hot/active/disabled/etc). +// Access status information (hot/pointer captured/disabled/etc). impl_context_method!( MutateCtx<'_>, EventCtx<'_>, @@ -254,16 +254,20 @@ impl_context_method!( /// mouse position have hot status. /// /// Discussion: there is currently some confusion about whether a - /// widget can be considered hot when some other widget is active (for - /// example, when clicking to one widget and dragging to the next). - /// The documentation should clearly state the resolution. + /// widget can be considered hot when some other widget has captured the + /// pointer (for example, when clicking one widget and dragging to the + /// next). The documentation should clearly state the resolution. pub fn is_hot(&self) -> bool { self.widget_state.is_hot } - // TODO - remove - pub fn is_active(&self) -> bool { - self.global_state.pointer_capture_target == Some(self.widget_id()) + /// Whether the pointer is captured by this widget. + /// + /// See [`capture_pointer`] for more information about pointer capture. + /// + /// [`capture_pointer`]: EventCtx::capture_pointer + pub fn has_pointer_capture(&self) -> bool { + self.global_state.pointer_capture_target == Some(self.widget_state.id) } /// The focus status of a widget. @@ -331,14 +335,14 @@ impl_context_method!(EventCtx<'_>, { /// Set the cursor icon. /// /// This setting will be retained until [`clear_cursor`] is called, but it will only take - /// effect when this widget is either [`hot`] or [`active`]. If a child widget also sets a - /// cursor, the child widget's cursor will take precedence. (If that isn't what you want, use - /// [`override_cursor`] instead.) + /// effect when this widget is [`hot`] and/or [`has_pointer_capture`]. If a child widget also + /// sets a cursor, the child widget's cursor will take precedence. (If that isn't what you + /// want, use [`override_cursor`] instead.) /// /// [`clear_cursor`]: EventCtx::clear_cursor /// [`override_cursor`]: EventCtx::override_cursor /// [`hot`]: EventCtx::is_hot - /// [`active`]: EventCtx::is_active + /// [`has_pointer_capture`]: EventCtx::has_pointer_capture pub fn set_cursor(&mut self, cursor: &CursorIcon) { trace!("set_cursor {:?}", cursor); self.widget_state.cursor = Some(*cursor); @@ -579,9 +583,35 @@ impl_context_method!( pub struct TimerToken; impl EventCtx<'_> { - // TODO - Document + // TODO - clearly document all semantics of pointer capture when they've been decided on // TODO - Figure out cases where widget should be notified of pointer capture // loss + /// Capture the pointer in the current widget. + /// + /// Pointer capture is only allowed during a [`PointerDown`] event. It is a logic error to + /// capture the pointer during any other event. + /// + /// A widget normally only receives pointer events when the pointer is inside the widget's + /// layout box. Pointer capture causes widget layout boxes to be ignored: when the pointer is + /// captured by a widget, that widget will continue receiving pointer events when the pointer + /// is outside the widget's layout box. Other widgets the pointer is over will not receive + /// events. Events that are not marked as handled by the capturing widget, bubble up to the + /// widget's ancestors, ignoring their layout boxes as well. + /// + /// The pointer cannot be captured by multiple widgets at the same time. If a widget has + /// captured the pointer and another widget captures it, the first widget loses the pointer + /// capture. + /// + /// # Releasing the pointer + /// + /// Any widget can [`release`] the pointer during any event. The pointer is automatically + /// released after handling of a [`PointerUp`] or [`PointerLeave`] event completes. A widget + /// holding the pointer capture will be the target of these events. + /// + /// [`PointerDown`]: crate::PointerEvent::PointerDown + /// [`PointerUp`]: crate::PointerEvent::PointerUp + /// [`PointerLeave`]: crate::PointerEvent::PointerLeave + /// [`release`]: Self::release_pointer #[track_caller] pub fn capture_pointer(&mut self) { debug_assert!( @@ -593,14 +623,13 @@ impl EventCtx<'_> { self.global_state.pointer_capture_target = Some(self.widget_state.id); } + /// Release the pointer previously captured through [`capture_pointer`]. + /// + /// [`capture_pointer`]: EventCtx::capture_pointer pub fn release_pointer(&mut self) { self.global_state.pointer_capture_target = None; } - pub fn has_pointer_capture(&self) -> bool { - self.global_state.pointer_capture_target == Some(self.widget_state.id) - } - /// Send a signal to parent widgets to scroll this widget into view. pub fn request_scroll_to_this(&mut self) { let rect = self.widget_state.layout_rect(); @@ -618,15 +647,6 @@ impl EventCtx<'_> { .push((self.widget_state.id, rect)); } - // TODO - Remove - pub fn set_active(&mut self, active: bool) { - if active { - self.global_state.pointer_capture_target = Some(self.widget_state.id); - } else { - self.global_state.pointer_capture_target = None; - } - } - /// Set the event as "handled", which stops its propagation to other /// widgets. pub fn set_handled(&mut self) { diff --git a/masonry/src/passes/event.rs b/masonry/src/passes/event.rs index ae6e5a3e..c360d80f 100644 --- a/masonry/src/passes/event.rs +++ b/masonry/src/passes/event.rs @@ -107,6 +107,16 @@ pub(crate) fn root_on_pointer_event( }, ); + if matches!( + event, + PointerEvent::PointerUp(..) | PointerEvent::PointerLeave(..) + ) { + // Automatically release the pointer on pointer up or leave. If a widget holds the capture, + // it is notified of the pointer event before the capture is released, so it knows it is + // about to lose the pointer. + root.state.pointer_capture_target = None; + } + if !event.is_high_density() { debug!( focused_widget = root.state.focused_widget.map(|id| id.0), diff --git a/masonry/src/widget/button.rs b/masonry/src/widget/button.rs index 542655a8..ecf9c023 100644 --- a/masonry/src/widget/button.rs +++ b/masonry/src/widget/button.rs @@ -81,23 +81,17 @@ impl Widget for Button { match event { PointerEvent::PointerDown(_, _) => { if !ctx.is_disabled() { - ctx.set_active(true); + ctx.capture_pointer(); ctx.request_paint(); trace!("Button {:?} pressed", ctx.widget_id()); } } PointerEvent::PointerUp(button, _) => { - if ctx.is_active() && ctx.is_hot() && !ctx.is_disabled() { + if ctx.has_pointer_capture() && ctx.is_hot() && !ctx.is_disabled() { ctx.submit_action(Action::ButtonPressed(*button)); trace!("Button {:?} released", ctx.widget_id()); } ctx.request_paint(); - ctx.set_active(false); - } - PointerEvent::PointerLeave(_) => { - // If the screen was locked whilst holding down the mouse button, we don't get a `PointerUp` - // event, but should no longer be active - ctx.set_active(false); } _ => (), } @@ -151,7 +145,7 @@ impl Widget for Button { } fn paint(&mut self, ctx: &mut PaintCtx, scene: &mut Scene) { - let is_active = ctx.is_active() && !ctx.is_disabled(); + let is_active = ctx.has_pointer_capture() && !ctx.is_disabled(); let is_hot = ctx.is_hot(); let size = ctx.size(); let stroke_width = theme::BUTTON_BORDER_WIDTH; diff --git a/masonry/src/widget/checkbox.rs b/masonry/src/widget/checkbox.rs index e3256676..51266606 100644 --- a/masonry/src/widget/checkbox.rs +++ b/masonry/src/widget/checkbox.rs @@ -68,20 +68,19 @@ impl Widget for Checkbox { match event { PointerEvent::PointerDown(_, _) => { if !ctx.is_disabled() { - ctx.set_active(true); + ctx.capture_pointer(); ctx.request_paint(); trace!("Checkbox {:?} pressed", ctx.widget_id()); } } PointerEvent::PointerUp(_, _) => { - if ctx.is_active() && ctx.is_hot() && !ctx.is_disabled() { + if ctx.has_pointer_capture() && ctx.is_hot() && !ctx.is_disabled() { self.checked = !self.checked; ctx.submit_action(Action::CheckboxChecked(self.checked)); ctx.request_accessibility_update(); trace!("Checkbox {:?} released", ctx.widget_id()); } ctx.request_paint(); - ctx.set_active(false); } _ => (), } diff --git a/masonry/src/widget/prose.rs b/masonry/src/widget/prose.rs index 69468502..992f82bd 100644 --- a/masonry/src/widget/prose.rs +++ b/masonry/src/widget/prose.rs @@ -154,7 +154,7 @@ impl Widget for Prose { ctx.request_layout(); ctx.request_paint(); ctx.request_focus(); - ctx.set_active(true); + ctx.capture_pointer(); } } } @@ -162,7 +162,9 @@ impl Widget for Prose { if !ctx.is_disabled() { // TODO: Set cursor if over link ctx.set_cursor(&CursorIcon::Text); - if ctx.is_active() && self.text_layout.pointer_move(inner_origin, state) { + if ctx.has_pointer_capture() + && self.text_layout.pointer_move(inner_origin, state) + { // We might have changed text colours, so we need to re-request a layout ctx.request_layout(); ctx.request_paint(); @@ -171,13 +173,9 @@ impl Widget for Prose { } PointerEvent::PointerUp(button, state) => { // TODO: Follow link (if not now dragging ?) - if !ctx.is_disabled() && ctx.is_active() { + if !ctx.is_disabled() && ctx.has_pointer_capture() { self.text_layout.pointer_up(inner_origin, state, *button); } - ctx.set_active(false); - } - PointerEvent::PointerLeave(_state) => { - ctx.set_active(false); } _ => {} } diff --git a/masonry/src/widget/scroll_bar.rs b/masonry/src/widget/scroll_bar.rs index 9763315b..4aa9b630 100644 --- a/masonry/src/widget/scroll_bar.rs +++ b/masonry/src/widget/scroll_bar.rs @@ -128,7 +128,7 @@ impl Widget for ScrollBar { fn on_pointer_event(&mut self, ctx: &mut EventCtx, event: &PointerEvent) { match event { PointerEvent::PointerDown(_, state) => { - ctx.set_active(true); + ctx.capture_pointer(); let cursor_min_length = theme::SCROLLBAR_MIN_SIZE; let cursor_rect = self.get_cursor_rect(ctx.size(), cursor_min_length); @@ -164,7 +164,6 @@ impl Widget for ScrollBar { } PointerEvent::PointerUp(_, _) => { self.grab_anchor = None; - ctx.set_active(false); ctx.request_paint(); } _ => {} diff --git a/masonry/src/widget/split.rs b/masonry/src/widget/split.rs index 20214092..7ef1f37d 100644 --- a/masonry/src/widget/split.rs +++ b/masonry/src/widget/split.rs @@ -375,7 +375,7 @@ impl Widget for Split { PointerEvent::PointerDown(PointerButton::Primary, state) => { if self.bar_hit_test(ctx.size(), state.position) { ctx.set_handled(); - ctx.set_active(true); + ctx.capture_pointer(); // Save the delta between the mouse click position and the split point self.click_offset = match self.split_axis { Axis::Horizontal => state.position.x, @@ -392,9 +392,8 @@ impl Widget for Split { } } PointerEvent::PointerUp(PointerButton::Primary, state) => { - if ctx.is_active() { + if ctx.has_pointer_capture() { ctx.set_handled(); - ctx.set_active(false); // Dependending on where the mouse cursor is when the button is released, // the cursor might or might not need to be changed self.is_bar_hover = @@ -405,7 +404,7 @@ impl Widget for Split { } } PointerEvent::PointerMove(state) => { - if ctx.is_active() { + if ctx.has_pointer_capture() { // If active, assume always hover/hot let effective_pos = match self.split_axis { Axis::Horizontal => { diff --git a/masonry/src/widget/tests/status_change.rs b/masonry/src/widget/tests/status_change.rs index 7a1d4250..8ea5e09c 100644 --- a/masonry/src/widget/tests/status_change.rs +++ b/masonry/src/widget/tests/status_change.rs @@ -8,6 +8,18 @@ use crate::testing::{widget_ids, Record, Recording, TestHarness, TestWidgetExt a use crate::widget::{Button, Flex, Label, SizedBox}; use crate::*; +fn next_pointer_event(recording: &Recording) -> Option { + while let Some(event) = recording.next() { + match event { + Record::PE(event) => { + return Some(event); + } + _ => {} + }; + } + None +} + fn is_hot(harness: &TestHarness, id: WidgetId) -> bool { harness.get_widget(id).state().is_hot } @@ -203,18 +215,6 @@ fn update_hot_from_layout() { #[test] fn get_pointer_events_while_active() { - fn next_pointer_event(recording: &Recording) -> Option { - while let Some(event) = recording.next() { - match event { - Record::PE(event) => { - return Some(event); - } - _ => {} - }; - } - None - } - let [button, root, empty, empty_2] = widget_ids(); let button_rec = Recording::default(); @@ -287,3 +287,62 @@ fn get_pointer_events_while_active() { harness.mouse_move_to(empty_2); assert_matches!(next_pointer_event(&button_rec), None); } + +#[test] +fn automatically_lose_pointer_on_pointer_leave() { + let [button, root, empty] = widget_ids(); + + let button_rec = Recording::default(); + + let widget = Flex::column() + .with_child_id(SizedBox::empty().width(10.0).height(10.0), empty) + .with_child_id(Button::new("hello").record(&button_rec), button) + .with_id(root); + + let mut harness = TestHarness::create(widget); + + // The default state is that nothing has captured the pointer. + assert_eq!(harness.pointer_capture_target_id(), None); + + // We press the button + harness.mouse_move_to(button); + harness.mouse_button_press(PointerButton::Primary); + + // The button should be notified of the move and pointer down events + assert_matches!( + next_pointer_event(&button_rec), + Some(PointerEvent::PointerMove(_)) + ); + assert_matches!( + next_pointer_event(&button_rec), + Some(PointerEvent::PointerDown(_, _)) + ); + + // and should now hold the capture. + assert_eq!(harness.pointer_capture_target_id(), Some(button)); + + // The pointer moves to empty space. The button is notified and still holds the capture. + harness.mouse_move_to(empty); + assert_matches!( + next_pointer_event(&button_rec), + Some(PointerEvent::PointerMove(_)) + ); + assert_eq!(harness.pointer_capture_target_id(), Some(button)); + + // The pointer leaves, without releasing the primary button first + harness.process_pointer_event(PointerEvent::PointerLeave(PointerState::empty())); + + // The button holds the capture during this event and should be notified the pointer is leaving + assert_matches!( + next_pointer_event(&button_rec), + Some(PointerEvent::PointerLeave(_)) + ); + + // The button should have lost the pointer capture + assert_eq!(harness.pointer_capture_target_id(), None); + + // If the pointer enters and leaves again, the button should not be notified + harness.process_pointer_event(PointerEvent::PointerEnter(PointerState::empty())); + harness.process_pointer_event(PointerEvent::PointerLeave(PointerState::empty())); + assert_matches!(next_pointer_event(&button_rec), None); +} diff --git a/masonry/src/widget/textbox.rs b/masonry/src/widget/textbox.rs index f37342bc..ac96b4ac 100644 --- a/masonry/src/widget/textbox.rs +++ b/masonry/src/widget/textbox.rs @@ -178,13 +178,13 @@ impl Widget for Textbox { ctx.request_paint(); ctx.request_accessibility_update(); ctx.request_focus(); - ctx.set_active(true); + ctx.capture_pointer(); } } } PointerEvent::PointerMove(state) => { if !ctx.is_disabled() - && ctx.is_active() + && ctx.has_pointer_capture() && self.editor.pointer_move(inner_origin, state) { // We might have changed text colours, so we need to re-request a layout @@ -195,13 +195,9 @@ impl Widget for Textbox { } PointerEvent::PointerUp(button, state) => { // TODO: Follow link (if not now dragging ?) - if !ctx.is_disabled() && ctx.is_active() { + if !ctx.is_disabled() && ctx.has_pointer_capture() { self.editor.pointer_up(inner_origin, state, *button); } - ctx.set_active(false); - } - PointerEvent::PointerLeave(_state) => { - ctx.set_active(false); } _ => {} }