Remove bloom filter (#536)

The filter was only used for RouteFocusChanged which is about to be
removed. Since it was a performance feature, we can remove it early.

Cargo.lock

@@ -1883,7 +1883,6 @@ dependencies = [
  "cursor-icon",
  "dpi",
  "float-cmp",
- "fnv",
  "futures-intrusive",
  "image",
  "insta",

Cargo.toml

@@ -115,7 +115,6 @@ winit = "0.30.4"
 tracing = { version = "0.1.40", default-features = false }
 smallvec = "1.13.2"
 dpi = "0.1.1"
-fnv = "1.0.7"
 image = { version = "0.25.2", default-features = false }
 web-time = "1.1.0"
 bitflags = "2.6.0"

masonry/Cargo.toml

@@ -26,7 +26,6 @@ parley.workspace = true
 winit.workspace = true
 smallvec.workspace = true
 tracing = { workspace = true, features = ["default"] }
-fnv.workspace = true
 image.workspace = true
 once_cell = "1.19.0"
 serde = { version = "1.0.204", features = ["derive"] }

masonry/src/bloom.rs

@@ -1,157 +0,0 @@
-// Copyright 2020 the Xilem Authors and the Druid Authors
-// SPDX-License-Identifier: Apache-2.0
-
-//! A simple Bloom filter, used to track child widgets.
-
-use std::hash::{Hash, Hasher};
-use std::marker::PhantomData;
-
-// TODO - Once the bloom filter goes away, remove the fnv dependency.
-use fnv::FnvHasher;
-
-const NUM_BITS: u64 = 64;
-
-// the 'offset_basis' for the fnv-1a hash algorithm.
-// see http://www.isthe.com/chongo/tech/comp/fnv/index.html#FNV-param
-//
-// The first of these is the one described in the algorithm, the second is random.
-const OFFSET_ONE: u64 = 0xcbf2_9ce4_8422_2325;
-const OFFSET_TWO: u64 = 0xe10_3ad8_2dad_8028;
-
-/// A very simple Bloom filter optimized for small values.
-#[derive(Clone, Copy)]
-pub(crate) struct Bloom<T: ?Sized> {
-    bits: u64,
-    data: PhantomData<T>,
-    entry_count: usize,
-}
-
-#[allow(dead_code)]
-impl<T: ?Sized + Hash> Bloom<T> {
-    /// Create a new filter.
-    pub fn new() -> Self {
-        Self::default()
-    }
-
-    /// Returns the number of items that have been added to the filter.
-    ///
-    /// Does not count unique entries; this is just the number of times
-    /// `add()` was called since the filter was created or last `clear()`ed.
-    // it feels wrong to call this 'len'?
-    #[cfg(test)]
-    pub fn entry_count(&self) -> usize {
-        self.entry_count
-    }
-
-    /// Return the raw bits of this filter.
-    #[allow(dead_code)]
-    pub fn to_raw(&self) -> u64 {
-        self.bits
-    }
-
-    /// Remove all entries from the filter.
-    pub fn clear(&mut self) {
-        self.bits = 0;
-        self.entry_count = 0;
-    }
-
-    /// Add an item to the filter.
-    pub fn add(&mut self, item: &T) {
-        let mask = self.make_bit_mask(item);
-        self.bits |= mask;
-        self.entry_count += 1;
-    }
-
-    /// Returns `true` if the item may have been added to the filter.
-    ///
-    /// This can return false positives, but never false negatives.
-    /// 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
-    }
-
-    /// Create a new `Bloom` with the items from both filters.
-    pub fn union(&self, other: Bloom<T>) -> Bloom<T> {
-        Bloom {
-            bits: self.bits | other.bits,
-            data: PhantomData,
-            entry_count: self.entry_count + other.entry_count,
-        }
-    }
-
-    #[inline]
-    fn make_bit_mask(&self, item: &T) -> u64 {
-        //NOTE: we use two hash functions, which performs better than a single hash
-        // with smaller numbers of items, but poorer with more items. Threshold
-        // (given 64 bits) is ~30 items.
-        // The reasoning is that with large numbers of items we're already in bad shape;
-        // optimize for fewer false positives as we get closer to the leaves.
-        // This can be tweaked after profiling.
-        let hash1 = self.make_hash(item, OFFSET_ONE);
-        let hash2 = self.make_hash(item, OFFSET_TWO);
-        (1 << (hash1 % NUM_BITS)) | (1 << (hash2 % NUM_BITS))
-    }
-
-    #[inline]
-    fn make_hash(&self, item: &T, seed: u64) -> u64 {
-        let mut hasher = FnvHasher::with_key(seed);
-        item.hash(&mut hasher);
-        hasher.finish()
-    }
-}
-
-impl<T: ?Sized> std::fmt::Debug for Bloom<T> {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        write!(f, "Bloom: {:064b}: ({})", self.bits, self.entry_count)
-    }
-}
-
-impl<T: ?Sized> Default for Bloom<T> {
-    fn default() -> Self {
-        Bloom {
-            bits: 0,
-            data: PhantomData,
-            entry_count: 0,
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn very_good_test() {
-        let mut bloom = Bloom::default();
-        for i in 0..100 {
-            bloom.add(&i);
-            assert!(bloom.may_contain(&i));
-        }
-        bloom.clear();
-        for i in 0..100 {
-            assert!(!bloom.may_contain(&i));
-        }
-    }
-
-    #[test]
-    fn union() {
-        let mut bloom1 = Bloom::default();
-        bloom1.add(&0);
-        bloom1.add(&1);
-        assert!(!bloom1.may_contain(&2));
-        assert!(!bloom1.may_contain(&3));
-        let mut bloom2 = Bloom::default();
-        bloom2.add(&2);
-        bloom2.add(&3);
-        assert!(!bloom2.may_contain(&0));
-        assert!(!bloom2.may_contain(&1));
-
-        let bloom3 = bloom1.union(bloom2);
-        assert!(bloom3.may_contain(&0));
-        assert!(bloom3.may_contain(&1));
-        assert!(bloom3.may_contain(&2));
-        assert!(bloom3.may_contain(&3));
-    }
-}

masonry/src/contexts.rs

@@ -674,18 +674,6 @@ impl EventCtx<'_> {
 }
 
 impl LifeCycleCtx<'_> {
-    /// Registers a child widget.
-    ///
-    /// This should only be called in response to a `LifeCycle::WidgetAdded` event.
-    ///
-    /// In general, you should not need to call this method; it is handled by
-    /// the `WidgetPod`.
-    // TODO - See https://github.com/linebender/xilem/issues/372
-    pub(crate) fn register_child(&mut self, child_id: WidgetId) {
-        trace!("register_child id={:?}", child_id);
-        self.widget_state.children.add(&child_id);
-    }
-
     /// Register this widget to be eligile to accept focus automatically.
     ///
     /// This should only be called in response to a [`LifeCycle::BuildFocusChain`] event.

masonry/src/lib.rs

@@ -105,7 +105,6 @@ pub use vello::kurbo;
 mod util;
 
 mod action;
-mod bloom;
 mod box_constraints;
 mod contexts;
 mod event;

masonry/src/widget/tests/lifecycle_basic.rs

@@ -57,34 +57,6 @@ fn adding_child() {
     assert_debug_snapshot!(record_new_child);
 }
 
-#[test]
-fn child_tracking() {
-    let [id_1, id_2, id_3, id_4] = widget_ids();
-
-    let widget = SizedBox::new_with_id(
-        SizedBox::new_with_id(
-            Flex::row()
-                .with_child_id(SizedBox::empty(), id_1)
-                .with_child_id(SizedBox::empty(), id_2),
-            id_3,
-        ),
-        id_4,
-    );
-
-    let harness = TestHarness::create(widget);
-
-    let root_state = harness.get_widget(id_4).state();
-    assert_eq!(root_state.children.entry_count(), 3);
-    assert!(root_state.children.may_contain(&id_1));
-    assert!(root_state.children.may_contain(&id_2));
-    assert!(root_state.children.may_contain(&id_3));
-
-    let child_state = harness.get_widget(id_3).state();
-    assert!(child_state.children.may_contain(&id_1));
-    assert!(child_state.children.may_contain(&id_2));
-    assert_eq!(child_state.children.entry_count(), 2);
-}
-
 /// Test that all children are registered correctly after a child is replaced.
 #[test]
 #[cfg(FALSE)]

masonry/src/widget/widget_pod.rs

@@ -307,14 +307,7 @@ impl<W: Widget> WidgetPod<W> {
 
         let call_widget = match event {
             LifeCycle::Internal(internal) => match internal {
-                InternalLifeCycle::RouteWidgetAdded => {
+                InternalLifeCycle::RouteWidgetAdded => state.children_changed,
-                    // TODO - explain
-                    if state.children_changed {
-                        // TODO - Separate "widget removed" case.
-                        state.children.clear();
-                    }
-                    state.children_changed
-                }
                 InternalLifeCycle::RouteDisabledChanged => {
                     state.update_focus_chain = true;
 
@@ -357,13 +350,9 @@ impl<W: Widget> WidgetPod<W> {
                         state.has_focus = false;
                     }
 
-                    // Recurse when the target widgets could be our descendants.
+                    // TODO - This returns a lot of false positives.
-                    // The bloom filter we're checking can return false positives.
+                    // We'll remove this code soon anyway.
-                    match (old, new) {
+                    matches!((old, new), (Some(_), _) | (_, Some(_)))
-                        (Some(old), _) if state.children.may_contain(old) => true,
-                        (_, Some(new)) if state.children.may_contain(new) => true,
-                        _ => false,
-                    }
                 }
             },
             LifeCycle::WidgetAdded => {
@@ -437,9 +426,6 @@ impl<W: Widget> WidgetPod<W> {
             // we need to (re)register children in case of one of the following events
             LifeCycle::Internal(InternalLifeCycle::RouteWidgetAdded) => {
                 state.children_changed = false;
-                parent_ctx.widget_state.children =
-                    parent_ctx.widget_state.children.union(state.children);
-                parent_ctx.register_child(self.id());
             }
             LifeCycle::DisabledChanged(_)
             | LifeCycle::Internal(InternalLifeCycle::RouteDisabledChanged) => {

masonry/src/widget/widget_ref.rs

@@ -230,16 +230,6 @@ impl<'w> WidgetRef<'w, dyn Widget> {
 
         for child in self.children() {
             child.debug_validate(after_layout);
-
-            if !self.state().children.may_contain(&child.state().id) {
-                debug_panic!(
-                    "Widget '{}' #{} is invalid: child widget '{}' #{} not registered in children filter",
-                    self.deref().short_type_name(),
-                    self.state().id.to_raw(),
-                    child.deref().short_type_name(),
-                    child.state().id.to_raw(),
-                );
-            }
         }
     }
 }

masonry/src/widget/widget_state.rs

@@ -6,7 +6,6 @@
 use std::sync::atomic::{AtomicBool, Ordering};
 use vello::kurbo::{Insets, Point, Rect, Size, Vec2};
 
-use crate::bloom::Bloom;
 use crate::text_helpers::TextFieldRegistration;
 use crate::{CursorIcon, WidgetId};
 
@@ -115,7 +114,6 @@ pub struct WidgetState {
 
     pub(crate) focus_chain: Vec<WidgetId>,
 
-    pub(crate) children: Bloom<WidgetId>,
     pub(crate) children_changed: bool,
 
     // TODO - Remove and handle in WidgetRoot instead
@@ -185,7 +183,6 @@ impl WidgetState {
             has_focus: false,
             request_anim: true,
             focus_chain: Vec::new(),
-            children: Bloom::new(),
             children_changed: true,
             cursor: None,
             is_explicitly_disabled_new: false,