-
Notifications
You must be signed in to change notification settings - Fork 10
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #569 from amelioro/avoid-edge-overlap
Avoid edge overlap
- Loading branch information
Showing
15 changed files
with
488 additions
and
82 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,8 +87,8 @@ enum NodeType { | |
| problem | ||
| cause | ||
| criterion | ||
| effect | ||
| benefit | ||
| effect | ||
| detriment | ||
| solutionComponent | ||
| solution | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| import { getBezierPath } from "reactflow"; | ||
|
|
||
| import { throwError } from "@/common/errorHandling"; | ||
| import { scalePxViaDefaultFontSize } from "@/pages/_document.page"; | ||
| import { EdgeProps } from "@/web/topic/components/Diagram/Diagram"; | ||
| import { labelWidthPx } from "@/web/topic/utils/layout"; | ||
|
|
||
| /** | ||
| * If `drawSimpleEdgePaths` is true, draw a simple bezier between the source and target. | ||
| * Otherwise, use the ELK layout's bend points to draw a more complex path. | ||
| * | ||
| * TODO: modify complex-path algorithm such that curve has vertical slopes at start and end points. | ||
| * The lack of this implementation is the main reason why the `drawSimpleEdgePaths` option exists. | ||
| * Tried inserting a control point directly below `startPoint` and above `endPoint`, and that | ||
| * resulted in vertical slopes, but the curve to/from the next bend points became jagged. | ||
| */ | ||
| export const getPathDefinitionForEdge = (flowEdge: EdgeProps, drawSimpleEdgePaths: boolean) => { | ||
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- reactflow types data as nullable but we always pass it, so it should always be here | ||
| const { elkLabel, elkSections } = flowEdge.data!; | ||
| const elkSection = elkSections[0]; | ||
| const bendPoints = elkSection?.bendPoints; | ||
| const firstBendPoint = bendPoints?.[0]; | ||
| const lastBendPoint = bendPoints?.[bendPoints.length - 1]; | ||
|
|
||
| const missingBendPoints = | ||
| elkSection === undefined || | ||
| bendPoints === undefined || | ||
| firstBendPoint === undefined || | ||
| lastBendPoint === undefined; | ||
|
|
||
| if (drawSimpleEdgePaths || missingBendPoints) { | ||
| // TODO: probably ideally would draw this path through the ELK label position if that's provided | ||
| const [pathDefinition, labelX, labelY] = getBezierPath({ | ||
| sourceX: flowEdge.sourceX, | ||
| sourceY: flowEdge.sourceY, | ||
| sourcePosition: flowEdge.sourcePosition, | ||
| targetX: flowEdge.targetX, | ||
| targetY: flowEdge.targetY, | ||
| targetPosition: flowEdge.targetPosition, | ||
| }); | ||
|
|
||
| return { pathDefinition, labelX, labelY }; | ||
| } | ||
|
|
||
| if (elkSections.length > 1) { | ||
| return throwError("No implementation yet for edge with multiple sections", flowEdge); | ||
| } | ||
|
|
||
| /** | ||
| * TODO: start/end would ideally use `flowEdge.source`/`.target` because those are calculated | ||
| * to include the size of the handles, so the path actually points to the edge of the handle | ||
| * rather than the edge of the node. | ||
| * | ||
| * However: the layout's bend points near the start/end might be too high/low and need to shift | ||
| * down/up in order to make the curve smooth when pointing to the node handles. | ||
| */ | ||
| const startPoint = elkSection.startPoint; | ||
| const endPoint = elkSection.endPoint; | ||
| const points = [startPoint, ...bendPoints, endPoint]; | ||
|
|
||
| // Awkwardly need to filter out duplicates because of a bug in the layout algorithm. | ||
| // Should be able to remove this logic after https://github.com/eclipse/elk/issues/1085. | ||
| const pointsWithoutDuplicates = points.filter((point, index) => { | ||
| const pointBefore = points[index - 1]; | ||
| if (index === 0 || pointBefore === undefined) return true; | ||
| return pointBefore.x !== point.x || pointBefore.y !== point.y; | ||
| }); | ||
| const bendPointsWithoutDuplicates = pointsWithoutDuplicates.slice(1, -1); | ||
|
|
||
| const pathDefinition = drawBezierCurvesFromPoints( | ||
| startPoint, | ||
| bendPointsWithoutDuplicates, | ||
| endPoint, | ||
| ); | ||
|
|
||
| const { x: labelX, y: labelY } = elkLabel | ||
| ? // Note: ELK label position is moved left by half of its width in order to center it. | ||
| // eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
| { x: elkLabel.x! + 0.5 * scalePxViaDefaultFontSize(labelWidthPx), y: elkLabel.y! } | ||
| : getPathMidpoint(pathDefinition); | ||
|
|
||
| return { pathDefinition, labelX, labelY }; | ||
| }; | ||
|
|
||
| const getPathMidpoint = (pathDefinition: string) => { | ||
| // This seems like a wild solution to calculate label position based on svg path, | ||
| // but on average, this takes 0.05ms per edge; 100 edges would take 5ms, which seems plenty fast enough. | ||
| // Note: got this from github copilot suggestion. | ||
| // Also tried reusing one `path` element globally, re-setting its `d` attribute each time, | ||
| // but that didn't seem to save any significant amount of performance. | ||
| const path = document.createElementNS("http://www.w3.org/2000/svg", "path"); | ||
| path.setAttribute("d", pathDefinition); | ||
| const pathLength = path.getTotalLength(); | ||
|
|
||
| return path.getPointAtLength(pathLength / 2); | ||
| }; | ||
|
|
||
| interface Point { | ||
| x: number; | ||
| y: number; | ||
| } | ||
|
|
||
| /** | ||
| * Copied mostly from https://github.com/eclipse/elk/issues/848#issuecomment-1248084547 | ||
| * | ||
| * Could refactor to ensure everything is safer, but logic seems fine enough to trust. | ||
| */ | ||
| /* eslint-disable @typescript-eslint/no-non-null-assertion */ | ||
| /* eslint-disable functional/no-let */ | ||
| /* eslint-disable functional/no-loop-statements */ | ||
| /* eslint-disable functional/immutable-data */ | ||
| const drawBezierCurvesFromPoints = ( | ||
| startPoint: Point, | ||
| bendPoints: Point[], | ||
| endPoint: Point, | ||
| ): string => { | ||
| // If no bend points, we should've drawn a simple curve before getting here | ||
| if (bendPoints.length === 0) throwError("Expected bend points", startPoint, bendPoints, endPoint); | ||
|
|
||
| // not sure why end is treated as a control point, but algo seems to work and not sure a better name | ||
| const controlPoints = [...bendPoints, endPoint]; | ||
|
|
||
| const path = [`M ${ptToStr(startPoint)}`]; | ||
|
|
||
| // if there are groups of 3 points, draw cubic bezier curves | ||
| if (controlPoints.length % 3 === 0) { | ||
| for (let i = 0; i < controlPoints.length; i = i + 3) { | ||
| const [c1, c2, p] = controlPoints.slice(i, i + 3); | ||
| path.push(`C ${ptToStr(c1!)}, ${ptToStr(c2!)}, ${ptToStr(p!)}`); | ||
| } | ||
| } | ||
| // if there's an even number of points, draw quadratic curves | ||
| else if (controlPoints.length % 2 === 0) { | ||
| for (let i = 0; i < controlPoints.length; i = i + 2) { | ||
| const [c, p] = controlPoints.slice(i, i + 2); | ||
| path.push(`Q ${ptToStr(c!)}, ${ptToStr(p!)}`); | ||
| } | ||
| } | ||
| // else, add missing points and try again | ||
| // https://stackoverflow.com/a/72577667/1010492 | ||
| else { | ||
| for (let i = controlPoints.length - 3; i >= 2; i = i - 2) { | ||
| const missingPoint = midPoint(controlPoints[i - 1]!, controlPoints[i]!); | ||
| controlPoints.splice(i, 0, missingPoint); | ||
| } | ||
| const newBendPoints = controlPoints.slice(0, -1); | ||
| return drawBezierCurvesFromPoints(startPoint, newBendPoints, endPoint); | ||
| } | ||
|
|
||
| return path.join(" "); | ||
| }; | ||
|
|
||
| export const midPoint = (pt1: Point, pt2: Point) => { | ||
| return { | ||
| x: (pt2.x + pt1.x) / 2, | ||
| y: (pt2.y + pt1.y) / 2, | ||
| }; | ||
| }; | ||
|
|
||
| export const ptToStr = ({ x, y }: Point) => { | ||
| return `${x} ${y}`; | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.