Move post-rank edge reorientation to the rank function

lib/layout.js

@@ -133,11 +133,6 @@ module.exports = function() {
       // above nodes of higher rank.
       util.time(rank)(g);
 
-      // Due to rank constraints it is possible that we have edges going
-      // against the flow of the graph. We need to reorient them for successive
-      // stages.
-      util.time(orientEdges)(g);
-
       // Normalize the graph by ensuring that every edge is proper (each edge has
       // a length of 1). We achieve this by adding dummy nodes to long edges,
       // thus shortening them.
@@ -176,16 +171,6 @@ module.exports = function() {
     });
   }
 
-  function orientEdges(g) {
-    g.eachEdge(function(e, u, v, value) {
-      if (g.node(u).rank > g.node(v).rank) {
-        g.delEdge(e);
-        value.reversed = true;
-        g.addEdge(e, v, u, value);
-      }
-    });
-  }
-
   /*
    * This function is responsible for "normalizing" the graph. The process of
    * normalization ensures that no edge in the graph has spans more than one

lib/rank.js

@@ -9,8 +9,6 @@ var util = require("./util"),
 module.exports = rank;
 
 function rank(g) {
-  g = g.filterNodes(util.filterNonSubgraphs(g));
-
   var reduced = combineRanks(g);
 
   initRank(reduced);
@@ -22,13 +20,17 @@ function rank(g) {
   });
 
   expandRanks(reduced, g);
+
+  orientEdges(g);
 }
 
 /*
  * If there are rank constraints on nodes, then build a condensed graph,
  * with one node per rank set.  Modify edges to that the minimum rank
  * will be ranked before any others and the maximum rank will be ranked
  * after any others.
+ *
+ * This function excludes subgraph nodes from the output graph.
  */
 function combineRanks(g) {
   var needsReduction = false,
@@ -40,7 +42,9 @@ function combineRanks(g) {
     }
   }
 
-  if (!needsReduction) { return g; }
+  if (!needsReduction) {
+    return g.filterNodes(util.filterNonSubgraphs(g));
+  }
 
   g = g.copy();
   g.graph({ compoundNodes: [] });
@@ -50,6 +54,11 @@ function combineRanks(g) {
     var rank = value.prefRank,
         newU;
 
+    if (g.children(u).length) {
+      g.delNode(u);
+      return;
+    }
+
     if (rank !== undefined) {
       newU = prefRankToNode[rank];
       if (newU === undefined) {
@@ -196,6 +205,23 @@ function feasibleTree(g) {
   }
 }
 
+/*
+ * When handling nodes with constrained ranks it is possible to end up with
+ * edges that point to previous ranks. Most of the subsequent algorithms assume
+ * that edges are pointing to successive ranks only. Here we reverse any "back
+ * edges" and mark them as such. The acyclic algorithm will reverse them as a
+ * post processing step.
+ */
+function orientEdges(g) {
+  g.eachEdge(function(e, u, v, value) {
+    if (g.node(u).rank > g.node(v).rank) {
+      g.delEdge(e);
+      value.reversed = true;
+      g.addEdge(e, v, u, value);
+    }
+  });
+}
+
 function normalize(g) {
   var m = util.min(g.nodes().map(function(u) { return g.node(u).rank; }));
   g.eachNode(function(u, node) { node.rank -= m; });

test/rank-test.js

@@ -129,5 +129,17 @@ describe("layout/rank", function() {
 
     assert.equal(g.node("B").rank, g.node("C").rank);
   });
+
+  it("returns a graph with edges all points to the same or successive ranks", function() {
+    // This should put B above A and without any other action would leave the
+    // out edge from B point to an earlier rank.
+    var g = dot.parse("digraph { A -> B; B [prefRank=min]; }");
+
+    rank(g);
+
+    assert.isTrue(g.node("B").rank < g.node("A").rank);
+    assert.sameMembers(g.successors("B"), ["A"]);
+    assert.sameMembers(g.successors("A"), []);
+  });
 });