Refactor LayoutEngine for further work (#3227)

* Added commenting for better understanding

* Refactored specific heirarchy fields to HierarchyStatus; more commenting.

* Small edits to LayoutEngine

lib/network/modules/LayoutEngine.js

@@ -1,5 +1,35 @@
 'use strict';
-
+/**
+ * There's a mix-up with terms in the code. Following are the formal definitions:
+ *
+ *   tree   - a strict hierarchical network, i.e. every node has at most one parent
+ *   forest - a collection of trees. These distinct trees are thus not connected.
+ *
+ * So:
+ * - in a network that is not a tree, there exist nodes with multiple parents.
+ * - a network consisting of unconnected sub-networks, of which at least one
+ *   is not a tree, is not a forest.
+ *
+ * In the code, the definitions are:
+ *
+ *   tree   - any disconnected sub-network, strict hierarchical or not.
+ *   forest - a bunch of these sub-networks
+ *
+ * The difference between tree and not-tree is important in the code, notably within
+ * to the block-shifting algorithm. The algorithm assumes formal trees and fails
+ * for not-trees, often in a spectacular manner (search for 'exploding network' in the issues).
+ *
+ * In order to distinguish the definitions in the following code, the adjective 'formal' is
+ * used. If 'formal' is absent, you must assume the non-formal definition.
+ *
+ * ----------------------------------------------------------------------------------
+ * NOTES
+ * =====
+ *
+ * A hierarchical layout is a different thing from a hierarchical network.
+ * The layout is a way to arrange the nodes in the view; this can be done
+ * on non-hierarchical networks as well. The converse is also possible.
+ */
 let util = require('../../util');
 var NetworkUtil = require('../NetworkUtil').default;
 
@@ -8,19 +38,20 @@ var NetworkUtil = require('../NetworkUtil').default;
  * Container for derived data on current network, relating to hierarchy.
  *
  * Local, private class.
- *
- * TODO: Perhaps move more code for hierarchy state handling to this class.
- *       Till now, only the required and most obvious has been done.
  */
 class HierarchicalStatus {
 
   constructor() {
-    this.childrenReference = {};
-    this.parentReference = {};
-    this.levels = {};
-    this.trees = {};
+    this.childrenReference = {};     // child id's per node id
+    this.parentReference = {};       // parent id's per node id
+    this.trees = {};                 // tree id per node id; i.e. to which tree does given node id belong
+
+    this.distributionOrdering = {};  // The nodes per level, in the display order
+    this.levels = {};                // hierarchy level per node id
+    this.distributionIndex = {};     // The position of the node in the level sorting order, per node id.
 
-    this.isTree = false;
+    this.isTree = false;             // True if current network is a formal tree 
+    this.treeIndex = -1;             // Highest tree id in current network.
   }
 
 
@@ -41,7 +72,7 @@ class HierarchicalStatus {
 
 
   /**
-   * Check if the current state is for a tree or forest network.
+   * Check if the current state is for a formal tree or formal forest.
    *
    * This is the case if every node has at most one parent.
    *
@@ -59,6 +90,25 @@ class HierarchicalStatus {
   }
 
 
+  /**
+   * Return the number of separate trees in the current network.
+   */
+  numTrees() {
+    return (this.treeIndex + 1);  // This assumes the indexes are assigned consecitively
+  }
+
+
+  /**
+   * Assign a tree id to a node
+   */
+  setTreeIndex(node, treeId) {
+    if (this.trees[node.id] === undefined) {
+      this.trees[node.id] = treeId;
+      this.treeIndex = Math.max(treeId, this.treeIndex);
+    }
+  }
+
+
   /**
    * Ensure level for given id is defined.
    *
@@ -165,6 +215,72 @@ class HierarchicalStatus {
       max_y: max_y
     };
   }
+
+
+  /**
+   * Check if two nodes have the same parent(s)
+   *
+   * @return true if the two nodes have a same ancestor node, false otherwise
+   */
+  hasSameParent(node1, node2) {
+    let parents1 = this.parentReference[node1.id];
+    let parents2 = this.parentReference[node2.id];
+    if (parents1 === undefined || parents2 === undefined) {
+      return false;
+    }
+
+    for (let i = 0; i < parents1.length; i++) {
+      for (let j = 0; j < parents2.length; j++) {
+        if (parents1[i] == parents2[j]) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
+
+  /**
+   * Check if two nodes are in the same tree.
+   *
+   * @return true if this is so, false otherwise
+   */
+  inSameSubNetwork(node1, node2) {
+    return (this.trees[node1.id] === this.trees[node2.id]);
+  }
+
+
+  /**
+   * Get a list of the distinct levels in the current network
+   */
+  getLevels() {
+    return Object.keys(this.distributionOrdering);
+  }
+
+
+  /**
+   * Add a node to the ordering per level
+   */
+  addToOrdering(node, level) {
+    if (this.distributionOrdering[level] === undefined) {
+      this.distributionOrdering[level] = [];
+    }
+
+    var isPresent = false;
+    var curLevel = this.distributionOrdering[level];
+    for (var n in curLevel) {
+      //if (curLevel[n].id === node.id) {
+      if (curLevel[n] === node) {
+        isPresent = true;
+        break;
+      }
+    }
+
+    if (!isPresent) {
+      this.distributionOrdering[level].push(node);
+      this.distributionIndex[node.id] = this.distributionOrdering[level].length - 1;
+    }
+  }
 }
 
 
@@ -504,12 +620,6 @@ class LayoutEngine {
       let undefinedLevel = false;
       this.lastNodeOnLevel = {};
       this.hierarchical = new HierarchicalStatus();
-      this.treeIndex = -1;
-
-      this.distributionOrdering = {};
-      this.distributionIndex = {};
-      this.distributionOrderingPresence = {};
-
 
       for (nodeId in this.body.nodes) {
         if (this.body.nodes.hasOwnProperty(nodeId)) {
@@ -593,9 +703,11 @@ class LayoutEngine {
 
     // shift a single tree by an offset
     let shiftTree = (index, offset) => {
-      for (let nodeId in this.hierarchical.trees) {
-        if (this.hierarchical.trees.hasOwnProperty(nodeId)) {
-          if (this.hierarchical.trees[nodeId] === index) {
+      let trees = this.hierarchical.trees;
+
+      for (let nodeId in trees) {
+        if (trees.hasOwnProperty(nodeId)) {
+          if (trees[nodeId] === index) {
             let node = this.body.nodes[nodeId];
             let pos = this._getPositionForHierarchy(node);
             this._setPositionForHierarchy(node, pos + offset, undefined, true);
@@ -617,7 +729,7 @@ class LayoutEngine {
     // get the width of all trees
     let getTreeSizes = () => {
       let treeWidths = [];
-      for (let i = 0; i <= this.treeIndex; i++) {
+      for (let i = 0; i <= this.hierarchical.numTrees(); i++) {
         treeWidths.push(getTreeSize(i));
       }
       return treeWidths;
@@ -677,40 +789,32 @@ class LayoutEngine {
       return Math.min(maxLevel1, maxLevel2);
     };
 
-    // check if two nodes have the same parent(s)
-    let hasSameParent = (node1, node2) => {
-      let parents1 = this.hierarchical.parentReference[node1.id];
-      let parents2 = this.hierarchical.parentReference[node2.id];
-      if (parents1 === undefined || parents2 === undefined) {
-        return false;
-      }
-
-      for (let i = 0; i < parents1.length; i++) {
-        for (let j = 0; j < parents2.length; j++) {
-          if (parents1[i] == parents2[j]) {
-            return true;
-          }
-        }
-      }
-      return false;
-    };
 
-    // condense elements. These can be nodes or branches depending on the callback.
+    /**
+     * Condense elements. These can be nodes or branches depending on the callback.
+     */
     let shiftElementsCloser = (callback, levels, centerParents) => {
+      let hier = this.hierarchical;
+
       for (let i = 0; i < levels.length; i++) {
         let level = levels[i];
-        let levelNodes = this.distributionOrdering[level];
+        let levelNodes = hier.distributionOrdering[level];
         if (levelNodes.length > 1) {
           for (let j = 0; j < levelNodes.length - 1; j++) {
-            if (hasSameParent(levelNodes[j],levelNodes[j+1]) === true)  {
-              if (this.hierarchical.trees[levelNodes[j].id] === this.hierarchical.trees[levelNodes[j+1].id])  {
-                callback(levelNodes[j],levelNodes[j+1], centerParents);
-              }
-            }}
+            let node1 = levelNodes[j];
+            let node2 = levelNodes[j+1];
+
+            // NOTE: logic maintained as it was; if nodes have same ancestor,
+            //       then of course they are in the same sub-network.
+            if (hier.hasSameParent(node1, node2) && hier.inSameSubNetwork(node1, node2) ) {
+              callback(node1, node2, centerParents);
+            }
+          }
         }
       }
     };
 
+
     // callback for shifting branches
     let branchShiftCallback = (node1, node2, centerParent = false) => {
       //window.CALLBACKS.push(() => {
@@ -884,13 +988,13 @@ class LayoutEngine {
 
     // method to remove whitespace between branches. Because we do bottom up, we can center the parents.
     let minimizeEdgeLengthBottomUp = (iterations) => {
-      let levels = Object.keys(this.distributionOrdering);
+      let levels = this.hierarchical.getLevels();
       levels = levels.reverse();
       for (let i = 0; i < iterations; i++) {
         stillShifting = false;
         for (let j = 0; j < levels.length; j++) {
           let level = levels[j];
-          let levelNodes = this.distributionOrdering[level];
+          let levelNodes = this.hierarchical.distributionOrdering[level];
           for (let k = 0; k < levelNodes.length; k++) {
             minimizeEdgeLength(1000, levelNodes[k]);
           }
@@ -904,7 +1008,7 @@ class LayoutEngine {
 
     // method to remove whitespace between branches. Because we do bottom up, we can center the parents.
     let shiftBranchesCloserBottomUp = (iterations) => {
-      let levels = Object.keys(this.distributionOrdering);
+      let levels = this.hierarchical.getLevels();
       levels = levels.reverse();
       for (let i = 0; i < iterations; i++) {
         stillShifting = false;
@@ -926,11 +1030,11 @@ class LayoutEngine {
 
     // center all parents
     let centerAllParentsBottomUp = () => {
-      let levels = Object.keys(this.distributionOrdering);
+      let levels = this.hierarchical.getLevels();
       levels = levels.reverse();
       for (let i = 0; i < levels.length; i++) {
         let level = levels[i];
-        let levelNodes = this.distributionOrdering[level];
+        let levelNodes = this.hierarchical.distributionOrdering[level];
         for (let j = 0; j < levelNodes.length; j++) {
           this._centerParent(levelNodes[j]);
         }
@@ -970,9 +1074,9 @@ class LayoutEngine {
     }
     let level = this.hierarchical.levels[node.id];
     if (level !== undefined) {
-      let index = this.distributionIndex[node.id];
+      let index = this.hierarchical.distributionIndex[node.id];
       let position = this._getPositionForHierarchy(node);
-      let ordering = this.distributionOrdering[level];
+      let ordering = this.hierarchical.distributionOrdering[level];
       let minSpace = 1e9;
       let maxSpace = 1e9;
       if (index !== 0) {
@@ -998,6 +1102,7 @@ class LayoutEngine {
     }
   }
 
+
   /**
    * We use this method to center a parent node and check if it does not cross other nodes when it does.
    * @param node
@@ -1028,7 +1133,6 @@ class LayoutEngine {
   }
 
 
-
   /**
    * This function places the nodes on the canvas based on the hierarchial distribution.
    *
@@ -1129,7 +1233,7 @@ class LayoutEngine {
    * @private
    */
   _validatePositionAndContinue(node, level, pos) {
-    // This only works for strict hierarchical networks, i.e. trees and forests
+    // This method only works for formal trees and formal forests
     // Early exit if this is not the case
     if (!this.hierarchical.isTree) return;
 
@@ -1143,11 +1247,8 @@ class LayoutEngine {
       }
     }
 
-    // store change in position.
-    this.lastNodeOnLevel[level] = node.id;
-
+    this.lastNodeOnLevel[level] = node.id;  // store change in position.
     this.positionedNodes[node.id] = true;
-
     this._placeBranchNodes(node.id, level);
   }
 
@@ -1362,15 +1463,10 @@ class LayoutEngine {
    */
   _crawlNetwork(callback = function() {}, startingNodeId) {
     let progress = {};
-    let treeIndex = 0;
 
     let crawler = (node, tree) => {
       if (progress[node.id] === undefined) {
-
-        if (this.hierarchical.trees[node.id] === undefined) {
-          this.hierarchical.trees[node.id] = tree;
-          this.treeIndex = Math.max(tree, this.treeIndex);
-        }
+        this.hierarchical.setTreeIndex(node, tree);
 
         progress[node.id] = true;
         let childNode;
@@ -1378,14 +1474,14 @@ class LayoutEngine {
         for (let i = 0; i < edges.length; i++) {
           let edge = edges[i];
           if (edge.connected === true) {
-            if (edge.toId == node.id) {         // '==' because id's can be string and numeric
+            if (edge.toId == node.id) {         // Not '===' because id's can be string and numeric
               childNode = edge.from;
             }
             else {
               childNode = edge.to;
             }
 
-            if (node.id != childNode.id) {      // '!=' because id's can be string and numeric
+            if (node.id != childNode.id) {      // Not '!==' because id's can be string and numeric
               callback(node, childNode, edge);
               crawler(childNode, tree);
             }
@@ -1395,17 +1491,22 @@ class LayoutEngine {
     };
 
 
-    // we can crawl from a specific node or over all nodes.
     if (startingNodeId === undefined) {
+      // Crawl over all nodes
+      let treeIndex = 0;      // Serves to pass a unique id for the current distinct tree
+
       for (let i = 0; i < this.body.nodeIndices.length; i++) {
-        let node = this.body.nodes[this.body.nodeIndices[i]];
-        if (progress[node.id] === undefined) {
+        let nodeId = this.body.nodeIndices[i];
+
+        if (progress[nodeId] === undefined) {
+          let node = this.body.nodes[nodeId];
           crawler(node, treeIndex);
           treeIndex += 1;
         }
       }
     }
     else {
+      // Crawl from the given starting node
       let node = this.body.nodes[startingNodeId];
       if (node === undefined) {
         console.error("Node not found:", startingNodeId);
@@ -1497,16 +1598,7 @@ class LayoutEngine {
   _setPositionForHierarchy(node, position, level, doNotUpdate = false) {
     //console.log('_setPositionForHierarchy',node.id, position)
     if (doNotUpdate !== true) {
-      if (this.distributionOrdering[level] === undefined) {
-        this.distributionOrdering[level] = [];
-        this.distributionOrderingPresence[level] = {};
-      }
-
-      if (this.distributionOrderingPresence[level][node.id] === undefined) {
-        this.distributionOrdering[level].push(node);
-        this.distributionIndex[node.id] = this.distributionOrdering[level].length - 1;
-      }
-      this.distributionOrderingPresence[level][node.id] = true;
+      this.hierarchical.addToOrdering(node, level);
     }
 
     if(this._isVertical()) {