Merge pull request #379 from weaveworks/dancing

Stop the rendered graph from dancing.

client/app/scripts/charts/nodes-chart.js

@@ -133,11 +133,10 @@ const NodesChart = React.createClass({
     _.each(topology, function(node, id) {
       nodes[id] = prevNodes[id] || {};
 
-      // initialize position for new nodes
+      // use cached positions if available
       _.defaults(nodes[id], {
         x: centerX,
-        y: centerY,
-        textAnchor: 'start'
+        y: centerY
       });
 
       // copy relevant fields to state nodes
@@ -184,12 +183,17 @@ const NodesChart = React.createClass({
   },
 
   updateGraphState: function(props) {
+    const n = _.size(props.nodes);
+
+    if (n === 0) {
+      return;
+    }
+
     const nodes = this.initNodes(props.nodes, this.state.nodes);
     const edges = this.initEdges(props.nodes, nodes);
 
     const expanse = Math.min(props.height, props.width);
     const nodeSize = expanse / 2;
-    const n = _.size(props.nodes);
     const nodeScale = d3.scale.linear().range([0, nodeSize / Math.pow(n, 0.7)]);
 
     const timedLayouter = timely(NodesLayout.doLayout);
@@ -199,7 +203,8 @@ const NodesChart = React.createClass({
       props.width,
       props.height,
       nodeScale,
-      MARGINS
+      MARGINS,
+      this.props.topologyId
     );
 
     debug('graph layout took ' + timedLayouter.time + 'ms');

client/app/scripts/charts/nodes-layout.js

@@ -1,61 +1,92 @@
 const dagre = require('dagre');
 const debug = require('debug')('scope:nodes-layout');
+const Naming = require('../constants/naming');
 const _ = require('lodash');
 
 const MAX_NODES = 100;
+const topologyGraphs = {};
 
-const doLayout = function(nodes, edges, width, height, scale, margins) {
+const doLayout = function(nodes, edges, width, height, scale, margins, topologyId) {
   let offsetX = 0 + margins.left;
   let offsetY = 0 + margins.top;
-  const g = new dagre.graphlib.Graph({});
+  let graph;
 
   if (_.size(nodes) > MAX_NODES) {
     debug('Too many nodes for graph layout engine. Limit: ' + MAX_NODES);
     return null;
   }
 
-  // configure node margins
+  // one engine per topology, to keep renderings similar
+  if (!topologyGraphs[topologyId]) {
+    topologyGraphs[topologyId] = new dagre.graphlib.Graph({});
+  }
+  graph = topologyGraphs[topologyId];
 
-  g.setGraph({
+  // configure node margins
+  graph.setGraph({
     nodesep: scale(2.5),
     ranksep: scale(2.5)
   });
 
-  // add nodes and edges to layout engine
-
+  // add nodes to the graph if not already there
   _.each(nodes, function(node) {
-    g.setNode(node.id, {id: node.id, width: scale(0.75), height: scale(0.75)});
+    if (!graph.hasNode(node.id)) {
+      graph.setNode(node.id, {
+        id: node.id,
+        width: scale(0.75),
+        height: scale(0.75)
+      });
+    }
   });
 
+  // remove nodes that are no longer there
+  _.each(graph.nodes(), function(nodeid) {
+    if (!_.has(nodes, nodeid)) {
+      graph.removeNode(nodeid);
+    }
+  });
+
+  // add edges to the graph if not already there
   _.each(edges, function(edge) {
-    const virtualNodes = edge.source.id === edge.target.id ? 1 : 0;
-    g.setEdge(edge.source.id, edge.target.id, {id: edge.id, minlen: virtualNodes});
+    if (!graph.hasEdge(edge.source.id, edge.target.id)) {
+      const virtualNodes = edge.source.id === edge.target.id ? 1 : 0;
+      graph.setEdge(edge.source.id, edge.target.id, {id: edge.id, minlen: virtualNodes});
+    }
   });
 
-  dagre.layout(g);
+  // remoed egdes that are no longer there
+  _.each(graph.edges(), function(edgeObj) {
+    const edge = [edgeObj.v, edgeObj.w];
+    const edgeId = edge.join(Naming.EDGE_ID_SEPARATOR);
+    if (!_.has(edges, edgeId)) {
+      graph.removeEdge(edgeObj.v, edgeObj.w);
+    }
+  });
 
-  const graph = g.graph();
+  dagre.layout(graph);
+
+  const layout = graph.graph();
 
   // shifting graph coordinates to center
 
-  if (graph.width < width) {
-    offsetX = (width - graph.width) / 2 + margins.left;
+  if (layout.width < width) {
+    offsetX = (width - layout.width) / 2 + margins.left;
   }
-  if (graph.height < height) {
-    offsetY = (height - graph.height) / 2 + margins.top;
+  if (layout.height < height) {
+    offsetY = (height - layout.height) / 2 + margins.top;
   }
 
   // apply coordinates to nodes and edges
 
-  g.nodes().forEach(function(id) {
+  graph.nodes().forEach(function(id) {
     const node = nodes[id];
-    const graphNode = g.node(id);
+    const graphNode = graph.node(id);
     node.x = graphNode.x + offsetX;
     node.y = graphNode.y + offsetY;
   });
 
-  g.edges().forEach(function(id) {
-    const graphEdge = g.edge(id);
+  graph.edges().forEach(function(id) {
+    const graphEdge = graph.edge(id);
     const edge = edges[graphEdge.id];
     _.each(graphEdge.points, function(point) {
       point.x += offsetX;
@@ -64,9 +95,9 @@ const doLayout = function(nodes, edges, width, height, scale, margins) {
     edge.points = graphEdge.points;
   });
 
-  // return object with width and height of layout
+  // return object with the width and height of layout
 
-  return graph;
+  return layout;
 };
 
 module.exports = {

client/app/scripts/components/app.js

@@ -15,6 +15,7 @@ const ESC_KEY_CODE = 27;
 function getStateFromStores() {
   return {
     currentTopology: AppStore.getCurrentTopology(),
+    currentTopologyId: AppStore.getCurrentTopologyId(),
     errorUrl: AppStore.getErrorUrl(),
     highlightedEdgeIds: AppStore.getHighlightedEdgeIds(),
     highlightedNodeIds: AppStore.getHighlightedNodeIds(),
@@ -70,7 +71,8 @@ const App = React.createClass({
         </div>
 
         <Nodes nodes={this.state.nodes} highlightedNodeIds={this.state.highlightedNodeIds}
-          highlightedEdgeIds={this.state.highlightedEdgeIds} />
+          highlightedEdgeIds={this.state.highlightedEdgeIds}
+          topologyId={this.state.currentTopologyId} />
 
         <div className="footer">
           {versionString}&nbsp;&nbsp;

client/app/scripts/components/nodes.js

@@ -37,6 +37,7 @@ const Nodes = React.createClass({
           onNodeClick={this.onNodeClick}
           width={this.state.width}
           height={this.state.height}
+          topologyId={this.props.topologyId}
           context="view"
         />
       </div>

client/app/scripts/stores/app-store.js

@@ -57,6 +57,10 @@ const AppStore = assign({}, EventEmitter.prototype, {
     return findCurrentTopology(topologies, currentTopologyId);
   },
 
+  getCurrentTopologyId: function() {
+    return currentTopologyId;
+  },
+
   getCurrentTopologyUrl: function() {
     const topology = this.getCurrentTopology();