fix: Better mermaid rendering

Defines intergraph edges on the parent region,
so mermaid renders them correctly .

src/algorithms.rs

@@ -2,10 +2,14 @@
 
 mod convex;
 mod dominators;
+mod lca;
 mod post_order;
 mod toposort;
 
 pub use convex::{ConvexChecker, TopoConvexChecker};
 pub use dominators::{dominators, dominators_filtered, DominatorTree};
 pub use post_order::{postorder, postorder_filtered, PostOrder};
 pub use toposort::{toposort, toposort_filtered, TopoSort};
+
+// We will make it public once an efficient implementation is available.
+pub(crate) use lca::{lca, LCA};

src/algorithms/lca.rs

@@ -0,0 +1,43 @@
+//! Lowest common ancestor algorithm on hierarchy forests.
+
+use std::collections::HashSet;
+
+use crate::{Hierarchy, NodeIndex};
+
+/// Constructs a new lowest common ancestor data structure.
+pub fn lca(hierarchy: &Hierarchy) -> LCA {
+    LCA {
+        hierarchy: hierarchy.clone(),
+    }
+}
+
+/// A precomputed data structure for lowest common ancestor queries between
+/// nodes in a hierarchy.
+#[derive(Debug, Clone)]
+pub struct LCA {
+    hierarchy: Hierarchy,
+}
+
+impl LCA {
+    /// Returns the lowest common ancestor of two nodes.
+    pub fn lca(&self, a: NodeIndex, b: NodeIndex) -> Option<NodeIndex> {
+        // Placeholder slow implementation.
+        let mut parents = HashSet::new();
+
+        let mut a = Some(a);
+        while let Some(node) = a {
+            parents.insert(node);
+            a = self.hierarchy.parent(node);
+        }
+
+        let mut b = Some(b);
+        while let Some(node) = b {
+            if parents.contains(&node) {
+                return Some(node);
+            }
+            b = self.hierarchy.parent(node);
+        }
+
+        None
+    }
+}

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,75 @@ 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.
+        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(lca);
+
+        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 +175,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"]