Merge pull request #482 from kas-gui/work3

Fixes

Author: dhardy

crates/kas-core/src/core/impls.rs

@@ -9,6 +9,7 @@ use crate::event::{ConfigCx, Event, EventCx, FocusSource, IsUsed, Scroll, Unused
 use crate::{Events, Id, NavAdvance, Node, Tile, Widget};
 
 /// Generic implementation of [`Widget::_send`]
+#[inline(always)]
 pub fn _send<W: Events>(
     widget: &mut W,
     cx: &mut EventCx,
@@ -76,6 +77,7 @@ pub fn _send<W: Events>(
 }
 
 /// Generic implementation of [`Widget::_replay`]
+#[inline(always)]
 pub fn _replay<W: Events>(widget: &mut W, cx: &mut EventCx, data: &<W as Widget>::Data, id: Id) {
     if let Some(index) = widget.find_child_index(&id) {
         let mut _found = false;
@@ -112,6 +114,7 @@ pub fn _replay<W: Events>(widget: &mut W, cx: &mut EventCx, data: &<W as Widget>
 }
 
 /// Generic implementation of [`Widget::_nav_next`]
+#[inline(always)]
 pub fn _nav_next<W: Events>(
     widget: &mut W,
     cx: &mut ConfigCx,
@@ -142,9 +145,10 @@ fn nav_next(
     let mut child = focus.and_then(|id| widget.find_child_index(id));
 
     if let Some(index) = child {
-        if let Some(Some(id)) =
-            widget.for_child(index, |mut node| node._nav_next(cx, focus, advance))
-        {
+        let mut opt_id = None;
+        let out = &mut opt_id;
+        widget.for_child(index, |mut node| *out = node._nav_next(cx, focus, advance));
+        if let Some(id) = opt_id {
             return Some(id);
         }
     }
@@ -168,11 +172,13 @@ fn nav_next(
     };
 
     while let Some(index) = widget.nav_next(rev, child) {
-        if let Some(Some(id)) =
-            widget.for_child(index, |mut node| node._nav_next(cx, focus, advance))
-        {
+        let mut opt_id = None;
+        let out = &mut opt_id;
+        widget.for_child(index, |mut node| *out = node._nav_next(cx, focus, advance));
+        if let Some(id) = opt_id {
             return Some(id);
         }
+
         child = Some(index);
     }
 

crates/kas-core/src/core/node.rs

@@ -214,34 +214,26 @@ impl<'a> Node<'a> {
         self.0.num_children()
     }
 
-    /// Run `f` on some child by index and, if valid, return the result.
+    /// Run `f` on some child by index.
     ///
-    /// Calls the closure and returns `Some(result)` exactly when
-    /// `index < self.num_children()`.
-    pub fn for_child<R>(&mut self, index: usize, f: impl FnOnce(Node<'_>) -> R) -> Option<R> {
-        let mut result = None;
-        let out = &mut result;
-        let f: Box<dyn for<'b> FnOnce(Node<'b>)> = Box::new(|node| {
-            *out = Some(f(node));
-        });
+    /// Calls the closure exactly when `index < self.num_children()`.
+    #[inline(always)]
+    pub fn for_child(&mut self, index: usize, f: impl FnOnce(Node<'_>)) {
+        let f: Box<dyn for<'b> FnOnce(Node<'b>)> = Box::new(f);
         cfg_if::cfg_if! {
             if #[cfg(feature = "unsafe_node")] {
                 self.0.for_child_node(self.1, index, f);
             } else {
                 self.0.for_child_node(index, f);
             }
         }
-        result
     }
 
     /// Run a `f` on all children
+    #[inline(always)]
     pub fn for_children(&mut self, mut f: impl FnMut(Node<'_>)) {
         for index in 0..self.0.num_children() {
-            // NOTE: for_child_node takes FnOnce hence we must wrap the closure
-            let f = &mut f;
-            let f: Box<dyn for<'b> FnOnce(Node<'b>)> = Box::new(|node| {
-                f(node);
-            });
+            let f: Box<dyn for<'b> FnOnce(Node<'b>)> = Box::new(&mut f);
             cfg_if::cfg_if! {
                 if #[cfg(feature = "unsafe_node")] {
                     self.0.for_child_node(self.1, index, f);
@@ -264,10 +256,13 @@ impl<'a> Node<'a> {
     /// Find the descendant with this `id`, if any, and call `cb` on it
     ///
     /// Returns `Some(result)` if and only if node `id` was found.
+    #[inline(always)]
     pub fn find_node<F: FnOnce(Node<'_>) -> T, T>(&mut self, id: &Id, cb: F) -> Option<T> {
         if let Some(index) = self.find_child_index(id) {
-            self.for_child(index, |mut node| node.find_node(id, cb))
-                .unwrap()
+            let mut result = None;
+            let out = &mut result;
+            self.for_child(index, |mut node| *out = node.find_node(id, cb));
+            result
         } else if self.eq_id(id) {
             Some(cb(self.re()))
         } else {

crates/kas-core/src/event/components.rs

@@ -17,7 +17,6 @@ use std::time::{Duration, Instant};
 
 const TIMER_SELECT: TimerHandle = TimerHandle::new(1 << 60, true);
 const TIMER_GLIDE: TimerHandle = TimerHandle::new((1 << 60) + 1, true);
-const GLIDE_POLL_MS: u64 = 3;
 const GLIDE_MAX_SAMPLES: usize = 8;
 
 #[derive(Clone, Debug)]
@@ -318,7 +317,7 @@ impl ScrollComponent {
                 if self.max_offset != Offset::ZERO && cx.config_enable_pan(*press) =>
             {
                 let _ = press
-                    .grab(id.clone())
+                    .grab(id.clone(), GrabMode::Grab)
                     .with_icon(CursorIcon::Grabbing)
                     .with_cx(cx);
                 self.glide.press_start();
@@ -336,21 +335,20 @@ impl ScrollComponent {
                 let timeout = cx.config().event().scroll_flick_timeout();
                 let pan_dist_thresh = cx.config().event().pan_dist_thresh();
                 if self.glide.press_end(timeout, pan_dist_thresh) {
-                    cx.request_timer(id.clone(), TIMER_GLIDE, Duration::ZERO);
+                    cx.request_frame_timer(id.clone(), TIMER_GLIDE);
                 }
             }
-            Event::Timer(pl) if pl == TIMER_GLIDE => {
+            Event::Timer(TIMER_GLIDE) => {
                 // Momentum/glide scrolling: update per arbitrary step time until movment stops.
                 let timeout = cx.config().event().scroll_flick_timeout();
                 let decay = cx.config().event().scroll_flick_decay();
                 if let Some(delta) = self.glide.step(timeout, decay) {
                     action = self.scroll_by_delta(cx, delta);
+                    cx.set_scroll(Scroll::Scrolled);
+                }
 
-                    if self.glide.vel != Vec2::ZERO {
-                        let dur = Duration::from_millis(GLIDE_POLL_MS);
-                        cx.request_timer(id.clone(), TIMER_GLIDE, dur);
-                        cx.set_scroll(Scroll::Scrolled);
-                    }
+                if self.glide.vel != Vec2::ZERO {
+                    cx.request_frame_timer(id.clone(), TIMER_GLIDE);
                 }
             }
             _ => return (false, Unused),
@@ -444,7 +442,10 @@ impl TextInput {
                         None
                     }
                 };
-                press.grab(w_id).with_opt_icon(icon).with_cx(cx);
+                press
+                    .grab(w_id, GrabMode::Grab)
+                    .with_opt_icon(icon)
+                    .with_cx(cx);
                 self.glide.press_start();
                 action
             }
@@ -484,11 +485,11 @@ impl TextInput {
                         || matches!(press.source, PressSource::Mouse(..) if cx.config_enable_mouse_text_pan()))
                 {
                     self.touch_phase = TouchPhase::None;
-                    cx.request_timer(w_id, TIMER_GLIDE, Duration::ZERO);
+                    cx.request_frame_timer(w_id, TIMER_GLIDE);
                 }
                 Action::None
             }
-            Event::Timer(pl) if pl == TIMER_SELECT => match self.touch_phase {
+            Event::Timer(TIMER_SELECT) => match self.touch_phase {
                 TouchPhase::Start(touch_id, coord) => {
                     self.touch_phase = TouchPhase::Cursor(touch_id);
                     Action::Focus {
@@ -498,13 +499,12 @@ impl TextInput {
                 }
                 _ => Action::None,
             },
-            Event::Timer(pl) if pl == TIMER_GLIDE => {
+            Event::Timer(TIMER_GLIDE) => {
                 // Momentum/glide scrolling: update per arbitrary step time until movment stops.
                 let timeout = cx.config().event().scroll_flick_timeout();
                 let decay = cx.config().event().scroll_flick_decay();
                 if let Some(delta) = self.glide.step(timeout, decay) {
-                    let dur = Duration::from_millis(GLIDE_POLL_MS);
-                    cx.request_timer(w_id, TIMER_GLIDE, dur);
+                    cx.request_frame_timer(w_id, TIMER_GLIDE);
                     Action::Pan(delta)
                 } else {
                     Action::None

crates/kas-core/src/event/cx/cx_pub.rs

@@ -166,7 +166,7 @@ impl EventState {
     /// target is disabled, and also cancels press/pan grabs.
     pub fn set_disabled(&mut self, target: Id, disable: bool) {
         if disable {
-            self.clear_events(&target);
+            self.cancel_event_focus(&target);
         }
 
         for (i, id) in self.disabled.iter().enumerate() {
@@ -186,8 +186,9 @@ impl EventState {
 
     /// Schedule a timed update
     ///
-    /// Widget updates may be used for animation and timed responses. See also
-    /// [`Draw::animate`](crate::draw::Draw::animate) for animation.
+    /// Widget updates may be used for delayed action. For animation, prefer to
+    /// use [`Draw::animate`](crate::draw::Draw::animate) or
+    /// [`Self::request_frame_timer`].
     ///
     /// Widget `id` will receive [`Event::Timer`] with this `handle` at
     /// approximately `time = now + delay` (or possibly a little later due to
@@ -223,6 +224,21 @@ impl EventState {
         self.time_updates.sort_by(|a, b| b.0.cmp(&a.0)); // reverse sort
     }
 
+    /// Schedule a frame timer update
+    ///
+    /// Widget `id` will receive [`Event::Timer`] with this `handle` either
+    /// before or soon after the next frame is drawn.
+    ///
+    /// This may be useful for animations which mutate widget state. Animations
+    /// which don't mutate widget state may use
+    /// [`Draw::animate`](crate::draw::Draw::animate) instead.
+    ///
+    /// It is expected that `handle.earliest() == true` (style guide).
+    pub fn request_frame_timer(&mut self, id: Id, handle: TimerHandle) {
+        debug_assert!(handle.earliest());
+        self.frame_updates.insert((id, handle));
+    }
+
     /// Notify that a widget must be redrawn
     ///
     /// This is equivalent to calling [`Self::action`] with [`Action::REDRAW`].

crates/kas-core/src/event/cx/mod.rs

@@ -8,7 +8,7 @@
 // Without winit, several things go unused
 #![cfg_attr(not(winit), allow(unused))]
 
-use linear_map::LinearMap;
+use linear_map::{set::LinearSet, LinearMap};
 use smallvec::SmallVec;
 use std::collections::{BTreeMap, HashMap, VecDeque};
 use std::future::Future;
@@ -60,7 +60,7 @@ impl GrabMode {
 
 #[derive(Clone, Debug)]
 enum GrabDetails {
-    Click { cur_id: Option<Id> },
+    Click,
     Grab,
     Pan((u16, u16)),
 }
@@ -81,24 +81,6 @@ struct MouseGrab {
     cancel: bool,
 }
 
-impl<'a> EventCx<'a> {
-    fn flush_mouse_grab_motion(&mut self) {
-        if let Some(grab) = self.mouse_grab.as_mut() {
-            if let GrabDetails::Click { ref cur_id } = grab.details {
-                if grab.start_id == cur_id {
-                    if grab.depress != *cur_id {
-                        grab.depress = cur_id.clone();
-                        self.action |= Action::REDRAW;
-                    }
-                } else if grab.depress.is_some() {
-                    grab.depress = None;
-                    self.action |= Action::REDRAW;
-                }
-            }
-        }
-    }
-}
-
 #[derive(Clone, Debug)]
 struct TouchGrab {
     id: u64,
@@ -210,6 +192,7 @@ pub struct EventState {
     popups: SmallVec<[(WindowId, crate::PopupDescriptor, Option<Id>); 16]>,
     popup_removed: SmallVec<[(Id, WindowId); 16]>,
     time_updates: Vec<(Instant, Id, TimerHandle)>,
+    frame_updates: LinearSet<(Id, TimerHandle)>,
     // Set of messages awaiting sending
     send_queue: VecDeque<(Id, Erased)>,
     // Set of futures of messages together with id of sending widget
@@ -380,14 +363,16 @@ impl EventState {
         window_id
     }
 
-    /// Clear all active events on `target`
-    fn clear_events(&mut self, target: &Id) {
+    /// Clear all focus and grabs on `target`
+    fn cancel_event_focus(&mut self, target: &Id) {
         if let Some(id) = self.sel_focus.as_ref() {
             if target.is_ancestor_of(id) {
                 if let Some(pending) = self.pending_sel_focus.as_mut() {
                     if pending.target.as_ref() == Some(id) {
                         pending.target = None;
                         pending.key_focus = false;
+                    } else {
+                        // We have a new focus target, hence the old one will be cleared
                     }
                 } else {
                     self.pending_sel_focus = Some(PendingSelFocus {