From 23959099e105ed2924d225668b0c9dac476b60af Mon Sep 17 00:00:00 2001 From: dwelle <5153846+dwelle@users.noreply.github.com> Date: Mon, 30 Mar 2026 12:27:35 +0200 Subject: [PATCH] fix rotated filled box-selection & refactor --- packages/element/src/bounds.ts | 13 +- packages/element/src/collision.ts | 19 ++- packages/element/src/selection.ts | 124 ++++++++++++------ packages/excalidraw/tests/selection.test.tsx | 131 ++++++++++++++++++- packages/math/src/types.ts | 6 + 5 files changed, 243 insertions(+), 50 deletions(-) diff --git a/packages/element/src/bounds.ts b/packages/element/src/bounds.ts index a615aea52d..ebecc7b5bb 100644 --- a/packages/element/src/bounds.ts +++ b/packages/element/src/bounds.ts @@ -27,6 +27,7 @@ import type { GlobalPoint, LineSegment, LocalPoint, + NonRotated, Radians, } from "@excalidraw/math"; @@ -996,12 +997,16 @@ const getLinearElementRotatedBounds = ( return coords; }; -export const getElementBounds = ( +export const getElementBounds = ( element: ExcalidrawElement, elementsMap: ElementsMap, - nonRotated: boolean = false, -): Bounds => { - return ElementBounds.getBounds(element, elementsMap, nonRotated); + nonRotated: T = false as T, +): T extends true ? NonRotated : Bounds => { + return ElementBounds.getBounds( + element, + elementsMap, + nonRotated, + ) as T extends true ? NonRotated : Bounds; }; export const getCommonBounds = ( diff --git a/packages/element/src/collision.ts b/packages/element/src/collision.ts index 4a5abf47d0..0d89f18cb6 100644 --- a/packages/element/src/collision.ts +++ b/packages/element/src/collision.ts @@ -22,6 +22,7 @@ import type { Curve, GlobalPoint, LineSegment, + NonRotated, Radians, } from "@excalidraw/math"; @@ -189,7 +190,17 @@ export const hitElementItself = ({ return result; }; -const getBoundsCorners = (bounds: Bounds, angle: Radians = 0 as Radians) => { +export function getBoundsCorners( + bounds: Bounds, +): readonly [GlobalPoint, GlobalPoint, GlobalPoint, GlobalPoint]; +export function getBoundsCorners( + bounds: NonRotated, + angle: Radians, +): readonly [GlobalPoint, GlobalPoint, GlobalPoint, GlobalPoint]; +export function getBoundsCorners( + bounds: Bounds, + angle: Radians = 0 as Radians, +) { const [x1, y1, x2, y2] = bounds; const center = getCenterForBounds(bounds); const corners = [ @@ -209,9 +220,9 @@ const getBoundsCorners = (bounds: Bounds, angle: Radians = 0 as Radians) => { GlobalPoint, GlobalPoint, ]; -}; +} -const getBoundsEdges = ( +export const getBoundsEdges = ( corners: readonly [GlobalPoint, GlobalPoint, GlobalPoint, GlobalPoint], ) => [ @@ -223,7 +234,7 @@ const getBoundsEdges = ( const isPointInRotatedBounds = ( point: GlobalPoint, - bounds: Bounds, + bounds: NonRotated, angle: Radians, tolerance = 0, ) => { diff --git a/packages/element/src/selection.ts b/packages/element/src/selection.ts index 0c11328581..7f634e4958 100644 --- a/packages/element/src/selection.ts +++ b/packages/element/src/selection.ts @@ -19,13 +19,17 @@ import { } from "./bounds"; import { doBoundsIntersectElementBoundingBox, + getBoundsCorners, + getBoundsEdges, intersectElementWithLineSegment, + isPointInElement, shouldTestInside, } from "./collision"; import { isElementInViewport } from "./sizeHelpers"; import { isBoundToContainer, isFrameLikeElement, + isFreeDrawElement, isLinearElement, isTextElement, } from "./typeChecks"; @@ -46,17 +50,15 @@ import type { NonDeletedExcalidrawElement, } from "./types"; -// Broad-phase only for overlap mode. This decides whether plain AABB overlap -// should be refined via the element's rotated local bounds. Elements that fail -// `shouldTestInside()` still go through the outline-specific path below, so -// this is intentionally not the whole overlap-selection policy. +// Broad-phase only for overlap mode. Rotated closed shapes should not select +// from the empty corners of their axis-aligned bounds. Linear elements and +// freedraw already rely on the outline-specific path below, so exclude them. const shouldUseRotatedOverlapBroadPhase = ( element: NonDeletedExcalidrawElement, ) => element.angle !== 0 && - (isTextElement(element) || - element.type === "freedraw" || - element.type === "image"); + !isLinearElement(element) && + !isFreeDrawElement(element); const clipLineSegmentToBounds = ( segment: LineSegment, @@ -112,6 +114,11 @@ const isPointWithinAabb = (point: GlobalPoint, bounds: Bounds) => point[1] >= bounds[1] && point[1] <= bounds[3]; +const shouldUsePreciseFilledOverlap = (element: NonDeletedExcalidrawElement) => + element.type === "ellipse" || + element.type === "diamond" || + (element.type === "rectangle" && !!element.roundness); + const shouldSkipElementFromSelection = (element: NonDeletedExcalidrawElement) => element.locked || element.type === "selection" || isBoundToContainer(element); @@ -149,38 +156,6 @@ const finalizeElementsInSelection = ( }); }; -const getSelectionEdges = ( - selectionBounds: Bounds, -): readonly LineSegment[] => { - const selectionTopLeft = pointFrom( - selectionBounds[0], - selectionBounds[1], - ); - const selectionBottomRight = pointFrom( - selectionBounds[2], - selectionBounds[3], - ); - - return [ - lineSegment( - selectionTopLeft, - pointFrom(selectionBounds[2], selectionBounds[1]), - ), - lineSegment( - pointFrom(selectionBounds[2], selectionBounds[1]), - selectionBottomRight, - ), - lineSegment( - selectionBottomRight, - pointFrom(selectionBounds[0], selectionBounds[3]), - ), - lineSegment( - pointFrom(selectionBounds[0], selectionBounds[3]), - selectionTopLeft, - ), - ]; -}; - const getVisibleElementOutlineSegments = ( element: NonDeletedExcalidrawElement, frameBounds: Bounds | null, @@ -220,6 +195,60 @@ const doesSelectionContainElementOutline = ( isPointWithinAabb(outlineSegment[1], selectionBounds), ); +const doesSelectionContainElementInterior = ( + element: NonDeletedExcalidrawElement, + frameBounds: Bounds | null, + selectionCorners: readonly GlobalPoint[], + elementsMap: ElementsMap, +) => + selectionCorners.some( + (selectionCorner) => + (!frameBounds || isPointWithinAabb(selectionCorner, frameBounds)) && + isPointInElement(selectionCorner, element, elementsMap), + ); + +const doesSelectionOverlapFilledElement = ( + element: NonDeletedExcalidrawElement, + frameBounds: Bounds | null, + selectionBounds: Bounds, + selectionCorners: readonly GlobalPoint[], + selectionEdges: readonly LineSegment[], + elementsMap: ElementsMap, +) => { + if ( + doesSelectionContainElementInterior( + element, + frameBounds, + selectionCorners, + elementsMap, + ) + ) { + return true; + } + + if ( + doesSelectionIntersectElementOutline( + element, + frameBounds, + selectionEdges, + elementsMap, + ) + ) { + return true; + } + + const outlineSegments = getVisibleElementOutlineSegments( + element, + frameBounds, + elementsMap, + ); + + return ( + outlineSegments.length > 0 && + doesSelectionContainElementOutline(outlineSegments, selectionBounds) + ); +}; + /** * Frames and their containing elements are not to be selected at the same time. * Given an array of selected elements, if there are frames and their containing elements @@ -306,7 +335,8 @@ export const getElementsWithinSelection = ( ); } - const selectionEdges = getSelectionEdges(selectionBounds); + const selectionCorners = getBoundsCorners(selectionBounds); + const selectionEdges = getBoundsEdges(selectionCorners); const elementsInSelection: NonDeletedExcalidrawElement[] = []; for (const element of elements) { @@ -353,7 +383,19 @@ export const getElementsWithinSelection = ( const shouldSelectFromInside = shouldTestInside(element); if (shouldSelectFromInside) { - if (isSelectionOverlappingElement) { + if ( + isSelectionOverlappingElement && + (!shouldUsePreciseFilledOverlap(element) || + isSelectionContainingElement || + doesSelectionOverlapFilledElement( + element, + frameBounds, + selectionBounds, + selectionCorners, + selectionEdges, + elementsMap, + )) + ) { elementsInSelection.push(element); } continue; diff --git a/packages/excalidraw/tests/selection.test.tsx b/packages/excalidraw/tests/selection.test.tsx index 449f6c13dc..a718fbd6e9 100644 --- a/packages/excalidraw/tests/selection.test.tsx +++ b/packages/excalidraw/tests/selection.test.tsx @@ -3,7 +3,7 @@ import { vi } from "vitest"; import { KEYS, ROUNDNESS, reseed } from "@excalidraw/common"; import { getElementBounds, getElementLineSegments } from "@excalidraw/element"; -import { pointFrom, type LocalPoint } from "@excalidraw/math"; +import { pointFrom, pointRotateRads, type LocalPoint } from "@excalidraw/math"; import { SHAPES } from "../components/shapes"; @@ -207,6 +207,42 @@ describe("box-selection overlap mode", () => { mouse.up(); }; + const boxSelectTopLeftAabbCorner = ( + element: ReturnType, + ) => { + const sceneElement = API.getElement(element); + const elementsMap = h.scene.getNonDeletedElementsMap(); + const [x1, y1] = getElementBounds(sceneElement, elementsMap); + + boxSelect(x1 + 2, y1 + 2, x1 + 12, y1 + 12); + }; + + const boxSelectTopRightAabbCorner = ( + element: ReturnType, + ) => { + const sceneElement = API.getElement(element); + const elementsMap = h.scene.getNonDeletedElementsMap(); + const [, y1, x2] = getElementBounds(sceneElement, elementsMap); + + boxSelect(x2 - 12, y1 + 2, x2 - 2, y1 + 12); + }; + + const boxSelectTopLeftRotatedLocalBoundsCorner = ( + element: ReturnType, + ) => { + const sceneElement = API.getElement(element); + const elementsMap = h.scene.getNonDeletedElementsMap(); + const [x1, y1, x2, y2] = getElementBounds(sceneElement, elementsMap, true); + const center = pointFrom((x1 + x2) / 2, (y1 + y2) / 2); + const [cornerX, cornerY] = pointRotateRads( + pointFrom(x1, y1), + center, + sceneElement.angle, + ); + + boxSelect(cornerX - 4, cornerY - 4, cornerX + 4, cornerY + 4); + }; + beforeEach(async () => { await render( { assertSelectedElements([rect.id]); }); + it("should not select a filled rotated rectangle when the selection box only overlaps its axis-aligned bounds", () => { + const rect = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 100, + height: 100, + angle: Math.PI / 4, + backgroundColor: "red", + fillStyle: "solid", + }); + + API.setElements([rect]); + + boxSelectTopLeftAabbCorner(rect); + + assertSelectedElements([]); + }); + + it("should not select a filled ellipse when the selection box only overlaps its bounds corner", () => { + const ellipse = API.createElement({ + type: "ellipse", + x: 0, + y: 0, + width: 100, + height: 100, + backgroundColor: "red", + fillStyle: "solid", + }); + + API.setElements([ellipse]); + + boxSelectTopRightAabbCorner(ellipse); + + assertSelectedElements([]); + }); + + it("should not select a filled diamond when the selection box only overlaps its bounds corner", () => { + const diamond = API.createElement({ + type: "diamond", + x: 0, + y: 0, + width: 100, + height: 100, + backgroundColor: "red", + fillStyle: "solid", + }); + + API.setElements([diamond]); + + boxSelectTopRightAabbCorner(diamond); + + assertSelectedElements([]); + }); + + it("should not select a filled rotated ellipse when the selection box only overlaps its axis-aligned bounds", () => { + const ellipse = API.createElement({ + type: "ellipse", + x: 0, + y: 0, + width: 100, + height: 100, + angle: Math.PI / 4, + backgroundColor: "red", + fillStyle: "solid", + }); + + API.setElements([ellipse]); + + boxSelectTopLeftRotatedLocalBoundsCorner(ellipse); + + assertSelectedElements([]); + }); + + it("should not select a filled rotated diamond when the selection box only overlaps its rotated local bounds", () => { + const diamond = API.createElement({ + type: "diamond", + x: 0, + y: 0, + width: 100, + height: 100, + angle: Math.PI / 4, + backgroundColor: "red", + fillStyle: "solid", + }); + + API.setElements([diamond]); + + boxSelectTopLeftRotatedLocalBoundsCorner(diamond); + + assertSelectedElements([]); + }); + it("should not select rotated text when the selection box only overlaps its axis-aligned bounds", () => { const text = API.createElement({ type: "text", diff --git a/packages/math/src/types.ts b/packages/math/src/types.ts index 9adb208291..9842901e68 100644 --- a/packages/math/src/types.ts +++ b/packages/math/src/types.ts @@ -23,6 +23,12 @@ export type Degrees = number & { _brand: "excalimath_degree" }; */ export type InclusiveRange = [number, number] & { _brand: "excalimath_degree" }; +/** + * Can be used for any type of point-likes that are non-rotated, such as + * non-AABB Bounds, or non-rotated Point. + */ +export type NonRotated = T & { _brand_nonrotated: "excalimath_nonrotated" }; + // // Point //