diff --git a/packages/element/src/arrowEndpointText.ts b/packages/element/src/arrowEndpointText.ts index 0c690c8529..f6b1d583d9 100644 --- a/packages/element/src/arrowEndpointText.ts +++ b/packages/element/src/arrowEndpointText.ts @@ -188,7 +188,7 @@ export const getTextBindingForArrowEndpoint = ( // than gap-outside the outline, so offsetting there would just shift them. const gap = isElbowArrow(arrow) ? 0 - : getBindingGap({ strokeWidth: targetStrokeWidth }, arrow); + : getBindingGap({ strokeWidth: targetStrokeWidth }); // How far back along the arrow the bound side has to sit for the tip to // stay exactly where it is. diff --git a/packages/element/src/arrows/focus.ts b/packages/element/src/arrows/focus.ts index e3710ea885..63d11ca2a2 100644 --- a/packages/element/src/arrows/focus.ts +++ b/packages/element/src/arrows/focus.ts @@ -10,7 +10,6 @@ import { getBindingGap, getGlobalFixedPointForBindableElement, isBindingEnabled, - maxBindingDistance_simple, unbindBindingElement, updateBoundPoint, } from "../binding"; @@ -20,7 +19,7 @@ import { isElbowArrow, } from "../typeChecks"; import { LinearElementEditor } from "../linearElementEditor"; -import { getHoveredElementForFocusPoint, hitElementItself } from "../collision"; +import { getHoveredElementForBinding, hitElementItself } from "../collision"; import { moveArrowAboveBindable } from "../zindex"; import type { @@ -94,7 +93,7 @@ export const isFocusPointVisible = ( element: bindableElement, elementsMap, point: focusPoint, - threshold: getBindingGap(bindableElement, arrow), + threshold: getBindingGap(bindableElement), overrideShouldTestInside: true, }) ); @@ -243,12 +242,11 @@ export const handleFocusPointDrag = ( pointerCoords.y - offsetY, ); const bindingField = isStartBinding ? "startBinding" : "endBinding"; - const hit = getHoveredElementForFocusPoint( + const hit = getHoveredElementForBinding( point, - arrow, scene.getNonDeletedElements(), elementsMap, - maxBindingDistance_simple(appState.zoom), + appState.zoom, ); // Hovering a bindable element @@ -279,6 +277,7 @@ export const handleFocusPointDrag = ( newMode || "orbit", linearElementEditor.draggedFocusPointBinding, scene, + appState.zoom, point, ); } diff --git a/packages/element/src/binding.ts b/packages/element/src/binding.ts index a4925b1329..91d6ab4839 100644 --- a/packages/element/src/binding.ts +++ b/packages/element/src/binding.ts @@ -62,6 +62,7 @@ import { updateElbowArrowPoints } from "./elbowArrow"; import { deconstructDiamondElement, deconstructRectanguloidElement, + getElbowArrowSnapMidPoint, projectFixedPointOntoDiagonal, } from "./utils"; @@ -114,7 +115,6 @@ export type BindingStrategy = * IMPORTANT: currently must be > 0 (this also applies to the computed gap) */ export const BASE_BINDING_GAP = 5; -export const BASE_BINDING_GAP_ELBOW = 5; export const BASE_ARROW_MIN_LENGTH = 10; export const FOCUS_POINT_SIZE = 10 / 1.5; @@ -126,12 +126,8 @@ export const getBindingGap = ( // only the stroke width is needed, so the gap can also be computed for a // bind target that doesn't exist yet (see `getTextBindingForArrowEndpoint`) bindTarget: Pick, - opts: Pick, ): number => { - return ( - (opts.elbowed ? BASE_BINDING_GAP_ELBOW : BASE_BINDING_GAP) + - bindTarget.strokeWidth / 2 - ); + return BASE_BINDING_GAP + bindTarget.strokeWidth / 2; }; export const maxBindingDistance_simple = (zoom?: AppState["zoom"]): number => { @@ -191,6 +187,7 @@ export const bindOrUnbindBindingElement = ( start, "start", scene, + appState.zoom, appState.isBindingEnabled, isMidpointSnappingEnabled, ); @@ -199,6 +196,7 @@ export const bindOrUnbindBindingElement = ( end, "end", scene, + appState.zoom, appState.isBindingEnabled, isMidpointSnappingEnabled, ); @@ -244,6 +242,7 @@ const bindOrUnbindBindingElementEdge = ( { mode, element, focusPoint }: BindingStrategy, startOrEnd: "start" | "end", scene: Scene, + zoom: AppState["zoom"], shouldSnapToOutline = true, isMidpointSnappingEnabled = true, ): void => { @@ -257,6 +256,7 @@ const bindOrUnbindBindingElementEdge = ( mode, startOrEnd, scene, + zoom, focusPoint, shouldSnapToOutline, isMidpointSnappingEnabled, @@ -269,7 +269,7 @@ const bindingStrategyForElbowArrowEndpointDragging = ( draggingPoints: PointsPositionUpdates, elementsMap: NonDeletedSceneElementsMap, elements: readonly Ordered[], - zoom?: AppState["zoom"], + zoom: AppState["zoom"], ): { start: BindingStrategy; end: BindingStrategy; @@ -293,7 +293,7 @@ const bindingStrategyForElbowArrowEndpointDragging = ( globalPoint, elements, elementsMap, - maxBindingDistance_simple(zoom), + zoom, ); const current = hit @@ -341,7 +341,12 @@ const bindingStrategyForNewSimpleArrowEndpointDragging = ( draggingPoints.get(startDragged ? startIdx : endIdx)!.point, elementsMap, ); - const hit = getHoveredElementForBinding(point, elements, elementsMap); + const hit = getHoveredElementForBinding( + point, + elements, + elementsMap, + appState.zoom, + ); // With new arrows this handles the binding at arrow creation if (startDragged) { @@ -386,7 +391,12 @@ const bindingStrategyForNewSimpleArrowEndpointDragging = ( // Check and handle nested shapes if (hit && arrow.startBinding) { const startBinding = arrow.startBinding; - const allHits = getAllHoveredElementAtPoint(point, elements, elementsMap); + const allHits = getAllHoveredElementAtPoint( + point, + elements, + elementsMap, + appState.zoom, + ); if (allHits.find((el) => el.id === startBinding.elementId)) { const otherElement = elementsMap.get( @@ -482,15 +492,16 @@ const bindingStrategyForSimpleArrowEndpointDragging_complex = ( elements: readonly Ordered[], globalBindMode: AppState["bindMode"], arrow: NonDeleted, + zoom: AppState["zoom"], finalize?: boolean, ): { current: BindingStrategy; other: BindingStrategy } => { let current: BindingStrategy = { mode: undefined }; let other: BindingStrategy = { mode: undefined }; const isMultiPoint = arrow.points.length > 2; - const hit = getHoveredElementForBinding(point, elements, elementsMap); + const hit = getHoveredElementForBinding(point, elements, elementsMap, zoom); const isOverlapping = oppositeBinding - ? getAllHoveredElementAtPoint(point, elements, elementsMap).some( + ? getAllHoveredElementAtPoint(point, elements, elementsMap, zoom).some( (el) => el.id === oppositeBinding.elementId, ) : false; @@ -614,7 +625,6 @@ export const getBindingStrategyForDraggingBindingElementEndpoints = ( altKey?: boolean; finalize?: boolean; initialBinding?: boolean; - zoom?: AppState["zoom"]; gridSize?: NullableGridSize; }, ): { start: BindingStrategy; end: BindingStrategy } => { @@ -655,7 +665,6 @@ const getBindingStrategyForDraggingBindingElementEndpoints_simple = ( altKey?: boolean; finalize?: boolean; initialBinding?: boolean; - zoom?: AppState["zoom"]; gridSize?: NullableGridSize; }, ): { start: BindingStrategy; end: BindingStrategy } => { @@ -702,7 +711,7 @@ const getBindingStrategyForDraggingBindingElementEndpoints_simple = ( draggingPoints, elementsMap, elements, - opts?.zoom, + appState.zoom, ); } @@ -727,7 +736,7 @@ const getBindingStrategyForDraggingBindingElementEndpoints_simple = ( : globalPoint, elements, elementsMap, - maxBindingDistance_simple(appState.zoom), + appState.zoom, ); const pointInElement = hit && @@ -1009,6 +1018,7 @@ const getBindingStrategyForDraggingBindingElementEndpoints_complex = ( draggingPoints, elementsMap, elements, + appState.zoom, ); } @@ -1050,6 +1060,7 @@ const getBindingStrategyForDraggingBindingElementEndpoints_complex = ( elements, globalBindMode, arrow, + appState.zoom, opts?.finalize, ); @@ -1074,6 +1085,7 @@ const getBindingStrategyForDraggingBindingElementEndpoints_complex = ( elements, globalBindMode, arrow, + appState.zoom, opts?.finalize, ); @@ -1132,6 +1144,7 @@ export const bindBindingElement = ( mode: BindMode, startOrEnd: "start" | "end", scene: Scene, + zoom: AppState["zoom"], focusPoint?: GlobalPoint, shouldSnapToOutline = true, isMidpointSnappingEnabled = true, @@ -1149,6 +1162,7 @@ export const bindBindingElement = ( hoveredElement, startOrEnd, elementsMap, + zoom, shouldSnapToOutline, isMidpointSnappingEnabled, ), @@ -1205,6 +1219,7 @@ export const unbindBindingElement = ( export const reanchorBindingsToOutline = ( changedElement: NonDeleted, scene: Scene, + zoom: AppState["zoom"], ) => { const elementsMap = scene.getNonDeletedElementsMap(); const center = elementCenterPoint(changedElement, elementsMap); @@ -1236,6 +1251,7 @@ export const reanchorBindingsToOutline = ( changedElement, startOrEndName, elementsMap, + zoom, ), }; continue; @@ -1252,7 +1268,7 @@ export const reanchorBindingsToOutline = ( element: changedElement, point: focusPoint, elementsMap, - threshold: getBindingGap(changedElement, element), + threshold: getBindingGap(changedElement), overrideShouldTestInside: true, }) ) { @@ -1469,6 +1485,7 @@ const updateArrowBindings = ( strategy[strategyName].mode, strategyName, scene, + appState.zoom, strategy[strategyName].focusPoint, ); } @@ -1578,6 +1595,7 @@ export const bindPointToSnapToElementOutline = ( bindableElement: ExcalidrawBindableElement, startOrEnd: "start" | "end", elementsMap: ElementsMap, + zoom: AppState["zoom"], customIntersector?: LineSegment, isMidpointSnappingEnabled = true, ): GlobalPoint => { @@ -1610,19 +1628,24 @@ export const bindPointToSnapToElementOutline = ( startOrEnd === "start" ? 1 : -2, elementsMap, ); - const bindingGap = getBindingGap(bindableElement, arrowElement); + const bindingGap = getBindingGap(bindableElement); const aabb = aabbForElement(bindableElement, elementsMap); const bindableCenter = getCenterForBounds(aabb); let intersection: GlobalPoint | null = null; if (elbowed) { - const isHorizontal = headingIsHorizontal( - headingForPointFromElement(bindableElement, aabb, point), - ); - const snapPoint = isMidpointSnappingEnabled - ? snapToMid(bindableElement, elementsMap, edgePoint, 0.05, arrowElement) + const snap = isMidpointSnappingEnabled + ? getElbowArrowSnapMidPoint(edgePoint, bindableElement, elementsMap, zoom) : undefined; - const resolved = snapPoint || point; + const resolved = snap?.point ?? point; + // A midpoint on the element's axes fixes the side to bind to. The pointer + // can report another side, e.g. horizontal when slightly inside a + // diamond's top vertex, which would bind next to the vertex. + const isHorizontal = headingIsHorizontal( + snap?.onAxis + ? vectorToHeading(vectorFromPoint(snap.point, bindableCenter)) + : headingForPointFromElement(bindableElement, aabb, point), + ); const otherPoint = pointFrom( isHorizontal ? bindableCenter[0] : resolved[0], !isHorizontal ? bindableCenter[1] : resolved[1], @@ -1639,12 +1662,16 @@ export const bindPointToSnapToElementOutline = ( otherPoint, ), ); + // The ray can cross the outline twice (e.g. a thin rotated diamond), so + // take the crossing nearest to the point we aim at + const byDistanceToResolved = (a: GlobalPoint, b: GlobalPoint) => + pointDistanceSq(a, resolved) - pointDistanceSq(b, resolved); intersection = intersectElementWithLineSegment( bindableElement, elementsMap, intersector, bindingGap, - ).sort(pointDistanceSq)[0]; + ).sort(byDistanceToResolved)[0]; if (!intersection) { const anotherPoint = pointFrom( @@ -1665,8 +1692,8 @@ export const bindPointToSnapToElementOutline = ( bindableElement, elementsMap, anotherIntersector, - BASE_BINDING_GAP_ELBOW, - ).sort(pointDistanceSq)[0]; + BASE_BINDING_GAP, + ).sort(byDistanceToResolved)[0]; } } else { let intersector = customIntersector; @@ -1724,7 +1751,7 @@ export const avoidRectangularCorner = ( -bindTarget.angle as Radians, ); - const bindingGap = getBindingGap(bindTarget, arrowElement); + const bindingGap = getBindingGap(bindTarget); if (nonRotatedPoint[0] < bindTarget.x && nonRotatedPoint[1] < bindTarget.y) { // Top left @@ -1802,121 +1829,6 @@ export const avoidRectangularCorner = ( return p; }; -export const snapToMid = ( - bindTarget: ExcalidrawBindableElement, - elementsMap: ElementsMap, - p: GlobalPoint, - tolerance: number = 0.05, - arrowElement?: ExcalidrawArrowElement, -): GlobalPoint | undefined => { - const { x, y, width, height, angle } = bindTarget; - const center = elementCenterPoint(bindTarget, elementsMap, -0.1, -0.1); - const nonRotated = pointRotateRads(p, center, -angle as Radians); - - const bindingGap = arrowElement ? getBindingGap(bindTarget, arrowElement) : 0; - - // snap-to-center point is adaptive to element size, but we don't want to go - // above and below certain px distance - const verticalThreshold = clamp(tolerance * height, 5, 80); - const horizontalThreshold = clamp(tolerance * width, 5, 80); - - // Too close to the center makes it hard to resolve direction precisely - if (pointDistance(center, nonRotated) < bindingGap) { - return undefined; - } - - if ( - nonRotated[0] <= x + width / 2 && - nonRotated[1] > center[1] - verticalThreshold && - nonRotated[1] < center[1] + verticalThreshold - ) { - // LEFT - return pointRotateRads( - pointFrom(x - bindingGap, center[1]), - center, - angle, - ); - } else if ( - nonRotated[1] <= y + height / 2 && - nonRotated[0] > center[0] - horizontalThreshold && - nonRotated[0] < center[0] + horizontalThreshold - ) { - // TOP - return pointRotateRads( - pointFrom(center[0], y - bindingGap), - center, - angle, - ); - } else if ( - nonRotated[0] >= x + width / 2 && - nonRotated[1] > center[1] - verticalThreshold && - nonRotated[1] < center[1] + verticalThreshold - ) { - // RIGHT - return pointRotateRads( - pointFrom(x + width + bindingGap, center[1]), - center, - angle, - ); - } else if ( - nonRotated[1] >= y + height / 2 && - nonRotated[0] > center[0] - horizontalThreshold && - nonRotated[0] < center[0] + horizontalThreshold - ) { - // DOWN - return pointRotateRads( - pointFrom(center[0], y + height + bindingGap), - center, - angle, - ); - } else if (bindTarget.type === "diamond") { - const distance = bindingGap; - const topLeft = pointFrom( - x + width / 4 - distance, - y + height / 4 - distance, - ); - const topRight = pointFrom( - x + (3 * width) / 4 + distance, - y + height / 4 - distance, - ); - const bottomLeft = pointFrom( - x + width / 4 - distance, - y + (3 * height) / 4 + distance, - ); - const bottomRight = pointFrom( - x + (3 * width) / 4 + distance, - y + (3 * height) / 4 + distance, - ); - - if ( - pointDistance(topLeft, nonRotated) < - Math.max(horizontalThreshold, verticalThreshold) - ) { - return pointRotateRads(topLeft, center, angle); - } - if ( - pointDistance(topRight, nonRotated) < - Math.max(horizontalThreshold, verticalThreshold) - ) { - return pointRotateRads(topRight, center, angle); - } - if ( - pointDistance(bottomLeft, nonRotated) < - Math.max(horizontalThreshold, verticalThreshold) - ) { - return pointRotateRads(bottomLeft, center, angle); - } - if ( - pointDistance(bottomRight, nonRotated) < - Math.max(horizontalThreshold, verticalThreshold) - ) { - return pointRotateRads(bottomRight, center, angle); - } - } - - return undefined; -}; - const extractBinding = ( arrow: ExcalidrawArrowElement, startOrEnd: "startBinding" | "endBinding", @@ -1979,7 +1891,7 @@ const snapBoundPointToGrid = ( ? vectorToHeading(vectorFromPoint(adjacentPoint, outlinePoint)) : headingForPointFromElement(bindableElement, aabb, outlinePoint); - const bindingGap = getBindingGap(bindableElement, arrowElement); + const bindingGap = getBindingGap(bindableElement); const extent = Math.max(bindableElement.width, bindableElement.height) + bindingGap * 2; const center = getCenterForBounds(aabb); @@ -2097,7 +2009,7 @@ export const updateBoundPoint = ( otherBindable, elementsMap, intersector, - getBindingGap(otherBindable, arrow), + getBindingGap(otherBindable), ).sort( (a, b) => pointDistanceSq(a, focusPoint) - pointDistanceSq(b, focusPoint), )[0]; @@ -2107,7 +2019,7 @@ export const updateBoundPoint = ( bindableElement, elementsMap, intersector, - getBindingGap(bindableElement, arrow), + getBindingGap(bindableElement), ).sort( (a, b) => pointDistanceSq(a, otherFocusPointOrArrowPoint) - @@ -2135,7 +2047,7 @@ export const updateBoundPoint = ( element: otherBindable, point: outlinePoint, elementsMap, - threshold: getBindingGap(otherBindable, arrow), + threshold: getBindingGap(otherBindable), overrideShouldTestInside: true, }) ) { @@ -2196,6 +2108,7 @@ export const calculateFixedPointForElbowArrowBinding = ( hoveredElement: ExcalidrawBindableElement, startOrEnd: "start" | "end", elementsMap: ElementsMap, + zoom: AppState["zoom"], shouldSnapToOutline = true, isMidpointSnappingEnabled = true, ): { fixedPoint: FixedPoint } => { @@ -2211,6 +2124,7 @@ export const calculateFixedPointForElbowArrowBinding = ( hoveredElement, startOrEnd, elementsMap, + zoom, undefined, isMidpointSnappingEnabled, ) @@ -2243,7 +2157,7 @@ export const calculateFixedPointForElbowArrowBinding = ( // elements smaller than the gap don't blow up the ratio: the snapped // point sits at most one gap-width outside the outline, so this keeps // the resulting fixedPoint bounded even for zero/near-zero-size elements. - const sizeFloor = getBindingGap(hoveredElement, linearElement); + const sizeFloor = getBindingGap(hoveredElement); return { fixedPoint: normalizeFixedPoint([ @@ -2293,7 +2207,7 @@ export const calculateFixedPointForNonElbowArrowBinding = ( // elements smaller than the gap don't blow up the ratio: the bound // point sits at most one gap-width outside the outline, so this keeps // the resulting fixedPoint bounded even for zero/near-zero-size elements. - const sizeFloor = getBindingGap(hoveredElement, linearElement); + const sizeFloor = getBindingGap(hoveredElement); // Calculate the ratio relative to the element's bounds const fixedPointX = diff --git a/packages/element/src/collision.ts b/packages/element/src/collision.ts index 7fe6e1f9df..809d26ea9b 100644 --- a/packages/element/src/collision.ts +++ b/packages/element/src/collision.ts @@ -27,7 +27,7 @@ import type { Radians, } from "@excalidraw/math"; -import type { FrameNameBounds } from "@excalidraw/excalidraw/types"; +import type { AppState, FrameNameBounds } from "@excalidraw/excalidraw/types"; import { isPathALoop } from "./utils"; import { @@ -61,7 +61,7 @@ import { LinearElementEditor } from "./linearElementEditor"; import { distanceToElement } from "./distance"; -import { getBindingGap } from "./binding"; +import { maxBindingDistance_simple } from "./binding"; import { hasBackground } from "./comparisons"; @@ -69,7 +69,6 @@ import { getFreedrawFillPolygon, getFreedrawMaxStrokeRadius } from "./shape"; import type { ElementsMap, - ExcalidrawArrowElement, ExcalidrawBindableElement, ExcalidrawDiamondElement, ExcalidrawElement, @@ -279,62 +278,81 @@ export const hitElementBoundText = ( return isPointInElement(point, boundTextElement, elementsMap); }; -const bindingBorderTest = ( - element: NonDeleted, - [x, y]: Readonly, - elementsMap: NonDeletedSceneElementsMap, - tolerance: number = 0, -): boolean => { - const p = pointFrom(x, y); - const shouldTestInside = - // disable fullshape snapping for frame elements so we - // can bind to frame children - !isFrameLikeElement(element); +// Frame children are clipped to their enclosing frame, so a point outside +// the frame cannot hit them +const isPointClippedByEnclosingFrame = ( + element: ExcalidrawElement, + point: Readonly, + elementsMap: ElementsMap, +) => { + if (!element.frameId) { + return false; + } + const enclosingFrame = elementsMap.get(element.frameId); + + return ( + !!enclosingFrame && + isFrameLikeElement(enclosingFrame) && + !pointInsideBounds(point, getElementBounds(enclosingFrame, elementsMap)) + ); +}; + +const bindableElementBorderDistanceIfClose = ( + element: NonDeleted, + point: GlobalPoint, + elementsMap: ElementsMap, + tolerance: number = 0, +) => { // PERF: Run a cheap test to see if the binding element // is even close to the element + const [x, y] = point; const t = Math.max(1, tolerance); const bounds = [x - t, y - t, x + t, y + t] as Bounds; const elementBounds = getElementBounds(element, elementsMap); if (!doBoundsIntersect(bounds, elementBounds)) { - return false; + return -Infinity; } - // If the element is inside a frame, we should clip the element - if (element.frameId) { - const enclosingFrame = elementsMap.get(element.frameId); - if (enclosingFrame && isFrameLikeElement(enclosingFrame)) { - const enclosingFrameBounds = getElementBounds( - enclosingFrame, - elementsMap, - ); - if (!pointInsideBounds(p, enclosingFrameBounds)) { - return false; - } - } + if (isPointClippedByEnclosingFrame(element, point, elementsMap)) { + return -Infinity; } - // Do the intersection test against the element since it's close enough - const intersections = intersectElementWithLineSegment( - element, - elementsMap, - lineSegment(elementCenterPoint(element, elementsMap), p), - ); - const distance = distanceToElement(element, elementsMap, p); + const isInside = isPointInElement(point, element, elementsMap); + // frames are only bindable from the outside, so arrows ending inside + // a frame can bind to its children (or stay unbound) + if (isInside && isFrameLikeElement(element)) { + return -Infinity; + } - return shouldTestInside - ? intersections.length === 0 || distance <= tolerance - : intersections.length > 0 && distance <= t; + const distance = distanceToElement(element, elementsMap, point); + if (isInside) { + return distance; + } + + return distance > tolerance ? -Infinity : -distance; }; -export const getAllHoveredElementAtPoint = ( +type BindingCandidate = { + element: NonDeleted; + /** distance to the outline: positive inside, negative outside */ + distance: number; +}; + +/** + * Bindable elements within binding distance of the point, front to back. + * Stops at the first opaque element containing the point, since it hides + * everything behind it. + */ +const getBindingCandidates = ( point: Readonly, elements: readonly Ordered[], elementsMap: NonDeletedSceneElementsMap, - tolerance?: number, -): NonDeleted[] => { - const candidateElements: NonDeleted[] = []; - // We need to to hit testing from front (end of the array) to back (beginning of the array) + zoom: AppState["zoom"], +): BindingCandidate[] => { + const maxDistance = maxBindingDistance_simple(zoom); + const candidates: BindingCandidate[] = []; + // We need to do hit testing from front (end of the array) to back (beginning of the array) // because array is ordered from lower z-index to highest and we want element z-index // with higher z-index for (let index = elements.length - 1; index >= 0; --index) { @@ -342,104 +360,98 @@ export const getAllHoveredElementAtPoint = ( invariant( !element.isDeleted, - "Elements in the function parameter for getAllElementsAtPositionForBinding() should not contain deleted elements", + "Elements passed to binding hit tests should not contain deleted elements", ); - if ( - isBindableElement(element, false) && - bindingBorderTest(element, point, elementsMap, tolerance) - ) { - candidateElements.push(element); + if (!isBindableElement(element, false)) { + continue; + } + + const distance = bindableElementBorderDistanceIfClose( + element, + point, + elementsMap, + maxDistance, + ); + + if (distance > -maxDistance) { + candidates.push({ element, distance }); if ( hasBackground(element.type) && - !isTransparent(element.backgroundColor) + !isTransparent(element.backgroundColor) && + distance >= 0 ) { break; } } } - return candidateElements; + return candidates; }; +/** + * All elements an arrow endpoint at the point could bind to. Always includes + * the result of `getHoveredElementForBinding` for the same arguments. + */ +export const getAllHoveredElementAtPoint = ( + point: Readonly, + elements: readonly Ordered[], + elementsMap: NonDeletedSceneElementsMap, + zoom: AppState["zoom"], +): NonDeleted[] => + getBindingCandidates(point, elements, elementsMap, zoom).map( + ({ element }) => element, + ); + export const getHoveredElementForBinding = ( point: Readonly, elements: readonly Ordered[], elementsMap: NonDeletedSceneElementsMap, - tolerance?: number, + zoom: AppState["zoom"], ): NonDeleted | null => { - const candidateElements = getAllHoveredElementAtPoint( - point, - elements, - elementsMap, - tolerance, + const candidates = getBindingCandidates(point, elements, elementsMap, zoom); + + if (candidates.length === 0) { + return null; + } + + if (candidates.length === 1) { + return candidates[0].element; + } + + const closestElements = candidates.sort( + (a, b) => Math.abs(a.distance) - Math.abs(b.distance), ); - if (!candidateElements || candidateElements.length === 0) { - return null; - } + const candidate = closestElements[0]; + const [cx1, cy1, cx2, cy2] = getElementBounds(candidate.element, elementsMap); + const candidateArea = Math.max( + 0.00001, + Math.abs(cx2 - cx1) * Math.abs(cy2 - cy1), + ); + // A smaller element overlapping the closest one takes precedence, but only + // when the point is inside it: otherwise the closest outline wins, e.g. an + // arrow ending just inside a container's edge next to a nested element + const overlaps = closestElements + .filter((c) => c.element !== candidate.element && c.distance >= 0) + .map((c) => { + const [x1, y1, x2, y2] = getElementBounds(c.element, elementsMap); + const overlapWidth = Math.max(0, Math.min(x2, cx2) - Math.max(x1, cx1)); + const overlapHeight = Math.max(0, Math.min(y2, cy2) - Math.max(y1, cy1)); + const area = Math.max(0.00001, Math.abs(x2 - x1) * Math.abs(y2 - y1)); - if (candidateElements.length === 1) { - return candidateElements[0]; - } + return { + ...c, + overlapPercent: (overlapHeight * overlapWidth) / area, + relativeArea: area / candidateArea, + }; + }) + .filter((c) => c.overlapPercent > 0.25 && c.relativeArea < 0.75); - // Prefer smaller shapes - return candidateElements - .sort( - (a, b) => b.width ** 2 + b.height ** 2 - (a.width ** 2 + a.height ** 2), - ) - .pop() as NonDeleted; -}; - -export const getHoveredElementForFocusPoint = ( - point: GlobalPoint, - arrow: ExcalidrawArrowElement, - elements: readonly Ordered[], - elementsMap: NonDeletedSceneElementsMap, - tolerance?: number, -): NonDeleted | null => { - const candidateElements: NonDeleted[] = []; - // We need to to hit testing from front (end of the array) to back (beginning of the array) - // because array is ordered from lower z-index to highest and we want element z-index - // with higher z-index - for (let index = elements.length - 1; index >= 0; --index) { - const element = elements[index]; - - invariant( - !element.isDeleted, - "Elements in the function parameter for getAllElementsAtPositionForBinding() should not contain deleted elements", - ); - - if ( - isBindableElement(element, false) && - bindingBorderTest(element, point, elementsMap, tolerance) - ) { - candidateElements.push(element); - } - } - - if (!candidateElements || candidateElements.length === 0) { - return null; - } - - if (candidateElements.length === 1) { - return candidateElements[0]; - } - - const distanceFilteredCandidateElements = candidateElements - // Resolve by distance - .filter( - (el) => - distanceToElement(el, elementsMap, point) <= getBindingGap(el, arrow) || - isPointInElement(point, el, elementsMap), - ); - - if (distanceFilteredCandidateElements.length === 0) { - return null; - } - - return distanceFilteredCandidateElements[0] as NonDeleted; + return candidate.distance >= 0 && overlaps.length > 0 + ? overlaps[0].element + : candidate.element; }; /** @@ -516,6 +528,8 @@ export const intersectElementWithLineSegment = ( } }; +const CURVE_BOUNDS_EPSILON = 1e-6; + const curveIntersections = ( curves: Curve[], segment: LineSegment, @@ -524,15 +538,19 @@ const curveIntersections = ( angle: Radians, onlyFirst = false, ) => { + // Pad the segment bounds so an axis-aligned segment passing exactly through + // the joint of two curves still overlaps their bounds, as + // `doBoundsIntersect` treats bounds that only touch as not intersecting + const b2 = [ + Math.min(segment[0][0], segment[1][0]) - CURVE_BOUNDS_EPSILON, + Math.min(segment[0][1], segment[1][1]) - CURVE_BOUNDS_EPSILON, + Math.max(segment[0][0], segment[1][0]) + CURVE_BOUNDS_EPSILON, + Math.max(segment[0][1], segment[1][1]) + CURVE_BOUNDS_EPSILON, + ] as Bounds; + for (const c of curves) { // Optimize by doing a cheap bounding box check first const b1 = getCubicBezierCurveBound(c[0], c[1], c[2], c[3]); - const b2 = [ - Math.min(segment[0][0], segment[1][0]), - Math.min(segment[0][1], segment[1][1]), - Math.max(segment[0][0], segment[1][0]), - Math.max(segment[0][1], segment[1][1]), - ] as Bounds; if (!doBoundsIntersect(b1, b2)) { continue; diff --git a/packages/element/src/dragElements.ts b/packages/element/src/dragElements.ts index 44137eae5a..a63f17624c 100644 --- a/packages/element/src/dragElements.ts +++ b/packages/element/src/dragElements.ts @@ -108,18 +108,6 @@ export const dragSelectedElements = ( ); elementsToUpdate.forEach((element) => { - const isArrow = !isArrowElement(element); - const isStartBoundElementSelected = - isArrow || - (element.startBinding - ? elementsToUpdateIds.has(element.startBinding.elementId) - : false); - const isEndBoundElementSelected = - isArrow || - (element.endBinding - ? elementsToUpdateIds.has(element.endBinding.elementId) - : false); - if (!isArrowElement(element)) { updateElementCoords(pointerDownState, element, scene, adjustedOffset); @@ -143,7 +131,6 @@ export const dragSelectedElements = ( // NOTE: Add a little initial drag to the arrow dragging when the arrow // is the single element being dragged to avoid accidentally unbinding // the arrow when the user just wants to select it. - elementsToUpdate.size > 1 || Math.max(Math.abs(adjustedOffset.x), Math.abs(adjustedOffset.y)) > DRAGGING_THRESHOLD || @@ -151,9 +138,12 @@ export const dragSelectedElements = ( ) { updateElementCoords(pointerDownState, element, scene, adjustedOffset); - const shouldUnbindStart = - element.startBinding && !isStartBoundElementSelected; - const shouldUnbindEnd = element.endBinding && !isEndBoundElementSelected; + const shouldUnbindStart = element.startBinding + ? !elementsToUpdateIds.has(element.startBinding.elementId) + : true; + const shouldUnbindEnd = element.endBinding + ? !elementsToUpdateIds.has(element.endBinding.elementId) + : true; if (shouldUnbindStart || shouldUnbindEnd) { // NOTE: Moving the bound arrow should unbind it, otherwise we would // have weird situations, like 0 lenght arrow when the user moves diff --git a/packages/element/src/elbowArrow.ts b/packages/element/src/elbowArrow.ts index 024b846b12..18eb986848 100644 --- a/packages/element/src/elbowArrow.ts +++ b/packages/element/src/elbowArrow.ts @@ -30,8 +30,7 @@ import { getHeadingForElbowArrowSnap, getGlobalFixedPointForBindableElement, getBindingGap, - maxBindingDistance_simple, - BASE_BINDING_GAP_ELBOW, + BASE_BINDING_GAP, } from "./binding"; import { distanceToElement } from "./distance"; import { @@ -1217,24 +1216,20 @@ const getElbowArrowData = ( GlobalPoint >(nextPoints[nextPoints.length - 1], vector(arrow.x, arrow.y)); + // NOTE: updates coming through `mutateElement` don't carry the zoom, so + // those fall back to the zoom=1 binding distance + const zoom = options?.zoom ?? ({ value: 1 } as AppState["zoom"]); + let hoveredStartElement = null; let hoveredEndElement = null; if (options?.isDragging && options?.isBindingEnabled !== false) { const elements = Array.from(elementsMap.values()); hoveredStartElement = - getHoveredElement( - origStartGlobalPoint, - elementsMap, - elements, - options?.zoom, - ) || null; + getHoveredElement(origStartGlobalPoint, elementsMap, elements, zoom) || + null; hoveredEndElement = - getHoveredElement( - origEndGlobalPoint, - elementsMap, - elements, - options?.zoom, - ) || null; + getHoveredElement(origEndGlobalPoint, elementsMap, elements, zoom) || + null; } else { hoveredStartElement = arrow.startBinding ? getBindableElementForId(arrow.startBinding.elementId, elementsMap) || @@ -1256,6 +1251,7 @@ const getElbowArrowData = ( "start", arrow.startBinding?.fixedPoint, origStartGlobalPoint, + zoom, hoveredStartElement, elementsMap, options?.isDragging, @@ -1273,6 +1269,7 @@ const getElbowArrowData = ( "end", arrow.endBinding?.fixedPoint, origEndGlobalPoint, + zoom, hoveredEndElement, elementsMap, options?.isDragging, @@ -1285,7 +1282,7 @@ const getElbowArrowData = ( hoveredStartElement, origStartGlobalPoint, elementsMap, - options?.zoom, + zoom, ); const endHeading = getBindPointHeading( endGlobalPoint, @@ -1293,7 +1290,7 @@ const getElbowArrowData = ( hoveredEndElement, origEndGlobalPoint, elementsMap, - options?.zoom, + zoom, ); const startPointBounds = [ startGlobalPoint[0] - 2, @@ -1314,8 +1311,8 @@ const getElbowArrowData = ( offsetFromHeading( startHeading, arrow.startArrowhead - ? getBindingGap(hoveredStartElement, { elbowed: true }) * 6 - : getBindingGap(hoveredStartElement, { elbowed: true }) * 2, + ? getBindingGap(hoveredStartElement) * 6 + : getBindingGap(hoveredStartElement) * 2, 1, ), ) @@ -1327,8 +1324,8 @@ const getElbowArrowData = ( offsetFromHeading( endHeading, arrow.endArrowhead - ? getBindingGap(hoveredEndElement, { elbowed: true }) * 6 - : getBindingGap(hoveredEndElement, { elbowed: true }) * 2, + ? getBindingGap(hoveredEndElement) * 6 + : getBindingGap(hoveredEndElement) * 2, 1, ), ) @@ -1375,8 +1372,8 @@ const getElbowArrowData = ( ? 0 : BASE_PADDING - (arrow.startArrowhead - ? BASE_BINDING_GAP_ELBOW * 6 - : BASE_BINDING_GAP_ELBOW * 2), + ? BASE_BINDING_GAP * 6 + : BASE_BINDING_GAP * 2), BASE_PADDING, ), boundsOverlap @@ -1391,8 +1388,8 @@ const getElbowArrowData = ( ? 0 : BASE_PADDING - (arrow.endArrowhead - ? BASE_BINDING_GAP_ELBOW * 6 - : BASE_BINDING_GAP_ELBOW * 2), + ? BASE_BINDING_GAP * 6 + : BASE_BINDING_GAP * 2), BASE_PADDING, ), boundsOverlap, @@ -2218,6 +2215,7 @@ const getGlobalPoint = ( startOrEnd: "start" | "end", fixedPointRatio: [number, number] | undefined | null, initialPoint: GlobalPoint, + zoom: AppState["zoom"], element?: ExcalidrawBindableElement | null, elementsMap?: ElementsMap, isDragging?: boolean, @@ -2231,6 +2229,7 @@ const getGlobalPoint = ( element, startOrEnd, elementsMap, + zoom, undefined, isMidpointSnappingEnabled, ); @@ -2282,14 +2281,9 @@ const getHoveredElement = ( origPoint: GlobalPoint, elementsMap: NonDeletedSceneElementsMap, elements: readonly Ordered[], - zoom?: AppState["zoom"], + zoom: AppState["zoom"], ) => { - return getHoveredElementForBinding( - origPoint, - elements, - elementsMap, - maxBindingDistance_simple(zoom), - ); + return getHoveredElementForBinding(origPoint, elements, elementsMap, zoom); }; const gridAddressesEqual = (a: GridAddress, b: GridAddress): boolean => diff --git a/packages/element/src/flowchart.ts b/packages/element/src/flowchart.ts index 432abb3b5d..aa59a5ff00 100644 --- a/packages/element/src/flowchart.ts +++ b/packages/element/src/flowchart.ts @@ -391,8 +391,16 @@ const createBindingArrow = ( "orbit", "start", scene, + appState.zoom, + ); + bindBindingElement( + bindingArrow, + endBindingElement, + "orbit", + "end", + scene, + appState.zoom, ); - bindBindingElement(bindingArrow, endBindingElement, "orbit", "end", scene); const changedElements = new Map(); changedElements.set( diff --git a/packages/element/src/linearElementEditor.ts b/packages/element/src/linearElementEditor.ts index cfc4a06ff7..4274b54228 100644 --- a/packages/element/src/linearElementEditor.ts +++ b/packages/element/src/linearElementEditor.ts @@ -58,7 +58,6 @@ import { calculateFixedPointForNonElbowArrowBinding, getBindingStrategyForDraggingBindingElementEndpoints, isBindingEnabled, - snapToMid, updateBoundPoint, } from "./binding"; import { @@ -365,7 +364,7 @@ export class LinearElementEditor { // Apply the point movement if needed let suggestedBinding: AppState["suggestedBinding"] = null; - const { positions, updates } = pointDraggingUpdates( + const { positions, updates, hit } = pointDraggingUpdates( [idx], deltaX, deltaY, @@ -407,17 +406,20 @@ export class LinearElementEditor { // Move the arrow over the bindable object in terms of z-index if (isBindingElement(element)) { - moveArrowAboveBindable( - LinearElementEditor.getPointGlobalCoordinates( + if (hit) { + moveArrowAboveBindable( + LinearElementEditor.getPointGlobalCoordinates( + element, + element.points[element.points.length - 1], + elementsMap, + ), element, - element.points[element.points.length - 1], + elements, elementsMap, - ), - element, - elements, - elementsMap, - app.scene, - ); + app.scene, + hit, + ); + } } // PERF: Avoid state updates if not absolutely necessary @@ -566,7 +568,7 @@ export class LinearElementEditor { // Apply the point movement if needed let suggestedBinding: AppState["suggestedBinding"] = null; - const { positions, updates } = pointDraggingUpdates( + const { positions, updates, hit } = pointDraggingUpdates( selectedPointsIndices, deltaX, deltaY, @@ -610,19 +612,22 @@ export class LinearElementEditor { // Move the arrow over the bindable object in terms of z-index if (isBindingElement(element) && startIsSelected !== endIsSelected) { - moveArrowAboveBindable( - LinearElementEditor.getPointGlobalCoordinates( + if (hit) { + moveArrowAboveBindable( + LinearElementEditor.getPointGlobalCoordinates( + element, + startIsSelected + ? element.points[0] + : element.points[element.points.length - 1], + elementsMap, + ), element, - startIsSelected - ? element.points[0] - : element.points[element.points.length - 1], + elements, elementsMap, - ), - element, - elements, - elementsMap, - app.scene, - ); + app.scene, + hit, + ); + } } // Attached text might need to update if arrow dimensions change @@ -2385,6 +2390,7 @@ const pointDraggingUpdates = ( ): { positions: PointsPositionUpdates; updates?: PointMoveOtherUpdates; + hit?: NonDeleted | null; } => { const naiveDraggingPoints = new Map( selectedPointsIndices.map((pointIndex) => { @@ -2443,18 +2449,21 @@ const pointDraggingUpdates = ( ? { element: suggestedBindingElement, midPoint: app.state.isMidpointSnappingEnabled - ? snapToMid( - suggestedBindingElement, - elementsMap, + ? getSnapOutlineMidPoint( pointFrom( scenePointerX - linearElementEditor.pointerOffset.x, scenePointerY - linearElementEditor.pointerOffset.y, ), + suggestedBindingElement, + elementsMap, + app.state.zoom, + element, ) : undefined, } : null, }, + hit: suggestedBindingElement, }; } @@ -2556,6 +2565,7 @@ const pointDraggingUpdates = ( start.element, elementsMap, app.state.zoom, + element, ), } : null; @@ -2595,6 +2605,7 @@ const pointDraggingUpdates = ( end.element, elementsMap, app.state.zoom, + element, ), } : null; @@ -2744,6 +2755,7 @@ const pointDraggingUpdates = ( ]; }), ), + hit: startIsDragged ? start.element : end.element, }; }; diff --git a/packages/element/src/transform.ts b/packages/element/src/transform.ts index 7199ec1dc5..fe3130e200 100644 --- a/packages/element/src/transform.ts +++ b/packages/element/src/transform.ts @@ -20,6 +20,7 @@ import { } from "@excalidraw/common"; import type { MarkOptional } from "@excalidraw/common/utility-types"; +import type { Zoom } from "@excalidraw/excalidraw/types"; import { bindBindingElement } from "./binding"; import { @@ -303,6 +304,7 @@ const bindLinearElementToElement = ( end: ValidLinearElement["end"], elementStore: ElementStore, scene: Scene, + zoom: Zoom, ): { linearElement: ExcalidrawLinearElement; startBoundElement?: ExcalidrawElement; @@ -390,6 +392,7 @@ const bindLinearElementToElement = ( "orbit", "start", scene, + zoom, ); } } @@ -466,6 +469,7 @@ const bindLinearElementToElement = ( "orbit", "end", scene, + zoom, ); } } @@ -782,6 +786,7 @@ export const convertToExcalidrawElements = ( originalEnd, elementStore, scene, + { value: 1 } as Zoom, ); container = linearElement; elementStore.add(linearElement); @@ -807,6 +812,7 @@ export const convertToExcalidrawElements = ( end, elementStore, scene, + { value: 1 } as Zoom, ); elementStore.add(linearElement); diff --git a/packages/element/src/utils.ts b/packages/element/src/utils.ts index d1f2aa8ccb..79a72183a8 100644 --- a/packages/element/src/utils.ts +++ b/packages/element/src/utils.ts @@ -8,6 +8,7 @@ import { import { bezierEquation, + clamp, curve, curveCatmullRomCubicApproxPoints, curveOffsetPoints, @@ -26,7 +27,7 @@ import { type GlobalPoint, } from "@excalidraw/math"; -import type { Curve, LineSegment, LocalPoint } from "@excalidraw/math"; +import type { Curve, LineSegment, LocalPoint, Radians } from "@excalidraw/math"; import type { AppState, @@ -41,7 +42,7 @@ import { generateLinearCollisionShape } from "./shape"; import { hitElementItself, isPointInElement } from "./collision"; import { LinearElementEditor } from "./linearElementEditor"; import { isElbowArrow, isRectangularElement } from "./typeChecks"; -import { maxBindingDistance_simple } from "./binding"; +import { getBindingGap, maxBindingDistance_simple } from "./binding"; import { getGlobalFixedPointForBindableElement, @@ -631,67 +632,179 @@ const getDiagonalsForBindableElement = ( return [diagonalOne, diagonalTwo]; }; -export const getSnapOutlineMidPoint = ( - point: GlobalPoint, +/** + * `onAxis` is true for the midpoints on the element's axes (the side midpoints, + * or a diamond's vertices), whose direction from the center is unambiguous, + * and false for a diamond's edge midpoints + */ +const getSnappedMidpointForElbowArrow = ( element: ExcalidrawBindableElement, + point: GlobalPoint, elementsMap: ElementsMap, - zoom: AppState["zoom"], -) => { - const center = elementCenterPoint(element, elementsMap); - const sideMidpoints = - element.type === "diamond" - ? getDiamondBaseCorners(element).map((curve) => { - const point = bezierEquation(curve, 0.5); - const rotatedPoint = pointRotateRads(point, center, element.angle); + center: GlobalPoint, + horizontalThreshold: number, + verticalThreshold: number, +): { point: GlobalPoint; onAxis: boolean } | undefined => { + const { x, y, width, height, angle } = element; + const nonRotated = pointRotateRads(point, center, -angle as Radians); - return pointFrom(rotatedPoint[0], rotatedPoint[1]); - }) - : [ - // RIGHT midpoint - pointRotateRads( - pointFrom( - element.x + element.width, - element.y + element.height / 2, - ), + const bindingGap = getBindingGap(element); + + if (pointDistance(center, nonRotated) < bindingGap) { + return undefined; + } + + const [right, bottom, left, top] = getAllMidpoints(element, elementsMap); + + if ( + nonRotated[0] <= x + width / 2 && + nonRotated[1] > center[1] - verticalThreshold && + nonRotated[1] < center[1] + verticalThreshold + ) { + return { point: left, onAxis: true }; + } else if ( + nonRotated[1] <= y + height / 2 && + nonRotated[0] > center[0] - horizontalThreshold && + nonRotated[0] < center[0] + horizontalThreshold + ) { + return { point: top, onAxis: true }; + } else if ( + nonRotated[0] >= x + width / 2 && + nonRotated[1] > center[1] - verticalThreshold && + nonRotated[1] < center[1] + verticalThreshold + ) { + return { point: right, onAxis: true }; + } else if ( + nonRotated[1] >= y + height / 2 && + nonRotated[0] > center[0] - horizontalThreshold && + nonRotated[0] < center[0] + horizontalThreshold + ) { + return { point: bottom, onAxis: true }; + } else if (element.type === "diamond") { + // Elbow arrows can also snap to the midpoints of a diamond's edges + const threshold = Math.max(horizontalThreshold, verticalThreshold); + const edgeMidpoints = [ + // top-left, top-right, bottom-left, bottom-right + [x + width / 4, y + height / 4, -1, -1], + [x + (3 * width) / 4, y + height / 4, 1, -1], + [x + width / 4, y + (3 * height) / 4, -1, 1], + [x + (3 * width) / 4, y + (3 * height) / 4, 1, 1], + ] as const; + + for (const [edgeX, edgeY, dirX, dirY] of edgeMidpoints) { + // the snap zone sits a binding gap outside the edge + const zoneCenter = pointFrom( + edgeX + dirX * bindingGap, + edgeY + dirY * bindingGap, + ); + + if (pointDistance(zoneCenter, nonRotated) < threshold) { + return { + point: pointRotateRads( + pointFrom(edgeX, edgeY), center, - element.angle, + angle, ), - // BOTTOM midpoint - pointRotateRads( - pointFrom( - element.x + element.width / 2, - element.y + element.height, - ), - center, - element.angle, - ), - // LEFT midpoint - pointRotateRads( - pointFrom(element.x, element.y + element.height / 2), - center, - element.angle, - ), - // TOP midpoint - pointRotateRads( - pointFrom(element.x + element.width / 2, element.y), - center, - element.angle, - ), - ]; - const candidate = sideMidpoints.find( - (midpoint) => - pointDistance(point, midpoint) <= - maxBindingDistance_simple(zoom) + element.strokeWidth / 2 && + onAxis: false, + }; + } + } + } + + return undefined; +}; + +const getSnappedMidpointIndexForSimpleArrow = ( + element: ExcalidrawBindableElement, + point: GlobalPoint, + elementsMap: ElementsMap, + threshold: number, +) => { + const baseMidpoints = getAllMidpoints(element, elementsMap); + + for (let i = 0; i < baseMidpoints.length; i++) { + if ( + pointDistance(baseMidpoints[i], point) <= threshold && !hitElementItself({ point, element, threshold: 0, elementsMap, overrideShouldTestInside: true, - }), + }) + ) { + return i; + } + } + + return -1; +}; + +export const getAllMidpoints = ( + element: ExcalidrawBindableElement, + elementsMap: ElementsMap, +): GlobalPoint[] => { + const center = elementCenterPoint(element, elementsMap); + + if (element.type === "diamond") { + return getDiamondBaseCorners(element).map((curve) => + pointRotateRads(bezierEquation(curve, 0.5), center, element.angle), + ); + } + + return [ + pointFrom(element.width, element.height / 2), + pointFrom(element.width / 2, element.height), + pointFrom(0, element.height / 2), + pointFrom(element.width / 2, 0), + ].map(([x, y]) => + pointRotateRads( + pointFrom(element.x + x, element.y + y), + center, + element.angle, + ), + ); +}; + +export const getElbowArrowSnapMidPoint = ( + point: GlobalPoint, + element: ExcalidrawBindableElement, + elementsMap: ElementsMap, + zoom: AppState["zoom"], +) => { + const TOLERANCE = 0.05; + const maxDistance = maxBindingDistance_simple(zoom) + element.strokeWidth / 2; + + return getSnappedMidpointForElbowArrow( + element, + point, + elementsMap, + elementCenterPoint(element, elementsMap), + clamp(TOLERANCE * element.width, 5, maxDistance), + clamp(TOLERANCE * element.height, 5, maxDistance), + ); +}; + +export const getSnapOutlineMidPoint = ( + point: GlobalPoint, + element: ExcalidrawBindableElement, + elementsMap: ElementsMap, + zoom: AppState["zoom"], + arrow: { elbowed: boolean }, +): GlobalPoint | undefined => { + if (arrow.elbowed) { + return getElbowArrowSnapMidPoint(point, element, elementsMap, zoom)?.point; + } + + const maxDistance = maxBindingDistance_simple(zoom) + element.strokeWidth / 2; + const idx = getSnappedMidpointIndexForSimpleArrow( + element, + point, + elementsMap, + maxDistance, ); - return candidate; + return idx === -1 ? undefined : getAllMidpoints(element, elementsMap)[idx]; }; export const projectFixedPointOntoDiagonal = ( @@ -704,22 +817,24 @@ export const projectFixedPointOntoDiagonal = ( isMidpointSnappingEnabled: boolean = true, ): GlobalPoint | null => { invariant(arrow.points.length >= 2, "Arrow must have at least two points"); - if (arrow.width < 3 && arrow.height < 3) { - return null; - } - if (isMidpointSnappingEnabled) { const sideMidPoint = getSnapOutlineMidPoint( point, element, elementsMap, zoom, + arrow, ); if (sideMidPoint) { return sideMidPoint; } } + // Projection needs a meaningful arrow direction + if (arrow.width < 3 && arrow.height < 3) { + return null; + } + // Do the projection onto the diagonals (or center lines // for non-rectangular shapes) const [diagonalOne, diagonalTwo] = getDiagonalsForBindableElement( diff --git a/packages/element/src/zindex.ts b/packages/element/src/zindex.ts index 0e5b66781f..f18bc9b27b 100644 --- a/packages/element/src/zindex.ts +++ b/packages/element/src/zindex.ts @@ -10,9 +10,6 @@ import { getElementsInGroup } from "./groups"; import { syncMovedIndices } from "./fractionalIndex"; import { getSelectedElements } from "./selection"; import { getBoundTextElement, getContainerElement } from "./textElement"; -import { getHoveredElementForBinding } from "./collision"; - -import { getNonDeletedElements } from "."; import type { Scene } from "./Scene"; import type { @@ -159,17 +156,8 @@ export const moveArrowAboveBindable = ( elements: readonly OrderedExcalidrawElement[], elementsMap: NonDeletedSceneElementsMap, scene: Scene, - hit?: NonDeletedExcalidrawElement, + hoveredElement: NonDeletedExcalidrawElement, ): readonly OrderedExcalidrawElement[] => { - const hoveredElement = hit - ? hit - : getHoveredElementForBinding( - point, - // SAFETY: callers that omit `hit` pass non-deleted elements - getNonDeletedElements(elements), - elementsMap, - ); - if (!hoveredElement) { return elements; } diff --git a/packages/element/tests/binding.test.tsx b/packages/element/tests/binding.test.tsx index 7f03e8fe1f..e7de9e7c42 100644 --- a/packages/element/tests/binding.test.tsx +++ b/packages/element/tests/binding.test.tsx @@ -1,4 +1,4 @@ -import { KEYS, arrayToMap } from "@excalidraw/common"; +import { KEYS, ROUNDNESS, arrayToMap } from "@excalidraw/common"; import { pointFrom } from "@excalidraw/math"; @@ -16,7 +16,16 @@ import { import { defaultLang, setLanguage } from "@excalidraw/excalidraw/i18n"; -import { bindBindingElement, updateBoundElements } from "../src/binding"; +import type { Radians } from "@excalidraw/math"; + +import type { Zoom } from "@excalidraw/excalidraw/types"; + +import { + bindBindingElement, + getBindingGap, + updateBoundElements, +} from "../src/binding"; +import { getAllMidpoints } from "../src/utils"; import { getTransformHandles } from "../src/transformHandles"; import { getTextEditor, @@ -237,6 +246,285 @@ describe("binding for simple arrows", () => { expect(h.state.multiElement).not.toBe(null); expect(h.state.activeTool.type).toBe("arrow"); }); + + it("inside -> inside stays in multi-point mode when the pointer moves on", () => { + const end: [number, number] = [INSIDE[0] + 50, INSIDE[1] + 50]; + drawSelfArrow(INSIDE, end); + // nudge within the commit zone of the point just placed + mouse.moveTo(end[0] + 2, end[1] + 1); + + expect(h.state.multiElement).not.toBe(null); + expect(h.state.activeTool.type).toBe("arrow"); + expect(h.state.multiElement!.points.length).toBe(3); + }); + + it("bend point inside a shape the arrow doesn't start on stays in multi-point mode when the pointer moves on", () => { + UI.clickTool("arrow"); + mouse.reset(); + mouse.clickAt(...MIDDLE); + mouse.moveTo(...INSIDE); + mouse.clickAt(...INSIDE); + mouse.moveTo(INSIDE[0] + 2, INSIDE[1] + 1); + + expect(h.state.multiElement).not.toBe(null); + expect(h.state.activeTool.type).toBe("arrow"); + expect(h.state.multiElement!.points.length).toBe(2); + }); + }); + + describe("binding to frames", () => { + // frame spans x:200..600, y:200..600, child rect x:300..400, y:300..400 + beforeEach(async () => { + mouse.reset(); + await act(() => setLanguage(defaultLang)); + await render(); + + const frame = API.createElement({ + id: "frame", + type: "frame", + x: 200, + y: 200, + width: 400, + height: 400, + }); + const child = API.createElement({ + id: "child", + type: "rectangle", + x: 300, + y: 300, + width: 100, + height: 100, + frameId: frame.id, + }); + API.setElements([frame, child]); + }); + + const drawArrow = (end: [number, number]) => { + UI.clickTool("arrow"); + mouse.reset(); + mouse.downAt(50, 50); + mouse.moveTo(end[0] - 10, end[1] - 10); + mouse.moveTo(...end); + mouse.up(); + + return h.elements[h.elements.length - 1] as ExcalidrawArrowElement; + }; + + it("doesn't bind an arrow ending in the frame's empty interior", () => { + const arrow = drawArrow([500, 500]); + + expect(arrow.endBinding).toBe(null); + }); + + it("binds an arrow ending on a frame child to the child", () => { + const arrow = drawArrow([350, 350]); + + expect(arrow.endBinding?.elementId).toBe("child"); + }); + + it("binds an arrow ending just outside the frame to the frame", () => { + const arrow = drawArrow([500, 610]); + + expect(arrow.endBinding?.elementId).toBe("frame"); + expect(arrow.endBinding?.mode).toBe("orbit"); + }); + + it("keeps an elbow arrow drawn inside the frame where it was drawn", () => { + UI.clickTool("arrow"); + UI.clickOnTestId("elbow-arrow"); + mouse.reset(); + mouse.downAt(250, 250); + mouse.moveTo(400, 450); + mouse.moveTo(550, 550); + mouse.up(); + + const arrow = h.elements[h.elements.length - 1] as ExcalidrawArrowElement; + expect(arrow.elbowed).toBe(true); + // no self-orbit binding to the frame, which would route the arrow + // around the frame's outside + expect(arrow.startBinding).toBe(null); + expect(arrow.endBinding).toBe(null); + expect(arrow.frameId).toBe("frame"); + expect([arrow.x, arrow.y]).toEqual([250, 250]); + expect(arrow.points[arrow.points.length - 1]).toEqual([300, 300]); + }); + }); + + describe("binding at an ellipse's center", () => { + beforeEach(async () => { + mouse.reset(); + await act(() => setLanguage(defaultLang)); + await render(); + }); + + it("binds an arrow dropped at a circle's exact center", () => { + const circle = API.createElement({ + type: "ellipse", + x: 100, + y: -100, + width: 200, + height: 200, + }); + API.setElements([circle]); + + UI.clickTool("arrow"); + mouse.reset(); + mouse.downAt(-100, 0); + mouse.moveTo(100, 0); + mouse.moveTo(200, 0); + mouse.up(); + + const arrow = h.elements[h.elements.length - 1] as ExcalidrawArrowElement; + expect(arrow.endBinding?.elementId).toBe(circle.id); + }); + }); + + describe("midpoint snapping on diamonds", () => { + beforeEach(async () => { + mouse.reset(); + await act(() => setLanguage(defaultLang)); + await render(); + }); + + for (const rounded of [false, true]) { + for (const elbowed of [false, true]) { + it(`${ + elbowed ? "elbow" : "simple" + } arrow ends a binding gap outside the ${ + rounded ? "rounded" : "sharp" + } left vertex`, () => { + const diamond = API.createElement({ + type: "diamond", + x: 100, + y: -100, + width: 200, + height: 200, + roundness: rounded ? { type: ROUNDNESS.PROPORTIONAL_RADIUS } : null, + }) as ExcalidrawBindableElement; + API.setElements([diamond]); + const [, , left] = getAllMidpoints(diamond, arrayToMap([diamond])); + + UI.clickTool("arrow"); + if (elbowed) { + UI.clickOnTestId("elbow-arrow"); + } + mouse.reset(); + mouse.downAt(-200, left[1]); + mouse.moveTo(left[0] - 50, left[1]); + // near (not on) the vertex, so the end snaps to it + mouse.moveTo(left[0] - 4, left[1] + 2); + mouse.up(); + + const arrow = h.elements[ + h.elements.length - 1 + ] as ExcalidrawArrowElement; + const [endX, endY] = arrow.points[arrow.points.length - 1]; + + expect(arrow.endBinding?.elementId).toBe(diamond.id); + expect(arrow.x + endX).toBeCloseTo( + left[0] - getBindingGap(diamond), + 0, + ); + expect(arrow.y + endY).toBeCloseTo(left[1], 0); + }); + } + + it(`elbow arrow binds above the ${ + rounded ? "rounded" : "sharp" + } top vertex when the pointer is slightly inside`, () => { + const diamond = API.createElement({ + type: "diamond", + x: 100, + y: -100, + width: 300, + height: 300, + roundness: rounded ? { type: ROUNDNESS.PROPORTIONAL_RADIUS } : null, + }) as ExcalidrawBindableElement; + API.setElements([diamond]); + const [, , , top] = getAllMidpoints(diamond, arrayToMap([diamond])); + + UI.clickTool("arrow"); + UI.clickOnTestId("elbow-arrow"); + mouse.reset(); + mouse.downAt(top[0] - 300, top[1] - 100); + mouse.moveTo(top[0] - 50, top[1] - 50); + // inside the diamond, a bit right of the vertex + mouse.moveTo(top[0] + 3, top[1] + 10); + mouse.up(); + + const arrow = h.elements[ + h.elements.length - 1 + ] as ExcalidrawArrowElement; + const points = arrow.points; + const [endX, endY] = points[points.length - 1]; + + expect(arrow.endBinding?.elementId).toBe(diamond.id); + expect(arrow.x + endX).toBeCloseTo(top[0], 0); + expect(arrow.y + endY).toBeCloseTo(top[1] - getBindingGap(diamond), 0); + // the last segment comes straight down into the vertex + expect(points[points.length - 2][0]).toBeCloseTo(endX); + }); + } + + it("elbow arrow binds next to the tip of a thin rotated diamond", () => { + const diamond = API.createElement({ + type: "diamond", + x: 0, + y: 0, + width: 600, + height: 100, + angle: ((15 * Math.PI) / 180) as Radians, + roundness: { type: ROUNDNESS.PROPORTIONAL_RADIUS }, + }) as ExcalidrawBindableElement; + API.setElements([diamond]); + const [right] = getAllMidpoints(diamond, arrayToMap([diamond])); + + UI.clickTool("arrow"); + UI.clickOnTestId("elbow-arrow"); + mouse.reset(); + mouse.downAt(right[0] + 300, right[1] + 300); + mouse.moveTo(right[0] + 50, right[1] + 50); + // just outside the right tip + mouse.moveTo(right[0] + 4, right[1] + 1); + mouse.up(); + + const arrow = h.elements[h.elements.length - 1] as ExcalidrawArrowElement; + const [endX, endY] = arrow.points[arrow.points.length - 1]; + + expect(arrow.endBinding?.elementId).toBe(diamond.id); + // a binding gap away, not on another edge of the diamond + expect( + Math.hypot(arrow.x + endX - right[0], arrow.y + endY - right[1]), + ).toBeLessThan(getBindingGap(diamond) + 2); + }); + + it("elbow arrow snaps to a diamond's edge midpoint", () => { + const diamond = API.createElement({ + type: "diamond", + x: 100, + y: -100, + width: 200, + height: 200, + }) as ExcalidrawBindableElement; + API.setElements([diamond]); + // midpoint of the top-left edge + const edgeMidpoint = [150, -50] as const; + + UI.clickTool("arrow"); + UI.clickOnTestId("elbow-arrow"); + mouse.reset(); + mouse.downAt(-200, -50); + mouse.moveTo(100, -60); + // near (not on) the edge midpoint + mouse.moveTo(edgeMidpoint[0] - 9, edgeMidpoint[1] - 3); + mouse.up(); + + const arrow = h.elements[h.elements.length - 1] as ExcalidrawArrowElement; + const [, endY] = arrow.points[arrow.points.length - 1]; + + expect(arrow.endBinding?.elementId).toBe(diamond.id); + expect(arrow.y + endY).toBeCloseTo(edgeMidpoint[1], 0); + }); }); describe("when arrow is outside of shape", () => { @@ -797,7 +1085,9 @@ describe("binding to a point-like (sub-pixel) element", () => { }) as NonDeleted; API.setElements([rect, arrow]); - bindBindingElement(arrow, rect, "orbit", "end", h.scene); + bindBindingElement(arrow, rect, "orbit", "end", h.scene, { + value: 1, + } as Zoom); const endBinding = arrow.endBinding as FixedPointBinding; expect(endBinding.elementId).toBe(rect.id); diff --git a/packages/element/tests/collision.test.tsx b/packages/element/tests/collision.test.tsx index d2f303ef1e..a58b90e006 100644 --- a/packages/element/tests/collision.test.tsx +++ b/packages/element/tests/collision.test.tsx @@ -1,7 +1,13 @@ -import { arrayToMap, getSizeFromPoints, reseed } from "@excalidraw/common"; +import { + ROUNDNESS, + arrayToMap, + getSizeFromPoints, + reseed, +} from "@excalidraw/common"; import { type GlobalPoint, type LocalPoint, + lineSegment, pointFrom, pointRotateRads, } from "@excalidraw/math"; @@ -11,11 +17,27 @@ import { UI } from "@excalidraw/excalidraw/tests/helpers/ui"; import "@excalidraw/utils/test-utils"; import { render } from "@excalidraw/excalidraw/tests/test-utils"; +import type { Zoom } from "@excalidraw/excalidraw/types"; + import * as distance from "../src/distance"; import { getElementBounds } from "../src/bounds"; -import { hitElementItself, isPointInElement } from "../src/collision"; +import { + getAllHoveredElementAtPoint, + getHoveredElementForBinding, + hitElementItself, + intersectElementWithLineSegment, + isPointInElement, +} from "../src/collision"; import { mutateElement } from "../src/mutateElement"; import { newFreeDrawElement } from "../src/newElement"; +import { getAllMidpoints } from "../src/utils"; + +import type { + ExcalidrawDiamondElement, + NonDeletedExcalidrawElement, + NonDeletedSceneElementsMap, + Ordered, +} from "../src/types"; describe("check rotated elements can be hit:", () => { beforeEach(async () => { @@ -620,3 +642,210 @@ describe("freedraw loop fill containment", () => { expect(isPointInElement(point, element, elementsMap)).toBe(false); }); }); + +describe("binding hit tests", () => { + type SceneElement = Ordered; + const zoom = (value: number) => ({ value } as Zoom); + + const hitTest = ( + elements: SceneElement[], + point: GlobalPoint, + zoomValue = 1, + ) => { + const elementsMap = arrayToMap(elements) as NonDeletedSceneElementsMap; + return { + hovered: getHoveredElementForBinding( + point, + elements, + elementsMap, + zoom(zoomValue), + )?.id, + all: getAllHoveredElementAtPoint( + point, + elements, + elementsMap, + zoom(zoomValue), + ).map((element) => element.id), + }; + }; + + it("both use the zoom-aware binding distance", () => { + const rect = API.createElement({ + id: "rect", + type: "rectangle", + x: 0, + y: 0, + width: 100, + height: 100, + }) as SceneElement; + // 20px outside the right edge + const point = pointFrom(120, 50); + + // 15px binding distance at zoom 1 + expect(hitTest([rect], point, 1)).toEqual({ hovered: undefined, all: [] }); + // 25px binding distance at zoom 0.4 + expect(hitTest([rect], point, 0.4)).toEqual({ + hovered: "rect", + all: ["rect"], + }); + }); + + it("both skip elements hidden behind an opaque element", () => { + const hidden = API.createElement({ + id: "hidden", + type: "rectangle", + x: 30, + y: 30, + width: 40, + height: 40, + index: "a0" as SceneElement["index"], + }) as SceneElement; + const cover = (backgroundColor: string) => + API.createElement({ + id: "cover", + type: "rectangle", + x: 0, + y: 0, + width: 100, + height: 100, + backgroundColor, + index: "a1" as SceneElement["index"], + }) as SceneElement; + const point = pointFrom(50, 50); + + expect(hitTest([hidden, cover("#ffc9c9")], point)).toEqual({ + hovered: "cover", + all: ["cover"], + }); + + const transparent = hitTest([hidden, cover("transparent")], point); + expect(transparent.hovered).toBe("hidden"); + expect(transparent.all).toEqual(["cover", "hidden"]); + }); + + describe("overlapping elements", () => { + const rect = (id: string, x: number, y: number, w: number, h: number) => + API.createElement({ + id, + type: "rectangle", + x, + y, + width: w, + height: h, + }) as SceneElement; + + it("binds to the container's edge next to a nested element", () => { + const container = rect("container", 0, 0, 200, 200); + // 18px inside the container's left edge + const child = rect("child", 18, 80, 60, 40); + + // 5px inside the container's edge, 13px outside the child + expect( + hitTest([container, child], pointFrom(5, 100)).hovered, + ).toBe("container"); + // closer to the child's outline than to the container's + expect( + hitTest([container, child], pointFrom(10, 100)).hovered, + ).toBe("child"); + }); + + it("binds to a smaller element straddling the container's edge when inside both", () => { + const container = rect("container", 0, 0, 200, 200); + const badge = rect("badge", -30, 80, 60, 40); + + // 3px inside the container's edge, inside the badge + expect( + hitTest([container, badge], pointFrom(3, 92)).hovered, + ).toBe("badge"); + // 5px outside the container's edge, inside the badge: the closer + // container outline wins + expect( + hitTest([container, badge], pointFrom(-5, 108)).hovered, + ).toBe("container"); + }); + }); + + it("binds at a circle's exact center, where an opaque circle still occludes", () => { + const hidden = API.createElement({ + id: "hidden", + type: "rectangle", + x: 90, + y: 90, + width: 20, + height: 20, + index: "a0" as SceneElement["index"], + }) as SceneElement; + const circle = (backgroundColor: string) => + API.createElement({ + id: "circle", + type: "ellipse", + x: 0, + y: 0, + width: 200, + height: 200, + backgroundColor, + index: "a1" as SceneElement["index"], + }) as SceneElement; + const center = pointFrom(100, 100); + + expect(hitTest([circle("transparent")], center).hovered).toBe("circle"); + expect(hitTest([hidden, circle("#ffc9c9")], center)).toEqual({ + hovered: "circle", + all: ["circle"], + }); + }); +}); + +describe("ellipse outline hit test", () => { + it("doesn't hit a transparent ellipse's outline from its center line", () => { + const ellipse = API.createElement({ + type: "ellipse", + x: 0, + y: 0, + width: 400, + height: 100, + backgroundColor: "transparent", + }); + + // on the horizontal center line, ~50px from the outline + expect( + hitElementItself({ + point: pointFrom(210, 50), + element: ellipse, + threshold: 10, + elementsMap: arrayToMap([ellipse]), + }), + ).toBe(false); + }); +}); + +describe("intersectElementWithLineSegment", () => { + it("hits a rounded diamond corner along a line through its apex", () => { + const diamond = API.createElement({ + type: "diamond", + x: 100, + y: -100, + width: 200, + height: 200, + roundness: { type: ROUNDNESS.PROPORTIONAL_RADIUS }, + }) as ExcalidrawDiamondElement; + const elementsMap = arrayToMap([diamond]); + const [, , left] = getAllMidpoints(diamond, elementsMap); + + // the offset corner is split into several curves, and this line passes + // exactly through the joint of two of them + const hits = intersectElementWithLineSegment( + diamond, + elementsMap, + lineSegment( + pointFrom(200, left[1]), + pointFrom(-200, left[1]), + ), + 6, + ); + + expect(hits.length).toBeGreaterThan(0); + expect(hits[0][0]).toBeCloseTo(left[0] - 6, 0); + expect(hits[0][1]).toBeCloseTo(left[1]); + }); +}); diff --git a/packages/element/tests/elbowArrow.test.tsx b/packages/element/tests/elbowArrow.test.tsx index bb8a4e7d75..444386ad1c 100644 --- a/packages/element/tests/elbowArrow.test.tsx +++ b/packages/element/tests/elbowArrow.test.tsx @@ -16,6 +16,7 @@ import "@excalidraw/utils/test-utils"; import { bindBindingElement } from "@excalidraw/element"; import type { LocalPoint } from "@excalidraw/math"; +import type { Zoom } from "@excalidraw/excalidraw/types"; import { Scene } from "../src/Scene"; @@ -188,8 +189,12 @@ describe("elbow arrow routing", () => { }) as NonDeleted; API.setElements([rectangle1, rectangle2, arrow]); - bindBindingElement(arrow, rectangle1, "orbit", "start", h.scene); - bindBindingElement(arrow, rectangle2, "orbit", "end", h.scene); + bindBindingElement(arrow, rectangle1, "orbit", "start", h.scene, { + value: 1, + } as Zoom); + bindBindingElement(arrow, rectangle2, "orbit", "end", h.scene, { + value: 1, + } as Zoom); expect(arrow.startBinding).not.toBe(null); expect(arrow.endBinding).not.toBe(null); diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx index 4464b80c37..d3effa85bb 100644 --- a/packages/excalidraw/actions/actionProperties.tsx +++ b/packages/excalidraw/actions/actionProperties.tsx @@ -2147,6 +2147,7 @@ export const actionChangeArrowType = register({ startElement, "start", elementsMap, + appState.zoom, appState.isBindingEnabled, ), } @@ -2161,6 +2162,7 @@ export const actionChangeArrowType = register({ endElement, "end", elementsMap, + appState.zoom, appState.isBindingEnabled, ), } @@ -2193,6 +2195,7 @@ export const actionChangeArrowType = register({ appState.bindMode === "inside" ? "inside" : "orbit", "start", app.scene, + appState.zoom, ); } } @@ -2207,6 +2210,7 @@ export const actionChangeArrowType = register({ appState.bindMode === "inside" ? "inside" : "orbit", "end", app.scene, + appState.zoom, ); } } diff --git a/packages/excalidraw/components/App.drawshape.ts b/packages/excalidraw/components/App.drawshape.ts index f38662ebbc..dc4f716174 100644 --- a/packages/excalidraw/components/App.drawshape.ts +++ b/packages/excalidraw/components/App.drawshape.ts @@ -8,7 +8,6 @@ import { isBindingElement, isLineElement, LinearElementEditor, - maxBindingDistance_simple, newArrowElement, updateBoundPoint, } from "@excalidraw/element"; @@ -92,7 +91,6 @@ export class AppDrawShape { const { app } = this; const elementsMap = app.scene.getNonDeletedElementsMap(); const elements = app.scene.getNonDeletedElements(); - const bindingDistance = maxBindingDistance_simple(app.state.zoom); const endpoints = (["start", "end"] as const).map((startOrEnd) => { const globalPoint = LinearElementEditor.getPointAtIndexGlobalCoordinates( @@ -107,7 +105,7 @@ export class AppDrawShape { globalPoint, elements, elementsMap, - bindingDistance, + app.state.zoom, ), }; }); @@ -125,6 +123,7 @@ export class AppDrawShape { sameTarget ? "inside" : "orbit", startOrEnd, app.scene, + app.state.zoom, globalPoint, app.state.isBindingEnabled, isMidpointSnappingEnabled, @@ -175,7 +174,6 @@ export class AppDrawShape { const { app } = this; const elementsMap = app.scene.getNonDeletedElementsMap(); const elements = app.scene.getNonDeletedElements(); - const bindingDistance = maxBindingDistance_simple(app.state.zoom); const [startTarget, endTarget] = ([0, -1] as const).map((index) => getHoveredElementForBinding( @@ -186,7 +184,7 @@ export class AppDrawShape { ), elements, elementsMap, - bindingDistance, + app.state.zoom, ), ); if ((!startTarget && !endTarget) || startTarget === endTarget) { diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 4f10409f9b..1d310179d3 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -245,7 +245,6 @@ import { getElementBounds, doBoundsIntersect, isPointInElement, - maxBindingDistance_simple, convertToExcalidrawElements, type ExcalidrawElementSkeleton, getSnapOutlineMidPoint, @@ -1204,6 +1203,7 @@ class App extends React.Component { ), this.scene.getNonDeletedElements(), elementsMap, + this.state.zoom, ); const element = LinearElementEditor.getElement( this.state.selectedLinearElement.elementId, @@ -1337,6 +1337,7 @@ class App extends React.Component { pointFrom(x, y), this.scene.getNonDeletedElements(), this.scene.getNonDeletedElementsMap(), + this.state.zoom, ); if (hoveredElement && this.state.bindMode !== "skip") { @@ -6021,12 +6022,6 @@ class App extends React.Component { this.state, ); - const hoveredElement = getHoveredElementForBinding( - pointFrom(scenePointer.x, scenePointer.y), - this.scene.getNonDeletedElements(), - this.scene.getNonDeletedElementsMap(), - ); - if (this.state.selectedLinearElement) { const element = LinearElementEditor.getElement( this.state.selectedLinearElement.elementId, @@ -6034,6 +6029,13 @@ class App extends React.Component { ); if (isBindingElement(element)) { + const hoveredElement = getHoveredElementForBinding( + pointFrom(scenePointer.x, scenePointer.y), + this.scene.getNonDeletedElements(), + this.scene.getNonDeletedElementsMap(), + this.state.zoom, + ); + this.handleDelayedBindModeChange(element, hoveredElement); } } @@ -7946,11 +7948,14 @@ class App extends React.Component { scenePointerY, ); const elementsMap = this.scene.getNonDeletedElementsMap(); + const arrow = { + elbowed: this.state.currentItemArrowType === ARROW_TYPE.elbow, + }; const hoveredElement = getHoveredElementForBinding( globalPoint, this.scene.getNonDeletedElements(), elementsMap, - maxBindingDistance_simple(this.state.zoom), + this.state.zoom, ); if (hoveredElement) { this.setState({ @@ -7961,6 +7966,7 @@ class App extends React.Component { hoveredElement, elementsMap, this.state.zoom, + arrow, ), }, }); @@ -8072,6 +8078,7 @@ class App extends React.Component { pointFrom(scenePointerX, scenePointerY), this.scene.getNonDeletedElements(), elementsMap, + this.state.zoom, ); if (getFeatureFlag("COMPLEX_BINDINGS")) { @@ -8107,7 +8114,7 @@ class App extends React.Component { scenePointer, this.scene.getNonDeletedElements(), this.scene.getNonDeletedElementsMap(), - maxBindingDistance_simple(this.state.zoom), + this.state.zoom, ); const elementsMap = this.scene.getNonDeletedElementsMap(); if (hit && !isPointInElement(scenePointer, hit, elementsMap)) { @@ -8119,6 +8126,7 @@ class App extends React.Component { hit, elementsMap, this.state.zoom, + { elbowed: this.state.currentItemArrowType === ARROW_TYPE.elbow }, ), }, }); @@ -10171,7 +10179,6 @@ class App extends React.Component { this.state, { newArrow: Boolean(this.state.newElement), - zoom: this.state.zoom, }, ) : { end: { mode: undefined } }; @@ -10305,6 +10312,7 @@ class App extends React.Component { point, this.scene.getNonDeletedElements(), elementsMap, + this.state.zoom, ) : null; @@ -10386,6 +10394,7 @@ class App extends React.Component { boundElement, elementsMap, this.state.zoom, + element, ), } : null, @@ -10816,16 +10825,15 @@ class App extends React.Component { return; } - if (isBindingElement(element)) { + if (isBindingElement(element) && getFeatureFlag("COMPLEX_BINDINGS")) { const hoveredElement = getHoveredElementForBinding( pointFrom(pointerCoords.x, pointerCoords.y), this.scene.getNonDeletedElements(), elementsMap, + this.state.zoom, ); - if (getFeatureFlag("COMPLEX_BINDINGS")) { - this.handleDelayedBindModeChange(element, hoveredElement); - } + this.handleDelayedBindModeChange(element, hoveredElement); } if ( diff --git a/packages/excalidraw/components/ConvertElementTypePopup.tsx b/packages/excalidraw/components/ConvertElementTypePopup.tsx index 51cb59348f..f0445ee2d0 100644 --- a/packages/excalidraw/components/ConvertElementTypePopup.tsx +++ b/packages/excalidraw/components/ConvertElementTypePopup.tsx @@ -480,7 +480,7 @@ export const convertElementTypes = ( for (const element of Object.values(convertedElements)) { if (isBindableElement(element)) { - reanchorBindingsToOutline(element, app.scene); + reanchorBindingsToOutline(element, app.scene, app.state.zoom); updateBindings(element, app.scene, app.state); } diff --git a/packages/excalidraw/renderer/interactiveScene.ts b/packages/excalidraw/renderer/interactiveScene.ts index 45d43c7f62..5a4b479c2b 100644 --- a/packages/excalidraw/renderer/interactiveScene.ts +++ b/packages/excalidraw/renderer/interactiveScene.ts @@ -7,7 +7,6 @@ import { type Radians, bezierEquation, pointRotateRads, - pointDistance, } from "@excalidraw/math"; import { @@ -26,13 +25,12 @@ import { deconstructDiamondElement, deconstructRectanguloidElement, elementCenterPoint, - getDiamondBaseCorners, + getAllMidpoints, FOCUS_POINT_SIZE, getOmitSidesForEditorInterface, getTransformHandles, getTransformHandlesFromCoords, hasBoundingBox, - hitElementItself, isArrowElement, isBindableElement, isElbowArrow, @@ -40,7 +38,6 @@ import { isImageElement, isLinearElement, isLineElement, - maxBindingDistance_simple, isTextElement, LinearElementEditor, getActiveTextElement, @@ -474,6 +471,7 @@ const renderBindingHighlightForBindableElement_simple = ( break; } + // Draw midpoint indicators if ( appState.isMidpointSnappingEnabled && !appState.gridModeEnabled && @@ -481,136 +479,33 @@ const renderBindingHighlightForBindableElement_simple = ( (isFrameLikeElement(suggestedBinding.element) || isBindableElement(suggestedBinding.element)) ) { - // Draw midpoint indicators - const linearElement = appState.selectedLinearElement; - const arrow = - linearElement?.elementId && - LinearElementEditor.getElement(linearElement?.elementId, elementsMap); - const cursorIsInsideBindable = - pointerCoords && - hitElementItself({ - point: pointerCoords, - element: suggestedBinding.element, - elementsMap, - threshold: 0, - overrideShouldTestInside: true, - }); + context.save(); - const isElbow = - (arrow && isElbowArrow(arrow)) || - (appState.activeTool.type === "arrow" && - appState.currentItemArrowType === "elbow"); + const midpointRadius = 4 / appState.zoom.value; - if (!cursorIsInsideBindable || isElbow) { - context.save(); - - const center = elementCenterPoint(suggestedBinding.element, elementsMap); - - let midpoints: GlobalPoint[]; - if (suggestedBinding.element.type === "diamond") { - const center = elementCenterPoint( - suggestedBinding.element, - elementsMap, - ); - midpoints = getDiamondBaseCorners(suggestedBinding.element).map( - (curve) => { - const point = bezierEquation(curve, 0.5); - const rotatedPoint = pointRotateRads( - point, - center, - suggestedBinding.element.angle, - ); - - return pointFrom(rotatedPoint[0], rotatedPoint[1]); - }, - ); - } else { - const basePoints = [ - { - x: suggestedBinding.element.width, - y: suggestedBinding.element.height / 2, - }, // RIGHT - { - x: suggestedBinding.element.width / 2, - y: suggestedBinding.element.height, - }, // BOTTOM - { x: 0, y: suggestedBinding.element.height / 2 }, // LEFT - { x: suggestedBinding.element.width / 2, y: 0 }, // TOP - ]; - midpoints = basePoints.map((point) => { - const globalPoint = pointFrom( - point.x + suggestedBinding.element.x, - point.y + suggestedBinding.element.y, - ); - const rotatedPoint = pointRotateRads( - globalPoint, - center, - suggestedBinding.element.angle, - ); - return pointFrom(rotatedPoint[0], rotatedPoint[1]); - }); - } - - const hoveredMidpoint = - pointerCoords && - midpoints.reduce( - ( - closestIdx: { - idx: number; - distance: number; - }, - point, - idx, - ) => { - const distance = pointDistance(point, pointerCoords); - if (idx === -1 || distance < closestIdx.distance) { - return { idx, distance }; - } - return closestIdx; - }, - { - idx: -1, - distance: Infinity, - }, - ); - - const midpointRadius = 4 / appState.zoom.value; - const highlightThreshold = - maxBindingDistance_simple(appState.zoom) + - suggestedBinding.element.strokeWidth / 2; - - midpoints.forEach((midpoint, idx) => { - const isHighlighted = - (!cursorIsInsideBindable || isElbow) && - hoveredMidpoint?.idx === idx && - hoveredMidpoint.distance <= highlightThreshold; - - // also render midpoint if cursor close but not highlighted - // (for elbows, always show all points) - const isShown = - !isHighlighted && - (isElbow || - (idx === hoveredMidpoint?.idx && - hoveredMidpoint.distance <= highlightThreshold * 2)); - - if (isHighlighted) { - context.fillStyle = `rgba(${ - BINDING_HIGHLIGHT_RGB[appState.theme] - }, 1)`; - - context.beginPath(); - context.arc(midpoint[0], midpoint[1], midpointRadius, 0, 2 * Math.PI); - context.fill(); - } else if (isShown) { - context.fillStyle = BINDING_MIDPOINT_COLOR[appState.theme]; - context.beginPath(); - context.arc(midpoint[0], midpoint[1], midpointRadius, 0, 2 * Math.PI); - context.fill(); - } - }); - - context.restore(); + // Render base midpoints + const midpoints = getAllMidpoints(suggestedBinding.element, elementsMap); + for (const midpoint of midpoints) { + context.fillStyle = BINDING_MIDPOINT_COLOR[appState.theme]; + context.beginPath(); + context.arc(midpoint[0], midpoint[1], midpointRadius, 0, 2 * Math.PI); + context.fill(); } + + // Render the highlighted midpoint if any + const midpoint = appState.suggestedBinding?.midPoint; + if (midpoint) { + context.fillStyle = + appState.theme === THEME.DARK + ? `rgba(3, 93, 161, 1)` + : `rgba(106, 189, 252, 1)`; + + context.beginPath(); + context.arc(midpoint[0], midpoint[1], midpointRadius, 0, 2 * Math.PI); + context.fill(); + } + + context.restore(); } }; diff --git a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap index c1fe53d95d..2e0179e098 100644 --- a/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/move.test.tsx.snap @@ -168,7 +168,7 @@ exports[`move element > rectangles with binding arrow 6`] = ` "type": "rectangle", "updated": 1, "version": 7, - "versionNonce": 735304455, + "versionNonce": 888958951, "width": 300, "x": 201, "y": 2, @@ -232,8 +232,8 @@ exports[`move element > rectangles with binding arrow 7`] = ` "strokeWidth": 2, "type": "arrow", "updated": 1, - "version": 13, - "versionNonce": 271613161, + "version": 12, + "versionNonce": 2066753033, "width": 89, "x": 106, "y": "56.01120", diff --git a/packages/excalidraw/tests/convertElementType.binding.test.tsx b/packages/excalidraw/tests/convertElementType.binding.test.tsx index 20021029b6..52f66b898a 100644 --- a/packages/excalidraw/tests/convertElementType.binding.test.tsx +++ b/packages/excalidraw/tests/convertElementType.binding.test.tsx @@ -83,9 +83,7 @@ describe("convert element type keeps bound arrows attached", () => { h.app.scene.getNonDeletedElementsMap(), globalEnd, ); - expect(distance).toBeLessThanOrEqual( - getBindingGap(converted, arrowAfter) + 1, - ); + expect(distance).toBeLessThanOrEqual(getBindingGap(converted) + 1); }, ); } diff --git a/packages/excalidraw/tests/move.test.tsx b/packages/excalidraw/tests/move.test.tsx index ae1db40fda..0dd0b1bdc3 100644 --- a/packages/excalidraw/tests/move.test.tsx +++ b/packages/excalidraw/tests/move.test.tsx @@ -17,6 +17,8 @@ import * as StaticScene from "../renderer/staticScene"; import { UI, Pointer, Keyboard } from "./helpers/ui"; import { render, fireEvent, act, unmountComponent } from "./test-utils"; +import type { Zoom } from "../types"; + unmountComponent(); const renderInteractiveScene = vi.spyOn( @@ -89,6 +91,7 @@ describe("move element", () => { "orbit", "start", h.app.scene, + { value: 1 } as Zoom, ); bindBindingElement( arrow.get() as NonDeleted, @@ -96,6 +99,7 @@ describe("move element", () => { "orbit", "end", h.app.scene, + { value: 1 } as Zoom, ); }); diff --git a/packages/math/src/ellipse.ts b/packages/math/src/ellipse.ts index 741a77df3e..7241d21a37 100644 --- a/packages/math/src/ellipse.ts +++ b/packages/math/src/ellipse.ts @@ -99,6 +99,12 @@ export const ellipseDistanceFromPoint = < vectorScale(vectorFromPoint(center), -1), ); + // A circle's outline is equidistant along any ray from the center, so this + // is exact, and it avoids the iteration below, which is 0/0 at the center + if (a === b) { + return Math.abs(Math.hypot(translatedPoint[0], translatedPoint[1]) - a); + } + const px = Math.abs(translatedPoint[0]); const py = Math.abs(translatedPoint[1]); @@ -121,6 +127,10 @@ export const ellipseDistanceFromPoint = < const r = Math.hypot(ry, rx); const q = Math.hypot(qy, qx); + if (q === 0) { + break; + } + tx = Math.min(1, Math.max(0, ((qx * r) / q + ex) / a)); ty = Math.min(1, Math.max(0, ((qy * r) / q + ey) / b)); const t = Math.hypot(ty, tx); @@ -128,9 +138,12 @@ export const ellipseDistanceFromPoint = < ty /= t; } + // Map the first-quadrant solution back to the point's quadrant. A point on + // an axis (coordinate exactly 0) can use either side, but not 0, which + // would collapse the closest point onto the axis const [minX, minY] = [ - a * tx * Math.sign(translatedPoint[0]), - b * ty * Math.sign(translatedPoint[1]), + a * tx * (translatedPoint[0] < 0 ? -1 : 1), + b * ty * (translatedPoint[1] < 0 ? -1 : 1), ]; return pointDistance(pointFromVector(translatedPoint), pointFrom(minX, minY)); diff --git a/packages/math/tests/ellipse.test.ts b/packages/math/tests/ellipse.test.ts index 4fa0d4e59f..e145fd20ae 100644 --- a/packages/math/tests/ellipse.test.ts +++ b/packages/math/tests/ellipse.test.ts @@ -1,5 +1,6 @@ import { ellipse, + ellipseDistanceFromPoint, ellipseSegmentInterceptPoints, ellipseIncludesPoint, ellipseTouchesPoint, @@ -60,6 +61,35 @@ describe("point and ellipse", () => { }); }); +describe("point distance to ellipse outline", () => { + const center = pointFrom(0, 0); + + it("is the radius at a circle's center", () => { + const circle = ellipse(center, 100, 100); + + expect(ellipseDistanceFromPoint(center, circle)).toBe(100); + expect(ellipseDistanceFromPoint(pointFrom(1, 0), circle)).toBe(99); + expect(ellipseDistanceFromPoint(pointFrom(30, 40), circle)).toBe(50); + }); + + it("measures points on the axes of a non-circular ellipse", () => { + const wide = ellipse(center, 200, 50); + + // closest to the top/bottom of the outline, not to the long axis + expect(ellipseDistanceFromPoint(center, wide)).toBeCloseTo(50); + expect(ellipseDistanceFromPoint(pointFrom(10, 0), wide)).toBeCloseTo( + 49.93, + 1, + ); + expect(ellipseDistanceFromPoint(pointFrom(-150, 0), wide)).toBeCloseTo( + 31.62, + 1, + ); + expect(ellipseDistanceFromPoint(pointFrom(0, 10), wide)).toBeCloseTo(40); + expect(ellipseDistanceFromPoint(pointFrom(250, 0), wide)).toBeCloseTo(50); + }); +}); + describe("segment and ellipse", () => { it("detects outside segment", () => { const e = ellipse(pointFrom(0, 0), 2, 2);