From ed02ded750624222649f020c4b99347e3400aade Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Fri, 28 Aug 2026 10:34:32 +0200 Subject: [PATCH] fix: Duplication Signed-off-by: Mark Tolmacs --- packages/element/src/bounds.ts | 44 ++++++++------------------- packages/element/src/shape.ts | 46 +++++------------------------ packages/element/src/splitPoints.ts | 36 ++++++++++++++++++++++ 3 files changed, 56 insertions(+), 70 deletions(-) diff --git a/packages/element/src/bounds.ts b/packages/element/src/bounds.ts index dab1dde17c..e2af703a2b 100644 --- a/packages/element/src/bounds.ts +++ b/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[] 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[] 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 [ diff --git a/packages/element/src/shape.ts b/packages/element/src/shape.ts index 30fb203d11..4f7f20d143 100644 --- a/packages/element/src/shape.ts +++ b/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, diff --git a/packages/element/src/splitPoints.ts b/packages/element/src/splitPoints.ts index 2597bf91b0..05f7c58570 100644 --- a/packages/element/src/splitPoints.ts +++ b/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 =

( 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 =

( + 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 ?? []), + })), + }; +};