fix: Freedraw hit test precision

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

packages/common/src/constants.strokeWidth.ts

@@ -1,63 +0,0 @@
-import type { ExcalidrawElement } from "@excalidraw/element/types";
-
-export type StrokeWidthKey = "thin" | "medium" | "bold";
-
-export const STROKE_WIDTH_KEYS: readonly StrokeWidthKey[] = [
-  "thin",
-  "medium",
-  "bold",
-];
-
-export const STROKE_WIDTH: Readonly<
-  Record<StrokeWidthKey | "extraBold", ExcalidrawElement["strokeWidth"]>
-> = {
-  thin: 1,
-  medium: 2,
-  bold: 4,
-  extraBold: 8, // unused (may be introduced in the future)
-};
-
-// freedraw schema 2.0 uses thinner stroke, but to maintain backwards and
-// forwards compatibility, instead of changing the shape renderer, we scale
-// the stroke width by 1/2 (previous, thin was 1, medium 2 etc.)
-//
-// note that in the UI, STROKE_WIDTH.thin == FREEDRAW_STROKE_WIDTH.thin still
-export const FREEDRAW_STROKE_WIDTH: Readonly<
-  Record<StrokeWidthKey | "extraBold", ExcalidrawElement["strokeWidth"]>
-> = {
-  thin: 0.5,
-  medium: 1,
-  bold: 2,
-  extraBold: 4, // legacy (may be used again in the future)
-};
-
-const STROKE_WIDTH_TO_KEY = {
-  generic: Object.fromEntries(
-    Object.entries(STROKE_WIDTH).map(([key, value]) => [value, key]),
-  ) as Record<ExcalidrawElement["strokeWidth"], StrokeWidthKey | undefined>,
-  freedraw: Object.fromEntries(
-    Object.entries(FREEDRAW_STROKE_WIDTH).map(([key, value]) => [value, key]),
-  ) as Record<ExcalidrawElement["strokeWidth"], StrokeWidthKey | undefined>,
-};
-
-export const getStrokeWidthKeyForElement = (
-  element: Pick<ExcalidrawElement, "type" | "strokeWidth">,
-): StrokeWidthKey | null => {
-  const strokeWidthToKey =
-    element.type === "freedraw"
-      ? STROKE_WIDTH_TO_KEY.freedraw
-      : STROKE_WIDTH_TO_KEY.generic;
-
-  return strokeWidthToKey[element.strokeWidth] ?? null;
-};
-
-export const getStrokeWidthByKey = (
-  elementType: ExcalidrawElement["type"],
-  strokeWidthKey: StrokeWidthKey,
-): ExcalidrawElement["strokeWidth"] => {
-  return elementType === "freedraw"
-    ? FREEDRAW_STROKE_WIDTH[strokeWidthKey]
-    : STROKE_WIDTH[strokeWidthKey];
-};
-
-export const DEFAULT_ELEMENT_STROKE_WIDTH_KEY: StrokeWidthKey = "medium";

packages/common/src/constants.ts

@@ -5,10 +5,6 @@ import type {
 import type { AppProps, AppState } from "@excalidraw/excalidraw/types";
 
 import { COLOR_PALETTE } from "./colors";
-import {
-  STROKE_WIDTH,
-  DEFAULT_ELEMENT_STROKE_WIDTH_KEY,
-} from "./constants.strokeWidth";
 
 export const supportsResizeObserver =
   typeof window !== "undefined" && "ResizeObserver" in window;
@@ -408,6 +404,48 @@ export const ROUGHNESS = {
   cartoonist: 2,
 } as const;
 
+export type StrokeWidthKey = "thin" | "medium" | "bold";
+
+export const STROKE_WIDTH_KEYS: readonly StrokeWidthKey[] = [
+  "thin",
+  "medium",
+  "bold",
+];
+
+export const STROKE_WIDTH: Readonly<
+  Record<StrokeWidthKey | "extraBold", ExcalidrawElement["strokeWidth"]>
+> = {
+  thin: 1,
+  medium: 2,
+  bold: 4,
+  extraBold: 8, // unused (may be introduced in the future)
+};
+
+// freedraw schema 2.0 uses thinner stroke, but to maintain backwards and
+// forwards compatibility, instead of changing the shape renderer, we scale
+// the stroke width by 1/2 (previous, thin was 1, medium 2 etc.)
+//
+// note that in the UI, STROKE_WIDTH.thin == FREEDRAW_STROKE_WIDTH.thin still
+export const FREEDRAW_STROKE_WIDTH: Readonly<
+  Record<StrokeWidthKey | "extraBold", ExcalidrawElement["strokeWidth"]>
+> = {
+  thin: 0.5,
+  medium: 1,
+  bold: 2,
+  extraBold: 4, // legacy (may be used again in the future)
+};
+
+export const getStrokeWidthByKey = (
+  elementType: ExcalidrawElement["type"],
+  strokeWidthKey: StrokeWidthKey,
+): ExcalidrawElement["strokeWidth"] => {
+  return elementType === "freedraw"
+    ? FREEDRAW_STROKE_WIDTH[strokeWidthKey]
+    : STROKE_WIDTH[strokeWidthKey];
+};
+
+export const DEFAULT_ELEMENT_STROKE_WIDTH_KEY: StrokeWidthKey = "medium";
+
 export const DEFAULT_ELEMENT_PROPS: {
   strokeColor: ExcalidrawElement["strokeColor"];
   backgroundColor: ExcalidrawElement["backgroundColor"];

packages/common/src/index.ts

@@ -2,7 +2,6 @@ export * from "./binary-heap";
 export * from "./bounds";
 export * from "./colors";
 export * from "./constants";
-export * from "./constants.strokeWidth";
 export * from "./font-metadata";
 export * from "./queue";
 export * from "./keys";

packages/element/src/distance.ts

@@ -2,6 +2,7 @@ import {
   curvePointDistance,
   distanceToLineSegment,
   pointRotateRads,
+  polygonIncludesPointNonZero,
 } from "@excalidraw/math";
 
 import { ellipse, ellipseDistanceFromPoint } from "@excalidraw/math/ellipse";
@@ -47,8 +48,9 @@ export const distanceToElement = (
       return distanceToEllipseElement(element, elementsMap, p);
     case "line":
     case "arrow":
-    case "freedraw":
       return distanceToLinearOrFreeDraElement(element, elementsMap, p);
+    case "freedraw":
+      return distanceToFreeDrawElement(element, elementsMap, p);
   }
 };
 
@@ -145,3 +147,30 @@ const distanceToLinearOrFreeDraElement = (
     ...curves.map((a) => curvePointDistance(a, p)),
   );
 };
