Merge pull request #2180 from weaveworks/reapply-graph-layout-optimiz…

…ations

Re-applied "Graph layout optimizations"

client/app/scripts/charts/__tests__/node-layout-test.js

@@ -167,54 +167,6 @@ describe('NodesLayout', () => {
     expect(hasUnseen).toBeTruthy();
   });
 
-  it('shifts layouts to center', () => {
-    let xMin;
-    let xMax;
-    let yMin;
-    let yMax;
-    let xCenter;
-    let yCenter;
-
-    // make sure initial layout is centered
-    const original = NodesLayout.doLayout(
-      nodeSets.initial4.nodes,
-      nodeSets.initial4.edges
-    );
-    xMin = original.nodes.minBy(n => n.get('x'));
-    xMax = original.nodes.maxBy(n => n.get('x'));
-    yMin = original.nodes.minBy(n => n.get('y'));
-    yMax = original.nodes.maxBy(n => n.get('y'));
-    xCenter = (xMin.get('x') + xMax.get('x')) / 2;
-    yCenter = (yMin.get('y') + yMax.get('y')) / 2;
-    expect(xCenter).toEqual(NodesLayout.DEFAULT_WIDTH / 2);
-    expect(yCenter).toEqual(NodesLayout.DEFAULT_HEIGHT / 2);
-
-    // make sure re-running is idempotent
-    const rerun = NodesLayout.shiftLayoutToCenter(original);
-    xMin = rerun.nodes.minBy(n => n.get('x'));
-    xMax = rerun.nodes.maxBy(n => n.get('x'));
-    yMin = rerun.nodes.minBy(n => n.get('y'));
-    yMax = rerun.nodes.maxBy(n => n.get('y'));
-    xCenter = (xMin.get('x') + xMax.get('x')) / 2;
-    yCenter = (yMin.get('y') + yMax.get('y')) / 2;
-    expect(xCenter).toEqual(NodesLayout.DEFAULT_WIDTH / 2);
-    expect(yCenter).toEqual(NodesLayout.DEFAULT_HEIGHT / 2);
-
-    // shift after window was resized
-    const shifted = NodesLayout.shiftLayoutToCenter(original, {
-      width: 128,
-      height: 256
-    });
-    xMin = shifted.nodes.minBy(n => n.get('x'));
-    xMax = shifted.nodes.maxBy(n => n.get('x'));
-    yMin = shifted.nodes.minBy(n => n.get('y'));
-    yMax = shifted.nodes.maxBy(n => n.get('y'));
-    xCenter = (xMin.get('x') + xMax.get('x')) / 2;
-    yCenter = (yMin.get('y') + yMax.get('y')) / 2;
-    expect(xCenter).toEqual(128 / 2);
-    expect(yCenter).toEqual(256 / 2);
-  });
-
   it('lays out initial nodeset in a rectangle', () => {
     const result = NodesLayout.doLayout(
       nodeSets.initial4.nodes,

client/app/scripts/charts/edge-container.js

@@ -5,106 +5,102 @@ import { Map as makeMap } from 'immutable';
 import { line, curveBasis } from 'd3-shape';
 import { each, omit, times, constant } from 'lodash';
 
+import { NODES_SPRING_ANIMATION_CONFIG } from '../constants/animation';
 import { uniformSelect } from '../utils/array-utils';
-import { round } from '../utils/math-utils';
 import Edge from './edge';
 
-// Spring stiffness & damping respectively
-const ANIMATION_CONFIG = [80, 20];
 // Tweak this value for the number of control
 // points along the edge curve, e.g. values:
 //   * 2 -> edges are simply straight lines
 //   * 4 -> minimal value for loops to look ok
-const WAYPOINTS_CAP = 8;
+const WAYPOINTS_COUNT = 8;
 
 const spline = line()
   .curve(curveBasis)
   .x(d => d.x)
   .y(d => d.y);
 
-const buildPath = (points, layoutPrecision) => {
-  const extracted = [];
-  each(points, (value, key) => {
-    const axis = key[0];
-    const index = key.slice(1);
-    if (!extracted[index]) {
-      extracted[index] = {};
-    }
-    extracted[index][axis] = round(value, layoutPrecision);
+const transformedEdge = (props, path) => (
+  <Edge {...props} path={spline(path)} />
+);
+
+// Converts a waypoints map of the format {x0: 11, y0: 22, x1: 33, y1: 44}
+// that is used by Motion to an array of waypoints in the format
+// [{x: 11, y: 22}, {x: 33, y: 44}] that can be used by D3.
+const waypointsMapToArray = (waypointsMap) => {
+  const waypointsArray = times(WAYPOINTS_COUNT, () => ({}));
+  each(waypointsMap, (value, key) => {
+    const [axis, index] = [key[0], key.slice(1)];
+    waypointsArray[index][axis] = value;
   });
-  return extracted;
+  return waypointsArray;
 };
 
-class EdgeContainer extends React.Component {
 
+class EdgeContainer extends React.Component {
   constructor(props, context) {
     super(props, context);
-    this.state = {
-      pointsMap: makeMap()
-    };
+    this.state = { waypointsMap: makeMap() };
   }
 
   componentWillMount() {
-    this.preparePoints(this.props.points);
+    if (this.props.isAnimated) {
+      this.prepareWaypointsForMotion(this.props.waypoints);
+    }
   }
 
   componentWillReceiveProps(nextProps) {
     // immutablejs allows us to `===`! \o/
-    if (nextProps.points !== this.props.points) {
-      this.preparePoints(nextProps.points);
+    if (this.props.isAnimated && nextProps.waypoints !== this.props.waypoints) {
+      this.prepareWaypointsForMotion(nextProps.waypoints);
     }
   }
 
   render() {
-    const { layoutPrecision, points } = this.props;
-    const other = omit(this.props, 'points');
+    const { isAnimated, waypoints } = this.props;
+    const forwardedProps = omit(this.props, 'isAnimated', 'waypoints');
 
-    if (layoutPrecision === 0) {
-      const path = spline(points.toJS());
-      return <Edge {...other} path={path} />;
+    if (!isAnimated) {
+      return transformedEdge(forwardedProps, waypoints.toJS());
     }
 
     return (
-      <Motion style={this.state.pointsMap.toJS()}>
-        {(interpolated) => {
-          // convert points to path string, because that lends itself to
-          // JS-equality checks in the child component
-          const path = spline(buildPath(interpolated, layoutPrecision));
-          return <Edge {...other} path={path} />;
-        }}
+      // For the Motion interpolation to work, the waypoints need to be in a map format like
+      // {x0: 11, y0: 22, x1: 33, y1: 44} that we convert to the array format when rendering.
+      <Motion style={this.state.waypointsMap.toJS()}>
+        {interpolated => transformedEdge(forwardedProps, waypointsMapToArray(interpolated))}
       </Motion>
     );
   }
 
-  preparePoints(nextPoints) {
-    nextPoints = nextPoints.toJS();
+  prepareWaypointsForMotion(nextWaypoints) {
+    nextWaypoints = nextWaypoints.toJS();
 
     // Motion requires a constant number of waypoints along the path of each edge
     // for the animation to work correctly, but dagre might be changing their number
     // depending on the dynamic topology reconfiguration. Here we are transforming
-    // the waypoints array given by dagre to the fixed size of `WAYPOINTS_CAP` that
+    // the waypoints array given by dagre to the fixed size of `WAYPOINTS_COUNT` that
     // Motion could take over.
-    const pointsMissing = WAYPOINTS_CAP - nextPoints.length;
-    if (pointsMissing > 0) {
+    const waypointsMissing = WAYPOINTS_COUNT - nextWaypoints.length;
+    if (waypointsMissing > 0) {
       // Whenever there are some waypoints missing, we simply populate the beginning of the
       // array with the first element, as this leaves the curve interpolation unchanged.
-      nextPoints = times(pointsMissing, constant(nextPoints[0])).concat(nextPoints);
-    } else if (pointsMissing < 0) {
+      nextWaypoints = times(waypointsMissing, constant(nextWaypoints[0])).concat(nextWaypoints);
+    } else if (waypointsMissing < 0) {
       // If there are 'too many' waypoints given by dagre, we select a sub-array of
       // uniformly distributed indices. Note that it is very important to keep the first
       // and the last endpoints in the array as they are the ones connecting the nodes.
-      nextPoints = uniformSelect(nextPoints, WAYPOINTS_CAP);
+      nextWaypoints = uniformSelect(nextWaypoints, WAYPOINTS_COUNT);
     }
 
-    let { pointsMap } = this.state;
-    nextPoints.forEach((point, index) => {
-      pointsMap = pointsMap.set(`x${index}`, spring(point.x, ANIMATION_CONFIG));
-      pointsMap = pointsMap.set(`y${index}`, spring(point.y, ANIMATION_CONFIG));
+    let { waypointsMap } = this.state;
+    nextWaypoints.forEach((point, index) => {
+      waypointsMap = waypointsMap.set(`x${index}`, spring(point.x, NODES_SPRING_ANIMATION_CONFIG));
+      waypointsMap = waypointsMap.set(`y${index}`, spring(point.y, NODES_SPRING_ANIMATION_CONFIG));
     });
 
-    this.setState({ pointsMap });
+    this.setState({ waypointsMap });
   }
-
 }
 
 export default connect()(EdgeContainer);

client/app/scripts/charts/edge.js

@@ -3,6 +3,8 @@ import { connect } from 'react-redux';
 import classNames from 'classnames';
 
 import { enterEdge, leaveEdge } from '../actions/app-actions';
+import { isContrastMode } from '../utils/contrast-utils';
+import { NODE_BASE_SIZE } from '../constants/styles';
 
 class Edge extends React.Component {
 
@@ -13,15 +15,19 @@ class Edge extends React.Component {
   }
 
   render() {
-    const { id, path, highlighted, blurred, focused } = this.props;
-    const className = classNames('edge', {highlighted, blurred, focused});
+    const { id, path, highlighted, blurred, focused, scale } = this.props;
+    const className = classNames('edge', { highlighted, blurred, focused });
+    const thickness = scale * (isContrastMode() ? 0.02 : 0.01) * NODE_BASE_SIZE;
 
+    // Draws the edge so that its thickness reflects the zoom scale.
+    // Edge shadow is always made 10x thicker than the edge itself.
     return (
       <g
-        className={className} onMouseEnter={this.handleMouseEnter}
-        onMouseLeave={this.handleMouseLeave} id={id}>
-        <path d={path} className="shadow" />
-        <path d={path} className="link" />
+        id={id} className={className}
+        onMouseEnter={this.handleMouseEnter}
+        onMouseLeave={this.handleMouseLeave}>
+        <path className="shadow" d={path} style={{ strokeWidth: 10 * thickness }} />
+        <path className="link" d={path} style={{ strokeWidth: thickness }} />
       </g>
     );
   }

client/app/scripts/charts/node-container.js

@@ -3,29 +3,39 @@ import { omit } from 'lodash';
 import { connect } from 'react-redux';
 import { Motion, spring } from 'react-motion';
 
-import { round } from '../utils/math-utils';
+import { NODES_SPRING_ANIMATION_CONFIG } from '../constants/animation';
+import { NODE_BLUR_OPACITY } from '../constants/styles';
 import Node from './node';
 
+const transformedNode = (otherProps, { x, y, k }) => (
+  <Node transform={`translate(${x},${y}) scale(${k})`} {...otherProps} />
+);
+
 class NodeContainer extends React.Component {
   render() {
-    const { dx, dy, focused, layoutPrecision, zoomScale } = this.props;
-    const animConfig = [80, 20]; // stiffness, damping
-    const scaleFactor = focused ? (1 / zoomScale) : 1;
-    const other = omit(this.props, 'dx', 'dy');
+    const { dx, dy, isAnimated, scale, blurred } = this.props;
+    const forwardedProps = omit(this.props, 'dx', 'dy', 'isAnimated', 'scale', 'blurred');
+    const opacity = blurred ? NODE_BLUR_OPACITY : 1;
 
+    // NOTE: Controlling blurring from here seems to re-render faster
+    // than adding a CSS class and controlling it from there.
     return (
-      <Motion
-        style={{
-          x: spring(dx, animConfig),
-          y: spring(dy, animConfig),
-          f: spring(scaleFactor, animConfig)
-        }}>
-        {(interpolated) => {
-          const transform = `translate(${round(interpolated.x, layoutPrecision)},`
-            + `${round(interpolated.y, layoutPrecision)})`;
-          return <Node {...other} transform={transform} scaleFactor={interpolated.f} />;
-        }}
-      </Motion>
+      <g className="node-container" style={{opacity}}>
+        {!isAnimated ?
+
+        // Show static node for optimized rendering
+        transformedNode(forwardedProps, { x: dx, y: dy, k: scale }) :
+
+        // Animate the node if the layout is sufficiently small
+        <Motion
+          style={{
+            x: spring(dx, NODES_SPRING_ANIMATION_CONFIG),
+            y: spring(dy, NODES_SPRING_ANIMATION_CONFIG),
+            k: spring(scale, NODES_SPRING_ANIMATION_CONFIG)
+          }}>
+          {interpolated => transformedNode(forwardedProps, interpolated)}
+        </Motion>}
+      </g>
     );
   }
 }

client/app/scripts/charts/node-networks-overlay.js

@@ -3,51 +3,43 @@ import { scaleBand } from 'd3-scale';
 import { List as makeList } from 'immutable';
 import { getNetworkColor } from '../utils/color-utils';
 import { isContrastMode } from '../utils/contrast-utils';
-
-
-// Gap size between bar segments.
-const minBarHeight = 3;
-const padding = 0.05;
-const rx = 1;
-const ry = rx;
+import { NODE_BASE_SIZE } from '../constants/styles';
+
+// Min size is about a quarter of the width, feels about right.
+const minBarWidth = 0.25;
+const barHeight = 0.08;
+const innerPadding = 0.04;
+const borderRadius = 0.01;
+const offset = 0.67;
 const x = scaleBand();
 
-function NodeNetworksOverlay({offset, size, stack, networks = makeList()}) {
-  // Min size is about a quarter of the width, feels about right.
-  const minBarWidth = (size / 4);
-  const barWidth = Math.max(size, minBarWidth * networks.size);
-  const barHeight = Math.max(size * 0.085, minBarHeight);
+function NodeNetworksOverlay({ stack, networks = makeList() }) {
+  const barWidth = Math.max(1, minBarWidth * networks.size);
+  const yPosition = offset - (barHeight * 0.5);
 
   // Update singleton scale.
   x.domain(networks.map((n, i) => i).toJS());
   x.range([barWidth * -0.5, barWidth * 0.5]);
-  x.paddingInner(padding);
+  x.paddingInner(innerPadding);
 
+  const bandwidth = x.bandwidth();
   const bars = networks.map((n, i) => (
     <rect
-      x={x(i)}
-      y={offset - (barHeight * 0.5)}
-      width={x.bandwidth()}
-      height={barHeight}
-      rx={rx}
-      ry={ry}
       className="node-network"
-      style={{
-        fill: getNetworkColor(n.get('colorKey', n.get('id')))
-      }}
       key={n.get('id')}
+      x={x(i)}
+      y={yPosition}
+      width={bandwidth}
+      height={barHeight}
+      rx={borderRadius}
+      ry={borderRadius}
+      style={{ fill: getNetworkColor(n.get('colorKey', n.get('id'))) }}
     />
   ));
 
-  let transform = '';
-  if (stack) {
-    const contrastMode = isContrastMode();
-    const [dx, dy] = contrastMode ? [0, 8] : [0, 0];
-    transform = `translate(${dx}, ${dy * -1.5})`;
-  }
-
+  const translateY = stack && isContrastMode() ? 0.15 : 0;
   return (
-    <g transform={transform}>
+    <g transform={`translate(0, ${translateY}) scale(${NODE_BASE_SIZE})`}>
       {bars.toJS()}
     </g>
   );