fix: Duplication

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

packages/element/src/bounds.ts

@@ -25,7 +25,6 @@ import type {
   Radians,
 } from "@excalidraw/math";
 import type { AppState } from "@excalidraw/excalidraw/types";
-import type { Mutable } from "@excalidraw/common/utility-types";
 
 import { generateRoughOptions } from "./shape";
 import { ShapeCache } from "./shape";
@@ -47,11 +46,10 @@ import {
   deconstructRectanguloidElement,
 } from "./utils";
 import { intersectElementWithLineSegment } from "./collision";
-import { getSplitPointGroups, getSplitPoints } from "./splitPoints";
+import { generateSplitCurves, getSplitPoints } from "./splitPoints";
 import { elementOverlapsWithFrame, getContainingFrame } from "./frame";
 
 import type { Drawable, Op } from "roughjs/bin/core";
-import type { Point as RoughPoint } from "roughjs/bin/geometry";
 import type {
   Arrowhead,
   ElementsMap,
@@ -909,7 +907,6 @@ export const getArrowheadPoints = (
   return [tx, ty, x3, y3, x4, y4];
 };
 
-// TODO reuse shape.ts
 const generateLinearElementShape = (
   element: ExcalidrawLinearElement,
 ): Drawable => {
@@ -927,33 +924,20 @@ const generateLinearElementShape = (
   })();
 
   if (method === "curve") {
-    const splitPoints = getSplitPoints(element);
-
-    if (splitPoints.length) {
-      // a split arrow is several curves; merge their ops into a single
-      // drawable so bounds are computed over the whole shape
-      const drawables = getSplitPointGroups(element.points, splitPoints).map(
-        (group) =>
-          generator.curve(
-            group as Mutable<LocalPoint>[] as RoughPoint[],
-            options,
-          ),
-      );
-
-      return {
-        ...drawables[0],
-        sets: drawables[0].sets.map((set, setIdx) => ({
-          ...set,
-          ops: drawables.flatMap(
-            (drawable) => drawable.sets[setIdx]?.ops ?? [],
-          ),
-        })),
-      };
-    }
+    // unsplit arrows are a single curve; split arrows are several curves
+    // with their ops merged into one drawable
+    return generateSplitCurves(
+      generator,
+      element.points,
+      getSplitPoints(element),
+      options,
+    );
   }
 
   return generator[method](
-    element.points as Mutable<LocalPoint>[] as RoughPoint[],
+    // SAFETY: LocalPoint pairs are readonly finite [x, y] numbers, exactly
+    // the shape rough.js consumes; the cast only drops readonly
+    element.points as unknown as [number, number][],
     options,
   );
 };
@@ -1131,9 +1115,7 @@ export const getElementPointsCoords = (
   const curves =
     element.roundness == null
       ? [gen.linearPath(points as [number, number][], options)]
-      : getSplitPointGroups(points, splitPoints).map((group) =>
-          gen.curve(group as [number, number][], options),
-        );
+      : [generateSplitCurves(gen, points, splitPoints, options)];
   const ops = curves.flatMap((curve) => getCurvePathOps(curve));
   const [minX, minY, maxX, maxY] = getMinMaxXYFromCurvePathOps(ops);
   return [

packages/element/src/shape.ts

@@ -55,7 +55,11 @@ import {
   isLinearElement,
 } from "./typeChecks";
 import { getCornerRadius, isPathALoop } from "./utils";
-import { getSplitPointGroups, getSplitPoints } from "./splitPoints";
+import {
+  generateSplitCurves,
+  getSplitPointGroups,
+  getSplitPoints,
+} from "./splitPoints";
 import { headingForPointIsHorizontal } from "./heading";
 
 import { canChangeRoundness } from "./comparisons";
@@ -584,49 +588,13 @@ const getArrowheadShapes = (
   }
 };
 
-const mergeDrawables = (drawables: Drawable[]): Drawable => {
-  if (drawables.length === 1) {
-    return drawables[0];
-  }
-
-  const [first] = drawables;
-
-  return {
-    ...first,
-    sets: first.sets.map((set, setIdx) => ({
-      ...set,
-      ops: drawables.flatMap((drawable) => drawable.sets[setIdx]?.ops ?? []),
-    })),
-  };
-};
-
 const generateSplittableCurve = (
   generator: RoughGenerator,
   element: ExcalidrawElement,
   points: readonly LocalPoint[],
   options: Options,
-): Drawable => {
-  const splitPoints = getSplitPoints(element);
-
-  if (!splitPoints.length) {
-    return generator.curve(
-      // SAFETY: LocalPoint pairs are readonly finite [x, y] numbers, exactly
-      // the shape roughjs consumes as RoughPoint; the cast only drops readonly
-      points as unknown as RoughPoint[],
-      options,
-    );
-  }
-
-  return mergeDrawables(
-    getSplitPointGroups(points, splitPoints).map((group) =>
-      generator.curve(
-        // SAFETY: LocalPoint -> RoughPoint; the cast only drops readonly
-        group as unknown as RoughPoint[],
-        options,
-      ),
-    ),
-  );
-};
+): Drawable =>
+  generateSplitCurves(generator, points, getSplitPoints(element), options);
 
 export const generateLinearCollisionShape = (
   element: ExcalidrawLinearElement | ExcalidrawFreeDrawElement,

packages/element/src/splitPoints.ts

@@ -1,5 +1,8 @@
 import { isArrowElement, isElbowArrow } from "./typeChecks";
 
+import type { Drawable, Options } from "roughjs/bin/core";
+import type { Point as RoughPoint } from "roughjs/bin/geometry";
+import type { RoughGenerator } from "roughjs/bin/generator";
 import type { ExcalidrawArrowElement, ExcalidrawElement } from "./types";
 
 /**
@@ -170,3 +173,36 @@ export const getSplitPointGroups = <P>(
 
   return groups;
 };
+
+/**
+ * Generates one rough.js curve per split group and merges their ops into a
+ * single drawable, so a split arrow is treated as one continuous shape for
+ * both rendering and bounds computation.
+ */
+export const generateSplitCurves = <P extends readonly [number, number]>(
+  generator: RoughGenerator,
+  points: readonly P[],
+  splitPoints: readonly number[],
+  options: Options,
+): Drawable => {
+  const drawables = getSplitPointGroups(points, splitPoints).map((group) =>
+    generator.curve(
+      // SAFETY: point pairs are finite [x, y] numbers, exactly the shape
+      // rough.js consumes; the cast only drops readonly
+      group as unknown as RoughPoint[],
+      options,
+    ),
+  );
+
+  if (drawables.length === 1) {
+    return drawables[0];
+  }
+
+  return {
+    ...drawables[0],
+    sets: drawables[0].sets.map((set, setIdx) => ({
+      ...set,
+      ops: drawables.flatMap((drawable) => drawable.sets[setIdx]?.ops ?? []),
+    })),
+  };
+};