From 26e32665a430ad75996031c96fcf5e040ff49fb6 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Tue, 29 Aug 2023 23:18:55 +0530 Subject: [PATCH 1/7] chore: Align edge markers properly in class --- packages/mermaid/src/dagre-wrapper/edges.js | 65 +++++++++++++++---- packages/mermaid/src/dagre-wrapper/index.js | 4 +- packages/mermaid/src/dagre-wrapper/markers.js | 38 +++++++---- .../src/dagre-wrapper/mermaid-graphlib.js | 4 +- 4 files changed, 85 insertions(+), 26 deletions(-) diff --git a/packages/mermaid/src/dagre-wrapper/edges.js b/packages/mermaid/src/dagre-wrapper/edges.js index 1581658b05..840c4effd5 100644 --- a/packages/mermaid/src/dagre-wrapper/edges.js +++ b/packages/mermaid/src/dagre-wrapper/edges.js @@ -368,7 +368,20 @@ const cutPathAtIntersect = (_points, boundryNode) => { return points; }; -//(edgePaths, e, edge, clusterDb, diagramtype, graph) +/** + * Calculate the deltas and angle between two points + * @param {{x: number, y:number}} point1 + * @param {{x: number, y:number}} point2 + * @returns {{angle: number, deltaX: number, deltaY: number}} + */ +function calculateDeltaAndAngle(point1, point2) { + const [x1, y1] = [point1.x, point1.y]; + const [x2, y2] = [point2.x, point2.y]; + const deltaX = x2 - x1; + const deltaY = y2 - y1; + return { angle: Math.atan(deltaY / deltaX), deltaX, deltaY }; +} + export const insertEdge = function (elem, e, edge, clusterDb, diagramType, graph) { let points = edge.points; let pointsHasChanged = false; @@ -435,22 +448,52 @@ export const insertEdge = function (elem, e, edge, clusterDb, diagramType, graph const lineData = points.filter((p) => !Number.isNaN(p.y)); // This is the accessor function we talked about above - let curve; + let curve = curveBasis; // Currently only flowcharts get the curve from the settings, perhaps this should // be expanded to a common setting? Restricting it for now in order not to cause side-effects that // have not been thought through - if (diagramType === 'graph' || diagramType === 'flowchart') { - curve = edge.curve || curveBasis; - } else { - curve = curveBasis; + if (edge.curve && (diagramType === 'graph' || diagramType === 'flowchart')) { + curve = edge.curve; } - // curve = curveLinear; + + const markerOffsets = { + aggregation: 18, + extension: 18, + composition: 18, + dependency: 6, + lollipop: 13.5, + }; + const lineFunction = line() - .x(function (d) { - return d.x; + .x(function (d, i, data) { + let offset = 0; + if (i === 0 && Object.hasOwn(markerOffsets, edge.arrowTypeStart)) { + const { angle, deltaX } = calculateDeltaAndAngle(data[0], data[1]); + offset = markerOffsets[edge.arrowTypeStart] * Math.cos(angle) * (deltaX >= 0 ? 1 : -1) || 0; + } else if (i === data.length - 1 && Object.hasOwn(markerOffsets, edge.arrowTypeEnd)) { + const { angle, deltaX } = calculateDeltaAndAngle( + data[data.length - 1], + data[data.length - 2] + ); + offset = markerOffsets[edge.arrowTypeEnd] * Math.cos(angle) * (deltaX >= 0 ? 1 : -1) || 0; + } + return d.x + offset; }) - .y(function (d) { - return d.y; + .y(function (d, i, data) { + let offset = 0; + if (i === 0 && Object.hasOwn(markerOffsets, edge.arrowTypeStart)) { + const { angle, deltaY } = calculateDeltaAndAngle(data[0], data[1]); + offset = + markerOffsets[edge.arrowTypeStart] * Math.abs(Math.sin(angle)) * (deltaY >= 0 ? 1 : -1); + } else if (i === data.length - 1 && Object.hasOwn(markerOffsets, edge.arrowTypeEnd)) { + const { angle, deltaY } = calculateDeltaAndAngle( + data[data.length - 1], + data[data.length - 2] + ); + offset = + markerOffsets[edge.arrowTypeEnd] * Math.abs(Math.sin(angle)) * (deltaY >= 0 ? 1 : -1); + } + return d.y + offset; }) .curve(curve); diff --git a/packages/mermaid/src/dagre-wrapper/index.js b/packages/mermaid/src/dagre-wrapper/index.js index 590242b029..28517d3407 100644 --- a/packages/mermaid/src/dagre-wrapper/index.js +++ b/packages/mermaid/src/dagre-wrapper/index.js @@ -155,9 +155,9 @@ export const render = async (elem, graph, markers, diagramtype, id) => { clearClusters(); clearGraphlib(); - log.warn('Graph at first:', graphlibJson.write(graph)); + log.warn('Graph at first:', structuredClone(graphlibJson.write(graph))); adjustClustersAndEdges(graph); - log.warn('Graph after:', graphlibJson.write(graph)); + log.warn('Graph after:', structuredClone(graphlibJson.write(graph))); // log.warn('Graph ever after:', graphlibJson.write(graph.node('A').graph)); await recursiveRender(elem, graph, diagramtype); }; diff --git a/packages/mermaid/src/dagre-wrapper/markers.js b/packages/mermaid/src/dagre-wrapper/markers.js index 57d092fdfe..ba6f08bb54 100644 --- a/packages/mermaid/src/dagre-wrapper/markers.js +++ b/packages/mermaid/src/dagre-wrapper/markers.js @@ -16,7 +16,7 @@ const extension = (elem, type, id) => { .append('marker') .attr('id', type + '-extensionStart') .attr('class', 'marker extension ' + type) - .attr('refX', 0) + .attr('refX', 18) .attr('refY', 7) .attr('markerWidth', 190) .attr('markerHeight', 240) @@ -29,7 +29,7 @@ const extension = (elem, type, id) => { .append('marker') .attr('id', type + '-extensionEnd') .attr('class', 'marker extension ' + type) - .attr('refX', 19) + .attr('refX', 1) .attr('refY', 7) .attr('markerWidth', 20) .attr('markerHeight', 28) @@ -44,7 +44,7 @@ const composition = (elem, type) => { .append('marker') .attr('id', type + '-compositionStart') .attr('class', 'marker composition ' + type) - .attr('refX', 0) + .attr('refX', 18) .attr('refY', 7) .attr('markerWidth', 190) .attr('markerHeight', 240) @@ -57,7 +57,7 @@ const composition = (elem, type) => { .append('marker') .attr('id', type + '-compositionEnd') .attr('class', 'marker composition ' + type) - .attr('refX', 19) + .attr('refX', 1) .attr('refY', 7) .attr('markerWidth', 20) .attr('markerHeight', 28) @@ -71,7 +71,7 @@ const aggregation = (elem, type) => { .append('marker') .attr('id', type + '-aggregationStart') .attr('class', 'marker aggregation ' + type) - .attr('refX', 0) + .attr('refX', 18) .attr('refY', 7) .attr('markerWidth', 190) .attr('markerHeight', 240) @@ -84,7 +84,7 @@ const aggregation = (elem, type) => { .append('marker') .attr('id', type + '-aggregationEnd') .attr('class', 'marker aggregation ' + type) - .attr('refX', 19) + .attr('refX', 1) .attr('refY', 7) .attr('markerWidth', 20) .attr('markerHeight', 28) @@ -98,7 +98,7 @@ const dependency = (elem, type) => { .append('marker') .attr('id', type + '-dependencyStart') .attr('class', 'marker dependency ' + type) - .attr('refX', 0) + .attr('refX', 6) .attr('refY', 7) .attr('markerWidth', 190) .attr('markerHeight', 240) @@ -111,7 +111,7 @@ const dependency = (elem, type) => { .append('marker') .attr('id', type + '-dependencyEnd') .attr('class', 'marker dependency ' + type) - .attr('refX', 19) + .attr('refX', 13) .attr('refY', 7) .attr('markerWidth', 20) .attr('markerHeight', 28) @@ -125,15 +125,31 @@ const lollipop = (elem, type) => { .append('marker') .attr('id', type + '-lollipopStart') .attr('class', 'marker lollipop ' + type) - .attr('refX', 0) + .attr('refX', 13) + .attr('refY', 7) + .attr('markerWidth', 190) + .attr('markerHeight', 240) + .attr('orient', 'auto') + .append('circle') + .attr('stroke', 'black') + .attr('fill', 'transparent') + .attr('cx', 7) + .attr('cy', 7) + .attr('r', 6); + elem + .append('defs') + .append('marker') + .attr('id', type + '-lollipopEnd') + .attr('class', 'marker lollipop ' + type) + .attr('refX', 1) .attr('refY', 7) .attr('markerWidth', 190) .attr('markerHeight', 240) .attr('orient', 'auto') .append('circle') .attr('stroke', 'black') - .attr('fill', 'white') - .attr('cx', 6) + .attr('fill', 'transparent') + .attr('cx', 7) .attr('cy', 7) .attr('r', 6); }; diff --git a/packages/mermaid/src/dagre-wrapper/mermaid-graphlib.js b/packages/mermaid/src/dagre-wrapper/mermaid-graphlib.js index 72ef969653..1e376054dd 100644 --- a/packages/mermaid/src/dagre-wrapper/mermaid-graphlib.js +++ b/packages/mermaid/src/dagre-wrapper/mermaid-graphlib.js @@ -291,8 +291,8 @@ export const adjustClustersAndEdges = (graph, depth) => { shape: 'labelRect', style: '', }); - const edge1 = JSON.parse(JSON.stringify(edge)); - const edge2 = JSON.parse(JSON.stringify(edge)); + const edge1 = structuredClone(edge); + const edge2 = structuredClone(edge); edge1.label = ''; edge1.arrowTypeEnd = 'none'; edge2.label = ''; From 7de1abbcc2f27c34c15fbd818524e19c9ad87d3e Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Tue, 29 Aug 2023 23:28:45 +0530 Subject: [PATCH 2/7] chore: Make extension arrow transparent --- packages/mermaid/src/diagrams/class/styles.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mermaid/src/diagrams/class/styles.js b/packages/mermaid/src/diagrams/class/styles.js index 15386bf9ef..9d08033353 100644 --- a/packages/mermaid/src/diagrams/class/styles.js +++ b/packages/mermaid/src/diagrams/class/styles.js @@ -109,13 +109,13 @@ g.classGroup line { } #extensionStart, .extension { - fill: ${options.mainBkg} !important; + fill: transparent !important; stroke: ${options.lineColor} !important; stroke-width: 1; } #extensionEnd, .extension { - fill: ${options.mainBkg} !important; + fill: transparent !important; stroke: ${options.lineColor} !important; stroke-width: 1; } From 58cb827839a5d76ce5402770d641cdfb8629ea5a Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Tue, 29 Aug 2023 23:32:10 +0530 Subject: [PATCH 3/7] chore: Remove structuredClone --- packages/mermaid/src/dagre-wrapper/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mermaid/src/dagre-wrapper/index.js b/packages/mermaid/src/dagre-wrapper/index.js index 28517d3407..279c5d9ddd 100644 --- a/packages/mermaid/src/dagre-wrapper/index.js +++ b/packages/mermaid/src/dagre-wrapper/index.js @@ -155,9 +155,9 @@ export const render = async (elem, graph, markers, diagramtype, id) => { clearClusters(); clearGraphlib(); - log.warn('Graph at first:', structuredClone(graphlibJson.write(graph))); + log.warn('Graph at first:', JSON.stringify(graphlibJson.write(graph))); adjustClustersAndEdges(graph); - log.warn('Graph after:', structuredClone(graphlibJson.write(graph))); + log.warn('Graph after:', JSON.stringify(graphlibJson.write(graph))); // log.warn('Graph ever after:', graphlibJson.write(graph.node('A').graph)); await recursiveRender(elem, graph, diagramtype); }; From f30a23f41eda82a0b7261b35e69e46191c239d36 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Tue, 29 Aug 2023 23:35:21 +0530 Subject: [PATCH 4/7] chore: Make aggregation arrow transparent --- packages/mermaid/src/diagrams/class/styles.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mermaid/src/diagrams/class/styles.js b/packages/mermaid/src/diagrams/class/styles.js index 9d08033353..f12f609f91 100644 --- a/packages/mermaid/src/diagrams/class/styles.js +++ b/packages/mermaid/src/diagrams/class/styles.js @@ -121,13 +121,13 @@ g.classGroup line { } #aggregationStart, .aggregation { - fill: ${options.mainBkg} !important; + fill: transparent !important; stroke: ${options.lineColor} !important; stroke-width: 1; } #aggregationEnd, .aggregation { - fill: ${options.mainBkg} !important; + fill: transparent !important; stroke: ${options.lineColor} !important; stroke-width: 1; } From cca96623654d6a3f75b7fc3a4a252f0a0021ec20 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Tue, 29 Aug 2023 23:49:00 +0530 Subject: [PATCH 5/7] chore: Add comments in edge handling --- packages/mermaid/src/dagre-wrapper/edges.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/mermaid/src/dagre-wrapper/edges.js b/packages/mermaid/src/dagre-wrapper/edges.js index 840c4effd5..ba95d1d4fe 100644 --- a/packages/mermaid/src/dagre-wrapper/edges.js +++ b/packages/mermaid/src/dagre-wrapper/edges.js @@ -456,6 +456,7 @@ export const insertEdge = function (elem, e, edge, clusterDb, diagramType, graph curve = edge.curve; } + // The offsets are calculated from the markers' dimensions. const markerOffsets = { aggregation: 18, extension: 18, @@ -468,9 +469,14 @@ export const insertEdge = function (elem, e, edge, clusterDb, diagramType, graph .x(function (d, i, data) { let offset = 0; if (i === 0 && Object.hasOwn(markerOffsets, edge.arrowTypeStart)) { + // Handle first point + // Calculate the angle and delta between the first two points const { angle, deltaX } = calculateDeltaAndAngle(data[0], data[1]); + // Calculate the offset based on the angle and the marker's dimensions offset = markerOffsets[edge.arrowTypeStart] * Math.cos(angle) * (deltaX >= 0 ? 1 : -1) || 0; } else if (i === data.length - 1 && Object.hasOwn(markerOffsets, edge.arrowTypeEnd)) { + // Handle last point + // Calculate the angle and delta between the last two points const { angle, deltaX } = calculateDeltaAndAngle( data[data.length - 1], data[data.length - 2] @@ -480,6 +486,7 @@ export const insertEdge = function (elem, e, edge, clusterDb, diagramType, graph return d.x + offset; }) .y(function (d, i, data) { + // Same handling as X above let offset = 0; if (i === 0 && Object.hasOwn(markerOffsets, edge.arrowTypeStart)) { const { angle, deltaY } = calculateDeltaAndAngle(data[0], data[1]); From bbbae7d59fc212465ac5c25371000622a5a7a063 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Wed, 30 Aug 2023 05:19:12 +0000 Subject: [PATCH 6/7] Apply suggestions from code review Co-authored-by: Alois Klink --- packages/mermaid/src/dagre-wrapper/edges.js | 2 ++ packages/mermaid/src/dagre-wrapper/markers.js | 1 + 2 files changed, 3 insertions(+) diff --git a/packages/mermaid/src/dagre-wrapper/edges.js b/packages/mermaid/src/dagre-wrapper/edges.js index ba95d1d4fe..2f5b2568c4 100644 --- a/packages/mermaid/src/dagre-wrapper/edges.js +++ b/packages/mermaid/src/dagre-wrapper/edges.js @@ -456,6 +456,8 @@ export const insertEdge = function (elem, e, edge, clusterDb, diagramType, graph curve = edge.curve; } + // We need to draw the lines a bit shorter to avoid drawing + // under any transparent markers. // The offsets are calculated from the markers' dimensions. const markerOffsets = { aggregation: 18, diff --git a/packages/mermaid/src/dagre-wrapper/markers.js b/packages/mermaid/src/dagre-wrapper/markers.js index ba6f08bb54..c7d06f1d4e 100644 --- a/packages/mermaid/src/dagre-wrapper/markers.js +++ b/packages/mermaid/src/dagre-wrapper/markers.js @@ -136,6 +136,7 @@ const lollipop = (elem, type) => { .attr('cx', 7) .attr('cy', 7) .attr('r', 6); + elem .append('defs') .append('marker') From 5b724b180faa4c593a1f5ea6f68b9c81f4dc5ac3 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Sun, 3 Sep 2023 03:02:58 +0530 Subject: [PATCH 7/7] chore: Fix flowchart arrow --- packages/mermaid/src/dagre-wrapper/edges.js | 1 + packages/mermaid/src/dagre-wrapper/markers.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/mermaid/src/dagre-wrapper/edges.js b/packages/mermaid/src/dagre-wrapper/edges.js index 2f5b2568c4..f89b4422be 100644 --- a/packages/mermaid/src/dagre-wrapper/edges.js +++ b/packages/mermaid/src/dagre-wrapper/edges.js @@ -465,6 +465,7 @@ export const insertEdge = function (elem, e, edge, clusterDb, diagramType, graph composition: 18, dependency: 6, lollipop: 13.5, + arrow_point: 5.3, }; const lineFunction = line() diff --git a/packages/mermaid/src/dagre-wrapper/markers.js b/packages/mermaid/src/dagre-wrapper/markers.js index c7d06f1d4e..051c987f62 100644 --- a/packages/mermaid/src/dagre-wrapper/markers.js +++ b/packages/mermaid/src/dagre-wrapper/markers.js @@ -160,7 +160,7 @@ const point = (elem, type) => { .attr('id', type + '-pointEnd') .attr('class', 'marker ' + type) .attr('viewBox', '0 0 10 10') - .attr('refX', 10) + .attr('refX', 6) .attr('refY', 5) .attr('markerUnits', 'userSpaceOnUse') .attr('markerWidth', 12)