Skip to content

Commit

Permalink
Simplify graph viewer rendering logic (rerun-io#8227)
Browse files Browse the repository at this point in the history
### Related

* This is one of the big action items of rerun-io#7897

<!--
Include links to any related issues/PRs in a bulleted list, for example:
* Closes rerun-io#1234
* Part of rerun-io#1337
-->

### What

<!--
Make sure the PR title and labels are set to maximize their usefulness
for the CHANGELOG,
and our `git log`.

If you have noticed any breaking changes, include them in the migration
guide.

We track various metrics at <https://build.rerun.io>.

For maintainers:
* To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
* To deploy documentation changes immediately after merging this PR, add
the `deploy docs` label.
-->

It changes two things:

1. Instead of using multiple `egui::Areas`, we now only have a single
top-level area.
2. We compute the sizes of the nodes (information that is important to
the layout) in a single preprocessing path instead of the weird
frame-to-frame computation that we did before.

Overall this will improve code quality, performance, and will unlock new
GraphViz-like layout algorithms (which require defined label size from
the beginning) down the road.

---------

Co-authored-by: Antoine Beyeler <[email protected]>
  • Loading branch information
grtlr and abey79 authored Nov 29, 2024
1 parent 9de74d7 commit 0e6c186
Show file tree
Hide file tree
Showing 23 changed files with 966 additions and 865 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6231,11 +6231,11 @@ name = "re_space_view_graph"
version = "0.21.0-alpha.1+dev"
dependencies = [
"ahash",
"bytemuck",
"egui",
"fjadra",
"nohash-hasher",
"re_chunk",
"re_entity_db",
"re_format",
"re_log_types",
"re_query",
Expand Down
2 changes: 1 addition & 1 deletion crates/viewer/re_space_view_graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ all-features = true

[dependencies]
re_chunk.workspace = true
re_entity_db.workspace = true
re_format.workspace = true
re_log_types.workspace = true
re_query.workspace = true
Expand All @@ -32,7 +33,6 @@ re_viewer_context.workspace = true
re_viewport_blueprint.workspace = true

ahash.workspace = true
bytemuck.workspace = true
egui.workspace = true
fjadra.workspace = true
nohash-hasher.workspace = true
15 changes: 7 additions & 8 deletions crates/viewer/re_space_view_graph/src/graph/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,24 @@ use re_types::components;

use super::GraphNodeHash;

#[derive(Clone, Copy, PartialEq, Eq)]
pub struct NodeIndex {
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct NodeId {
pub entity_hash: EntityPathHash,
pub node_hash: GraphNodeHash,
}

impl nohash_hasher::IsEnabled for NodeIndex {}
impl nohash_hasher::IsEnabled for NodeId {}

// We implement `Hash` manually, because `nohash_hasher` requires a single call to `state.write_*`.
// More info: https://crates.io/crates/nohash-hasher
impl std::hash::Hash for NodeIndex {
impl std::hash::Hash for NodeId {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
// TODO(grtlr): Consider using `write_usize` here, to further decrease the risk of collision.
let combined = self.entity_hash.hash64() << 32 | self.node_hash.hash64();
let combined = self.entity_hash.hash64() ^ self.node_hash.hash64();
state.write_u64(combined);
}
}

impl NodeIndex {
impl NodeId {
pub fn from_entity_node(entity_path: &EntityPath, node: &components::GraphNode) -> Self {
Self {
entity_hash: entity_path.hash(),
Expand All @@ -30,7 +29,7 @@ impl NodeIndex {
}
}

impl std::fmt::Debug for NodeIndex {
impl std::fmt::Debug for NodeId {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "NodeIndex({:?}@{:?})", self.node_hash, self.entity_hash)
}
Expand Down
159 changes: 118 additions & 41 deletions crates/viewer/re_space_view_graph/src/graph/mod.rs
Original file line number Diff line number Diff line change
@@ -1,87 +1,164 @@
mod hash;

use egui::{Pos2, Vec2};
pub(crate) use hash::GraphNodeHash;
mod index;
pub(crate) use index::NodeIndex;
pub(crate) use index::NodeId;

use re_chunk::EntityPath;
use re_types::components::{self, GraphType};

use re_types::components::{GraphNode, GraphType};
use crate::{
ui::DrawableLabel,
visualizers::{EdgeData, NodeData, NodeInstance},
};

use crate::visualizers::{EdgeData, EdgeInstance, NodeData, NodeInstance};
/// Describes the different kind of nodes that we can have in a graph.
pub enum Node {
/// An explicit node is a node that was provided via [`re_types::archetypes::GraphNodes`].
///
/// It therefore has an instance, as well as all properties that can be added via that archetype.
Explicit {
instance: NodeInstance,
label: DrawableLabel,
},

pub struct NodeInstanceImplicit {
pub node: GraphNode,
pub index: NodeIndex,
/// An implicit node is a node that was provided via [`re_types::archetypes::GraphEdges`], but does not have a corresponding [`re_types::components::GraphNode`] in an [`re_types::archetypes::GraphNodes`] archetype.
///
/// Because it was never specified directly, it also does not have many of the properties that an [`Node::Explicit`] has.
Implicit {
id: NodeId,
graph_node: components::GraphNode,
label: DrawableLabel,
},
}

impl std::hash::Hash for NodeInstanceImplicit {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.index.hash(state);
impl Node {
pub fn id(&self) -> NodeId {
match self {
Self::Explicit { instance, .. } => instance.id,
Self::Implicit { id, .. } => *id,
}
}

/// The original [`components::GraphNode`] id that was logged by the user.
pub fn graph_node(&self) -> &components::GraphNode {
match self {
Self::Explicit { instance, .. } => &instance.graph_node,
Self::Implicit { graph_node, .. } => graph_node,
}
}

pub fn label(&self) -> &DrawableLabel {
match self {
Self::Explicit { label, .. } | Self::Implicit { label, .. } => label,
}
}

pub fn size(&self) -> Vec2 {
self.label().size()
}

pub fn position(&self) -> Option<Pos2> {
match self {
Self::Explicit {
instance: NodeInstance { position, .. },
..
} => *position,
Self::Implicit { .. } => None,
}
}
}

#[derive(Hash)]
pub struct Graph<'a> {
explicit: &'a [NodeInstance],
implicit: Vec<NodeInstanceImplicit>,
edges: &'a [EdgeInstance],
pub struct Edge {
pub from: NodeId,
pub to: NodeId,
pub arrow: bool,
}

pub struct Graph {
entity: EntityPath,
nodes: Vec<Node>,
edges: Vec<Edge>,
kind: GraphType,
}

impl<'a> Graph<'a> {
pub fn new(node_data: Option<&'a NodeData>, edge_data: Option<&'a EdgeData>) -> Self {
impl Graph {
pub fn new<'a>(
ui: &egui::Ui,
entity: EntityPath,
node_data: Option<&'a NodeData>,
edge_data: Option<&'a EdgeData>,
) -> Self {
// We keep track of the nodes to find implicit nodes.
let mut seen = ahash::HashSet::default();

let explicit = if let Some(data) = node_data {
seen.extend(data.nodes.iter().map(|n| n.index));
data.nodes.as_slice()
let mut nodes: Vec<Node> = if let Some(data) = node_data {
seen.extend(data.nodes.iter().map(|n| n.id));
// TODO(grtlr): We should see if we can get rid of some of the cloning here.
data.nodes
.iter()
.map(|n| Node::Explicit {
instance: n.clone(),
label: DrawableLabel::from_label(ui, &n.label),
})
.collect()
} else {
&[][..]
Vec::new()
};

let (edges, implicit, kind) = if let Some(data) = edge_data {
let mut implicit = Vec::new();
let (edges, kind) = if let Some(data) = edge_data {
for edge in &data.edges {
if !seen.contains(&edge.source_index) {
implicit.push(NodeInstanceImplicit {
node: edge.source.clone(),
index: edge.source_index,
nodes.push(Node::Implicit {
id: edge.source_index,
graph_node: edge.source.clone(),
label: DrawableLabel::implicit_circle(),
});
seen.insert(edge.source_index);
}
if !seen.contains(&edge.target_index) {
implicit.push(NodeInstanceImplicit {
node: edge.target.clone(),
index: edge.target_index,
nodes.push(Node::Implicit {
id: edge.target_index,
graph_node: edge.target.clone(),
label: DrawableLabel::implicit_circle(),
});
seen.insert(edge.target_index);
}
}
(data.edges.as_slice(), implicit, Some(data.graph_type))

let es = data.edges.iter().map(|e| Edge {
from: e.source_index,
to: e.target_index,
arrow: data.graph_type == GraphType::Directed,
});

(es.collect(), data.graph_type)
} else {
(&[][..], Vec::new(), None)
(Vec::new(), GraphType::default())
};

Self {
explicit,
implicit,
entity,
nodes,
edges,
kind: kind.unwrap_or_default(),
kind,
}
}

pub fn nodes_explicit(&self) -> impl Iterator<Item = &NodeInstance> {
self.explicit.iter()
}

pub fn nodes_implicit(&self) -> impl Iterator<Item = &NodeInstanceImplicit> + '_ {
self.implicit.iter()
pub fn nodes(&self) -> &[Node] {
&self.nodes
}

pub fn edges(&self) -> impl Iterator<Item = &EdgeInstance> {
self.edges.iter()
pub fn edges(&self) -> &[Edge] {
&self.edges
}

pub fn kind(&self) -> GraphType {
self.kind
}

pub fn entity(&self) -> &EntityPath {
&self.entity
}
}
Loading

0 comments on commit 0e6c186

Please sign in to comment.