From 02cdbac377148fa2c71eb721d0a1bbe36ce4056f Mon Sep 17 00:00:00 2001 From: CTomlyn Date: Thu, 26 Sep 2024 04:35:09 -0700 Subject: [PATCH] #2162 Fix vertical links crossing over each other (#2163) Signed-off-by: CTomlyn Co-authored-by: Matt Howard --- .../src/common-canvas/common-canvas-utils.js | 17 +++ .../src/common-canvas/svg-canvas-renderer.js | 128 +++++++++++------- .../common-canvas/svg-canvas-utils-links.js | 1 + .../harness/cypress/e2e/canvas/links.cy.js | 9 +- .../cypress/e2e/canvas/nodes-resize.cy.js | 32 ++--- 5 files changed, 119 insertions(+), 68 deletions(-) diff --git a/canvas_modules/common-canvas/src/common-canvas/common-canvas-utils.js b/canvas_modules/common-canvas/src/common-canvas/common-canvas-utils.js index 1941c0a375..290038890b 100644 --- a/canvas_modules/common-canvas/src/common-canvas/common-canvas-utils.js +++ b/canvas_modules/common-canvas/src/common-canvas/common-canvas-utils.js @@ -364,6 +364,23 @@ export default class CanvasUtils { return { x: startPointX, y: startPointY, originX, originY, dir }; } + // Assisted by WCA@IBM + // Latest GenAI contribution: ibm/granite-20b-code-instruct-v2 + // Returns the angle between two points where the angle + // returned is always positive. The angle starts at the 3 o'clock + // position which is 0 degrees and increases in a clock-wise + // direction. + static calculateAngle(x1, y1, x2, y2) { + const dx = x2 - x1; + const dy = y2 - y1; + const angle = Math.atan2(dy, dx); + let angleInDegrees = angle * (180 / Math.PI); + if (angleInDegrees < 0) { + angleInDegrees += 360; + } + return angleInDegrees; + } + // Returns a direction NORTH, SOUTH, EAST or WEST which is the direction // from the origin position within the rectangle described by x, y, w and h // to the end position described by endX and endY. diff --git a/canvas_modules/common-canvas/src/common-canvas/svg-canvas-renderer.js b/canvas_modules/common-canvas/src/common-canvas/svg-canvas-renderer.js index aa7f0c6aae..4001522d8f 100644 --- a/canvas_modules/common-canvas/src/common-canvas/svg-canvas-renderer.js +++ b/canvas_modules/common-canvas/src/common-canvas/svg-canvas-renderer.js @@ -5040,7 +5040,7 @@ export default class SVGCanvasRenderer { // Updates the data links for all the nodes with two optional fields // (called srcFreeformInfo and trgFreeformInfo) based on the location of the // nodes the links go from and to. The info in these fields is used to - // calculate the starting and ending position of freeform line links. + // calculate the starting and ending position of freeform link lines. // This ensures that input and output links that go in a certain direction // (NORTH, SOUTH, EAST or WEST) are grouped together so they can be // separated out when freeform lines are drawn between nodes. @@ -5063,13 +5063,13 @@ export default class SVGCanvasRenderer { // Self-referencing link if (node.id === link.srcObj?.id && link.srcObj?.id === link.trgNode?.id) { - linksInfo[NORTH].push({ type: "in", endNode: link.srcObj, link }); - linksInfo[EAST].push({ type: "out", endNode: link.trgNode, link }); + linksInfo[NORTH].push({ type: "in", startNode: link.srcObj, endNode: link.trgNode, link }); + linksInfo[EAST].push({ type: "out", startNode: link.srcObj, endNode: link.trgNode, link }); } else if (link.trgNode && link.trgNode.id === node.id) { if (link.srcObj) { - const dir = this.getDirAdjusted(link.trgNode, link.srcObj); - linksInfo[dir].push({ type: "in", endNode: link.srcObj, link }); + const dir = this.getDirToNode(link.trgNode, link.srcObj); + linksInfo[dir].push({ type: "in", startNode: link.trgNode, endNode: link.srcObj, link }); } else if (link.srcPos) { const dir = this.getDirToEndPos(link.trgNode, link.srcPos.x_pos, link.srcPos.y_pos); @@ -5078,8 +5078,8 @@ export default class SVGCanvasRenderer { } else if (link.srcObj && link.srcObj.id === node.id) { if (link.trgNode) { - const dir = this.getDirAdjusted(link.srcObj, link.trgNode); - linksInfo[dir].push({ type: "out", endNode: link.trgNode, link }); + const dir = this.getDirToNode(link.srcObj, link.trgNode); + linksInfo[dir].push({ type: "out", startNode: link.srcObj, endNode: link.trgNode, link }); } else if (link.trgPos) { const dir = this.getDirToEndPos(link.srcObj, link.trgPos.x_pos, link.trgPos.y_pos); @@ -5089,10 +5089,15 @@ export default class SVGCanvasRenderer { } }); - linksInfo.n = this.sortLinksInfo(linksInfo.n, NORTH); - linksInfo.s = this.sortLinksInfo(linksInfo.s, SOUTH); - linksInfo.e = this.sortLinksInfo(linksInfo.e, EAST); - linksInfo.w = this.sortLinksInfo(linksInfo.w, WEST); + const startCenter = { + x: node.x_pos + node.width / 2, + y: node.y_pos + node.height / 2 + }; + + linksInfo.n = this.sortLinksInfo(linksInfo.n, NORTH, startCenter); + linksInfo.s = this.sortLinksInfo(linksInfo.s, SOUTH, startCenter); + linksInfo.e = this.sortLinksInfo(linksInfo.e, EAST, startCenter); + linksInfo.w = this.sortLinksInfo(linksInfo.w, WEST, startCenter); this.updateLinksInfo(linksInfo.n, NORTH); this.updateLinksInfo(linksInfo.s, SOUTH); @@ -5100,31 +5105,6 @@ export default class SVGCanvasRenderer { this.updateLinksInfo(linksInfo.w, WEST); } - // Returns the direction of a link from the start node to the end node - // as either NORTH, SOUTH, EAST or WEST. Some direction combinations - // have to be overriden to prevent link lines overlapping. - getDirAdjusted(startNode, endNode) { - let dir = this.getDirToNode(startNode, endNode); - - // When start -> end is SOUTH and end -> start is WEST the returned direction - // becomes EAST instead of SOUTH to prevent link lines overlapping. - if (dir === SOUTH) { - const dir2 = this.getDirToNode(endNode, startNode); - if (dir2 === WEST) { - dir = EAST; - } - - // When start -> end is NORTH and end -> start is EAST the returned direction - // becomes WEST instead of NORTH to prevent link lines overlapping. - } else if (dir === NORTH) { - const dir2 = this.getDirToNode(endNode, startNode); - if (dir2 === EAST) { - dir = WEST; - } - } - return dir; - } - // Returns the direction (NORTH, SOUTH, EAST or WEST) from the start node // to the end node. getDirToNode(startNode, endNode) { @@ -5148,10 +5128,13 @@ export default class SVGCanvasRenderer { } // Returns the linksDirArray passed in with the linkInfo objects in the - // array ordered by the position of the end of each link line, depending on - // the direction (dir) of the lines. This is achieved by spliting the links - // into groups where links in the same group go to/from the same node. - sortLinksInfo(linksDirArrayIn, dir) { + // array ordered by the angle that each link makes with the center of the + // start node. When handling multiple links that go to the same node + // the links have to be grouped where links in the same group go to/from + // the same node. For groups, the x and y are *projected* corrdinates to + // allow us to calculate the angles and ordering etc. The actual x and y + // for the links is calculated in getAttachedLinkObj and getDetachedLinkObj. + sortLinksInfo(linksDirArrayIn, dir, startCenter) { let linksDirArray = linksDirArrayIn; if (linksDirArray.length > 1) { const groups = this.getLinkInfoGroups(linksDirArray); @@ -5168,23 +5151,72 @@ export default class SVGCanvasRenderer { li.x = this.nodeUtils.getNodeCenterPosX(node); li.y = node.y_pos + ((node.height / parts) * (i + 1)); } + // Special case where links that go SOUTH from the node and + // point to the WEST of the end node, get crossed over each other. + // In this case the x coordinates of the link items need to be + // reversed. + if (dir === SOUTH) { + const reverseDir = this.getDirToNode(li.endNode, li.startNode); + if (reverseDir === WEST) { + li.x = node.x_pos + ((node.width / parts) * (group.length - i)); + } + } + // Special case where links that go NORTH from the node and + // point to the EAST of the end node, get crossed over each other. + // In this case the x coordinates of the link items need to be + // reversed. + if (dir === NORTH) { + const reverseDir = this.getDirToNode(li.endNode, li.startNode); + if (reverseDir === EAST) { + li.x = node.x_pos + ((node.width / parts) * (group.length - i)); + } + } }); }); - // For NORTH and SOUTH links we sort linksDirArray by the x coordinate - // of the end of each link. For EAST and WEST we sort by the y - // coordinate. - if (dir === NORTH || dir === SOUTH) { - linksDirArray = linksDirArray.sort((a, b) => (a.x > b.x ? 1 : -1)); + // Set an angle for each linkDir so that they can be sorted so they do not + // overlap when being drawn to or from the node. The angle is from the + // center of the node we are handling to their projected end point. + linksDirArray.forEach((ld) => { + ld.angle = CanvasUtils.calculateAngle(startCenter.x, startCenter.y, ld.x, ld.y); + + // Make sure the angles for links on the EAST side of the node are + // increasing in the clockwise direction by decrementing the angles from + // 270 to 360 by 360 degrees. (This is because calculateAngle returns + // positive angles from the 3 o'clock position in clockwise direction.) + if (dir === EAST && ld.angle >= 270) { + ld.angle -= 360; + } + + // For self-referencing links we overwrite the angle to ensure that + // the outward direction (EAST) is always drawn at the top of any + // EAST links and the inward direction (NORTH) is always drawn to + // the right of any NORTH links. + if (ld.startNode && ld.endNode && ld.startNode === ld.endNode) { + if (dir === EAST) { + ld.angle = -90; + } else if (dir === NORTH) { + ld.angle = 360; + } + } + }); + + // Sort the linksDirArray by the angle that each link forms with the + // center of the start node. + if (dir === NORTH || dir === EAST) { + linksDirArray = linksDirArray.sort((a, b) => (a.angle > b.angle ? 1 : -1)); } else { - linksDirArray = linksDirArray.sort((a, b) => (a.y > b.y ? 1 : -1)); + linksDirArray = linksDirArray.sort((a, b) => (a.angle < b.angle ? 1 : -1)); } } return linksDirArray; } - // Returns a 'groups' object where each field is index by a node ID and - // contains an array of linkInfo objects that go to/from the node. + // Returns a 'groups' object where each field is indexed by a node ID and + // contains an array of linkInfo objects that go to/from the node. Note: + // endNode is the target node for link that point away from the node we + // are handling and is the source node for links the point to the node + // we are handling. getLinkInfoGroups(linksDirArray) { const groups = {}; linksDirArray.forEach((li) => { diff --git a/canvas_modules/common-canvas/src/common-canvas/svg-canvas-utils-links.js b/canvas_modules/common-canvas/src/common-canvas/svg-canvas-utils-links.js index d72f075c39..560d5d8b54 100644 --- a/canvas_modules/common-canvas/src/common-canvas/svg-canvas-utils-links.js +++ b/canvas_modules/common-canvas/src/common-canvas/svg-canvas-utils-links.js @@ -732,6 +732,7 @@ export default class SvgCanvasLinks { const path = "M " + link.x1 + " " + link.y1 + " L " + + rightInc + " " + link.y1 + " " + rightInc + " " + topInc + " " + link.x2 + " " + topInc + " " + link.x2 + " " + link.y2; diff --git a/canvas_modules/harness/cypress/e2e/canvas/links.cy.js b/canvas_modules/harness/cypress/e2e/canvas/links.cy.js index 77d63edfd9..1a044fc5ce 100644 --- a/canvas_modules/harness/cypress/e2e/canvas/links.cy.js +++ b/canvas_modules/harness/cypress/e2e/canvas/links.cy.js @@ -214,11 +214,12 @@ describe("Test basic link construction", function() { "M 367 167.5 L 397 167.5 L 397 118.5 L 267 118.5 L 267 167.5 L 297 167.5" ); - cy.setCanvasConfig({ "selectedLinkType": "Straight", "selectedLinkMethod": "Ports" }); + cy.setCanvasConfig({ "selectedLinkType": "Straight", "selectedLinkMethod": "Ports", + "selectedStraightLinksAsFreeform": false }); cy.wait(10); cy.verifyLinkPath( "Execution node", "outPort", "Execution node", "inPort", - "M 374 176 L 404 101.5 332 101.5 332 131.5" + "M 367 167 L 397 118 L 267 118.5 L 267 167.5 L 297 167.5" ); // Test the 4 Freeform combinations @@ -226,7 +227,7 @@ describe("Test basic link construction", function() { cy.setCanvasConfig({ "selectedLinkType": "Curve", "selectedLinkMethod": "Freeform" }); cy.verifyLinkPath( "Execution node", "outPort", "Execution node", "inPort", - "M 374 176 L 404 101.5 332 101.5 332 131.5" + "M 367 167.5 L 397 118.5 L 267 118.5 L 267 167.5 L 297 167.5" ); cy.setCanvasConfig({ "selectedLinkType": "Elbow", "selectedLinkMethod": "Freeform" }); @@ -248,7 +249,7 @@ describe("Test basic link construction", function() { cy.wait(10); cy.verifyLinkPath( "Execution node", "outPort", "Execution node", "inPort", - "M 374 176 L 404 101.5 332 101.5 332 131.5" + "M 374 176 L 404 176 404 101.5 332 101.5 332 131.5" ); }); }); diff --git a/canvas_modules/harness/cypress/e2e/canvas/nodes-resize.cy.js b/canvas_modules/harness/cypress/e2e/canvas/nodes-resize.cy.js index f3b25fc30d..f859739036 100644 --- a/canvas_modules/harness/cypress/e2e/canvas/nodes-resize.cy.js +++ b/canvas_modules/harness/cypress/e2e/canvas/nodes-resize.cy.js @@ -85,28 +85,28 @@ describe("Test adding resizing nodes", function() { // Test the two links from Node 2 to Node 1 cy.verifyLinkPath("Node 2", "outPort1", "Node 1", "inPort", - "M 231.62981184143024 188.49662691354752 L 457.8811607037008 454.3276824951172"); + "M 239 188 L 457 454"); cy.verifyLinkPath("Node 2", "outPort2", "Node 1", "inPort", - "M 191.06748102181547 188.49662691354752 L 435.2596130371094 463.8410519984919"); + "M 183 188 L 435 465"); // Test the one link from Node 1 to Node 3 cy.verifyLinkPath("Node 1", "outPort", "Node 3", "inPort", - "M 435.2596130371094 536.74108289395 L 153.09522247314453 414.152362905051"); + "M 435 536 L 153 414"); // Test the one link from Node 1 to Node 4 cy.verifyLinkPath("Node 1", "outPort", "Node 4", "inPort", - "M 549.2596130371094 501.21871949140086 L 713.8162978100015 327.1740475337241"); + "M 549 501 L 713 327"); // Test the two detached links pointing away from Node 1 cy.verifyDetachedLinkPathFromSource("Node 1", "outPort", 2, [ - "M 527.9181758114686 454.3276824951172 L 728 134", - "M 435.2596130371094 642.5937664374292 L 338 667" + "M 527 454 L 728 134", + "M 435 642 L 338 667" ]); // Test the two detached links pointing at Node 1 cy.verifyDetachedLinkPathToTarget("Node 1", "inPort", 2, [ - "M 764 644 L 549.2596130371094 618.3914396565383", - "M 158 644 L 435.2596130371094 603.2709500023171" + "M 764 644 L 549 618", + "M 158 644 L 435 603" ]); }); @@ -144,28 +144,28 @@ function verifyOnOpen() { // Test the two links from Node 2 to Node 1 cy.verifyLinkPath("Node 2", "outPort1", "Node 1", "inPort", - "M 242.630126953125 132.52344837411354 L 435.2596130371094 228.94234778966086"); + "M 242 139 L 435 230"); cy.verifyLinkPath("Node 2", "outPort2", "Node 1", "inPort", - "M 237.00137409101524 188.49662691354752 L 435.2596130371094 305.67435851625555"); + "M 242 188 L 435 305"); // Test the one link from Node 1 to Node 3 cy.verifyLinkPath("Node 1", "outPort", "Node 3", "inPort", - "M 435.2596130371094 414.5440487350059 L 153.09522247314453 398.32003960221454"); + "M 435 414 L 153 398"); // Test the one link from Node 1 to Node 4 cy.verifyLinkPath("Node 1", "outPort", "Node 4", "inPort", - "M 538.2596130371094 391.7122543839917 L 713.8162978100015 303.78745532835455"); + "M 538 391 L 713 303"); // Test the two detached links pointing away from Node 1 cy.verifyDetachedLinkPathFromSource("Node 1", "outPort", 2, [ - "M 538.2596130371094 260.9572649119533 L 728 134", - "M 435.2596130371094 610.3088228810263 L 338 667" + "M 538 260 L 728 134", + "M 435 610 L 338 667" ]); // Test the two detached links pointing at Node 1 cy.verifyDetachedLinkPathToTarget("Node 1", "inPort", 2, [ - "M 764 644 L 538.2596130371094 558.9883811067392", - "M 158 644 L 435.2596130371094 521.6279149026254" + "M 764 644 L 538 558.9883811067392", + "M 158 644 L 435 521" ]); }