+
+/**
+ * Returns the distance of a point to a freedraw element.
+ *
+ * @param element The freedraw element
+ * @param p The point to consider
+ * @returns 0 if the point is within the inked area, otherwise the euclidean
+ * distance to the stroke outline
+ */
+const distanceToFreeDrawElement = (
+  element: ExcalidrawFreeDrawElement,
+  elementsMap: ElementsMap,
+  p: GlobalPoint,
+) => {
+  const [lines] = deconstructLinearOrFreeDrawElement(element, elementsMap);
+
+  if (lines.length === 0) {
+    return Infinity;
+  }
+
+  const polygon = lines.map((line) => line[0]);
+  if (polygonIncludesPointNonZero(p, polygon)) {
+    return 0;
+  }
+
+  return Math.min(...lines.map((s) => distanceToLineSegment(p, s)));
+};

packages/element/src/shape.ts

@@ -677,69 +677,45 @@ export const generateLinearCollisionShape = (
         });
     }
     case "freedraw": {
-      if (element.points.length < 2) {
+      const outlinePoints = getFreedrawOutlinePoints(element);
+
+      if (outlinePoints.length < 2) {
         return [];
       }
 
-      const simplifiedPoints = simplify(
-        element.points as Mutable<LocalPoint[]>,
-        0.75,
-      );
+      const collisionOutline =
+        element.strokeOptions?.variability === "constant" &&
+        CONSTANT_WIDTH_FREEDRAW.STREAMLINE > 0
+          ? (simplify(
+              outlinePoints as Mutable<LocalPoint[]>,
+              CONSTANT_WIDTH_FREEDRAW.STREAMLINE,
+            ) as [number, number][])
+          : outlinePoints;
 
-      return generator
-        .curve(simplifiedPoints as [number, number][], options)
-        .sets[0].ops.slice(0, element.points.length)
-        .map((op, i) => {
-          if (i === 0) {
-            const p = pointRotateRads<GlobalPoint>(
-              pointFrom<GlobalPoint>(
-                element.x + op.data[0],
-                element.y + op.data[1],
-              ),
-              center,
-              element.angle,
-            );
+      if (collisionOutline.length < 2) {
+        return [];
+      }
 
-            return {
-              op: "move",
-              data: pointFrom<LocalPoint>(p[0] - element.x, p[1] - element.y),
-            };
-          }
+      // Close the outline polygon so its boundary never has a gap at the seam.
+      const [firstX, firstY] = collisionOutline[0];
+      const [lastX, lastY] = collisionOutline[collisionOutline.length - 1];
+      const closedOutline =
+        firstX === lastX && firstY === lastY
+          ? collisionOutline
+          : [...collisionOutline, collisionOutline[0]];
 
-          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(),
-          };
-        });
+      return closedOutline.map((point, idx) => {
+        const p = pointRotateRads<GlobalPoint>(
+          pointFrom<GlobalPoint>(element.x + point[0], element.y + point[1]),
+          center,
+          element.angle,
+        );
+
+        return {
+          op: idx === 0 ? "move" : "lineTo",
+          data: pointFrom<LocalPoint>(p[0] - element.x, p[1] - element.y),
+        };
+      });
     }
   }
 };

