From 8b248eda94a7f31e148c6e36157d3fbef2f4a5d5 Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Sun, 18 May 2025 10:02:06 +0200 Subject: [PATCH] Refactor eraser and lasso hit tests Signed-off-by: Mark Tolmacs --- packages/element/src/collision.ts | 12 ++++---- packages/excalidraw/components/App.tsx | 4 +-- packages/excalidraw/eraser/index.ts | 42 +++++++++----------------- packages/excalidraw/lasso/index.ts | 1 + packages/excalidraw/lasso/utils.ts | 41 +++++++++---------------- packages/math/src/index.ts | 1 + 6 files changed, 40 insertions(+), 61 deletions(-) diff --git a/packages/element/src/collision.ts b/packages/element/src/collision.ts index fd6f63c85..5fbdb1091 100644 --- a/packages/element/src/collision.ts +++ b/packages/element/src/collision.ts @@ -111,9 +111,9 @@ export const hitElementItself = ({ ? shouldTestInside(element) ? // Since `inShape` tests STRICTLY againt the insides of a shape // we would need `onShape` as well to include the "borders" - isPointInShape(point, element) || - isPointOnShape(point, element, threshold) - : isPointOnShape(point, element, threshold) + isPointInElement(point, element) || + isPointOnElementOutline(point, element, threshold) + : isPointOnElementOutline(point, element, threshold) : false; // hit test against a frame's name @@ -177,7 +177,7 @@ export const hitElementBoundText = ( } : boundTextElementCandidate; - return isPointInShape(point, boundTextElement); + return isPointInElement(point, boundTextElement); }; /** @@ -371,14 +371,14 @@ const intersectEllipseWithLineSegment = ( }; // check if the given point is considered on the given shape's border -const isPointOnShape = ( +const isPointOnElementOutline = ( 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 = ( +export const isPointInElement = ( point: GlobalPoint, element: ExcalidrawElement, ) => { diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 3ef3ce84a..ee018d463 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -130,7 +130,7 @@ import { refreshTextDimensions, deepCopyElement, duplicateElements, - isPointInShape, + isPointInElement, hasBoundTextElement, isArrowElement, isBindingElement, @@ -5164,7 +5164,7 @@ class App extends React.Component { ) { // if hitting the bounding box, return early // but if not, we should check for other cases as well (e.g. frame name) - if (isPointInShape(pointFrom(x, y), element)) { + if (isPointInElement(pointFrom(x, y), element)) { return true; } } diff --git a/packages/excalidraw/eraser/index.ts b/packages/excalidraw/eraser/index.ts index 7adc2668c..31974adc5 100644 --- a/packages/excalidraw/eraser/index.ts +++ b/packages/excalidraw/eraser/index.ts @@ -1,10 +1,10 @@ import { arrayToMap, easeOut, THEME } from "@excalidraw/common"; -import { getElementLineSegments, isPointInShape } from "@excalidraw/element"; import { - lineSegment, - lineSegmentIntersectionPoints, - pointFrom, -} from "@excalidraw/math"; + getBoundTextElement, + intersectElementWithLineSegment, + isPointInElement, +} from "@excalidraw/element"; +import { lineSegment, pointFrom } from "@excalidraw/math"; import { getElementsInGroup } from "@excalidraw/element"; @@ -12,7 +12,6 @@ import { shouldTestInside } from "@excalidraw/element"; import { hasBoundTextElement, isBoundToContainer } from "@excalidraw/element"; import { getBoundTextElementId } from "@excalidraw/element"; -import type { GeometricShape } from "@excalidraw/utils/shape"; import type { ElementsSegmentsMap, GlobalPoint, @@ -33,8 +32,6 @@ export class EraserTrail extends AnimatedTrail { private elementsToErase: Set = new Set(); private groupsToErase: Set = new Set(); private segmentsCache: Map[]> = new Map(); - private geometricShapesCache: Map> = - new Map(); constructor(animationFrameHandler: AnimationFrameHandler, app: App) { super(animationFrameHandler, app, { @@ -111,7 +108,6 @@ export class EraserTrail extends AnimatedTrail { pathSegments, element, this.segmentsCache, - this.geometricShapesCache, candidateElementsMap, this.app, ); @@ -149,7 +145,6 @@ export class EraserTrail extends AnimatedTrail { pathSegments, element, this.segmentsCache, - this.geometricShapesCache, candidateElementsMap, this.app, ); @@ -202,30 +197,23 @@ const eraserTest = ( pathSegments: LineSegment[], element: ExcalidrawElement, elementsSegments: ElementsSegmentsMap, - shapesCache: Map>, elementsMap: ElementsMap, app: App, ): boolean => { const lastPoint = pathSegments[pathSegments.length - 1][1]; - if (shouldTestInside(element) && isPointInShape(lastPoint, element)) { + if (shouldTestInside(element) && isPointInElement(lastPoint, element)) { return true; } - let elementSegments = elementsSegments.get(element.id); + const offset = app.getElementHitThreshold(); + const boundTextElement = getBoundTextElement(element, elementsMap); - if (!elementSegments) { - elementSegments = getElementLineSegments(element, elementsMap); - elementsSegments.set(element.id, elementSegments); - } - - return pathSegments.some((pathSegment) => - elementSegments?.some( - (elementSegment) => - lineSegmentIntersectionPoints( - pathSegment, - elementSegment, - app.getElementHitThreshold(), - ) !== null, - ), + return pathSegments.some( + (pathSegment) => + intersectElementWithLineSegment(element, pathSegment, offset).length > + 0 || + (boundTextElement && + intersectElementWithLineSegment(boundTextElement, pathSegment, offset) + .length > 0), ); }; diff --git a/packages/excalidraw/lasso/index.ts b/packages/excalidraw/lasso/index.ts index 163a8b7a9..86a9914d3 100644 --- a/packages/excalidraw/lasso/index.ts +++ b/packages/excalidraw/lasso/index.ts @@ -203,6 +203,7 @@ export class LassoTrail extends AnimatedTrail { intersectedElements: this.intersectedElements, enclosedElements: this.enclosedElements, simplifyDistance: 5 / this.app.state.zoom.value, + hitThreshold: this.app.getElementHitThreshold(), }); this.selectElementsFromIds(selectedElementIds); diff --git a/packages/excalidraw/lasso/utils.ts b/packages/excalidraw/lasso/utils.ts index d05f39998..a2e5d730c 100644 --- a/packages/excalidraw/lasso/utils.ts +++ b/packages/excalidraw/lasso/utils.ts @@ -3,15 +3,12 @@ import { simplify } from "points-on-curve"; import { polygonFromPoints, lineSegment, - lineSegmentIntersectionPoints, polygonIncludesPointNonZero, } from "@excalidraw/math"; -import type { - ElementsSegmentsMap, - GlobalPoint, - LineSegment, -} from "@excalidraw/math/types"; +import { intersectElementWithLineSegment } from "@excalidraw/element"; + +import type { ElementsSegmentsMap, GlobalPoint } from "@excalidraw/math/types"; import type { ExcalidrawElement } from "@excalidraw/element/types"; export const getLassoSelectedElementIds = (input: { @@ -21,6 +18,7 @@ export const getLassoSelectedElementIds = (input: { intersectedElements: Set; enclosedElements: Set; simplifyDistance?: number; + hitThreshold: number; }): { selectedElementIds: string[]; } => { @@ -31,6 +29,7 @@ export const getLassoSelectedElementIds = (input: { intersectedElements, enclosedElements, simplifyDistance, + hitThreshold, } = input; // simplify the path to reduce the number of points let path: GlobalPoint[] = lassoPath; @@ -48,7 +47,7 @@ export const getLassoSelectedElementIds = (input: { if (enclosed) { enclosedElements.add(element.id); } else { - const intersects = intersectionTest(path, element, elementsSegments); + const intersects = intersectionTest(path, element, hitThreshold); if (intersects) { intersectedElements.add(element.id); } @@ -84,26 +83,16 @@ const enclosureTest = ( const intersectionTest = ( lassoPath: GlobalPoint[], element: ExcalidrawElement, - elementsSegments: ElementsSegmentsMap, + hitThreshold: number, ): boolean => { - const elementSegments = elementsSegments.get(element.id); - if (!elementSegments) { - return false; - } + const lassoSegments = lassoPath + .slice(1) + .map((point: GlobalPoint, index) => lineSegment(lassoPath[index], point)) + .concat([lineSegment(lassoPath[lassoPath.length - 1], lassoPath[0])]); - const lassoSegments = lassoPath.reduce((acc, point, index) => { - if (index === 0) { - return acc; - } - acc.push(lineSegment(lassoPath[index - 1], point)); - return acc; - }, [] as LineSegment[]); - - return lassoSegments.some((lassoSegment) => - elementSegments.some( - (elementSegment) => - // introduce a bit of tolerance to account for roughness and simplification of paths - lineSegmentIntersectionPoints(lassoSegment, elementSegment, 1) !== null, - ), + return lassoSegments.some( + (lassoSegment) => + intersectElementWithLineSegment(element, lassoSegment, hitThreshold) + .length > 0, ); }; diff --git a/packages/math/src/index.ts b/packages/math/src/index.ts index d00ab469d..e487ac333 100644 --- a/packages/math/src/index.ts +++ b/packages/math/src/index.ts @@ -1,5 +1,6 @@ export * from "./angle"; export * from "./curve"; +export * from "./ellipse"; export * from "./line"; export * from "./point"; export * from "./polygon";