From bbc667596d323251bb18623c7891e737c25a42f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Thu, 28 Nov 2024 15:05:14 +0100 Subject: [PATCH 01/12] Implement bezier edges --- .../re_space_view_graph/src/layout/mod.rs | 2 +- .../src/layout/provider.rs | 54 ++++++++++++------ .../re_space_view_graph/src/layout/request.rs | 31 +++++++++-- .../re_space_view_graph/src/layout/result.rs | 48 ++++++++++++++-- .../viewer/re_space_view_graph/src/ui/draw.rs | 55 +++++++++++++++---- .../re_space_view_graph/src/ui/state.rs | 45 +++++---------- .../release_checklist/check_graph_view.py | 25 ++++++--- 7 files changed, 185 insertions(+), 75 deletions(-) diff --git a/crates/viewer/re_space_view_graph/src/layout/mod.rs b/crates/viewer/re_space_view_graph/src/layout/mod.rs index 8c280d81c782..bca83ad84ac9 100644 --- a/crates/viewer/re_space_view_graph/src/layout/mod.rs +++ b/crates/viewer/re_space_view_graph/src/layout/mod.rs @@ -4,4 +4,4 @@ mod result; pub use provider::ForceLayoutProvider; pub use request::LayoutRequest; -pub use result::Layout; +pub use result::{EdgeGeometry, Layout}; diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index d3f845f45457..d04a51cee11e 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -3,7 +3,7 @@ use fjadra as fj; use crate::graph::NodeId; -use super::{request::NodeTemplate, Layout, LayoutRequest}; +use super::{request::NodeTemplate, result::EdgeGeometry, Layout, LayoutRequest}; impl<'a> From<&'a NodeTemplate> for fj::Node { fn from(node: &'a NodeTemplate) -> Self { @@ -17,11 +17,11 @@ impl<'a> From<&'a NodeTemplate> for fj::Node { pub struct ForceLayoutProvider { simulation: fj::Simulation, node_index: ahash::HashMap, - edges: Vec<(NodeId, NodeId)>, + pub request: LayoutRequest, } impl ForceLayoutProvider { - pub fn new(request: &LayoutRequest) -> Self { + pub fn new(request: LayoutRequest) -> Self { let nodes = request.graphs.iter().flat_map(|(_, graph_template)| { graph_template .nodes @@ -43,9 +43,11 @@ impl ForceLayoutProvider { .iter() .flat_map(|(_, graph_template)| graph_template.edges.iter()); - let all_edges = all_edges_iter + // Looking at self-edges does not make sense in a force-based layout, so we filter those out. + let considered_edges = all_edges_iter .clone() - .map(|(a, b)| (node_index[a], node_index[b])); + .filter(|((from, to), _)| (from != to)) + .map(|((from, to), _)| (node_index[from], node_index[to])); // TODO(grtlr): Currently we guesstimate good forces. Eventually these should be exposed as blueprints. let simulation = fj::SimulationBuilder::default() @@ -53,7 +55,7 @@ impl ForceLayoutProvider { .build(all_nodes) .add_force( "link", - fj::Link::new(all_edges).distance(50.0).iterations(2), + fj::Link::new(considered_edges).distance(50.0).iterations(2), ) .add_force("charge", fj::ManyBody::new()) // TODO(grtlr): This is a small stop-gap until we have blueprints to prevent nodes from flying away. @@ -63,15 +65,15 @@ impl ForceLayoutProvider { Self { simulation, node_index, - edges: all_edges_iter.copied().collect(), + request, } } - pub fn init(&self, request: &LayoutRequest) -> Layout { + pub fn init(&self) -> Layout { let positions = self.simulation.positions().collect::>(); let mut extents = ahash::HashMap::default(); - for graph in request.graphs.values() { + for graph in self.request.graphs.values() { for (id, node) in &graph.nodes { let i = self.node_index[id]; let [x, y] = positions[i]; @@ -100,11 +102,28 @@ impl ForceLayoutProvider { extent.set_center(pos); } - for (from, to) in &self.edges { - layout.edges.insert( - (*from, *to), - line_segment(layout.nodes[from], layout.nodes[to]), - ); + for (from, to) in self.request.graphs.values().flat_map(|g| g.edges.keys()) { + match from == to { + true => { + // Self-edges are not supported in force-based layouts. + let anchor = intersects_ray_from_center(layout.nodes[from], Vec2::UP); + layout.edges.insert( + (*from, *to), + EdgeGeometry::CubicBezier { + start: anchor + Vec2::LEFT * 5., + end: anchor + Vec2::RIGHT * 5., + // TODO(grtlr): The actual length of that spline should follow the `distance` parameter of the link force. + control: [anchor + Vec2::new(-50., -60.), anchor + Vec2::new(50., -60.)], + }, + ); + } + false => { + layout.edges.insert( + (*from, *to), + line_segment(layout.nodes[from], layout.nodes[to]), + ); + } + } } self.simulation.finished() @@ -112,7 +131,7 @@ impl ForceLayoutProvider { } /// Helper function to calculate the line segment between two rectangles. -fn line_segment(source: Rect, target: Rect) -> [Pos2; 2] { +fn line_segment(source: Rect, target: Rect) -> EdgeGeometry { let source_center = source.center(); let target_center = target.center(); @@ -123,7 +142,10 @@ fn line_segment(source: Rect, target: Rect) -> [Pos2; 2] { let source_point = intersects_ray_from_center(source, direction); let target_point = intersects_ray_from_center(target, -direction); // Reverse direction for target - [source_point, target_point] + EdgeGeometry::Line { + start: source_point, + end: target_point, + } } /// Helper function to find the point where the line intersects the border of a rectangle diff --git a/crates/viewer/re_space_view_graph/src/layout/request.rs b/crates/viewer/re_space_view_graph/src/layout/request.rs index be17a02e4395..af96fde52c12 100644 --- a/crates/viewer/re_space_view_graph/src/layout/request.rs +++ b/crates/viewer/re_space_view_graph/src/layout/request.rs @@ -1,4 +1,12 @@ -use std::collections::{BTreeMap, BTreeSet}; +//! Contains all the information that is considered when performing a graph layout. +//! +//! We support: +//! * Multiple multiple edges between the same two nodes. +//! * Self-edges +//! +//!
Duplicated graph nodes are undefined behavior.
+ +use std::collections::BTreeMap; use egui::{Pos2, Vec2}; use re_chunk::EntityPath; @@ -11,10 +19,17 @@ pub(super) struct NodeTemplate { pub(super) fixed_position: Option, } +#[derive(PartialEq)] +pub(super) struct EdgeTemplate; + #[derive(Default, PartialEq)] pub(super) struct GraphTemplate { pub(super) nodes: BTreeMap, - pub(super) edges: BTreeSet<(NodeId, NodeId)>, + + /// The edges in the layout. + /// + /// Each entry can contain multiple edges. + pub(super) edges: BTreeMap<(NodeId, NodeId), Vec>, } /// A [`LayoutRequest`] encapsulates all the information that is considered when computing a layout. @@ -39,11 +54,19 @@ impl LayoutRequest { size: node.size(), fixed_position: node.position(), }; - entity.nodes.insert(node.id(), shape); + let duplicate = entity.nodes.insert(node.id(), shape); + debug_assert!( + duplicate.is_none(), + "duplicated nodes are undefined behavior" + ); } for edge in graph.edges() { - entity.edges.insert((edge.from, edge.to)); + let es = entity + .edges + .entry((edge.from, edge.to)) + .or_insert(Vec::new()); + es.push(EdgeTemplate); } } diff --git a/crates/viewer/re_space_view_graph/src/layout/result.rs b/crates/viewer/re_space_view_graph/src/layout/result.rs index 78734514ee13..0c98ff0c2f46 100644 --- a/crates/viewer/re_space_view_graph/src/layout/result.rs +++ b/crates/viewer/re_space_view_graph/src/layout/result.rs @@ -4,10 +4,50 @@ use crate::graph::NodeId; pub type LineSegment = [Pos2; 2]; -#[derive(Debug, PartialEq, Eq)] +fn bounding_rect_points(points: impl IntoIterator>) -> egui::Rect { + points + .into_iter() + .fold(egui::Rect::NOTHING, |mut acc, pos| { + acc.extend_with(pos.into()); + acc + }) +} + +#[derive(Clone, Debug)] +pub enum EdgeGeometry { + Line { + start: Pos2, + end: Pos2, + }, + /// Represents a cubic bezier curve. + /// + /// In the future we could probably support more complex splines. + CubicBezier { + start: Pos2, + end: Pos2, + control: [Pos2; 2], + }, + // We could add other geometries, such as `Orthogonal` here too. +} + +impl EdgeGeometry { + pub fn bounding_rect(&self) -> Rect { + match self { + EdgeGeometry::Line { start, end } => Rect::from_two_pos(*start, *end), + // TODO(grtlr): This is just a crude (upper) approximation, as the resulting bounding box can be too large. + EdgeGeometry::CubicBezier { + start, + end, + ref control, + } => Rect::from_points(&[&[*start, *end], control.as_slice()].concat()), + } + } +} + +#[derive(Debug)] pub struct Layout { pub(super) nodes: ahash::HashMap, - pub(super) edges: ahash::HashMap<(NodeId, NodeId), LineSegment>, + pub(super) edges: ahash::HashMap<(NodeId, NodeId), EdgeGeometry>, // TODO(grtlr): Consider adding the entity rects here too. } @@ -29,7 +69,7 @@ impl Layout { } /// Gets the shape of an edge in the final layout. - pub fn get_edge(&self, from: NodeId, to: NodeId) -> Option { - self.edges.get(&(from, to)).copied() + pub fn get_edge(&self, from: NodeId, to: NodeId) -> Option<&EdgeGeometry> { + self.edges.get(&(from, to)) } } diff --git a/crates/viewer/re_space_view_graph/src/ui/draw.rs b/crates/viewer/re_space_view_graph/src/ui/draw.rs index ff094f1c0045..cf04280bb547 100644 --- a/crates/viewer/re_space_view_graph/src/ui/draw.rs +++ b/crates/viewer/re_space_view_graph/src/ui/draw.rs @@ -1,8 +1,8 @@ use std::sync::Arc; use egui::{ - Align2, Color32, FontId, FontSelection, Frame, Galley, Painter, Pos2, Rect, Response, RichText, - Sense, Shape, Stroke, TextWrapMode, Ui, UiBuilder, Vec2, WidgetText, + epaint::CubicBezierShape, Align2, Color32, FontId, FontSelection, Frame, Galley, Painter, Pos2, + Rect, Response, RichText, Sense, Shape, Stroke, TextWrapMode, Ui, UiBuilder, Vec2, WidgetText, }; use re_chunk::EntityPath; use re_entity_db::InstancePath; @@ -14,7 +14,7 @@ use re_viewer_context::{ use crate::{ graph::{Graph, Node}, - layout::Layout, + layout::{EdgeGeometry, Layout}, visualizers::Label, }; @@ -191,25 +191,55 @@ fn draw_arrow(painter: &Painter, tip: Pos2, direction: Vec2, color: Color32) { )); } -/// Draws an edge between two points, optionally with an arrow at the target point. -fn draw_edge(ui: &mut Ui, points: [Pos2; 2], show_arrow: bool) -> Response { - let fg = ui.style().visuals.text_color(); +fn draw_edge_line(ui: &Ui, points: [Pos2; 2], stroke: Stroke, show_arrow: bool) { + let painter = ui.painter(); + painter.line_segment(points, stroke); + // Calculate direction vector from source to target + let direction = (points[1] - points[0]).normalized(); - let rect = Rect::from_points(&points); + // Conditionally draw an arrow at the target point + if show_arrow { + draw_arrow(painter, points[1], direction, stroke.color); + } +} + +fn draw_edge_bezier(ui: &Ui, points: [Pos2; 4], stroke: Stroke, show_arrow: bool) { let painter = ui.painter(); - painter.line_segment(points, Stroke::new(1.0, fg)); + painter.add(CubicBezierShape { + points, + closed: false, + fill: Color32::TRANSPARENT, + stroke: stroke.into(), + }); // Calculate direction vector from source to target let direction = (points[1] - points[0]).normalized(); // Conditionally draw an arrow at the target point if show_arrow { - draw_arrow(painter, points[1], direction, fg); + draw_arrow(painter, points[1], direction, stroke.color); + } +} + +/// Draws an edge between two points, optionally with an arrow at the target point. +pub fn draw_edge(ui: &mut Ui, geometry: &EdgeGeometry, show_arrow: bool) -> Response { + let fg = ui.style().visuals.text_color(); + let stroke = Stroke::new(1.0, fg); + + match geometry { + &EdgeGeometry::Line { start, end } => { + draw_edge_line(ui, [start, end], stroke, show_arrow); + } + &EdgeGeometry::CubicBezier { + start, + end, + control, + } => draw_edge_bezier(ui, [start, control[0], control[1], end], stroke, show_arrow), } // We can add interactions in the future, for now we simply allocate the // rect, so that bounding boxes are computed correctly. - ui.allocate_rect(rect, NON_SENSE) + ui.allocate_rect(geometry.bounding_rect(), NON_SENSE) } pub fn draw_entity_rect( @@ -303,7 +333,10 @@ pub fn draw_graph( for edge in graph.edges() { let points = layout .get_edge(edge.from, edge.to) - .unwrap_or([Pos2::ZERO, Pos2::ZERO]); + .unwrap_or(&EdgeGeometry::Line { + start: Pos2::ZERO, + end: Pos2::ZERO, + }); let resp = draw_edge(ui, points, edge.arrow); current_rect = current_rect.union(resp.rect); } diff --git a/crates/viewer/re_space_view_graph/src/ui/state.rs b/crates/viewer/re_space_view_graph/src/ui/state.rs index ccf77707ebf4..5536f8c31755 100644 --- a/crates/viewer/re_space_view_graph/src/ui/state.rs +++ b/crates/viewer/re_space_view_graph/src/ui/state.rs @@ -65,14 +65,12 @@ pub enum LayoutState { #[default] None, InProgress { - request: LayoutRequest, layout: Layout, provider: ForceLayoutProvider, }, Finished { - request: LayoutRequest, layout: Layout, - _provider: ForceLayoutProvider, + provider: ForceLayoutProvider, }, } @@ -102,46 +100,29 @@ impl LayoutState { fn update(self, new_request: LayoutRequest) -> Self { match self { // Layout is up to date, nothing to do here. - Self::Finished { ref request, .. } if request == &new_request => { + Self::Finished { ref provider, .. } if &provider.request == &new_request => { self // no op } // We need to recompute the layout. Self::None | Self::Finished { .. } => { - let provider = ForceLayoutProvider::new(&new_request); - let layout = provider.init(&new_request); - - Self::InProgress { - request: new_request, - layout, - provider, - } + let provider = ForceLayoutProvider::new(new_request); + let layout = provider.init(); + + Self::InProgress { layout, provider } } - Self::InProgress { request, .. } if request != new_request => { - let provider = ForceLayoutProvider::new(&new_request); - let layout = provider.init(&new_request); - - Self::InProgress { - request: new_request, - layout, - provider, - } + Self::InProgress { provider, .. } if provider.request != new_request => { + let provider = ForceLayoutProvider::new(new_request); + let layout = provider.init(); + + Self::InProgress { layout, provider } } // We keep iterating on the layout until it is stable. Self::InProgress { - request, mut layout, mut provider, } => match provider.tick(&mut layout) { - true => Self::Finished { - request, - layout, - _provider: provider, - }, - false => Self::InProgress { - request, - layout, - provider, - }, + true => Self::Finished { layout, provider }, + false => Self::InProgress { layout, provider }, }, } } diff --git a/tests/python/release_checklist/check_graph_view.py b/tests/python/release_checklist/check_graph_view.py index dc0b5edeeaf7..ec92a16f6d80 100644 --- a/tests/python/release_checklist/check_graph_view.py +++ b/tests/python/release_checklist/check_graph_view.py @@ -21,6 +21,12 @@ def log_readme() -> None: rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), static=True) +def log_weird_graph() -> None: + rr.log( + "weird", + rr.GraphNodes(["A", "B"], labels=["A", "B"]), + rr.GraphEdges([("A", "A")], graph_type=rr.GraphType.Directed), + ) def log_graphs() -> None: DATA = [ @@ -45,9 +51,21 @@ def log_graphs() -> None: rr.log("graph", rr.GraphNodes(nodes, labels=nodes), rr.GraphEdges(edges, graph_type=rr.GraphType.Directed)) rr.log("graph2", rr.GraphNodes(nodes, labels=nodes), rr.GraphEdges(edges, graph_type=rr.GraphType.Undirected)) + +def run(args: Namespace) -> None: + rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4()) + + log_readme() + log_graphs() + log_weird_graph() + rr.send_blueprint( rrb.Blueprint( rrb.Grid( + rrb.GraphView(origin="weird", name="Weird Graph"), + rrb.GraphView( + origin="weird", name="Weird Graph (without labels)", defaults=[rr.components.ShowLabels(False)] + ), rrb.GraphView(origin="graph", name="Graph 1"), rrb.GraphView(origin="graph2", name="Graph 2"), rrb.GraphView(name="Both", contents=["/graph", "/graph2"]), @@ -57,13 +75,6 @@ def log_graphs() -> None: ) -def run(args: Namespace) -> None: - rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4()) - - log_readme() - log_graphs() - - if __name__ == "__main__": import argparse From 0eef53a11f3bb5b47167f491d7f3d2d6d173e96c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Fri, 29 Nov 2024 09:31:51 +0100 Subject: [PATCH 02/12] Almost working multi self-edges --- .../re_space_view_graph/src/graph/mod.rs | 8 +- .../src/layout/provider.rs | 36 +++++---- .../re_space_view_graph/src/layout/request.rs | 2 +- .../re_space_view_graph/src/layout/result.rs | 72 ++++++++++------- .../viewer/re_space_view_graph/src/ui/draw.rs | 78 ++++++++----------- .../release_checklist/check_graph_view.py | 15 +++- 6 files changed, 117 insertions(+), 94 deletions(-) diff --git a/crates/viewer/re_space_view_graph/src/graph/mod.rs b/crates/viewer/re_space_view_graph/src/graph/mod.rs index 947216daf86c..21e38bf73665 100644 --- a/crates/viewer/re_space_view_graph/src/graph/mod.rs +++ b/crates/viewer/re_space_view_graph/src/graph/mod.rs @@ -71,8 +71,8 @@ impl Node { } pub struct Edge { - pub from: NodeId, - pub to: NodeId, + pub source: NodeId, + pub target: NodeId, pub arrow: bool, } @@ -128,8 +128,8 @@ impl Graph { } let es = data.edges.iter().map(|e| Edge { - from: e.source_index, - to: e.target_index, + source: e.source_index, + target: e.target_index, arrow: data.graph_type == GraphType::Directed, }); diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index d04a51cee11e..a781ffafedaa 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -102,26 +102,28 @@ impl ForceLayoutProvider { extent.set_center(pos); } - for (from, to) in self.request.graphs.values().flat_map(|g| g.edges.keys()) { + // TODO(grtlr): Better to create slots for each edge. + for ((from, to), template) in self.request.graphs.values().flat_map(|g| g.edges.iter()) { + let geometries = layout.edges.entry((*from, *to)).or_default(); match from == to { true => { - // Self-edges are not supported in force-based layouts. - let anchor = intersects_ray_from_center(layout.nodes[from], Vec2::UP); - layout.edges.insert( - (*from, *to), - EdgeGeometry::CubicBezier { - start: anchor + Vec2::LEFT * 5., - end: anchor + Vec2::RIGHT * 5., + for i in 1..=template.len() { + // Self-edges are not supported in force-based layouts. + let anchor = intersects_ray_from_center(layout.nodes[from], Vec2::UP); + geometries.push(EdgeGeometry::CubicBezier { + // TODO(grtlr): We could probably consider the actual node size here. + source: anchor + Vec2::LEFT * 4., + target: anchor + Vec2::RIGHT * 4., // TODO(grtlr): The actual length of that spline should follow the `distance` parameter of the link force. - control: [anchor + Vec2::new(-50., -60.), anchor + Vec2::new(50., -60.)], - }, - ); + control: [ + anchor + Vec2::new(-50. * i as f32, -60. * i as f32), + anchor + Vec2::new(50. * i as f32, -60. * i as f32), + ], + }); + } } false => { - layout.edges.insert( - (*from, *to), - line_segment(layout.nodes[from], layout.nodes[to]), - ); + geometries.push(line_segment(layout.nodes[from], layout.nodes[to])); } } } @@ -143,8 +145,8 @@ fn line_segment(source: Rect, target: Rect) -> EdgeGeometry { let target_point = intersects_ray_from_center(target, -direction); // Reverse direction for target EdgeGeometry::Line { - start: source_point, - end: target_point, + source: source_point, + target: target_point, } } diff --git a/crates/viewer/re_space_view_graph/src/layout/request.rs b/crates/viewer/re_space_view_graph/src/layout/request.rs index af96fde52c12..13ccad1c4ef7 100644 --- a/crates/viewer/re_space_view_graph/src/layout/request.rs +++ b/crates/viewer/re_space_view_graph/src/layout/request.rs @@ -64,7 +64,7 @@ impl LayoutRequest { for edge in graph.edges() { let es = entity .edges - .entry((edge.from, edge.to)) + .entry((edge.source, edge.target)) .or_insert(Vec::new()); es.push(EdgeTemplate); } diff --git a/crates/viewer/re_space_view_graph/src/layout/result.rs b/crates/viewer/re_space_view_graph/src/layout/result.rs index 0c98ff0c2f46..b32e00d94c78 100644 --- a/crates/viewer/re_space_view_graph/src/layout/result.rs +++ b/crates/viewer/re_space_view_graph/src/layout/result.rs @@ -1,30 +1,18 @@ -use egui::{Pos2, Rect}; +use egui::{Pos2, Rect, Vec2}; use crate::graph::NodeId; -pub type LineSegment = [Pos2; 2]; - -fn bounding_rect_points(points: impl IntoIterator>) -> egui::Rect { - points - .into_iter() - .fold(egui::Rect::NOTHING, |mut acc, pos| { - acc.extend_with(pos.into()); - acc - }) -} - #[derive(Clone, Debug)] pub enum EdgeGeometry { - Line { - start: Pos2, - end: Pos2, - }, + /// A simple straight edge. + Line { source: Pos2, target: Pos2 }, + /// Represents a cubic bezier curve. /// /// In the future we could probably support more complex splines. CubicBezier { - start: Pos2, - end: Pos2, + source: Pos2, + target: Pos2, control: [Pos2; 2], }, // We could add other geometries, such as `Orthogonal` here too. @@ -33,13 +21,45 @@ pub enum EdgeGeometry { impl EdgeGeometry { pub fn bounding_rect(&self) -> Rect { match self { - EdgeGeometry::Line { start, end } => Rect::from_two_pos(*start, *end), + Self::Line { source, target } => Rect::from_two_pos(*source, *target), // TODO(grtlr): This is just a crude (upper) approximation, as the resulting bounding box can be too large. - EdgeGeometry::CubicBezier { - start, - end, + Self::CubicBezier { + source, + target, ref control, - } => Rect::from_points(&[&[*start, *end], control.as_slice()].concat()), + } => Rect::from_points(&[&[*source, *target], control.as_slice()].concat()), + } + } + + pub fn source_pos(&self) -> Pos2 { + match self { + Self::Line { source, .. } | Self::CubicBezier { source, .. } => *source, + } + } + + pub fn target_pos(&self) -> Pos2 { + match self { + Self::Line { target, .. } | Self::CubicBezier { target, .. } => *target, + } + } + + /// The direction of the edge at the source node (normalized). + pub fn source_arrow_direction(&self) -> Vec2 { + match self { + Self::Line { source, target } => (source.to_vec2() - target.to_vec2()).normalized(), + Self::CubicBezier { + source, control, .. + } => (control[0].to_vec2() - source.to_vec2()).normalized(), + } + } + + /// The direction of the edge at the target node (normalized). + pub fn target_arrow_direction(&self) -> Vec2 { + match self { + Self::Line { source, target } => (target.to_vec2() - source.to_vec2()).normalized(), + Self::CubicBezier { + target, control, .. + } => (target.to_vec2() - control[1].to_vec2()).normalized(), } } } @@ -47,7 +67,7 @@ impl EdgeGeometry { #[derive(Debug)] pub struct Layout { pub(super) nodes: ahash::HashMap, - pub(super) edges: ahash::HashMap<(NodeId, NodeId), EdgeGeometry>, + pub(super) edges: ahash::HashMap<(NodeId, NodeId), Vec>, // TODO(grtlr): Consider adding the entity rects here too. } @@ -69,7 +89,7 @@ impl Layout { } /// Gets the shape of an edge in the final layout. - pub fn get_edge(&self, from: NodeId, to: NodeId) -> Option<&EdgeGeometry> { - self.edges.get(&(from, to)) + pub fn get_edge(&self, from: NodeId, to: NodeId) -> Option<&[EdgeGeometry]> { + self.edges.get(&(from, to)).map(|es| es.as_slice()) } } diff --git a/crates/viewer/re_space_view_graph/src/ui/draw.rs b/crates/viewer/re_space_view_graph/src/ui/draw.rs index cf04280bb547..1f4283ef5da9 100644 --- a/crates/viewer/re_space_view_graph/src/ui/draw.rs +++ b/crates/viewer/re_space_view_graph/src/ui/draw.rs @@ -191,50 +191,39 @@ fn draw_arrow(painter: &Painter, tip: Pos2, direction: Vec2, color: Color32) { )); } -fn draw_edge_line(ui: &Ui, points: [Pos2; 2], stroke: Stroke, show_arrow: bool) { - let painter = ui.painter(); - painter.line_segment(points, stroke); - // Calculate direction vector from source to target - let direction = (points[1] - points[0]).normalized(); - - // Conditionally draw an arrow at the target point - if show_arrow { - draw_arrow(painter, points[1], direction, stroke.color); - } -} - -fn draw_edge_bezier(ui: &Ui, points: [Pos2; 4], stroke: Stroke, show_arrow: bool) { - let painter = ui.painter(); - painter.add(CubicBezierShape { - points, - closed: false, - fill: Color32::TRANSPARENT, - stroke: stroke.into(), - }); - - // Calculate direction vector from source to target - let direction = (points[1] - points[0]).normalized(); - - // Conditionally draw an arrow at the target point - if show_arrow { - draw_arrow(painter, points[1], direction, stroke.color); - } -} - /// Draws an edge between two points, optionally with an arrow at the target point. pub fn draw_edge(ui: &mut Ui, geometry: &EdgeGeometry, show_arrow: bool) -> Response { let fg = ui.style().visuals.text_color(); let stroke = Stroke::new(1.0, fg); - match geometry { - &EdgeGeometry::Line { start, end } => { - draw_edge_line(ui, [start, end], stroke, show_arrow); + let painter = ui.painter(); + + match *geometry { + EdgeGeometry::Line { source, target } => { + painter.line_segment([source, target], stroke); } - &EdgeGeometry::CubicBezier { - start, - end, + EdgeGeometry::CubicBezier { + source, + target, control, - } => draw_edge_bezier(ui, [start, control[0], control[1], end], stroke, show_arrow), + } => { + painter.add(CubicBezierShape { + points: [source, control[0], control[1], target], + closed: false, + fill: Color32::TRANSPARENT, + stroke: stroke.into(), + }); + } + } + + // Conditionally draw an arrow at the target point + if show_arrow { + draw_arrow( + painter, + geometry.target_pos(), + geometry.target_arrow_direction(), + stroke.color, + ); } // We can add interactions in the future, for now we simply allocate the @@ -331,14 +320,13 @@ pub fn draw_graph( } for edge in graph.edges() { - let points = layout - .get_edge(edge.from, edge.to) - .unwrap_or(&EdgeGeometry::Line { - start: Pos2::ZERO, - end: Pos2::ZERO, - }); - let resp = draw_edge(ui, points, edge.arrow); - current_rect = current_rect.union(resp.rect); + for geometry in layout + .get_edge(edge.source, edge.target) + .unwrap_or_default() + { + let resp = draw_edge(ui, geometry, edge.arrow); + current_rect = current_rect.union(resp.rect); + } } current_rect diff --git a/tests/python/release_checklist/check_graph_view.py b/tests/python/release_checklist/check_graph_view.py index ec92a16f6d80..b393e0d069cf 100644 --- a/tests/python/release_checklist/check_graph_view.py +++ b/tests/python/release_checklist/check_graph_view.py @@ -25,7 +25,20 @@ def log_weird_graph() -> None: rr.log( "weird", rr.GraphNodes(["A", "B"], labels=["A", "B"]), - rr.GraphEdges([("A", "A")], graph_type=rr.GraphType.Directed), + rr.GraphEdges( + [ + # self-edges + ("A", "A"), + ("B", "B"), + # duplicated edges + ("A", "B"), + ("B", "A"), + ("B", "A"), + # duplicated self-edges + ("A", "A"), + ], + graph_type=rr.GraphType.Directed, + ), ) def log_graphs() -> None: From 2c03394c1ade6627f68d15dd73ceda75097d9237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Fri, 29 Nov 2024 11:10:29 +0100 Subject: [PATCH 03/12] WIP: Almost woking except bounding_rects. --- .../re_space_view_graph/src/layout/mod.rs | 2 +- .../src/layout/provider.rs | 49 +++++++++++++------ .../re_space_view_graph/src/layout/request.rs | 15 +++--- .../re_space_view_graph/src/layout/result.rs | 45 +++++++++++------ .../viewer/re_space_view_graph/src/ui/draw.rs | 43 +++++++++++----- .../re_space_view_graph/src/ui/state.rs | 2 +- 6 files changed, 105 insertions(+), 51 deletions(-) diff --git a/crates/viewer/re_space_view_graph/src/layout/mod.rs b/crates/viewer/re_space_view_graph/src/layout/mod.rs index bca83ad84ac9..b140159c1f1d 100644 --- a/crates/viewer/re_space_view_graph/src/layout/mod.rs +++ b/crates/viewer/re_space_view_graph/src/layout/mod.rs @@ -4,4 +4,4 @@ mod result; pub use provider::ForceLayoutProvider; pub use request::LayoutRequest; -pub use result::{EdgeGeometry, Layout}; +pub use result::{EdgeGeometry, Layout, PathGeometry}; diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index a781ffafedaa..1b579abcd8df 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -3,7 +3,11 @@ use fjadra as fj; use crate::graph::NodeId; -use super::{request::NodeTemplate, result::EdgeGeometry, Layout, LayoutRequest}; +use super::{ + request::NodeTemplate, + result::{EdgeGeometry, PathGeometry}, + Layout, LayoutRequest, +}; impl<'a> From<&'a NodeTemplate> for fj::Node { fn from(node: &'a NodeTemplate) -> Self { @@ -103,29 +107,42 @@ impl ForceLayoutProvider { } // TODO(grtlr): Better to create slots for each edge. - for ((from, to), template) in self.request.graphs.values().flat_map(|g| g.edges.iter()) { - let geometries = layout.edges.entry((*from, *to)).or_default(); + for ((from, to), templates) in self.request.graphs.values().flat_map(|g| g.edges.iter()) { + let mut geometries = Vec::with_capacity(templates.len()); match from == to { true => { - for i in 1..=template.len() { + for (i, template) in templates.iter().enumerate() { + let offset = (i + 1) as f32; + let target_arrow = template.target_arrow; // Self-edges are not supported in force-based layouts. let anchor = intersects_ray_from_center(layout.nodes[from], Vec2::UP); - geometries.push(EdgeGeometry::CubicBezier { - // TODO(grtlr): We could probably consider the actual node size here. - source: anchor + Vec2::LEFT * 4., - target: anchor + Vec2::RIGHT * 4., - // TODO(grtlr): The actual length of that spline should follow the `distance` parameter of the link force. - control: [ - anchor + Vec2::new(-50. * i as f32, -60. * i as f32), - anchor + Vec2::new(50. * i as f32, -60. * i as f32), - ], + geometries.push(EdgeGeometry { + target_arrow, + path: PathGeometry::CubicBezier { + // TODO(grtlr): We could probably consider the actual node size here. + source: anchor + Vec2::LEFT * 4., + target: anchor + Vec2::RIGHT * 4., + // TODO(grtlr): The actual length of that spline should follow the `distance` parameter of the link force. + control: [ + anchor + Vec2::new(-30. * offset, -40. * offset), + anchor + Vec2::new(30. * offset, -40. * offset), + ], + }, }); } } false => { - geometries.push(line_segment(layout.nodes[from], layout.nodes[to])); + // TODO(grtlr): perform similar computations here + for template in templates { + let target_arrow = template.target_arrow; + geometries.push(EdgeGeometry { + target_arrow, + path: line_segment(layout.nodes[from], layout.nodes[to]), + }); + } } } + layout.edges.insert((*from, *to), geometries); } self.simulation.finished() @@ -133,7 +150,7 @@ impl ForceLayoutProvider { } /// Helper function to calculate the line segment between two rectangles. -fn line_segment(source: Rect, target: Rect) -> EdgeGeometry { +fn line_segment(source: Rect, target: Rect) -> PathGeometry { let source_center = source.center(); let target_center = target.center(); @@ -144,7 +161,7 @@ fn line_segment(source: Rect, target: Rect) -> EdgeGeometry { let source_point = intersects_ray_from_center(source, direction); let target_point = intersects_ray_from_center(target, -direction); // Reverse direction for target - EdgeGeometry::Line { + PathGeometry::Line { source: source_point, target: target_point, } diff --git a/crates/viewer/re_space_view_graph/src/layout/request.rs b/crates/viewer/re_space_view_graph/src/layout/request.rs index 13ccad1c4ef7..ac3b9d4eade5 100644 --- a/crates/viewer/re_space_view_graph/src/layout/request.rs +++ b/crates/viewer/re_space_view_graph/src/layout/request.rs @@ -19,8 +19,12 @@ pub(super) struct NodeTemplate { pub(super) fixed_position: Option, } + +// TODO(grtlr): Does it make sense to consolidate some of the types, i.e. `Edge` ad `EdgeTemplate`? #[derive(PartialEq)] -pub(super) struct EdgeTemplate; +pub(super) struct EdgeTemplate { + pub(super) target_arrow: bool, +} #[derive(Default, PartialEq)] pub(super) struct GraphTemplate { @@ -62,11 +66,10 @@ impl LayoutRequest { } for edge in graph.edges() { - let es = entity - .edges - .entry((edge.source, edge.target)) - .or_insert(Vec::new()); - es.push(EdgeTemplate); + let es = entity.edges.entry((edge.source, edge.target)).or_default(); + es.push(EdgeTemplate { + target_arrow: edge.arrow, + }); } } diff --git a/crates/viewer/re_space_view_graph/src/layout/result.rs b/crates/viewer/re_space_view_graph/src/layout/result.rs index b32e00d94c78..e32ab8657dc9 100644 --- a/crates/viewer/re_space_view_graph/src/layout/result.rs +++ b/crates/viewer/re_space_view_graph/src/layout/result.rs @@ -3,7 +3,7 @@ use egui::{Pos2, Rect, Vec2}; use crate::graph::NodeId; #[derive(Clone, Debug)] -pub enum EdgeGeometry { +pub enum PathGeometry { /// A simple straight edge. Line { source: Pos2, target: Pos2 }, @@ -18,36 +18,43 @@ pub enum EdgeGeometry { // We could add other geometries, such as `Orthogonal` here too. } +#[derive(Debug)] +pub struct EdgeGeometry { + pub target_arrow: bool, + pub path: PathGeometry, +} + impl EdgeGeometry { pub fn bounding_rect(&self) -> Rect { - match self { - Self::Line { source, target } => Rect::from_two_pos(*source, *target), + match self.path { + PathGeometry::Line { source, target } => Rect::from_two_pos(source, target), // TODO(grtlr): This is just a crude (upper) approximation, as the resulting bounding box can be too large. - Self::CubicBezier { + PathGeometry::CubicBezier { source, target, ref control, - } => Rect::from_points(&[&[*source, *target], control.as_slice()].concat()), + } => Rect::from_points(&[&[source, target], control.as_slice()].concat()), } } pub fn source_pos(&self) -> Pos2 { - match self { - Self::Line { source, .. } | Self::CubicBezier { source, .. } => *source, + match self.path { + PathGeometry::Line { source, .. } | PathGeometry::CubicBezier { source, .. } => source, } } pub fn target_pos(&self) -> Pos2 { - match self { - Self::Line { target, .. } | Self::CubicBezier { target, .. } => *target, + match self.path { + PathGeometry::Line { target, .. } | PathGeometry::CubicBezier { target, .. } => target, } } /// The direction of the edge at the source node (normalized). pub fn source_arrow_direction(&self) -> Vec2 { - match self { - Self::Line { source, target } => (source.to_vec2() - target.to_vec2()).normalized(), - Self::CubicBezier { + use PathGeometry::{CubicBezier, Line}; + match self.path { + Line { source, target } => (source.to_vec2() - target.to_vec2()).normalized(), + CubicBezier { source, control, .. } => (control[0].to_vec2() - source.to_vec2()).normalized(), } @@ -55,9 +62,10 @@ impl EdgeGeometry { /// The direction of the edge at the target node (normalized). pub fn target_arrow_direction(&self) -> Vec2 { - match self { - Self::Line { source, target } => (target.to_vec2() - source.to_vec2()).normalized(), - Self::CubicBezier { + use PathGeometry::{CubicBezier, Line}; + match self.path { + Line { source, target } => (target.to_vec2() - source.to_vec2()).normalized(), + CubicBezier { target, control, .. } => (target.to_vec2() - control[1].to_vec2()).normalized(), } @@ -92,4 +100,11 @@ impl Layout { pub fn get_edge(&self, from: NodeId, to: NodeId) -> Option<&[EdgeGeometry]> { self.edges.get(&(from, to)).map(|es| es.as_slice()) } + + /// Returns an iterator over all edges in the layout. + pub fn edges(&self) -> impl Iterator { + self.edges + .iter() + .map(|((from, to), es)| (from, to, es.as_slice())) + } } diff --git a/crates/viewer/re_space_view_graph/src/ui/draw.rs b/crates/viewer/re_space_view_graph/src/ui/draw.rs index 1f4283ef5da9..481ff43ba3ce 100644 --- a/crates/viewer/re_space_view_graph/src/ui/draw.rs +++ b/crates/viewer/re_space_view_graph/src/ui/draw.rs @@ -14,7 +14,7 @@ use re_viewer_context::{ use crate::{ graph::{Graph, Node}, - layout::{EdgeGeometry, Layout}, + layout::{EdgeGeometry, Layout, PathGeometry}, visualizers::Label, }; @@ -132,7 +132,7 @@ fn draw_text_label(ui: &mut Ui, label: &TextLabel, highlight: InteractionHighlig .sense(Sense::click()), ) }) - .inner + .response } /// Draws a node at the given position. @@ -198,11 +198,11 @@ pub fn draw_edge(ui: &mut Ui, geometry: &EdgeGeometry, show_arrow: bool) -> Resp let painter = ui.painter(); - match *geometry { - EdgeGeometry::Line { source, target } => { + match geometry.path { + PathGeometry::Line { source, target } => { painter.line_segment([source, target], stroke); } - EdgeGeometry::CubicBezier { + PathGeometry::CubicBezier { source, target, control, @@ -316,18 +316,37 @@ pub fn draw_graph( } }; + ui.painter().rect( + response.rect, + 0.0, + Color32::RED.gamma_multiply(0.5), + Stroke::new(1.0, Color32::RED), + ); + current_rect = current_rect.union(response.rect); } - for edge in graph.edges() { - for geometry in layout - .get_edge(edge.source, edge.target) - .unwrap_or_default() - { - let resp = draw_edge(ui, geometry, edge.arrow); - current_rect = current_rect.union(resp.rect); + for (_, _, geometries) in layout.edges() { + for geometry in geometries { + let response = draw_edge(ui, geometry, geometry.target_arrow); + + ui.painter().rect( + response.rect, + 0.0, + Color32::BLUE.gamma_multiply(0.5), + Stroke::new(1.0, Color32::BLUE), + ); + + current_rect = current_rect.union(response.rect); } } + ui.painter().rect( + current_rect, + 0.0, + Color32::GREEN.gamma_multiply(0.5), + Stroke::new(1.0, Color32::GREEN), + ); + current_rect } diff --git a/crates/viewer/re_space_view_graph/src/ui/state.rs b/crates/viewer/re_space_view_graph/src/ui/state.rs index 5536f8c31755..dad235b3586a 100644 --- a/crates/viewer/re_space_view_graph/src/ui/state.rs +++ b/crates/viewer/re_space_view_graph/src/ui/state.rs @@ -100,7 +100,7 @@ impl LayoutState { fn update(self, new_request: LayoutRequest) -> Self { match self { // Layout is up to date, nothing to do here. - Self::Finished { ref provider, .. } if &provider.request == &new_request => { + Self::Finished { ref provider, .. } if provider.request == new_request => { self // no op } // We need to recompute the layout. From e2f3ea1737d57b23c71fb0723a8f0ecf7a18956a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Fri, 29 Nov 2024 12:10:34 +0100 Subject: [PATCH 04/12] Include entity rects into layout --- .../src/layout/provider.rs | 95 +++++++++++-------- .../re_space_view_graph/src/layout/result.rs | 13 ++- .../viewer/re_space_view_graph/src/ui/draw.rs | 35 +++---- crates/viewer/re_space_view_graph/src/view.rs | 25 +---- .../release_checklist/check_graph_view.py | 3 + 5 files changed, 87 insertions(+), 84 deletions(-) diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index 1b579abcd8df..eb8f7fd2bef4 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -1,5 +1,5 @@ use egui::{Pos2, Rect, Vec2}; -use fjadra as fj; +use fjadra::{self as fj}; use crate::graph::NodeId; @@ -90,6 +90,7 @@ impl ForceLayoutProvider { nodes: extents, // Without any real node positions, we probably don't want to draw edges either. edges: ahash::HashMap::default(), + entities: Vec::new(), } } @@ -99,50 +100,64 @@ impl ForceLayoutProvider { let positions = self.simulation.positions().collect::>(); - for (node, extent) in &mut layout.nodes { - let i = self.node_index[node]; - let [x, y] = positions[i]; - let pos = Pos2::new(x as f32, y as f32); - extent.set_center(pos); - } + // We clear all unnecessary data deom the previous layout, but keep its space allocated. + layout.entities.clear(); + layout.edges.clear(); + + for (entity, graph) in &self.request.graphs { + + let mut current_rect = Rect::NOTHING; + + for node in graph.nodes.keys() { + let extent = layout.nodes.get_mut(node).expect("node has to be present"); + let i = self.node_index[node]; + let [x, y] = positions[i]; + let pos = Pos2::new(x as f32, y as f32); + extent.set_center(pos); + current_rect = current_rect.union(*extent); + } - // TODO(grtlr): Better to create slots for each edge. - for ((from, to), templates) in self.request.graphs.values().flat_map(|g| g.edges.iter()) { - let mut geometries = Vec::with_capacity(templates.len()); - match from == to { - true => { - for (i, template) in templates.iter().enumerate() { - let offset = (i + 1) as f32; - let target_arrow = template.target_arrow; - // Self-edges are not supported in force-based layouts. - let anchor = intersects_ray_from_center(layout.nodes[from], Vec2::UP); - geometries.push(EdgeGeometry { - target_arrow, - path: PathGeometry::CubicBezier { - // TODO(grtlr): We could probably consider the actual node size here. - source: anchor + Vec2::LEFT * 4., - target: anchor + Vec2::RIGHT * 4., - // TODO(grtlr): The actual length of that spline should follow the `distance` parameter of the link force. - control: [ - anchor + Vec2::new(-30. * offset, -40. * offset), - anchor + Vec2::new(30. * offset, -40. * offset), - ], - }, - }); + layout.entities.push((entity.clone(), current_rect)); + + // TODO(grtlr): Better to create slots for each edge. + for ((from, to), edges) in &graph.edges { + + let mut geometries = Vec::with_capacity(edges.len()); + match from == to { + true => { + for (i, template) in edges.iter().enumerate() { + let offset = (i + 1) as f32; + let target_arrow = template.target_arrow; + // Self-edges are not supported in force-based layouts. + let anchor = intersects_ray_from_center(layout.nodes[from], Vec2::UP); + geometries.push(EdgeGeometry { + target_arrow, + path: PathGeometry::CubicBezier { + // TODO(grtlr): We could probably consider the actual node size here. + source: anchor + Vec2::LEFT * 4., + target: anchor + Vec2::RIGHT * 4., + // TODO(grtlr): The actual length of that spline should follow the `distance` parameter of the link force. + control: [ + anchor + Vec2::new(-30. * offset, -40. * offset), + anchor + Vec2::new(30. * offset, -40. * offset), + ], + }, + }); + } } - } - false => { - // TODO(grtlr): perform similar computations here - for template in templates { - let target_arrow = template.target_arrow; - geometries.push(EdgeGeometry { - target_arrow, - path: line_segment(layout.nodes[from], layout.nodes[to]), - }); + false => { + // TODO(grtlr): perform similar computations here + for template in edges { + let target_arrow = template.target_arrow; + geometries.push(EdgeGeometry { + target_arrow, + path: line_segment(layout.nodes[from], layout.nodes[to]), + }); + } } } + layout.edges.insert((*from, *to), geometries); } - layout.edges.insert((*from, *to), geometries); } self.simulation.finished() diff --git a/crates/viewer/re_space_view_graph/src/layout/result.rs b/crates/viewer/re_space_view_graph/src/layout/result.rs index e32ab8657dc9..d01c562b9ff5 100644 --- a/crates/viewer/re_space_view_graph/src/layout/result.rs +++ b/crates/viewer/re_space_view_graph/src/layout/result.rs @@ -1,4 +1,5 @@ use egui::{Pos2, Rect, Vec2}; +use re_chunk::EntityPath; use crate::graph::NodeId; @@ -76,7 +77,7 @@ impl EdgeGeometry { pub struct Layout { pub(super) nodes: ahash::HashMap, pub(super) edges: ahash::HashMap<(NodeId, NodeId), Vec>, - // TODO(grtlr): Consider adding the entity rects here too. + pub(super) entities: Vec<(EntityPath, Rect)>, } fn bounding_rect_from_iter(rectangles: impl Iterator) -> egui::Rect { @@ -107,4 +108,14 @@ impl Layout { .iter() .map(|((from, to), es)| (from, to, es.as_slice())) } + + /// Returns the number of entities in the layout. + pub fn num_entities(&self) -> usize { + self.entities.len() + } + + /// Returns an iterator over all edges in the layout. + pub fn entities(&self) -> impl Iterator { + self.entities.iter() + } } diff --git a/crates/viewer/re_space_view_graph/src/ui/draw.rs b/crates/viewer/re_space_view_graph/src/ui/draw.rs index 481ff43ba3ce..7e468e84734e 100644 --- a/crates/viewer/re_space_view_graph/src/ui/draw.rs +++ b/crates/viewer/re_space_view_graph/src/ui/draw.rs @@ -270,7 +270,6 @@ pub fn draw_entity_rect( } /// Draws the graph using the layout. -#[must_use] pub fn draw_graph( ui: &mut Ui, ctx: &ViewerContext<'_>, @@ -316,37 +315,29 @@ pub fn draw_graph( } }; - ui.painter().rect( - response.rect, - 0.0, - Color32::RED.gamma_multiply(0.5), - Stroke::new(1.0, Color32::RED), - ); - current_rect = current_rect.union(response.rect); } for (_, _, geometries) in layout.edges() { for geometry in geometries { let response = draw_edge(ui, geometry, geometry.target_arrow); - - ui.painter().rect( - response.rect, - 0.0, - Color32::BLUE.gamma_multiply(0.5), - Stroke::new(1.0, Color32::BLUE), - ); - current_rect = current_rect.union(response.rect); } } - ui.painter().rect( - current_rect, - 0.0, - Color32::GREEN.gamma_multiply(0.5), - Stroke::new(1.0, Color32::GREEN), - ); + // We only show entity rects if there are multiple entities. + // For now, these entity rects are not part of the layout, but rather tracked on the fly. + if layout.num_entities() > 1 { + for (entity_path, rect) in layout.entities() { + let resp = draw_entity_rect(ui, *rect, entity_path, &query.highlights); + current_rect = current_rect.union(resp.rect); + let instance_path = InstancePath::entity_all(entity_path.clone()); + ctx.select_hovered_on_click( + &resp, + vec![(Item::DataResult(query.space_view_id, instance_path), None)].into_iter(), + ); + } + } current_rect } diff --git a/crates/viewer/re_space_view_graph/src/view.rs b/crates/viewer/re_space_view_graph/src/view.rs index cef909294e39..d38f51d0d745 100644 --- a/crates/viewer/re_space_view_graph/src/view.rs +++ b/crates/viewer/re_space_view_graph/src/view.rs @@ -1,4 +1,3 @@ -use re_entity_db::InstancePath; use re_log_types::EntityPath; use re_space_view::{ controls::{DRAG_PAN2D_BUTTON, ZOOM_SCROLL_MODIFIER}, @@ -14,7 +13,7 @@ use re_ui::{ ModifiersMarkdown, MouseButtonMarkdown, UiExt as _, }; use re_viewer_context::{ - IdentifiedViewSystem as _, Item, RecommendedSpaceView, SpaceViewClass, + IdentifiedViewSystem as _, RecommendedSpaceView, SpaceViewClass, SpaceViewClassLayoutPriority, SpaceViewClassRegistryError, SpaceViewId, SpaceViewSpawnHeuristics, SpaceViewState, SpaceViewStateExt as _, SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, SystemExecutionOutput, ViewQuery, @@ -25,7 +24,7 @@ use re_viewport_blueprint::ViewProperty; use crate::{ graph::Graph, layout::LayoutRequest, - ui::{draw_debug, draw_entity_rect, draw_graph, GraphSpaceViewState}, + ui::{draw_debug, draw_graph, GraphSpaceViewState}, visualizers::{merge, EdgesVisualizer, NodeVisualizer}, }; @@ -178,24 +177,8 @@ Display a graph of nodes and edges. let mut world_bounding_rect = egui::Rect::NOTHING; for graph in &graphs { - let mut current_rect = draw_graph(ui, ctx, graph, layout, query); - - // We only show entity rects if there are multiple entities. - // For now, these entity rects are not part of the layout, but rather tracked on the fly. - if graphs.len() > 1 { - let resp = - draw_entity_rect(ui, current_rect, graph.entity(), &query.highlights); - - let instance_path = InstancePath::entity_all(graph.entity().clone()); - ctx.select_hovered_on_click( - &resp, - vec![(Item::DataResult(query.space_view_id, instance_path), None)] - .into_iter(), - ); - current_rect = current_rect.union(resp.rect); - } - - world_bounding_rect = world_bounding_rect.union(current_rect); + let graph_rect = draw_graph(ui, ctx, graph, layout, query); + world_bounding_rect = world_bounding_rect.union(graph_rect); } // We need to draw the debug information after the rest to ensure that we have the correct bounding box. diff --git a/tests/python/release_checklist/check_graph_view.py b/tests/python/release_checklist/check_graph_view.py index b393e0d069cf..8282610e2cf9 100644 --- a/tests/python/release_checklist/check_graph_view.py +++ b/tests/python/release_checklist/check_graph_view.py @@ -12,6 +12,9 @@ Please check the following: * All graphs have a proper layout. +* The `Weird Graph` views show: + * two self-edges for `A`, a single one for `B`. + * Additionally, there should be three edges between `A` and `B`. * `graph` has directed edges, while `graph2` has undirected edges. * `graph` and `graph2` are shown in two different viewers. * There is a third viewer, `Both`, that shows both `graph` and `graph2` in the same viewer. From 817eed0c492bf5fedead6ead25e8f27d59fae084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Fri, 29 Nov 2024 12:18:12 +0100 Subject: [PATCH 05/12] Fix lint --- crates/viewer/re_space_view_graph/src/ui/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/viewer/re_space_view_graph/src/ui/mod.rs b/crates/viewer/re_space_view_graph/src/ui/mod.rs index e387d8568276..57c7974e5b72 100644 --- a/crates/viewer/re_space_view_graph/src/ui/mod.rs +++ b/crates/viewer/re_space_view_graph/src/ui/mod.rs @@ -1,5 +1,5 @@ mod draw; mod state; -pub use draw::{draw_debug, draw_entity_rect, draw_graph, DrawableLabel}; +pub use draw::{draw_debug, draw_graph, DrawableLabel}; pub use state::GraphSpaceViewState; From 70bc926579a6b9225e8268278e9e5a04853c8d2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Fri, 29 Nov 2024 16:48:22 +0100 Subject: [PATCH 06/12] [skip ci] Most infrastructure in place --- .../src/graph/{index.rs => ids.rs} | 13 +++ .../re_space_view_graph/src/graph/mod.rs | 13 ++- .../re_space_view_graph/src/layout/mod.rs | 1 + .../src/layout/provider.rs | 102 ++++++++++++++---- .../re_space_view_graph/src/layout/request.rs | 11 +- .../re_space_view_graph/src/layout/result.rs | 12 +-- .../re_space_view_graph/src/layout/slots.rs | 57 ++++++++++ .../viewer/re_space_view_graph/src/ui/draw.rs | 2 +- 8 files changed, 176 insertions(+), 35 deletions(-) rename crates/viewer/re_space_view_graph/src/graph/{index.rs => ids.rs} (78%) create mode 100644 crates/viewer/re_space_view_graph/src/layout/slots.rs diff --git a/crates/viewer/re_space_view_graph/src/graph/index.rs b/crates/viewer/re_space_view_graph/src/graph/ids.rs similarity index 78% rename from crates/viewer/re_space_view_graph/src/graph/index.rs rename to crates/viewer/re_space_view_graph/src/graph/ids.rs index 257c7ca389a9..396a2dc3ef8c 100644 --- a/crates/viewer/re_space_view_graph/src/graph/index.rs +++ b/crates/viewer/re_space_view_graph/src/graph/ids.rs @@ -34,3 +34,16 @@ impl std::fmt::Debug for NodeId { write!(f, "NodeIndex({:?}@{:?})", self.node_hash, self.entity_hash) } } + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub struct EdgeId { + // TODO(grtlr): Consider something more storage efficient here + pub source: NodeId, + pub target: NodeId, +} + +impl EdgeId { + pub fn is_self_edge(&self) -> bool { + self.source == self.target + } +} diff --git a/crates/viewer/re_space_view_graph/src/graph/mod.rs b/crates/viewer/re_space_view_graph/src/graph/mod.rs index 21e38bf73665..03a2ab374337 100644 --- a/crates/viewer/re_space_view_graph/src/graph/mod.rs +++ b/crates/viewer/re_space_view_graph/src/graph/mod.rs @@ -2,8 +2,8 @@ mod hash; use egui::{Pos2, Vec2}; pub(crate) use hash::GraphNodeHash; -mod index; -pub(crate) use index::NodeId; +mod ids; +pub(crate) use ids::{EdgeId, NodeId}; use re_chunk::EntityPath; use re_types::components::{self, GraphType}; @@ -76,6 +76,15 @@ pub struct Edge { pub arrow: bool, } +impl Edge { + pub fn id(&self) -> EdgeId { + EdgeId { + source: self.source, + target: self.target, + } + } +} + pub struct Graph { entity: EntityPath, nodes: Vec, diff --git a/crates/viewer/re_space_view_graph/src/layout/mod.rs b/crates/viewer/re_space_view_graph/src/layout/mod.rs index b140159c1f1d..fdcd7c5dfbb5 100644 --- a/crates/viewer/re_space_view_graph/src/layout/mod.rs +++ b/crates/viewer/re_space_view_graph/src/layout/mod.rs @@ -1,6 +1,7 @@ mod provider; mod request; mod result; +mod slots; pub use provider::ForceLayoutProvider; pub use request::LayoutRequest; diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index eb8f7fd2bef4..148f2c38e3ef 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -1,12 +1,13 @@ use egui::{Pos2, Rect, Vec2}; use fjadra::{self as fj}; -use crate::graph::NodeId; +use crate::graph::{EdgeId, NodeId}; use super::{ request::NodeTemplate, - result::{EdgeGeometry, PathGeometry}, - Layout, LayoutRequest, + result::PathGeometry, + slots::{slotted_edges, Slot, SlotKind}, + EdgeGeometry, Layout, LayoutRequest, }; impl<'a> From<&'a NodeTemplate> for fj::Node { @@ -50,8 +51,8 @@ impl ForceLayoutProvider { // Looking at self-edges does not make sense in a force-based layout, so we filter those out. let considered_edges = all_edges_iter .clone() - .filter(|((from, to), _)| (from != to)) - .map(|((from, to), _)| (node_index[from], node_index[to])); + .filter(|(id, _)| !id.is_self_edge()) + .map(|(id, _)| (node_index[&id.source], node_index[&id.target])); // TODO(grtlr): Currently we guesstimate good forces. Eventually these should be exposed as blueprints. let simulation = fj::SimulationBuilder::default() @@ -59,7 +60,9 @@ impl ForceLayoutProvider { .build(all_nodes) .add_force( "link", - fj::Link::new(considered_edges).distance(50.0).iterations(2), + fj::Link::new(considered_edges) + .distance(150.0) + .iterations(2), ) .add_force("charge", fj::ManyBody::new()) // TODO(grtlr): This is a small stop-gap until we have blueprints to prevent nodes from flying away. @@ -105,7 +108,6 @@ impl ForceLayoutProvider { layout.edges.clear(); for (entity, graph) in &self.request.graphs { - let mut current_rect = Rect::NOTHING; for node in graph.nodes.keys() { @@ -119,17 +121,25 @@ impl ForceLayoutProvider { layout.entities.push((entity.clone(), current_rect)); - // TODO(grtlr): Better to create slots for each edge. - for ((from, to), edges) in &graph.edges { - - let mut geometries = Vec::with_capacity(edges.len()); - match from == to { - true => { - for (i, template) in edges.iter().enumerate() { + // Multiple edges can occupy the same space in the layout. + for Slot { kind, edges } in + slotted_edges(graph.edges.values().flat_map(|ts| ts.iter())).values() + { + match kind { + SlotKind::SelfEdge => { + for (i, edge) in edges.iter().enumerate() { let offset = (i + 1) as f32; - let target_arrow = template.target_arrow; + let target_arrow = edge.target_arrow; // Self-edges are not supported in force-based layouts. - let anchor = intersects_ray_from_center(layout.nodes[from], Vec2::UP); + let anchor = + intersects_ray_from_center(layout.nodes[&edge.source], Vec2::UP); + let geometries = layout + .edges + .entry(EdgeId { + source: edge.source, + target: edge.target, + }) + .or_default(); geometries.push(EdgeGeometry { target_arrow, path: PathGeometry::CubicBezier { @@ -145,18 +155,66 @@ impl ForceLayoutProvider { }); } } - false => { - // TODO(grtlr): perform similar computations here - for template in edges { - let target_arrow = template.target_arrow; + SlotKind::Regular => { + if let &[edge] = edges.as_slice() { + // A single regular straight edge. + + let target_arrow = edge.target_arrow; + let geometries = layout + .edges + .entry(EdgeId { + source: edge.source, + target: edge.target, + }) + .or_default(); geometries.push(EdgeGeometry { target_arrow, - path: line_segment(layout.nodes[from], layout.nodes[to]), + path: line_segment( + layout.nodes[&edge.source], + layout.nodes[&edge.target], + ), }); + } else { + // Multiple edges occupy the same space. + let num_edges = edges.len(); + let fan_amount = 20.0; + + for (i, edge) in edges.iter().enumerate() { + // Calculate an offset for the control points based on index `i` + let offset = (i as f32 - (num_edges as f32 / 2.0)) * fan_amount; + + let source_rect = layout.nodes[&edge.source]; + let target_rect = layout.nodes[&edge.target]; + + let d = (target_rect.center() - source_rect.center()).normalized(); + + let source_pos = intersects_ray_from_center(source_rect, d); + let target_pos = intersects_ray_from_center(target_rect, -d); + + // Compute control points, `c1` and `c2`, based on the offset + let c1 = Pos2::new(source_pos.x + offset, source_pos.y - offset); + let c2 = Pos2::new(target_pos.x + offset, target_pos.y + offset); + + let geometries = layout + .edges + .entry(EdgeId { + source: edge.source, + target: edge.target, + }) + .or_default(); + + geometries.push(EdgeGeometry { + target_arrow: edge.target_arrow, + path: PathGeometry::CubicBezier { + source: source_pos, + target: target_pos, + control: [c1, c2], + }, + }); + } } } } - layout.edges.insert((*from, *to), geometries); } } diff --git a/crates/viewer/re_space_view_graph/src/layout/request.rs b/crates/viewer/re_space_view_graph/src/layout/request.rs index ac3b9d4eade5..99cf91d1571d 100644 --- a/crates/viewer/re_space_view_graph/src/layout/request.rs +++ b/crates/viewer/re_space_view_graph/src/layout/request.rs @@ -11,7 +11,7 @@ use std::collections::BTreeMap; use egui::{Pos2, Vec2}; use re_chunk::EntityPath; -use crate::graph::{Graph, NodeId}; +use crate::graph::{EdgeId, Graph, NodeId}; #[derive(PartialEq)] pub(super) struct NodeTemplate { @@ -19,10 +19,11 @@ pub(super) struct NodeTemplate { pub(super) fixed_position: Option, } - // TODO(grtlr): Does it make sense to consolidate some of the types, i.e. `Edge` ad `EdgeTemplate`? #[derive(PartialEq)] pub(super) struct EdgeTemplate { + pub(super) source: NodeId, + pub(super) target: NodeId, pub(super) target_arrow: bool, } @@ -33,7 +34,7 @@ pub(super) struct GraphTemplate { /// The edges in the layout. /// /// Each entry can contain multiple edges. - pub(super) edges: BTreeMap<(NodeId, NodeId), Vec>, + pub(super) edges: BTreeMap>, } /// A [`LayoutRequest`] encapsulates all the information that is considered when computing a layout. @@ -66,8 +67,10 @@ impl LayoutRequest { } for edge in graph.edges() { - let es = entity.edges.entry((edge.source, edge.target)).or_default(); + let es = entity.edges.entry(edge.id()).or_default(); es.push(EdgeTemplate { + source: edge.source, + target: edge.target, target_arrow: edge.arrow, }); } diff --git a/crates/viewer/re_space_view_graph/src/layout/result.rs b/crates/viewer/re_space_view_graph/src/layout/result.rs index d01c562b9ff5..84b6796ad299 100644 --- a/crates/viewer/re_space_view_graph/src/layout/result.rs +++ b/crates/viewer/re_space_view_graph/src/layout/result.rs @@ -1,7 +1,7 @@ use egui::{Pos2, Rect, Vec2}; use re_chunk::EntityPath; -use crate::graph::NodeId; +use crate::graph::{EdgeId, NodeId}; #[derive(Clone, Debug)] pub enum PathGeometry { @@ -76,7 +76,7 @@ impl EdgeGeometry { #[derive(Debug)] pub struct Layout { pub(super) nodes: ahash::HashMap, - pub(super) edges: ahash::HashMap<(NodeId, NodeId), Vec>, + pub(super) edges: ahash::HashMap>, pub(super) entities: Vec<(EntityPath, Rect)>, } @@ -98,15 +98,15 @@ impl Layout { } /// Gets the shape of an edge in the final layout. - pub fn get_edge(&self, from: NodeId, to: NodeId) -> Option<&[EdgeGeometry]> { - self.edges.get(&(from, to)).map(|es| es.as_slice()) + pub fn get_edge(&self, edge: &EdgeId) -> Option<&[EdgeGeometry]> { + self.edges.get(edge).map(|es| es.as_slice()) } /// Returns an iterator over all edges in the layout. - pub fn edges(&self) -> impl Iterator { + pub fn edges(&self) -> impl Iterator { self.edges .iter() - .map(|((from, to), es)| (from, to, es.as_slice())) + .map(|(id, es)| (*id, es.as_slice())) } /// Returns the number of entities in the layout. diff --git a/crates/viewer/re_space_view_graph/src/layout/slots.rs b/crates/viewer/re_space_view_graph/src/layout/slots.rs new file mode 100644 index 000000000000..1a3e3a5ab426 --- /dev/null +++ b/crates/viewer/re_space_view_graph/src/layout/slots.rs @@ -0,0 +1,57 @@ +//! Force-directed graph layouts assume edges to be straight lines. This +//! function brings edges in a canonical form and finds the ones that occupy the same space. + +use crate::graph::{EdgeId, NodeId}; + +use super::request::EdgeTemplate; + +/// Uniquely identifies an [`EdgeSlot`] by ordering the [`NodeIds`](NodeId). +#[derive(Clone, Copy, Hash, PartialEq, Eq)] +pub struct SlotId(NodeId, NodeId); + +impl SlotId { + pub fn new(source: NodeId, target: NodeId) -> Self { + if source < target { + Self(source, target) + } else { + Self(target, source) + } + } +} + +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum SlotKind { + /// A regular edge going from `source` to `target`. + Regular, + /// An edge where `source == target`. + SelfEdge, +} + +pub struct Slot<'a> { + pub kind: SlotKind, + pub edges: Vec<&'a EdgeTemplate>, +} + +pub fn slotted_edges<'a>( + edges: impl Iterator, +) -> ahash::HashMap> { + let mut slots: ahash::HashMap> = ahash::HashMap::default(); + + for e in edges { + let slot = slots + .entry(SlotId::new(e.source, e.target)) + .or_insert_with(|| Slot { + kind: match e.source == e.target { + true => SlotKind::SelfEdge, + false => SlotKind::Regular, + }, + edges: Vec::new(), + }); + + slot.edges.push(e); + } + + slots +} + +// TODO(grtlr): Write test cases diff --git a/crates/viewer/re_space_view_graph/src/ui/draw.rs b/crates/viewer/re_space_view_graph/src/ui/draw.rs index 7e468e84734e..6c51dd576a87 100644 --- a/crates/viewer/re_space_view_graph/src/ui/draw.rs +++ b/crates/viewer/re_space_view_graph/src/ui/draw.rs @@ -318,7 +318,7 @@ pub fn draw_graph( current_rect = current_rect.union(response.rect); } - for (_, _, geometries) in layout.edges() { + for (_, geometries) in layout.edges() { for geometry in geometries { let response = draw_edge(ui, geometry, geometry.target_arrow); current_rect = current_rect.union(response.rect); From c550b56479228b510fe71118ff889048964b111d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Mon, 2 Dec 2024 09:35:10 +0100 Subject: [PATCH 07/12] [skip c] Basic fan edges working --- Cargo.lock | 22 +-- Cargo.toml | 15 +- .../src/layout/provider.rs | 146 ++++++------------ .../re_space_view_graph/src/layout/slots.rs | 19 ++- .../viewer/re_space_view_graph/src/ui/draw.rs | 3 + .../release_checklist/check_graph_view.py | 6 +- 6 files changed, 85 insertions(+), 126 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e717241ce88c..65d07b54d466 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1917,7 +1917,7 @@ checksum = "0d6ef0072f8a535281e4876be788938b528e9a1d43900b82c2569af7da799125" [[package]] name = "ecolor" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "bytemuck", "emath", @@ -1933,7 +1933,7 @@ checksum = "18aade80d5e09429040243ce1143ddc08a92d7a22820ac512610410a4dd5214f" [[package]] name = "eframe" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "ahash", "bytemuck", @@ -1972,7 +1972,7 @@ dependencies = [ [[package]] name = "egui" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "accesskit", "ahash", @@ -1989,7 +1989,7 @@ dependencies = [ [[package]] name = "egui-wgpu" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "ahash", "bytemuck", @@ -2008,7 +2008,7 @@ dependencies = [ [[package]] name = "egui-winit" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "accesskit_winit", "ahash", @@ -2050,7 +2050,7 @@ dependencies = [ [[package]] name = "egui_extras" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "ahash", "egui", @@ -2067,7 +2067,7 @@ dependencies = [ [[package]] name = "egui_glow" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "ahash", "bytemuck", @@ -2085,7 +2085,7 @@ dependencies = [ [[package]] name = "egui_kittest" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "dify", "egui", @@ -2154,7 +2154,7 @@ checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" [[package]] name = "emath" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "bytemuck", "serde", @@ -2270,7 +2270,7 @@ dependencies = [ [[package]] name = "epaint" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" dependencies = [ "ab_glyph", "ahash", @@ -2289,7 +2289,7 @@ dependencies = [ [[package]] name = "epaint_default_fonts" version = "0.29.1" -source = "git+https://github.com/emilk/egui.git?rev=84cc1572b175d49a64f1b323a6d7e56b1f1fba66#84cc1572b175d49a64f1b323a6d7e56b1f1fba66" +source = "git+https://github.com/emilk/egui.git?rev=6833cf56e17d2581163c43eb36dc3ee5d758771b#6833cf56e17d2581163c43eb36dc3ee5d758771b" [[package]] name = "equivalent" diff --git a/Cargo.toml b/Cargo.toml index 5e63e6e0c5d2..25c61da38bd1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -558,14 +558,13 @@ significant_drop_tightening = "allow" # An update of parking_lot made this trigg # As a last resport, patch with a commit to our own repository. # ALWAYS document what PR the commit hash is part of, or when it was merged into the upstream trunk. -ecolor = { git = "https://github.com/emilk/egui.git", rev = "84cc1572b175d49a64f1b323a6d7e56b1f1fba66" } # egui master 2024-11-27 -eframe = { git = "https://github.com/emilk/egui.git", rev = "84cc1572b175d49a64f1b323a6d7e56b1f1fba66" } # egui master 2024-11-27 -egui = { git = "https://github.com/emilk/egui.git", rev = "84cc1572b175d49a64f1b323a6d7e56b1f1fba66" } # egui master 2024-11-27 -egui_extras = { git = "https://github.com/emilk/egui.git", rev = "84cc1572b175d49a64f1b323a6d7e56b1f1fba66" } # egui master 2024-11-27 -egui-wgpu = { git = "https://github.com/emilk/egui.git", rev = "84cc1572b175d49a64f1b323a6d7e56b1f1fba66" } # egui master 2024-11-27 -emath = { git = "https://github.com/emilk/egui.git", rev = "84cc1572b175d49a64f1b323a6d7e56b1f1fba66" } # egui master 2024-11-27 - -egui_kittest = { git = "https://github.com/emilk/egui.git", rev = "84cc1572b175d49a64f1b323a6d7e56b1f1fba66" } # egui master 2024-11-27 +ecolor = { git = "https://github.com/emilk/egui.git", rev = "6833cf56e17d2581163c43eb36dc3ee5d758771b" } # egui master 2024-12-02 +eframe = { git = "https://github.com/emilk/egui.git", rev = "6833cf56e17d2581163c43eb36dc3ee5d758771b" } # egui master 2024-12-02 +egui = { git = "https://github.com/emilk/egui.git", rev = "6833cf56e17d2581163c43eb36dc3ee5d758771b" } # egui master 2024-12-02 +egui_extras = { git = "https://github.com/emilk/egui.git", rev = "6833cf56e17d2581163c43eb36dc3ee5d758771b" } # egui master 2024-12-02 +egui_kittest = { git = "https://github.com/emilk/egui.git", rev = "6833cf56e17d2581163c43eb36dc3ee5d758771b" } # egui master 2024-12-02 +egui-wgpu = { git = "https://github.com/emilk/egui.git", rev = "6833cf56e17d2581163c43eb36dc3ee5d758771b" } # egui master 2024-12-02 +emath = { git = "https://github.com/emilk/egui.git", rev = "6833cf56e17d2581163c43eb36dc3ee5d758771b" } # egui master 2024-12-02 # Useful while developing: # ecolor = { path = "../../egui/crates/ecolor" } diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index 148f2c38e3ef..338ac8df2c87 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -132,7 +132,7 @@ impl ForceLayoutProvider { let target_arrow = edge.target_arrow; // Self-edges are not supported in force-based layouts. let anchor = - intersects_ray_from_center(layout.nodes[&edge.source], Vec2::UP); + layout.nodes[&edge.source].intersects_ray_from_center(Vec2::UP); let geometries = layout .edges .entry(EdgeId { @@ -155,10 +155,12 @@ impl ForceLayoutProvider { }); } } - SlotKind::Regular => { + SlotKind::Regular { + source: slot_source, + target: slot_target, + } => { if let &[edge] = edges.as_slice() { // A single regular straight edge. - let target_arrow = edge.target_arrow; let geometries = layout .edges @@ -175,7 +177,7 @@ impl ForceLayoutProvider { ), }); } else { - // Multiple edges occupy the same space. + // Multiple edges occupy the same space, so we fan them out. let num_edges = edges.len(); let fan_amount = 20.0; @@ -183,17 +185,33 @@ impl ForceLayoutProvider { // Calculate an offset for the control points based on index `i` let offset = (i as f32 - (num_edges as f32 / 2.0)) * fan_amount; - let source_rect = layout.nodes[&edge.source]; - let target_rect = layout.nodes[&edge.target]; + let source_rect = layout.nodes[slot_source]; + let target_rect = layout.nodes[slot_target]; let d = (target_rect.center() - source_rect.center()).normalized(); - let source_pos = intersects_ray_from_center(source_rect, d); - let target_pos = intersects_ray_from_center(target_rect, -d); + let source_pos = source_rect.intersects_ray_from_center(d); + let target_pos = target_rect.intersects_ray_from_center(-d); + + // How far along the edge should the control points be? + let c1_base = source_pos + (target_pos - source_pos) * 0.25; + let c2_base = source_pos + (target_pos - source_pos) * 0.75; + + let c1_base_n = Vec2::new(-c1_base.y, c1_base.x).normalized(); + let mut c2_base_n = Vec2::new(-c2_base.y, c2_base.x).normalized(); - // Compute control points, `c1` and `c2`, based on the offset - let c1 = Pos2::new(source_pos.x + offset, source_pos.y - offset); - let c2 = Pos2::new(target_pos.x + offset, target_pos.y + offset); + // Make sure both point to the same side of the edge. + if c1_base_n.dot(c2_base_n) < 0.0 { + // If they point in opposite directions, flip one of them. + c2_base_n = -c2_base_n; + } + + let c1_left = c1_base + c1_base_n * offset; + let c2_left = c2_base + c2_base_n * offset; + + // // Compute control points, `c1` and `c2`, based on the offset + // let c1 = Pos2::new(source_pos.x + offset, source_pos.y - offset); + // let c2 = Pos2::new(target_pos.x + offset, target_pos.y + offset); let geometries = layout .edges @@ -203,13 +221,24 @@ impl ForceLayoutProvider { }) .or_default(); - geometries.push(EdgeGeometry { - target_arrow: edge.target_arrow, - path: PathGeometry::CubicBezier { + // We potentially need to restore the direction of the edge, after we have used it's cannonical form earlier. + let path = if edge.source == *slot_source { + PathGeometry::CubicBezier { source: source_pos, target: target_pos, - control: [c1, c2], - }, + control: [c1_left, c2_left], + } + } else { + PathGeometry::CubicBezier { + source: target_pos, + target: source_pos, + control: [c2_left, c1_left], + } + }; + + geometries.push(EdgeGeometry { + target_arrow: edge.target_arrow, + path, }); } } @@ -231,92 +260,11 @@ fn line_segment(source: Rect, target: Rect) -> PathGeometry { let direction = (target_center - source_center).normalized(); // Find the border points on both rectangles - let source_point = intersects_ray_from_center(source, direction); - let target_point = intersects_ray_from_center(target, -direction); // Reverse direction for target + let source_point = source.intersects_ray_from_center(direction); + let target_point = target.intersects_ray_from_center(-direction); // Reverse direction for target PathGeometry::Line { source: source_point, target: target_point, } } - -/// Helper function to find the point where the line intersects the border of a rectangle -fn intersects_ray_from_center(rect: Rect, direction: Vec2) -> Pos2 { - let mut tmin = f32::NEG_INFINITY; - let mut tmax = f32::INFINITY; - - for i in 0..2 { - let inv_d = 1.0 / -direction[i]; - let mut t0 = (rect.min[i] - rect.center()[i]) * inv_d; - let mut t1 = (rect.max[i] - rect.center()[i]) * inv_d; - - if inv_d < 0.0 { - std::mem::swap(&mut t0, &mut t1); - } - - tmin = tmin.max(t0); - tmax = tmax.min(t1); - } - - let t = tmax.min(tmin); // Pick the first intersection - rect.center() + t * -direction -} - -#[cfg(test)] -mod test { - use super::*; - use egui::pos2; - - #[test] - fn test_ray_intersection() { - let rect = Rect::from_min_max(pos2(1.0, 1.0), pos2(3.0, 3.0)); - - assert_eq!( - intersects_ray_from_center(rect, Vec2::RIGHT), - pos2(3.0, 2.0), - "rightward ray" - ); - - assert_eq!( - intersects_ray_from_center(rect, Vec2::UP), - pos2(2.0, 1.0), - "upward ray" - ); - - assert_eq!( - intersects_ray_from_center(rect, Vec2::LEFT), - pos2(1.0, 2.0), - "leftward ray" - ); - - assert_eq!( - intersects_ray_from_center(rect, Vec2::DOWN), - pos2(2.0, 3.0), - "downward ray" - ); - - assert_eq!( - intersects_ray_from_center(rect, (Vec2::LEFT + Vec2::DOWN).normalized()), - pos2(1.0, 3.0), - "bottom-left corner ray" - ); - - assert_eq!( - intersects_ray_from_center(rect, (Vec2::LEFT + Vec2::UP).normalized()), - pos2(1.0, 1.0), - "top-left corner ray" - ); - - assert_eq!( - intersects_ray_from_center(rect, (Vec2::RIGHT + Vec2::DOWN).normalized()), - pos2(3.0, 3.0), - "bottom-right corner ray" - ); - - assert_eq!( - intersects_ray_from_center(rect, (Vec2::RIGHT + Vec2::UP).normalized()), - pos2(3.0, 1.0), - "top-right corner ray" - ); - } -} diff --git a/crates/viewer/re_space_view_graph/src/layout/slots.rs b/crates/viewer/re_space_view_graph/src/layout/slots.rs index 1a3e3a5ab426..93671488fce0 100644 --- a/crates/viewer/re_space_view_graph/src/layout/slots.rs +++ b/crates/viewer/re_space_view_graph/src/layout/slots.rs @@ -1,7 +1,7 @@ //! Force-directed graph layouts assume edges to be straight lines. This //! function brings edges in a canonical form and finds the ones that occupy the same space. -use crate::graph::{EdgeId, NodeId}; +use crate::graph::NodeId; use super::request::EdgeTemplate; @@ -21,8 +21,11 @@ impl SlotId { #[derive(Clone, Copy, PartialEq, Eq)] pub enum SlotKind { - /// A regular edge going from `source` to `target`. - Regular, + /// An edge slot going from `source` to `target`. Source and target represent the canonical order of the slot, as specified by [`SlotId`] + Regular { + source: NodeId, + target: NodeId, + }, /// An edge where `source == target`. SelfEdge, } @@ -38,12 +41,16 @@ pub fn slotted_edges<'a>( let mut slots: ahash::HashMap> = ahash::HashMap::default(); for e in edges { + let id = SlotId::new(e.source, e.target); let slot = slots - .entry(SlotId::new(e.source, e.target)) - .or_insert_with(|| Slot { + .entry(id) + .or_insert_with_key(|id| Slot { kind: match e.source == e.target { true => SlotKind::SelfEdge, - false => SlotKind::Regular, + false => SlotKind::Regular { + source: id.0, + target: id.1, + }, }, edges: Vec::new(), }); diff --git a/crates/viewer/re_space_view_graph/src/ui/draw.rs b/crates/viewer/re_space_view_graph/src/ui/draw.rs index 6c51dd576a87..f16bd4b9a6e5 100644 --- a/crates/viewer/re_space_view_graph/src/ui/draw.rs +++ b/crates/viewer/re_space_view_graph/src/ui/draw.rs @@ -213,6 +213,9 @@ pub fn draw_edge(ui: &mut Ui, geometry: &EdgeGeometry, show_arrow: bool) -> Resp fill: Color32::TRANSPARENT, stroke: stroke.into(), }); + + painter.circle(control[0], 1.0, Color32::RED, Stroke::NONE); + painter.circle(control[1], 1.0, Color32::GREEN, Stroke::NONE); } } diff --git a/tests/python/release_checklist/check_graph_view.py b/tests/python/release_checklist/check_graph_view.py index 8282610e2cf9..91a9ccbe84c7 100644 --- a/tests/python/release_checklist/check_graph_view.py +++ b/tests/python/release_checklist/check_graph_view.py @@ -14,7 +14,9 @@ * All graphs have a proper layout. * The `Weird Graph` views show: * two self-edges for `A`, a single one for `B`. - * Additionally, there should be three edges between `A` and `B`. + * Additionally, there should be: + * two edges from `A` to `B`. + * one edge from `B` to `A`. * `graph` has directed edges, while `graph2` has undirected edges. * `graph` and `graph2` are shown in two different viewers. * There is a third viewer, `Both`, that shows both `graph` and `graph2` in the same viewer. @@ -35,7 +37,7 @@ def log_weird_graph() -> None: ("B", "B"), # duplicated edges ("A", "B"), - ("B", "A"), + ("A", "B"), ("B", "A"), # duplicated self-edges ("A", "A"), From d2c44807797d16c1040f6a380985c49851d2c2b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Mon, 2 Dec 2024 10:02:12 +0100 Subject: [PATCH 08/12] Slots working as expected --- .../src/layout/provider.rs | 25 +++++++++++------- .../re_space_view_graph/src/layout/result.rs | 4 +-- .../re_space_view_graph/src/layout/slots.rs | 26 ++++++++----------- .../viewer/re_space_view_graph/src/ui/draw.rs | 3 --- crates/viewer/re_space_view_graph/src/view.rs | 9 +++---- 5 files changed, 31 insertions(+), 36 deletions(-) diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index 338ac8df2c87..818e1fc5e2bf 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -179,12 +179,11 @@ impl ForceLayoutProvider { } else { // Multiple edges occupy the same space, so we fan them out. let num_edges = edges.len(); + + // Controls the amount of space (in scene coordinates) that a slot can occupy. let fan_amount = 20.0; for (i, edge) in edges.iter().enumerate() { - // Calculate an offset for the control points based on index `i` - let offset = (i as f32 - (num_edges as f32 / 2.0)) * fan_amount; - let source_rect = layout.nodes[slot_source]; let target_rect = layout.nodes[slot_target]; @@ -206,12 +205,18 @@ impl ForceLayoutProvider { c2_base_n = -c2_base_n; } - let c1_left = c1_base + c1_base_n * offset; - let c2_left = c2_base + c2_base_n * offset; + let c1_left = c1_base + c1_base_n * (fan_amount / 2.); + let c2_left = c2_base + c2_base_n * (fan_amount / 2.); + + let c1_right = c1_base - c1_base_n * (fan_amount / 2.); + let c2_right = c2_base - c2_base_n * (fan_amount / 2.); + + // Calculate an offset for the control points based on index `i`, spreading points equidistantly. + let t = (i as f32) / (num_edges - 1) as f32; - // // Compute control points, `c1` and `c2`, based on the offset - // let c1 = Pos2::new(source_pos.x + offset, source_pos.y - offset); - // let c2 = Pos2::new(target_pos.x + offset, target_pos.y + offset); + // Compute control points, `c1` and `c2`, based on the offset + let c1 = c1_right + (c1_left - c1_right) * t; + let c2 = c2_right + (c2_left - c2_right) * t; let geometries = layout .edges @@ -226,13 +231,13 @@ impl ForceLayoutProvider { PathGeometry::CubicBezier { source: source_pos, target: target_pos, - control: [c1_left, c2_left], + control: [c1, c2], } } else { PathGeometry::CubicBezier { source: target_pos, target: source_pos, - control: [c2_left, c1_left], + control: [c2, c1], } }; diff --git a/crates/viewer/re_space_view_graph/src/layout/result.rs b/crates/viewer/re_space_view_graph/src/layout/result.rs index 84b6796ad299..ef4ad184fd2f 100644 --- a/crates/viewer/re_space_view_graph/src/layout/result.rs +++ b/crates/viewer/re_space_view_graph/src/layout/result.rs @@ -104,9 +104,7 @@ impl Layout { /// Returns an iterator over all edges in the layout. pub fn edges(&self) -> impl Iterator { - self.edges - .iter() - .map(|(id, es)| (*id, es.as_slice())) + self.edges.iter().map(|(id, es)| (*id, es.as_slice())) } /// Returns the number of entities in the layout. diff --git a/crates/viewer/re_space_view_graph/src/layout/slots.rs b/crates/viewer/re_space_view_graph/src/layout/slots.rs index 93671488fce0..3995ca08162f 100644 --- a/crates/viewer/re_space_view_graph/src/layout/slots.rs +++ b/crates/viewer/re_space_view_graph/src/layout/slots.rs @@ -22,10 +22,8 @@ impl SlotId { #[derive(Clone, Copy, PartialEq, Eq)] pub enum SlotKind { /// An edge slot going from `source` to `target`. Source and target represent the canonical order of the slot, as specified by [`SlotId`] - Regular { - source: NodeId, - target: NodeId, - }, + Regular { source: NodeId, target: NodeId }, + /// An edge where `source == target`. SelfEdge, } @@ -42,18 +40,16 @@ pub fn slotted_edges<'a>( for e in edges { let id = SlotId::new(e.source, e.target); - let slot = slots - .entry(id) - .or_insert_with_key(|id| Slot { - kind: match e.source == e.target { - true => SlotKind::SelfEdge, - false => SlotKind::Regular { - source: id.0, - target: id.1, - }, + let slot = slots.entry(id).or_insert_with_key(|id| Slot { + kind: match e.source == e.target { + true => SlotKind::SelfEdge, + false => SlotKind::Regular { + source: id.0, + target: id.1, }, - edges: Vec::new(), - }); + }, + edges: Vec::new(), + }); slot.edges.push(e); } diff --git a/crates/viewer/re_space_view_graph/src/ui/draw.rs b/crates/viewer/re_space_view_graph/src/ui/draw.rs index f16bd4b9a6e5..6c51dd576a87 100644 --- a/crates/viewer/re_space_view_graph/src/ui/draw.rs +++ b/crates/viewer/re_space_view_graph/src/ui/draw.rs @@ -213,9 +213,6 @@ pub fn draw_edge(ui: &mut Ui, geometry: &EdgeGeometry, show_arrow: bool) -> Resp fill: Color32::TRANSPARENT, stroke: stroke.into(), }); - - painter.circle(control[0], 1.0, Color32::RED, Stroke::NONE); - painter.circle(control[1], 1.0, Color32::GREEN, Stroke::NONE); } } diff --git a/crates/viewer/re_space_view_graph/src/view.rs b/crates/viewer/re_space_view_graph/src/view.rs index d38f51d0d745..6539892a1447 100644 --- a/crates/viewer/re_space_view_graph/src/view.rs +++ b/crates/viewer/re_space_view_graph/src/view.rs @@ -13,11 +13,10 @@ use re_ui::{ ModifiersMarkdown, MouseButtonMarkdown, UiExt as _, }; use re_viewer_context::{ - IdentifiedViewSystem as _, RecommendedSpaceView, SpaceViewClass, - SpaceViewClassLayoutPriority, SpaceViewClassRegistryError, SpaceViewId, - SpaceViewSpawnHeuristics, SpaceViewState, SpaceViewStateExt as _, - SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, SystemExecutionOutput, ViewQuery, - ViewerContext, + IdentifiedViewSystem as _, RecommendedSpaceView, SpaceViewClass, SpaceViewClassLayoutPriority, + SpaceViewClassRegistryError, SpaceViewId, SpaceViewSpawnHeuristics, SpaceViewState, + SpaceViewStateExt as _, SpaceViewSystemExecutionError, SpaceViewSystemRegistrator, + SystemExecutionOutput, ViewQuery, ViewerContext, }; use re_viewport_blueprint::ViewProperty; From b4a4f7049d18f82811b0a50a4752fcd02cdf8223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Mon, 2 Dec 2024 11:54:30 +0100 Subject: [PATCH 09/12] Cleanup --- .../re_space_view_graph/src/graph/ids.rs | 7 ++ .../re_space_view_graph/src/graph/mod.rs | 30 +++----- .../src/layout/geometry.rs | 76 +++++++++++++++++++ .../re_space_view_graph/src/layout/mod.rs | 6 +- .../src/layout/provider.rs | 74 +++++++++--------- .../re_space_view_graph/src/layout/request.rs | 22 +++--- .../re_space_view_graph/src/layout/result.rs | 74 +----------------- .../re_space_view_graph/src/layout/slots.rs | 17 +++-- 8 files changed, 163 insertions(+), 143 deletions(-) create mode 100644 crates/viewer/re_space_view_graph/src/layout/geometry.rs diff --git a/crates/viewer/re_space_view_graph/src/graph/ids.rs b/crates/viewer/re_space_view_graph/src/graph/ids.rs index 396a2dc3ef8c..22021c02223e 100644 --- a/crates/viewer/re_space_view_graph/src/graph/ids.rs +++ b/crates/viewer/re_space_view_graph/src/graph/ids.rs @@ -43,6 +43,13 @@ pub struct EdgeId { } impl EdgeId { + pub fn self_edge(node: NodeId) -> Self { + Self { + source: node, + target: node, + } + } + pub fn is_self_edge(&self) -> bool { self.source == self.target } diff --git a/crates/viewer/re_space_view_graph/src/graph/mod.rs b/crates/viewer/re_space_view_graph/src/graph/mod.rs index 03a2ab374337..8e6b29b7d47f 100644 --- a/crates/viewer/re_space_view_graph/src/graph/mod.rs +++ b/crates/viewer/re_space_view_graph/src/graph/mod.rs @@ -1,3 +1,9 @@ +//! Provides a basic abstraction over a graph that was logged to an entity. + +// For now this is pretty basic, but in the future we might replace this with +// something like `petgraph`, which will unlock more user interactions, such as +// highlighting of neighboring nodes and edges. + mod hash; use egui::{Pos2, Vec2}; @@ -9,6 +15,7 @@ use re_chunk::EntityPath; use re_types::components::{self, GraphType}; use crate::{ + layout::EdgeTemplate, ui::DrawableLabel, visualizers::{EdgeData, NodeData, NodeInstance}, }; @@ -70,25 +77,10 @@ impl Node { } } -pub struct Edge { - pub source: NodeId, - pub target: NodeId, - pub arrow: bool, -} - -impl Edge { - pub fn id(&self) -> EdgeId { - EdgeId { - source: self.source, - target: self.target, - } - } -} - pub struct Graph { entity: EntityPath, nodes: Vec, - edges: Vec, + edges: Vec, kind: GraphType, } @@ -136,10 +128,10 @@ impl Graph { } } - let es = data.edges.iter().map(|e| Edge { + let es = data.edges.iter().map(|e| EdgeTemplate { source: e.source_index, target: e.target_index, - arrow: data.graph_type == GraphType::Directed, + target_arrow: data.graph_type == GraphType::Directed, }); (es.collect(), data.graph_type) @@ -159,7 +151,7 @@ impl Graph { &self.nodes } - pub fn edges(&self) -> &[Edge] { + pub fn edges(&self) -> &[EdgeTemplate] { &self.edges } diff --git a/crates/viewer/re_space_view_graph/src/layout/geometry.rs b/crates/viewer/re_space_view_graph/src/layout/geometry.rs new file mode 100644 index 000000000000..52498c900049 --- /dev/null +++ b/crates/viewer/re_space_view_graph/src/layout/geometry.rs @@ -0,0 +1,76 @@ +//! Provides geometric (shape) abstractions for the different elements of a graph layout. + +use egui::{Pos2, Rect, Vec2}; + +#[derive(Clone, Debug)] +pub enum PathGeometry { + /// A simple straight edge. + Line { source: Pos2, target: Pos2 }, + + /// Represents a cubic bezier curve. + /// + /// In the future we could probably support more complex splines. + CubicBezier { + source: Pos2, + target: Pos2, + control: [Pos2; 2], + }, + // We could add other geometries, such as `Orthogonal` here too. +} + +#[derive(Debug)] +pub struct EdgeGeometry { + pub target_arrow: bool, + pub path: PathGeometry, +} + +impl EdgeGeometry { + pub fn bounding_rect(&self) -> Rect { + match self.path { + PathGeometry::Line { source, target } => Rect::from_two_pos(source, target), + // TODO(grtlr): This is just a crude (upper) approximation, as the resulting bounding box can be too large. + // For now this is fine, as there are no interactions on edges yet. + PathGeometry::CubicBezier { + source, + target, + ref control, + } => Rect::from_points(&[&[source, target], control.as_slice()].concat()), + } + } + + /// The starting position of an edge. + pub fn source_pos(&self) -> Pos2 { + match self.path { + PathGeometry::Line { source, .. } | PathGeometry::CubicBezier { source, .. } => source, + } + } + + /// The end position of an edge. + pub fn target_pos(&self) -> Pos2 { + match self.path { + PathGeometry::Line { target, .. } | PathGeometry::CubicBezier { target, .. } => target, + } + } + + /// The direction of the edge at the source node (normalized). + pub fn source_arrow_direction(&self) -> Vec2 { + use PathGeometry::{CubicBezier, Line}; + match self.path { + Line { source, target } => (source.to_vec2() - target.to_vec2()).normalized(), + CubicBezier { + source, control, .. + } => (control[0].to_vec2() - source.to_vec2()).normalized(), + } + } + + /// The direction of the edge at the target node (normalized). + pub fn target_arrow_direction(&self) -> Vec2 { + use PathGeometry::{CubicBezier, Line}; + match self.path { + Line { source, target } => (target.to_vec2() - source.to_vec2()).normalized(), + CubicBezier { + target, control, .. + } => (target.to_vec2() - control[1].to_vec2()).normalized(), + } + } +} diff --git a/crates/viewer/re_space_view_graph/src/layout/mod.rs b/crates/viewer/re_space_view_graph/src/layout/mod.rs index fdcd7c5dfbb5..a06d3748be2f 100644 --- a/crates/viewer/re_space_view_graph/src/layout/mod.rs +++ b/crates/viewer/re_space_view_graph/src/layout/mod.rs @@ -1,8 +1,10 @@ +mod geometry; mod provider; mod request; mod result; mod slots; +pub use geometry::{EdgeGeometry, PathGeometry}; pub use provider::ForceLayoutProvider; -pub use request::LayoutRequest; -pub use result::{EdgeGeometry, Layout, PathGeometry}; +pub use request::{EdgeTemplate, LayoutRequest}; +pub use result::Layout; diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index 818e1fc5e2bf..28cfea45d379 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -1,3 +1,10 @@ +//! Performs the layout of the graph, i.e. converting an [`LayoutRequest`] into a [`Layout`]. + +// For now we have only a single layout provider that is based on a force-directed model. +// In the future, this could be expanded to support different (specialized layout alorithms). +// Low-hanging fruit would be tree-based layouts. But we could also think about more complex +// layouts, such as `dot` from `graphviz`. + use egui::{Pos2, Rect, Vec2}; use fjadra::{self as fj}; @@ -5,9 +12,8 @@ use crate::graph::{EdgeId, NodeId}; use super::{ request::NodeTemplate, - result::PathGeometry, slots::{slotted_edges, Slot, SlotKind}, - EdgeGeometry, Layout, LayoutRequest, + EdgeGeometry, EdgeTemplate, Layout, LayoutRequest, PathGeometry, }; impl<'a> From<&'a NodeTemplate> for fj::Node { @@ -60,9 +66,7 @@ impl ForceLayoutProvider { .build(all_nodes) .add_force( "link", - fj::Link::new(considered_edges) - .distance(150.0) - .iterations(2), + fj::Link::new(considered_edges).distance(50.0).iterations(2), ) .add_force("charge", fj::ManyBody::new()) // TODO(grtlr): This is a small stop-gap until we have blueprints to prevent nodes from flying away. @@ -126,34 +130,11 @@ impl ForceLayoutProvider { slotted_edges(graph.edges.values().flat_map(|ts| ts.iter())).values() { match kind { - SlotKind::SelfEdge => { - for (i, edge) in edges.iter().enumerate() { - let offset = (i + 1) as f32; - let target_arrow = edge.target_arrow; - // Self-edges are not supported in force-based layouts. - let anchor = - layout.nodes[&edge.source].intersects_ray_from_center(Vec2::UP); - let geometries = layout - .edges - .entry(EdgeId { - source: edge.source, - target: edge.target, - }) - .or_default(); - geometries.push(EdgeGeometry { - target_arrow, - path: PathGeometry::CubicBezier { - // TODO(grtlr): We could probably consider the actual node size here. - source: anchor + Vec2::LEFT * 4., - target: anchor + Vec2::RIGHT * 4., - // TODO(grtlr): The actual length of that spline should follow the `distance` parameter of the link force. - control: [ - anchor + Vec2::new(-30. * offset, -40. * offset), - anchor + Vec2::new(30. * offset, -40. * offset), - ], - }, - }); - } + SlotKind::SelfEdge { node } => { + let rect = layout.nodes[node]; + let id = EdgeId::self_edge(*node); + let geometries = layout.edges.entry(id).or_default(); + geometries.extend(layout_self_edges(rect, edges)); } SlotKind::Regular { source: slot_source, @@ -226,7 +207,7 @@ impl ForceLayoutProvider { }) .or_default(); - // We potentially need to restore the direction of the edge, after we have used it's cannonical form earlier. + // We potentially need to restore the direction of the edge, after we have used it's canonical form earlier. let path = if edge.source == *slot_source { PathGeometry::CubicBezier { source: source_pos, @@ -273,3 +254,28 @@ fn line_segment(source: Rect, target: Rect) -> PathGeometry { target: target_point, } } + +fn layout_self_edges<'a>( + rect: Rect, + edges: &'a [&EdgeTemplate], +) -> impl Iterator + 'a { + edges.iter().enumerate().map(move |(i, edge)| { + let offset = (i + 1) as f32; + let target_arrow = edge.target_arrow; + let anchor = rect.center_top(); + + EdgeGeometry { + target_arrow, + path: PathGeometry::CubicBezier { + // TODO(grtlr): We could probably consider the actual node size here. + source: anchor + Vec2::LEFT * 4., + target: anchor + Vec2::RIGHT * 4., + // TODO(grtlr): The actual length of that spline should follow the `distance` parameter of the link force. + control: [ + anchor + Vec2::new(-30. * offset, -40. * offset), + anchor + Vec2::new(30. * offset, -40. * offset), + ], + }, + } + }) +} diff --git a/crates/viewer/re_space_view_graph/src/layout/request.rs b/crates/viewer/re_space_view_graph/src/layout/request.rs index 99cf91d1571d..2de4bc9261a6 100644 --- a/crates/viewer/re_space_view_graph/src/layout/request.rs +++ b/crates/viewer/re_space_view_graph/src/layout/request.rs @@ -1,4 +1,4 @@ -//! Contains all the information that is considered when performing a graph layout. +//! Contains all the (geometric) information that is considered when performing a graph layout. //! //! We support: //! * Multiple multiple edges between the same two nodes. @@ -19,12 +19,11 @@ pub(super) struct NodeTemplate { pub(super) fixed_position: Option, } -// TODO(grtlr): Does it make sense to consolidate some of the types, i.e. `Edge` ad `EdgeTemplate`? -#[derive(PartialEq)] -pub(super) struct EdgeTemplate { - pub(super) source: NodeId, - pub(super) target: NodeId, - pub(super) target_arrow: bool, +#[derive(Clone, PartialEq, Eq)] +pub struct EdgeTemplate { + pub source: NodeId, + pub target: NodeId, + pub target_arrow: bool, } #[derive(Default, PartialEq)] @@ -67,12 +66,13 @@ impl LayoutRequest { } for edge in graph.edges() { - let es = entity.edges.entry(edge.id()).or_default(); - es.push(EdgeTemplate { + let id = EdgeId { source: edge.source, target: edge.target, - target_arrow: edge.arrow, - }); + }; + + let es = entity.edges.entry(id).or_default(); + es.push(edge.clone()); } } diff --git a/crates/viewer/re_space_view_graph/src/layout/result.rs b/crates/viewer/re_space_view_graph/src/layout/result.rs index ef4ad184fd2f..31a9219b909c 100644 --- a/crates/viewer/re_space_view_graph/src/layout/result.rs +++ b/crates/viewer/re_space_view_graph/src/layout/result.rs @@ -1,77 +1,11 @@ -use egui::{Pos2, Rect, Vec2}; +//! Defines the output of a layout algorithm, i.e. everything that we need to render the graph. + +use egui::Rect; use re_chunk::EntityPath; use crate::graph::{EdgeId, NodeId}; -#[derive(Clone, Debug)] -pub enum PathGeometry { - /// A simple straight edge. - Line { source: Pos2, target: Pos2 }, - - /// Represents a cubic bezier curve. - /// - /// In the future we could probably support more complex splines. - CubicBezier { - source: Pos2, - target: Pos2, - control: [Pos2; 2], - }, - // We could add other geometries, such as `Orthogonal` here too. -} - -#[derive(Debug)] -pub struct EdgeGeometry { - pub target_arrow: bool, - pub path: PathGeometry, -} - -impl EdgeGeometry { - pub fn bounding_rect(&self) -> Rect { - match self.path { - PathGeometry::Line { source, target } => Rect::from_two_pos(source, target), - // TODO(grtlr): This is just a crude (upper) approximation, as the resulting bounding box can be too large. - PathGeometry::CubicBezier { - source, - target, - ref control, - } => Rect::from_points(&[&[source, target], control.as_slice()].concat()), - } - } - - pub fn source_pos(&self) -> Pos2 { - match self.path { - PathGeometry::Line { source, .. } | PathGeometry::CubicBezier { source, .. } => source, - } - } - - pub fn target_pos(&self) -> Pos2 { - match self.path { - PathGeometry::Line { target, .. } | PathGeometry::CubicBezier { target, .. } => target, - } - } - - /// The direction of the edge at the source node (normalized). - pub fn source_arrow_direction(&self) -> Vec2 { - use PathGeometry::{CubicBezier, Line}; - match self.path { - Line { source, target } => (source.to_vec2() - target.to_vec2()).normalized(), - CubicBezier { - source, control, .. - } => (control[0].to_vec2() - source.to_vec2()).normalized(), - } - } - - /// The direction of the edge at the target node (normalized). - pub fn target_arrow_direction(&self) -> Vec2 { - use PathGeometry::{CubicBezier, Line}; - match self.path { - Line { source, target } => (target.to_vec2() - source.to_vec2()).normalized(), - CubicBezier { - target, control, .. - } => (target.to_vec2() - control[1].to_vec2()).normalized(), - } - } -} +use super::EdgeGeometry; #[derive(Debug)] pub struct Layout { diff --git a/crates/viewer/re_space_view_graph/src/layout/slots.rs b/crates/viewer/re_space_view_graph/src/layout/slots.rs index 3995ca08162f..472e003edb53 100644 --- a/crates/viewer/re_space_view_graph/src/layout/slots.rs +++ b/crates/viewer/re_space_view_graph/src/layout/slots.rs @@ -1,11 +1,14 @@ -//! Force-directed graph layouts assume edges to be straight lines. This -//! function brings edges in a canonical form and finds the ones that occupy the same space. +//! Force-directed graph layouts assume edges to be straight lines. A [`Slot`] +//! represents the space that a single edge, _or multiple_ edges can occupy between two nodes. +//! +//! We achieve this by bringing edges into a canonical form via [`SlotId`], which +//! we can then use to find duplicates. use crate::graph::NodeId; use super::request::EdgeTemplate; -/// Uniquely identifies an [`EdgeSlot`] by ordering the [`NodeIds`](NodeId). +/// Uniquely identifies an [`EdgeSlot`] by ordering the [`NodeIds`](NodeId) that make up an edge. #[derive(Clone, Copy, Hash, PartialEq, Eq)] pub struct SlotId(NodeId, NodeId); @@ -19,13 +22,14 @@ impl SlotId { } } +/// There are different types of [`Slots`](Slot) that are laid out differently. #[derive(Clone, Copy, PartialEq, Eq)] pub enum SlotKind { /// An edge slot going from `source` to `target`. Source and target represent the canonical order of the slot, as specified by [`SlotId`] Regular { source: NodeId, target: NodeId }, /// An edge where `source == target`. - SelfEdge, + SelfEdge { node: NodeId }, } pub struct Slot<'a> { @@ -33,6 +37,7 @@ pub struct Slot<'a> { pub edges: Vec<&'a EdgeTemplate>, } +/// Converts a list of edges into their slotted form. pub fn slotted_edges<'a>( edges: impl Iterator, ) -> ahash::HashMap> { @@ -42,7 +47,7 @@ pub fn slotted_edges<'a>( let id = SlotId::new(e.source, e.target); let slot = slots.entry(id).or_insert_with_key(|id| Slot { kind: match e.source == e.target { - true => SlotKind::SelfEdge, + true => SlotKind::SelfEdge{ node: e.source }, false => SlotKind::Regular { source: id.0, target: id.1, @@ -56,5 +61,3 @@ pub fn slotted_edges<'a>( slots } - -// TODO(grtlr): Write test cases From 0da106a19d699d551458f03ae03e44a1f49d3f42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Mon, 2 Dec 2024 12:19:40 +0100 Subject: [PATCH 10/12] Fmt --- crates/viewer/re_space_view_graph/src/layout/slots.rs | 2 +- tests/python/release_checklist/check_graph_view.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/viewer/re_space_view_graph/src/layout/slots.rs b/crates/viewer/re_space_view_graph/src/layout/slots.rs index 472e003edb53..f27b5a2b3af3 100644 --- a/crates/viewer/re_space_view_graph/src/layout/slots.rs +++ b/crates/viewer/re_space_view_graph/src/layout/slots.rs @@ -47,7 +47,7 @@ pub fn slotted_edges<'a>( let id = SlotId::new(e.source, e.target); let slot = slots.entry(id).or_insert_with_key(|id| Slot { kind: match e.source == e.target { - true => SlotKind::SelfEdge{ node: e.source }, + true => SlotKind::SelfEdge { node: e.source }, false => SlotKind::Regular { source: id.0, target: id.1, diff --git a/tests/python/release_checklist/check_graph_view.py b/tests/python/release_checklist/check_graph_view.py index 91a9ccbe84c7..ad94d94c3850 100644 --- a/tests/python/release_checklist/check_graph_view.py +++ b/tests/python/release_checklist/check_graph_view.py @@ -26,6 +26,7 @@ def log_readme() -> None: rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), static=True) + def log_weird_graph() -> None: rr.log( "weird", @@ -46,6 +47,7 @@ def log_weird_graph() -> None: ), ) + def log_graphs() -> None: DATA = [ ("A", None), From c2d478f8fccd6cdccd716392c2e9b005c94b6b65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Tue, 3 Dec 2024 13:07:22 +0100 Subject: [PATCH 11/12] Address feedback --- .../src/layout/provider.rs | 2 +- .../re_space_view_graph/src/layout/request.rs | 2 +- .../viewer/re_space_view_graph/src/ui/draw.rs | 10 +-- .../release_checklist/check_graph_view.py | 31 -------- .../check_graph_view_multi_self_edges.py | 78 +++++++++++++++++++ 5 files changed, 81 insertions(+), 42 deletions(-) create mode 100644 tests/python/release_checklist/check_graph_view_multi_self_edges.py diff --git a/crates/viewer/re_space_view_graph/src/layout/provider.rs b/crates/viewer/re_space_view_graph/src/layout/provider.rs index 28cfea45d379..e6482ea1be1d 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -107,7 +107,7 @@ impl ForceLayoutProvider { let positions = self.simulation.positions().collect::>(); - // We clear all unnecessary data deom the previous layout, but keep its space allocated. + // We clear all unnecessary data from the previous layout, but keep its space allocated. layout.entities.clear(); layout.edges.clear(); diff --git a/crates/viewer/re_space_view_graph/src/layout/request.rs b/crates/viewer/re_space_view_graph/src/layout/request.rs index 2de4bc9261a6..177d05fbec15 100644 --- a/crates/viewer/re_space_view_graph/src/layout/request.rs +++ b/crates/viewer/re_space_view_graph/src/layout/request.rs @@ -1,7 +1,7 @@ //! Contains all the (geometric) information that is considered when performing a graph layout. //! //! We support: -//! * Multiple multiple edges between the same two nodes. +//! * Multiple edges between the same two nodes. //! * Self-edges //! //!
Duplicated graph nodes are undefined behavior.
diff --git a/crates/viewer/re_space_view_graph/src/ui/draw.rs b/crates/viewer/re_space_view_graph/src/ui/draw.rs index 6c51dd576a87..e8db060fda71 100644 --- a/crates/viewer/re_space_view_graph/src/ui/draw.rs +++ b/crates/viewer/re_space_view_graph/src/ui/draw.rs @@ -18,14 +18,6 @@ use crate::{ visualizers::Label, }; -// Sorry for the pun, could not resist 😎. -// On a serious note, is there no other way to create a `Sense` that does nothing? -const NON_SENSE: Sense = Sense { - click: false, - drag: false, - focusable: false, -}; - pub enum DrawableLabel { Circle(CircleLabel), Text(TextLabel), @@ -228,7 +220,7 @@ pub fn draw_edge(ui: &mut Ui, geometry: &EdgeGeometry, show_arrow: bool) -> Resp // We can add interactions in the future, for now we simply allocate the // rect, so that bounding boxes are computed correctly. - ui.allocate_rect(geometry.bounding_rect(), NON_SENSE) + ui.allocate_rect(geometry.bounding_rect(), Sense::hover()) } pub fn draw_entity_rect( diff --git a/tests/python/release_checklist/check_graph_view.py b/tests/python/release_checklist/check_graph_view.py index ad94d94c3850..97e9964394bd 100644 --- a/tests/python/release_checklist/check_graph_view.py +++ b/tests/python/release_checklist/check_graph_view.py @@ -12,11 +12,6 @@ Please check the following: * All graphs have a proper layout. -* The `Weird Graph` views show: - * two self-edges for `A`, a single one for `B`. - * Additionally, there should be: - * two edges from `A` to `B`. - * one edge from `B` to `A`. * `graph` has directed edges, while `graph2` has undirected edges. * `graph` and `graph2` are shown in two different viewers. * There is a third viewer, `Both`, that shows both `graph` and `graph2` in the same viewer. @@ -27,27 +22,6 @@ def log_readme() -> None: rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), static=True) -def log_weird_graph() -> None: - rr.log( - "weird", - rr.GraphNodes(["A", "B"], labels=["A", "B"]), - rr.GraphEdges( - [ - # self-edges - ("A", "A"), - ("B", "B"), - # duplicated edges - ("A", "B"), - ("A", "B"), - ("B", "A"), - # duplicated self-edges - ("A", "A"), - ], - graph_type=rr.GraphType.Directed, - ), - ) - - def log_graphs() -> None: DATA = [ ("A", None), @@ -77,15 +51,10 @@ def run(args: Namespace) -> None: log_readme() log_graphs() - log_weird_graph() rr.send_blueprint( rrb.Blueprint( rrb.Grid( - rrb.GraphView(origin="weird", name="Weird Graph"), - rrb.GraphView( - origin="weird", name="Weird Graph (without labels)", defaults=[rr.components.ShowLabels(False)] - ), rrb.GraphView(origin="graph", name="Graph 1"), rrb.GraphView(origin="graph2", name="Graph 2"), rrb.GraphView(name="Both", contents=["/graph", "/graph2"]), diff --git a/tests/python/release_checklist/check_graph_view_multi_self_edges.py b/tests/python/release_checklist/check_graph_view_multi_self_edges.py new file mode 100644 index 000000000000..8b43cac93fc0 --- /dev/null +++ b/tests/python/release_checklist/check_graph_view_multi_self_edges.py @@ -0,0 +1,78 @@ +from __future__ import annotations + +import os +from argparse import Namespace +from uuid import uuid4 + +import rerun as rr +import rerun.blueprint as rrb + +README = """\ +# Graph view (multi- and self-edges) + +Please check that both graph views show: +* two self-edges for `A`, a single one for `B`. +* Additionally, there should be: + * two edges from `A` to `B`. + * one edge from `B` to `A`. + * one edge connecting `B` to an implicit node `C`. + * a self-edge for `C`. +""" + + +def log_readme() -> None: + rr.log("readme", rr.TextDocument(README, media_type=rr.MediaType.MARKDOWN), static=True) + + +def log_multi_and_self_graph() -> None: + rr.log( + "graph", + rr.GraphNodes(["A", "B"], labels=["A", "B"]), + rr.GraphEdges( + [ + # self-edges + ("A", "A"), + ("B", "B"), + # duplicated edges + ("A", "B"), + ("A", "B"), + ("B", "A"), + # duplicated self-edges + ("A", "A"), + # implicit edges + ("B", "C"), + ("C", "C"), + ], + graph_type=rr.GraphType.Directed, + ), + ) + + +def run(args: Namespace) -> None: + rr.script_setup(args, f"{os.path.basename(__file__)}", recording_id=uuid4()) + + log_multi_and_self_graph() + log_readme() + + rr.send_blueprint( + rrb.Blueprint( + rrb.Grid( + rrb.GraphView(origin="graph", name="Multiple edges and self-edges"), + rrb.GraphView( + origin="graph", + name="Multiple edges and self-edges (without labels)", + defaults=[rr.components.ShowLabels(False)], + ), + rrb.TextDocumentView(origin="readme", name="Instructions"), + ) + ) + ) + + +if __name__ == "__main__": + import argparse + + parser = argparse.ArgumentParser(description="Interactive release checklist") + rr.script_add_args(parser) + args = parser.parse_args() + run(args) From c89c6f98725ba03da931c51f6ae5f241da251617 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jochen=20G=C3=B6rtler?= Date: Tue, 3 Dec 2024 13:31:24 +0100 Subject: [PATCH 12/12] Fix CI --- crates/viewer/re_space_view_graph/src/layout/slots.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/viewer/re_space_view_graph/src/layout/slots.rs b/crates/viewer/re_space_view_graph/src/layout/slots.rs index f27b5a2b3af3..02dc4f05b2a4 100644 --- a/crates/viewer/re_space_view_graph/src/layout/slots.rs +++ b/crates/viewer/re_space_view_graph/src/layout/slots.rs @@ -8,7 +8,7 @@ use crate::graph::NodeId; use super::request::EdgeTemplate; -/// Uniquely identifies an [`EdgeSlot`] by ordering the [`NodeIds`](NodeId) that make up an edge. +/// Uniquely identifies a [`Slot`] by ordering the [`NodeIds`](NodeId) that make up an edge. #[derive(Clone, Copy, Hash, PartialEq, Eq)] pub struct SlotId(NodeId, NodeId);