Fixed point binding for simple arrows

packages/element/src/binding.ts

@@ -37,7 +37,7 @@ import {
   getCenterForBounds,
   getElementBounds,
 } from "./bounds";
-import { intersectElementWithLineSegment } from "./collision";
+import { intersectElementWithLineSegment, isPointInElement } from "./collision";
 import { distanceToElement } from "./distance";
 import {
   headingForPointFromElement,
@@ -127,6 +127,9 @@ export const bindOrUnbindLinearElement = (
   endBindingElement: ExcalidrawBindableElement | null | "keep",
   scene: Scene,
 ): void => {
+  const bothEndBoundToTheSameElement =
+    linearElement.startBinding?.elementId ===
+      linearElement.endBinding?.elementId && !!linearElement.startBinding;
   const elementsMap = scene.getNonDeletedElementsMap();
   const boundToElementIds: Set<ExcalidrawBindableElement["id"]> = new Set();
   const unboundFromElementIds: Set<ExcalidrawBindableElement["id"]> = new Set();
@@ -151,18 +154,20 @@ export const bindOrUnbindLinearElement = (
     elementsMap,
   );
 
-  const onlyUnbound = Array.from(unboundFromElementIds).filter(
+  if (!bothEndBoundToTheSameElement) {
-    (id) => !boundToElementIds.has(id),
+    const onlyUnbound = Array.from(unboundFromElementIds).filter(
-  );
+      (id) => !boundToElementIds.has(id),
+    );
 
-  getNonDeletedElements(scene, onlyUnbound).forEach((element) => {
+    getNonDeletedElements(scene, onlyUnbound).forEach((element) => {
-    scene.mutateElement(element, {
+      scene.mutateElement(element, {
-      boundElements: element.boundElements?.filter(
+        boundElements: element.boundElements?.filter(
-        (element) =>
+          (element) =>
-          element.type !== "arrow" || element.id !== linearElement.id,
+            element.type !== "arrow" || element.id !== linearElement.id,
-      ),
+        ),
+      });
     });
-  });
+  }
 };
 
 const bindOrUnbindLinearElementEdge = (
@@ -203,6 +208,7 @@ const bindOrUnbindLinearElementEdge = (
             linearElement,
             bindableElement,
             startOrEnd,
+            elementsMap,
           )
         : startOrEnd === "start" ||
           otherEdgeBindableElement.id !== bindableElement.id)
@@ -459,6 +465,7 @@ export const maybeBindLinearElement = (
         linearElement,
         hoveredElement,
         "end",
+        elementsMap,
       )
     ) {
       bindLinearElement(linearElement, hoveredElement, "end", scene);
@@ -487,29 +494,64 @@ export const bindLinearElement = (
     return;
   }
 
-  let binding: PointBinding | FixedPointBinding = {
+  const elementsMap = scene.getNonDeletedElementsMap();
-    elementId: hoveredElement.id,
+  let binding: PointBinding | FixedPointBinding;
-    ...normalizePointBinding(
-      calculateFocusAndGap(
-        linearElement,
-        hoveredElement,
-        startOrEnd,
-        scene.getNonDeletedElementsMap(),
-      ),
-      hoveredElement,
-    ),
-  };
 
   if (isElbowArrow(linearElement)) {
     binding = {
-      ...binding,
+      elementId: hoveredElement.id,
+      ...normalizePointBinding(
+        calculateFocusAndGap(
+          linearElement,
+          hoveredElement,
+          startOrEnd,
+          elementsMap,
+        ),
+        hoveredElement,
+      ),
       ...calculateFixedPointForElbowArrowBinding(
         linearElement,
         hoveredElement,
         startOrEnd,
-        scene.getNonDeletedElementsMap(),
+        elementsMap,
       ),
     };
+  } else {
+    // For non-elbow arrows, check if the endpoint is inside the shape
+    const edgePoint = LinearElementEditor.getPointAtIndexGlobalCoordinates(
+      linearElement,
+      startOrEnd === "start" ? 0 : -1,
+      elementsMap,
+    );
+
+    if (isPointInElement(edgePoint, hoveredElement, elementsMap)) {
+      // Use FixedPoint binding when the arrow endpoint is inside the shape
+      binding = {
+        elementId: hoveredElement.id,
+        focus: 0,
+        gap: 0,
+        ...calculateFixedPointForNonElbowArrowBinding(
+          linearElement,
+          hoveredElement,
+          startOrEnd,
+          elementsMap,
+        ),
+      };
+    } else {
+      // Use traditional focus/gap binding when the endpoint is outside the shape
+      binding = {
+        elementId: hoveredElement.id,
+        ...normalizePointBinding(
+          calculateFocusAndGap(
+            linearElement,
+            hoveredElement,
+            startOrEnd,
+            elementsMap,
+          ),
+          hoveredElement,
+        ),
+      };
+    }
   }
 
   scene.mutateElement(linearElement, {
@@ -532,14 +574,36 @@ const isLinearElementSimpleAndAlreadyBoundOnOppositeEdge = (
   linearElement: NonDeleted<ExcalidrawLinearElement>,
   bindableElement: ExcalidrawBindableElement,
   startOrEnd: "start" | "end",
+  elementsMap: ElementsMap,
 ): boolean => {
   const otherBinding =
     linearElement[startOrEnd === "start" ? "endBinding" : "startBinding"];
-  return isLinearElementSimpleAndAlreadyBound(
+
-    linearElement,
+  // Only prevent binding if opposite end is bound to the same element
-    otherBinding?.elementId,
+  if (
-    bindableElement,
+    otherBinding?.elementId !== bindableElement.id ||
-  );
+    !isLinearElementSimple(linearElement)
+  ) {
+    return false;
+  }
+
+  // For non-elbow arrows, allow FixedPoint binding even when both ends bind to the same element
+  if (!isElbowArrow(linearElement)) {
+    const currentEndPoint =
+      LinearElementEditor.getPointAtIndexGlobalCoordinates(
+        linearElement,
+        startOrEnd === "start" ? 0 : -1,
+        elementsMap,
+      );
+
+    // If current end would use FixedPoint binding, allow it
+    if (isPointInElement(currentEndPoint, bindableElement, elementsMap)) {
+      return false;
+    }
+  }
+
+  // Prevent traditional focus/gap binding when both ends would bind to the same element
+  return true;
 };
 
 export const isLinearElementSimpleAndAlreadyBound = (
@@ -1254,15 +1318,22 @@ const updateBoundPoint = (
   const direction = startOrEnd === "startBinding" ? -1 : 1;
   const edgePointIndex = direction === -1 ? 0 : linearElement.points.length - 1;
 
-  if (isElbowArrow(linearElement) && isFixedPointBinding(binding)) {
+  if (isFixedPointBinding(binding)) {
     const fixedPoint =
       normalizeFixedPoint(binding.fixedPoint) ??
-      calculateFixedPointForElbowArrowBinding(
+      (isElbowArrow(linearElement)
-        linearElement,
+        ? calculateFixedPointForElbowArrowBinding(
-        bindableElement,
+            linearElement,
-        startOrEnd === "startBinding" ? "start" : "end",
+            bindableElement,
-        elementsMap,
+            startOrEnd === "startBinding" ? "start" : "end",
-      ).fixedPoint;
+            elementsMap,
+          ).fixedPoint
+        : calculateFixedPointForNonElbowArrowBinding(
+            linearElement,
+            bindableElement,
+            startOrEnd === "startBinding" ? "start" : "end",
+            elementsMap,
+          ).fixedPoint);
     const globalMidPoint = elementCenterPoint(bindableElement, elementsMap);
     const global = pointFrom<GlobalPoint>(
       bindableElement.x + fixedPoint[0] * bindableElement.width,
@@ -1401,6 +1472,42 @@ export const calculateFixedPointForElbowArrowBinding = (
   };
 };
 
+export const calculateFixedPointForNonElbowArrowBinding = (
+  linearElement: NonDeleted<ExcalidrawLinearElement>,
+  hoveredElement: ExcalidrawBindableElement,
+  startOrEnd: "start" | "end",
+  elementsMap: ElementsMap,
+): { fixedPoint: FixedPoint } => {
+  const edgePoint = LinearElementEditor.getPointAtIndexGlobalCoordinates(
+    linearElement,
+    startOrEnd === "start" ? 0 : -1,
+    elementsMap,
+  );
+
+  // Convert the global point to element-local coordinates
+  const elementCenter = pointFrom(
+    hoveredElement.x + hoveredElement.width / 2,
+    hoveredElement.y + hoveredElement.height / 2,
+  );
+
+  // Rotate the point to account for element rotation
+  const nonRotatedPoint = pointRotateRads(
+    edgePoint,
+    elementCenter,
+    -hoveredElement.angle as Radians,
+  );
+
+  // Calculate the ratio relative to the element's bounds
+  const fixedPointX =
+    (nonRotatedPoint[0] - hoveredElement.x) / hoveredElement.width;
+  const fixedPointY =
+    (nonRotatedPoint[1] - hoveredElement.y) / hoveredElement.height;
+
+  return {
+    fixedPoint: normalizeFixedPoint([fixedPointX, fixedPointY]),
+  };
+};
+
 const maybeCalculateNewGapWhenScaling = (
   changedElement: ExcalidrawBindableElement,
   currentBinding: PointBinding | null | undefined,

packages/excalidraw/data/__snapshots__/transform.test.ts.snap

@@ -88,8 +88,12 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "endArrowhead": "arrow",
   "endBinding": {
     "elementId": "ellipse-1",
-    "focus": -0.007519379844961235,
+    "fixedPoint": [
-    "gap": 11.562288374879595,
+      0.04,
+      0.4633333333333333,
+    ],
+    "focus": 0,
+    "gap": 0,
   },
   "fillStyle": "solid",
   "frameId": null,
@@ -174,8 +178,12 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing s
   "startArrowhead": null,
   "startBinding": {
     "elementId": "diamond-1",
+    "fixedPoint": [
+      0.9357142857142857,
+      0.5001,
+    ],
     "focus": 0,
-    "gap": 4.535423522449215,
+    "gap": 0,
   },
   "strokeColor": "#e67700",
   "strokeStyle": "solid",
@@ -1539,8 +1547,12 @@ exports[`Test Transform > should transform the elements correctly when linear el
   "endArrowhead": "arrow",
   "endBinding": {
     "elementId": "B",
+    "fixedPoint": [
+      0.46387050630528887,
+      0.48466257668711654,
+    ],
     "focus": 0,
-    "gap": 32,
+    "gap": 0,
   },
   "fillStyle": "solid",
   "frameId": null,

packages/excalidraw/data/restore.ts

@@ -28,9 +28,14 @@ import { LinearElementEditor } from "@excalidraw/element";
 import { bumpVersion } from "@excalidraw/element";
 import { getContainerElement } from "@excalidraw/element";
 import { detectLineHeight } from "@excalidraw/element";
+import {
+  isPointInElement,
+  calculateFixedPointForNonElbowArrowBinding,
+} from "@excalidraw/element";
 import {
   isArrowBoundToElement,
   isArrowElement,
+  isBindableElement,
   isElbowArrow,
   isFixedPointBinding,
   isLinearElement,
@@ -519,6 +524,87 @@ const repairFrameMembership = (
   }
 };
 
+/**
+ * Migrates old PointBinding to FixedPointBinding for non-elbow arrows
+ * when arrow endpoints are inside bindable shapes.
+ *
+ * NOTE mutates element.
+ */
+const migratePointBindingToFixedPoint = (
+  element: Mutable<ExcalidrawElement>,
+  elementsMap: Map<string, Mutable<ExcalidrawElement>>,
+) => {
+  if (!isArrowElement(element) || isElbowArrow(element)) {
+    return;
+  }
+
+  let shouldUpdateElement = false;
+  let newStartBinding: FixedPointBinding | PointBinding | null =
+    element.startBinding;
+  let newEndBinding: FixedPointBinding | PointBinding | null =
+    element.endBinding;
+
+  // Check start binding
+  if (element.startBinding && !isFixedPointBinding(element.startBinding)) {
+    const boundElement = elementsMap.get(element.startBinding.elementId);
+    if (boundElement && isBindableElement(boundElement)) {
+      const edgePoint = LinearElementEditor.getPointAtIndexGlobalCoordinates(
+        element,
+        0,
+        elementsMap,
+      );
+      if (isPointInElement(edgePoint, boundElement, elementsMap)) {
+        const { fixedPoint } = calculateFixedPointForNonElbowArrowBinding(
+          element,
+          boundElement,
+          "start",
+          elementsMap,
+        );
+        newStartBinding = {
+          elementId: element.startBinding.elementId,
+          focus: 0,
+          gap: 0,
+          fixedPoint,
+        };
+        shouldUpdateElement = true;
+      }
+    }
+  }
+
+  // Check end binding
+  if (element.endBinding && !isFixedPointBinding(element.endBinding)) {
+    const boundElement = elementsMap.get(element.endBinding.elementId);
+    if (boundElement && isBindableElement(boundElement)) {
+      const edgePoint = LinearElementEditor.getPointAtIndexGlobalCoordinates(
+        element,
+        -1,
+        elementsMap,
+      );
+      if (isPointInElement(edgePoint, boundElement, elementsMap)) {
+        const { fixedPoint } = calculateFixedPointForNonElbowArrowBinding(
+          element,
+          boundElement,
+          "end",
+          elementsMap,
+        );
+        newEndBinding = {
+          elementId: element.endBinding.elementId,
+          focus: 0,
+          gap: 0,
+          fixedPoint,
+        };
+        shouldUpdateElement = true;
+      }
+    }
+  }
+
+  if (shouldUpdateElement) {
+    (element as Mutable<ExcalidrawLinearElement>).startBinding =
+      newStartBinding;
+    (element as Mutable<ExcalidrawLinearElement>).endBinding = newEndBinding;
+  }
+};
+
 export const restoreElements = (
   elements: ImportedDataState["elements"],
   /** NOTE doesn't serve for reconciliation */
@@ -598,6 +684,9 @@ export const restoreElements = (
         (element as Mutable<ExcalidrawLinearElement>).endBinding = null;
       }
     }
+
+    // Migrate old PointBinding to FixedPointBinding for non-elbow arrows
+    migratePointBindingToFixedPoint(element, restoredElementsMap);
   }
 
   // NOTE (mtolmacs): Temporary fix for extremely large arrows

packages/excalidraw/tests/__snapshots__/history.test.tsx.snap

@@ -195,7 +195,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "fillStyle": "solid",
   "frameId": null,
   "groupIds": [],
-  "height": "99.19972",
+  "height": 150,
   "id": "id4",
   "index": "a2",
   "isDeleted": false,
@@ -209,8 +209,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
       0,
     ],
     [
-      "98.40611",
+      "124.00500",
-      "99.19972",
+      150,
     ],
   ],
   "roughness": 1,
@@ -225,7 +225,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "type": "arrow",
   "updated": 1,
   "version": 35,
-  "width": "98.40611",
+  "width": "124.00500",
   "x": 1,
   "y": 0,
 }
@@ -332,15 +332,15 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               "focus": 0,
               "gap": 1,
             },
-            "height": "68.58402",
+            "height": "104.34908",
             "points": [
               [
                 0,
                 0,
               ],
               [
-                98,
+                "124.00500",
-                "68.58402",
+                "104.34908",
               ],
             ],
             "startBinding": {
@@ -356,7 +356,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               "focus": "-0.02000",
               "gap": 1,
             },
-            "height": "0.00656",
+            "height": "0.00849",
             "points": [
               [
                 0,
@@ -364,7 +364,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               ],
               [
                 "98.00000",
-                "-0.00656",
+                "-0.00849",
               ],
             ],
             "startBinding": {
@@ -415,15 +415,15 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
         },
         "id4": {
           "deleted": {
-            "height": "99.19972",
+            "height": 150,
             "points": [
               [
                 0,
                 0,
               ],
               [
-                "98.40611",
+                "124.00500",
-                "99.19972",
+                150,
               ],
             ],
             "startBinding": null,
@@ -431,15 +431,15 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
             "y": 0,
           },
           "inserted": {
-            "height": "68.58402",
+            "height": "104.34908",
             "points": [
               [
                 0,
                 0,
               ],
               [
-                98,
+                "124.00500",
-                "68.58402",
+                "104.34908",
               ],
             ],
             "startBinding": {
@@ -448,7 +448,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
               "gap": 1,
             },
             "version": 33,
-            "y": "35.82151",
+            "y": "45.65092",
           },
         },
       },
@@ -1228,7 +1228,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "fillStyle": "solid",
   "frameId": null,
   "groupIds": [],
-  "height": "1.36342",
+  "height": "49.99000",
   "id": "id4",
   "index": "Zz",
   "isDeleted": false,
@@ -1242,8 +1242,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
       0,
     ],
     [
-      98,
+      "150.01000",
-      "1.36342",
+      "49.99000",
     ],
   ],
   "roughness": 1,
@@ -1264,9 +1264,9 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "type": "arrow",
   "updated": 1,
   "version": 11,
-  "width": 98,
+  "width": "150.01000",
-  "x": 1,
+  "x": 0,
-  "y": 0,
+  "y": "0.01000",
 }
 `;
 
@@ -1591,7 +1591,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "fillStyle": "solid",
   "frameId": null,
   "groupIds": [],
-  "height": "1.36342",
+  "height": "49.99000",
   "id": "id5",
   "index": "a0",
   "isDeleted": false,
@@ -1605,8 +1605,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
       0,
     ],
     [
-      98,
+      "249.99000",
-      "1.36342",
+      "-49.99000",
     ],
   ],
   "roughness": 1,
@@ -1627,9 +1627,9 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "type": "arrow",
   "updated": 1,
   "version": 11,
-  "width": 98,
+  "width": "249.99000",
-  "x": 1,
+  "x": "-49.99000",
-  "y": 0,
+  "y": 50,
 }
 `;
 
@@ -1741,7 +1741,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
             "fillStyle": "solid",
             "frameId": null,
             "groupIds": [],
-            "height": "1.36342",
+            "height": "49.99000",
             "index": "a0",
             "isDeleted": false,
             "lastCommittedPoint": null,
@@ -1754,8 +1754,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
                 0,
               ],
               [
-                98,
+                "249.99000",
-                "1.36342",
+                "-49.99000",
               ],
             ],
             "roughness": 1,
@@ -1775,9 +1775,9 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
             "strokeWidth": 2,
             "type": "arrow",
             "version": 11,
-            "width": 98,
+            "width": "249.99000",
-            "x": 1,
+            "x": "-49.99000",
-            "y": 0,
+            "y": 50,
           },
           "inserted": {
             "isDeleted": true,