fix: Reimplement rectangle intersection (#8367)

packages/excalidraw/element/binding.ts

@@ -72,6 +72,7 @@ import {
   vectorToHeading,
   type Heading,
 } from "./heading";
+import { segmentIntersectRectangleElement } from "../../utils/geometry/geometry";
 
 export type SuggestedBinding =
   | NonDeleted<ExcalidrawBindableElement>
@@ -753,6 +754,7 @@ export const bindPointToSnapToElementOutline = (
 
   if (bindableElement && aabb) {
     // TODO: Dirty hacks until tangents are properly calculated
+    const heading = headingForPointFromElement(bindableElement, aabb, point);
     const intersections = [
       ...intersectElementWithLine(
         bindableElement,
@@ -760,61 +762,22 @@ export const bindPointToSnapToElementOutline = (
         [point[0], point[1] + 2 * bindableElement.height],
         FIXED_BINDING_DISTANCE,
         elementsMap,
-      ).map((i) => {
+      ),
-        if (!isRectangularElement(bindableElement)) {
-          return i;
-        }
-
-        const d = distanceToBindableElement(
-          {
-            ...bindableElement,
-            x: Math.round(bindableElement.x),
-            y: Math.round(bindableElement.y),
-            width: Math.round(bindableElement.width),
-            height: Math.round(bindableElement.height),
-          },
-          [Math.round(i[0]), Math.round(i[1])],
-          new Map(),
-        );
-
-        return d >= bindableElement.height / 2 || d < FIXED_BINDING_DISTANCE
-          ? ([point[0], -1 * i[1]] as Point)
-          : ([point[0], i[1]] as Point);
-      }),
       ...intersectElementWithLine(
         bindableElement,
         [point[0] - 2 * bindableElement.width, point[1]],
         [point[0] + 2 * bindableElement.width, point[1]],
         FIXED_BINDING_DISTANCE,
         elementsMap,
-      ).map((i) => {
+      ),
-        if (!isRectangularElement(bindableElement)) {
-          return i;
-        }
-
-        const d = distanceToBindableElement(
-          {
-            ...bindableElement,
-            x: Math.round(bindableElement.x),
-            y: Math.round(bindableElement.y),
-            width: Math.round(bindableElement.width),
-            height: Math.round(bindableElement.height),
-          },
-          [Math.round(i[0]), Math.round(i[1])],
-          new Map(),
-        );
-
-        return d >= bindableElement.width / 2 || d < FIXED_BINDING_DISTANCE
-          ? ([-1 * i[0], point[1]] as Point)
-          : ([i[0], point[1]] as Point);
-      }),
     ];
 
-    const heading = headingForPointFromElement(bindableElement, aabb, point);
     const isVertical =
       compareHeading(heading, HEADING_LEFT) ||
       compareHeading(heading, HEADING_RIGHT);
-    const dist = distanceToBindableElement(bindableElement, point, elementsMap);
+    const dist = Math.abs(
+      distanceToBindableElement(bindableElement, point, elementsMap),
+    );
     const isInner = isVertical
       ? dist < bindableElement.width * -0.1
       : dist < bindableElement.height * -0.1;
@@ -1641,6 +1604,10 @@ const intersectElementWithLine = (
   gap: number = 0,
   elementsMap: ElementsMap,
 ): Point[] => {
+  if (isRectangularElement(element)) {
+    return segmentIntersectRectangleElement(element, [a, b], gap);
+  }
+
   const relateToCenter = relativizationToElementCenter(element, elementsMap);
   const aRel = GATransform.apply(relateToCenter, GAPoint.from(a));
   const bRel = GATransform.apply(relateToCenter, GAPoint.from(b));

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

@@ -191,7 +191,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "fillStyle": "solid",
   "frameId": null,
   "groupIds": [],
-  "height": "99.19726",
+  "height": 99,
   "id": "id166",
   "index": "a2",
   "isDeleted": false,
@@ -205,8 +205,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
       0,
     ],
     [
-      "98.40368",
+      "98.20800",
-      "99.19726",
+      99,
     ],
   ],
   "roughness": 1,
@@ -221,7 +221,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "type": "arrow",
   "updated": 1,
   "version": 40,
-  "width": "98.40368",
+  "width": "98.20800",
   "x": 1,
   "y": 0,
 }
