Fix elbow arrows

packages/excalidraw/element/binding.ts

@@ -24,7 +24,10 @@ import { KEYS } from "../keys";
 import { aabbForElement, getElementShape, pointInsideBounds } from "../shapes";
 import {
   arrayToMap,
+  invariant,
   isBindingFallthroughEnabled,
+  isDevEnv,
+  isTestEnv,
   tupleToCoors,
 } from "../utils";
 
@@ -41,6 +44,7 @@ import {
   HEADING_RIGHT,
   HEADING_UP,
   headingForPointFromElement,
+  headingIsHorizontal,
   vectorToHeading,
   type Heading,
 } from "./heading";
@@ -81,6 +85,7 @@ import type {
 } from "./types";
 import type Scene from "../scene/Scene";
 import type { AppState } from "../types";
+import { debugClear, debugDrawPoint } from "../visualdebug";
 
 export type SuggestedBinding =
   | NonDeleted<ExcalidrawBindableElement>
@@ -925,103 +930,105 @@ const getDistanceForBinding = (
 
 export const bindPointToSnapToElementOutline = (
   arrow: ExcalidrawElbowArrowElement,
-  bindableElement: ExcalidrawBindableElement | undefined,
+  bindableElement: ExcalidrawBindableElement,
   startOrEnd: "start" | "end",
 ): GlobalPoint => {
-  const aabb = bindableElement && aabbForElement(bindableElement);
+  if (isDevEnv() || isTestEnv()) {
+    invariant(arrow.points.length > 1, "Arrow should have at least 2 points");
+  }
+
+  const aabb = aabbForElement(bindableElement);
   const localP =
     arrow.points[startOrEnd === "start" ? 0 : arrow.points.length - 1];
   const globalP = pointFrom<GlobalPoint>(
     arrow.x + localP[0],
     arrow.y + localP[1],
   );
-  const p = isRectanguloidElement(bindableElement)
+  const edgePoint = isRectanguloidElement(bindableElement)
     ? avoidRectangularCorner(bindableElement, globalP)
     : globalP;
+  const elbowed = isElbowArrow(arrow);
+  const center = getCenterForBounds(aabb);
+  const adjacentPointIdx = startOrEnd === "start" ? 1 : arrow.points.length - 2;
+  const adjacentPoint = pointRotateRads(
+    pointFrom<GlobalPoint>(
+      arrow.x + arrow.points[adjacentPointIdx][0],
+      arrow.y + arrow.points[adjacentPointIdx][1],
+    ),
+    center,
+    arrow.angle ?? 0,
+  );
 
-  if (bindableElement && aabb) {
+  let intersection: GlobalPoint | null = null;
-    const center = getCenterForBounds(aabb);
+  if (elbowed) {
-
+    const isHorizontal = headingIsHorizontal(
-    const intersection = intersectElementWithLineSegment(
+      headingForPointFromElement(bindableElement, aabb, globalP),
+    );
+    const otherPoint = pointFrom<GlobalPoint>(
+      isHorizontal ? center[0] : edgePoint[0],
+      !isHorizontal ? center[1] : edgePoint[1],
+    );
+    intersection = intersectElementWithLineSegment(
       bindableElement,
       lineSegment(
-        center,
+        otherPoint,
         pointFromVector(
           vectorScale(
-            vectorNormalize(vectorFromPoint(p, center)),
+            vectorNormalize(vectorFromPoint(edgePoint, otherPoint)),
-            Math.max(bindableElement.width, bindableElement.height),
+            Math.max(bindableElement.width, bindableElement.height) * 2,
           ),
-          center,
+          otherPoint,
         ),
       ),
+      FIXED_BINDING_DISTANCE,
     )[0];
-    const currentDistance = pointDistance(p, center);
+  } else {
-    const fullDistance = Math.max(
+    intersection = intersectElementWithLineSegment(
-      pointDistance(intersection ?? p, center),
+      bindableElement,
-      PRECISION,
+      lineSegment(
-    );
+        adjacentPoint,
-    const ratio = round(currentDistance / fullDistance, 6);
+        pointFromVector(
-
-    switch (true) {
-      case ratio > 0.9:
-        if (
-          currentDistance - fullDistance > FIXED_BINDING_DISTANCE ||
-          // Too close to determine vector from intersection to p
-          pointDistanceSq(p, intersection) < PRECISION
-        ) {
-          return p;
-        }
-
-        return pointFromVector(
           vectorScale(
-            vectorNormalize(vectorFromPoint(p, intersection ?? center)),
+            vectorNormalize(vectorFromPoint(edgePoint, adjacentPoint)),
-            ratio > 1 ? FIXED_BINDING_DISTANCE : -FIXED_BINDING_DISTANCE,
+            pointDistance(edgePoint, adjacentPoint) +
+              Math.max(bindableElement.width, bindableElement.height) * 2,
           ),
-          intersection ?? center,
+          adjacentPoint,
-        );
+        ),
-
+      ),
-      default:
+      FIXED_BINDING_DISTANCE,
-        return headingToMidBindPoint(p, bindableElement, aabb);
+    ).sort(
-    }
+      (g, h) =>
+        pointDistanceSq(g, adjacentPoint) - pointDistanceSq(h, adjacentPoint),
+    )[0];
   }
 
-  return p;
+  if (
-};
+    !intersection ||
-
+    // Too close to determine vector from intersection to edgePoint
-const headingToMidBindPoint = (
+    pointDistanceSq(edgePoint, intersection) < PRECISION
-  p: GlobalPoint,
+  ) {
-  bindableElement: ExcalidrawBindableElement,
+    return edgePoint;
-  aabb: Bounds,
-): GlobalPoint => {
-  const center = getCenterForBounds(aabb);
-  const heading = vectorToHeading(vectorFromPoint(p, center));
-
-  switch (true) {
-    case compareHeading(heading, HEADING_UP):
-      return pointRotateRads(
-        pointFrom((aabb[0] + aabb[2]) / 2 + 0.1, aabb[1]),
-        center,
-        bindableElement.angle,
-      );
-    case compareHeading(heading, HEADING_RIGHT):
-      return pointRotateRads(
-        pointFrom(aabb[2], (aabb[1] + aabb[3]) / 2 + 0.1),
-        center,
-        bindableElement.angle,
-      );
-    case compareHeading(heading, HEADING_DOWN):
-      return pointRotateRads(
-        pointFrom((aabb[0] + aabb[2]) / 2 - 0.1, aabb[3]),
-        center,
-        bindableElement.angle,
-      );
-    default:
-      return pointRotateRads(
-        pointFrom(aabb[0], (aabb[1] + aabb[3]) / 2 - 0.1),
-        center,
-        bindableElement.angle,
-      );
   }
+
+  if (elbowed) {
+    const scalar =
+      pointDistanceSq(edgePoint, center) -
+        pointDistanceSq(intersection, center) >
+      0
+        ? FIXED_BINDING_DISTANCE
+        : -FIXED_BINDING_DISTANCE;
+
+    return pointFromVector(
+      vectorScale(
+        vectorNormalize(vectorFromPoint(edgePoint, intersection)),
+        scalar,
+      ),
+      intersection,
+    );
+  }
+
+  return edgePoint;
 };
 
 export const avoidRectangularCorner = (
@@ -1146,7 +1153,7 @@ export const snapToMid = (
   ) {
     // LEFT
     return pointRotateRads(
-      pointFrom(x - FIXED_BINDING_DISTANCE, center[1]),
+      pointFrom(x - 2 * FIXED_BINDING_DISTANCE, center[1]),
       center,
       angle,
     );
@@ -1157,7 +1164,7 @@ export const snapToMid = (
   ) {
     // TOP
     return pointRotateRads(
-      pointFrom(center[0], y - FIXED_BINDING_DISTANCE),
+      pointFrom(center[0], y - 2 * FIXED_BINDING_DISTANCE),
       center,
       angle,
     );
@@ -1168,7 +1175,7 @@ export const snapToMid = (
   ) {
     // RIGHT
     return pointRotateRads(
-      pointFrom(x + width + FIXED_BINDING_DISTANCE, center[1]),
+      pointFrom(x + width + 2 * FIXED_BINDING_DISTANCE, center[1]),
       center,
       angle,
     );
@@ -1179,7 +1186,7 @@ export const snapToMid = (
   ) {
     // DOWN
     return pointRotateRads(
-      pointFrom(center[0], y + height + FIXED_BINDING_DISTANCE),
+      pointFrom(center[0], y + height + 2 * FIXED_BINDING_DISTANCE),
       center,
       angle,
     );

packages/excalidraw/element/elbowArrow.ts

@@ -40,7 +40,7 @@ import {
   headingForPoint,
 } from "./heading";
 import { type ElementUpdate } from "./mutateElement";
-import { isBindableElement } from "./typeChecks";
+import { isBindableElement, isElbowArrow } from "./typeChecks";
 import {
   type ExcalidrawElbowArrowElement,
   type NonDeletedSceneElementsMap,
@@ -58,6 +58,7 @@ import type {
   NonDeletedExcalidrawElement,
 } from "./types";
 import type { AppState } from "../types";
+import { debugClear } from "../visualdebug";
 
 type GridAddress = [number, number] & { _brand: "gridaddress" };
 
@@ -238,16 +239,6 @@ const handleSegmentRenormalization = (
                 nextPoints.map((p) =>
                   pointFrom<LocalPoint>(p[0] - arrow.x, p[1] - arrow.y),
                 ),
-                arrow.startBinding &&
-                  getBindableElementForId(
-                    arrow.startBinding.elementId,
-                    elementsMap,
-                  ),
-                arrow.endBinding &&
-                  getBindableElementForId(
-                    arrow.endBinding.elementId,
-                    elementsMap,
-                  ),
               ),
             ) ?? [],
           ),
@@ -341,9 +332,6 @@ const handleSegmentRelease = (
           y,
       ),
     ],
-    startBinding &&
-      getBindableElementForId(startBinding.elementId, elementsMap),
-    endBinding && getBindableElementForId(endBinding.elementId, elementsMap),
     { isDragging: false },
   );
 
@@ -983,6 +971,8 @@ export const updateElbowArrowPoints = (
     );
   }
 
+  const fixedSegments = updates.fixedSegments ?? arrow.fixedSegments ?? [];
+
   const updatedPoints: readonly LocalPoint[] = updates.points
     ? updates.points && updates.points.length === 2
       ? arrow.points.map((p, idx) =>
@@ -1055,12 +1045,22 @@ export const updateElbowArrowPoints = (
     },
     elementsMap,
     updatedPoints,
-    startElement,
-    endElement,
     options,
   );
 
-  const fixedSegments = updates.fixedSegments ?? arrow.fixedSegments ?? [];
+  // 0. During all element replacement in the scene, we just need to renormalize
+  // the arrow
+  // TODO (dwelle,mtolmacs): Remove this once Scene.getScene() is removed
+  if (elementsMap.size === 0 && validateElbowPoints(updatedPoints)) {
+    return normalizeArrowElementUpdate(
+      updatedPoints.map((p) =>
+        pointFrom<GlobalPoint>(arrow.x + p[0], arrow.y + p[1]),
+      ),
+      arrow.fixedSegments,
+      arrow.startIsSpecial,
+      arrow.endIsSpecial,
+    );
+  }
 
   ////
   // 1. Renormalize the arrow
@@ -1195,8 +1195,6 @@ const getElbowArrowData = (
   },
   elementsMap: NonDeletedSceneElementsMap,
   nextPoints: readonly LocalPoint[],
-  startElement: ExcalidrawBindableElement | null,
-  endElement: ExcalidrawBindableElement | null,
   options?: {
     isDragging?: boolean;
     zoom?: AppState["zoom"];
@@ -1211,8 +1209,8 @@ const getElbowArrowData = (
     GlobalPoint
   >(nextPoints[nextPoints.length - 1], vector(arrow.x, arrow.y));
 
-  let hoveredStartElement = startElement;
+  let hoveredStartElement = null;
-  let hoveredEndElement = endElement;
+  let hoveredEndElement = null;
   if (options?.isDragging) {
     const elements = Array.from(elementsMap.values());
     hoveredStartElement =
@@ -1221,39 +1219,48 @@ const getElbowArrowData = (
         elementsMap,
         elements,
         options?.zoom,
-      ) || startElement;
+      ) || null;
     hoveredEndElement =
       getHoveredElement(
         origEndGlobalPoint,
         elementsMap,
         elements,
         options?.zoom,
-      ) || endElement;
+      ) || null;
+  } else {
+    hoveredStartElement = arrow.startBinding
+      ? getBindableElementForId(arrow.startBinding.elementId, elementsMap) ||
+        null
+      : null;
+    hoveredEndElement = arrow.endBinding
+      ? getBindableElementForId(arrow.endBinding.elementId, elementsMap) || null
+      : null;
   }
-
+  debugClear();
+  console.log("----");
   const startGlobalPoint = getGlobalPoint(
     {
       ...arrow,
+      type: "arrow",
       elbowed: true,
       points: nextPoints,
     } as ExcalidrawElbowArrowElement,
     "start",
     arrow.startBinding?.fixedPoint,
     origStartGlobalPoint,
-    startElement,
     hoveredStartElement,
     options?.isDragging,
   );
   const endGlobalPoint = getGlobalPoint(
     {
       ...arrow,
+      type: "arrow",
       elbowed: true,
       points: nextPoints,
     } as ExcalidrawElbowArrowElement,
     "end",
     arrow.endBinding?.fixedPoint,
     origEndGlobalPoint,
-    endElement,
     hoveredEndElement,
     options?.isDragging,
   );
@@ -2199,36 +2206,35 @@ const getGlobalPoint = (
   startOrEnd: "start" | "end",
   fixedPointRatio: [number, number] | undefined | null,
   initialPoint: GlobalPoint,
-  boundElement?: ExcalidrawBindableElement | null,
+  element?: ExcalidrawBindableElement | null,
-  hoveredElement?: ExcalidrawBindableElement | null,
   isDragging?: boolean,
 ): GlobalPoint => {
   if (isDragging) {
-    if (hoveredElement) {
+    if (element) {
       const snapPoint = bindPointToSnapToElementOutline(
         arrow,
-        hoveredElement,
+        element,
         startOrEnd,
       );
 
-      return snapToMid(hoveredElement, snapPoint);
+      return snapToMid(element, snapPoint);
     }
 
     return initialPoint;
   }
 
-  if (boundElement) {
+  if (element) {
     const fixedGlobalPoint = getGlobalFixedPointForBindableElement(
       fixedPointRatio || [0, 0],
-      boundElement,
+      element,
     );
 
     // NOTE: Resize scales the binding position point too, so we need to update it
     return Math.abs(
-      distanceToBindableElement(boundElement, fixedGlobalPoint) -
+      distanceToBindableElement(element, fixedGlobalPoint) -
         FIXED_BINDING_DISTANCE,
     ) > 0.01
-      ? bindPointToSnapToElementOutline(arrow, boundElement, startOrEnd)
+      ? bindPointToSnapToElementOutline(arrow, element, startOrEnd)
       : fixedGlobalPoint;
   }