Inverted polygon now works just as well for hit testing

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

packages/element/src/Shape.ts

@@ -1,10 +1,17 @@
 import { simplify } from "points-on-curve";
 
-import { pointFrom, pointDistance, type LocalPoint } from "@excalidraw/math";
+import {
+  pointFrom,
+  pointDistance,
+  type LocalPoint,
+  pointRotateRads,
+} from "@excalidraw/math";
 import { ROUGHNESS, isTransparent, assertNever } from "@excalidraw/common";
 
 import { RoughGenerator } from "roughjs/bin/generator";
 
+import type { GlobalPoint, Radians } from "@excalidraw/math";
+
 import type { Mutable } from "@excalidraw/common/utility-types";
 
 import type { EmbedsValidationStatus } from "@excalidraw/excalidraw/types";
@@ -22,7 +29,11 @@ import { headingForPointIsHorizontal } from "./heading";
 
 import { canChangeRoundness } from "./comparisons";
 import { generateFreeDrawShape } from "./renderElement";
-import { getArrowheadPoints, getDiamondPoints } from "./bounds";
+import {
+  getArrowheadPoints,
+  getDiamondPoints,
+  getElementBounds,
+} from "./bounds";
 
 import type {
   ExcalidrawElement,
@@ -320,33 +331,92 @@ export const generateLinearCollisionShape = (
   switch (element.type) {
     case "line":
     case "arrow": {
-      let shape: any;
-
       // points array can be empty in the beginning, so it is important to add
       // initial position to it
       const points = element.points.length
         ? element.points
         : [pointFrom<LocalPoint>(0, 0)];
+      const [x1, y1, x2, y2] = getElementBounds(
+        {
+          ...element,
+          angle: 0 as Radians,
+        },
+        new Map(),
+      );
+      const center = pointFrom<GlobalPoint>((x1 + x2) / 2, (y1 + y2) / 2);
 
       if (isElbowArrow(element)) {
-        shape = generator.path(generateElbowArrowShape(points, 16), options)
+        return generator.path(generateElbowArrowShape(points, 16), options)
           .sets[0].ops;
       } else if (!element.roundness) {
-        shape = points.map((point, idx) => {
+        return points.map((point, idx) => {
-          return idx === 0
+          const p = pointRotateRads(
-            ? { op: "move", data: point }
+            pointFrom<GlobalPoint>(element.x + point[0], element.y + point[1]),
-            : {
+            center,
-                op: "lineTo",
+            element.angle,
-                data: [point[0], point[1]],
+          );
+
+          return {
+            op: idx === 0 ? "move" : "lineTo",
+            data: pointFrom<LocalPoint>(p[0] - element.x, p[1] - element.y),
           };
         });
-      } else {
-        shape = generator
-          .curve(points as unknown as RoughPoint[], options)
-          .sets[0].ops.slice(0, element.points.length);
       }
 
-      return shape;
+      return generator
+        .curve(points as unknown as RoughPoint[], options)
+        .sets[0].ops.slice(0, element.points.length)
+        .map((op, i, arr) => {
+          if (i === 0) {
+            const p = pointRotateRads<GlobalPoint>(
+              pointFrom<GlobalPoint>(
+                element.x + op.data[0],
+                element.y + op.data[1],
+              ),
+              center,
+              element.angle,
+            );
+
+            return {
+              op: "move",
+              data: pointFrom<LocalPoint>(p[0] - element.x, p[1] - element.y),
+            };
+          }
+
+          return {
+            op: "bcurveTo",
+            data: [
+              pointRotateRads(
+                pointFrom<GlobalPoint>(
+                  element.x + op.data[0],
+                  element.y + op.data[1],
+                ),
+                center,
+                element.angle,
+              ),
+              pointRotateRads(
+                pointFrom<GlobalPoint>(
+                  element.x + op.data[2],
+                  element.y + op.data[3],
+                ),
+                center,
+                element.angle,
+              ),
+              pointRotateRads(
+                pointFrom<GlobalPoint>(
+                  element.x + op.data[4],
+                  element.y + op.data[5],
+                ),
+                center,
+                element.angle,
+              ),
+            ]
+              .map((p) =>
+                pointFrom<LocalPoint>(p[0] - element.x, p[1] - element.y),
+              )
+              .flat(),
+          };
+        });
     }
     case "freedraw": {
       const simplifiedPoints = simplify(

packages/element/src/collision.ts

@@ -11,8 +11,12 @@ import {
   lineSegment,
   lineSegmentIntersectionPoints,
   pointFrom,
+  pointFromVector,
   pointRotateRads,
   pointsEqual,
+  vectorFromPoint,
+  vectorNormalize,
+  vectorScale,
 } from "@excalidraw/math";
 
 import {
@@ -20,8 +24,6 @@ import {
   ellipseSegmentInterceptPoints,
 } from "@excalidraw/math/ellipse";
 
-import { isPointInShape, isPointOnShape } from "@excalidraw/utils/collision";
-
 import type {
   Curve,
   GlobalPoint,
@@ -35,6 +37,7 @@ import { isPathALoop } from "./shapes";
 import { getElementBounds } from "./bounds";
 import {
   hasBoundTextElement,
+  isFreeDrawElement,
   isIframeLikeElement,
   isImageElement,
   isLinearElement,
@@ -50,6 +53,8 @@ import { getBoundTextElement } from "./textElement";
 
 import { LinearElementEditor } from "./linearElementEditor";
 
+import { distanceToElement } from "./distance";
+
 import type {
   ElementsMap,
   ExcalidrawDiamondElement,
@@ -219,11 +224,13 @@ const intersectLinearOrFreeDrawWithLineSegment = (
   for (const shape of shapes) {
     switch (true) {
       case isCurve(shape):
+        //debugDrawCubicBezier(shape);
         intersections.push(
           ...curveIntersectLineSegment(shape as Curve<GlobalPoint>, segment),
         );
         continue;
       case isLineSegment(shape):
+        //debugDrawLine(shape);
         const point = lineSegmentIntersectionPoints(
           segment,
           shape as LineSegment<GlobalPoint>,
@@ -362,3 +369,41 @@ const intersectEllipseWithLineSegment = (
     lineSegment(rotatedA, rotatedB),
   ).map((p) => pointRotateRads(p, center, element.angle));
 };
+
+// check if the given point is considered on the given shape's border
+const isPointOnShape = (
+  point: GlobalPoint,
+  element: ExcalidrawElement,
+  tolerance = 1,
+) => distanceToElement(element, point) <= tolerance;
+
+// check if the given point is considered inside the element's border
+export const isPointInShape = (
+  point: GlobalPoint,
+  element: ExcalidrawElement,
+) => {
+  if (
+    (isLinearElement(element) || isFreeDrawElement(element)) &&
+    !isPathALoop(element.points)
+  ) {
+    // There isn't any "inside" for a non-looping path
+    return false;
+  }
+
+  const [x1, y1, x2, y2] = getElementBounds(element, new Map());
+  const center = pointFrom<GlobalPoint>((x1 + x2) / 2, (y1 + y2) / 2);
+  const otherPoint = pointFromVector(
+    vectorScale(
+      vectorNormalize(vectorFromPoint(point, center, 0.1)),
+      Math.max(element.width, element.height) * 2,
+    ),
+    center,
+  );
+  const intersector = lineSegment(point, otherPoint);
+  const intersections = intersectElementWithLineSegment(
+    element,
+    intersector,
+  ).filter((item, pos, arr) => arr.indexOf(item) === pos);
+
+  return intersections.length % 2 === 1;
+};

packages/excalidraw/components/App.tsx

@@ -17,7 +17,7 @@ import {
   vectorDot,
   vectorNormalize,
 } from "@excalidraw/math";
-import { isPointInShape } from "@excalidraw/utils/collision";
+import { isPointInShape } from "@excalidraw/element/collision";
 
 import {
   COLOR_PALETTE,

packages/excalidraw/eraser/index.ts

@@ -1,5 +1,5 @@
 import { arrayToMap, easeOut, THEME } from "@excalidraw/common";
-import { getElementLineSegments } from "@excalidraw/element";
+import { getElementLineSegments, isPointInShape } from "@excalidraw/element";
 import {
   lineSegment,
   lineSegmentIntersectionPoints,
@@ -9,7 +9,6 @@ import {
 import { getElementsInGroup } from "@excalidraw/element";
 
 import { shouldTestInside } from "@excalidraw/element";
-import { isPointInShape } from "@excalidraw/utils/collision";
 import { hasBoundTextElement, isBoundToContainer } from "@excalidraw/element";
 import { getBoundTextElementId } from "@excalidraw/element";
 

packages/utils/src/collision.ts

@@ -1,85 +0,0 @@
-import {
-  lineSegment,
-  pointFrom,
-  type GlobalPoint,
-  vectorFromPoint,
-  vectorNormalize,
-  vectorScale,
-  pointFromVector,
-} from "@excalidraw/math";
-
-import { intersectElementWithLineSegment } from "@excalidraw/element/collision";
-
-import { elementCenterPoint } from "@excalidraw/common";
-
-import { distanceToElement } from "@excalidraw/element/distance";
-
-import { getCommonBounds, isLinearElement } from "@excalidraw/excalidraw";
-import { isFreeDrawElement } from "@excalidraw/element/typeChecks";
-import { isPathALoop } from "@excalidraw/element/shapes";
-
-import {
-  debugDrawLine,
-  debugDrawPoint,
-} from "@excalidraw/excalidraw/visualdebug";
-
-import type { ExcalidrawElement } from "@excalidraw/element/types";
-
-// check if the given point is considered on the given shape's border
-export const isPointOnShape = (
-  point: GlobalPoint,
-  element: ExcalidrawElement,
-  tolerance = 1,
-) => {
-  const distance = distanceToElement(element, point);
-
-  return distance <= tolerance;
-};
-
-// check if the given point is considered inside the element's border
-export const isPointInShape = (
-  point: GlobalPoint,
-  element: ExcalidrawElement,
-) => {
-  if (isLinearElement(element) || isFreeDrawElement(element)) {
-    if (isPathALoop(element.points)) {
-      const [minX, minY, maxX, maxY] = getCommonBounds([element]);
-      const center = pointFrom<GlobalPoint>(
-        (maxX + minX) / 2,
-        (maxY + minY) / 2,
-      );
-      const otherPoint = pointFromVector(
-        vectorScale(
-          vectorNormalize(vectorFromPoint(point, center, 0.1)),
-          Math.max(element.width, element.height) * 2,
-        ),
-        center,
-      );
-      const intersector = lineSegment(point, otherPoint);
-
-      // What about being on the center exactly?
-      const intersections = intersectElementWithLineSegment(
-        element,
-        intersector,
-      );
-
-      const hit = intersections.length % 2 === 1;
-
-      debugDrawLine(intersector, { color: hit ? "green" : "red" });
-      debugDrawPoint(point, { color: "black" });
-      debugDrawPoint(otherPoint, { color: "blue" });
-
-      return hit;
-    }
-
-    // There isn't any "inside" for a non-looping path
-    return false;
-  }
-
-  const intersections = intersectElementWithLineSegment(
-    element,
-    lineSegment(elementCenterPoint(element), point),
-  );
-
-  return intersections.length === 0;
-};

packages/utils/tests/collision.test.ts

@@ -1,90 +0,0 @@
-import {
-  curve,
-  degreesToRadians,
-  lineSegment,
-  lineSegmentRotate,
-  pointFrom,
-  pointRotateDegs,
-} from "@excalidraw/math";
-
-import type { Curve, Degrees, GlobalPoint } from "@excalidraw/math";
-
-import { pointOnCurve, pointOnPolyline } from "../src/collision";
-
-import type { Polyline } from "../src/shape";
-
-describe("point and curve", () => {
-  const c: Curve<GlobalPoint> = curve(
-    pointFrom(1.4, 1.65),
-    pointFrom(1.9, 7.9),
-    pointFrom(5.9, 1.65),
-    pointFrom(6.44, 4.84),
-  );
-
-  it("point on curve", () => {
-    expect(pointOnCurve(c[0], c, 10e-5)).toBe(true);
-    expect(pointOnCurve(c[3], c, 10e-5)).toBe(true);
-
-    expect(pointOnCurve(pointFrom(2, 4), c, 0.1)).toBe(true);
-    expect(pointOnCurve(pointFrom(4, 4.4), c, 0.1)).toBe(true);
-    expect(pointOnCurve(pointFrom(5.6, 3.85), c, 0.1)).toBe(true);
-
-    expect(pointOnCurve(pointFrom(5.6, 4), c, 0.1)).toBe(false);
-    expect(pointOnCurve(c[1], c, 0.1)).toBe(false);
-    expect(pointOnCurve(c[2], c, 0.1)).toBe(false);
-  });
-});
-
-describe("point and polylines", () => {
-  const polyline: Polyline<GlobalPoint> = [
-    lineSegment(pointFrom(1, 0), pointFrom(1, 2)),
-    lineSegment(pointFrom(1, 2), pointFrom(2, 2)),
-    lineSegment(pointFrom(2, 2), pointFrom(2, 1)),
-    lineSegment(pointFrom(2, 1), pointFrom(3, 1)),
-  ];
-
-  it("point on the line", () => {
-    expect(pointOnPolyline(pointFrom(1, 0), polyline)).toBe(true);
-    expect(pointOnPolyline(pointFrom(1, 2), polyline)).toBe(true);
-    expect(pointOnPolyline(pointFrom(2, 2), polyline)).toBe(true);
-    expect(pointOnPolyline(pointFrom(2, 1), polyline)).toBe(true);
-    expect(pointOnPolyline(pointFrom(3, 1), polyline)).toBe(true);
-
-    expect(pointOnPolyline(pointFrom(1, 1), polyline)).toBe(true);
-    expect(pointOnPolyline(pointFrom(2, 1.5), polyline)).toBe(true);
-    expect(pointOnPolyline(pointFrom(2.5, 1), polyline)).toBe(true);
-
-    expect(pointOnPolyline(pointFrom(0, 1), polyline)).toBe(false);
-    expect(pointOnPolyline(pointFrom(2.1, 1.5), polyline)).toBe(false);
-  });
-
-  it("point on the line with rotation", () => {
-    const truePoints = [
-      pointFrom(1, 0),
-      pointFrom(1, 2),
-      pointFrom(2, 2),
-      pointFrom(2, 1),
-      pointFrom(3, 1),
-    ];
-
-    truePoints.forEach((p) => {
-      const rotation = (Math.random() * 360) as Degrees;
-      const rotatedPoint = pointRotateDegs(p, pointFrom(0, 0), rotation);
-      const rotatedPolyline = polyline.map((line) =>
-        lineSegmentRotate(line, degreesToRadians(rotation), pointFrom(0, 0)),
-      );
-      expect(pointOnPolyline(rotatedPoint, rotatedPolyline)).toBe(true);
-    });
-
-    const falsePoints = [pointFrom(0, 1), pointFrom(2.1, 1.5)];
-
-    falsePoints.forEach((p) => {
-      const rotation = (Math.random() * 360) as Degrees;
-      const rotatedPoint = pointRotateDegs(p, pointFrom(0, 0), rotation);
-      const rotatedPolyline = polyline.map((line) =>
-        lineSegmentRotate(line, degreesToRadians(rotation), pointFrom(0, 0)),
-      );
-      expect(pointOnPolyline(rotatedPoint, rotatedPolyline)).toBe(false);
-    });
-  });
-});