@@ -387,15 +387,15 @@ History {
                 "focus": 0,
                 "gap": 1,
               },
-              "height": "99.19726",
+              "height": 99,
               "points": [
                 [
                   0,
                   0,
                 ],
                 [
-                  "98.40368",
+                  "98.20800",
-                  "99.19726",
+                  99,
                 ],
               ],
               "startBinding": null,
@@ -813,7 +813,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "updated": 1,
   "version": 30,
   "width": 0,
-  "x": 251,
+  "x": 200,
   "y": 0,
 }
 `;
@@ -1242,7 +1242,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
       0,
     ],
     [
-      98,
+      "98.00000",
       "-2.61991",
     ],
   ],
@@ -1266,8 +1266,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "type": "arrow",
   "updated": 1,
   "version": 11,
-  "width": 98,
+  "width": "98.00000",
-  "x": 1,
+  "x": "1.00000",
   "y": "3.98333",
 }
 `;
@@ -1607,7 +1607,7 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
       0,
     ],
     [
-      98,
+      "98.00000",
       "-2.61991",
     ],
   ],
@@ -1631,8 +1631,8 @@ exports[`history > multiplayer undo/redo > conflicts in arrows and their bindabl
   "type": "arrow",
   "updated": 1,
   "version": 11,
-  "width": 98,
+  "width": "98.00000",
-  "x": 1,
+  "x": "1.00000",
   "y": "3.98333",
 }
 `;
@@ -1764,7 +1764,7 @@ History {
                   0,
                 ],
                 [
-                  98,
+                  "98.00000",
                   "-22.36242",
                 ],
               ],
@@ -1786,9 +1786,9 @@ History {
               "strokeStyle": "solid",
               "strokeWidth": 2,
               "type": "arrow",
-              "width": 98,
+              "width": "98.00000",
               "x": 1,
-              "y": "34.00000",
+              "y": 34,
             },
             "inserted": {
               "isDeleted": true,
@@ -14847,7 +14847,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
       0,
     ],
     [
-      98,
+      "98.00000",
       0,
     ],
   ],
@@ -14868,7 +14868,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "type": "arrow",
   "updated": 1,
   "version": 10,
-  "width": 98,
+  "width": "98.00000",
   "x": 1,
   "y": 0,
 }
@@ -15540,7 +15540,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
       0,
     ],
     [
-      98,
+      "98.00000",
       0,
     ],
   ],
@@ -15561,7 +15561,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "type": "arrow",
   "updated": 1,
   "version": 10,
-  "width": 98,
+  "width": "98.00000",
   "x": 1,
   "y": 0,
 }
@@ -16157,7 +16157,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
       0,
     ],
     [
-      98,
+      "98.00000",
       0,
     ],
   ],
@@ -16178,7 +16178,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "type": "arrow",
   "updated": 1,
   "version": 10,
-  "width": 98,
+  "width": "98.00000",
   "x": 1,
   "y": 0,
 }
@@ -16772,7 +16772,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
       0,
     ],
     [
-      98,
+      "98.00000",
       0,
     ],
   ],
@@ -16793,7 +16793,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "type": "arrow",
   "updated": 1,
   "version": 10,
-  "width": 98,
+  "width": "98.00000",
   "x": 1,
   "y": 0,
 }
@@ -17484,7 +17484,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
       0,
     ],
     [
-      98,
+      "98.00000",
       0,
     ],
   ],
