From 5ceb97e6b5e6a3b55a2c6c0e972990aa5e63e296 Mon Sep 17 00:00:00 2001 From: Ryan Di Date: Mon, 11 May 2026 17:13:18 +1000 Subject: [PATCH] refactor(snaps): prioritize nearby point references --- packages/excalidraw/snapping.ts | 206 ++++++++- packages/excalidraw/tests/snapping.test.ts | 509 +++++++++++++++++++++ 2 files changed, 700 insertions(+), 15 deletions(-) create mode 100644 packages/excalidraw/tests/snapping.test.ts diff --git a/packages/excalidraw/snapping.ts b/packages/excalidraw/snapping.ts index c5da70dff2..c598a5ee23 100644 --- a/packages/excalidraw/snapping.ts +++ b/packages/excalidraw/snapping.ts @@ -39,6 +39,10 @@ import type { } from "./types"; const SNAP_DISTANCE = 8; +// Point snaps can come from both nearby and distant references within snap +// distance. If their visual distances have a large break, prefer the nearest +// cluster before comparing offsets. +const SNAP_REFERENCE_CLUSTER_BREAK_DISTANCE = 200; // do not comput more gaps per axis than this limit // TODO increase or remove once we optimize @@ -49,6 +53,13 @@ export const getSnapDistance = (zoomValue: number) => { return SNAP_DISTANCE / zoomValue; }; +/** + * Keeps the cluster threshold visually stable across zoom levels. + */ +const getSnapReferenceClusterBreakDistance = (zoomValue: number) => { + return SNAP_REFERENCE_CLUSTER_BREAK_DISTANCE / zoomValue; +}; + type Vector2D = { x: number; y: number; @@ -84,6 +95,9 @@ export type PointSnap = { points: PointPair; pointTypes: [SnapPointType, SnapPointType]; sourceIds: [SnapPointSourceId, SnapPointSourceId]; + // Distance along the rendered snapline, used to group visually nearby + // references before choosing which point snaps should win. + visualDistance: number; offset: number; }; @@ -361,18 +375,37 @@ const getSnapPointSourceId = (elements: ExcalidrawElement[]) => { .join(","); }; +const getSelectedFrameIdsForSnapping = ( + selectedElements: readonly ExcalidrawElement[], +) => { + const selectedFrameIds = new Set(); + + for (const element of selectedElements) { + if (element.frameId) { + selectedFrameIds.add(element.frameId); + } + } + + return selectedFrameIds; +}; + const getReferenceElements = ( elements: readonly NonDeletedExcalidrawElement[], selectedElements: NonDeletedExcalidrawElement[], appState: AppState, elementsMap: ElementsMap, -) => - getVisibleAndNonSelectedElements( +) => { + const selectedFrameIds = getSelectedFrameIdsForSnapping(selectedElements); + + return getVisibleAndNonSelectedElements( elements, selectedElements, appState, elementsMap, - ); + ).filter((element) => { + return !element.frameId || selectedFrameIds.has(element.frameId); + }); +}; export const getVisibleGaps = ( elements: readonly NonDeletedExcalidrawElement[], @@ -662,6 +695,119 @@ const getGapSnaps = ( } }; +/** + * Narrows point snaps to the nearest visual cluster, then keeps the best offset + * inside that cluster. If all references look continuous, this preserves the + * old smallest-offset behavior. + */ +const filterPointSnapsToNearestCluster = ( + snaps: Snaps, + clusterBreakDistance: number, +) => { + const pointSnaps = snaps.filter( + (snap): snap is PointSnap => snap.type === "point", + ); + + if (pointSnaps.length < 2) { + return; + } + + const snapsByOffset = new Map(); + + for (const snap of pointSnaps) { + const offset = round(snap.offset); + const offsetSnaps = snapsByOffset.get(offset) ?? []; + offsetSnaps.push(snap); + snapsByOffset.set(offset, offsetSnaps); + } + + const keptPointSnaps = new Set(); + const offsetGroups = Array.from(snapsByOffset.entries()).map( + ([offset, offsetSnaps]) => ({ + offset, + snaps: offsetSnaps, + visualDistance: Math.min( + ...offsetSnaps.map((snap) => snap.visualDistance), + ), + }), + ); + const sortedOffsetGroups = offsetGroups + .slice() + .sort((a, b) => a.visualDistance - b.visualDistance); + let keepOffsetGroupsUntil = sortedOffsetGroups.length; + + // Prefer a nearby reference cluster before comparing offset. If there is no + // clear distance break, this keeps all offsets and falls back to the old + // smallest-offset behavior. + for (let i = 1; i < sortedOffsetGroups.length; i++) { + const distanceGap = + sortedOffsetGroups[i].visualDistance - + sortedOffsetGroups[i - 1].visualDistance; + + if (distanceGap > clusterBreakDistance) { + keepOffsetGroupsUntil = i; + break; + } + } + + const nearestOffsetGroups = sortedOffsetGroups.slice( + 0, + keepOffsetGroupsUntil, + ); + const minOffset = Math.min( + ...nearestOffsetGroups.map(({ offset }) => Math.abs(offset)), + ); + const selectedOffsets = new Set( + nearestOffsetGroups + .filter(({ offset }) => Math.abs(offset) === minOffset) + .map(({ offset }) => offset), + ); + + for (const offsetSnaps of snapsByOffset.values()) { + if (!selectedOffsets.has(round(offsetSnaps[0].offset))) { + continue; + } + + if (offsetSnaps.length < 2) { + offsetSnaps.forEach((snap) => keptPointSnaps.add(snap)); + continue; + } + + const sortedSnaps = offsetSnaps + .slice() + .sort((a, b) => a.visualDistance - b.visualDistance); + + let keepUntil = sortedSnaps.length; + + // This runs after the winning snap offset is known. A big visual-distance + // gap means references after it belong to a separate, farther cluster. + for (let i = 1; i < sortedSnaps.length; i++) { + const distanceGap = + sortedSnaps[i].visualDistance - sortedSnaps[i - 1].visualDistance; + + if (distanceGap > clusterBreakDistance) { + keepUntil = i; + break; + } + } + + for (let i = 0; i < keepUntil; i++) { + keptPointSnaps.add(sortedSnaps[i]); + } + } + + let writeIndex = 0; + + for (const snap of snaps) { + if (snap.type !== "point" || keptPointSnaps.has(snap)) { + snaps[writeIndex] = snap; + writeIndex++; + } + } + + snaps.length = writeIndex; +}; + export const getReferenceSnapPoints = ( elements: readonly NonDeletedExcalidrawElement[], selectedElements: ExcalidrawElement[], @@ -689,6 +835,11 @@ export const getReferenceSnapPoints = ( ); }; +/** + * Collects point snaps within snap distance. Unlike gap snaps, point snaps first + * go through visual-cluster filtering so a nearby reference can beat a slightly + * better offset from a far-away reference. + */ const getPointSnaps = ( selectedElements: ExcalidrawElement[], selectionSnapPoints: SnapPoint[], @@ -714,39 +865,52 @@ const getPointSnaps = ( const offsetY = otherSnapPoint.point[1] - thisSnapPoint.point[1]; if (Math.abs(offsetX) <= minOffset.x) { - if (Math.abs(offsetX) < minOffset.x) { - nearestSnapsX.length = 0; - } - nearestSnapsX.push({ type: "point", points: [thisSnapPoint.point, otherSnapPoint.point], pointTypes: [thisSnapPoint.type, otherSnapPoint.type], sourceIds: [thisSnapPoint.sourceId, otherSnapPoint.sourceId], + visualDistance: Math.abs( + otherSnapPoint.point[1] - thisSnapPoint.point[1], + ), offset: offsetX, }); - - minOffset.x = Math.abs(offsetX); } if (Math.abs(offsetY) <= minOffset.y) { - if (Math.abs(offsetY) < minOffset.y) { - nearestSnapsY.length = 0; - } - nearestSnapsY.push({ type: "point", points: [thisSnapPoint.point, otherSnapPoint.point], pointTypes: [thisSnapPoint.type, otherSnapPoint.type], sourceIds: [thisSnapPoint.sourceId, otherSnapPoint.sourceId], + visualDistance: Math.abs( + otherSnapPoint.point[0] - thisSnapPoint.point[0], + ), offset: offsetY, }); - - minOffset.y = Math.abs(offsetY); } } } } + + const clusterBreakDistance = getSnapReferenceClusterBreakDistance( + app.state.zoom.value, + ); + + filterPointSnapsToNearestCluster(nearestSnapsX, clusterBreakDistance); + filterPointSnapsToNearestCluster(nearestSnapsY, clusterBreakDistance); + + if (nearestSnapsX.length > 0) { + minOffset.x = Math.min( + ...nearestSnapsX.map((snap) => Math.abs(snap.offset)), + ); + } + + if (nearestSnapsY.length > 0) { + minOffset.y = Math.min( + ...nearestSnapsY.map((snap) => Math.abs(snap.offset)), + ); + } }; export const snapDraggedElements = ( @@ -899,6 +1063,10 @@ type PointSnapLineBucket = { matches: PointSnapLineMatch[]; }; +/** + * Describes which source pair produced a rendered point snapline, so later + * filtering can reason about redundancy without mixing unrelated references. + */ const getPointSnapLineMatch = (snap: PointSnap): PointSnapLineMatch => { return { sourceKey: snap.sourceIds.join("->"), @@ -908,6 +1076,10 @@ const getPointSnapLineMatch = (snap: PointSnap): PointSnapLineMatch => { }; }; +/** + * Builds the outer snapline coordinates by source pair. Center snaplines use + * this map to decide whether the surrounding outer lines already explain them. + */ const getOuterCoordinatesBySourceKey = ( snapLines: PointSnapLineCandidate[], getCoordinate: (snapLine: PointSnapLine) => number, @@ -963,6 +1135,10 @@ const isRedundantOuterSnapLine = ( return coordinate !== minCoordinate && coordinate !== maxCoordinate; }; +/** + * Removes point snaplines that are visually redundant, while preserving any line + * that is still needed by at least one source pair represented on that line. + */ const filterRedundantPointSnapLines = ( snapLines: PointSnapLineCandidate[], getCoordinate: (snapLine: PointSnapLine) => number, diff --git a/packages/excalidraw/tests/snapping.test.ts b/packages/excalidraw/tests/snapping.test.ts new file mode 100644 index 0000000000..1cf2599d54 --- /dev/null +++ b/packages/excalidraw/tests/snapping.test.ts @@ -0,0 +1,509 @@ +import { arrayToMap } from "@excalidraw/common"; +import { pointFrom, type GlobalPoint, type Radians } from "@excalidraw/math"; + +import type { ExcalidrawElement } from "@excalidraw/element/types"; + +import { getDefaultAppState } from "../appState"; +import { + getElementsCorners, + getReferenceSnapPoints, + SnapCache, + snapDraggedElements, +} from "../snapping"; + +import { API } from "./helpers/api"; + +import type { AppClassProperties, AppState } from "../types"; + +type ReferenceSnapPoints = NonNullable< + ReturnType +>; + +const NO_MODIFIER_KEYS = { + altKey: false, + ctrlKey: false, + metaKey: false, + shiftKey: false, +}; + +const createSnappingApp = (appState: Partial = {}) => + ({ + props: {}, + state: { + ...getDefaultAppState(), + objectsSnapModeEnabled: true, + width: 1000, + height: 1000, + offsetLeft: 0, + offsetTop: 0, + ...appState, + }, + } as AppClassProperties); + +const getHorizontalPointSnapLineCoordinates = ( + snapLines: ReturnType["snapLines"], +) => { + return snapLines + .filter((snapLine) => snapLine.type === "points") + .filter((snapLine) => { + const [firstPoint, lastPoint] = snapLine.points; + + return firstPoint[1] === lastPoint[1]; + }) + .map((snapLine) => { + return snapLine.points[0][1]; + }) + .sort((a, b) => a - b); +}; + +const getHorizontalPointSnapLineMaxX = ( + snapLines: ReturnType["snapLines"], +) => { + const horizontalSnapLine = snapLines + .filter((snapLine) => snapLine.type === "points") + .find((snapLine) => { + const [firstPoint, lastPoint] = snapLine.points; + + return firstPoint[1] === lastPoint[1]; + }); + + if (!horizontalSnapLine) { + return null; + } + + return horizontalSnapLine.points[horizontalSnapLine.points.length - 1][0]; +}; + +const getPointKeys = (points: ReturnType) => { + return points.map((point) => point.join(",")); +}; + +const getReferenceSnapPointKeys = ( + elements: ExcalidrawElement[], + selectedElements: ExcalidrawElement[], + app: AppClassProperties, +) => { + return new Set( + getReferenceSnapPoints( + elements, + selectedElements, + app.state, + arrayToMap(elements), + ).map((snapPoint) => snapPoint.point.join(",")), + ); +}; + +const primeReferenceSnapPoints = ( + elements: ExcalidrawElement[], + selectedElements: ExcalidrawElement[], +) => { + const selectedElementIds = new Set( + selectedElements.map((element) => element.id), + ); + const elementsMap = arrayToMap(elements); + + SnapCache.setReferenceSnapPoints( + elements + .filter((element) => !selectedElementIds.has(element.id)) + .flatMap((element) => { + const corners = getElementsCorners([element], elementsMap); + + return corners.map((point, index) => ({ + point, + type: index === corners.length - 1 ? "center" : "outer", + sourceId: element.id, + })); + }) as Parameters[0], + ); +}; + +describe("snapping", () => { + afterEach(() => { + SnapCache.destroy(); + }); + + it("does not use frame children as references when snapping outside elements", () => { + const frame = API.createElement({ + type: "frame", + id: "frame", + x: 0, + y: 0, + width: 300, + height: 300, + }); + const frameChild = API.createElement({ + type: "rectangle", + id: "frameChild", + x: 37, + y: 53, + width: 71, + height: 83, + frameId: frame.id, + }); + const selected = API.createElement({ + type: "rectangle", + id: "selected", + x: 400, + y: 50, + width: 100, + height: 100, + }); + const elements = [frame, frameChild, selected]; + const app = createSnappingApp({ + selectedElementIds: { [selected.id]: true }, + }); + + const referenceSnapPointKeys = getReferenceSnapPointKeys( + elements, + [selected], + app, + ); + const frameChildPointKeys = getPointKeys( + getElementsCorners([frameChild], arrayToMap(elements)), + ); + + expect( + frameChildPointKeys.some((pointKey) => + referenceSnapPointKeys.has(pointKey), + ), + ).toBe(false); + }); + + it("uses frame siblings as references when snapping elements in the same frame", () => { + const frame = API.createElement({ + type: "frame", + id: "frame", + x: 0, + y: 0, + width: 300, + height: 300, + }); + const sibling = API.createElement({ + type: "rectangle", + id: "sibling", + x: 37, + y: 53, + width: 71, + height: 83, + frameId: frame.id, + }); + const selected = API.createElement({ + type: "rectangle", + id: "selected", + x: 150, + y: 50, + width: 100, + height: 100, + frameId: frame.id, + }); + const elements = [frame, sibling, selected]; + const app = createSnappingApp({ + selectedElementIds: { [selected.id]: true }, + }); + + const referenceSnapPointKeys = getReferenceSnapPointKeys( + elements, + [selected], + app, + ); + const siblingPointKeys = getPointKeys( + getElementsCorners([sibling], arrayToMap(elements)), + ); + + expect( + siblingPointKeys.some((pointKey) => referenceSnapPointKeys.has(pointKey)), + ).toBe(true); + }); + + it("does not use frame children as references when snapping the frame itself", () => { + const frame = API.createElement({ + type: "frame", + id: "frame", + x: 0, + y: 0, + width: 300, + height: 300, + }); + const frameChild = API.createElement({ + type: "rectangle", + id: "frameChild", + x: 37, + y: 53, + width: 71, + height: 83, + frameId: frame.id, + }); + const elements = [frame, frameChild]; + const app = createSnappingApp({ + selectedElementIds: { [frame.id]: true }, + }); + + const referenceSnapPointKeys = getReferenceSnapPointKeys( + elements, + [frame], + app, + ); + const frameChildPointKeys = getPointKeys( + getElementsCorners([frameChild], arrayToMap(elements)), + ); + + expect( + frameChildPointKeys.some((pointKey) => + referenceSnapPointKeys.has(pointKey), + ), + ).toBe(false); + }); + + it("filters center and inner outer point snaplines for the same reference", () => { + const angle = 0.68 as Radians; + const reference = API.createElement({ + type: "rectangle", + id: "reference", + x: 0, + y: 0, + width: 140, + height: 140, + angle, + }); + const selected = API.createElement({ + type: "rectangle", + id: "selected", + x: 200, + y: 0, + width: 140, + height: 140, + angle, + }); + const elements = [reference, selected]; + const app = createSnappingApp({ + selectedElementIds: { [selected.id]: true }, + }); + + primeReferenceSnapPoints(elements, [selected]); + + const { snapLines } = snapDraggedElements( + elements, + { x: 0, y: 0 }, + app, + NO_MODIFIER_KEYS, + arrayToMap(elements), + ); + + expect(getHorizontalPointSnapLineCoordinates(snapLines)).toHaveLength(2); + }); + + it("keeps a snapline that is redundant for one reference but needed for another", () => { + const angle = 0.68 as Radians; + const selected = API.createElement({ + type: "rectangle", + id: "selected", + x: 200, + y: 0, + width: 140, + height: 140, + angle, + }); + const elements = [selected]; + const elementsMap = arrayToMap(elements); + const selectedSnapPoints = getElementsCorners([selected], elementsMap); + const outerSnapPoints = selectedSnapPoints.slice(0, -1); + const centerSnapPoint = selectedSnapPoints[selectedSnapPoints.length - 1]; + const innerOuterSnapPoint = [...outerSnapPoints].sort( + (a, b) => a[1] - b[1], + )[1]; + const app = createSnappingApp({ + selectedElementIds: { [selected.id]: true }, + }); + + const referenceSnapPoints: ReferenceSnapPoints = [ + ...outerSnapPoints.map((point) => ({ + point: pointFrom(point[0] - 200, point[1]), + type: "outer" as const, + sourceId: "referenceA", + })), + { + point: pointFrom( + centerSnapPoint[0] - 200, + centerSnapPoint[1], + ), + type: "center" as const, + sourceId: "referenceA", + }, + { + point: pointFrom( + innerOuterSnapPoint[0] - 300, + innerOuterSnapPoint[1], + ), + type: "outer" as const, + sourceId: "referenceB", + }, + ]; + + SnapCache.setReferenceSnapPoints(referenceSnapPoints); + + const { snapLines } = snapDraggedElements( + elements, + { x: 0, y: 0 }, + app, + NO_MODIFIER_KEYS, + elementsMap, + ); + + expect(getHorizontalPointSnapLineCoordinates(snapLines)).toHaveLength(3); + }); + + it("keeps a center snapline when no outer snaplines imply it", () => { + const reference = API.createElement({ + type: "rectangle", + id: "reference", + x: 0, + y: 0, + width: 100, + height: 100, + }); + const selected = API.createElement({ + type: "rectangle", + id: "selected", + x: 200, + y: 25, + width: 50, + height: 50, + }); + const elements = [reference, selected]; + const app = createSnappingApp({ + selectedElementIds: { [selected.id]: true }, + }); + + primeReferenceSnapPoints(elements, [selected]); + + const { snapLines } = snapDraggedElements( + elements, + { x: 0, y: 0 }, + app, + NO_MODIFIER_KEYS, + arrayToMap(elements), + ); + + expect(getHorizontalPointSnapLineCoordinates(snapLines)).toEqual([50]); + }); + + it("prefers the nearest visual cluster for same-offset point snaps", () => { + const selected = API.createElement({ + type: "rectangle", + id: "selected", + x: 0, + y: 0, + width: 100, + height: 100, + }); + const elements = [selected]; + const app = createSnappingApp({ + selectedElementIds: { [selected.id]: true }, + }); + + SnapCache.setReferenceSnapPoints([ + { + point: pointFrom(220, 50), + type: "center", + sourceId: "near", + }, + { + point: pointFrom(900, 50), + type: "center", + sourceId: "far", + }, + ]); + + const { snapLines } = snapDraggedElements( + elements, + { x: 0, y: 0 }, + app, + NO_MODIFIER_KEYS, + arrayToMap(elements), + ); + + expect(getHorizontalPointSnapLineMaxX(snapLines)).toBe(220); + }); + + it("prefers a nearby point snap over a slightly better far offset", () => { + const selected = API.createElement({ + type: "rectangle", + id: "selected", + x: 0, + y: 0, + width: 100, + height: 100, + }); + const elements = [selected]; + const app = createSnappingApp({ + selectedElementIds: { [selected.id]: true }, + }); + + SnapCache.setReferenceSnapPoints([ + { + point: pointFrom(220, 54), + type: "center", + sourceId: "near", + }, + { + point: pointFrom(900, 50), + type: "center", + sourceId: "far", + }, + ]); + + const { snapOffset, snapLines } = snapDraggedElements( + elements, + { x: 0, y: 0 }, + app, + NO_MODIFIER_KEYS, + arrayToMap(elements), + ); + + expect(snapOffset.y).toBe(4); + expect(getHorizontalPointSnapLineMaxX(snapLines)).toBe(220); + }); + + it("keeps same-offset point snaps when references form a continuous cluster", () => { + const selected = API.createElement({ + type: "rectangle", + id: "selected", + x: 0, + y: 0, + width: 100, + height: 100, + }); + const elements = [selected]; + const app = createSnappingApp({ + selectedElementIds: { [selected.id]: true }, + }); + + SnapCache.setReferenceSnapPoints([ + { + point: pointFrom(200, 50), + type: "center", + sourceId: "referenceA", + }, + { + point: pointFrom(350, 50), + type: "center", + sourceId: "referenceB", + }, + { + point: pointFrom(500, 50), + type: "center", + sourceId: "referenceC", + }, + ]); + + const { snapLines } = snapDraggedElements( + elements, + { x: 0, y: 0 }, + app, + NO_MODIFIER_KEYS, + arrayToMap(elements), + ); + + expect(getHorizontalPointSnapLineMaxX(snapLines)).toBe(500); + }); +});