Merge 868ec3f505 into 6e655cdb24

packages/element/src/binding.ts

@@ -27,7 +27,7 @@ import {
   PRECISION,
 } from "@excalidraw/math";
 
-import { isPointOnShape } from "@excalidraw/utils/collision";
+import { isPointInShape, isPointOnShape } from "@excalidraw/utils/collision";
 
 import type { LocalPoint, Radians } from "@excalidraw/math";
 
@@ -63,7 +63,7 @@ import {
   isTextElement,
 } from "./typeChecks";
 
-import { aabbForElement, getElementShape, pointInsideBounds } from "./shapes";
+import { aabbForElement, getElementShape } from "./shapes";
 import { updateElbowArrowPoints } from "./elbowArrow";
 
 import type Scene from "./Scene";
@@ -107,8 +107,7 @@ export const isBindingEnabled = (appState: AppState): boolean => {
 };
 
 export const FIXED_BINDING_DISTANCE = 5;
-export const BINDING_HIGHLIGHT_THICKNESS = 10;
-export const BINDING_HIGHLIGHT_OFFSET = 4;
+const BINDING_HIGHLIGHT_THICKNESS = 10;
 
 const getNonDeletedElements = (
   scene: Scene,
@@ -230,7 +229,13 @@ const getOriginalBindingIfStillCloseOfLinearElementEdge = (
     const element = elementsMap.get(elementId);
     if (
       isBindableElement(element) &&
-      bindingBorderTest(element, coors, elementsMap, zoom)
+      bindingBorderTest(
+        element,
+        coors,
+        elementsMap,
+        zoom,
+        isElbowArrow(element),
+      )
     ) {
       return element;
     }
@@ -440,22 +445,13 @@ export const maybeBindLinearElement = (
 const normalizePointBinding = (
   binding: { focus: number; gap: number },
   hoveredElement: ExcalidrawBindableElement,
-) => {
-  let gap = binding.gap;
-  const maxGap = maxBindingGap(
-    hoveredElement,
-    hoveredElement.width,
-    hoveredElement.height,
-  );
-
-  if (gap > maxGap) {
-    gap = BINDING_HIGHLIGHT_THICKNESS + BINDING_HIGHLIGHT_OFFSET;
-  }
-  return {
-    ...binding,
-    gap,
-  };
-};
+) => ({
+  ...binding,
+  gap: Math.min(
+    binding.gap,
+    maxBindingGap(hoveredElement, hoveredElement.width, hoveredElement.height),
+  ),
+});
 
 export const bindLinearElement = (
   linearElement: NonDeleted<ExcalidrawLinearElement>,
@@ -567,19 +563,7 @@ export const getHoveredElementForBinding = (
       elements,
       (element) =>
         isBindableElement(element, false) &&
-        bindingBorderTest(
-          element,
-          pointerCoords,
-          elementsMap,
-          zoom,
-          (fullShape ||
-            !isBindingFallthroughEnabled(
-              element as ExcalidrawBindableElement,
-            )) &&
-            // disable fullshape snapping for frame elements so we
-            // can bind to frame children
-            !isFrameLikeElement(element),
-        ),
+        bindingBorderTest(element, pointerCoords, elementsMap, zoom, fullShape),
     ).filter((element) => {
       if (cullRest) {
         return false;
@@ -621,16 +605,7 @@ export const getHoveredElementForBinding = (
     elements,
     (element) =>
       isBindableElement(element, false) &&
-      bindingBorderTest(
-        element,
-        pointerCoords,
-        elementsMap,
-        zoom,
-        // disable fullshape snapping for frame elements so we
-        // can bind to frame children
-        (fullShape || !isBindingFallthroughEnabled(element)) &&
-          !isFrameLikeElement(element),
-      ),
+      bindingBorderTest(element, pointerCoords, elementsMap, zoom, fullShape),
   );
 
   return hoveredElement as NonDeleted<ExcalidrawBindableElement> | null;
@@ -1128,9 +1103,11 @@ export const snapToMid = (
   const horizontalThrehsold = clamp(tolerance * width, 5, 80);
 
   if (
-    nonRotated[0] <= x + width / 2 &&
-    nonRotated[1] > center[1] - verticalThrehsold &&
-    nonRotated[1] < center[1] + verticalThrehsold
+    element.type === "diamond"
+      ? nonRotated[0] <= x + width * (element.roundness ? 0.035 : 0)
+      : nonRotated[0] <= x + width / 2 &&
+        nonRotated[1] > center[1] - verticalThrehsold &&
+        nonRotated[1] < center[1] + verticalThrehsold
   ) {
     // LEFT
     return pointRotateRads(
@@ -1139,9 +1116,11 @@ export const snapToMid = (
       angle,
     );
   } else if (
-    nonRotated[1] <= y + height / 2 &&
-    nonRotated[0] > center[0] - horizontalThrehsold &&
-    nonRotated[0] < center[0] + horizontalThrehsold
+    element.type === "diamond"
+      ? nonRotated[1] <= y + height * (element.roundness ? 0.035 : 0)
+      : nonRotated[1] <= y + height / 2 &&
+        nonRotated[0] > center[0] - horizontalThrehsold &&
+        nonRotated[0] < center[0] + horizontalThrehsold
   ) {
     // TOP
     return pointRotateRads(
@@ -1150,9 +1129,11 @@ export const snapToMid = (
       angle,
     );
   } else if (
-    nonRotated[0] >= x + width / 2 &&
-    nonRotated[1] > center[1] - verticalThrehsold &&
-    nonRotated[1] < center[1] + verticalThrehsold
+    element.type === "diamond"
+      ? nonRotated[0] >= x + width * (element.roundness ? 1 - 0.035 : 1)
+      : nonRotated[0] >= x + width / 2 &&
+        nonRotated[1] > center[1] - verticalThrehsold &&
+        nonRotated[1] < center[1] + verticalThrehsold
   ) {
     // RIGHT
     return pointRotateRads(
@@ -1161,9 +1142,11 @@ export const snapToMid = (
       angle,
     );
   } else if (
-    nonRotated[1] >= y + height / 2 &&
-    nonRotated[0] > center[0] - horizontalThrehsold &&
-    nonRotated[0] < center[0] + horizontalThrehsold
+    element.type === "diamond"
+      ? nonRotated[1] >= y + height * (element.roundness ? 1 - 0.035 : 1)
+      : nonRotated[1] >= y + height / 2 &&
+        nonRotated[0] > center[0] - horizontalThrehsold &&
+        nonRotated[0] < center[0] + horizontalThrehsold
   ) {
     // DOWN
     return pointRotateRads(
@@ -1512,13 +1495,19 @@ export const bindingBorderTest = (
   fullShape?: boolean,
 ): boolean => {
   const threshold = maxBindingGap(element, element.width, element.height, zoom);
-
   const shape = getElementShape(element, elementsMap);
-  return (
-    isPointOnShape(pointFrom(x, y), shape, threshold) ||
-    (fullShape === true &&
-      pointInsideBounds(pointFrom(x, y), aabbForElement(element)))
-  );
+  const shouldTestInside =
+    // disable fullshape snapping for frame elements so we
+    // can bind to frame children
+    (fullShape || !isBindingFallthroughEnabled(element)) &&
+    !isFrameLikeElement(element);
+
+  return shouldTestInside
+    ? // Since `inShape` tests STRICTLY againt the insides of a shape
+      // we would need `onShape` as well to include the "borders"
+      isPointInShape(pointFrom(x, y), shape) ||
+        isPointOnShape(pointFrom(x, y), shape, threshold)
+    : isPointOnShape(pointFrom(x, y), shape, threshold);
 };
 
 export const maxBindingGap = (
@@ -1538,7 +1527,7 @@ export const maxBindingGap = (
     // bigger bindable boundary for bigger elements
     Math.min(0.25 * smallerDimension, 32),
     // keep in sync with the zoomed highlight
-    BINDING_HIGHLIGHT_THICKNESS / zoomValue + BINDING_HIGHLIGHT_OFFSET,
+    BINDING_HIGHLIGHT_THICKNESS / zoomValue + FIXED_BINDING_DISTANCE,
   );
 };
 

packages/excalidraw/actions/actionProperties.tsx

@@ -18,7 +18,6 @@ import {
   arrayToMap,
   getFontFamilyString,
   getShortcutKey,
-  tupleToCoors,
   getLineHeight,
 } from "@excalidraw/common";
 
@@ -26,9 +25,7 @@ import { getNonDeletedElements } from "@excalidraw/element";
 
 import {
   bindLinearElement,
-  bindPointToSnapToElementOutline,
   calculateFixedPointForElbowArrowBinding,
-  getHoveredElementForBinding,
   updateBoundElements,
 } from "@excalidraw/element/binding";
 
@@ -1607,63 +1604,16 @@ export const actionChangeArrowType = register({
             -1,
             elementsMap,
           );
-        const startHoveredElement =
-          !newElement.startBinding &&
-          getHoveredElementForBinding(
-            tupleToCoors(startGlobalPoint),
-            elements,
-            elementsMap,
-            appState.zoom,
-            false,
-            true,
-          );
-        const endHoveredElement =
-          !newElement.endBinding &&
-          getHoveredElementForBinding(
-            tupleToCoors(endGlobalPoint),
-            elements,
-            elementsMap,
-            appState.zoom,
-            false,
-            true,
-          );
-        const startElement = startHoveredElement
-          ? startHoveredElement
-          : newElement.startBinding &&
-            (elementsMap.get(
-              newElement.startBinding.elementId,
-            ) as ExcalidrawBindableElement);
-        const endElement = endHoveredElement
-          ? endHoveredElement
-          : newElement.endBinding &&
-            (elementsMap.get(
-              newElement.endBinding.elementId,
-            ) as ExcalidrawBindableElement);
-
-        const finalStartPoint = startHoveredElement
-          ? bindPointToSnapToElementOutline(
-              newElement,
-              startHoveredElement,
-              "start",
-            )
-          : startGlobalPoint;
-        const finalEndPoint = endHoveredElement
-          ? bindPointToSnapToElementOutline(
-              newElement,
-              endHoveredElement,
-              "end",
-            )
-          : endGlobalPoint;
-
-        startHoveredElement &&
-          bindLinearElement(
-            newElement,
-            startHoveredElement,
-            "start",
-            app.scene,
-          );
-        endHoveredElement &&
-          bindLinearElement(newElement, endHoveredElement, "end", app.scene);
+        const startElement =
+          newElement.startBinding &&
+          (elementsMap.get(
+            newElement.startBinding.elementId,
+          ) as ExcalidrawBindableElement);
+        const endElement =
+          newElement.endBinding &&
+          (elementsMap.get(
+            newElement.endBinding.elementId,
+          ) as ExcalidrawBindableElement);
 
         const startBinding =
           startElement && newElement.startBinding
@@ -1695,7 +1645,7 @@ export const actionChangeArrowType = register({
           startBinding,
           endBinding,
           ...updateElbowArrowPoints(newElement, elementsMap, {
-            points: [finalStartPoint, finalEndPoint].map(
+            points: [startGlobalPoint, endGlobalPoint].map(
               (p): LocalPoint =>
                 pointFrom(p[0] - newElement.x, p[1] - newElement.y),
             ),

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

@@ -335,7 +335,7 @@ exports[`Test Transform > Test arrow bindings > should bind arrows to existing t
   "endBinding": {
     "elementId": "text-2",
     "focus": 0,
-    "gap": 14,
+    "gap": 16,
   },
   "fillStyle": "solid",
   "frameId": null,
@@ -1538,7 +1538,7 @@ exports[`Test Transform > should transform the elements correctly when linear el
   "endBinding": {
     "elementId": "B",
     "focus": 0,
-    "gap": 14,
+    "gap": 32,
   },
   "fillStyle": "solid",
   "frameId": null,

packages/excalidraw/data/transform.test.ts

@@ -781,7 +781,7 @@ describe("Test Transform", () => {
       expect((arrow as ExcalidrawArrowElement).endBinding).toStrictEqual({
         elementId: "rect-1",
         focus: -0,
-        gap: 14,
+        gap: 25,
       });
       expect(rect.boundElements).toStrictEqual([
         {

packages/excalidraw/renderer/interactiveScene.ts

@@ -15,11 +15,7 @@ import {
   throttleRAF,
 } from "@excalidraw/common";
 
-import {
-  BINDING_HIGHLIGHT_OFFSET,
-  BINDING_HIGHLIGHT_THICKNESS,
-  maxBindingGap,
-} from "@excalidraw/element/binding";
+import { maxBindingGap } from "@excalidraw/element/binding";
 import { LinearElementEditor } from "@excalidraw/element/linearElementEditor";
 import {
   getOmitSidesForDevice,
@@ -96,6 +92,9 @@ import type {
   RenderableElementsMap,
 } from "../scene/types";
 
+const BINDING_HIGHLIGHT_OFFSET = 4;
+const BINDING_HIGHLIGHT_THICKNESS = 10;
+
 const renderElbowArrowMidPointHighlight = (
   context: CanvasRenderingContext2D,
   appState: InteractiveCanvasAppState,