Merge pull request #818 from xStrom/bloom_docs

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

@@ -240,49 +240,62 @@ impl<'a> EventCtx<'a> {
 
     /// The focus status of a widget.
     ///
+    /// Returns `true` if this specific widget is focused.
+    /// To check if any descendants are focused use [`has_focus`].
+    ///
     /// Focus means that the widget receives keyboard events.
     ///
     /// A widget can request focus using the [`request_focus`] method.
-    /// This will generally result in a separate event propagation of
-    /// a `FocusChanged` method, including sending `false` to the previous
-    /// widget that held focus.
+    /// It's also possible to register for automatic focus via [`register_for_focus`].
     ///
-    /// Only one leaf widget at a time has focus. However, in a container
-    /// hierarchy, all ancestors of that leaf widget are also invoked with
-    /// `FocusChanged(true)`.
+    /// If a widget gains or loses focus it will get a [`LifeCycle::FocusChanged`] event.
     ///
-    /// Discussion question: is "is_focused" a better name?
+    /// Only one widget at a time is focused. However due to the way events are routed,
+    /// all ancestors of that widget will also receive keyboard events.
     ///
     /// [`request_focus`]: struct.EventCtx.html#method.request_focus
+    /// [`register_for_focus`]: struct.LifeCycleCtx.html#method.register_for_focus
+    /// [`LifeCycle::FocusChanged`]: enum.LifeCycle.html#variant.FocusChanged
+    /// [`has_focus`]: struct.EventCtx.html#method.has_focus
+    pub fn is_focused(&self) -> bool {
+        self.focus_widget == Some(self.widget_id())
+    }
+
+    /// The (tree) focus status of a widget.
+    ///
+    /// Returns `true` if either this specific widget or any one of its descendants is focused.
+    /// To check if only this specific widget is focused use [`is_focused`].
+    ///
+    /// See [`is_focused`] for more information about focus.
+    ///
+    /// [`is_focused`]: struct.EventCtx.html#method.is_focused
     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`].
-    ///
-    /// [`has_focus`]: struct.EventCtx.html#method.has_focus
-    pub fn is_focused(&self) -> bool {
-        self.focus_widget == Some(self.widget_id())
-    }
-
     /// Request keyboard focus.
     ///
-    /// See [`has_focus`] for more information.
+    /// See [`is_focused`] for more information about focus.
     ///
-    /// [`has_focus`]: struct.EventCtx.html#method.has_focus
+    /// [`is_focused`]: struct.EventCtx.html#method.is_focused
     pub fn request_focus(&mut self) {
         self.base_state.request_focus = Some(FocusChange::Focus(self.widget_id()));
     }
 
     /// Transfer focus to the next focusable widget.
     ///
     /// This should only be called by a widget that currently has focus.
+    ///
+    /// See [`is_focused`] for more information about focus.
+    ///
+    /// [`is_focused`]: struct.EventCtx.html#method.is_focused
     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");
@@ -292,8 +305,12 @@ impl<'a> EventCtx<'a> {
     /// Transfer focus to the previous focusable widget.
     ///
     /// This should only be called by a widget that currently has focus.
+    ///
+    /// See [`is_focused`] for more information about focus.
+    ///
+    /// [`is_focused`]: struct.EventCtx.html#method.is_focused
     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");
@@ -303,8 +320,12 @@ impl<'a> EventCtx<'a> {
     /// Give up focus.
     ///
     /// This should only be called by a widget that currently has focus.
+    ///
+    /// See [`is_focused`] for more information about focus.
+    ///
+    /// [`is_focused`]: struct.EventCtx.html#method.is_focused
     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 +435,13 @@ 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.
+    ///
+    /// See [`EventCtx::is_focused`] for more information about focus.
+    ///
+    /// [`LifeCycle::WidgetAdded`]: enum.Lifecycle.html#variant.WidgetAdded
+    /// [`EventCtx::is_focused`]: struct.EventCtx.html#method.is_focused
     pub fn register_for_focus(&mut self) {
         self.base_state.focus_chain.push(self.widget_id());
     }
@@ -557,20 +585,33 @@ impl<'a, 'b: 'a> PaintCtx<'a, 'b> {
         self.base_state.size()
     }
 
-    /// Query the focus state of the widget.
+    /// The focus status of a widget.
+    ///
+    /// Returns `true` if this specific widget is focused.
+    /// To check if any descendants are focused use [`has_focus`].
+    ///
+    /// See [`EventCtx::is_focused`] for more information about focus.
     ///
-    /// This is true only if this widget has focus.
+    /// [`has_focus`]: #method.has_focus
+    /// [`EventCtx::is_focused`]: struct.EventCtx.html#method.is_focused
     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.
+    ///
+    /// Returns `true` if either this specific widget or any one of its descendants is focused.
+    /// To check if only this specific widget is focused use [`is_focused`].
+    ///
+    /// See [`EventCtx::is_focused`] for more information about focus.
     ///
-    /// See [`has_focus`](struct.EventCtx.html#method.has_focus).
+    /// [`is_focused`]: #method.is_focused
+    /// [`EventCtx::is_focused`]: struct.EventCtx.html#method.is_focused
     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())
     }

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/event.rs

@@ -186,11 +186,13 @@ pub enum LifeCycle {
     },
     /// Called when the focus status changes.
     ///
-    /// This will always be called immediately after an event where a widget
-    /// has requested focus.
+    /// This will always be called immediately after a new widget gains focus.
+    /// The newly focused widget will receive this with `true` and the widget
+    /// that lost focus will receive this with `false`.
     ///
-    /// See [`has_focus`](struct.EventCtx.html#method.has_focus) for
-    /// discussion about the focus status.
+    /// See [`EventCtx::is_focused`] for more information about focus.
+    ///
+    /// [`EventCtx::is_focused`]: struct.EventCtx.html#method.is_focused
     FocusChanged(bool),
     /// Testing only: request the `BaseState` of a specific widget.
     ///

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);
     })
 }