Merge pull request #276 from kas-gui/work2

Local configure, Ord for WidgetId

CHANGELOG.md

@@ -10,6 +10,15 @@ This is a small patch:
 -   Fix loading the font DB before parsing Markdown (#246)
 -   Support `#[widget(align = stretch)]` (#246)
 
+## [0.11.0] — unreleased
+
+### Breaking changes
+
+-   `WidgetId` has been completely re-written, and now represents a path
+-   `Ord for WidgetId` now considers a parent to come *before* its children.
+    Note: previously ordering was used in `send` logic; this is no longer
+    recommended (use e.g. `WidgetId::index_of_child` instead).
+
 ## [0.10.0] — 2021-09-05
 
 This release responds to three key criticisms of KAS: (1) slow compile times,

crates/kas-core/Cargo.toml

@@ -71,6 +71,7 @@ serde_json = { version = "1.0.61", optional = true }
 serde_yaml = { version = "0.8.16", optional = true }
 dep_ron = { version = "0.6.4", package = "ron", optional = true }
 image = "0.23.14"
+num_enum = "0.5.6"
 
 [dependencies.kas-macros]
 version = "0.10.1"

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

@@ -63,7 +63,7 @@ impl<M: 'static> WidgetChildren for Box<dyn Widget<Msg = M>> {
 }
 
 impl<M: 'static> WidgetConfig for Box<dyn Widget<Msg = M>> {
-    fn configure(&mut self, mgr: &mut EventMgr) {
+    fn configure(&mut self, mgr: &mut SetRectMgr) {
         self.as_mut().configure(mgr);
     }
 

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

@@ -8,13 +8,15 @@
 use std::any::Any;
 use std::fmt;
 
-use crate::event::{self, ConfigureManager, EventMgr};
+use crate::event;
+#[allow(unused)]
+use crate::event::{EventMgr, EventState};
 use crate::geom::{Coord, Offset, Rect};
 use crate::layout::{self, AlignHints, AxisInfo, SetRectMgr, SizeRules};
+#[allow(unused)]
+use crate::theme::DrawCtx;
 use crate::theme::{DrawMgr, SizeMgr};
 use crate::util::IdentifyWidget;
-#[allow(unused)]
-use crate::{event::EventState, theme::DrawCtx};
 use crate::{CoreData, TkAction, WidgetId};
 
 impl dyn WidgetCore {
@@ -148,12 +150,9 @@ pub trait WidgetCore: Any + fmt::Debug {
 /// [`derive(Widget)`] unless `#[widget(children = noauto)]` is used.
 ///
 /// Dynamic widgets must implement this trait manually, since [`derive(Widget)`]
-/// cannot currently handle fields like `Vec<SomeWidget>`.
-///
-/// Whenever the number of child widgets changes or child widgets are replaced,
-/// one must send [`TkAction::RECONFIGURE`].
-/// (TODO: this is slow. Find an option for partial reconfigures. This requires
-/// better widget identifiers; see #91.)
+/// cannot currently handle fields like `Vec<SomeWidget>`. Additionally, any
+/// parent adding child widgets must ensure they get configured, either via
+/// [`TkAction::RECONFIGURE`] or via [`SetRectMgr::configure`].
 ///
 /// [`derive(Widget)`]: https://docs.rs/kas/latest/kas/macros/index.html#the-derivewidget-macro
 pub trait WidgetChildren: WidgetCore {
@@ -172,7 +171,7 @@ pub trait WidgetChildren: WidgetCore {
     ///
     /// Warning: directly adjusting a widget without requiring reconfigure or
     /// redraw may break the UI. If a widget is replaced, a reconfigure **must**
-    /// be requested. This can be done via [`EventMgr::send_action`].
+    /// be requested. This can be done via [`EventState::send_action`].
     /// This method may be removed in the future.
     fn get_child_mut(&mut self, index: usize) -> Option<&mut dyn WidgetConfig>;
 
@@ -251,37 +250,45 @@ pub trait WidgetChildren: WidgetCore {
 // TODO(specialization): provide a blanket implementation, so that users only
 // need implement manually when they have something to configure.
 pub trait WidgetConfig: Layout {
+    /// Pre-configure widget
+    ///
+    /// Widgets are *configured* on window creation (before sizing) and when
+    /// [`TkAction::RECONFIGURE`] is sent. Child-widgets may alternatively be
+    /// configured locally by calling [`SetRectMgr::configure`].
+    ///
+    /// Configuration happens at least once
+    /// before sizing and drawing, and may be repeated at a later time.
+    /// Configuration happens in this order: (1) `pre_configure`,
+    /// (2) recurse over children, (3) `configure`.
+    ///
+    /// This method assigns the widget's [`WidgetId`] and may be used to
+    /// affect the manager in ways which influence the child, for example
+    /// [`EventState::new_accel_layer`].
+    fn pre_configure(&mut self, mgr: &mut SetRectMgr, id: WidgetId) {
+        let _ = mgr;
+        self.core_data_mut().id = id;
+    }
+
     /// Configure widget
     ///
-    /// Widgets are *configured* on window creation and when
-    /// [`TkAction::RECONFIGURE`] is sent.
+    /// Widgets are *configured* on window creation (before sizing) and when
+    /// [`TkAction::RECONFIGURE`] is sent. Child-widgets may alternatively be
+    /// configured locally by calling [`SetRectMgr::configure`].
+    ///
+    /// Configuration happens at least once
+    /// before sizing and drawing, and may be repeated at a later time.
+    /// Configuration happens in this order: (1) `pre_configure`,
+    /// (2) recurse over children, (3) `configure`.
     ///
-    /// Configure is called before resizing (but after calculation of the
-    /// initial window size). This method is called after
-    /// a [`WidgetId`] has been assigned to self, and after `configure` has
-    /// been called on each child.
+    /// This method may be used to perform local initialization and bindings,
+    /// e.g. [`EventState::add_accel_keys`].
     ///
     /// It is not advised to perform any action requiring a reconfigure (e.g.
     /// adding a child widget) during configure due to the possibility of
     /// getting stuck in a reconfigure-loop. See issue kas#91 for more on this.
     /// KAS has a crude mechanism to detect this and panic.
-    ///
-    /// The default implementation of this method does nothing.
-    fn configure(&mut self, _: &mut EventMgr) {}
-
-    /// Configure self and children
-    ///
-    /// In most cases one should not override the default implementation of this
-    /// method but instead use [`WidgetConfig::configure`]; the exception is
-    /// widgets with pop-ups.
-    fn configure_recurse(&mut self, mut cmgr: ConfigureManager) {
-        self.core_data_mut().id = cmgr.get_id();
-        for i in 0..self.num_children() {
-            if let Some(w) = self.get_child_mut(i) {
-                w.configure_recurse(cmgr.child(i));
-            }
-        }
-        self.configure(cmgr.mgr());
+    fn configure(&mut self, mgr: &mut SetRectMgr) {
+        let _ = mgr;
     }
 
     /// Is this widget navigable via Tab key?

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

@@ -9,7 +9,7 @@
 #![allow(clippy::precedence)]
 
 use crate::cast::{Cast, Conv};
-use std::cmp::{Eq, PartialEq};
+use std::cmp::Ordering;
 use std::hash::{Hash, Hasher};
 use std::iter::once;
 use std::marker::PhantomData;
@@ -203,7 +203,8 @@ impl<'a> Iterator for PathIter<'a> {
 /// reference counting internally.
 ///
 /// Identifiers are assigned when configured and when re-configured
-/// (via [`crate::TkAction::RECONFIGURE`]). Since user-code is not notified of a
+/// (via [`crate::TkAction::RECONFIGURE`] or [`crate::layout::SetRectMgr::configure`]).
+/// Since user-code is not notified of a
 /// re-configure, user-code should not store a `WidgetId`.
 #[derive(Clone)]
 pub struct WidgetId(IntOrPtr);
@@ -451,6 +452,22 @@ impl PartialEq for WidgetId {
 }
 impl Eq for WidgetId {}
 
+impl PartialOrd for WidgetId {
+    fn partial_cmp(&self, rhs: &Self) -> Option<Ordering> {
+        Some(self.cmp(rhs))
+    }
+}
+
+impl Ord for WidgetId {
+    fn cmp(&self, rhs: &Self) -> Ordering {
+        match (self.0.get(), rhs.0.get()) {
+            (Variant::Invalid, _) | (_, Variant::Invalid) => panic!("WidgetId::cmp: invalid id"),
+            (Variant::Int(x), Variant::Int(y)) => x.cmp(&y),
+            _ => self.iter_path().cmp(rhs.iter_path()),
+        }
+    }
+}
+
 impl Hash for WidgetId {
     fn hash<H: Hasher>(&self, state: &mut H) {
         match self.0.get() {
@@ -567,19 +584,47 @@ mod test {
     #[test]
     #[should_panic]
     fn test_partial_eq_invalid_1() {
-        assert_eq!(WidgetId::INVALID, WidgetId::INVALID);
+        let _ = WidgetId::INVALID == WidgetId::INVALID;
     }
 
     #[test]
     #[should_panic]
     fn test_partial_eq_invalid_2() {
-        assert_eq!(WidgetId::ROOT, WidgetId::INVALID);
+        let _ = WidgetId::ROOT == WidgetId::INVALID;
     }
 
     #[test]
     #[should_panic]
     fn test_partial_eq_invalid_3() {
-        assert_eq!(WidgetId::INVALID, WidgetId::ROOT);
+        let _ = WidgetId::INVALID == WidgetId::ROOT;
+    }
+
+    #[test]
+    fn test_ord() {
+        let root = WidgetId::ROOT;
+        let c_0 = root.make_child(0);
+        let c_0_0 = c_0.make_child(0);
+        assert!(root < c_0);
+        assert!(c_0 < c_0_0);
+
+        let c_1 = root.make_child(1);
+        assert!(c_0_0 < c_1);
+        assert!(c_1 < root.make_child(8));
+
+        let d_0 = WidgetId(IntOrPtr::new_iter([0].iter().cloned()));
+        let d_0_0 = WidgetId(IntOrPtr::new_iter([0, 0].iter().cloned()));
+        let d_1 = WidgetId(IntOrPtr::new_iter([1].iter().cloned()));
+        assert_eq!(d_0.cmp(&c_0), Ordering::Equal);
+        assert_eq!(d_0_0.cmp(&c_0_0), Ordering::Equal);
+        assert_eq!(d_1.cmp(&c_1), Ordering::Equal);
+        assert!(d_0 < d_0_0);
+        assert!(d_0_0 < d_1);
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_ord_invalid() {
+        let _ = WidgetId::INVALID < WidgetId::ROOT;
     }
 
     #[test]

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

@@ -414,7 +414,7 @@ impl TextInput {
                 match source {
                     PressSource::Touch(touch_id) => {
                         self.touch_phase = TouchPhase::Start(touch_id, coord);
-                        let delay = mgr.config().touch_text_sel_delay();
+                        let delay = mgr.config().touch_select_delay();
                         mgr.update_on_timer(delay, w_id, PAYLOAD_SELECT);
                         Action::Focus
                     }

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

@@ -21,7 +21,7 @@ use std::time::Duration;
 /// This is serializable (using `feature = "config"`) with the following fields:
 ///
 /// > `menu_delay_ms`: `u32` (milliseconds) \
-/// > `touch_text_sel_delay_ms`: `u32` (milliseconds) \
+/// > `touch_select_delay_ms`: `u32` (milliseconds) \
 /// > `scroll_flick_timeout_ms`: `u32` (milliseconds) \
 /// > `scroll_flick_mul`: `f32` (unitless, applied each second) \
 /// > `scroll_flick_sub`: `f32` (pixels per second) \
@@ -37,48 +37,45 @@ use std::time::Duration;
 #[cfg_attr(feature = "config", derive(Serialize, Deserialize))]
 pub struct Config {
     #[cfg_attr(feature = "config", serde(default = "defaults::menu_delay_ms"))]
-    menu_delay_ms: u32,
+    pub menu_delay_ms: u32,
 
-    #[cfg_attr(
-        feature = "config",
-        serde(default = "defaults::touch_text_sel_delay_ms")
-    )]
-    touch_text_sel_delay_ms: u32,
+    #[cfg_attr(feature = "config", serde(default = "defaults::touch_select_delay_ms"))]
+    pub touch_select_delay_ms: u32,
 
     #[cfg_attr(
         feature = "config",
         serde(default = "defaults::scroll_flick_timeout_ms")
     )]
-    scroll_flick_timeout_ms: u32,
+    pub scroll_flick_timeout_ms: u32,
 
     #[cfg_attr(feature = "config", serde(default = "defaults::scroll_flick_mul"))]
-    scroll_flick_mul: f32,
+    pub scroll_flick_mul: f32,
 
     #[cfg_attr(feature = "config", serde(default = "defaults::scroll_flick_sub"))]
-    scroll_flick_sub: f32,
+    pub scroll_flick_sub: f32,
 
     #[cfg_attr(feature = "config", serde(default = "defaults::pan_dist_thresh"))]
-    pan_dist_thresh: f32,
+    pub pan_dist_thresh: f32,
 
     #[cfg_attr(feature = "config", serde(default = "defaults::mouse_pan"))]
-    mouse_pan: MousePan,
+    pub mouse_pan: MousePan,
     #[cfg_attr(feature = "config", serde(default = "defaults::mouse_text_pan"))]
-    mouse_text_pan: MousePan,
+    pub mouse_text_pan: MousePan,
 
     #[cfg_attr(feature = "config", serde(default = "defaults::mouse_nav_focus"))]
-    mouse_nav_focus: bool,
+    pub mouse_nav_focus: bool,
     #[cfg_attr(feature = "config", serde(default = "defaults::touch_nav_focus"))]
-    touch_nav_focus: bool,
+    pub touch_nav_focus: bool,
 
     #[cfg_attr(feature = "config", serde(default = "Shortcuts::platform_defaults"))]
-    shortcuts: Shortcuts,
+    pub shortcuts: Shortcuts,
 }
 
 impl Default for Config {
     fn default() -> Self {
         Config {
             menu_delay_ms: defaults::menu_delay_ms(),
-            touch_text_sel_delay_ms: defaults::touch_text_sel_delay_ms(),
+            touch_select_delay_ms: defaults::touch_select_delay_ms(),
             scroll_flick_timeout_ms: defaults::scroll_flick_timeout_ms(),
             scroll_flick_mul: defaults::scroll_flick_mul(),
             scroll_flick_sub: defaults::scroll_flick_sub(),
@@ -129,10 +126,10 @@ impl WindowConfig {
         Duration::from_millis(self.config.borrow().menu_delay_ms.cast())
     }
 
-    /// Delay before switching from panning to text-selection mode
+    /// Delay before switching from panning to (text) selection mode
     #[inline]
-    pub fn touch_text_sel_delay(&self) -> Duration {
-        Duration::from_millis(self.config.borrow().touch_text_sel_delay_ms.cast())
+    pub fn touch_select_delay(&self) -> Duration {
+        Duration::from_millis(self.config.borrow().touch_select_delay_ms.cast())
     }
 
     /// Controls activation of glide/momentum scrolling
@@ -222,6 +219,8 @@ impl Config {
 /// acceptable (equivalent to touch scrolling).
 #[derive(Clone, Copy, Debug, Hash, PartialEq)]
 #[cfg_attr(feature = "config", derive(Serialize, Deserialize))]
+#[derive(num_enum::IntoPrimitive, num_enum::TryFromPrimitive)]
+#[repr(u8)]
 pub enum MousePan {
     /// Disable
     Never,
@@ -251,7 +250,7 @@ mod defaults {
     pub fn menu_delay_ms() -> u32 {
         250
     }
-    pub fn touch_text_sel_delay_ms() -> u32 {
+    pub fn touch_select_delay_ms() -> u32 {
         1000
     }
     pub fn scroll_flick_timeout_ms() -> u32 {

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

@@ -9,7 +9,7 @@
 use serde::{Deserialize, Serialize};
 
 #[allow(unused)]
-use super::{EventMgr, GrabMode, Response, SendEvent}; // for doc-links
+use super::{EventMgr, EventState, GrabMode, Response, SendEvent}; // for doc-links
 use super::{MouseButton, UpdateHandle, VirtualKeyCode};
 
 use crate::geom::{Coord, DVec2, Offset};
@@ -51,7 +51,7 @@ pub enum Event {
     /// Widget receives a character of text input
     ///
     /// This is only received by a widget with character focus (see
-    /// [`EventMgr::request_char_focus`]). There is no overlap with
+    /// [`EventState::request_char_focus`]). There is no overlap with
     /// [`Event::Command`]: key presses result in at most one of these events
     /// being sent to a widget.
     ReceivedCharacter(char),
@@ -171,15 +171,15 @@ pub enum Event {
     /// Update from a timer
     ///
     /// This event is received after requesting timed wake-up(s)
-    /// (see [`EventMgr::update_on_timer`]).
+    /// (see [`EventState::update_on_timer`]).
     ///
     /// The `u64` payload may be used to identify the corresponding
-    /// [`EventMgr::update_on_timer`] call.
+    /// [`EventState::update_on_timer`] call.
     TimerUpdate(u64),
     /// Update triggerred via an [`UpdateHandle`]
     ///
     /// This event may be received after registering an [`UpdateHandle`] via
-    /// [`EventMgr::update_on_handle`].
+    /// [`EventState::update_on_handle`].
     ///
     /// A user-defined payload is passed. Interpretation of this payload is
     /// user-defined and unfortunately not type safe.