WIP: update discriminator to use hash of data

Cargo.lock

@@ -6171,6 +6171,7 @@ name = "re_space_view_graph"
 version = "0.21.0-alpha.1+dev"
 dependencies = [
  "ahash",
+ "bytemuck",
  "egui",
  "fjadra",
  "nohash-hasher",

crates/viewer/re_space_view_graph/Cargo.toml

@@ -33,6 +33,7 @@ re_viewer_context.workspace = true
 re_viewport_blueprint.workspace = true
 
 ahash.workspace = true
+bytemuck.workspace = true
 egui.workspace = true
 fjadra.workspace = true
 nohash-hasher.workspace = true

crates/viewer/re_space_view_graph/src/graph/mod.rs

@@ -12,6 +12,13 @@ pub struct NodeInstanceImplicit {
     pub index: NodeIndex,
 }
 
+impl std::hash::Hash for NodeInstanceImplicit {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.index.hash(state);
+    }
+}
+
+#[derive(Hash)]
 pub struct Graph<'a> {
     explicit: &'a [NodeInstance],
     implicit: Vec<NodeInstanceImplicit>,

crates/viewer/re_space_view_graph/src/layout/mod.rs

@@ -23,19 +23,19 @@ impl Layout {
     }
 
     /// Updates the size and position of a node, for example after size changes.
-    pub fn update(&mut self, node: &NodeIndex, rect: Rect) {
+    /// Returns `true` if the node was updated.
+    pub fn update(&mut self, node: &NodeIndex, rect: Rect) -> bool {
         debug_assert!(
             self.extents.contains_key(node),
             "node should exist in the layout"
         );
         if let Some(extent) = self.extents.get_mut(node) {
-            *extent = rect;
+            if extent != &rect {
+                *extent = rect;
+                return true;
+            }
         }
-    }
-
-    /// Returns `true` if any node has a zero size.
-    pub fn has_zero_size(&self) -> bool {
-        self.extents.values().any(|r| r.size() == Vec2::ZERO)
+        false
     }
 }
 

crates/viewer/re_space_view_graph/src/ui/mod.rs

@@ -3,7 +3,7 @@ mod state;
 
 pub mod scene;
 
-pub use state::GraphSpaceViewState;
+pub use state::{Discriminator, GraphSpaceViewState};
 
 pub fn bounding_rect_from_iter(rectangles: impl Iterator<Item = egui::Rect>) -> egui::Rect {
     rectangles.fold(egui::Rect::NOTHING, |acc, rect| acc.union(rect))

crates/viewer/re_space_view_graph/src/ui/state.rs

@@ -1,5 +1,4 @@
 use egui::Rect;
-use re_chunk::{EntityPath, TimeInt, Timeline};
 use re_format::format_f32;
 use re_types::blueprint::components::VisualBounds2D;
 use re_ui::UiExt;
@@ -61,25 +60,15 @@ impl SpaceViewState for GraphSpaceViewState {
     }
 }
 
-/// Used to determine if a layout is up-to-date or outdated.
+/// Used to determine if a layout is up-to-date or outdated. For now we use a
+/// hash of the data the comes from the visualizers.
 #[derive(Debug, PartialEq, Eq)]
