Add documentation, e.g. around bloom usage.

Author: xStrom

druid-shell/src/common_util.rs

@@ -55,7 +55,7 @@ impl<F: FnOnce(&dyn Any) + Send> IdleCallback for F {
 ///
 /// This can be used safely from multiple threads.
 ///
-/// The counter will overflow if `next()` iscalled 2^64 - 2 times.
+/// The counter will overflow if `next()` is called 2^64 - 2 times.
 /// If this is possible for your application, and reuse would be undesirable,
 /// use something else.
 pub struct Counter(AtomicU64);

druid/src/bloom.rs

@@ -71,10 +71,12 @@ impl<T: ?Sized + Hash> Bloom<T> {
         self.entry_count += 1;
     }
 
-    /// Return whether an item exists in the filter.
+    /// Returns `true` if the item may have been added to the filter.
     ///
     /// This can return false positives, but never false negatives.
-    pub fn contains(&self, item: &T) -> bool {
+    /// Thus `true` means that the item may have been added - or not,
+    /// while `false` means that the item has definitely not been added.
+    pub fn may_contain(&self, item: &T) -> bool {
         let mask = self.make_bit_mask(item);
         self.bits & mask == mask
     }
@@ -134,11 +136,11 @@ mod tests {
         let mut bloom = Bloom::default();
         for i in 0..100 {
             bloom.add(&i);
-            assert!(bloom.contains(&i));
+            assert!(bloom.may_contain(&i));
         }
         bloom.clear();
         for i in 0..100 {
-            assert!(!bloom.contains(&i));
+            assert!(!bloom.may_contain(&i));
         }
     }
 
@@ -147,18 +149,18 @@ mod tests {
         let mut bloom1 = Bloom::default();
         bloom1.add(&0);
         bloom1.add(&1);
-        assert!(!bloom1.contains(&2));
-        assert!(!bloom1.contains(&3));
+        assert!(!bloom1.may_contain(&2));
+        assert!(!bloom1.may_contain(&3));
         let mut bloom2 = Bloom::default();
         bloom2.add(&2);
         bloom2.add(&3);
-        assert!(!bloom2.contains(&0));
-        assert!(!bloom2.contains(&1));
+        assert!(!bloom2.may_contain(&0));
+        assert!(!bloom2.may_contain(&1));
 
         let bloom3 = bloom1.union(bloom2);
-        assert!(bloom3.contains(&0));
-        assert!(bloom3.contains(&1));
-        assert!(bloom3.contains(&2));
-        assert!(bloom3.contains(&3));
+        assert!(bloom3.may_contain(&0));
+        assert!(bloom3.may_contain(&1));
+        assert!(bloom3.may_contain(&2));
+        assert!(bloom3.may_contain(&3));
     }
 }

druid/src/contexts.rs

@@ -238,7 +238,7 @@ impl<'a> EventCtx<'a> {
         self.is_handled
     }
 
-    /// The focus status of a widget.
+    /// The (tree) focus status of a widget.
     ///
     /// Focus means that the widget receives keyboard events.
     ///
@@ -251,20 +251,21 @@ impl<'a> EventCtx<'a> {
     /// hierarchy, all ancestors of that leaf widget are also invoked with
     /// `FocusChanged(true)`.
     ///
-    /// Discussion question: is "is_focused" a better name?
+    /// Discussion question: is "may_have_focus" a better name?
     ///
     /// [`request_focus`]: struct.EventCtx.html#method.request_focus
     pub fn has_focus(&self) -> bool {
+        // The bloom filter we're checking can return false positives.
         let is_child = self
             .focus_widget
-            .map(|id| self.base_state.children.contains(&id))
+            .map(|id| self.base_state.children.may_contain(&id))
             .unwrap_or(false);
         is_child || self.focus_widget == Some(self.widget_id())
     }
 
-    /// The (leaf) focus status of a widget. See [`has_focus`].
+    /// The (leaf) focus status of a widget.
     ///
-    /// [`has_focus`]: struct.EventCtx.html#method.has_focus
+    /// See [`has_focus`](struct.EventCtx.html#method.has_focus).
     pub fn is_focused(&self) -> bool {
         self.focus_widget == Some(self.widget_id())
     }
@@ -282,7 +283,7 @@ impl<'a> EventCtx<'a> {
     ///
     /// This should only be called by a widget that currently has focus.
     pub fn focus_next(&mut self) {
-        if self.focus_widget == Some(self.widget_id()) {
+        if self.is_focused() {
             self.base_state.request_focus = Some(FocusChange::Next);
         } else {
             log::warn!("focus_next can only be called by the currently focused widget");
@@ -293,7 +294,7 @@ impl<'a> EventCtx<'a> {
     ///
     /// This should only be called by a widget that currently has focus.
     pub fn focus_prev(&mut self) {
-        if self.focus_widget == Some(self.widget_id()) {
+        if self.is_focused() {
             self.base_state.request_focus = Some(FocusChange::Previous);
         } else {
             log::warn!("focus_prev can only be called by the currently focused widget");
@@ -304,7 +305,7 @@ impl<'a> EventCtx<'a> {
     ///
     /// This should only be called by a widget that currently has focus.
     pub fn resign_focus(&mut self) {
-        if self.focus_widget == Some(self.widget_id()) {
+        if self.is_focused() {
             self.base_state.request_focus = Some(FocusChange::Resign);
         } else {
             log::warn!("resign_focus can only be called by the currently focused widget");
@@ -414,6 +415,8 @@ impl<'a> LifeCycleCtx<'a> {
     }
 
     /// Register this widget to be eligile to accept focus automatically.
+    ///
+    /// This should only be called in response to a `LifeCycle::WidgetAdded` event.
     pub fn register_for_focus(&mut self) {
         self.base_state.focus_chain.push(self.widget_id());
     }
@@ -557,24 +560,25 @@ impl<'a, 'b: 'a> PaintCtx<'a, 'b> {
         self.base_state.size()
     }
 
-    /// Query the focus state of the widget.
-    ///
-    /// This is true only if this widget has focus.
-    pub fn is_focused(&self) -> bool {
-        self.focus_widget == Some(self.widget_id())
-    }
-
-    /// The focus status of a widget.
+    /// The (tree) focus status of a widget.
     ///
     /// See [`has_focus`](struct.EventCtx.html#method.has_focus).
     pub fn has_focus(&self) -> bool {
+        // The bloom filter we're checking can return false positives.
         let is_child = self
             .focus_widget
-            .map(|id| self.base_state.children.contains(&id))
+            .map(|id| self.base_state.children.may_contain(&id))
             .unwrap_or(false);
         is_child || self.focus_widget == Some(self.widget_id())
     }
 
+    /// The (leaf) focus status of a widget.
+    ///
+    /// See [`has_focus`](struct.EventCtx.html#method.has_focus).
+    pub fn is_focused(&self) -> bool {
+        self.focus_widget == Some(self.widget_id())
+    }
+
     /// Returns the currently visible [`Region`].
     ///
     /// [`Region`]: struct.Region.html

druid/src/core.rs

@@ -369,7 +369,7 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
         let mut child_ctx = EventCtx {
             cursor: ctx.cursor,
             command_queue: ctx.command_queue,
-            window: &ctx.window,
+            window: ctx.window,
             window_id: ctx.window_id,
             base_state: &mut self.state,
             had_active,
@@ -446,7 +446,9 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
                 Target::Window(_) => Event::Command(cmd.clone()),
                 Target::Widget(id) if *id == child_ctx.widget_id() => Event::Command(cmd.clone()),
                 Target::Widget(id) => {
-                    recurse = child_ctx.base_state.children.contains(id);
+                    // Recurse when the target widget could be our descendant.
+                    // The bloom filter we're checking can return false positives.
+                    recurse = child_ctx.base_state.children.may_contain(id);
                     Event::TargetedCommand(*target, cmd.clone())
                 }
                 Target::Global => panic!("Target::Global should be converted before WidgetPod"),
@@ -496,7 +498,6 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
                         self.state.children.clear();
                         self.state.focus_chain.clear();
                     }
-
                     self.state.children_changed
                 }
             }
@@ -517,8 +518,10 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
                     self.inner.lifecycle(ctx, &event, data, env);
                     false
                 } else {
-                    old.map(|id| self.state.children.contains(&id))
-                        .or_else(|| new.map(|id| self.state.children.contains(&id)))
+                    // Recurse when the target widgets could be our descendants.
+                    // The bloom filter we're checking can return false positives.
+                    old.map(|id| self.state.children.may_contain(&id))
+                        .or_else(|| new.map(|id| self.state.children.may_contain(&id)))
                         .unwrap_or(false)
                 }
             }
@@ -532,7 +535,9 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
                     state_cell.set(self.state.clone());
                     false
                 } else {
-                    self.state.children.contains(&widget)
+                    // Recurse when the target widget could be our descendant.
+                    // The bloom filter we're checking can return false positives.
+                    self.state.children.may_contain(&widget)
                 }
             }
             #[cfg(test)]
@@ -542,13 +547,12 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
             }
         };
 
-        let mut child_ctx = LifeCycleCtx {
-            command_queue: ctx.command_queue,
-            base_state: &mut self.state,
-            window_id: ctx.window_id,
-        };
-
         if recurse {
+            let mut child_ctx = LifeCycleCtx {
+                command_queue: ctx.command_queue,
+                base_state: &mut self.state,
+                window_id: ctx.window_id,
+            };
             self.inner.lifecycle(&mut child_ctx, event, data, env);
         }
 
@@ -691,9 +695,9 @@ mod tests {
         let env = Env::default();
 
         widget.lifecycle(&mut ctx, &LifeCycle::WidgetAdded, &None, &env);
-        assert!(ctx.base_state.children.contains(&ID_1));
-        assert!(ctx.base_state.children.contains(&ID_2));
-        assert!(ctx.base_state.children.contains(&ID_3));
+        assert!(ctx.base_state.children.may_contain(&ID_1));
+        assert!(ctx.base_state.children.may_contain(&ID_2));
+        assert!(ctx.base_state.children.may_contain(&ID_3));
         assert_eq!(ctx.base_state.children.entry_count(), 7);
     }
 }

druid/src/tests/mod.rs

@@ -275,13 +275,13 @@ fn child_tracking() {
         harness.send_initial_events();
         let root = harness.get_state(id_4);
         assert_eq!(root.children.entry_count(), 3);
-        assert!(root.children.contains(&id_1));
-        assert!(root.children.contains(&id_2));
-        assert!(root.children.contains(&id_3));
+        assert!(root.children.may_contain(&id_1));
+        assert!(root.children.may_contain(&id_2));
+        assert!(root.children.may_contain(&id_3));
 
         let split = harness.get_state(id_3);
-        assert!(split.children.contains(&id_1));
-        assert!(split.children.contains(&id_2));
+        assert!(split.children.may_contain(&id_1));
+        assert!(split.children.may_contain(&id_2));
         assert_eq!(split.children.entry_count(), 2);
     });
 }
@@ -302,18 +302,18 @@ fn register_after_adding_child() {
     Harness::create(String::new(), widget, |harness| {
         harness.send_initial_events();
 
-        assert!(harness.get_state(id_5).children.contains(&id_6));
-        assert!(harness.get_state(id_5).children.contains(&id_1));
-        assert!(harness.get_state(id_5).children.contains(&id_4));
+        assert!(harness.get_state(id_5).children.may_contain(&id_6));
+        assert!(harness.get_state(id_5).children.may_contain(&id_1));
+        assert!(harness.get_state(id_5).children.may_contain(&id_4));
         assert_eq!(harness.get_state(id_5).children.entry_count(), 3);
 
         harness.submit_command(REPLACE_CHILD, None);
 
-        assert!(harness.get_state(id_5).children.contains(&id_6));
-        assert!(harness.get_state(id_5).children.contains(&id_4));
-        assert!(harness.get_state(id_5).children.contains(&id_7));
-        assert!(harness.get_state(id_5).children.contains(&id_2));
-        assert!(harness.get_state(id_5).children.contains(&id_3));
+        assert!(harness.get_state(id_5).children.may_contain(&id_6));
+        assert!(harness.get_state(id_5).children.may_contain(&id_4));
+        assert!(harness.get_state(id_5).children.may_contain(&id_7));
+        assert!(harness.get_state(id_5).children.may_contain(&id_2));
+        assert!(harness.get_state(id_5).children.may_contain(&id_3));
         assert_eq!(harness.get_state(id_5).children.entry_count(), 5);
     })
 }

druid/src/widget/textbox.rs

@@ -240,7 +240,7 @@ impl Widget<String> for TextBox {
 
         match event {
             Event::MouseDown(mouse) => {
-                if !ctx.has_focus() {
+                if !ctx.is_focused() {
                     ctx.request_focus();
                 }
                 ctx.set_active(true);
@@ -281,7 +281,7 @@ impl Widget<String> for TextBox {
                 }
             }
             Event::Command(ref cmd)
-                if ctx.has_focus()
+                if ctx.is_focused()
                     && (cmd.selector == crate::commands::COPY
                         || cmd.selector == crate::commands::CUT) =>
             {
@@ -392,9 +392,9 @@ impl Widget<String> for TextBox {
         let placeholder_color = env.get(theme::PLACEHOLDER_COLOR);
         let cursor_color = env.get(theme::CURSOR_COLOR);
 
-        let has_focus = ctx.has_focus();
+        let is_focused = ctx.is_focused();
 
-        let border_color = if has_focus {
+        let border_color = if is_focused {
             env.get(theme::PRIMARY_LIGHT)
         } else {
             env.get(theme::BORDER_DARK)
@@ -449,7 +449,7 @@ impl Widget<String> for TextBox {
             rc.draw_text(&text_layout, text_pos, color);
 
             // Paint the cursor if focused and there's no selection
-            if has_focus && self.cursor_on && self.selection.is_caret() {
+            if is_focused && self.cursor_on && self.selection.is_caret() {
                 let cursor_x = self.x_for_offset(&text_layout, self.cursor());
                 let xy = text_pos + Vec2::new(cursor_x, 2. - font_size);
                 let x2y2 = xy + Vec2::new(0., font_size + 2.);

druid/src/widget/widget.rs

@@ -46,6 +46,11 @@ use super::prelude::*;
 /// [`LifeCycleCtx::widget_id`]: struct.LifeCycleCtx.html#method.widget_id
 /// [`WidgetExt::with_id`]: trait.WidgetExt.html#method.with_id
 /// [`IdentityWrapper`]: widget/struct.IdentityWrapper.html
+//
+// The widget id does not necessarily identify only a single widget
+// in the strictest sense. Internally WidgetPod uses its child's id,
+// and the window base state uses the root widget's id.
+//
 // this is NonZeroU64 because we regularly store Option<WidgetId>
 #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
 pub struct WidgetId(NonZeroU64);

druid/src/window.rs

@@ -72,8 +72,12 @@ impl<T: Data> Window<T> {
         &self.root.state().focus_chain
     }
 
+    /// Returns `true` if the provided widget may be in this window,
+    /// but it may also be a false positive.
+    /// However when this returns `false` the widget is definitely not in this window.
     pub(crate) fn may_contain_widget(&self, widget_id: WidgetId) -> bool {
-        self.root.state().children.contains(&widget_id)
+        // The bloom filter we're checking can return false positives.
+        self.root.state().children.may_contain(&widget_id)
     }
 
     pub(crate) fn set_menu(&mut self, mut menu: MenuDesc<T>, data: &T, env: &Env) {