From e6ec02bf52e40b71782f28844244c6e5f418584a Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Sun, 16 Mar 2025 19:51:13 +0100 Subject: [PATCH] Fix elbow arrows --- packages/excalidraw/element/binding.ts | 161 +++++++++++----------- packages/excalidraw/element/elbowArrow.ts | 76 +++++----- 2 files changed, 125 insertions(+), 112 deletions(-) diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts index 93c5292f1..285e08310 100644 --- a/packages/excalidraw/element/binding.ts +++ b/packages/excalidraw/element/binding.ts @@ -24,7 +24,10 @@ import { KEYS } from "../keys"; import { aabbForElement, getElementShape, pointInsideBounds } from "../shapes"; import { arrayToMap, + invariant, isBindingFallthroughEnabled, + isDevEnv, + isTestEnv, tupleToCoors, } from "../utils"; @@ -41,6 +44,7 @@ import { HEADING_RIGHT, HEADING_UP, headingForPointFromElement, + headingIsHorizontal, vectorToHeading, type Heading, } from "./heading"; @@ -81,6 +85,7 @@ import type { } from "./types"; import type Scene from "../scene/Scene"; import type { AppState } from "../types"; +import { debugClear, debugDrawPoint } from "../visualdebug"; export type SuggestedBinding = | NonDeleted @@ -925,103 +930,105 @@ const getDistanceForBinding = ( export const bindPointToSnapToElementOutline = ( arrow: ExcalidrawElbowArrowElement, - bindableElement: ExcalidrawBindableElement | undefined, + bindableElement: ExcalidrawBindableElement, startOrEnd: "start" | "end", ): GlobalPoint => { - const aabb = bindableElement && aabbForElement(bindableElement); + if (isDevEnv() || isTestEnv()) { + invariant(arrow.points.length > 1, "Arrow should have at least 2 points"); + } + + const aabb = aabbForElement(bindableElement); const localP = arrow.points[startOrEnd === "start" ? 0 : arrow.points.length - 1]; const globalP = pointFrom( arrow.x + localP[0], arrow.y + localP[1], ); - const p = isRectanguloidElement(bindableElement) + const edgePoint = isRectanguloidElement(bindableElement) ? avoidRectangularCorner(bindableElement, globalP) : globalP; + const elbowed = isElbowArrow(arrow); + const center = getCenterForBounds(aabb); + const adjacentPointIdx = startOrEnd === "start" ? 1 : arrow.points.length - 2; + const adjacentPoint = pointRotateRads( + pointFrom( + arrow.x + arrow.points[adjacentPointIdx][0], + arrow.y + arrow.points[adjacentPointIdx][1], + ), + center, + arrow.angle ?? 0, + ); - if (bindableElement && aabb) { - const center = getCenterForBounds(aabb); - - const intersection = intersectElementWithLineSegment( + let intersection: GlobalPoint | null = null; + if (elbowed) { + const isHorizontal = headingIsHorizontal( + headingForPointFromElement(bindableElement, aabb, globalP), + ); + const otherPoint = pointFrom( + isHorizontal ? center[0] : edgePoint[0], + !isHorizontal ? center[1] : edgePoint[1], + ); + intersection = intersectElementWithLineSegment( bindableElement, lineSegment( - center, + otherPoint, pointFromVector( vectorScale( - vectorNormalize(vectorFromPoint(p, center)), - Math.max(bindableElement.width, bindableElement.height), + vectorNormalize(vectorFromPoint(edgePoint, otherPoint)), + Math.max(bindableElement.width, bindableElement.height) * 2, ), - center, + otherPoint, ), ), + FIXED_BINDING_DISTANCE, )[0]; - const currentDistance = pointDistance(p, center); - const fullDistance = Math.max( - pointDistance(intersection ?? p, center), - PRECISION, - ); - const ratio = round(currentDistance / fullDistance, 6); - - switch (true) { - case ratio > 0.9: - if ( - currentDistance - fullDistance > FIXED_BINDING_DISTANCE || - // Too close to determine vector from intersection to p - pointDistanceSq(p, intersection) < PRECISION - ) { - return p; - } - - return pointFromVector( + } else { + intersection = intersectElementWithLineSegment( + bindableElement, + lineSegment( + adjacentPoint, + pointFromVector( vectorScale( - vectorNormalize(vectorFromPoint(p, intersection ?? center)), - ratio > 1 ? FIXED_BINDING_DISTANCE : -FIXED_BINDING_DISTANCE, + vectorNormalize(vectorFromPoint(edgePoint, adjacentPoint)), + pointDistance(edgePoint, adjacentPoint) + + Math.max(bindableElement.width, bindableElement.height) * 2, ), - intersection ?? center, - ); - - default: - return headingToMidBindPoint(p, bindableElement, aabb); - } + adjacentPoint, + ), + ), + FIXED_BINDING_DISTANCE, + ).sort( + (g, h) => + pointDistanceSq(g, adjacentPoint) - pointDistanceSq(h, adjacentPoint), + )[0]; } - return p; -}; - -const headingToMidBindPoint = ( - p: GlobalPoint, - bindableElement: ExcalidrawBindableElement, - aabb: Bounds, -): GlobalPoint => { - const center = getCenterForBounds(aabb); - const heading = vectorToHeading(vectorFromPoint(p, center)); - - switch (true) { - case compareHeading(heading, HEADING_UP): - return pointRotateRads( - pointFrom((aabb[0] + aabb[2]) / 2 + 0.1, aabb[1]), - center, - bindableElement.angle, - ); - case compareHeading(heading, HEADING_RIGHT): - return pointRotateRads( - pointFrom(aabb[2], (aabb[1] + aabb[3]) / 2 + 0.1), - center, - bindableElement.angle, - ); - case compareHeading(heading, HEADING_DOWN): - return pointRotateRads( - pointFrom((aabb[0] + aabb[2]) / 2 - 0.1, aabb[3]), - center, - bindableElement.angle, - ); - default: - return pointRotateRads( - pointFrom(aabb[0], (aabb[1] + aabb[3]) / 2 - 0.1), - center, - bindableElement.angle, - ); + if ( + !intersection || + // Too close to determine vector from intersection to edgePoint + pointDistanceSq(edgePoint, intersection) < PRECISION + ) { + return edgePoint; } + + if (elbowed) { + const scalar = + pointDistanceSq(edgePoint, center) - + pointDistanceSq(intersection, center) > + 0 + ? FIXED_BINDING_DISTANCE + : -FIXED_BINDING_DISTANCE; + + return pointFromVector( + vectorScale( + vectorNormalize(vectorFromPoint(edgePoint, intersection)), + scalar, + ), + intersection, + ); + } + + return edgePoint; }; export const avoidRectangularCorner = ( @@ -1146,7 +1153,7 @@ export const snapToMid = ( ) { // LEFT return pointRotateRads( - pointFrom(x - FIXED_BINDING_DISTANCE, center[1]), + pointFrom(x - 2 * FIXED_BINDING_DISTANCE, center[1]), center, angle, ); @@ -1157,7 +1164,7 @@ export const snapToMid = ( ) { // TOP return pointRotateRads( - pointFrom(center[0], y - FIXED_BINDING_DISTANCE), + pointFrom(center[0], y - 2 * FIXED_BINDING_DISTANCE), center, angle, ); @@ -1168,7 +1175,7 @@ export const snapToMid = ( ) { // RIGHT return pointRotateRads( - pointFrom(x + width + FIXED_BINDING_DISTANCE, center[1]), + pointFrom(x + width + 2 * FIXED_BINDING_DISTANCE, center[1]), center, angle, ); @@ -1179,7 +1186,7 @@ export const snapToMid = ( ) { // DOWN return pointRotateRads( - pointFrom(center[0], y + height + FIXED_BINDING_DISTANCE), + pointFrom(center[0], y + height + 2 * FIXED_BINDING_DISTANCE), center, angle, ); diff --git a/packages/excalidraw/element/elbowArrow.ts b/packages/excalidraw/element/elbowArrow.ts index 5e44b6ea6..0c8872df2 100644 --- a/packages/excalidraw/element/elbowArrow.ts +++ b/packages/excalidraw/element/elbowArrow.ts @@ -40,7 +40,7 @@ import { headingForPoint, } from "./heading"; import { type ElementUpdate } from "./mutateElement"; -import { isBindableElement } from "./typeChecks"; +import { isBindableElement, isElbowArrow } from "./typeChecks"; import { type ExcalidrawElbowArrowElement, type NonDeletedSceneElementsMap, @@ -58,6 +58,7 @@ import type { NonDeletedExcalidrawElement, } from "./types"; import type { AppState } from "../types"; +import { debugClear } from "../visualdebug"; type GridAddress = [number, number] & { _brand: "gridaddress" }; @@ -238,16 +239,6 @@ const handleSegmentRenormalization = ( nextPoints.map((p) => pointFrom(p[0] - arrow.x, p[1] - arrow.y), ), - arrow.startBinding && - getBindableElementForId( - arrow.startBinding.elementId, - elementsMap, - ), - arrow.endBinding && - getBindableElementForId( - arrow.endBinding.elementId, - elementsMap, - ), ), ) ?? [], ), @@ -341,9 +332,6 @@ const handleSegmentRelease = ( y, ), ], - startBinding && - getBindableElementForId(startBinding.elementId, elementsMap), - endBinding && getBindableElementForId(endBinding.elementId, elementsMap), { isDragging: false }, ); @@ -983,6 +971,8 @@ export const updateElbowArrowPoints = ( ); } + const fixedSegments = updates.fixedSegments ?? arrow.fixedSegments ?? []; + const updatedPoints: readonly LocalPoint[] = updates.points ? updates.points && updates.points.length === 2 ? arrow.points.map((p, idx) => @@ -1055,12 +1045,22 @@ export const updateElbowArrowPoints = ( }, elementsMap, updatedPoints, - startElement, - endElement, options, ); - const fixedSegments = updates.fixedSegments ?? arrow.fixedSegments ?? []; + // 0. During all element replacement in the scene, we just need to renormalize + // the arrow + // TODO (dwelle,mtolmacs): Remove this once Scene.getScene() is removed + if (elementsMap.size === 0 && validateElbowPoints(updatedPoints)) { + return normalizeArrowElementUpdate( + updatedPoints.map((p) => + pointFrom(arrow.x + p[0], arrow.y + p[1]), + ), + arrow.fixedSegments, + arrow.startIsSpecial, + arrow.endIsSpecial, + ); + } //// // 1. Renormalize the arrow @@ -1195,8 +1195,6 @@ const getElbowArrowData = ( }, elementsMap: NonDeletedSceneElementsMap, nextPoints: readonly LocalPoint[], - startElement: ExcalidrawBindableElement | null, - endElement: ExcalidrawBindableElement | null, options?: { isDragging?: boolean; zoom?: AppState["zoom"]; @@ -1211,8 +1209,8 @@ const getElbowArrowData = ( GlobalPoint >(nextPoints[nextPoints.length - 1], vector(arrow.x, arrow.y)); - let hoveredStartElement = startElement; - let hoveredEndElement = endElement; + let hoveredStartElement = null; + let hoveredEndElement = null; if (options?.isDragging) { const elements = Array.from(elementsMap.values()); hoveredStartElement = @@ -1221,39 +1219,48 @@ const getElbowArrowData = ( elementsMap, elements, options?.zoom, - ) || startElement; + ) || null; hoveredEndElement = getHoveredElement( origEndGlobalPoint, elementsMap, elements, options?.zoom, - ) || endElement; + ) || null; + } else { + hoveredStartElement = arrow.startBinding + ? getBindableElementForId(arrow.startBinding.elementId, elementsMap) || + null + : null; + hoveredEndElement = arrow.endBinding + ? getBindableElementForId(arrow.endBinding.elementId, elementsMap) || null + : null; } - + debugClear(); + console.log("----"); const startGlobalPoint = getGlobalPoint( { ...arrow, + type: "arrow", elbowed: true, points: nextPoints, } as ExcalidrawElbowArrowElement, "start", arrow.startBinding?.fixedPoint, origStartGlobalPoint, - startElement, hoveredStartElement, options?.isDragging, ); const endGlobalPoint = getGlobalPoint( { ...arrow, + type: "arrow", elbowed: true, points: nextPoints, } as ExcalidrawElbowArrowElement, "end", arrow.endBinding?.fixedPoint, origEndGlobalPoint, - endElement, hoveredEndElement, options?.isDragging, ); @@ -2199,36 +2206,35 @@ const getGlobalPoint = ( startOrEnd: "start" | "end", fixedPointRatio: [number, number] | undefined | null, initialPoint: GlobalPoint, - boundElement?: ExcalidrawBindableElement | null, - hoveredElement?: ExcalidrawBindableElement | null, + element?: ExcalidrawBindableElement | null, isDragging?: boolean, ): GlobalPoint => { if (isDragging) { - if (hoveredElement) { + if (element) { const snapPoint = bindPointToSnapToElementOutline( arrow, - hoveredElement, + element, startOrEnd, ); - return snapToMid(hoveredElement, snapPoint); + return snapToMid(element, snapPoint); } return initialPoint; } - if (boundElement) { + if (element) { const fixedGlobalPoint = getGlobalFixedPointForBindableElement( fixedPointRatio || [0, 0], - boundElement, + element, ); // NOTE: Resize scales the binding position point too, so we need to update it return Math.abs( - distanceToBindableElement(boundElement, fixedGlobalPoint) - + distanceToBindableElement(element, fixedGlobalPoint) - FIXED_BINDING_DISTANCE, ) > 0.01 - ? bindPointToSnapToElementOutline(arrow, boundElement, startOrEnd) + ? bindPointToSnapToElementOutline(arrow, element, startOrEnd) : fixedGlobalPoint; }