From e8eba61c37428790ad7fd75cf5da3127c5b0b29d Mon Sep 17 00:00:00 2001 From: fbarl Date: Tue, 29 Nov 2016 18:06:40 +0100 Subject: [PATCH] Remove the dagre waypoints limitation for edges and replace it with WAYPOINTS_CAP constant for fine tunning (fix #1187) --- client/app/scripts/charts/edge-container.js | 45 ++++++++++++------- .../utils/__tests__/array-utils-test.js | 36 +++++++++++++++ .../utils/__tests__/math-utils-test.js | 2 +- client/app/scripts/utils/array-utils.js | 11 +++++ 4 files changed, 76 insertions(+), 18 deletions(-) create mode 100644 client/app/scripts/utils/__tests__/array-utils-test.js create mode 100644 client/app/scripts/utils/array-utils.js diff --git a/client/app/scripts/charts/edge-container.js b/client/app/scripts/charts/edge-container.js index 40bb25a85..8a1defec3 100644 --- a/client/app/scripts/charts/edge-container.js +++ b/client/app/scripts/charts/edge-container.js @@ -5,11 +5,17 @@ import { Motion, spring } from 'react-motion'; import { Map as makeMap } from 'immutable'; import { line, curveBasis } from 'd3-shape'; +import { uniformSelect } from '../utils/array-utils'; import { round } from '../utils/math-utils'; import Edge from './edge'; -const animConfig = [80, 20]; // stiffness, damping -const pointCount = 30; +// 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 spline = line() .curve(curveBasis) @@ -71,24 +77,29 @@ class EdgeContainer extends React.Component { } preparePoints(nextPoints) { - // Spring needs constant field count, hoping that dagre will insert never more than `pointCount` - let { pointsMap } = this.state; + nextPoints = nextPoints.toJS(); - // filling up the map with copies of the first point - const filler = nextPoints.first(); - const missing = pointCount - nextPoints.size; - let index = 0; - if (missing > 0) { - while (index < missing) { - pointsMap = pointsMap.set(`x${index}`, spring(filler.get('x'), animConfig)); - pointsMap = pointsMap.set(`y${index}`, spring(filler.get('y'), animConfig)); - index++; - } + // 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 + // Motion could take over. + const pointsMissing = WAYPOINTS_CAP - nextPoints.length; + if (pointsMissing > 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) { + // 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); } - nextPoints.forEach((point, i) => { - pointsMap = pointsMap.set(`x${index + i}`, spring(point.get('x'), animConfig)); - pointsMap = pointsMap.set(`y${index + i}`, spring(point.get('y'), animConfig)); + 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)); }); this.setState({ pointsMap }); diff --git a/client/app/scripts/utils/__tests__/array-utils-test.js b/client/app/scripts/utils/__tests__/array-utils-test.js new file mode 100644 index 000000000..7f8603a84 --- /dev/null +++ b/client/app/scripts/utils/__tests__/array-utils-test.js @@ -0,0 +1,36 @@ +import _ from 'lodash'; + +describe('ArrayUtils', () => { + const ArrayUtils = require('../array-utils'); + + describe('uniformSelect', () => { + const f = ArrayUtils.uniformSelect; + + it('it should select the array elements uniformly, including the endpoints', () => { + expect(f(['x', 'y'], 3)).toEqual(['x', 'y']); + expect(f(['x', 'y'], 2)).toEqual(['x', 'y']); + + expect(f(['A', 'B', 'C', 'D', 'E'], 6)).toEqual(['A', 'B', 'C', 'D', 'E']); + expect(f(['A', 'B', 'C', 'D', 'E'], 5)).toEqual(['A', 'B', 'C', 'D', 'E']); + expect(f(['A', 'B', 'C', 'D', 'E'], 4)).toEqual(['A', 'B', 'D', 'E']); + expect(f(['A', 'B', 'C', 'D', 'E'], 3)).toEqual(['A', 'C', 'E']); + expect(f(['A', 'B', 'C', 'D', 'E'], 2)).toEqual(['A', 'E']); + + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 12)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 11)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 10)).toEqual([1, 2, 3, 4, 5, 7, 8, 9, 10, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 9)).toEqual([1, 2, 3, 5, 6, 7, 9, 10, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 8)).toEqual([1, 2, 4, 5, 7, 8, 10, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 7)).toEqual([1, 2, 4, 6, 8, 10, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 6)).toEqual([1, 3, 5, 7, 9, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 5)).toEqual([1, 3, 6, 9, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 4)).toEqual([1, 4, 8, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 3)).toEqual([1, 6, 11]); + expect(f([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11], 2)).toEqual([1, 11]); + + expect(f(_.range(1, 10001), 4)).toEqual([1, 3334, 6667, 10000]); + expect(f(_.range(1, 10001), 3)).toEqual([1, 5000, 10000]); + expect(f(_.range(1, 10001), 2)).toEqual([1, 10000]); + }); + }); +}); diff --git a/client/app/scripts/utils/__tests__/math-utils-test.js b/client/app/scripts/utils/__tests__/math-utils-test.js index 2ad946a55..6d5f95398 100644 --- a/client/app/scripts/utils/__tests__/math-utils-test.js +++ b/client/app/scripts/utils/__tests__/math-utils-test.js @@ -2,7 +2,7 @@ describe('MathUtils', () => { const MathUtils = require('../math-utils'); - describe('module', () => { + describe('modulo', () => { const f = MathUtils.modulo; it('it should calculate the modulo (also for negatives)', () => { diff --git a/client/app/scripts/utils/array-utils.js b/client/app/scripts/utils/array-utils.js new file mode 100644 index 000000000..52cc098e0 --- /dev/null +++ b/client/app/scripts/utils/array-utils.js @@ -0,0 +1,11 @@ +import _ from 'lodash'; + +export function uniformSelect(array, size) { + if (size > array.length) { + return array; + } + + return _.range(size).map(index => + array[parseInt(index * array.length / (size - 1 + 1e-9), 10)] + ); +}