@@ -17505,7 +17505,7 @@ exports[`history > singleplayer undo/redo > should support bidirectional binding
   "type": "arrow",
   "updated": 1,
   "version": 11,
-  "width": 98,
+  "width": "98.00000",
   "x": 1,
   "y": 0,
 }

packages/excalidraw/tests/rotate.test.tsx

@@ -77,6 +77,6 @@ test("unselected bound arrows update when rotating their target elements", async
   expect(textArrow.x).toEqual(360);
   expect(textArrow.y).toEqual(300);
   expect(textArrow.points[0]).toEqual([0, 0]);
-  expect(textArrow.points[1][0]).toBeCloseTo(-94, 1);
+  expect(textArrow.points[1][0]).toBeCloseTo(-94, 0);
-  expect(textArrow.points[1][1]).toBeCloseTo(-116.1, 1);
+  expect(textArrow.points[1][1]).toBeCloseTo(-116.1, 0);
 });

packages/utils/geometry/geometry.ts

@@ -1,4 +1,13 @@
-import { distance2d } from "../../excalidraw/math";
+import type { ExcalidrawBindableElement } from "../../excalidraw/element/types";
+import {
+  addVectors,
+  distance2d,
+  rotatePoint,
+  scaleVector,
+  subtractVectors,
+} from "../../excalidraw/math";
+import type { LineSegment } from "../bbox";
+import { crossProduct } from "../bbox";
 import type {
   Point,
   Line,
@@ -968,3 +977,84 @@ export const pointInEllipse = (point: Point, ellipse: Ellipse) => {
     1
   );
 };
+
+/**
+ * Calculates the point two line segments with a definite start and end point
+ * intersect at.
+ */
+export const segmentsIntersectAt = (
+  a: Readonly<LineSegment>,
+  b: Readonly<LineSegment>,
+): Point | null => {
+  const r = subtractVectors(a[1], a[0]);
+  const s = subtractVectors(b[1], b[0]);
+  const denominator = crossProduct(r, s);
+
+  if (denominator === 0) {
+    return null;
+  }
+
+  const i = subtractVectors(b[0], a[0]);
+  const u = crossProduct(i, r) / denominator;
+  const t = crossProduct(i, s) / denominator;
+
+  if (u === 0) {
+    return null;
+  }
+
+  const p = addVectors(a[0], scaleVector(r, t));
+
+  if (t >= 0 && t < 1 && u >= 0 && u < 1) {
+    return p;
+  }
+
+  return null;
+};
+
+/**
+ * Determine intersection of a rectangular shaped element and a
+ * line segment.
+ *
+ * @param element The rectangular element to test against
+ * @param segment The segment intersecting the element
+ * @param gap Optional value to inflate the shape before testing
+ * @returns An array of intersections
+ */
+// TODO: Replace with final rounded rectangle code
+export const segmentIntersectRectangleElement = (
+  element: ExcalidrawBindableElement,
+  segment: LineSegment,
+  gap: number = 0,
+): Point[] => {
+  const bounds = [
+    element.x - gap,
+    element.y - gap,
+    element.x + element.width + gap,
+    element.y + element.height + gap,
+  ];
+  const center = [
+    (bounds[0] + bounds[2]) / 2,
+    (bounds[1] + bounds[3]) / 2,
+  ] as Point;
+
+  return [
+    [
+      rotatePoint([bounds[0], bounds[1]], center, element.angle),
+      rotatePoint([bounds[2], bounds[1]], center, element.angle),
+    ] as LineSegment,
+    [
+      rotatePoint([bounds[2], bounds[1]], center, element.angle),
+      rotatePoint([bounds[2], bounds[3]], center, element.angle),
+    ] as LineSegment,
+    [
+      rotatePoint([bounds[2], bounds[3]], center, element.angle),
+      rotatePoint([bounds[0], bounds[3]], center, element.angle),
+    ] as LineSegment,
+    [
+      rotatePoint([bounds[0], bounds[3]], center, element.angle),
+      rotatePoint([bounds[0], bounds[1]], center, element.angle),
+    ] as LineSegment,
+  ]
+    .map((s) => segmentsIntersectAt(segment, s))
+    .filter((i): i is Point => !!i);
+};