You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2021/09/23 18:34:44 UTC
[GitHub] [echarts] 100pah commented on a change in pull request #15405: fix:node self pointed -add cubicBezierCurve
100pah commented on a change in pull request #15405:
URL: https://github.com/apache/echarts/pull/15405#discussion_r708867851
##########
File path: src/chart/graph/edgeVisual.ts
##########
@@ -75,6 +79,26 @@ export default function graphEdgeVisual(ecModel: GlobalModel) {
symbolType[1] && edge.setVisual('toSymbol', symbolType[1]);
symbolSize[0] && edge.setVisual('fromSymbolSize', symbolSize[0]);
symbolSize[1] && edge.setVisual('toSymbolSize', symbolSize[1]);
+
+
+ if (edge.node1 === edge.node2 && toSymbol && toSymbol !== 'none') {
+ const edgeData = edge.getLayout();
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2;
+
+ let t = intersectCurveCircle(edgeData, edgeData[0], radius);
+ if (t < 0.5) {
+ t = 1 - t;
+ }
+ const tdx = cubicDerivativeAt(edgeData[0][0], edgeData[1][0], edgeData[2][0], edgeData[3][0], t);
+ const tdy = cubicDerivativeAt(edgeData[0][1], edgeData[1][1], edgeData[2][1], edgeData[3][1], t);
+ const degree = Math.atan2(tdy, tdx) / Math.PI * 180;
+ if( degree > 90 || degree < 0 && degree > -90) {
Review comment:
eslint error
##########
File path: test/force.html
##########
@@ -92,7 +91,7 @@
edges: item.edges.map(function (e) {
return {
source: e[0],
- target: e[1]
+ target: e[0]
Review comment:
I think we'd better not to change the original force case,
otherwise the previous force case does not exits any more.
We could create a new force case in `test/graphSimple.html`.
##########
File path: src/chart/graph/forceLayout.ts
##########
@@ -144,7 +146,80 @@ export default function graphForceLayout(ecModel: GlobalModel) {
points[1] = points[1] || [];
vec2.copy(points[0], p1);
vec2.copy(points[1], p2);
- if (+e.curveness) {
+ if (e.n1 === e.n2) {
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(graphSeries) * size / 2;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
Review comment:
Should not be `any` if we known the type.
##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -88,16 +89,82 @@ export function circularLayout(
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
let cp1;
+ let cp2;
const x12 = (p1[0] + p2[0]) / 2;
const y12 = (p1[1] + p2[1]) / 2;
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
Review comment:
I think the entire logic below (find the proper direction for the self-loop edge) or part of them should be put in a separate helper method and shared by `circularLayoutHelper`, `simpleLayoutHelper` and `forceLayoutHelper`, rather than repeat them.
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -49,7 +69,80 @@ export function simpleLayoutEdge(graph: Graph, seriesModel: GraphSeriesModel) {
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
const points = [p1, p2];
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const curve = getCurvenessForEdge(edge, seriesModel, index, true);
+ const curveness = curve >= 1 ? curve : 1 - curve;
+ const symbolSize = seriesModel.get('symbolSize');
+ const size = zrUtil.isArray(symbolSize) ? Number((symbolSize[0] + symbolSize[1]) / 2) : Number(symbolSize);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2 * curveness;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
+ const v: any[] = [];
+ vec2.sub(v, node.getLayout(), edge.node1.getLayout());
+ vec2.normalize(v, v);
+ vectors.push(v);
+ });
+ // find the max angle
+ for (let i = 0; i < vectors.length; i++) {
+ for (let j = i + 1; j < vectors.length; j++) {
+ if (vec2.distSquare(vectors[i], vectors[j]) > d) {
+ d = vec2.distSquare(vectors[i], vectors[j]);
+ pt1 = vectors[i];
+ pt2 = vectors[j];
+ }
+ }
+ }
+ // if the angle is more than sixty degree
Review comment:
Math.sqrt(3) is 120 degree rather than sixty degree?
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -49,7 +69,80 @@ export function simpleLayoutEdge(graph: Graph, seriesModel: GraphSeriesModel) {
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
const points = [p1, p2];
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const curve = getCurvenessForEdge(edge, seriesModel, index, true);
+ const curveness = curve >= 1 ? curve : 1 - curve;
+ const symbolSize = seriesModel.get('symbolSize');
+ const size = zrUtil.isArray(symbolSize) ? Number((symbolSize[0] + symbolSize[1]) / 2) : Number(symbolSize);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2 * curveness;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
+ const v: any[] = [];
+ vec2.sub(v, node.getLayout(), edge.node1.getLayout());
+ vec2.normalize(v, v);
+ vectors.push(v);
+ });
+ // find the max angle
+ for (let i = 0; i < vectors.length; i++) {
+ for (let j = i + 1; j < vectors.length; j++) {
+ if (vec2.distSquare(vectors[i], vectors[j]) > d) {
+ d = vec2.distSquare(vectors[i], vectors[j]);
+ pt1 = vectors[i];
+ pt2 = vectors[j];
+ }
+ }
+ }
Review comment:
I think we need to find `the max angle of two adjacent edges` rather than `the max angle of any two edges`.
The latter one probably make the self-loop edge intersect with some other edge between the found edges.
<img width="442" alt="Screen Shot 2021-09-15 at 9 37 35 PM" src="https://user-images.githubusercontent.com/1956569/133443745-333a18a0-e2c5-4ae0-970d-a1bf8f7f4fa9.png">
And I think we should better take into account every edges rather than only every nodes:
<img width="437" alt="Screen Shot 2021-09-16 at 10 21 32 PM" src="https://user-images.githubusercontent.com/1956569/133629659-20565517-a394-4a3b-94ca-f48c7fdbccb4.png">
And when there are more than one self-loop edges, the strategy probably complicated to make it look good. For example, consider these cases:
<img width="294" alt="Screen Shot 2021-09-16 at 10 29 58 PM" src="https://user-images.githubusercontent.com/1956569/133634612-a346a3d5-423c-457c-bf31-8372d66bf052.png">
<img width="287" alt="Screen Shot 2021-09-16 at 10 35 54 PM" src="https://user-images.githubusercontent.com/1956569/133634670-a07afb9f-0d81-4118-9ef8-e459db13d02e.png">
Some other cases:
<img width="248" alt="Screen Shot 2021-09-21 at 12 26 32 AM" src="https://user-images.githubusercontent.com/1956569/134038784-e6dd8435-7be9-4c5c-858e-163a6beae536.png">
So wrap up this above, I think we should better to find the max `n` intersection angles for the next step to arrange the self-loop edges. May be we could:
0. Do this step after all of the non-self-loop-edges layout finished.
1. Use `Math.atan2` to get the radians to x positive.
2. sort the edges by their radians
3. Get a list of intersection angles by `sortedEdges[n+1].radian - sortedEdges[n].radian`, and sort desc.
##########
File path: src/chart/graph/edgeVisual.ts
##########
@@ -20,6 +20,9 @@
import GlobalModel from '../../model/Global';
import GraphSeriesModel, { GraphEdgeItemOption } from './GraphSeries';
import { extend } from 'zrender/src/core/util';
+import { intersectCurveCircle } from './adjustEdge'
Review comment:
eslint error
##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -88,16 +89,82 @@ export function circularLayout(
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
let cp1;
+ let cp2;
const x12 = (p1[0] + p2[0]) / 2;
const y12 = (p1[1] + p2[1]) / 2;
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
Review comment:
Follow the convention, use `zrUtil.filter` instead.
##########
File path: src/chart/graph/adjustEdge.ts
##########
@@ -26,67 +26,120 @@ const v1: number[] = [];
const v2: number[] = [];
const v3: number[] = [];
const quadraticAt = curveTool.quadraticAt;
+const cubicAt = curveTool.cubicAt;
const v2DistSquare = vec2.distSquare;
const mathAbs = Math.abs;
-function intersectCurveCircle(
+export function intersectCurveCircle(
curvePoints: number[][],
center: number[],
radius: number
) {
const p0 = curvePoints[0];
const p1 = curvePoints[1];
const p2 = curvePoints[2];
+ const p3 = curvePoints[3];
let d = Infinity;
let t;
const radiusSquare = radius * radius;
let interval = 0.1;
- for (let _t = 0.1; _t <= 0.9; _t += 0.1) {
- v1[0] = quadraticAt(p0[0], p1[0], p2[0], _t);
- v1[1] = quadraticAt(p0[1], p1[1], p2[1], _t);
- const diff = mathAbs(v2DistSquare(v1, center) - radiusSquare);
- if (diff < d) {
- d = diff;
- t = _t;
+ if (p3) {
Review comment:
I think we should better not repeat the entire code.
The only difference is "cubic" or "quadratic", so we could make a function like:
```js
const bezierAt = p3
? cubicAt
: function (p0: number, p1: number, p2: number, p3: number, t: number): number {
return quadraticAt(p0, p1, p2, t);
};
```
And substitute `bezierAt` for the previous `quadraticAt` below.
##########
File path: src/chart/graph/edgeVisual.ts
##########
@@ -75,6 +79,26 @@ export default function graphEdgeVisual(ecModel: GlobalModel) {
symbolType[1] && edge.setVisual('toSymbol', symbolType[1]);
symbolSize[0] && edge.setVisual('fromSymbolSize', symbolSize[0]);
symbolSize[1] && edge.setVisual('toSymbolSize', symbolSize[1]);
+
+
+ if (edge.node1 === edge.node2 && toSymbol && toSymbol !== 'none') {
+ const edgeData = edge.getLayout();
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2;
+
+ let t = intersectCurveCircle(edgeData, edgeData[0], radius);
+ if (t < 0.5) {
+ t = 1 - t;
+ }
+ const tdx = cubicDerivativeAt(edgeData[0][0], edgeData[1][0], edgeData[2][0], edgeData[3][0], t);
+ const tdy = cubicDerivativeAt(edgeData[0][1], edgeData[1][1], edgeData[2][1], edgeData[3][1], t);
+ const degree = Math.atan2(tdy, tdx) / Math.PI * 180;
+ if( degree > 90 || degree < 0 && degree > -90) {
+ edge.setVisual('toSymbolRotate', degree + 188);
Review comment:
The hard-coded `8` seems can not make a correct rotate in every cases.
##########
File path: src/chart/graph/adjustEdge.ts
##########
@@ -26,67 +26,120 @@ const v1: number[] = [];
const v2: number[] = [];
const v3: number[] = [];
const quadraticAt = curveTool.quadraticAt;
+const cubicAt = curveTool.cubicAt;
const v2DistSquare = vec2.distSquare;
const mathAbs = Math.abs;
-function intersectCurveCircle(
+export function intersectCurveCircle(
curvePoints: number[][],
center: number[],
radius: number
) {
const p0 = curvePoints[0];
const p1 = curvePoints[1];
const p2 = curvePoints[2];
+ const p3 = curvePoints[3];
let d = Infinity;
let t;
const radiusSquare = radius * radius;
let interval = 0.1;
- for (let _t = 0.1; _t <= 0.9; _t += 0.1) {
- v1[0] = quadraticAt(p0[0], p1[0], p2[0], _t);
- v1[1] = quadraticAt(p0[1], p1[1], p2[1], _t);
- const diff = mathAbs(v2DistSquare(v1, center) - radiusSquare);
- if (diff < d) {
- d = diff;
- t = _t;
+ if (p3) {
+ for (let _t = 0.1; _t <= 0.9; _t += 0.1) {
+ v1[0] = cubicAt(p0[0], p1[0], p2[0], p3[0], _t);
+ v1[1] = cubicAt(p0[1], p1[1], p2[1], p3[1], _t);
+ const diff = mathAbs(v2DistSquare(v1, center) - radiusSquare);
+ if (diff < d) {
+ d = diff;
+ t = _t;
+ }
}
- }
+ // Assume the segment is monotoneļ¼Find root through Bisection method
+ // At most 32 iteration
+ for (let i = 0; i < 32; i++) {
+ // const prev = t - interval;
+ const next = t + interval;
+ // v1[0] = cubicAt(p0[0], p1[0], p2[0], p3[0], prev);
+ // v1[1] = cubicAt(p0[1], p1[1], p2[1], p3[1], prev);
+ v2[0] = cubicAt(p0[0], p1[0], p2[0], p3[0], t);
+ v2[1] = cubicAt(p0[1], p1[1], p2[1], p3[1], t);
+ v3[0] = cubicAt(p0[0], p1[0], p2[0], p3[0], next);
+ v3[1] = cubicAt(p0[1], p1[1], p2[1], p3[1], next);
- // Assume the segment is monotoneļ¼Find root through Bisection method
- // At most 32 iteration
- for (let i = 0; i < 32; i++) {
- // let prev = t - interval;
- const next = t + interval;
- // v1[0] = quadraticAt(p0[0], p1[0], p2[0], prev);
- // v1[1] = quadraticAt(p0[1], p1[1], p2[1], prev);
- v2[0] = quadraticAt(p0[0], p1[0], p2[0], t);
- v2[1] = quadraticAt(p0[1], p1[1], p2[1], t);
- v3[0] = quadraticAt(p0[0], p1[0], p2[0], next);
- v3[1] = quadraticAt(p0[1], p1[1], p2[1], next);
-
- const diff = v2DistSquare(v2, center) - radiusSquare;
- if (mathAbs(diff) < 1e-2) {
- break;
- }
+ const diff = v2DistSquare(v2, center) - radiusSquare;
+ if (mathAbs(diff) < 1e-2) {
+ break;
+ }
- // let prevDiff = v2DistSquare(v1, center) - radiusSquare;
- const nextDiff = v2DistSquare(v3, center) - radiusSquare;
+ // const prevDiff = v2DistSquare(v1, center) - radiusSquare;
+ const nextDiff = v2DistSquare(v3, center) - radiusSquare;
- interval /= 2;
- if (diff < 0) {
- if (nextDiff >= 0) {
- t = t + interval;
- }
- else {
- t = t - interval;
+ interval /= 2;
+ if (diff < 0) {
Review comment:
wrong indent
##########
File path: src/chart/graph/adjustEdge.ts
##########
@@ -98,16 +151,17 @@ function intersectCurveCircle(
export default function adjustEdge(graph: Graph, scale: number) {
const tmp0: number[] = [];
const quadraticSubdivide = curveTool.quadraticSubdivide;
+ const cubicSubdivide = curveTool.cubicSubdivide;
const pts: number[][] = [[], [], []];
const pts2: number[][] = [[], []];
+ const pts3 : number[][] = [[], [], [], []];
Review comment:
The variable `pts` actually is `pts3`.
This variable `pts3` should be `pts4`.
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -19,10 +19,30 @@
import * as vec2 from 'zrender/src/core/vector';
import GraphSeriesModel, { GraphNodeItemOption, GraphEdgeItemOption } from './GraphSeries';
-import Graph from '../../data/Graph';
+import Graph, { GraphNode } from '../../data/Graph';
import * as zrUtil from 'zrender/src/core/util';
import {getCurvenessForEdge} from '../helper/multipleGraphEdgeHelper';
+import { getNodeGlobalScale } from './graphHelper';
+export function cubicPosition(pt: number[], center: number[], radius: number) {
Review comment:
If this function is to shared by `simpleLayoutHelper`, `forceLayoutHelper` and `circularLayoutHelper`,
it should better not to be put here. We can put it in some common place like create a new file named `layoutHelper.ts`.
##########
File path: src/chart/graph/circularLayoutHelper.ts
##########
@@ -88,16 +89,82 @@ export function circularLayout(
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
let cp1;
+ let cp2;
const x12 = (p1[0] + p2[0]) / 2;
const y12 = (p1[1] + p2[1]) / 2;
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
Review comment:
Follow the convention, should better use `zrUtil.each` instead.
##########
File path: src/chart/graph/forceLayout.ts
##########
@@ -144,7 +146,80 @@ export default function graphForceLayout(ecModel: GlobalModel) {
points[1] = points[1] || [];
vec2.copy(points[0], p1);
vec2.copy(points[1], p2);
- if (+e.curveness) {
+ if (e.n1 === e.n2) {
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(graphSeries) * size / 2;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
+ const v: any[] = [];
Review comment:
Should not be `any` if we known the type.
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -49,7 +69,80 @@ export function simpleLayoutEdge(graph: Graph, seriesModel: GraphSeriesModel) {
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
const points = [p1, p2];
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const curve = getCurvenessForEdge(edge, seriesModel, index, true);
+ const curveness = curve >= 1 ? curve : 1 - curve;
Review comment:
1. I think `1 - curve` might not be correct. It make the curveness of the self-loop edge not to be proportional to the user specified curveness.
2. In `circularLayoutHelper` and `forceLayout`, there is no `1 - curve`.
##########
File path: src/chart/graph/forceLayout.ts
##########
@@ -144,7 +146,80 @@ export default function graphForceLayout(ecModel: GlobalModel) {
points[1] = points[1] || [];
vec2.copy(points[0], p1);
vec2.copy(points[1], p2);
- if (+e.curveness) {
+ if (e.n1 === e.n2) {
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(graphSeries) * size / 2;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
+ const v: any[] = [];
+ vec2.sub(v, node.getLayout(), edge.node1.getLayout());
+ vec2.normalize(v, v);
+ vectors.push(v);
+ });
+ // find the max angle
+ for (let i = 0; i < vectors.length; i++) {
+ for (let j = i + 1; j < vectors.length; j++) {
+ if (vec2.distSquare(vectors[i], vectors[j]) > d) {
+ d = vec2.distSquare(vectors[i], vectors[j]);
+ pt1 = vectors[i];
+ pt2 = vectors[j];
+ }
+ }
+ }
+ // if the angle is more than sixty degree
+ if (vec2.distSquare(pt1, pt2) > Math.sqrt(3)) {
+ vec2.scaleAndAdd(pt1, p1, pt1, radius);
Review comment:
`vec2.scaleAndAdd(pt1, p1, pt1, radius);`
is not necessary.
##########
File path: src/chart/graph/edgeVisual.ts
##########
@@ -75,6 +79,26 @@ export default function graphEdgeVisual(ecModel: GlobalModel) {
symbolType[1] && edge.setVisual('toSymbol', symbolType[1]);
symbolSize[0] && edge.setVisual('fromSymbolSize', symbolSize[0]);
symbolSize[1] && edge.setVisual('toSymbolSize', symbolSize[1]);
+
+
+ if (edge.node1 === edge.node2 && toSymbol && toSymbol !== 'none') {
Review comment:
I think this logic (adjust the arrow rotate) should not be put here.
+ In this step we probably can not get the edge layout result (the sequence of `1 visual 2 layout` or `1 layout 2 visual` is decided in the `install.ts`).
+ The arrow rotation is calculated in https://github.com/apache/echarts/blob/5.2.0/src/chart/helper/Line.ts#L360 . If there is any issue about the rotation, we should better change the code in that place.
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -49,7 +69,80 @@ export function simpleLayoutEdge(graph: Graph, seriesModel: GraphSeriesModel) {
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
const points = [p1, p2];
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const curve = getCurvenessForEdge(edge, seriesModel, index, true);
+ const curveness = curve >= 1 ? curve : 1 - curve;
+ const symbolSize = seriesModel.get('symbolSize');
+ const size = zrUtil.isArray(symbolSize) ? Number((symbolSize[0] + symbolSize[1]) / 2) : Number(symbolSize);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2 * curveness;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
+ const v: any[] = [];
+ vec2.sub(v, node.getLayout(), edge.node1.getLayout());
+ vec2.normalize(v, v);
+ vectors.push(v);
+ });
+ // find the max angle
+ for (let i = 0; i < vectors.length; i++) {
+ for (let j = i + 1; j < vectors.length; j++) {
+ if (vec2.distSquare(vectors[i], vectors[j]) > d) {
+ d = vec2.distSquare(vectors[i], vectors[j]);
+ pt1 = vectors[i];
+ pt2 = vectors[j];
+ }
+ }
+ }
+ // if the angle is more than sixty degree
+ if (vec2.distSquare(pt1, pt2) > Math.sqrt(3)) {
+ vec2.scaleAndAdd(pt1, p1, pt1, radius);
+ vec2.scaleAndAdd(pt2, p2, pt2, radius);
+ const point1 = cubicPosition(pt1, p1, 10 * radius);
+ const point2 = cubicPosition(pt2, p2, 10 * radius);
+ const mid = [(point1[0] + point2[0]) / 2, (point1[1] + point2[1]) / 2];
+ vec2.sub(mid, mid, p1);
+ const degree = Math.atan2(mid[1], mid[0]) / Math.PI * 180;
+ const v1 = [Math.cos((degree - 30) * Math.PI / 180), Math.sin((degree - 30) * Math.PI / 180)];
+ const v2 = [Math.cos((degree + 30) * Math.PI / 180), Math.sin((degree + 30) * Math.PI / 180)];
+ vec2.scaleAndAdd(v1, p1, v1, 10 * radius);
+ vec2.scaleAndAdd(v2, p2, v2, 10 * radius);
+ points.push(v1, v2);
Review comment:
I think we do not need to convert them to degree. We can just use the radians.
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -49,7 +69,80 @@ export function simpleLayoutEdge(graph: Graph, seriesModel: GraphSeriesModel) {
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
const points = [p1, p2];
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const curve = getCurvenessForEdge(edge, seriesModel, index, true);
+ const curveness = curve >= 1 ? curve : 1 - curve;
+ const symbolSize = seriesModel.get('symbolSize');
+ const size = zrUtil.isArray(symbolSize) ? Number((symbolSize[0] + symbolSize[1]) / 2) : Number(symbolSize);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2 * curveness;
Review comment:
1. Here we `* curveness` and latter we `10 * radius`.
I think we should better make those code in one place.
2. The user specified or auto calculated (by `autoCurveness: true`) curveness should be fetched by :
```ts
const curveness = zrUtil.retrieve3(
edge.getModel<GraphEdgeItemOption>().get(['lineStyle', 'curveness']),
-getCurvenessForEdge(edge, seriesModel, index, true),
0
);
```
3. The `curveness` we get above is `0` ~ `Infinity`.
But in self-loop case the final bezier point should be `symbolSize / 2` ~ `Infinity`.
So probably we can make a formula like
```ts
const radius = getNodeGlobalScale(seriesModel) * symbolSize / 2;
const distBezierPointToCenter = 10 * (curveness + 1) * radius;
// and then
points.push(cubicPosition(pt1, p1, distBezierPointToCenter));
points.push(cubicPosition(pt2, p2, distBezierPointToCenter));
```
4. `p1` and `p2` are the same point. We should better rename it like `center` to make the readability better.
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -49,7 +69,80 @@ export function simpleLayoutEdge(graph: Graph, seriesModel: GraphSeriesModel) {
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
const points = [p1, p2];
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const curve = getCurvenessForEdge(edge, seriesModel, index, true);
+ const curveness = curve >= 1 ? curve : 1 - curve;
+ const symbolSize = seriesModel.get('symbolSize');
Review comment:
Can
```ts
registers.registerLayout(registers.PRIORITY.VISUAL.POST_CHART_LAYOUT, simpleLayout);
```
and use `getSymbolSize` of `graphHelper.ts`.
##########
File path: src/chart/graph/forceLayout.ts
##########
@@ -144,7 +146,80 @@ export default function graphForceLayout(ecModel: GlobalModel) {
points[1] = points[1] || [];
vec2.copy(points[0], p1);
vec2.copy(points[1], p2);
- if (+e.curveness) {
+ if (e.n1 === e.n2) {
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(graphSeries) * size / 2;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
+ const v: any[] = [];
+ vec2.sub(v, node.getLayout(), edge.node1.getLayout());
+ vec2.normalize(v, v);
+ vectors.push(v);
+ });
+ // find the max angle
+ for (let i = 0; i < vectors.length; i++) {
+ for (let j = i + 1; j < vectors.length; j++) {
+ if (vec2.distSquare(vectors[i], vectors[j]) > d) {
+ d = vec2.distSquare(vectors[i], vectors[j]);
+ pt1 = vectors[i];
+ pt2 = vectors[j];
+ }
+ }
+ }
+ // if the angle is more than sixty degree
+ if (vec2.distSquare(pt1, pt2) > Math.sqrt(3)) {
+ vec2.scaleAndAdd(pt1, p1, pt1, radius);
+ vec2.scaleAndAdd(pt2, p2, pt2, radius);
+ const point1 = cubicPosition(pt1, p1, 10 * radius);
+ const point2 = cubicPosition(pt2, p2, 10 * radius);
+ const mid = [(point1[0] + point2[0]) / 2, (point1[1] + point2[1]) / 2];
+ vec2.sub(mid, mid, p1);
+ const degree = Math.atan2(mid[1], mid[0]) / Math.PI * 180;
+ const v1 = [Math.cos((degree - 30) * Math.PI / 180),
+ Math.sin((degree - 30) * Math.PI / 180)];
+ const v2 = [Math.cos((degree + 30) * Math.PI / 180),
+ Math.sin((degree + 30) * Math.PI / 180)];
+ vec2.scaleAndAdd(v1, p1, v1, 10 * radius);
+ vec2.scaleAndAdd(v2, p2, v2, 10 * radius);
+ points[2] = v1;
+ points[3] = v2;
+ }
+ else {
+ vec2.scaleAndAdd(pt1, p1, pt1, radius);
Review comment:
`vec2.scaleAndAdd(pt1, p1, pt1, radius);`
is not necessary.
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -49,7 +69,80 @@ export function simpleLayoutEdge(graph: Graph, seriesModel: GraphSeriesModel) {
const p1 = vec2.clone(edge.node1.getLayout());
const p2 = vec2.clone(edge.node2.getLayout());
const points = [p1, p2];
- if (+curveness) {
+ if (edge.node1 === edge.node2) {
+ const curve = getCurvenessForEdge(edge, seriesModel, index, true);
+ const curveness = curve >= 1 ? curve : 1 - curve;
+ const symbolSize = seriesModel.get('symbolSize');
+ const size = zrUtil.isArray(symbolSize) ? Number((symbolSize[0] + symbolSize[1]) / 2) : Number(symbolSize);
+ const radius = getNodeGlobalScale(seriesModel) * size / 2 * curveness;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
+ const v: any[] = [];
+ vec2.sub(v, node.getLayout(), edge.node1.getLayout());
+ vec2.normalize(v, v);
+ vectors.push(v);
+ });
+ // find the max angle
+ for (let i = 0; i < vectors.length; i++) {
+ for (let j = i + 1; j < vectors.length; j++) {
+ if (vec2.distSquare(vectors[i], vectors[j]) > d) {
+ d = vec2.distSquare(vectors[i], vectors[j]);
+ pt1 = vectors[i];
+ pt2 = vectors[j];
+ }
+ }
+ }
+ // if the angle is more than sixty degree
+ if (vec2.distSquare(pt1, pt2) > Math.sqrt(3)) {
+ vec2.scaleAndAdd(pt1, p1, pt1, radius);
+ vec2.scaleAndAdd(pt2, p2, pt2, radius);
+ const point1 = cubicPosition(pt1, p1, 10 * radius);
+ const point2 = cubicPosition(pt2, p2, 10 * radius);
+ const mid = [(point1[0] + point2[0]) / 2, (point1[1] + point2[1]) / 2];
+ vec2.sub(mid, mid, p1);
+ const degree = Math.atan2(mid[1], mid[0]) / Math.PI * 180;
+ const v1 = [Math.cos((degree - 30) * Math.PI / 180), Math.sin((degree - 30) * Math.PI / 180)];
+ const v2 = [Math.cos((degree + 30) * Math.PI / 180), Math.sin((degree + 30) * Math.PI / 180)];
+ vec2.scaleAndAdd(v1, p1, v1, 10 * radius);
+ vec2.scaleAndAdd(v2, p2, v2, 10 * radius);
+ points.push(v1, v2);
+ }
+ else {
+ vec2.scaleAndAdd(pt1, p1, pt1, radius);
+ vec2.scaleAndAdd(pt2, p2, pt2, radius);
+ points.push(cubicPosition(pt1, p1, 10 * radius));
+ points.push(cubicPosition(pt2, p2, 10 * radius));
+ }
+ }
+ else {
+ points.push([
+ p1[0] - radius * 4,
+ p2[1] - radius * 6
+ ]);
+ points.push([
+ p1[0] + radius * 4,
+ p2[1] - radius * 6
+ ]);
+ }
Review comment:
I think when there is only one point, the self-loop edge also need to calculate the direction rather than hard code a direction.
<img width="408" alt="Screen Shot 2021-09-16 at 3 42 43 AM" src="https://user-images.githubusercontent.com/1956569/133499049-b62655eb-52f0-49ab-8e0f-bab0f1111f0f.png">
##########
File path: src/chart/graph/simpleLayoutHelper.ts
##########
@@ -19,10 +19,30 @@
import * as vec2 from 'zrender/src/core/vector';
import GraphSeriesModel, { GraphNodeItemOption, GraphEdgeItemOption } from './GraphSeries';
-import Graph from '../../data/Graph';
+import Graph, { GraphNode } from '../../data/Graph';
import * as zrUtil from 'zrender/src/core/util';
import {getCurvenessForEdge} from '../helper/multipleGraphEdgeHelper';
+import { getNodeGlobalScale } from './graphHelper';
+export function cubicPosition(pt: number[], center: number[], radius: number) {
Review comment:
But I did not understand what does this function do.
So I tried to visualize this formula [here](https://www.desmos.com/calculator/lio53lzsa3),
and understood that it seems to make a position that have a distance `radius` to center, and the angle is determined by `pt` and `center`.
So could the function be simplified as follows?
```ts
export function cubicPosition(pt: number[], center: number[], radius: number) {
const pt2center = vec2.sub([], center, pt);
const radian = Math.atan2(pt2center[1], pt2center[0]); // from -PI to PI
return [
Math.cos(radian) * radius + center[0],
Math.sin(radian) * radius + center[1]
];
}
```
##########
File path: src/chart/graph/forceLayout.ts
##########
@@ -144,7 +146,80 @@ export default function graphForceLayout(ecModel: GlobalModel) {
points[1] = points[1] || [];
vec2.copy(points[0], p1);
vec2.copy(points[1], p2);
- if (+e.curveness) {
+ if (e.n1 === e.n2) {
+ const size = getSymbolSize(edge.node1);
+ const radius = getNodeGlobalScale(graphSeries) * size / 2;
+ const inEdges = edge.node1.inEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const outEdges = edge.node1.outEdges.filter((edge) => {
+ return edge.node1 !== edge.node2;
+ });
+ const allNodes: GraphNode[] = [];
+ inEdges.forEach((edge) => {
+ allNodes.push(edge.node1);
+ });
+ outEdges.forEach((edge) => {
+ allNodes.push(edge.node2);
+ });
+ const vectors: any[][] = [];
+ let d = -Infinity;
+ let pt1: number[] = [];
+ let pt2: number[] = [];
+ if (allNodes.length > 1) {
+ allNodes.forEach(node => {
+ const v: any[] = [];
+ vec2.sub(v, node.getLayout(), edge.node1.getLayout());
+ vec2.normalize(v, v);
+ vectors.push(v);
+ });
+ // find the max angle
+ for (let i = 0; i < vectors.length; i++) {
+ for (let j = i + 1; j < vectors.length; j++) {
+ if (vec2.distSquare(vectors[i], vectors[j]) > d) {
+ d = vec2.distSquare(vectors[i], vectors[j]);
+ pt1 = vectors[i];
+ pt2 = vectors[j];
+ }
+ }
+ }
+ // if the angle is more than sixty degree
+ if (vec2.distSquare(pt1, pt2) > Math.sqrt(3)) {
+ vec2.scaleAndAdd(pt1, p1, pt1, radius);
+ vec2.scaleAndAdd(pt2, p2, pt2, radius);
+ const point1 = cubicPosition(pt1, p1, 10 * radius);
Review comment:
The fixed number `10` might be not suitable for large symbolSize:
<img width="237" alt="Screen Shot 2021-09-18 at 2 13 15 AM" src="https://user-images.githubusercontent.com/1956569/133835074-12db66bc-6cf1-4c89-9490-32c74584cbb7.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org