fix: Intergraph edges in mermaid rendering

benches/bench_main.rs

@@ -6,6 +6,7 @@ use criterion::criterion_main;
 criterion_main! {
     benchmarks::hierarchy::benches,
     benchmarks::portgraph::benches,
+    benchmarks::render::benches,
     benchmarks::toposort::benches,
     benchmarks::convex::benches,
 }

benches/benchmarks/mod.rs

@@ -3,4 +3,5 @@ pub mod generators;
 pub mod convex;
 pub mod hierarchy;
 pub mod portgraph;
+pub mod render;
 pub mod toposort;

benches/benchmarks/render.rs

@@ -0,0 +1,60 @@
+//! Benchmarks for the graph renderers.
+
+use criterion::{black_box, criterion_group, AxisScale, BenchmarkId, Criterion, PlotConfiguration};
+use portgraph::render::{DotFormat, MermaidFormat};
+
+use super::generators::{make_hierarchy, make_two_track_dag, make_weights};
+
+fn bench_render_mermaid(c: &mut Criterion) {
+    let mut g = c.benchmark_group("Mermaid rendering. Graph with hierarchy.");
+    g.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
+
+    for size in [100, 1_000, 10_000] {
+        let graph = make_two_track_dag(size);
+        let hierarchy = make_hierarchy(&graph);
+        let weights = make_weights(&graph);
+        g.bench_with_input(BenchmarkId::new("hierarchy", size), &size, |b, _size| {
+            b.iter(|| {
+                black_box(
+                    graph
+                        .mermaid_format()
+                        .with_hierarchy(&hierarchy)
+                        .with_weights(&weights)
+                        .finish(),
+                )
+            })
+        });
+    }
+    g.finish();
+}
+
+fn bench_render_dot(c: &mut Criterion) {
+    let mut g = c.benchmark_group("Dot rendering. Graph with tree hierarchy.");
+    g.plot_config(PlotConfiguration::default().summary_scale(AxisScale::Logarithmic));
+
+    for size in [100, 1_000, 10_000] {
+        let graph = make_two_track_dag(size);
+        let hierarchy = make_hierarchy(&graph);
+        let weights = make_weights(&graph);
+        g.bench_with_input(BenchmarkId::new("hierarchy", size), &size, |b, _size| {
+            b.iter(|| {
+                black_box(
+                    graph
+                        .dot_format()
+                        .with_hierarchy(&hierarchy)
+                        .with_weights(&weights)
+                        .finish(),
+                )
+            })
+        });
+    }
+    g.finish();
+}
+
+criterion_group! {
+    name = benches;
+    config = Criterion::default();
+    targets =
+        bench_render_mermaid,
+        bench_render_dot,
+}

src/render/mermaid.rs

@@ -1,7 +1,9 @@
 //! Functions to encode a `PortGraph` in mermaid format.
 
+use std::collections::HashMap;
 use std::fmt::Display;
 
+use crate::algorithms::{lca, LCA};
 use crate::{Hierarchy, LinkView, NodeIndex, Weights};
 
 use super::{EdgeStyle, NodeStyle};
@@ -92,31 +94,76 @@ where
         let mut mermaid = MermaidBuilder::init(self.node_style.take(), self.edge_style.take());
 
         // Explore the hierarchy from the root nodes, and add the nodes and edges to the mermaid definition.
-        for root in self.graph.nodes_iter().filter(|n| self.is_root(*n)) {
-            self.explore_node(&mut mermaid, root);
-        }
+        self.explore_forest(&mut mermaid);
 
         mermaid.finish()
     }
 
