diff --git a/client/app/scripts/charts/edge-container.js b/client/app/scripts/charts/edge-container.js index 735afab79..a3a917b53 100644 --- a/client/app/scripts/charts/edge-container.js +++ b/client/app/scripts/charts/edge-container.js @@ -5,14 +5,9 @@ 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 { EDGE_WAYPOINTS_CAP } from '../constants/styles'; import Edge from './edge'; -// 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_COUNT = 8; const spline = line() .curve(curveBasis) @@ -23,11 +18,11 @@ const transformedEdge = (props, path) => ( ); -// Converts a waypoints map of the format {x0: 11, y0: 22, x1: 33, y1: 44} +// 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. +// [{ x: 11, y: 22 }, { x: 33, y: 44 }] that can be used by D3. const waypointsMapToArray = (waypointsMap) => { - const waypointsArray = times(WAYPOINTS_COUNT, () => ({ x: 0, y: 0})); + const waypointsArray = times(EDGE_WAYPOINTS_CAP, () => ({})); each(waypointsMap, (value, key) => { const [axis, index] = [key[0], key.slice(1)]; waypointsArray[index][axis] = value; @@ -35,6 +30,17 @@ const waypointsMapToArray = (waypointsMap) => { return waypointsArray; }; +// Converts a waypoints array of the input format [{ x: 11, y: 22 }, { x: 33, y: 44 }] +// to an array of waypoints that is used by Motion in the format { x0: 11, y0: 22, x1: 33, y1: 44 }. +const waypointsArrayToMap = (waypointsArray) => { + let waypointsMap = makeMap(); + waypointsArray.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)); + }); + return waypointsMap; +}; + export default class EdgeContainer extends React.PureComponent { constructor(props, context) { @@ -65,39 +71,24 @@ export default class EdgeContainer extends React.PureComponent { return ( // 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. + // { x0: 11, y0: 22, x1: 33, y1: 44 } that we convert to the array format when rendering. {interpolated => transformedEdge(forwardedProps, waypointsMapToArray(interpolated))} ); } - prepareWaypointsForMotion(nextWaypoints) { - nextWaypoints = nextWaypoints.toJS(); + prepareWaypointsForMotion(waypoints) { + waypoints = waypoints.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_COUNT` that - // Motion could take over. - const waypointsMissing = WAYPOINTS_COUNT - nextWaypoints.length; + // The Motion library requires the number of waypoints to be constant, so we fill in for + // the missing ones by reusing the edge source point, which doesn't affect the edge shape + // because of how the curveBasis interpolation is done. + const waypointsMissing = EDGE_WAYPOINTS_CAP - waypoints.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. - 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. - nextWaypoints = uniformSelect(nextWaypoints, WAYPOINTS_COUNT); + waypoints = times(waypointsMissing, constant(waypoints[0])).concat(waypoints); } - 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({ waypointsMap }); + this.setState({ waypointsMap: waypointsArrayToMap(waypoints) }); } } diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 7a2e42bc9..f061f8315 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -2,18 +2,21 @@ import dagre from 'dagre'; import debug from 'debug'; import { fromJS, Map as makeMap, Set as ImmSet } from 'immutable'; -import { NODE_BASE_SIZE } from '../constants/styles'; +import { NODE_BASE_SIZE, EDGE_WAYPOINTS_CAP } from '../constants/styles'; import { EDGE_ID_SEPARATOR } from '../constants/naming'; import { featureIsEnabledAny } from '../utils/feature-utils'; import { buildTopologyCacheId, updateNodeDegrees } from '../utils/topology-utils'; +import { uniformSelect } from '../utils/array-utils'; const log = debug('scope:nodes-layout'); const topologyCaches = {}; -export const DEFAULT_MARGINS = {top: 0, left: 0}; -const NODE_SIZE_FACTOR = NODE_BASE_SIZE; -const NODE_SEPARATION_FACTOR = 1.5 * NODE_BASE_SIZE; -const RANK_SEPARATION_FACTOR = 2.5 * NODE_BASE_SIZE; +export const DEFAULT_MARGINS = { top: 0, left: 0 }; +// Pretend the nodes are bigger than they are so that the edges would not enter +// them under a high curvature which would cause arrow heads to be misplaced. +const NODE_SIZE_FACTOR = 1.5 * NODE_BASE_SIZE; +const NODE_SEPARATION_FACTOR = 1 * NODE_BASE_SIZE; +const RANK_SEPARATION_FACTOR = 2 * NODE_BASE_SIZE; let layoutRuns = 0; let layoutRunsTrivial = 0; @@ -25,6 +28,42 @@ function fromGraphNodeId(encodedId) { return encodedId.replace('', '.'); } +// Adds some additional waypoints to the edge to make sure the it connects the node +// centers and that the edge enters the target node relatively straight so that the +// arrow is drawn correctly. The total number of waypoints is capped to EDGE_WAYPOINTS_CAP. +function correctedEdgePath(waypoints, source, target) { + // Get the relevant waypoints that will be added/replicated. + const sourcePoint = fromJS({ x: source.get('x'), y: source.get('y') }); + const targetPoint = fromJS({ x: target.get('x'), y: target.get('y') }); + const entrancePoint = waypoints.last(); + + if (target !== source) { + // The strategy for the non-loop edges is the following: + // * Uniformly select at most CAP - 4 of the central waypoints ignoring the target node + // entrance point. Such a selection will ensure that both the source node exit point and + // the point before the target node entrance point are taken as boundaries of the interval. + // * Now manually add those 4 points that we always want to have included in the edge path - + // centers of source/target nodes and twice the target node entrance point to ensure the + // edge path actually goes through it and thus doesn't miss the arrow element. + // * In the end, what matters for the arrow is that the last 4 points of the array are always + // fixed regardless of the total number of waypoints. That way we ensure the arrow is drawn + // correctly, but also that the edge path enters the target node smoothly. + waypoints = fromJS(uniformSelect(waypoints.butLast().toJS(), EDGE_WAYPOINTS_CAP - 4)); + waypoints = waypoints.unshift(sourcePoint); + waypoints = waypoints.push(entrancePoint); + waypoints = waypoints.push(entrancePoint); + waypoints = waypoints.push(targetPoint); + } else { + // For loops we simply set the endpoints at the center of source/target node to + // make them smoother and, of course, we cap the total number of waypoints. + waypoints = fromJS(uniformSelect(waypoints.toJS(), EDGE_WAYPOINTS_CAP)); + waypoints = waypoints.set(0, sourcePoint); + waypoints = waypoints.set(waypoints.size - 1, targetPoint); + } + + return waypoints; +} + /** * Layout engine runner * After the layout engine run nodes and edges have x-y-coordinates. Engine is @@ -102,15 +141,12 @@ function runLayoutEngine(graph, imNodes, imEdges) { graph.edges().forEach((graphEdge) => { const graphEdgeMeta = graph.edge(graphEdge); const edge = edges.get(graphEdgeMeta.id); - let points = fromJS(graphEdgeMeta.points); - // set beginning and end points to node coordinates to ignore node bounding box const source = nodes.get(fromGraphNodeId(edge.get('source'))); const target = nodes.get(fromGraphNodeId(edge.get('target'))); - points = points.mergeIn([0], {x: source.get('x'), y: source.get('y')}); - points = points.mergeIn([points.size - 1], {x: target.get('x'), y: target.get('y')}); + const waypoints = correctedEdgePath(fromJS(graphEdgeMeta.points), source, target); - edges = edges.setIn([graphEdgeMeta.id, 'points'], points); + edges = edges.setIn([graphEdgeMeta.id, 'points'], waypoints); }); // return object with the width and height of layout diff --git a/client/app/scripts/constants/styles.js b/client/app/scripts/constants/styles.js index 53d951037..8508fb999 100644 --- a/client/app/scripts/constants/styles.js +++ b/client/app/scripts/constants/styles.js @@ -31,6 +31,11 @@ export const UNIT_CLOUD_PATH = 'M-1.25 0.233Q-1.25 0.44-1.104 0.587-0.957 0.733- // are given on a small unit scale as foreign objects in SVG. export const NODE_BASE_SIZE = 100; +// This value represents the upper bound on the number of control points along the graph edge +// curve. Any integer value >= 6 should result in valid edges, but generally the greater this +// value is, the nicer the edge bundling will be. On the other hand, big values would result +// in slower rendering of the graph. +export const EDGE_WAYPOINTS_CAP = 10; export const CANVAS_MARGINS = { [GRAPH_VIEW_MODE]: { top: 160, left: 40, right: 40, bottom: 150 },