From dfdd994dbbefc0562c0c5e42f267f23ae8d9d62d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rk=20Tolm=C3=A1cs?= Date: Mon, 26 Jan 2026 20:50:44 +0100 Subject: [PATCH] perf: Cache hits in collision detection (#10648) * feat: Cache hits and scene lookups Signed-off-by: Mark Tolmacs * chore: Remove debug Signed-off-by: Mark Tolmacs * fix: Consider hit threshold and inside override too Signed-off-by: Mark Tolmacs * chore: Remove Map caching Signed-off-by: Mark Tolmacs * fix: incorrect threshold Signed-off-by: Mark Tolmacs * fix: threshold setting Signed-off-by: Mark Tolmacs * fix: Hit caching Signed-off-by: Mark Tolmacs * fix: cache override Signed-off-by: Mark Tolmacs --------- Signed-off-by: Mark Tolmacs --- packages/element/src/collision.ts | 35 +++- packages/element/tests/collision.test.tsx | 184 +++++++++++++++++- .../tests/linearElementEditor.test.tsx | 2 +- 3 files changed, 217 insertions(+), 4 deletions(-) diff --git a/packages/element/src/collision.ts b/packages/element/src/collision.ts index a96c3ebc14..d93363a4d8 100644 --- a/packages/element/src/collision.ts +++ b/packages/element/src/collision.ts @@ -105,6 +105,12 @@ export type HitTestArgs = { overrideShouldTestInside?: boolean; }; +let cachedPoint: GlobalPoint | null = null; +let cachedElement: WeakRef | null = null; +let cachedThreshold: number = Infinity; +let cachedHit: boolean = false; +let cachedOverrideShouldTestInside = false; + export const hitElementItself = ({ point, element, @@ -113,6 +119,24 @@ export const hitElementItself = ({ frameNameBound = null, overrideShouldTestInside = false, }: HitTestArgs) => { + // Return cached result if the same point and element version is tested again + if ( + cachedPoint && + pointsEqual(point, cachedPoint) && + cachedThreshold <= threshold && + overrideShouldTestInside === cachedOverrideShouldTestInside + ) { + const derefElement = cachedElement?.deref(); + if ( + derefElement && + derefElement.id === element.id && + derefElement.version === element.version && + derefElement.versionNonce === element.versionNonce + ) { + return cachedHit; + } + } + // Hit test against a frame's name const hitFrameName = frameNameBound ? isPointWithinBounds( @@ -153,7 +177,16 @@ export const hitElementItself = ({ isPointOnElementOutline(point, element, elementsMap, threshold) : isPointOnElementOutline(point, element, elementsMap, threshold); - return hitElement || hitFrameName; + const result = hitElement || hitFrameName; + + // Cache end result + cachedPoint = point; + cachedElement = new WeakRef(element); + cachedThreshold = threshold; + cachedOverrideShouldTestInside = overrideShouldTestInside; + cachedHit = result; + + return result; }; export const hitElementBoundingBox = ( diff --git a/packages/element/tests/collision.test.tsx b/packages/element/tests/collision.test.tsx index 72996bdb1f..4061a16cb6 100644 --- a/packages/element/tests/collision.test.tsx +++ b/packages/element/tests/collision.test.tsx @@ -1,9 +1,12 @@ +import { arrayToMap } from "@excalidraw/common"; import { type GlobalPoint, type LocalPoint, pointFrom } from "@excalidraw/math"; import { Excalidraw } from "@excalidraw/excalidraw"; +import { API } from "@excalidraw/excalidraw/tests/helpers/api"; import { UI } from "@excalidraw/excalidraw/tests/helpers/ui"; import "@excalidraw/utils/test-utils"; import { render } from "@excalidraw/excalidraw/tests/test-utils"; +import * as distance from "../src/distance"; import { hitElementItself } from "../src/collision"; describe("check rotated elements can be hit:", () => { @@ -25,8 +28,6 @@ describe("check rotated elements can be hit:", () => { [-4, -302], ] as LocalPoint[], }); - //const p = [120, -211]; - //const p = [0, 13]; const hit = hitElementItself({ point: pointFrom(88, -68), element: window.h.elements[0], @@ -36,3 +37,182 @@ describe("check rotated elements can be hit:", () => { expect(hit).toBe(true); }); }); + +describe("hitElementItself cache", () => { + beforeEach(async () => { + // reset cache + hitElementItself({ + point: pointFrom(50, 50), + element: API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 100, + height: 100, + backgroundColor: "#ffffff", + }), + threshold: Infinity, + elementsMap: new Map([]), + }); + + localStorage.clear(); + await render(); + }); + + it("reuses cached result when threshold increases", () => { + const element = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 100, + height: 100, + backgroundColor: "#ffffff", + }); + const elementsMap = arrayToMap([element]); + const point = pointFrom(100.5, 50); + + const distanceSpy = jest.spyOn(distance, "distanceToElement"); + + expect( + hitElementItself({ + point, + element, + threshold: 1, + elementsMap, + }), + ).toBe(true); + + expect(distanceSpy).toHaveBeenCalledTimes(1); + + expect( + hitElementItself({ + point, + element, + threshold: 10, + elementsMap, + }), + ).toBe(true); + + expect(distanceSpy).toHaveBeenCalledTimes(1); + + distanceSpy.mockRestore(); + }); + + it("does not reuse cache when threshold decreases", () => { + const element = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 100, + height: 100, + backgroundColor: "transparent", + }); + const elementsMap = arrayToMap([element]); + const point = pointFrom(105, 50); + + const distanceSpy = jest.spyOn(distance, "distanceToElement"); + + expect( + hitElementItself({ + point, + element, + threshold: 10, + elementsMap, + }), + ).toBe(true); + + expect(distanceSpy).toHaveBeenCalledTimes(1); + + expect( + hitElementItself({ + point, + element, + threshold: 6, + elementsMap, + }), + ).toBe(true); + + expect(distanceSpy).toHaveBeenCalledTimes(2); + distanceSpy.mockRestore(); + }); + + it("invalidates cache when element version changes", () => { + const element = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 100, + height: 100, + backgroundColor: "#ffffff", + }); + const elementsMap = arrayToMap([element]); + const point = pointFrom(100.5, 50); + + const distanceSpy = jest.spyOn(distance, "distanceToElement"); + + expect( + hitElementItself({ + point, + element, + threshold: 1, + elementsMap, + }), + ).toBe(true); + + expect(distanceSpy).toHaveBeenCalledTimes(1); + + const movedElement = { + ...element, + version: element.version + 1, + versionNonce: element.versionNonce + 1, + }; + + expect( + hitElementItself({ + point, + element: movedElement, + threshold: 1, + elementsMap, + }), + ).toBe(true); + + expect(distanceSpy).toHaveBeenCalledTimes(2); + distanceSpy.mockRestore(); + }); + + it("override does not affect caching", () => { + const element = API.createElement({ + type: "rectangle", + x: 0, + y: 0, + width: 100, + height: 100, + backgroundColor: "transparent", + }); + const elementsMap = arrayToMap([element]); + const point = pointFrom(50, 50); + + const distanceSpy = jest.spyOn(distance, "distanceToElement"); + + expect( + hitElementItself({ + point, + element, + threshold: 10, + elementsMap, + }), + ).toBe(false); + + expect(distanceSpy).toHaveBeenCalledTimes(1); + + expect( + hitElementItself({ + point, + element, + threshold: 10, + elementsMap, + overrideShouldTestInside: true, + }), + ).toBe(true); + }); +}); diff --git a/packages/element/tests/linearElementEditor.test.tsx b/packages/element/tests/linearElementEditor.test.tsx index 5759c591dd..5b31892b4d 100644 --- a/packages/element/tests/linearElementEditor.test.tsx +++ b/packages/element/tests/linearElementEditor.test.tsx @@ -218,7 +218,7 @@ describe("Test Linear Elements", () => { // drag line from midpoint drag(midpoint, pointFrom(midpoint[0] + delta, midpoint[1] + delta)); expect(renderInteractiveScene.mock.calls.length).toMatchInlineSnapshot(`8`); - expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`7`); + expect(renderStaticScene.mock.calls.length).toMatchInlineSnapshot(`6`); expect(line.points.length).toEqual(3); expect(line.points).toMatchInlineSnapshot(` [