From d08414c2a98af5a994b97439cb6420197e1eeea7 Mon Sep 17 00:00:00 2001 From: Mark Tolmacs Date: Fri, 9 May 2025 21:25:35 +0200 Subject: [PATCH] Inverted polygon now works just as well for hit testing Signed-off-by: Mark Tolmacs --- packages/element/src/Shape.ts | 104 +++++++++++++++++++++---- packages/element/src/collision.ts | 49 +++++++++++- packages/excalidraw/components/App.tsx | 2 +- packages/excalidraw/eraser/index.ts | 3 +- packages/utils/src/collision.ts | 85 -------------------- packages/utils/tests/collision.test.ts | 90 --------------------- 6 files changed, 136 insertions(+), 197 deletions(-) delete mode 100644 packages/utils/src/collision.ts delete mode 100644 packages/utils/tests/collision.test.ts diff --git a/packages/element/src/Shape.ts b/packages/element/src/Shape.ts index 2d0235bd3..5164ae34a 100644 --- a/packages/element/src/Shape.ts +++ b/packages/element/src/Shape.ts @@ -1,10 +1,17 @@ import { simplify } from "points-on-curve"; -import { pointFrom, pointDistance, type LocalPoint } from "@excalidraw/math"; +import { + pointFrom, + pointDistance, + type LocalPoint, + pointRotateRads, +} from "@excalidraw/math"; import { ROUGHNESS, isTransparent, assertNever } from "@excalidraw/common"; import { RoughGenerator } from "roughjs/bin/generator"; +import type { GlobalPoint, Radians } from "@excalidraw/math"; + import type { Mutable } from "@excalidraw/common/utility-types"; import type { EmbedsValidationStatus } from "@excalidraw/excalidraw/types"; @@ -22,7 +29,11 @@ import { headingForPointIsHorizontal } from "./heading"; import { canChangeRoundness } from "./comparisons"; import { generateFreeDrawShape } from "./renderElement"; -import { getArrowheadPoints, getDiamondPoints } from "./bounds"; +import { + getArrowheadPoints, + getDiamondPoints, + getElementBounds, +} from "./bounds"; import type { ExcalidrawElement, @@ -320,33 +331,92 @@ export const generateLinearCollisionShape = ( switch (element.type) { case "line": case "arrow": { - let shape: any; - // points array can be empty in the beginning, so it is important to add // initial position to it const points = element.points.length ? element.points : [pointFrom(0, 0)]; + const [x1, y1, x2, y2] = getElementBounds( + { + ...element, + angle: 0 as Radians, + }, + new Map(), + ); + const center = pointFrom((x1 + x2) / 2, (y1 + y2) / 2); if (isElbowArrow(element)) { - shape = generator.path(generateElbowArrowShape(points, 16), options) + return generator.path(generateElbowArrowShape(points, 16), options) .sets[0].ops; } else if (!element.roundness) { - shape = points.map((point, idx) => { - return idx === 0 - ? { op: "move", data: point } - : { - op: "lineTo", - data: [point[0], point[1]], - }; + return points.map((point, idx) => { + const p = pointRotateRads( + pointFrom(element.x + point[0], element.y + point[1]), + center, + element.angle, + ); + + return { + op: idx === 0 ? "move" : "lineTo", + data: pointFrom(p[0] - element.x, p[1] - element.y), + }; }); - } else { - shape = generator - .curve(points as unknown as RoughPoint[], options) - .sets[0].ops.slice(0, element.points.length); } - return shape; + return generator + .curve(points as unknown as RoughPoint[], options) + .sets[0].ops.slice(0, element.points.length) + .map((op, i, arr) => { + if (i === 0) { + const p = pointRotateRads( + pointFrom( + element.x + op.data[0], + element.y + op.data[1], + ), + center, + element.angle, + ); + + return { + op: "move", + data: pointFrom(p[0] - element.x, p[1] - element.y), + }; + } + + return { + op: "bcurveTo", + data: [ + pointRotateRads( + pointFrom( + element.x + op.data[0], + element.y + op.data[1], + ), + center, + element.angle, + ), + pointRotateRads( + pointFrom( + element.x + op.data[2], + element.y + op.data[3], + ), + center, + element.angle, + ), + pointRotateRads( + pointFrom( + element.x + op.data[4], + element.y + op.data[5], + ), + center, + element.angle, + ), + ] + .map((p) => + pointFrom(p[0] - element.x, p[1] - element.y), + ) + .flat(), + }; + }); } case "freedraw": { const simplifiedPoints = simplify( diff --git a/packages/element/src/collision.ts b/packages/element/src/collision.ts index 66df3ac3e..fd6f63c85 100644 --- a/packages/element/src/collision.ts +++ b/packages/element/src/collision.ts @@ -11,8 +11,12 @@ import { lineSegment, lineSegmentIntersectionPoints, pointFrom, + pointFromVector, pointRotateRads, pointsEqual, + vectorFromPoint, + vectorNormalize, + vectorScale, } from "@excalidraw/math"; import { @@ -20,8 +24,6 @@ import { ellipseSegmentInterceptPoints, } from "@excalidraw/math/ellipse"; -import { isPointInShape, isPointOnShape } from "@excalidraw/utils/collision"; - import type { Curve, GlobalPoint, @@ -35,6 +37,7 @@ import { isPathALoop } from "./shapes"; import { getElementBounds } from "./bounds"; import { hasBoundTextElement, + isFreeDrawElement, isIframeLikeElement, isImageElement, isLinearElement, @@ -50,6 +53,8 @@ import { getBoundTextElement } from "./textElement"; import { LinearElementEditor } from "./linearElementEditor"; +import { distanceToElement } from "./distance"; + import type { ElementsMap, ExcalidrawDiamondElement, @@ -219,11 +224,13 @@ const intersectLinearOrFreeDrawWithLineSegment = ( for (const shape of shapes) { switch (true) { case isCurve(shape): + //debugDrawCubicBezier(shape); intersections.push( ...curveIntersectLineSegment(shape as Curve, segment), ); continue; case isLineSegment(shape): + //debugDrawLine(shape); const point = lineSegmentIntersectionPoints( segment, shape as LineSegment, @@ -362,3 +369,41 @@ const intersectEllipseWithLineSegment = ( lineSegment(rotatedA, rotatedB), ).map((p) => pointRotateRads(p, center, element.angle)); }; + +// check if the given point is considered on the given shape's border +const isPointOnShape = ( + 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 = ( + point: GlobalPoint, + element: ExcalidrawElement, +) => { + if ( + (isLinearElement(element) || isFreeDrawElement(element)) && + !isPathALoop(element.points) + ) { + // There isn't any "inside" for a non-looping path + return false; + } + + const [x1, y1, x2, y2] = getElementBounds(element, new Map()); + const center = pointFrom((x1 + x2) / 2, (y1 + y2) / 2); + const otherPoint = pointFromVector( + vectorScale( + vectorNormalize(vectorFromPoint(point, center, 0.1)), + Math.max(element.width, element.height) * 2, + ), + center, + ); + const intersector = lineSegment(point, otherPoint); + const intersections = intersectElementWithLineSegment( + element, + intersector, + ).filter((item, pos, arr) => arr.indexOf(item) === pos); + + return intersections.length % 2 === 1; +}; diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index b8fc4cb65..2766f36b8 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -17,7 +17,7 @@ import { vectorDot, vectorNormalize, } from "@excalidraw/math"; -import { isPointInShape } from "@excalidraw/utils/collision"; +import { isPointInShape } from "@excalidraw/element/collision"; import { COLOR_PALETTE, diff --git a/packages/excalidraw/eraser/index.ts b/packages/excalidraw/eraser/index.ts index 7b822f0a9..7adc2668c 100644 --- a/packages/excalidraw/eraser/index.ts +++ b/packages/excalidraw/eraser/index.ts @@ -1,5 +1,5 @@ import { arrayToMap, easeOut, THEME } from "@excalidraw/common"; -import { getElementLineSegments } from "@excalidraw/element"; +import { getElementLineSegments, isPointInShape } from "@excalidraw/element"; import { lineSegment, lineSegmentIntersectionPoints, @@ -9,7 +9,6 @@ import { import { getElementsInGroup } from "@excalidraw/element"; import { shouldTestInside } from "@excalidraw/element"; -import { isPointInShape } from "@excalidraw/utils/collision"; import { hasBoundTextElement, isBoundToContainer } from "@excalidraw/element"; import { getBoundTextElementId } from "@excalidraw/element"; diff --git a/packages/utils/src/collision.ts b/packages/utils/src/collision.ts deleted file mode 100644 index 55a60b74c..000000000 --- a/packages/utils/src/collision.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { - lineSegment, - pointFrom, - type GlobalPoint, - vectorFromPoint, - vectorNormalize, - vectorScale, - pointFromVector, -} from "@excalidraw/math"; - -import { intersectElementWithLineSegment } from "@excalidraw/element/collision"; - -import { elementCenterPoint } from "@excalidraw/common"; - -import { distanceToElement } from "@excalidraw/element/distance"; - -import { getCommonBounds, isLinearElement } from "@excalidraw/excalidraw"; -import { isFreeDrawElement } from "@excalidraw/element/typeChecks"; -import { isPathALoop } from "@excalidraw/element/shapes"; - -import { - debugDrawLine, - debugDrawPoint, -} from "@excalidraw/excalidraw/visualdebug"; - -import type { ExcalidrawElement } from "@excalidraw/element/types"; - -// check if the given point is considered on the given shape's border -export const isPointOnShape = ( - point: GlobalPoint, - element: ExcalidrawElement, - tolerance = 1, -) => { - const distance = distanceToElement(element, point); - - return distance <= tolerance; -}; - -// check if the given point is considered inside the element's border -export const isPointInShape = ( - point: GlobalPoint, - element: ExcalidrawElement, -) => { - if (isLinearElement(element) || isFreeDrawElement(element)) { - if (isPathALoop(element.points)) { - const [minX, minY, maxX, maxY] = getCommonBounds([element]); - const center = pointFrom( - (maxX + minX) / 2, - (maxY + minY) / 2, - ); - const otherPoint = pointFromVector( - vectorScale( - vectorNormalize(vectorFromPoint(point, center, 0.1)), - Math.max(element.width, element.height) * 2, - ), - center, - ); - const intersector = lineSegment(point, otherPoint); - - // What about being on the center exactly? - const intersections = intersectElementWithLineSegment( - element, - intersector, - ); - - const hit = intersections.length % 2 === 1; - - debugDrawLine(intersector, { color: hit ? "green" : "red" }); - debugDrawPoint(point, { color: "black" }); - debugDrawPoint(otherPoint, { color: "blue" }); - - return hit; - } - - // There isn't any "inside" for a non-looping path - return false; - } - - const intersections = intersectElementWithLineSegment( - element, - lineSegment(elementCenterPoint(element), point), - ); - - return intersections.length === 0; -}; diff --git a/packages/utils/tests/collision.test.ts b/packages/utils/tests/collision.test.ts deleted file mode 100644 index 35bc28b34..000000000 --- a/packages/utils/tests/collision.test.ts +++ /dev/null @@ -1,90 +0,0 @@ -import { - curve, - degreesToRadians, - lineSegment, - lineSegmentRotate, - pointFrom, - pointRotateDegs, -} from "@excalidraw/math"; - -import type { Curve, Degrees, GlobalPoint } from "@excalidraw/math"; - -import { pointOnCurve, pointOnPolyline } from "../src/collision"; - -import type { Polyline } from "../src/shape"; - -describe("point and curve", () => { - const c: Curve = curve( - pointFrom(1.4, 1.65), - pointFrom(1.9, 7.9), - pointFrom(5.9, 1.65), - pointFrom(6.44, 4.84), - ); - - it("point on curve", () => { - expect(pointOnCurve(c[0], c, 10e-5)).toBe(true); - expect(pointOnCurve(c[3], c, 10e-5)).toBe(true); - - expect(pointOnCurve(pointFrom(2, 4), c, 0.1)).toBe(true); - expect(pointOnCurve(pointFrom(4, 4.4), c, 0.1)).toBe(true); - expect(pointOnCurve(pointFrom(5.6, 3.85), c, 0.1)).toBe(true); - - expect(pointOnCurve(pointFrom(5.6, 4), c, 0.1)).toBe(false); - expect(pointOnCurve(c[1], c, 0.1)).toBe(false); - expect(pointOnCurve(c[2], c, 0.1)).toBe(false); - }); -}); - -describe("point and polylines", () => { - const polyline: Polyline = [ - lineSegment(pointFrom(1, 0), pointFrom(1, 2)), - lineSegment(pointFrom(1, 2), pointFrom(2, 2)), - lineSegment(pointFrom(2, 2), pointFrom(2, 1)), - lineSegment(pointFrom(2, 1), pointFrom(3, 1)), - ]; - - it("point on the line", () => { - expect(pointOnPolyline(pointFrom(1, 0), polyline)).toBe(true); - expect(pointOnPolyline(pointFrom(1, 2), polyline)).toBe(true); - expect(pointOnPolyline(pointFrom(2, 2), polyline)).toBe(true); - expect(pointOnPolyline(pointFrom(2, 1), polyline)).toBe(true); - expect(pointOnPolyline(pointFrom(3, 1), polyline)).toBe(true); - - expect(pointOnPolyline(pointFrom(1, 1), polyline)).toBe(true); - expect(pointOnPolyline(pointFrom(2, 1.5), polyline)).toBe(true); - expect(pointOnPolyline(pointFrom(2.5, 1), polyline)).toBe(true); - - expect(pointOnPolyline(pointFrom(0, 1), polyline)).toBe(false); - expect(pointOnPolyline(pointFrom(2.1, 1.5), polyline)).toBe(false); - }); - - it("point on the line with rotation", () => { - const truePoints = [ - pointFrom(1, 0), - pointFrom(1, 2), - pointFrom(2, 2), - pointFrom(2, 1), - pointFrom(3, 1), - ]; - - truePoints.forEach((p) => { - const rotation = (Math.random() * 360) as Degrees; - const rotatedPoint = pointRotateDegs(p, pointFrom(0, 0), rotation); - const rotatedPolyline = polyline.map((line) => - lineSegmentRotate(line, degreesToRadians(rotation), pointFrom(0, 0)), - ); - expect(pointOnPolyline(rotatedPoint, rotatedPolyline)).toBe(true); - }); - - const falsePoints = [pointFrom(0, 1), pointFrom(2.1, 1.5)]; - - falsePoints.forEach((p) => { - const rotation = (Math.random() * 360) as Degrees; - const rotatedPoint = pointRotateDegs(p, pointFrom(0, 0), rotation); - const rotatedPolyline = polyline.map((line) => - lineSegmentRotate(line, degreesToRadians(rotation), pointFrom(0, 0)), - ); - expect(pointOnPolyline(rotatedPoint, rotatedPolyline)).toBe(false); - }); - }); -});