From 1161f1b8ba860a34602fc4aa211ea9f5b28c30eb Mon Sep 17 00:00:00 2001 From: Christopher Tangonan <161169629+cTangonan123@users.noreply.github.com> Date: Sun, 14 Sep 2025 04:33:43 -0700 Subject: [PATCH] fix: eraser can handle dots without regressing prior performance improvements (#9946) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Márk Tolmács --- packages/element/src/renderElement.ts | 74 +++++++++++++++++++++- packages/excalidraw/eraser/index.ts | 90 ++++++++++++++++++++++++++- packages/math/src/segment.ts | 16 +++++ 3 files changed, 176 insertions(+), 4 deletions(-) diff --git a/packages/element/src/renderElement.ts b/packages/element/src/renderElement.ts index 008d6afc4..8c17863ee 100644 --- a/packages/element/src/renderElement.ts +++ b/packages/element/src/renderElement.ts @@ -1,7 +1,14 @@ import rough from "roughjs/bin/rough"; import { getStroke } from "perfect-freehand"; -import { isRightAngleRads } from "@excalidraw/math"; +import { + type GlobalPoint, + isRightAngleRads, + lineSegment, + pointFrom, + pointRotateRads, + type Radians, +} from "@excalidraw/math"; import { BOUND_TEXT_PADDING, @@ -14,6 +21,7 @@ import { getFontString, isRTL, getVerticalOffset, + invariant, } from "@excalidraw/common"; import type { @@ -32,7 +40,7 @@ import type { InteractiveCanvasRenderConfig, } from "@excalidraw/excalidraw/scene/types"; -import { getElementAbsoluteCoords } from "./bounds"; +import { getElementAbsoluteCoords, getElementBounds } from "./bounds"; import { getUncroppedImageElement } from "./cropElement"; import { LinearElementEditor } from "./linearElementEditor"; import { @@ -1039,6 +1047,66 @@ export function getFreeDrawPath2D(element: ExcalidrawFreeDrawElement) { } export function getFreeDrawSvgPath(element: ExcalidrawFreeDrawElement) { + return getSvgPathFromStroke(getFreedrawOutlinePoints(element)); +} + +export function getFreedrawOutlineAsSegments( + element: ExcalidrawFreeDrawElement, + points: [number, number][], + elementsMap: ElementsMap, +) { + const bounds = getElementBounds( + { + ...element, + angle: 0 as Radians, + }, + elementsMap, + ); + const center = pointFrom( + (bounds[0] + bounds[2]) / 2, + (bounds[1] + bounds[3]) / 2, + ); + + invariant(points.length >= 2, "Freepath outline must have at least 2 points"); + + return points.slice(2).reduce( + (acc, curr) => { + acc.push( + lineSegment( + acc[acc.length - 1][1], + pointRotateRads( + pointFrom(curr[0] + element.x, curr[1] + element.y), + center, + element.angle, + ), + ), + ); + return acc; + }, + [ + lineSegment( + pointRotateRads( + pointFrom( + points[0][0] + element.x, + points[0][1] + element.y, + ), + center, + element.angle, + ), + pointRotateRads( + pointFrom( + points[1][0] + element.x, + points[1][1] + element.y, + ), + center, + element.angle, + ), + ), + ], + ); +} + +export function getFreedrawOutlinePoints(element: ExcalidrawFreeDrawElement) { // If input points are empty (should they ever be?) return a dot const inputPoints = element.simulatePressure ? element.points @@ -1057,7 +1125,7 @@ export function getFreeDrawSvgPath(element: ExcalidrawFreeDrawElement) { last: !!element.lastCommittedPoint, // LastCommittedPoint is added on pointerup }; - return getSvgPathFromStroke(getStroke(inputPoints as number[][], options)); + return getStroke(inputPoints as number[][], options) as [number, number][]; } function med(A: number[], B: number[]) { diff --git a/packages/excalidraw/eraser/index.ts b/packages/excalidraw/eraser/index.ts index 6fbe2bd4c..567d0a396 100644 --- a/packages/excalidraw/eraser/index.ts +++ b/packages/excalidraw/eraser/index.ts @@ -1,11 +1,26 @@ import { arrayToMap, easeOut, THEME } from "@excalidraw/common"; + import { computeBoundTextPosition, + distanceToElement, + doBoundsIntersect, getBoundTextElement, + getElementBounds, + getFreedrawOutlineAsSegments, + getFreedrawOutlinePoints, intersectElementWithLineSegment, + isArrowElement, + isFreeDrawElement, + isLineElement, isPointInElement, } from "@excalidraw/element"; -import { lineSegment, pointFrom } from "@excalidraw/math"; +import { + lineSegment, + lineSegmentsDistance, + pointFrom, + polygon, + polygonIncludesPoint, +} from "@excalidraw/math"; import { getElementsInGroup } from "@excalidraw/element"; @@ -13,6 +28,8 @@ import { shouldTestInside } from "@excalidraw/element"; import { hasBoundTextElement, isBoundToContainer } from "@excalidraw/element"; import { getBoundTextElementId } from "@excalidraw/element"; +import type { Bounds } from "@excalidraw/element"; + import type { GlobalPoint, LineSegment } from "@excalidraw/math/types"; import type { ElementsMap, ExcalidrawElement } from "@excalidraw/element/types"; @@ -96,6 +113,7 @@ export class EraserTrail extends AnimatedTrail { pathSegment, element, candidateElementsMap, + this.app.state.zoom.value, ); if (intersects) { @@ -131,6 +149,7 @@ export class EraserTrail extends AnimatedTrail { pathSegment, element, candidateElementsMap, + this.app.state.zoom.value, ); if (intersects) { @@ -180,8 +199,33 @@ const eraserTest = ( pathSegment: LineSegment, element: ExcalidrawElement, elementsMap: ElementsMap, + zoom: number, ): boolean => { const lastPoint = pathSegment[1]; + + // PERF: Do a quick bounds intersection test first because it's cheap + const threshold = isFreeDrawElement(element) ? 15 : element.strokeWidth / 2; + const segmentBounds = [ + Math.min(pathSegment[0][0], pathSegment[1][0]) - threshold, + Math.min(pathSegment[0][1], pathSegment[1][1]) - threshold, + Math.max(pathSegment[0][0], pathSegment[1][0]) + threshold, + Math.max(pathSegment[0][1], pathSegment[1][1]) + threshold, + ] as Bounds; + const origElementBounds = getElementBounds(element, elementsMap); + const elementBounds: Bounds = [ + origElementBounds[0] - threshold, + origElementBounds[1] - threshold, + origElementBounds[2] + threshold, + origElementBounds[3] + threshold, + ]; + + if (!doBoundsIntersect(segmentBounds, elementBounds)) { + return false; + } + + // There are shapes where the inner area should trigger erasing + // even though the eraser path segment doesn't intersect with or + // get close to the shape's stroke if ( shouldTestInside(element) && isPointInElement(lastPoint, element, elementsMap) @@ -189,6 +233,50 @@ const eraserTest = ( return true; } + // Freedraw elements are tested for erasure by measuring the distance + // of the eraser path and the freedraw shape outline lines to a tolerance + // which offers a good visual precision at various zoom levels + if (isFreeDrawElement(element)) { + const outlinePoints = getFreedrawOutlinePoints(element); + const strokeSegments = getFreedrawOutlineAsSegments( + element, + outlinePoints, + elementsMap, + ); + const tolerance = Math.max(2.25, 5 / zoom); // NOTE: Visually fine-tuned approximation + + for (const seg of strokeSegments) { + if (lineSegmentsDistance(seg, pathSegment) <= tolerance) { + return true; + } + } + + const poly = polygon( + ...(outlinePoints.map(([x, y]) => + pointFrom(element.x + x, element.y + y), + ) as GlobalPoint[]), + ); + + // PERF: Check only one point of the eraser segment. If the eraser segment + // start is inside the closed freedraw shape, the other point is either also + // inside or the eraser segment will intersect the shape outline anyway + if (polygonIncludesPoint(pathSegment[0], poly)) { + return true; + } + + return false; + } else if ( + isArrowElement(element) || + (isLineElement(element) && !element.polygon) + ) { + const tolerance = Math.max( + element.strokeWidth, + (element.strokeWidth * 2) / zoom, + ); + + return distanceToElement(element, elementsMap, lastPoint) <= tolerance; + } + const boundTextElement = getBoundTextElement(element, elementsMap); return ( diff --git a/packages/math/src/segment.ts b/packages/math/src/segment.ts index dade79039..ae282bfee 100644 --- a/packages/math/src/segment.ts +++ b/packages/math/src/segment.ts @@ -177,3 +177,19 @@ export function lineSegmentIntersectionPoints< return candidate; } + +export function lineSegmentsDistance( + s1: LineSegment, + s2: LineSegment, +): number { + if (lineSegmentIntersectionPoints(s1, s2)) { + return 0; + } + + return Math.min( + distanceToLineSegment(s1[0], s2), + distanceToLineSegment(s1[1], s2), + distanceToLineSegment(s2[0], s1), + distanceToLineSegment(s2[1], s1), + ); +}