-pub struct Discriminator {
-    timeline: Timeline,
-    time: TimeInt,
-    entities: ahash::HashSet<EntityPath>,
-}
+#[repr(transparent)]
+pub struct Discriminator(u64);
 
 impl Discriminator {
-    pub fn new(
-        timeline: Timeline,
-        time: TimeInt,
-        entities: impl IntoIterator<Item = EntityPath>,
-    ) -> Self {
-        Self {
-            timeline,
-            time,
-            entities: entities.into_iter().collect(),
-        }
+    pub fn new(hash: u64) -> Self {
+        Self(hash)
     }
 }
 
@@ -183,12 +172,10 @@ impl LayoutState {
     /// This method is lazy. A new layout is only computed if the current timestamp requires it.
     pub fn get<'a>(
         &'a mut self,
-        timeline: Timeline,
-        time: TimeInt,
-        entities: impl IntoIterator<Item = EntityPath>,
+        hash: Discriminator,
         graphs: impl Iterator<Item = &'a Graph<'a>> + Clone,
     ) -> &'a mut Layout {
-        *self = std::mem::take(self).update(Discriminator::new(timeline, time, entities), graphs);
+        *self = std::mem::take(self).update(hash, graphs);
 
         match self {
             Self::Finished { layout, .. } | Self::InProgress { layout, .. } => layout,

crates/viewer/re_space_view_graph/src/view.rs

@@ -1,3 +1,5 @@
+use std::hash::{Hash as _, Hasher as _};
+
 use re_log_types::EntityPath;
 use re_space_view::{
     controls::{DRAG_PAN2D_BUTTON, ZOOM_SCROLL_MODIFIER},
@@ -19,7 +21,7 @@ use re_viewport_blueprint::ViewProperty;
 
 use crate::{
     graph::Graph,
-    ui::{scene::SceneBuilder, GraphSpaceViewState},
+    ui::{scene::SceneBuilder, Discriminator, GraphSpaceViewState},
     visualizers::{merge, EdgesVisualizer, NodeVisualizer},
 };
 
@@ -148,6 +150,14 @@ Display a graph of nodes and edges.
             .map(|(ent, nodes, edges)| (ent, Graph::new(nodes, edges)))
             .collect::<Vec<_>>();
 
+        // We could move this computation to the visualizers to improve
+        // performance if needed.
+        let discriminator = {
+            let mut hasher = ahash::AHasher::default();
+            graphs.hash(&mut hasher);
+            Discriminator::new(hasher.finish())
+        };
+
         let state = state.downcast_mut::<GraphSpaceViewState>()?;
 
         let bounds_property = ViewProperty::from_archetype::<VisualBounds2D>(
@@ -160,42 +170,41 @@ Display a graph of nodes and edges.
             bounds_property.component_or_fallback(ctx, self, state)?;
 
         let layout_was_empty = state.layout_state.is_none();
-        let layout = state.layout_state.get(
-            query.timeline,
-            query.latest_at,
-            graphs.iter().map(|g| (g.0).clone()),
-            graphs.iter().map(|(_, graph)| graph),
-        );
-        let needs_remeasure = layout.has_zero_size();
+        let layout = state
+            .layout_state
+            .get(discriminator, graphs.iter().map(|(_, graph)| graph));
+
+        let mut needs_remeasure = false;
 
         state.world_bounds = Some(bounds);
         let bounds_rect: egui::Rect = bounds.into();
 
         let mut scene_builder = SceneBuilder::from_world_bounds(bounds_rect);
 
-        // TODO(grtlr): Is there a blueprint archetype for debug information?
         if state.show_debug {
             scene_builder.show_debug();
         }
 
-
-
         let (new_world_bounds, response) = scene_builder.add(ui, |mut scene| {
             for (entity, graph) in &graphs {
                 // We use the following to keep track of the bounding box over nodes in an entity.
                 let mut entity_rect = egui::Rect::NOTHING;
 
-                let ent_highlights = query.highlights.entity_highlight(entity.hash());
+                let ent_highlights = query.highlights.entity_highlight((*entity).hash());
 
                 // Draw explicit nodes.
                 for node in graph.nodes_explicit() {
                     let pos = layout.get(&node.index).unwrap_or(egui::Rect::ZERO);
 
-
-                    let response = scene.explicit_node(pos.min, node, ent_highlights.index_highlight(node.instance));
+                    let response = scene.explicit_node(
+                        pos.min,
+                        node,
+                        ent_highlights.index_highlight(node.instance),
+                    );
 
                     if response.clicked() {
-                        let instance_path = InstancePath::instance((*entity).clone(), node.instance);
+                        let instance_path =
+                            InstancePath::instance((*entity).clone(), node.instance);
                         ctx.select_hovered_on_click(
                             &response,
                             vec![(Item::DataResult(query.space_view_id, instance_path), None)]
@@ -204,15 +213,15 @@ Display a graph of nodes and edges.
                     }
 
                     entity_rect = entity_rect.union(response.rect);
-                    layout.update(&node.index, response.rect);
+                    needs_remeasure |= layout.update(&node.index, response.rect);
                 }
 
                 // Draw implicit nodes.
                 for node in graph.nodes_implicit() {
                     let current = layout.get(&node.index).unwrap_or(egui::Rect::ZERO);
                     let response = scene.implicit_node(current.min, node);
                     entity_rect = entity_rect.union(response.rect);
-                    layout.update(&node.index, response.rect);
+                    needs_remeasure |= layout.update(&node.index, response.rect);
                 }
 
                 // Draw edges.

crates/viewer/re_space_view_graph/src/visualizers/edges.rs

@@ -25,6 +25,24 @@ pub struct EdgeInstance {
     pub target_index: NodeIndex,
 }
 
+impl std::hash::Hash for EdgeInstance {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        // We use the more verbose destructring here, to make sure that we
+        // exhaustively consider all fields when hashing (we get a compiler
+        // warning when we forget a field).
+        let Self {
+            // The index fields already uniquely identify `source` and `target`.
+            source: _,
+            target: _,
+            source_index,
+            target_index,
+        } = self;
+        source_index.hash(state);
+        target_index.hash(state);
+    }
+}
+
+#[derive(Hash)]
 pub struct EdgeData {
     pub graph_type: components::GraphType,
     pub edges: Vec<EdgeInstance>,

crates/viewer/re_space_view_graph/src/visualizers/nodes.rs

@@ -33,6 +33,35 @@ pub struct NodeInstance {
     pub radius: Option<components::Radius>,
 }
 
+impl std::hash::Hash for NodeInstance {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        // We use the more verbose destructring here, to make sure that we
+        // exhaustively consider all fields when hashing (we get a compiler
+        // warning when we forget a field).
+        let Self {
+            // The index already uniquely identifies a node, so we don't need to
+            // hash the node itself.
+            node: _,
+            instance,
+            index,
+            label,
+            show_label,
+            color,
+            position,
+            radius,
+        } = self;
+        instance.hash(state);
+        index.hash(state);
+        label.hash(state);
+        show_label.hash(state);
+        color.hash(state);
+        // The following fields don't implement `Hash`.
+        position.as_ref().map(bytemuck::bytes_of).hash(state);
+        radius.as_ref().map(bytemuck::bytes_of).hash(state);
+    }
+}
+
+#[derive(Hash)]
 pub struct NodeData {
     pub nodes: Vec<NodeInstance>,
 }
@@ -110,7 +139,7 @@ impl VisualizerSystem for NodeVisualizer {
                         radius: radius.copied(),
                     },
                 )
-                .collect();
+                .collect::<Vec<_>>();
 
                 self.data
                     .insert(data_result.entity_path.clone(), NodeData { nodes });