Merge branch 'master' into mtolmacs/fix/small-elbow-routing

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

excalidraw-app/components/DebugCanvas.tsx

@@ -18,7 +18,7 @@ import {
 } from "@excalidraw/math";
 import { isCurve } from "@excalidraw/math/curve";
 
-import type { DebugElement } from "@excalidraw/excalidraw/visualdebug";
+import type { DebugElement } from "@excalidraw/utils/visualdebug";
 
 import type { Curve } from "@excalidraw/math";
 

packages/element/src/binding.ts

@@ -43,6 +43,12 @@ import {
 import { intersectElementWithLineSegment } from "./collision";
 import { distanceToBindableElement } from "./distance";
 import {
+  compareHeading,
+  HEADING_DOWN,
+  HEADING_LEFT,
+  HEADING_RIGHT,
+  HEADING_UP,
+  headingForPoint,
   headingForPointFromElement,
   headingIsHorizontal,
   vectorToHeading,
@@ -1025,7 +1031,14 @@ export const avoidRectangularCorner = (
 
   if (nonRotatedPoint[0] < element.x && nonRotatedPoint[1] < element.y) {
     // Top left
-    if (nonRotatedPoint[1] - element.y > -FIXED_BINDING_DISTANCE) {
+    const heading = headingForPoint(
+      nonRotatedPoint,
+      pointFrom(element.x, element.y),
+    );
+    if (
+      compareHeading(heading, HEADING_DOWN) ||
+      compareHeading(heading, HEADING_LEFT)
+    ) {
       return pointRotateRads<GlobalPoint>(
         pointFrom(element.x - FIXED_BINDING_DISTANCE, element.y),
         center,
@@ -1042,7 +1055,14 @@ export const avoidRectangularCorner = (
     nonRotatedPoint[1] > element.y + element.height
   ) {
     // Bottom left
-    if (nonRotatedPoint[0] - element.x > -FIXED_BINDING_DISTANCE) {
+    const heading = headingForPoint(
+      nonRotatedPoint,
+      pointFrom(element.x, element.y + element.height),
+    );
+    if (
+      compareHeading(heading, HEADING_DOWN) ||
+      compareHeading(heading, HEADING_RIGHT)
+    ) {
       return pointRotateRads(
         pointFrom(
           element.x,
@@ -1062,9 +1082,13 @@ export const avoidRectangularCorner = (
     nonRotatedPoint[1] > element.y + element.height
   ) {
     // Bottom right
+    const heading = headingForPoint(
+      nonRotatedPoint,
+      pointFrom(element.x + element.width, element.y + element.height),
+    );
     if (
-      nonRotatedPoint[0] - element.x <
-      element.width + FIXED_BINDING_DISTANCE
+      compareHeading(heading, HEADING_DOWN) ||
+      compareHeading(heading, HEADING_LEFT)
     ) {
       return pointRotateRads(
         pointFrom(
@@ -1088,9 +1112,13 @@ export const avoidRectangularCorner = (
     nonRotatedPoint[1] < element.y
   ) {
     // Top right
+    const heading = headingForPoint(
+      nonRotatedPoint,
+      pointFrom(element.x + element.width, element.y),
+    );
     if (
-      nonRotatedPoint[0] - element.x <
-      element.width + FIXED_BINDING_DISTANCE
+      compareHeading(heading, HEADING_UP) ||
+      compareHeading(heading, HEADING_LEFT)
     ) {
       return pointRotateRads(
         pointFrom(
@@ -1108,6 +1136,17 @@ export const avoidRectangularCorner = (
     );
   }
 
+  // Break up explicit border bindings to have better elbow arrow routing
+  if (p[0] === element.x) {
+    return pointFrom(p[0] - FIXED_BINDING_DISTANCE, p[1]);
+  } else if (p[0] === element.x + element.width) {
+    return pointFrom(p[0] + FIXED_BINDING_DISTANCE, p[1]);
+  } else if (p[1] === element.y) {
+    return pointFrom(p[0], p[1] - FIXED_BINDING_DISTANCE);
+  } else if (p[1] === element.y + element.height) {
+    return pointFrom(p[0], p[1] + FIXED_BINDING_DISTANCE);
+  }
+
   return p;
 };
 

packages/element/src/elbowArrow.ts

@@ -52,7 +52,7 @@ import {
   type NonDeletedSceneElementsMap,
 } from "./types";
 
-import { aabbForElement, pointInsideBounds } from "./shapes";
+import { aabbForElement, aabbForPoints, pointInsideBounds } from "./shapes";
 
 import type { Bounds } from "./bounds";
 import type { Heading } from "./heading";
@@ -65,6 +65,8 @@ import type {
   NonDeletedExcalidrawElement,
 } from "./types";
 
+import { debugDrawBounds } from "@excalidraw/utils/visualdebug";
+
 type GridAddress = [number, number] & { _brand: "gridaddress" };
 
 type Node = {
@@ -106,8 +108,32 @@ type ElbowArrowData = {
   hoveredEndElement: ExcalidrawBindableElement | null;
 };
 
-const DEDUP_TRESHOLD = 1;
-export const BASE_PADDING = 40;
+const calculateDedupTreshhold = <Point extends GlobalPoint | LocalPoint>(
+  a: Point,
+  b: Point,
+) => 1 + pointDistance(a, b) / 300;
+
+const calculatePadding = (
+  aabb: Bounds,
+  startBoundingBox: Bounds,
+  endBoundingBox: Bounds,
+) => {
+  return Math.max(
+    Math.min(
+      Math.hypot(
+        startBoundingBox[2] - startBoundingBox[0],
+        startBoundingBox[3] - startBoundingBox[1],
+      ) / 4,
+      Math.hypot(
+        endBoundingBox[2] - endBoundingBox[0],
+        endBoundingBox[3] - endBoundingBox[1],
+      ) / 4,
+      Math.hypot(aabb[2] - aabb[0], aabb[3] - aabb[1]) / 4,
+      40,
+    ),
+    30,
+  );
+};
 
 const handleSegmentRenormalization = (
   arrow: ExcalidrawElbowArrowElement,
@@ -183,7 +209,11 @@ const handleSegmentRenormalization = (
 
       if (
         // Remove segments that are too short
-        pointDistance(points[i - 2], points[i - 1]) < DEDUP_TRESHOLD
+        pointDistance(points[i - 2], points[i - 1]) <
+        calculateDedupTreshhold(
+          points[i - 3] ?? points[i - 3],
+          points[i] ?? points[i - 1],
+        )
       ) {
         const prevPrevSegmentIdx =
           nextFixedSegments?.findIndex((segment) => segment.index === i - 2) ??
@@ -359,6 +389,10 @@ const handleSegmentRelease = (
     null,
   );
 
+  if (!restoredPoints) {
+    return {};
+  }
+
   const nextPoints: GlobalPoint[] = [];
 
   // First part of the arrow are the old points
@@ -463,6 +497,13 @@ const handleSegmentMove = (
   hoveredStartElement: ExcalidrawBindableElement | null,
   hoveredEndElement: ExcalidrawBindableElement | null,
 ): ElementUpdate<ExcalidrawElbowArrowElement> => {
+  const BASE_PADDING = calculatePadding(
+    aabbForElement(arrow),
+    hoveredStartElement
+      ? aabbForElement(hoveredStartElement)
+      : [10, 10, 10, 10],
+    hoveredEndElement ? aabbForElement(hoveredEndElement) : [10, 10, 10, 10],
+  );
   const activelyModifiedSegmentIdx = fixedSegments
     .map((segment, i) => {
       if (
@@ -707,6 +748,13 @@ const handleEndpointDrag = (
   hoveredStartElement: ExcalidrawBindableElement | null,
   hoveredEndElement: ExcalidrawBindableElement | null,
 ) => {
+  const BASE_PADDING = calculatePadding(
+    aabbForPoints([startGlobalPoint, endGlobalPoint]),
+    hoveredStartElement
+      ? aabbForElement(hoveredStartElement)
+      : [10, 10, 10, 10],
+    hoveredEndElement ? aabbForElement(hoveredEndElement) : [10, 10, 10, 10],
+  );
   let startIsSpecial = arrow.startIsSpecial ?? null;
   let endIsSpecial = arrow.endIsSpecial ?? null;
   const globalUpdatedPoints = updatedPoints.map((p, i) =>
@@ -741,6 +789,7 @@ const handleEndpointDrag = (
 
   // Calculate the moving second point connection and add the start point
   {
+    startIsSpecial = arrow.startIsSpecial && globalUpdatedPoints.length > 2;
     const secondPoint = globalUpdatedPoints[startIsSpecial ? 2 : 1];
     const thirdPoint = globalUpdatedPoints[startIsSpecial ? 3 : 2];
     const startIsHorizontal = headingIsHorizontal(startHeading);
@@ -801,6 +850,7 @@ const handleEndpointDrag = (
 
   // Calculate the moving second to last point connection
   {
+    endIsSpecial = arrow.endIsSpecial && globalUpdatedPoints.length > 2;
     const secondToLastPoint =
       globalUpdatedPoints[globalUpdatedPoints.length - (endIsSpecial ? 3 : 2)];
     const thirdToLastPoint =
@@ -1293,29 +1343,28 @@ const getElbowArrowData = (
     endGlobalPoint[0] + 2,
     endGlobalPoint[1] + 2,
   ] as Bounds;
+  const BASE_PADDING = calculatePadding(
+    aabbForPoints([startGlobalPoint, endGlobalPoint]),
+    hoveredStartElement
+      ? aabbForElement(hoveredStartElement)
+      : [10, 10, 10, 10],
+    hoveredEndElement ? aabbForElement(hoveredEndElement) : [10, 10, 10, 10],
+  );
+  const startOffsets = offsetFromHeading(
+    startHeading,
+    arrow.startArrowhead ? FIXED_BINDING_DISTANCE * 4 : FIXED_BINDING_DISTANCE,
+    1,
+  );
+  const endOffsets = offsetFromHeading(
+    endHeading,
+    arrow.endArrowhead ? FIXED_BINDING_DISTANCE * 4 : FIXED_BINDING_DISTANCE,
+    1,
+  );
   const startElementBounds = hoveredStartElement
-    ? aabbForElement(
-        hoveredStartElement,
-        offsetFromHeading(
-          startHeading,
-          arrow.startArrowhead
-            ? FIXED_BINDING_DISTANCE * 6
-            : FIXED_BINDING_DISTANCE * 2,
-          1,
-        ),
-      )
+    ? aabbForElement(hoveredStartElement, startOffsets)
     : startPointBounds;
   const endElementBounds = hoveredEndElement
-    ? aabbForElement(
-        hoveredEndElement,
-        offsetFromHeading(
-          endHeading,
-          arrow.endArrowhead
-            ? FIXED_BINDING_DISTANCE * 6
-            : FIXED_BINDING_DISTANCE * 2,
-          1,
-        ),
-      )
+    ? aabbForElement(hoveredEndElement, endOffsets)
     : endPointBounds;
   const boundsOverlap =
     pointInsideBounds(
@@ -1358,7 +1407,7 @@ const getElbowArrowData = (
             : BASE_PADDING -
                 (arrow.startArrowhead
                   ? FIXED_BINDING_DISTANCE * 6
-                  : FIXED_BINDING_DISTANCE * 2),
+                  : FIXED_BINDING_DISTANCE),
           BASE_PADDING,
         ),
     boundsOverlap
@@ -1374,13 +1423,29 @@ const getElbowArrowData = (
             : BASE_PADDING -
                 (arrow.endArrowhead
                   ? FIXED_BINDING_DISTANCE * 6
-                  : FIXED_BINDING_DISTANCE * 2),
+                  : FIXED_BINDING_DISTANCE),
           BASE_PADDING,
         ),
     boundsOverlap,
-    hoveredStartElement && aabbForElement(hoveredStartElement),
-    hoveredEndElement && aabbForElement(hoveredEndElement),
+    hoveredStartElement
+      ? aabbForElement(hoveredStartElement)
+      : startPointBounds,
+    hoveredEndElement ? aabbForElement(hoveredEndElement) : endPointBounds,
   );
+
+  debugDrawBounds(startElementBounds, {
+    permanent: false,
+    color: "red",
+  });
+  debugDrawBounds(endElementBounds, {
+    permanent: false,
+    color: "green",
+  });
+  debugDrawBounds(dynamicAABBs, {
+    permanent: false,
+    color: "blue",
+  });
+
   const startDonglePosition = getDonglePosition(
     dynamicAABBs[0],
     startHeading,
@@ -1651,11 +1716,11 @@ const generateDynamicAABBs = (
   a: Bounds,
   b: Bounds,
   common: Bounds,
-  startDifference?: [number, number, number, number],
-  endDifference?: [number, number, number, number],
-  disableSideHack?: boolean,
-  startElementBounds?: Bounds | null,
-  endElementBounds?: Bounds | null,
+  startDifference: [number, number, number, number],
+  endDifference: [number, number, number, number],
+  disableSideHack: boolean,
+  startElementBounds: Bounds,
+  endElementBounds: Bounds,
 ): Bounds[] => {
   const startEl = startElementBounds ?? a;
   const endEl = endElementBounds ?? b;
@@ -1735,15 +1800,24 @@ const generateDynamicAABBs = (
       (second[0] + second[2]) / 2,
       (second[1] + second[3]) / 2,
     ];
-    if (b[0] > a[2] && a[1] > b[3]) {
+    if (
+      endElementBounds[0] > startElementBounds[2] &&
+      startElementBounds[1] > endElementBounds[3]
+    ) {
       // BOTTOM LEFT
       const cX = first[2] + (second[0] - first[2]) / 2;
       const cY = second[3] + (first[1] - second[3]) / 2;
 
       if (
         vectorCross(
-          vector(a[2] - endCenterX, a[1] - endCenterY),
-          vector(a[0] - endCenterX, a[3] - endCenterY),
+          vector(
+            startElementBounds[2] - endCenterX,
+            startElementBounds[1] - endCenterY,
+          ),
+          vector(
+            startElementBounds[0] - endCenterX,
+            startElementBounds[3] - endCenterY,
+          ),
         ) > 0
       ) {
         return [
@@ -1756,15 +1830,24 @@ const generateDynamicAABBs = (
         [first[0], cY, first[2], first[3]],
         [second[0], second[1], second[2], cY],
       ];
-    } else if (a[2] < b[0] && a[3] < b[1]) {
+    } else if (
+      startElementBounds[2] < endElementBounds[0] &&
+      startElementBounds[3] < endElementBounds[1]
+    ) {
       // TOP LEFT
       const cX = first[2] + (second[0] - first[2]) / 2;
       const cY = first[3] + (second[1] - first[3]) / 2;
 
       if (
         vectorCross(
-          vector(a[0] - endCenterX, a[1] - endCenterY),
-          vector(a[2] - endCenterX, a[3] - endCenterY),
+          vector(
+            startElementBounds[0] - endCenterX,
+            startElementBounds[1] - endCenterY,
+          ),
+          vector(
+            startElementBounds[2] - endCenterX,
+            startElementBounds[3] - endCenterY,
+          ),
         ) > 0
       ) {
         return [
@@ -1777,15 +1860,24 @@ const generateDynamicAABBs = (
         [first[0], first[1], cX, first[3]],
         [cX, second[1], second[2], second[3]],
       ];
-    } else if (a[0] > b[2] && a[3] < b[1]) {
+    } else if (
+      startElementBounds[0] > endElementBounds[2] &&
+      startElementBounds[3] < endElementBounds[1]
+    ) {
       // TOP RIGHT
       const cX = second[2] + (first[0] - second[2]) / 2;
       const cY = first[3] + (second[1] - first[3]) / 2;
 
       if (
         vectorCross(
-          vector(a[2] - endCenterX, a[1] - endCenterY),
-          vector(a[0] - endCenterX, a[3] - endCenterY),
+          vector(
+            startElementBounds[2] - endCenterX,
+            startElementBounds[1] - endCenterY,
+          ),
+          vector(
+            startElementBounds[0] - endCenterX,
+            startElementBounds[3] - endCenterY,
+          ),
         ) > 0
       ) {
         return [
@@ -1798,15 +1890,24 @@ const generateDynamicAABBs = (
         [first[0], first[1], first[2], cY],
         [second[0], cY, second[2], second[3]],
       ];
-    } else if (a[0] > b[2] && a[1] > b[3]) {
+    } else if (
+      startElementBounds[0] > endElementBounds[2] &&
+      startElementBounds[1] > endElementBounds[3]
+    ) {
       // BOTTOM RIGHT
       const cX = second[2] + (first[0] - second[2]) / 2;
       const cY = second[3] + (first[1] - second[3]) / 2;
 
       if (
         vectorCross(
-          vector(a[0] - endCenterX, a[1] - endCenterY),
-          vector(a[2] - endCenterX, a[3] - endCenterY),
+          vector(
+            startElementBounds[0] - endCenterX,
+            startElementBounds[1] - endCenterY,
+          ),
+          vector(
+            startElementBounds[2] - endCenterX,
+            startElementBounds[3] - endCenterY,
+          ),
         ) > 0
       ) {
         return [
@@ -2088,16 +2189,11 @@ const normalizeArrowElementUpdate = (
   nextFixedSegments: readonly FixedSegment[] | null,
   startIsSpecial?: ExcalidrawElbowArrowElement["startIsSpecial"],
   endIsSpecial?: ExcalidrawElbowArrowElement["startIsSpecial"],
-): {
-  points: LocalPoint[];
-  x: number;
-  y: number;
-  width: number;
-  height: number;
-  fixedSegments: readonly FixedSegment[] | null;
-  startIsSpecial?: ExcalidrawElbowArrowElement["startIsSpecial"];
-  endIsSpecial?: ExcalidrawElbowArrowElement["startIsSpecial"];
-} => {
+): ElementUpdate<ExcalidrawElbowArrowElement> => {
+  if (global.length === 0) {
+    return {};
+  }
+
   const offsetX = global[0][0];
   const offsetY = global[0][1];
   let points = global.map((p) =>
@@ -2185,7 +2281,10 @@ const removeElbowArrowShortSegments = (
 
       const prev = points[idx - 1];
       const prevDist = pointDistance(prev, p);
-      return prevDist > DEDUP_TRESHOLD;
+      return (
+        prevDist >
+        calculateDedupTreshhold(points[idx - 2] ?? prev, points[idx + 1] ?? p)
+      );
     });
   }
 
@@ -2288,13 +2387,16 @@ const gridAddressesEqual = (a: GridAddress, b: GridAddress): boolean =>
 
 export const validateElbowPoints = <P extends GlobalPoint | LocalPoint>(
   points: readonly P[],
-  tolerance: number = DEDUP_TRESHOLD,
+  tolerance?: number,
 ) =>
   points
     .slice(1)
-    .map(
-      (p, i) =>
-        Math.abs(p[0] - points[i][0]) < tolerance ||
-        Math.abs(p[1] - points[i][1]) < tolerance,
-    )
+    .map((p, i) => {
+      const t =
+        tolerance ??
+        calculateDedupTreshhold(points[i - 1] ?? points[i], points[i + 2] ?? p);
+      return (
+        Math.abs(p[0] - points[i][0]) < t || Math.abs(p[1] - points[i][1]) < t
+      );
+    })
     .every(Boolean);

packages/element/src/shapes.ts

@@ -282,6 +282,15 @@ export const mapIntervalToBezierT = <P extends GlobalPoint | LocalPoint>(
   );
 };
 
+export const aabbForPoints = <Point extends GlobalPoint | LocalPoint>(
+  points: Point[],
+): Bounds => [
+  Math.min(...points.map((point) => point[0])),
+  Math.min(...points.map((point) => point[1])),
+  Math.max(...points.map((point) => point[0])),
+  Math.max(...points.map((point) => point[1])),
+];
+
 /**
  * Get the axis-aligned bounding box for a given element
  */

packages/element/tests/elbowArrow.test.tsx

@@ -195,7 +195,7 @@ describe("elbow arrow routing", () => {
     expect(arrow.startBinding).not.toBe(null);
     expect(arrow.endBinding).not.toBe(null);
 
-    h.app.scene.mutateElement(arrow, {
+    scene.mutateElement(arrow, {
       points: [pointFrom<LocalPoint>(0, 0), pointFrom<LocalPoint>(90, 200)],
     });
 
@@ -295,11 +295,11 @@ describe("elbow arrow ui", () => {
     ) as HTMLInputElement;
     UI.updateInput(inputAngle, String("40"));
 
-    expect(arrow.points.map((point) => point.map(Math.round))).toEqual([
+    expect(arrow.points).toCloselyEqualPoints([
       [0, 0],
-      [35, 0],
-      [35, 165],
-      [103, 165],
+      [34.7791, 0],
+      [34.7791, 164.67],
+      [102.931, 164.67],
     ]);
   });
 

packages/element/tests/resize.test.tsx

@@ -510,12 +510,12 @@ describe("arrow element", () => {
       h.state,
     )[0] as ExcalidrawElbowArrowElement;
 
-    expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1);
+    expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.05);
     expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75);
 
     UI.resize(rectangle, "se", [-200, -150]);
 
-    expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1);
+    expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.05);
     expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75);
   });
 
@@ -538,11 +538,11 @@ describe("arrow element", () => {
       h.state,
     )[0] as ExcalidrawElbowArrowElement;
 
-    expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1);
+    expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(1.05);
     expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.75);
 
     UI.resize([rectangle, arrow], "nw", [300, 350]);
-    expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(0);
+    expect(arrow.startBinding?.fixedPoint?.[0]).toBeCloseTo(-0.05);
     expect(arrow.startBinding?.fixedPoint?.[1]).toBeCloseTo(0.25);
   });
 });

packages/excalidraw/actions/actionFlip.test.tsx

@@ -73,12 +73,12 @@ describe("flipping re-centers selection", () => {
     API.executeAction(actionFlipHorizontal);
 
     const rec1 = h.elements.find((el) => el.id === "rec1")!;
-    expect(rec1.x).toBeCloseTo(100, 0);
-    expect(rec1.y).toBeCloseTo(100, 0);
+    expect(rec1.x).toBeCloseTo(97.8678, 0);
+    expect(rec1.y).toBeCloseTo(97.444, 0);
 
     const rec2 = h.elements.find((el) => el.id === "rec2")!;
-    expect(rec2.x).toBeCloseTo(220, 0);
-    expect(rec2.y).toBeCloseTo(250, 0);
+    expect(rec2.x).toBeCloseTo(218, 0);
+    expect(rec2.y).toBeCloseTo(247, 0);
   });
 });