From 9ba0927be307aa91432ae7920f0c7d75d67fe796 Mon Sep 17 00:00:00 2001
From: bru5 <45365769+bru5@users.noreply.github.com>
Date: Tue, 16 Mar 2021 10:08:53 +0000
Subject: [PATCH 1/5] [KED-2365] Simplify graph algorithm, improve graph layout
 quality and performance

---
 src/utils/graph/constraints.js | 133 +++++++------------
 src/utils/graph/graph.js       |   6 +-
 src/utils/graph/layout.js      | 236 ++++++++++++---------------------
 3 files changed, 136 insertions(+), 239 deletions(-)

diff --git a/src/utils/graph/constraints.js b/src/utils/graph/constraints.js
index 545851a380..6b0e5ec157 100644
--- a/src/utils/graph/constraints.js
+++ b/src/utils/graph/constraints.js
@@ -12,28 +12,13 @@ import { Constraint, Operator, Strength } from 'kiwi.js';
 export const rowConstraint = {
   property: 'y',
 
-  solve: (constraint, constants) => {
-    const { a, b } = constraint;
-    const difference = a.y - b.y;
-    const target = constants.spaceY;
-
-    if (difference >= target) {
-      return;
-    }
-
-    const resolve = difference - target;
-    a.y -= 0.5 * resolve;
-    b.y += 0.5 * resolve;
-  },
-
-  strict: (constraint, constants, variableA, variableB) => {
-    return new Constraint(
+  strict: (constraint, constants, variableA, variableB) =>
+    new Constraint(
       variableA.minus(variableB),
       Operator.Ge,
       constants.spaceY,
       Strength.required
-    );
-  },
+    ),
 };
 
 /**
@@ -42,28 +27,13 @@ export const rowConstraint = {
 export const layerConstraint = {
   property: 'y',
 
-  solve: (constraint, constants) => {
-    const { a, b } = constraint;
-    const difference = a.y - b.y;
-    const target = constants.layerSpace;
-
-    if (difference >= target) {
-      return;
-    }
-
-    const resolve = difference - target;
-    a.y -= 0.5 * resolve;
-    b.y += 0.5 * resolve;
-  },
-
-  strict: (constraint, constants, variableA, variableB) => {
-    return new Constraint(
+  strict: (constraint, constants, variableA, variableB) =>
+    new Constraint(
       variableA.minus(variableB),
       Operator.Ge,
       constants.layerSpace,
       Strength.required
-    );
-  },
+    ),
 };
 
 /**
@@ -73,54 +43,58 @@ export const parallelConstraint = {
   property: 'x',
 
   solve: (constraint) => {
-    const { a, b } = constraint;
-    const difference = a.x - b.x;
-
-    if (difference === 0) {
-      return;
-    }
-
-    const strength =
-      1 / Math.max(1, 0.5 * (a.targets.length + b.sources.length));
-
-    const resolve = strength * difference;
-    a.x -= 0.5 * resolve;
-    b.x += 0.5 * resolve;
+    const { a, b, strength } = constraint;
+    const resolve = strength * (a.x - b.x);
+    a.x -= resolve;
+    b.x += resolve;
   },
 
-  strict: (constraint, constants, variableA, variableB) => {
-    return new Constraint(
+  strict: (constraint, constants, variableA, variableB) =>
+    new Constraint(
       variableA.minus(variableB),
       Operator.Eq,
       0,
-      Strength.strong
-    );
-  },
+      Strength.create(1, 0, 0, constraint.strength)
+    ),
 };
 
 /**
- * Layout constraint in X for minimising edge crossings
+ * Crossing constraint in X for minimising edge crossings
  */
 export const crossingConstraint = {
   property: 'x',
 
-  solve: (constraint, constants) => {
-    const { a, b, edgeA, edgeB } = constraint;
-    const difference = a.x - b.x;
-    const sourceDelta = edgeA.sourceNode.x - edgeB.sourceNode.x;
-    const targetDelta = edgeA.targetNode.x - edgeB.targetNode.x;
-    const target =
-      sourceDelta + targetDelta < 0 ? -constants.basisX : constants.basisX;
-
-    if (target >= 0 ? difference >= target : difference <= target) {
+  solve: (constraint) => {
+    const {
+      edgeA,
+      edgeB,
+      separationA,
+      separationB,
+      strength,
+    } = constraint;
+
+    const sourceSeparation = edgeA.sourceNode.x - edgeB.sourceNode.x;
+    const targetSeparation = edgeA.targetNode.x - edgeB.targetNode.x;
+
+    // Done if constraints are not crossing
+    if (sourceSeparation * targetSeparation < 0) {
       return;
     }
 
-    const strength = 1 / constants.basisX;
-
-    const resolve = strength * (difference - target);
-    a.x -= 0.5 * resolve;
-    b.x += 0.5 * resolve;
+    // Resolve larger separations more strongly
+    const resolveA =
+      strength * ((sourceSeparation - separationA) / separationA);
+    const resolveB =
+      strength * ((targetSeparation - separationB) / separationB);
+
+    // Choose the minimal solution that resolves crossing
+    if (Math.abs(resolveA) < Math.abs(resolveB)) {
+      edgeA.sourceNode.x -= resolveA;
+      edgeB.sourceNode.x += resolveA;
+    } else {
+      edgeA.targetNode.x -= resolveB;
+      edgeB.targetNode.x += resolveB;
+    }
   },
 };
 
@@ -130,26 +104,11 @@ export const crossingConstraint = {
 export const separationConstraint = {
   property: 'x',
 
-  solve: (constraint) => {
-    const { a, b } = constraint;
-    const difference = b.x - a.x;
-    const target = constraint.separation;
-
-    if (difference >= target) {
-      return;
-    }
-
-    const resolve = difference - target;
-    a.x += 0.5 * resolve;
-    b.x -= 0.5 * resolve;
-  },
-
-  strict: (constraint, constants, variableA, variableB) => {
-    return new Constraint(
+  strict: (constraint, constants, variableA, variableB) =>
+    new Constraint(
       variableB.minus(variableA),
       Operator.Ge,
       constraint.separation,
       Strength.required
-    );
-  },
+    ),
 };
diff --git a/src/utils/graph/graph.js b/src/utils/graph/graph.js
index dd6c931e9d..003db4c504 100644
--- a/src/utils/graph/graph.js
+++ b/src/utils/graph/graph.js
@@ -4,12 +4,12 @@ import { routing } from './routing';
 
 const defaultOptions = {
   layout: {
-    spaceX: 16,
+    spaceX: 14,
     spaceY: 110,
     layerSpaceY: 55,
-    basisX: 1500,
+    spreadX: 2.2,
     padding: 100,
-    iterations: 20,
+    iterations: 25,
   },
   routing: {
     spaceX: 26,
diff --git a/src/utils/graph/layout.js b/src/utils/graph/layout.js
index 204e03a4c8..e188d3afee 100644
--- a/src/utils/graph/layout.js
+++ b/src/utils/graph/layout.js
@@ -16,7 +16,7 @@ import {
  * @param {array} params.nodes The input nodes
  * @param {array} params.edges The input edges
  * @param {object=} params.layers The node layers if specified
- * @param {number} params.basisX The basis relating diagram width in X
+ * @param {number} params.spreadX The amount to adapt spacing in X
  * @param {number} params.spaceX The minimum gap between nodes in X
  * @param {number} params.spaceY The minimum gap between nodes in Y
  * @param {number} params.layerSpaceY The additional gap between nodes in Y between layers
@@ -27,7 +27,7 @@ export const layout = ({
   nodes,
   edges,
   layers,
-  basisX,
+  spreadX,
   spaceX,
   spaceY,
   layerSpaceY,
@@ -39,56 +39,36 @@ export const layout = ({
     node.y = 0;
   }
 
-  // Constants passed to solver
+  // Constants used by constraints
   const constants = {
     spaceX,
     spaceY,
-    basisX,
+    spreadX,
     layerSpace: (spaceY + layerSpaceY) * 0.5,
   };
 
-  // Constraints to separate nodes into rows
+  // Constraints to separate nodes into rows and layers
   const rowConstraints = createRowConstraints(edges);
-
-  // Constraints to separate nodes into layers
   const layerConstraints = createLayerConstraints(nodes, layers);
 
   // Find the node positions given these constraints
   solve([...rowConstraints, ...layerConstraints], constants, 1, true);
 
-  // Find the rows using the node positions after solving
+  // Find the solved rows using the node positions after solving
   const rows = groupByRow(nodes);
 
-  // Constraints to avoid edges crossing
-  const crossingConstraints = createCrossingConstraints(edges);
-
-  // Constraints to maintain parallel vertical edges
-  const {
-    parallelConstraints,
-    parallelSingleConstraints,
-    parallelDoubleConstraints,
-  } = createParallelConstraints(edges);
-
-  // Constraints to maintain a minimum horizontal node spacing
-  const separationConstraints = createSeparationConstraints(rows);
+  // Constraints to avoid edges crossing and maintain parallel vertical edges
+  const crossingConstraints = createCrossingConstraints(edges, constants);
+  const parallelConstraints = createParallelConstraints(edges, constants);
 
-  // Solve these constraints using multiple iterations
+  // Solve these constraints iteratively
   for (let i = 0; i < iterations; i += 1) {
-    // Solve main constraints
     solve(crossingConstraints, constants, 1);
-    solve(parallelConstraints, constants, 1);
-
-    // Further improve special cases with more effort
-    solve(parallelSingleConstraints, constants, iterations * 0.5);
-    solve(parallelDoubleConstraints, constants, iterations * 0.5);
-
-    // Update and solve separation constraints given the updated positions
-    updateSeparationConstraints(separationConstraints, rows, spaceX);
-    solve(separationConstraints, constants, iterations * 0.5);
+    solve(parallelConstraints, constants, 50);
   }
 
-  // Update separation constraints but ensure spacing is exact
-  updateSeparationConstraints(separationConstraints, rows, spaceX, true);
+  // Constraints to maintain a minimum horizontal node spacing
+  const separationConstraints = createSeparationConstraints(rows, constants);
 
   // Find the final node positions given these strict constraints
   solve([...separationConstraints, ...parallelConstraints], constants, 1, true);
@@ -161,39 +141,52 @@ const createLayerConstraints = (nodes, layers) => {
 /**
  * Creates crossing constraints for the given edges.
  * @param {array} edges The input edges
+ * @param {object} constants The constraint constants
+ * @param {number} constants.spaceX The minimum gap between nodes in X
  * @returns {array} The constraints
  */
-const createCrossingConstraints = (edges) => {
+const createCrossingConstraints = (edges, constants) => {
+  const { spaceX } = constants;
   const crossingConstraints = [];
 
   // For every pair of edges
   for (let i = 0; i < edges.length; i += 1) {
     const edgeA = edges[i];
+    const { sourceNode: sourceA, targetNode: targetA } = edgeA;
+
+    // Count the connected edges
+    const edgeADegree =
+      sourceA.sources.length +
+      sourceA.targets.length +
+      targetA.sources.length +
+      targetA.targets.length;
 
     for (let j = i + 1; j < edges.length; j += 1) {
       const edgeB = edges[j];
+      const { sourceNode: sourceB, targetNode: targetB } = edgeB;
 
-      // Add crossing constraint between edge source nodes, where different
-      if (edgeA.source !== edgeB.source) {
-        crossingConstraints.push({
-          base: crossingConstraint,
-          a: edgeA.sourceNode,
-          b: edgeB.sourceNode,
-          edgeA: edgeA,
-          edgeB: edgeB,
-        });
+      // Skip if edges are not intersecting by row so can't cross
+      if (sourceA.row >= targetB.row || targetA.row <= sourceB.row) {
+        continue;
       }
 
-      // Add crossing constraint between edge target nodes, where different
-      if (edgeA.target !== edgeB.target) {
-        crossingConstraints.push({
-          base: crossingConstraint,
-          a: edgeA.targetNode,
-          b: edgeB.targetNode,
-          edgeA: edgeA,
-          edgeB: edgeB,
-        });
-      }
+      // Count the connected edges
+      const edgeBDegree =
+        sourceB.sources.length +
+        sourceB.targets.length +
+        targetB.sources.length +
+        targetB.targets.length;
+
+      crossingConstraints.push({
+        base: crossingConstraint,
+        edgeA: edgeA,
+        edgeB: edgeB,
+        // The required horizontal spacing between connected nodes
+        separationA: sourceA.width * 0.5 + spaceX + sourceB.width * 0.5,
+        separationB: targetA.width * 0.5 + spaceX + targetB.width * 0.5,
+        // Evenly distribute the constraint
+        strength: 1 / Math.max(1, (edgeADegree + edgeBDegree) / 4),
+      });
     }
   }
 
@@ -208,117 +201,62 @@ const createCrossingConstraints = (edges) => {
  * @param {array} edges The input edges
  * @returns {object} An object containing the constraints
  */
-const createParallelConstraints = (edges) => {
-  const parallelConstraints = [];
-  const parallelSingleConstraints = [];
-  const parallelDoubleConstraints = [];
-
-  // For each edge
-  for (const edge of edges) {
-    // Constraint to keep it vertical and therefore parallel
-    const constraint = {
-      base: parallelConstraint,
-      a: edge.sourceNode,
-      b: edge.targetNode,
-    };
-
-    parallelConstraints.push(constraint);
-
-    // Identify special cases
-    const sourceHasOneTarget = edge.sourceNode.targets.length === 1;
-    const targetHasOneSource = edge.targetNode.sources.length === 1;
-
-    // Collect edges connected to single-degree nodes at either end
-    if (sourceHasOneTarget || targetHasOneSource) {
-      parallelSingleConstraints.push(constraint);
-    }
-
-    // Collect edges connected to single-degree nodes at both ends
-    if (sourceHasOneTarget && targetHasOneSource) {
-      parallelDoubleConstraints.push(constraint);
-    }
-  }
-
-  return {
-    parallelConstraints,
-    parallelSingleConstraints,
-    parallelDoubleConstraints,
-  };
-};
+const createParallelConstraints = (edges) =>
+  edges.map(({ sourceNode, targetNode }) => ({
+    base: parallelConstraint,
+    a: sourceNode,
+    b: targetNode,
+    // Evenly distribute the constraint
+    strength:
+      0.6 /
+      Math.max(1, sourceNode.targets.length + targetNode.sources.length - 2),
+  }));
 
 /**
- * Creates horizontal separation constraints for the given rows.
+ * Creates horizontal separation constraints for the given rows of nodes.
  * @param {array} rows The rows containing nodes
  * @returns {array} The constraints
  */
-const createSeparationConstraints = (rows) => {
+const createSeparationConstraints = (rows, constants) => {
+  const { spaceX } = constants;
   const separationConstraints = [];
 
-  // Constraints to maintain horizontal node separation
-  for (const rowNodes of rows) {
-    for (let j = 0; j < rowNodes.length - 1; j += 1) {
-      separationConstraints.push({
-        base: separationConstraint,
-        a: null,
-        b: null,
-      });
-    }
-  }
-
-  return separationConstraints;
-};
-
-/**
- * Updates horizontal separation constraints for the given rows.
- * @param {array} separationConstraints The constraints to update
- * @param {array} rows The rows containing nodes
- * @param {number} spaceX The desired separation in X
- * @returns {array} The constraints
- */
-const updateSeparationConstraints = (
-  separationConstraints,
-  rows,
-  spaceX,
-  snapped = false
-) => {
-  let k = 0;
-
-  // For each row
+  // For each row of nodes
   for (let l = 0; l < rows.length; l += 1) {
     const rowNodes = rows[l];
 
-    // Sort rows horizontally, breaks ties with ids for stability
+    // Stable sort row nodes horizontally, breaks ties with ids
     rowNodes.sort((a, b) => compare(a.x, b.x, a.id, b.id));
 
-    // Update constraints given updated row order
+    // Update constraints given updated row node order
     for (let j = 0; j < rowNodes.length - 1; j += 1) {
-      const constraint = separationConstraints[k];
-
-      // Update the constraint objects in order
-      constraint.a = rowNodes[j];
-      constraint.b = rowNodes[j + 1];
-
-      // Find the minimal required horizontal separation
-      const minSeparation =
-        spaceX + constraint.a.width * 0.5 + constraint.b.width * 0.5;
-
-      if (!snapped) {
-        // Use the minimal separation
-        constraint.separation = minSeparation;
-      } else {
-        // Find the current node horizontal separation
-        const separation = constraint.b.x - constraint.a.x;
-
-        // Snap the current horizontal separation to a unit amount
-        constraint.separation = Math.max(
-          snap(separation * 0.8, spaceX),
-          minSeparation
-        );
-      }
+      const nodeA = rowNodes[j];
+      const nodeB = rowNodes[j + 1];
+
+      // Count the connected edges
+      const degreeA = Math.max(
+        1,
+        nodeA.targets.length + nodeA.sources.length - 2
+      );
+      const degreeB = Math.max(
+        1,
+        nodeB.targets.length + nodeB.sources.length - 2
+      );
+
+      // Allow more spacing for nodes with more edges
+      const spread = Math.min(10, degreeA * degreeB * constants.spreadX);
+      const space = snap(spread * spaceX, spaceX);
 
-      k += 1;
+      separationConstraints.push({
+        base: separationConstraint,
+        a: nodeA,
+        b: nodeB,
+        separation: nodeA.width * 0.5 + space + nodeB.width * 0.5,
+      });
     }
   }
+
+  return separationConstraints;
 };
 
 /**
@@ -337,7 +275,7 @@ const expandDenseRows = (edges, rows, spaceY) => {
     const density = densities[i];
 
     // Snap to improve vertical rhythm
-    const offsetY = snap(density * spaceY, Math.round(spaceY * 0.25));
+    const offsetY = snap(density * 1.25 * spaceY, Math.round(spaceY * 0.25));
     currentOffsetY += offsetY;
 
     for (const node of rows[i + 1]) {

From fc06307ca3d14b44c8763d30ad2292ab06afcab5 Mon Sep 17 00:00:00 2001
From: bru5 <45365769+bru5@users.noreply.github.com>
Date: Wed, 31 Mar 2021 19:00:57 +0100
Subject: [PATCH 2/5] [KED-2528] Refactor and split graph solve functions to
 further simplify

---
 src/utils/graph/graph.test.js | 29 +++++++++--------------
 src/utils/graph/layout.js     | 10 ++++----
 src/utils/graph/solver.js     | 43 +++++++++++------------------------
 3 files changed, 29 insertions(+), 53 deletions(-)

diff --git a/src/utils/graph/graph.test.js b/src/utils/graph/graph.test.js
index 7f23a568af..cdc51ca4a3 100644
--- a/src/utils/graph/graph.test.js
+++ b/src/utils/graph/graph.test.js
@@ -4,7 +4,7 @@ import { getVisibleEdges } from '../../selectors/edges';
 import { getVisibleLayerIDs } from '../../selectors/disabled';
 import { Constraint, Operator, Strength } from 'kiwi.js';
 import { graph } from './graph';
-import { solve } from './solver';
+import { solveLoose, solveStrict } from './solver';
 
 import {
   clamp,
@@ -392,7 +392,7 @@ describe('solver', () => {
       },
     };
 
-    solve(
+    solveLoose(
       [
         constraintXA,
         constraintXB,
@@ -401,9 +401,7 @@ describe('solver', () => {
         constraintYB,
         constraintYC,
       ],
-      null,
-      8,
-      false
+      8
     );
 
     expect(Math.abs(testA.x - testB.x)).toBeCloseTo(5);
@@ -508,19 +506,14 @@ describe('solver', () => {
       },
     };
 
-    solve(
-      [
-        constraintXA,
-        constraintXB,
-        constraintXC,
-        constraintYA,
-        constraintYB,
-        constraintYC,
-      ],
-      null,
-      1,
-      true
-    );
+    solveStrict([
+      constraintXA,
+      constraintXB,
+      constraintXC,
+      constraintYA,
+      constraintYB,
+      constraintYC,
+    ]);
 
     expect(Math.abs(testA.x - testB.x)).toEqual(5);
     expect(Math.abs(testB.x - testC.x)).toBeGreaterThanOrEqual(8);
diff --git a/src/utils/graph/layout.js b/src/utils/graph/layout.js
index e188d3afee..d4c72f52aa 100644
--- a/src/utils/graph/layout.js
+++ b/src/utils/graph/layout.js
@@ -1,5 +1,5 @@
 import { halfPI, snap, angle, compare, groupByRow } from './common';
-import { solve } from './solver';
+import { solveLoose, solveStrict } from './solver';
 import {
   rowConstraint,
   layerConstraint,
@@ -52,7 +52,7 @@ export const layout = ({
   const layerConstraints = createLayerConstraints(nodes, layers);
 
   // Find the node positions given these constraints
-  solve([...rowConstraints, ...layerConstraints], constants, 1, true);
+  solveStrict([...rowConstraints, ...layerConstraints], constants, 1);
 
   // Find the solved rows using the node positions after solving
   const rows = groupByRow(nodes);
@@ -63,15 +63,15 @@ export const layout = ({
 
   // Solve these constraints iteratively
   for (let i = 0; i < iterations; i += 1) {
-    solve(crossingConstraints, constants, 1);
-    solve(parallelConstraints, constants, 50);
+    solveLoose(crossingConstraints, 1, constants);
+    solveLoose(parallelConstraints, 50, constants);
   }
 
   // Constraints to maintain a minimum horizontal node spacing
   const separationConstraints = createSeparationConstraints(rows, constants);
 
   // Find the final node positions given these strict constraints
-  solve([...separationConstraints, ...parallelConstraints], constants, 1, true);
+  solveStrict([...separationConstraints, ...parallelConstraints], constants, 1);
 
   // Adjust vertical spacing between rows for legibility
   expandDenseRows(edges, rows, spaceY);
diff --git a/src/utils/graph/solver.js b/src/utils/graph/solver.js
index 718f9d4af6..28e1fce46d 100644
--- a/src/utils/graph/solver.js
+++ b/src/utils/graph/solver.js
@@ -9,38 +9,15 @@
  **/
 import { Solver, Variable } from 'kiwi.js';
 
-/**
- * Applies the given constraints to the objects in-place.
- * If `strict` is set, limitations apply but an exact solution is attempted,
- * otherwise a solution is approximated iteratively
- * @param {array} constraints The constraints to apply
- * @param {object} constraint.a The first object to constrain
- * @param {object} constraint.b The second object to constrain
- * @param {string} constraint.base.property The property name on `a` and `b` to constrain
- * @param {?function} constraint.base.solve A function that solves the constraint in-place
- * @param {?function} constraint.base.strict A function returns the constraint in strict form
- * @param {?object} constants The constants used by constraints
- * @param {number=1} iterations The number of iterations
- * @param {boolean=false} strict
- */
-export const solve = (
-  constraints,
-  constants = {},
-  iterations = 1,
-  strict = false
-) => {
-  if (strict) return solveStrict(constraints, constants);
-  return solveLoose(constraints, constants, Math.ceil(iterations));
-};
-
 /**
  * Applies the given constraints to the objects in-place.
  * A solution is approximated iteratively.
- * @param {array} constraints The constraints. See docs for `solve`
- * @param {object} constants The constants used by constraints
+ * @param {array} constraints The constraints
+ * @param {function} constraint.base.solve A function that solves the constraint in-place
  * @param {number} iterations The number of iterations
+ * @param {?object} constants The constants used by constraints
  */
-const solveLoose = (constraints, constants, iterations) => {
+export const solveLoose = (constraints, iterations, constants) => {
   for (let i = 0; i < iterations; i += 1) {
     for (const co of constraints) {
       co.base.solve(co, constants);
@@ -51,10 +28,16 @@ const solveLoose = (constraints, constants, iterations) => {
 /**
  * Applies the given constraints to the objects in-place.
  * A solution is found exactly if possible, otherwise throws an error.
- * @param {array} constraints The constraints. See docs for `solve`
- * @param {object} constants The constants used by constraints
+ * @param {array} constraints The constraints
+ * @param {string} constraint.base.property The property name on `a` and `b` to constrain
+ * @param {function} constraint.base.strict A function returns the constraint in strict form
+ * @param {object} constraint.a The first object to constrain
+ * @param {object} constraint.b The second object to constrain
+ * @param {object} constraint.a.id A unique id for the first object
+ * @param {object} constraint.b.id A unique id for the second object
+ * @param {?object} constants The constants used by constraints
  */
-const solveStrict = (constraints, constants) => {
+export const solveStrict = (constraints, constants) => {
   const solver = new Solver();
   const variables = {};
 

From 124ae6ae96edd5b10c35132349fca377857f30de Mon Sep 17 00:00:00 2001
From: bru5 <45365769+bru5@users.noreply.github.com>
Date: Wed, 31 Mar 2021 21:20:00 +0100
Subject: [PATCH 3/5] [KED-2528] Resolve crossing constraints symmetrically

---
 src/utils/graph/constraints.js | 46 ++++++++++++----------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

diff --git a/src/utils/graph/constraints.js b/src/utils/graph/constraints.js
index 6b0e5ec157..c49bef4d9f 100644
--- a/src/utils/graph/constraints.js
+++ b/src/utils/graph/constraints.js
@@ -65,36 +65,22 @@ export const crossingConstraint = {
   property: 'x',
 
   solve: (constraint) => {
-    const {
-      edgeA,
-      edgeB,
-      separationA,
-      separationB,
-      strength,
-    } = constraint;
-
-    const sourceSeparation = edgeA.sourceNode.x - edgeB.sourceNode.x;
-    const targetSeparation = edgeA.targetNode.x - edgeB.targetNode.x;
-
-    // Done if constraints are not crossing
-    if (sourceSeparation * targetSeparation < 0) {
-      return;
-    }
-
-    // Resolve larger separations more strongly
-    const resolveA =
-      strength * ((sourceSeparation - separationA) / separationA);
-    const resolveB =
-      strength * ((targetSeparation - separationB) / separationB);
-
-    // Choose the minimal solution that resolves crossing
-    if (Math.abs(resolveA) < Math.abs(resolveB)) {
-      edgeA.sourceNode.x -= resolveA;
-      edgeB.sourceNode.x += resolveA;
-    } else {
-      edgeA.targetNode.x -= resolveB;
-      edgeB.targetNode.x += resolveB;
-    }
+    const { edgeA, edgeB, separationA, separationB, strength } = constraint;
+
+    // Amount to move each node towards required separation
+    const resolveSource =
+      strength *
+      ((edgeA.sourceNode.x - edgeB.sourceNode.x - separationA) / separationA);
+    
+    const resolveTarget =
+      strength *
+      ((edgeA.targetNode.x - edgeB.targetNode.x - separationB) / separationB);
+
+    // Apply the resolve each node
+    edgeA.sourceNode.x -= resolveSource;
+    edgeB.sourceNode.x += resolveSource;
+    edgeA.targetNode.x -= resolveTarget;
+    edgeB.targetNode.x += resolveTarget;
   },
 };
 

From 1201ee883af947f7dd4e92b690dcac64df08bbc9 Mon Sep 17 00:00:00 2001
From: bru5 <45365769+bru5@users.noreply.github.com>
Date: Wed, 31 Mar 2021 21:25:26 +0100
Subject: [PATCH 4/5] [KED-2528] Add more specific tests for layout constraints

---
 src/utils/graph/graph.test.js | 242 ++++++++++++++++++++++++++++++++++
 1 file changed, 242 insertions(+)

diff --git a/src/utils/graph/graph.test.js b/src/utils/graph/graph.test.js
index cdc51ca4a3..be8d9df480 100644
--- a/src/utils/graph/graph.test.js
+++ b/src/utils/graph/graph.test.js
@@ -5,6 +5,13 @@ import { getVisibleLayerIDs } from '../../selectors/disabled';
 import { Constraint, Operator, Strength } from 'kiwi.js';
 import { graph } from './graph';
 import { solveLoose, solveStrict } from './solver';
+import {
+  rowConstraint,
+  layerConstraint,
+  parallelConstraint,
+  crossingConstraint,
+  separationConstraint,
+} from './constraints';
 
 import {
   clamp,
@@ -290,6 +297,241 @@ describe('commmon', () => {
   });
 });
 
+describe('constraints', () => {
+  it('rowConstraint separates nodes in `y` in order with at least the given spaceY with strict solve', () => {
+    const spaceY = 10;
+
+    // Set up test nodes
+    const testA = { id: 0, x: 1, y: 0 };
+    const testB = { id: 1, x: 2, y: 0 };
+    const testC = { id: 2, x: 3, y: 0 };
+
+    // Set up test constraints
+    const rowConstraintAB = {
+      base: rowConstraint,
+      a: testB,
+      b: testA,
+    };
+
+    const rowConstraintBC = {
+      base: rowConstraint,
+      a: testC,
+      b: testB,
+    };
+
+    // Expect initial y values with no separation
+    expect(testB.y - testA.y).toBe(0);
+    expect(testC.y - testB.y).toBe(0);
+
+    // Solve test constraints
+    solveStrict([rowConstraintAB, rowConstraintBC], { spaceY });
+
+    // Expect order in y is A -> B -> C
+    expect(testA.y).toBeLessThan(testB.y);
+    expect(testB.y).toBeLessThan(testC.y);
+
+    // Expect y values have been separated by at least the expected amount and direction
+    expect(testB.y - testA.y).toBeGreaterThanOrEqual(spaceY);
+    expect(testC.y - testB.y).toBeGreaterThanOrEqual(spaceY);
+  });
+
+  it('layerConstraint separates nodes in `y` in order with at least the given layerSpace with strict solve', () => {
+    const layerSpace = 10;
+
+    // Set up test nodes
+    const testA = { id: 0, x: 1, y: 0 };
+    const testB = { id: 1, x: 2, y: 0 };
+    const testC = { id: 2, x: 3, y: 0 };
+
+    // Set up test constraints
+    const layerConstraintAB = {
+      base: layerConstraint,
+      a: testB,
+      b: testA,
+    };
+
+    const layerConstraintBC = {
+      base: layerConstraint,
+      a: testC,
+      b: testB,
+    };
+
+    // Expect initial y values have no separation
+    expect(testB.y - testA.y).toBe(0);
+    expect(testC.y - testB.y).toBe(0);
+
+    // Solve test constraints
+    solveStrict([layerConstraintAB, layerConstraintBC], { layerSpace });
+
+    // Expect order in y is A -> B -> C
+    expect(testA.y).toBeLessThan(testB.y);
+    expect(testB.y).toBeLessThan(testC.y);
+
+    // Expect y values have been separated by at least the expected amount and direction
+    expect(testB.y - testA.y).toBeGreaterThanOrEqual(layerSpace);
+    expect(testC.y - testB.y).toBeGreaterThanOrEqual(layerSpace);
+  });
+
+  it('parallelConstraint minimises nodes `x` separation to exactly 0 with strict solve', () => {
+    const initialSepration = 10;
+
+    // Set up test nodes
+    const testA = { id: 0, x: initialSepration, y: 1 };
+    const testB = { id: 1, x: initialSepration * 2, y: 2 };
+    const testC = { id: 2, x: initialSepration * 3, y: 3 };
+
+    // Set up test constraints
+    const parallelConstraintAB = {
+      base: parallelConstraint,
+      strength: 0.5,
+      a: testA,
+      b: testB,
+    };
+
+    const parallelConstraintBC = {
+      base: parallelConstraint,
+      strength: 0.5,
+      a: testB,
+      b: testC,
+    };
+
+    // Expect initial x values have some separation
+    expect(testB.x - testA.x).toBeGreaterThan(0);
+    expect(testC.x - testB.x).toBeGreaterThan(0);
+
+    // Solve test constraints
+    solveStrict([parallelConstraintAB, parallelConstraintBC]);
+
+    // Expect x value separation has been minimised to exactly 0
+    expect(Math.abs(testA.x - testB.x)).toEqual(0);
+    expect(Math.abs(testB.x - testC.x)).toEqual(0);
+  });
+
+  it('parallelConstraint minimises nodes `x` separation to near 0 with loose solve', () => {
+    const initialSepration = 10;
+
+    // Set up test nodes
+    const testA = { id: 0, x: initialSepration, y: 1 };
+    const testB = { id: 1, x: initialSepration * 2, y: 2 };
+    const testC = { id: 2, x: initialSepration * 3, y: 3 };
+
+    // Set up test constraints
+    const parallelConstraintAB = {
+      base: parallelConstraint,
+      strength: 0.5,
+      a: testA,
+      b: testB,
+    };
+
+    const parallelConstraintBC = {
+      base: parallelConstraint,
+      strength: 0.5,
+      a: testB,
+      b: testC,
+    };
+
+    // Expect initial x values have some separation
+    expect(testB.x - testA.x).toBeGreaterThan(0);
+    expect(testC.x - testB.x).toBeGreaterThan(0);
+
+    // Solve test constraints
+    solveLoose([parallelConstraintAB, parallelConstraintBC], 10);
+
+    // Expect x value separation has been minimised near to 0
+    expect(Math.abs(testA.x - testB.x)).toBeCloseTo(0);
+    expect(Math.abs(testB.x - testC.x)).toBeCloseTo(0);
+  });
+
+  it('separationConstraint separates nodes in `x` in order with at least the given separation with strict solve', () => {
+    const separation = 10;
+
+    // Set up test nodes
+    const testA = { id: 0, x: 0, y: 0 };
+    const testB = { id: 1, x: 0, y: 0 };
+    const testC = { id: 2, x: 0, y: 0 };
+
+    // Set up test constraints
+    const separationConstraintAB = {
+      base: separationConstraint,
+      a: testA,
+      b: testB,
+      separation,
+    };
+
+    const separationConstraintBC = {
+      base: separationConstraint,
+      a: testB,
+      b: testC,
+      separation,
+    };
+
+    // Expect initial x values have no separation
+    expect(testB.x - testA.x).toBe(0);
+    expect(testC.x - testB.x).toBe(0);
+
+    // Solve test constraints
+    solveStrict([separationConstraintAB, separationConstraintBC]);
+
+    // Expect order in x is A -> B -> C
+    expect(testA.x).toBeLessThan(testB.x);
+    expect(testB.x).toBeLessThan(testC.x);
+
+    // Expect x values have been separated by at least the expected amount and direction
+    expect(testB.x - testA.x).toBeGreaterThanOrEqual(separation);
+    expect(testC.x - testB.x).toBeGreaterThanOrEqual(separation);
+  });
+
+  it('crossingConstraint resolves crossing to given separation in `x` between two edges with loose solve', () => {
+    const separation = 10;
+
+    // Set up test edges such that they are crossing
+    const testEdgeA = {
+      sourceNode: { id: 0, x: -5, y: 0 },
+      targetNode: { id: 1, x: 5, y: 0 },
+    };
+
+    const testEdgeB = {
+      sourceNode: { id: 2, x: 10, y: 0 },
+      targetNode: { id: 3, x: -10, y: 0 },
+    };
+
+    // Set up test constraints
+    const crossingConstraintA = {
+      base: crossingConstraint,
+      edgeA: testEdgeA,
+      edgeB: testEdgeB,
+      strength: 0.9,
+      separationA: separation,
+      separationB: separation,
+    };
+
+    // Use the dot product to determine if edges cross in X
+    const isCrossing = (edgeA, edgeB) =>
+      (edgeA.sourceNode.x - edgeB.sourceNode.x) *
+        (edgeA.targetNode.x - edgeB.targetNode.x) <
+      0;
+    
+    // Expect edges to be initially crossing
+    expect(isCrossing(testEdgeA, testEdgeB)).toBe(true);
+
+    // Solve test constraints
+    solveLoose([crossingConstraintA], 50);
+
+    // Expect edges to no longer be crossing
+    expect(isCrossing(testEdgeA, testEdgeB)).toBe(false);
+
+    // Expect source nodes to be separated by close to the expected separation
+    expect(
+      Math.abs(testEdgeA.sourceNode.x - testEdgeB.sourceNode.x)
+    ).toBeGreaterThanOrEqual(separation * 0.99);
+
+    // Expect target nodes to be separated by close to the expected separation
+    expect(
+      Math.abs(testEdgeA.targetNode.x - testEdgeB.targetNode.x)
+    ).toBeGreaterThanOrEqual(separation * 0.99);
+  });
+});
+
 describe('solver', () => {
   it('solve finds a valid solution to given constraints (loose)', () => {
     const testA = { id: 0, x: 0, y: 0 };

From 13aeba76789af08d4fecbc7eb24ee9fd4b448267 Mon Sep 17 00:00:00 2001
From: bru5 <45365769+bru5@users.noreply.github.com>
Date: Thu, 1 Apr 2021 11:06:44 +0100
Subject: [PATCH 5/5] [KED-2528] Improve docs on graph layout functions

---
 src/utils/graph/layout.js | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/utils/graph/layout.js b/src/utils/graph/layout.js
index d4c72f52aa..221aece7d2 100644
--- a/src/utils/graph/layout.js
+++ b/src/utils/graph/layout.js
@@ -16,9 +16,9 @@ import {
  * @param {array} params.nodes The input nodes
  * @param {array} params.edges The input edges
  * @param {object=} params.layers The node layers if specified
- * @param {number} params.spreadX The amount to adapt spacing in X
  * @param {number} params.spaceX The minimum gap between nodes in X
  * @param {number} params.spaceY The minimum gap between nodes in Y
+ * @param {number} params.spreadX Adjusts the gap for each node in X based on the number of connected edges it has
  * @param {number} params.layerSpaceY The additional gap between nodes in Y between layers
  * @param {number} params.iterations The number of solver iterations to perform
  * @returns {void}
@@ -27,9 +27,9 @@ export const layout = ({
   nodes,
   edges,
   layers,
-  spreadX,
   spaceX,
   spaceY,
+  spreadX,
   layerSpaceY,
   iterations,
 }) => {
@@ -260,24 +260,28 @@ const createSeparationConstraints = (rows, constants) => {
 };
 
 /**
- * Adds additional spacing in Y for rows containing many crossing edges.
+ * Adds additional spacing in Y relative to row density, see function `rowDensity` for definition.
  * Node positions are updated in-place
  * @param {array} edges The input edges
  * @param {array} rows The input rows of nodes
- * @param {number} spaceY The minimum spacing between nodes in Y
+ * @param {number} spaceY The spacing between nodes in Y
+ * @param {number} [scale=1.25] The amount of expansion to apply relative to row density
+ * @param {number} [unit=0.25] The unit size for rounding expansion relative to spaceY
  */
-const expandDenseRows = (edges, rows, spaceY) => {
+const expandDenseRows = (edges, rows, spaceY, scale = 1.25, unit = 0.25) => {
   const densities = rowDensity(edges);
+  const spaceYUnit = Math.round(spaceY * unit);
   let currentOffsetY = 0;
 
-  // Add spacing based on density
+  // Add spacing based relative to row density
   for (let i = 0; i < densities.length; i += 1) {
     const density = densities[i];
 
-    // Snap to improve vertical rhythm
-    const offsetY = snap(density * 1.25 * spaceY, Math.round(spaceY * 0.25));
+    // Round offset to a common unit amount to improve vertical rhythm
+    const offsetY = snap(density * scale * spaceY, spaceYUnit);
     currentOffsetY += offsetY;
 
+    // Apply offset to all nodes following the current node
     for (const node of rows[i + 1]) {
       node.y += currentOffsetY;
     }
@@ -286,8 +290,9 @@ const expandDenseRows = (edges, rows, spaceY) => {
 
 /**
  * Estimates an average 'density' for each row based on average edge angle at that row.
- * Rows are decided by each edge's source and target node Y positions.
- * Intermediate rows are assumed always vertical as a simplification.
+ * Rows with edges close to horizontal are more 'dense' than rows with straight vertical edges.
+ * Rows are determined by each edge's source and target node Y positions.
+ * Intermediate row edges are assumed always vertical as a simplification, only the start end rows are measured.
  * Returns a list of values in `(0, 1)` where `0` means all edges on that row are vertical and `1` means all horizontal
  * @param {array} edges The input edges
  * @returns {array} The density of each row