-    /// Encode the nodes, starting from a set of roots.
-    fn explore_node(&self, mmd: &mut MermaidBuilder<'g, G>, node: NodeIndex) {
-        if self.is_leaf(node) {
-            mmd.add_leaf(node);
-        } else {
-            mmd.start_subgraph(node);
-            for child in self
-                .forest
-                .map_or_else(Vec::new, |f| f.children(node).collect())
-            {
-                self.explore_node(mmd, child);
+    /// Visit each tree of nodes and encode them in the mermaid format.
+    fn explore_forest(&self, mmd: &mut MermaidBuilder<'g, G>) {
+        // A stack of exploration steps to take.
+        let mut exploration_stack: Vec<ExplorationStep> = Vec::new();
+
+        // Add all the root nodes in the hierarchy to the stack.
+        for root in self.graph.nodes_iter().filter(|n| self.is_root(*n)) {
+            exploration_stack.push(ExplorationStep::ExploreNode { node: root });
+        }
+
+        // Delay emitting edges until we are in a region that is the parent of both the source and target nodes.
+        #[allow(clippy::type_complexity)]
+        let mut edges: HashMap<
+            Option<NodeIndex>,
+            Vec<(NodeIndex, G::LinkEndpoint, NodeIndex, G::LinkEndpoint)>,
+        > = HashMap::new();
+
+        // An efficient structure for retrieving the lowest common ancestor of two nodes.
+        let lca: Option<LCA> = self.forest.map(|f| lca(self.graph, f));
+
+        while let Some(instr) = exploration_stack.pop() {
+            match instr {
+                ExplorationStep::ExploreNode { node } => {
+                    if self.is_leaf(node) {
+                        mmd.add_leaf(node);
+                    } else {
+                        mmd.start_subgraph(node);
+
+                        // Add the descendants and an exit instruction to the
+                        // stack, in reverse order.
+                        exploration_stack.push(ExplorationStep::ExitSubgraph { subgraph: node });
+                        for child in self.node_children(node).rev() {
+                            exploration_stack.push(ExplorationStep::ExploreNode { node: child });
+                        }
+                    }
+
+                    // Add the edges originating from this node to the edge list.
+                    // They will be added once we reach a region that is the parent of both nodes.
+                    for (src, tgt) in self.graph.output_links(node) {
+                        let src_node = self.graph.port_node(src).unwrap();
+                        let tgt_node = self.graph.port_node(tgt).unwrap();
+                        let lca = lca.as_ref().and_then(|l| l.lca(src_node, tgt_node));
+                        edges
+                            .entry(lca)
+                            .or_insert_with(Vec::new)
+                            .push((src_node, src, tgt_node, tgt));
+                    }
+                }
+                ExplorationStep::ExitSubgraph { subgraph } => {
+                    if let Some(es) = edges.remove(&Some(subgraph)) {
+                        for (src_node, src, tgt_node, tgt) in es {
+                            mmd.add_link(src_node, src, tgt_node, tgt);
+                        }
+                    }
+
+                    mmd.end_subgraph();
+                }
             }
-            mmd.end_subgraph();
         }
-        for (src, tgt) in self.graph.output_links(node) {
-            let src_node = self.graph.port_node(src).unwrap();
-            let tgt_node = self.graph.port_node(tgt).unwrap();
-            mmd.add_link(src_node, src, tgt_node, tgt);
+
+        // Add any remaining edges that were not added to the mermaid definition.
+        for (_, es) in edges {
+            for (src_node, src, tgt_node, tgt) in es {
+                mmd.add_link(src_node, src, tgt_node, tgt);
+            }
         }
     }
 
@@ -129,6 +176,19 @@ where
     fn is_leaf(&self, node: NodeIndex) -> bool {
         self.forest.map_or(true, |f| !f.has_children(node))
     }
+
+    /// Returns the children of a node in the hierarchy.
+    fn node_children(&self, node: NodeIndex) -> impl DoubleEndedIterator<Item = NodeIndex> + '_ {
+        self.forest.iter().flat_map(move |f| f.children(node))
+    }
+}
+
+/// A set of instructions to queue while exploring hierarchical graphs in [`MermaidFormatter::explore_tree`].
+enum ExplorationStep {
+    /// Explore a new node and its children.
+    ExploreNode { node: NodeIndex },
+    /// Finish the current subgraph, and continue with the parent node.
+    ExitSubgraph { subgraph: NodeIndex },
 }
 
 /// A trait for encoding a graph in mermaid format.

src/snapshots/portgraph__render__test__flat__mermaid.snap

@@ -3,8 +3,8 @@ source: src/render.rs
 expression: mermaid
 ---
 graph LR
+    2[2]
+    1[1]
     0[0]
     0-->1
     0-->2
-    1[1]
-    2[2]

src/snapshots/portgraph__render__test__hierarchy__mermaid.snap

@@ -6,6 +6,6 @@ graph LR
     subgraph 0 [0]
         direction LR
         1[1]
-        1-->2
         2[2]
+        1-->2
     end

src/snapshots/portgraph__render__test__hierarchy_interregional__mermaid.snap

@@ -8,11 +8,11 @@ graph LR
         subgraph 1 [1]
             direction LR
             3[3]
-            3-->4
         end
-        1-->2
         subgraph 2 [2]
             direction LR
             4[4]
         end
+        1-->2
+        3-->4
     end

src/snapshots/portgraph__render__test__weighted__mermaid.snap

@@ -3,8 +3,8 @@ source: src/render.rs
 expression: mermaid
 ---
 graph LR
+    2["node3"]
+    1["node2"]
     0["node1"]
     0-->1
     0-->2
-    1["node2"]
-    2["node3"]