factor out shape cache into ShapeCache class

and more non-renderScene shape cache retrieval more resilient

src/components/App.tsx

@@ -196,7 +196,6 @@ import {
   getGridPoint,
   isPathALoop,
 } from "../math";
-import { invalidateShapeForElement } from "../renderer/renderElement";
 import {
   calculateScrollCenter,
   getElementsAtPosition,
@@ -353,6 +352,7 @@ import { ValueOf } from "../utility-types";
 import { isSidebarDockedAtom } from "./Sidebar/Sidebar";
 import { StaticCanvas, InteractiveCanvas } from "./canvases";
 import { Renderer } from "../scene/Renderer";
+import { ShapeCache } from "../scene/ShapeCache";
 
 const AppContext = React.createContext<AppClassProperties>(null!);
 const AppPropsContext = React.createContext<AppProps>(null!);
@@ -764,7 +764,7 @@ class App extends React.Component<AppProps, AppState> {
           );
 
           mutateElement(element, { validated }, false);
-          invalidateShapeForElement(element);
+          ShapeCache.delete(element);
         }
       }
       return false;
@@ -1704,6 +1704,7 @@ class App extends React.Component<AppProps, AppState> {
     this.removeEventListeners();
     this.scene.destroy();
     this.library.destroy();
+    ShapeCache.destroy();
     clearTimeout(touchTimeout);
     isSomeElementSelected.clearCache();
     selectGroupsForSelectedElements.clearCache();
@@ -1713,7 +1714,7 @@ class App extends React.Component<AppProps, AppState> {
   private onResize = withBatchedUpdates(() => {
     this.scene
       .getElementsIncludingDeleted()
-      .forEach((element) => invalidateShapeForElement(element));
+      .forEach((element) => ShapeCache.delete(element));
     this.setState({});
   });
 
@@ -2701,7 +2702,7 @@ class App extends React.Component<AppProps, AppState> {
           filesMap.has(element.fileId)
         ) {
           this.imageCache.delete(element.fileId);
-          invalidateShapeForElement(element);
+          ShapeCache.delete(element);
         }
       });
       this.scene.informMutation();
@@ -7262,7 +7263,7 @@ class App extends React.Component<AppProps, AppState> {
     if (updatedFiles.size || erroredFiles.size) {
       for (const element of elements) {
         if (updatedFiles.has(element.fileId)) {
-          invalidateShapeForElement(element);
+          ShapeCache.delete(element);
         }
       }
     }

src/components/EyeDropper.tsx

@@ -8,9 +8,9 @@ import { mutateElement } from "../element/mutateElement";
 import { useCreatePortalContainer } from "../hooks/useCreatePortalContainer";
 import { useOutsideClick } from "../hooks/useOutsideClick";
 import { KEYS } from "../keys";
-import { invalidateShapeForElement } from "../renderer/renderElement";
 import { getSelectedElements } from "../scene";
 import Scene from "../scene/Scene";
+import { ShapeCache } from "../scene/ShapeCache";
 import { useApp, useExcalidrawContainer, useExcalidrawElements } from "./App";
 
 import "./EyeDropper.scss";
@@ -98,7 +98,7 @@ export const EyeDropper: React.FC<{
             },
             false,
           );
