perf: Cache hits in collision detection (#10648)

* feat: Cache hits and scene lookups

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

* chore: Remove debug

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

* fix: Consider hit threshold and inside override too

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

* chore: Remove Map caching

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

* fix: incorrect threshold

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

* fix: threshold setting

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

* fix: Hit caching

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

* fix: cache override

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

---------

Signed-off-by: Mark Tolmacs <mark@lazycat.hu>

packages/element/src/collision.ts

@@ -105,6 +105,12 @@ export type HitTestArgs = {
   overrideShouldTestInside?: boolean;
 };
 
+let cachedPoint: GlobalPoint | null = null;
+let cachedElement: WeakRef<ExcalidrawElement> | 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 = (

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<GlobalPoint>(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<GlobalPoint>(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(<Excalidraw handleKeyboardGlobally={true} />);
+  });
+
+  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<GlobalPoint>(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<GlobalPoint>(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<GlobalPoint>(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<GlobalPoint>(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);
+  });
+});

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(`
       [