From 5d352161fd3d4e375d4c097d9ae50d86ffbdd2ee Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Tue, 18 Feb 2025 15:38:48 +0100 Subject: [PATCH] Fix elbow arrow binding logic --- packages/excalidraw/element/binding.ts | 177 ++++++++++------------- packages/excalidraw/element/collision.ts | 19 +-- packages/excalidraw/element/distance.ts | 6 +- packages/excalidraw/element/utils.ts | 8 +- 4 files changed, 85 insertions(+), 125 deletions(-) diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts index 216f8515c1..86119be59f 100644 --- a/packages/excalidraw/element/binding.ts +++ b/packages/excalidraw/element/binding.ts @@ -46,11 +46,9 @@ import { aabbForElement, getElementShape, pointInsideBounds } from "../shapes"; import { compareHeading, HEADING_DOWN, - HEADING_LEFT, HEADING_RIGHT, HEADING_UP, headingForPointFromElement, - headingIsHorizontal, vectorToHeading, type Heading, } from "./heading"; @@ -72,9 +70,8 @@ import { vectorNormalize, vectorRotate, } from "../../math"; -import { distanceToBindableElement } from "./distance"; import { intersectElementWithLineSegment } from "./collision"; -import { debugClear, debugDrawLine } from "../visualdebug"; +import { distanceToBindableElement } from "./distance"; export type SuggestedBinding = | NonDeleted @@ -728,12 +725,29 @@ const calculateFocusAndGap = ( adjacentPointIndex, elementsMap, ); + + const focus = determineFocusDistance( + hoveredElement, + adjacentPoint, + edgePoint, + ); + const focusPointAbsolute = determineFocusPoint( + hoveredElement, + focus, + adjacentPoint, + ); + const intersection = + intersectElementWithLineSegment( + hoveredElement, + lineSegment(adjacentPoint, focusPointAbsolute), + ).sort( + (g, h) => pointDistanceSq(g, edgePoint) - pointDistanceSq(h, edgePoint), + )[0] ?? edgePoint; + const gap = Math.max(1, pointDistance(intersection, edgePoint)); + return { - focus: determineFocusDistance(hoveredElement, adjacentPoint, edgePoint), - gap: Math.max( - 1, - determineGapSize(edgePoint, adjacentPoint, hoveredElement, zoom), - ), + focus, + gap, }; }; @@ -928,53 +942,44 @@ export const bindPointToSnapToElementOutline = ( const p = isRectanguloidElement(bindableElement) ? avoidRectangularCorner(bindableElement, globalP) : globalP; - const localOtherPoint = - arrow.points[startOrEnd === "start" ? arrow.points.length - 1 : 0]; - const otherPoint = pointFrom( - arrow.x + localOtherPoint[0], - arrow.y + localOtherPoint[1], - ); - const prev = - arrow.points[startOrEnd === "start" ? 1 : arrow.points.length - 2]; - const isHorizontal = headingIsHorizontal( - vectorToHeading(vectorFromPoint(localP, prev)), - ); if (bindableElement && aabb) { - const heading = headingForPointFromElement(bindableElement, aabb, p); const center = getCenterForBounds(aabb); - const intersections = ( - intersectElementWithLineSegment( - bindableElement, - lineSegment( - p, - pointFrom( - isHorizontal ? center[0] : p[0], - !isHorizontal ? center[1] : p[1], + + const intersection = intersectElementWithLineSegment( + bindableElement, + lineSegment( + center, + pointFromVector( + vectorScale( + vectorNormalize(vectorFromPoint(p, center)), + Math.max(bindableElement.width, bindableElement.height), ), + center, ), - FIXED_BINDING_DISTANCE, - ) ?? [] - ) - .filter((p) => p != null) - .sort((g, h) => pointDistanceSq(g!, p) - pointDistanceSq(h!, p)); - const isVertical = - compareHeading(heading, HEADING_LEFT) || - compareHeading(heading, HEADING_RIGHT); - const dist = Math.abs(distanceToBindableElement(bindableElement, p)); - const isInner = isVertical - ? dist < bindableElement.width * -0.1 - : dist < bindableElement.height * -0.1; - - intersections.sort((a, b) => pointDistanceSq(a, p) - pointDistanceSq(b, p)); - - return isInner - ? headingToMidBindPoint(otherPoint, bindableElement, aabb) - : intersections.filter((i) => - isVertical - ? Math.abs(p[1] - i[1]) < 0.1 - : Math.abs(p[0] - i[0]) < 0.1, - )[0] ?? p; + ), + )[0]; + const currentDistance = pointDistance(p, center); + const fullDistance = pointDistance(intersection, center); + const ratio = currentDistance / fullDistance; + + switch (true) { + case ratio > 0.9: + if (currentDistance - fullDistance > FIXED_BINDING_DISTANCE) { + return p; + } + + return pointFromVector( + vectorScale( + vectorNormalize(vectorFromPoint(p, intersection)), + ratio > 1 ? FIXED_BINDING_DISTANCE : -FIXED_BINDING_DISTANCE, + ), + intersection, + ); + + default: + return headingToMidBindPoint(p, bindableElement, aabb); + } } return p; @@ -1255,25 +1260,27 @@ const updateBoundPoint = ( elementsMap, ); - const intersections = intersectElementWithLineSegment( - bindableElement, - lineSegment(adjacentPoint, focusPointAbsolute), - binding.gap, - ); - if (!intersections || intersections.length === 0) { - // This should never happen, since focusPoint should always be - // inside the element, but just in case, bail out - // Note: Might happen with rounded elements due to FP imprecision - newEdgePoint = edgePointAbsolute; - } else { - // Guaranteed to intersect because focusPoint is always inside the shape - intersections.sort( + const intersection = + intersectElementWithLineSegment( + bindableElement, + lineSegment(adjacentPoint, focusPointAbsolute), + ).sort( (g, h) => pointDistanceSq(g!, edgePointAbsolute) - pointDistanceSq(h!, edgePointAbsolute), - ); - newEdgePoint = intersections[0]; - } + )[0] ?? edgePointAbsolute; + + const gapOffsetPoint = intersection + ? pointFromVector( + vectorScale( + vectorNormalize(vectorFromPoint(adjacentPoint, intersection)), + binding.gap, + ), + intersection, + ) + : edgePointAbsolute; + + newEdgePoint = gapOffsetPoint; } return LinearElementEditor.pointFromAbsoluteCoords( @@ -1581,42 +1588,6 @@ const determineFocusDistance = ( ); }; -/** - * Determines gap size between element and edgePoint by intersecting the element - * in the direction of adjacentPoint -> edgePoint, then measuring the length of - * the intersection point and edgePoint. - * - * NOTE: This is not always the same as distance from edgePoint to the closest - * point on the element outline! - */ -const determineGapSize = ( - edgePoint: GlobalPoint, - adjacentPoint: GlobalPoint, - element: ExcalidrawBindableElement, - zoom?: AppState["zoom"], -): number => { - const s = lineSegment( - edgePoint, - pointFromVector( - // Create a vector from the adjacent point to the edge point - // scale it to cross the maximum gap distance + 1 to ensure intersection - vectorScale( - vectorNormalize(vectorFromPoint(edgePoint, adjacentPoint)), - maxBindingGap(element, element.width, element.height, zoom) + 1, - ), - edgePoint, - ), - ); - debugClear(); - debugDrawLine(s, { color: "red", permanent: true }); - const distances = intersectElementWithLineSegment(element, s) - .map((p) => pointDistance(edgePoint, p)) - .sort((a, b) => b - a); - const distance = distances.pop(); - console.log(distance); - return distance ?? 0; -}; - const determineFocusPoint = ( element: ExcalidrawBindableElement, // The oriented, relative distance from the center of `element` of the diff --git a/packages/excalidraw/element/collision.ts b/packages/excalidraw/element/collision.ts index 04ad0f6106..431323505d 100644 --- a/packages/excalidraw/element/collision.ts +++ b/packages/excalidraw/element/collision.ts @@ -156,7 +156,6 @@ export const hitElementBoundText = ( export const intersectElementWithLineSegment = ( element: ExcalidrawElement, line: LineSegment, - offset: number = 0, ): GlobalPoint[] => { switch (element.type) { case "rectangle": @@ -166,11 +165,11 @@ export const intersectElementWithLineSegment = ( case "embeddable": case "frame": case "magicframe": - return intersectRectanguloidWithLineSegment(element, line, offset); + return intersectRectanguloidWithLineSegment(element, line); case "diamond": - return intersectDiamondWithLineSegment(element, line, offset); + return intersectDiamondWithLineSegment(element, line); case "ellipse": - return intersectEllipseWithLineSegment(element, line, offset); + return intersectEllipseWithLineSegment(element, line); default: throw new Error(`Unimplemented element type '${element.type}'`); } @@ -179,7 +178,6 @@ export const intersectElementWithLineSegment = ( const intersectRectanguloidWithLineSegment = ( element: ExcalidrawRectanguloidElement, l: LineSegment, - offset: number, ): GlobalPoint[] => { const center = pointFrom( element.x + element.width / 2, @@ -199,10 +197,7 @@ const intersectRectanguloidWithLineSegment = ( ); // Get the element's building components we can test against - const [sides, corners] = deconstructRectanguloidElement( - element, - offset, - ); + const [sides, corners] = deconstructRectanguloidElement(element); return ( [ @@ -244,7 +239,6 @@ const intersectRectanguloidWithLineSegment = ( const intersectDiamondWithLineSegment = ( element: ExcalidrawDiamondElement, l: LineSegment, - offset: number = 0, ): GlobalPoint[] => { const center = pointFrom( element.x + element.width / 2, @@ -256,7 +250,7 @@ const intersectDiamondWithLineSegment = ( const rotatedA = pointRotateRads(l[0], center, -element.angle as Radians); const rotatedB = pointRotateRads(l[1], center, -element.angle as Radians); - const [sides, curves] = deconstructDiamondElement(element, offset); + const [sides, curves] = deconstructDiamondElement(element); return ( [ @@ -295,7 +289,6 @@ const intersectDiamondWithLineSegment = ( const intersectEllipseWithLineSegment = ( element: ExcalidrawEllipseElement, l: LineSegment, - offset: number = 0, ): GlobalPoint[] => { const center = pointFrom( element.x + element.width / 2, @@ -306,7 +299,7 @@ const intersectEllipseWithLineSegment = ( const rotatedB = pointRotateRads(l[1], center, -element.angle as Radians); return ellipseLineIntersectionPoints( - ellipse(center, element.width / 2 + offset, element.height / 2 + offset), + ellipse(center, element.width / 2, element.height / 2), line(rotatedA, rotatedB), ).map((p) => pointRotateRads(p, center, element.angle)); }; diff --git a/packages/excalidraw/element/distance.ts b/packages/excalidraw/element/distance.ts index f4eda47ddc..f521279198 100644 --- a/packages/excalidraw/element/distance.ts +++ b/packages/excalidraw/element/distance.ts @@ -45,7 +45,7 @@ export const distanceToBindableElement = ( * @param p The point to consider * @returns The eucledian distance to the outline of the rectanguloid element */ -export const distanceToRectanguloidElement = ( +const distanceToRectanguloidElement = ( element: ExcalidrawRectanguloidElement, p: GlobalPoint, ) => { @@ -76,7 +76,7 @@ export const distanceToRectanguloidElement = ( * @param p The point to consider * @returns The eucledian distance to the outline of the diamond */ -export const distanceToDiamondElement = ( +const distanceToDiamondElement = ( element: ExcalidrawDiamondElement, p: GlobalPoint, ): number => { @@ -107,7 +107,7 @@ export const distanceToDiamondElement = ( * @param p The point to consider * @returns The eucledian distance to the outline of the ellipse */ -export const distanceToEllipseElement = ( +const distanceToEllipseElement = ( element: ExcalidrawEllipseElement, p: GlobalPoint, ): number => { diff --git a/packages/excalidraw/element/utils.ts b/packages/excalidraw/element/utils.ts index 1ab6b881eb..2958ac7dc9 100644 --- a/packages/excalidraw/element/utils.ts +++ b/packages/excalidraw/element/utils.ts @@ -26,14 +26,10 @@ export function deconstructRectanguloidElement< Point extends GlobalPoint | LocalPoint, >( element: ExcalidrawRectanguloidElement, - offset: number = 0, ): [LineSegment[], Curve[]] { const r = rectangle( - pointFrom(element.x - offset, element.y - offset), - pointFrom( - element.x + element.width + offset, - element.y + element.height + offset, - ), + pointFrom(element.x, element.y), + pointFrom(element.x + element.width, element.y + element.height), ); const roundness = getCornerRadius( Math.min(element.width, element.height),