packages/element/tests/collision.test.tsx

@@ -218,3 +218,54 @@ describe("hitElementItself cache", () => {
     ).toBe(true);
   });
 });
+
+describe("freedraw collision matches the rendered stroke width", () => {
+  // A straight, horizontal stroke centered on y === 0.
+  const points = Array.from({ length: 21 }, (_, i) =>
+    pointFrom<LocalPoint>(i * 5, 0),
+  );
+
+  const createFreeDraw = (variability: "variable" | "constant") =>
+    API.createElement({
+      type: "freedraw",
+      x: 0,
+      y: 0,
+      strokeWidth: 10,
+      points,
+      strokeOptions: { variability, streamline: 0.5 },
+    });
+
+  const distanceAt = (
+    element: ReturnType<typeof createFreeDraw>,
+    x: number,
+    y: number,
+  ) =>
+    distance.distanceToElement(
+      element,
+      arrayToMap([element]),
+      pointFrom<GlobalPoint>(x, y),
+    );
+
+  it("treats a point on the centerline as a direct hit for both modes", () => {
+    expect(distanceAt(createFreeDraw("variable"), 50, 0)).toBe(0);
+    expect(distanceAt(createFreeDraw("constant"), 50, 0)).toBe(0);
+  });
+
+  it("hits across the body of a thick stroke, not just near the centerline", () => {
+    expect(distanceAt(createFreeDraw("variable"), 50, 20)).toBe(0);
+  });
+
+  it("uses a wider hit area for variable-width than for constant-width strokes", () => {
+    const offset = 20;
+    const variableDistance = distanceAt(createFreeDraw("variable"), 50, offset);
+    const constantDistance = distanceAt(createFreeDraw("constant"), 50, offset);
+
+    expect(variableDistance).toBe(0);
+    expect(constantDistance).toBeGreaterThan(0);
+    expect(variableDistance).toBeLessThan(constantDistance);
+  });
+
+  it("does not hit points clearly outside even the widest stroke", () => {
+    expect(distanceAt(createFreeDraw("variable"), 50, 45)).toBeGreaterThan(0);
+  });
+});

packages/excalidraw/actions/actionProperties.tsx

@@ -12,6 +12,7 @@ import {
   DEFAULT_FONT_SIZE,
   FONT_FAMILY,
   ROUNDNESS,
+  STROKE_WIDTH_KEYS,
   VERTICAL_ALIGN,
   KEYS,
   randomInteger,
@@ -24,7 +25,6 @@ import {
   invariant,
   FONT_SIZES,
   type StrokeWidthKey,
-  getStrokeWidthKeyForElement,
 } from "@excalidraw/common";
 
 import { canBecomePolygon, getNonDeletedElements } from "@excalidraw/element";
@@ -554,6 +554,23 @@ export const actionChangeFillStyle = register<ExcalidrawElement["fillStyle"]>({
   },
 });
 
+const getStrokeWidthKeyForElement = (
+  element: ExcalidrawElement,
+): StrokeWidthKey | null => {
+  return (
+    STROKE_WIDTH_KEYS.find(
+      (key) => getStrokeWidthByKey(element.type, key) === element.strokeWidth,
+    ) ?? null
+  );
+};
+
+const getStrokeWidthForElement = (
+  element: ExcalidrawElement,
+  strokeWidthKey: StrokeWidthKey,
+): ExcalidrawElement["strokeWidth"] => {
+  return getStrokeWidthByKey(element.type, strokeWidthKey);
+};
+
 export const actionChangeStrokeWidth = register<StrokeWidthKey>({
   name: "changeStrokeWidth",
   label: "labels.strokeWidth",
@@ -564,7 +581,7 @@ export const actionChangeStrokeWidth = register<StrokeWidthKey>({
     return {
       elements: changeProperty(elements, appState, (el) =>
         newElementWith(el, {
-          strokeWidth: getStrokeWidthByKey(el.type, value),
+          strokeWidth: getStrokeWidthForElement(el, value),
         }),
       ),
       appState: { ...appState, currentItemStrokeWidthKey: value },

packages/excalidraw/data/restore.ts

@@ -19,8 +19,9 @@ import {
   getSizeFromPoints,
   normalizeLink,
   getLineHeight,
-  STROKE_WIDTH_KEYS,
   STROKE_WIDTH,
+  STROKE_WIDTH_KEYS,
+  type StrokeWidthKey,
 } from "@excalidraw/common";
 import {
   calculateFixedPointForNonElbowArrowBinding,
@@ -57,8 +58,6 @@ import { getNormalizedDimensions } from "@excalidraw/element";
 
 import { isInvisiblySmallElement } from "@excalidraw/element";
 
-import type { StrokeWidthKey } from "@excalidraw/common";
-
 import type { LocalPoint, Radians } from "@excalidraw/math";
 
 import type {