-          invalidateShapeForElement(element);
+          ShapeCache.delete(element);
         }
         Scene.getScene(
           metaStuffRef.current.selectedElements[0],

src/element/Hyperlink.tsx

@@ -25,10 +25,7 @@ import {
 } from "react";
 import clsx from "clsx";
 import { KEYS } from "../keys";
-import {
+import { DEFAULT_LINK_SIZE } from "../renderer/renderElement";
-  DEFAULT_LINK_SIZE,
-  invalidateShapeForElement,
-} from "../renderer/renderElement";
 import { rotate } from "../math";
 import { EVENT, HYPERLINK_TOOLTIP_DELAY, MIME_TYPES } from "../constants";
 import { Bounds } from "./bounds";
@@ -42,6 +39,7 @@ import "./Hyperlink.scss";
 import { trackEvent } from "../analytics";
 import { useAppProps, useExcalidrawAppState } from "../components/App";
 import { isEmbeddableElement } from "./typeChecks";
+import { ShapeCache } from "../scene/ShapeCache";
 
 const CONTAINER_WIDTH = 320;
 const SPACE_BOTTOM = 85;
@@ -115,7 +113,7 @@ export const Hyperlink = ({
           validated: false,
           link,
         });
-        invalidateShapeForElement(element);
+        ShapeCache.delete(element);
       } else {
         const { width, height } = element;
         const embedLink = getEmbedLink(link);
@@ -147,7 +145,7 @@ export const Hyperlink = ({
           validated: true,
           link,
         });
-        invalidateShapeForElement(element);
+        ShapeCache.delete(element);
         if (embeddableLinkCache.has(element.id)) {
           embeddableLinkCache.delete(element.id);
         }

src/element/bounds.ts

@@ -10,10 +10,7 @@ import { distance2d, rotate, rotatePoint } from "../math";
 import rough from "roughjs/bin/rough";
 import { Drawable, Op } from "roughjs/bin/core";
 import { Point } from "../types";
-import {
+import { generateRoughOptions } from "../renderer/renderElement";
-  getShapeForElement,
-  generateRoughOptions,
-} from "../renderer/renderElement";
 import {
   isArrowElement,
   isFreeDrawElement,
@@ -24,6 +21,7 @@ import { rescalePoints } from "../points";
 import { getBoundTextElement, getContainerElement } from "./textElement";
 import { LinearElementEditor } from "./linearElementEditor";
 import { Mutable } from "../utility-types";
+import { ShapeCache } from "../scene/ShapeCache";
 
 export type RectangleBox = {
   x: number;
@@ -621,7 +619,7 @@ const getLinearElementRotatedBounds = (
   }
 
   // first element is always the curve
-  const cachedShape = getShapeForElement(element)?.[0];
+  const cachedShape = ShapeCache.get(element)?.[0];
   const shape = cachedShape ?? generateLinearElementShape(element);
   const ops = getCurvePathOps(shape);
   const transformXY = (x: number, y: number) =>

src/element/collision.ts

@@ -39,7 +39,6 @@ import {
 import { FrameNameBoundsCache, Point } from "../types";
 import { Drawable } from "roughjs/bin/core";
 import { AppState } from "../types";
-import { getShapeForElement } from "../renderer/renderElement";
 import {
   hasBoundTextElement,
   isEmbeddableElement,
@@ -50,6 +49,7 @@ import { isTransparent } from "../utils";
 import { shouldShowBoundingBox } from "./transformHandles";
 import { getBoundTextElement } from "./textElement";
 import { Mutable } from "../utility-types";
+import { ShapeCache } from "../scene/ShapeCache";
 
 const isElementDraggableFromInside = (
   element: NonDeletedExcalidrawElement,
@@ -489,7 +489,7 @@ const hitTestFreeDrawElement = (
     B = element.points[i + 1];
   }
 
-  const shape = getShapeForElement(element);
+  const shape = ShapeCache.get(element);
 
   // for filled freedraw shapes, support
   // selecting from inside
@@ -502,7 +502,7 @@ const hitTestFreeDrawElement = (
 
 const hitTestLinear = (args: HitTestArgs): boolean => {
   const { element, threshold } = args;
-  if (!getShapeForElement(element)) {
+  if (!ShapeCache.get(element)) {
     return false;
   }
 
@@ -520,7 +520,7 @@ const hitTestLinear = (args: HitTestArgs): boolean => {
   }
   const [relX, relY] = GAPoint.toTuple(point);
 
-  const shape = getShapeForElement(element as ExcalidrawLinearElement);
+  const shape = ShapeCache.get(element as ExcalidrawLinearElement);
 
   if (!shape) {
     return false;

src/element/linearElementEditor.ts

@@ -44,9 +44,9 @@ import { tupleToCoors } from "../utils";
 import { isBindingElement } from "./typeChecks";
 import { shouldRotateWithDiscreteAngle } from "../keys";
 import { getBoundTextElement, handleBindTextResize } from "./textElement";
-import { getShapeForElement } from "../renderer/renderElement";
 import { DRAGGING_THRESHOLD } from "../constants";
 import { Mutable } from "../utility-types";
+import { ShapeCache } from "../scene/ShapeCache";
 
 const editorMidPointsCache: {
   version: number | null;
@@ -1423,7 +1423,7 @@ export class LinearElementEditor {
     let y1;
     let x2;
     let y2;
-    if (element.points.length < 2 || !getShapeForElement(element)) {
+    if (element.points.length < 2 || !ShapeCache.get(element)) {
       // XXX this is just a poor estimate and not very useful
       const { minX, minY, maxX, maxY } = element.points.reduce(
         (limits, [x, y]) => {
@@ -1442,7 +1442,7 @@ export class LinearElementEditor {
       x2 = maxX + element.x;
       y2 = maxY + element.y;
     } else {
-      const shape = getShapeForElement(element)!;
+      const shape = ShapeCache.generateElementShape(element);
 
       // first element is always the curve
       const ops = getCurvePathOps(shape[0]);

src/element/mutateElement.ts

@@ -1,11 +1,11 @@
 import { ExcalidrawElement } from "./types";
-import { invalidateShapeForElement } from "../renderer/renderElement";
 import Scene from "../scene/Scene";
 import { getSizeFromPoints } from "../points";
 import { randomInteger } from "../random";
 import { Point } from "../types";
 import { getUpdatedTimestamp } from "../utils";
 import { Mutable } from "../utility-types";
+import { ShapeCache } from "../scene/ShapeCache";
 
 type ElementUpdate<TElement extends ExcalidrawElement> = Omit<
   Partial<TElement>,
@@ -89,7 +89,7 @@ export const mutateElement = <TElement extends Mutable<ExcalidrawElement>>(
     typeof fileId != "undefined" ||
     typeof points !== "undefined"
   ) {
-    invalidateShapeForElement(element);
+    ShapeCache.delete(element);
   }
 
   element.version++;

src/math.ts

@@ -10,9 +10,9 @@ import {
   ExcalidrawLinearElement,
   NonDeleted,
 } from "./element/types";
-import { getShapeForElement } from "./renderer/renderElement";
 import { getCurvePathOps } from "./element/bounds";
 import { Mutable } from "./utility-types";
+import { ShapeCache } from "./scene/ShapeCache";
 
 export const rotate = (
   x1: number,
@@ -303,7 +303,7 @@ export const getControlPointsForBezierCurve = (
   element: NonDeleted<ExcalidrawLinearElement>,
   endPoint: Point,
 ) => {
-  const shape = getShapeForElement(element as ExcalidrawLinearElement);
+  const shape = ShapeCache.generateElementShape(element);
   if (!shape) {
     return null;
   }

src/renderer/renderElement.ts

@@ -67,6 +67,7 @@ import {
 } from "../element/embeddable";
 import { getContainingFrame } from "../frame";
 import { normalizeLink, toValidURL } from "../data/url";
+import { ShapeCache } from "../scene/ShapeCache";
 
 // using a stronger invert (100% vs our regular 93%) and saturate
 // as a temp hack to make images in dark theme look closer to original
@@ -270,6 +271,7 @@ const drawImagePlaceholder = (
     size,
   );
 };
+
 const drawElementOnCanvas = (
   element: NonDeletedExcalidrawElement,
   rc: RoughCanvas,
@@ -286,7 +288,7 @@ const drawElementOnCanvas = (
     case "ellipse": {
       context.lineJoin = "round";
       context.lineCap = "round";
-      rc.draw(getShapeForElement(element)!);
+      rc.draw(ShapeCache.get(element)!);
       break;
     }
     case "arrow":
@@ -294,7 +296,7 @@ const drawElementOnCanvas = (
       context.lineJoin = "round";
       context.lineCap = "round";
 
-      getShapeForElement(element)!.forEach((shape) => {
+      ShapeCache.get(element)!.forEach((shape) => {
         rc.draw(shape);
       });
       break;
@@ -305,7 +307,7 @@ const drawElementOnCanvas = (
       context.fillStyle = element.strokeColor;
 
       const path = getFreeDrawPath2D(element) as Path2D;
-      const fillShape = getShapeForElement(element);
+      const fillShape = ShapeCache.get(element);
 
       if (fillShape) {
         rc.draw(fillShape);
@@ -387,33 +389,6 @@ const elementWithCanvasCache = new WeakMap<
   ExcalidrawElementWithCanvas
 >();
 
-const shapeCache = new WeakMap<ExcalidrawElement, ElementShape>();
-
-type ElementShape = Drawable | Drawable[] | null;
-
-type ElementShapes = {
-  freedraw: Drawable | null;
-  arrow: Drawable[];
-  line: Drawable[];
-  text: null;
-  image: null;
-};
-
-export const getShapeForElement = <T extends ExcalidrawElement>(element: T) =>
-  shapeCache.get(element) as T["type"] extends keyof ElementShapes
-    ? ElementShapes[T["type"]] | undefined
-    : Drawable | null | undefined;
-
-export const setShapeForElement = <T extends ExcalidrawElement>(
-  element: T,
-  shape: T["type"] extends keyof ElementShapes
-    ? ElementShapes[T["type"]]
-    : Drawable,
-) => shapeCache.set(element, shape);
-
-export const invalidateShapeForElement = (element: ExcalidrawElement) =>
-  shapeCache.delete(element);
-
 export const generateRoughOptions = (
   element: ExcalidrawElement,
   continuousPath = false,
@@ -503,16 +478,22 @@ const modifyEmbeddableForRoughOptions = (
  * @param element
  * @param generator
  */
-const generateElementShape = (
+export const generateElementShape = (
   element: NonDeletedExcalidrawElement,
   generator: RoughGenerator,
   isExporting: boolean = false,
-) => {
+): Drawable | Drawable[] | null => {
-  let shape = isExporting ? undefined : shapeCache.get(element);
+  const cachedShape = isExporting ? undefined : ShapeCache.get(element);
+
+  if (cachedShape) {
+    return cachedShape;
+  }
 
   // `null` indicates no rc shape applicable for this element type
   // (= do not generate anything)
-  if (shape === undefined) {
+  if (cachedShape === undefined) {
+    let shape: Drawable | Drawable[] | null = null;
+
     elementWithCanvasCache.delete(element);
 
     switch (element.type) {
@@ -548,7 +529,7 @@ const generateElementShape = (
             ),
           );
         }
-        setShapeForElement(element, shape);
+        ShapeCache.set(element, shape);
 
         break;
       }
@@ -598,7 +579,7 @@ const generateElementShape = (
             generateRoughOptions(element),
           );
         }
-        setShapeForElement(element, shape);
+        ShapeCache.set(element, shape);
 
         break;
       }
@@ -610,7 +591,7 @@ const generateElementShape = (
           element.height,
           generateRoughOptions(element),
         );
-        setShapeForElement(element, shape);
+        ShapeCache.set(element, shape);
 
         break;
       case "line":
@@ -735,7 +716,7 @@ const generateElementShape = (
           }
         }
 
-        setShapeForElement(element, shape);
+        ShapeCache.set(element, shape);
 
         break;
       }
@@ -751,17 +732,19 @@ const generateElementShape = (
         } else {
           shape = null;
         }
-        setShapeForElement(element, shape);
+        ShapeCache.set(element, shape);
         break;
       }
       case "text":
       case "image": {
         // just to ensure we don't regenerate element.canvas on rerenders
-        setShapeForElement(element, null);
+        ShapeCache.set(element, null);
         break;
       }
     }
+    return shape;
   }
+  return null;
 };
 
 const generateElementWithCanvas = (
@@ -1300,7 +1283,7 @@ export const renderElementToSvg = (
       generateElementShape(element, generator);
       const node = roughSVGDrawWithPrecision(
         rsvg,
-        getShapeForElement(element)!,
+        ShapeCache.get(element)!,
         MAX_DECIMALS_FOR_SVG_EXPORT,
       );
       if (opacity !== 1) {
@@ -1330,7 +1313,7 @@ export const renderElementToSvg = (
       generateElementShape(element, generator, true);
       const node = roughSVGDrawWithPrecision(
         rsvg,
-        getShapeForElement(element)!,
+        ShapeCache.get(element)!,
         MAX_DECIMALS_FOR_SVG_EXPORT,
       );
       const opacity = element.opacity / 100;
@@ -1364,7 +1347,7 @@ export const renderElementToSvg = (
       // render embeddable element + iframe
       const embeddableNode = roughSVGDrawWithPrecision(
         rsvg,
-        getShapeForElement(element)!,
+        ShapeCache.get(element)!,
         MAX_DECIMALS_FOR_SVG_EXPORT,
       );
       embeddableNode.setAttribute("stroke-linecap", "round");
@@ -1477,7 +1460,7 @@ export const renderElementToSvg = (
       }
       group.setAttribute("stroke-linecap", "round");
 
-      getShapeForElement(element)!.forEach((shape) => {
+      ShapeCache.get(element)!.forEach((shape) => {
         const node = roughSVGDrawWithPrecision(
           rsvg,
           shape,
@@ -1520,7 +1503,7 @@ export const renderElementToSvg = (
     case "freedraw": {
       generateElementShape(element, generator);
       generateFreeDrawShape(element);
-      const shape = getShapeForElement(element);
+      const shape = ShapeCache.get(element);
       const node = shape
         ? roughSVGDrawWithPrecision(rsvg, shape, MAX_DECIMALS_FOR_SVG_EXPORT)
         : svgRoot.ownerDocument!.createElementNS(SVG_NS, "g");

src/scene/Fonts.ts

@@ -2,9 +2,9 @@ import { isTextElement, refreshTextDimensions } from "../element";
 import { newElementWith } from "../element/mutateElement";
 import { isBoundToContainer } from "../element/typeChecks";
 import { ExcalidrawElement, ExcalidrawTextElement } from "../element/types";
-import { invalidateShapeForElement } from "../renderer/renderElement";
 import { getFontString } from "../utils";
 import type Scene from "./Scene";
+import { ShapeCache } from "./ShapeCache";
 
 export class Fonts {
   private scene: Scene;
@@ -54,7 +54,7 @@ export class Fonts {
 
     this.scene.mapElements((element) => {
       if (isTextElement(element) && !isBoundToContainer(element)) {
-        invalidateShapeForElement(element);
+        ShapeCache.delete(element);
         didUpdate = true;
         return newElementWith(element, {
           ...refreshTextDimensions(element),

src/scene/ShapeCache.ts

@@ -0,0 +1,61 @@
+import { Drawable } from "roughjs/bin/core";
+import { RoughGenerator } from "roughjs/bin/generator";
+import { ExcalidrawElement } from "../element/types";
+import { generateElementShape } from "../renderer/renderElement";
+
+type ElementShape = Drawable | Drawable[] | null;
+
+type ElementShapes = {
+  freedraw: Drawable | null;
+  arrow: Drawable[];
+  line: Drawable[];
+  text: null;
+  image: null;
+};
+
+export class ShapeCache {
+  private static rg = new RoughGenerator();
+  private static cache = new WeakMap<ExcalidrawElement, ElementShape>();
+
+  public static get = <T extends ExcalidrawElement>(element: T) => {
+    return ShapeCache.cache.get(
+      element,
+    ) as T["type"] extends keyof ElementShapes
+      ? ElementShapes[T["type"]] | undefined
+      : Drawable | null | undefined;
+  };
+
+  public static set = <T extends ExcalidrawElement>(
+    element: T,
+    shape: T["type"] extends keyof ElementShapes
+      ? ElementShapes[T["type"]]
+      : Drawable,
+  ) => ShapeCache.cache.set(element, shape);
+
+  public static delete = (element: ExcalidrawElement) =>
+    ShapeCache.cache.delete(element);
+
+  public static destroy = () => {
+    ShapeCache.cache = new WeakMap();
+  };
+
+  /**
+   * Generates & caches shape for element if not already cached, otherwise
+   * return cached shape.
+   */
+  public static generateElementShape = <T extends ExcalidrawElement>(
+    element: T,
+  ) => {
+    const shape = generateElementShape(
+      element,
+      ShapeCache.rg,
+      /* so it prefers cache */ false,
+    ) as T["type"] extends keyof ElementShapes
+      ? ElementShapes[T["type"]]
+      : Drawable | null;
+
+    ShapeCache.cache.set(element, shape);
+
+    return shape;
+  };
+}

src/tests/linearElementEditor.test.tsx

@@ -567,8 +567,8 @@ describe("Test Linear Elements", () => {
           lastSegmentMidpoint[0] + delta,
           lastSegmentMidpoint[1] + delta,
         ]);
-        expect(renderInteractiveScene).toHaveBeenCalledTimes(20);
+        expect(renderInteractiveScene).toHaveBeenCalledTimes(21);
-        expect(renderStaticScene).toHaveBeenCalledTimes(8);
+        expect(renderStaticScene).toHaveBeenCalledTimes(7);
         expect(line.points.length).toEqual(5);
 
         expect((h.elements[0] as ExcalidrawLinearElement).points)
@@ -622,8 +622,14 @@ describe("Test Linear Elements", () => {
         expect(midPoints[1]).not.toEqual(newMidPoints[1]);
         expect(newMidPoints).toMatchInlineSnapshot(`
           [
-            null,
+            [
-            null,
+              31.884084517616053,
+              23.13275505472383,
+            ],
+            [
+              77.74792546875662,
+              44.57840982272327,
+            ],
           ]
         `);
       });