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 71% 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..22021c02223e 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,23 @@ 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 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 947216daf86c..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,14 +1,21 @@ +//! 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}; 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}; use crate::{ + layout::EdgeTemplate, ui::DrawableLabel, visualizers::{EdgeData, NodeData, NodeInstance}, }; @@ -70,16 +77,10 @@ impl Node { } } -pub struct Edge { - pub from: NodeId, - pub to: NodeId, - pub arrow: bool, -} - pub struct Graph { entity: EntityPath, nodes: Vec, - edges: Vec, + edges: Vec, kind: GraphType, } @@ -127,10 +128,10 @@ impl Graph { } } - let es = data.edges.iter().map(|e| Edge { - from: e.source_index, - to: e.target_index, - arrow: data.graph_type == GraphType::Directed, + let es = data.edges.iter().map(|e| EdgeTemplate { + source: e.source_index, + target: e.target_index, + target_arrow: data.graph_type == GraphType::Directed, }); (es.collect(), data.graph_type) @@ -150,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 8c280d81c782..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,7 +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 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 d3f845f45457..e6482ea1be1d 100644 --- a/crates/viewer/re_space_view_graph/src/layout/provider.rs +++ b/crates/viewer/re_space_view_graph/src/layout/provider.rs @@ -1,9 +1,20 @@ +//! 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 as fj; +use fjadra::{self as fj}; -use crate::graph::NodeId; +use crate::graph::{EdgeId, NodeId}; -use super::{request::NodeTemplate, Layout, LayoutRequest}; +use super::{ + request::NodeTemplate, + slots::{slotted_edges, Slot, SlotKind}, + EdgeGeometry, EdgeTemplate, Layout, LayoutRequest, PathGeometry, +}; impl<'a> From<&'a NodeTemplate> for fj::Node { fn from(node: &'a NodeTemplate) -> Self { @@ -17,11 +28,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 +54,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(|(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() @@ -53,7 +66,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 +76,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]; @@ -84,6 +97,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(), } } @@ -93,18 +107,130 @@ 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 from the previous layout, but keep its space allocated. + layout.entities.clear(); + layout.edges.clear(); - for (from, to) in &self.edges { - layout.edges.insert( - (*from, *to), - line_segment(layout.nodes[from], layout.nodes[to]), - ); + 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); + } + + layout.entities.push((entity.clone(), current_rect)); + + // 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 { 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, + target: slot_target, + } => { + 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[&edge.source], + layout.nodes[&edge.target], + ), + }); + } 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() { + 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 = 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(); + + // 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 * (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 = c1_right + (c1_left - c1_right) * t; + let c2 = c2_right + (c2_left - c2_right) * t; + + let geometries = layout + .edges + .entry(EdgeId { + source: edge.source, + target: edge.target, + }) + .or_default(); + + // 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, + target: target_pos, + control: [c1, c2], + } + } else { + PathGeometry::CubicBezier { + source: target_pos, + target: source_pos, + control: [c2, c1], + } + }; + + geometries.push(EdgeGeometry { + target_arrow: edge.target_arrow, + path, + }); + } + } + } + } + } } self.simulation.finished() @@ -112,7 +238,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) -> PathGeometry { let source_center = source.center(); let target_center = target.center(); @@ -120,89 +246,36 @@ fn line_segment(source: Rect, target: Rect) -> [Pos2; 2] { 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 - [source_point, 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); + PathGeometry::Line { + source: source_point, + target: target_point, } - - 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" - ); - } +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 be17a02e4395..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,9 +1,17 @@ -use std::collections::{BTreeMap, BTreeSet}; +//! Contains all the (geometric) information that is considered when performing a graph layout. +//! +//! We support: +//! * 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; -use crate::graph::{Graph, NodeId}; +use crate::graph::{EdgeId, Graph, NodeId}; #[derive(PartialEq)] pub(super) struct NodeTemplate { @@ -11,10 +19,21 @@ pub(super) struct NodeTemplate { pub(super) fixed_position: Option, } +#[derive(Clone, PartialEq, Eq)] +pub struct EdgeTemplate { + pub source: NodeId, + pub target: NodeId, + pub target_arrow: bool, +} + #[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>, } /// A [`LayoutRequest`] encapsulates all the information that is considered when computing a layout. @@ -39,11 +58,21 @@ 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 id = EdgeId { + source: edge.source, + target: edge.target, + }; + + 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 78734514ee13..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,14 +1,17 @@ -use egui::{Pos2, Rect}; +//! Defines the output of a layout algorithm, i.e. everything that we need to render the graph. -use crate::graph::NodeId; +use egui::Rect; +use re_chunk::EntityPath; -pub type LineSegment = [Pos2; 2]; +use crate::graph::{EdgeId, NodeId}; -#[derive(Debug, PartialEq, Eq)] +use super::EdgeGeometry; + +#[derive(Debug)] pub struct Layout { pub(super) nodes: ahash::HashMap, - pub(super) edges: ahash::HashMap<(NodeId, NodeId), LineSegment>, - // TODO(grtlr): Consider adding the entity rects here too. + pub(super) edges: ahash::HashMap>, + pub(super) entities: Vec<(EntityPath, Rect)>, } fn bounding_rect_from_iter(rectangles: impl Iterator) -> egui::Rect { @@ -29,7 +32,22 @@ 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, 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 { + self.edges.iter().map(|(id, es)| (*id, 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/layout/slots.rs b/crates/viewer/re_space_view_graph/src/layout/slots.rs new file mode 100644 index 000000000000..02dc4f05b2a4 --- /dev/null +++ b/crates/viewer/re_space_view_graph/src/layout/slots.rs @@ -0,0 +1,63 @@ +//! 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 a [`Slot`] by ordering the [`NodeIds`](NodeId) that make up an edge. +#[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) + } + } +} + +/// 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 { node: NodeId }, +} + +pub struct Slot<'a> { + pub kind: SlotKind, + pub edges: Vec<&'a EdgeTemplate>, +} + +/// Converts a list of edges into their slotted form. +pub fn slotted_edges<'a>( + edges: impl Iterator, +) -> ahash::HashMap> { + let mut slots: ahash::HashMap> = ahash::HashMap::default(); + + 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 { node: e.source }, + false => SlotKind::Regular { + source: id.0, + target: id.1, + }, + }, + edges: Vec::new(), + }); + + slot.edges.push(e); + } + + slots +} 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..e8db060fda71 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,18 +14,10 @@ use re_viewer_context::{ use crate::{ graph::{Graph, Node}, - layout::Layout, + layout::{EdgeGeometry, Layout, PathGeometry}, 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), @@ -132,7 +124,7 @@ fn draw_text_label(ui: &mut Ui, label: &TextLabel, highlight: InteractionHighlig .sense(Sense::click()), ) }) - .inner + .response } /// Draws a node at the given position. @@ -192,24 +184,43 @@ 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 { +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); - let rect = Rect::from_points(&points); let painter = ui.painter(); - painter.line_segment(points, Stroke::new(1.0, fg)); - // Calculate direction vector from source to target - let direction = (points[1] - points[0]).normalized(); + match geometry.path { + PathGeometry::Line { source, target } => { + painter.line_segment([source, target], stroke); + } + PathGeometry::CubicBezier { + source, + target, + control, + } => { + 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, points[1], direction, fg); + 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 // rect, so that bounding boxes are computed correctly. - ui.allocate_rect(rect, NON_SENSE) + ui.allocate_rect(geometry.bounding_rect(), Sense::hover()) } pub fn draw_entity_rect( @@ -251,7 +262,6 @@ pub fn draw_entity_rect( } /// Draws the graph using the layout. -#[must_use] pub fn draw_graph( ui: &mut Ui, ctx: &ViewerContext<'_>, @@ -300,12 +310,25 @@ pub fn draw_graph( current_rect = current_rect.union(response.rect); } - for edge in graph.edges() { - let points = layout - .get_edge(edge.from, edge.to) - .unwrap_or([Pos2::ZERO, Pos2::ZERO]); - let resp = draw_edge(ui, points, 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); + current_rect = current_rect.union(response.rect); + } + } + + // 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/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; 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..dad235b3586a 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/crates/viewer/re_space_view_graph/src/view.rs b/crates/viewer/re_space_view_graph/src/view.rs index cef909294e39..6539892a1447 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,18 +13,17 @@ use re_ui::{ ModifiersMarkdown, MouseButtonMarkdown, UiExt as _, }; use re_viewer_context::{ - IdentifiedViewSystem as _, Item, 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; 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 +176,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 dc0b5edeeaf7..97e9964394bd 100644 --- a/tests/python/release_checklist/check_graph_view.py +++ b/tests/python/release_checklist/check_graph_view.py @@ -45,6 +45,13 @@ 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() + rr.send_blueprint( rrb.Blueprint( rrb.Grid( @@ -57,13 +64,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 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)