Extracting common calculations to the canvases wrapper

src/components/App.tsx

@@ -187,7 +187,7 @@ import {
   KEYS,
 } from "../keys";
 import { distance2d, getGridPoint, isPathALoop } from "../math";
-import { isVisibleElement } from "../renderer/renderScene";
+import { isVisibleElement } from "../element/sizeHelpers";
 import { invalidateShapeForElement } from "../renderer/renderElement";
 import {
   calculateScrollCenter,
@@ -839,16 +839,18 @@ class App extends React.Component<AppProps, AppState> {
                           appState={this.state}
                           scene={this.scene}
                         >
-                          {(elements, versionNonce) => (
+                          {(versionNonce, elements, visibleElements) => (
                             <>
                               <StaticCanvas
                                 canvas={this.canvas}
                                 rc={this.rc}
                                 elements={elements}
+                                visibleElements={visibleElements}
                                 versionNonce={versionNonce}
                                 selectionNonce={
                                   this.state.selectionElement?.versionNonce
                                 }
+                                scale={window.devicePixelRatio}
                                 appState={this.state}
                                 renderConfig={{
                                   imageCache: this.imageCache,
@@ -860,10 +862,12 @@ class App extends React.Component<AppProps, AppState> {
                               <InteractiveCanvas
                                 canvas={this.interactiveCanvas}
                                 elements={elements}
+                                visibleElements={visibleElements}
                                 versionNonce={versionNonce}
                                 selectionNonce={
                                   this.state.selectionElement?.versionNonce
                                 }
+                                scale={window.devicePixelRatio}
                                 appState={this.state}
                                 renderInteractiveSceneCallback={
                                   this.renderInteractiveSceneCallback

src/components/canvases/CanvasesWrapper.tsx

@@ -1,25 +1,30 @@
 import { ReactNode } from "react";
-import { useMutatedElements } from "../../hooks/useMutatedElements";
+import { useCanvasElements } from "../../hooks/useMutatedElements";
-import { AppState } from "../../types";
+import { CommonCanvasAppState } from "../../types";
 import { NonDeletedExcalidrawElement } from "../../element/types";
 import Scene from "../../scene/Scene";
+import { useVisibleCanvasElements } from "../../hooks/useVisibleElements";
 
 type CanvasesWrapperProps = {
-  appState: AppState;
+  appState: CommonCanvasAppState;
   scene: Scene;
   children: (
-    elements: readonly NonDeletedExcalidrawElement[],
     versionNonce: number | undefined,
+    elements: readonly NonDeletedExcalidrawElement[],
+    visibleElements: readonly NonDeletedExcalidrawElement[],
   ) => ReactNode;
 };
 
-const CanvasesWrapper = (props: CanvasesWrapperProps) => {
+const CanvasesWrapper = ({
-  const [elements, versionNonce] = useMutatedElements({
+  appState,
-    appState: props.appState,
+  scene,
-    scene: props.scene,
+  children,
-  });
+}: CanvasesWrapperProps) => {
+  const versionNonce = scene.getVersionNonce();
+  const elements = useCanvasElements(appState, scene);
+  const visibleElements = useVisibleCanvasElements(appState, elements);
 
-  return <main>{props.children(elements, versionNonce)}</main>;
+  return <main>{children(versionNonce, elements, visibleElements)}</main>;
 };
 
 export default CanvasesWrapper;

src/components/canvases/InteractiveCanvas.tsx

@@ -14,8 +14,10 @@ import type { NonDeletedExcalidrawElement } from "../../element/types";
 type InteractiveCanvasProps = {
   canvas: HTMLCanvasElement | null;
   elements: readonly NonDeletedExcalidrawElement[];
+  visibleElements: readonly NonDeletedExcalidrawElement[];
   versionNonce: number | undefined;
   selectionNonce: number | undefined;
+  scale: number;
   appState: InteractiveCanvasAppState;
   renderInteractiveSceneCallback: (
     data: RenderInteractiveSceneCallback,
@@ -83,9 +85,10 @@ const InteractiveCanvas = (props: InteractiveCanvasProps) => {
 
     renderInteractiveScene(
       {
-        scale: window.devicePixelRatio,
-        elements: props.elements,
         canvas: props.canvas,
+        elements: props.elements,
+        visibleElements: props.visibleElements,
+        scale: window.devicePixelRatio,
         appState: props.appState,
         renderConfig: {
           remotePointerViewportCoords: pointerViewportCoords,
@@ -117,8 +120,8 @@ const InteractiveCanvas = (props: InteractiveCanvasProps) => {
           ? CURSOR_TYPE.GRAB
           : CURSOR_TYPE.AUTO,
       }}
-      width={props.appState.width * window.devicePixelRatio}
+      width={props.appState.width * props.scale}
-      height={props.appState.height * window.devicePixelRatio}
+      height={props.appState.height * props.scale}
       ref={props.handleCanvasRef}
       onContextMenu={props.onContextMenu}
       onPointerMove={props.onPointerMove}
@@ -133,7 +136,7 @@ const InteractiveCanvas = (props: InteractiveCanvasProps) => {
   );
 };
 
-const stripIrrelevantAppStateProps = (
+const getRelevantAppStateProps = (
   appState: AppState,
 ): Omit<InteractiveCanvasAppState, "editingElement"> => ({
   zoom: appState.zoom,
@@ -167,10 +170,11 @@ const areEqual = (
   prevProps: InteractiveCanvasProps,
   nextProps: InteractiveCanvasProps,
 ) => {
-  // This could be further optimised if needed, as we don't have to render interactive canvas on each mutation
+  // This could be further optimised if needed, as we don't have to render interactive canvas on each scene mutation
   if (
     prevProps.selectionNonce !== nextProps.selectionNonce ||
-    prevProps.versionNonce !== nextProps.versionNonce
+    prevProps.versionNonce !== nextProps.versionNonce ||
+    prevProps.scale !== nextProps.scale
   ) {
     return false;
   }
@@ -179,8 +183,8 @@ const areEqual = (
   return isShallowEqual(
     // asserting AppState because we're being passed the whole AppState
     // but resolve to only the InteractiveCanvas-relevant props
-    stripIrrelevantAppStateProps(prevProps.appState as AppState),
+    getRelevantAppStateProps(prevProps.appState as AppState),
-    stripIrrelevantAppStateProps(nextProps.appState as AppState),
+    getRelevantAppStateProps(nextProps.appState as AppState),
   );
 };
 

src/components/canvases/StaticCanvas.tsx

@@ -10,8 +10,10 @@ type StaticCanvasProps = {
   canvas: HTMLCanvasElement | null;
   rc: RoughCanvas | null;
   elements: readonly NonDeletedExcalidrawElement[];
+  visibleElements: readonly NonDeletedExcalidrawElement[];
   versionNonce: number | undefined;
   selectionNonce: number | undefined;
+  scale: number;
   appState: StaticCanvasAppState;
   renderConfig: StaticCanvasRenderConfig;
   handleCanvasRef: (canvas: HTMLCanvasElement) => void;
@@ -27,10 +29,11 @@ const StaticCanvas = (props: StaticCanvasProps) => {
     }
     renderStaticScene(
       {
-        scale: window.devicePixelRatio,
-        elements: props.elements,
         canvas: props.canvas,
         rc: props.rc!,
+        scale: props.scale,
+        elements: props.elements,
+        visibleElements: props.visibleElements,
         appState: props.appState,
         renderConfig: props.renderConfig,
       },
@@ -51,14 +54,14 @@ const StaticCanvas = (props: StaticCanvasProps) => {
         height: props.appState.height,
         pointerEvents: "none",
       }}
-      width={props.appState.width * window.devicePixelRatio}
+      width={props.appState.width * props.scale}
-      height={props.appState.height * window.devicePixelRatio}
+      height={props.appState.height * props.scale}
       ref={props.handleCanvasRef}
     />
   );
 };
 
-const stripIrrelevantAppStateProps = (
+const getRelevantAppStateProps = (
   appState: AppState,
 ): Omit<
   StaticCanvasAppState,
@@ -89,15 +92,18 @@ const areEqual = (
   prevProps: StaticCanvasProps,
   nextProps: StaticCanvasProps,
 ) => {
-  if (prevProps.versionNonce !== nextProps.versionNonce) {
+  if (
+    prevProps.versionNonce !== nextProps.versionNonce ||
+    prevProps.scale !== nextProps.scale
+  ) {
     return false;
   }
 
   return isShallowEqual(
     // asserting AppState because we're being passed the whole AppState
-    // but resolve to only the InteractiveCanvas-relevant props
+    // but resolve to only the StaticCanvas-relevant props
-    stripIrrelevantAppStateProps(prevProps.appState as AppState),
+    getRelevantAppStateProps(prevProps.appState as AppState),
-    stripIrrelevantAppStateProps(nextProps.appState as AppState),
+    getRelevantAppStateProps(nextProps.appState as AppState),
   );
 };
 

src/element/sizeHelpers.ts

@@ -2,7 +2,9 @@ import { ExcalidrawElement } from "./types";
 import { mutateElement } from "./mutateElement";
 import { isFreeDrawElement, isLinearElement } from "./typeChecks";
 import { SHIFT_LOCKING_ANGLE } from "../constants";
-import { AppState } from "../types";
+import { AppState, Zoom } from "../types";
+import { getElementBounds } from "./bounds";
+import { viewportCoordsToSceneCoords } from "../utils";
 
 export const isInvisiblySmallElement = (
   element: ExcalidrawElement,
@@ -13,6 +15,42 @@ export const isInvisiblySmallElement = (
   return element.width === 0 && element.height === 0;
 };
 
+export const isVisibleElement = (
+  element: ExcalidrawElement,
+  width: number,
+  height: number,
+  viewTransformations: {
+    zoom: Zoom;
+    offsetLeft: number;
+    offsetTop: number;
+    scrollX: number;
+    scrollY: number;
+  },
+) => {
+  const [x1, y1, x2, y2] = getElementBounds(element); // scene coordinates
+  const topLeftSceneCoords = viewportCoordsToSceneCoords(
+    {
+      clientX: viewTransformations.offsetLeft,
+      clientY: viewTransformations.offsetTop,
+    },
+    viewTransformations,
+  );
+  const bottomRightSceneCoords = viewportCoordsToSceneCoords(
+    {
+      clientX: viewTransformations.offsetLeft + width,
+      clientY: viewTransformations.offsetTop + height,
+    },
+    viewTransformations,
+  );
+
+  return (
+    topLeftSceneCoords.x <= x2 &&
+    topLeftSceneCoords.y <= y2 &&
+    bottomRightSceneCoords.x >= x1 &&
+    bottomRightSceneCoords.y >= y1
+  );
+};
+
 /**
  * Makes a perfect shape or diagonal/horizontal/vertical line
  */

src/hooks/useMutatedElements.ts

@@ -1,17 +1,13 @@
 import Scene from "../scene/Scene";
 import { useMemo } from "react";
-import { InteractiveCanvasAppState, StaticCanvasAppState } from "../types";
 import { isImageElement } from "../element/typeChecks";
 import { NonDeletedExcalidrawElement } from "../element/types";
+import { CommonCanvasAppState } from "../types";
 
-export const useMutatedElements = ({
+export const useCanvasElements = (
-  appState,
+  appState: CommonCanvasAppState,
-  scene,
+  scene: Scene,
-}: {
+): readonly NonDeletedExcalidrawElement[] => {
-  appState: InteractiveCanvasAppState | StaticCanvasAppState;
-  scene: Scene;
-}): [readonly NonDeletedExcalidrawElement[], number | undefined] => {
-  const versionNonce = scene.getVersionNonce();
   const nonDeletedElements = scene.getNonDeletedElements();
 
   const elements = useMemo(() => {
@@ -38,5 +34,5 @@ export const useMutatedElements = ({
     appState.pendingImageElementId,
   ]);
 
-  return [elements, versionNonce];
+  return elements;
 };

src/hooks/useVisibleElements.ts

@@ -0,0 +1,39 @@
+import { useMemo } from "react";
+import { CommonCanvasAppState } from "../types";
+import { NonDeletedExcalidrawElement } from "../element/types";
+import { isVisibleElement } from "../element/sizeHelpers";
+
+export const useVisibleCanvasElements = (
+  appState: CommonCanvasAppState,
+  elements: readonly NonDeletedExcalidrawElement[],
+): readonly NonDeletedExcalidrawElement[] => {
+  const visibleElements = useMemo(() => {
+    const viewTransformations = {
+      zoom: appState.zoom,
+      offsetLeft: appState.offsetLeft,
+      offsetTop: appState.offsetTop,
+      scrollX: appState.scrollX,
+      scrollY: appState.scrollY,
+    };
+
+    return elements.filter((element) =>
+      isVisibleElement(
+        element,
+        appState.width,
+        appState.height,
+        viewTransformations,
+      ),
+    );
+  }, [
+    appState.offsetLeft,
+    appState.offsetTop,
+    appState.scrollX,
+    appState.scrollY,
+    appState.height,
+    appState.width,
+    appState.zoom,
+    elements,
+  ]);
+
+  return visibleElements;
+};

src/renderer/renderScene.ts

@@ -6,7 +6,6 @@ import {
   StaticCanvasAppState,
   BinaryFiles,
   Point,
-  Zoom,
   CommonCanvasAppState,
 } from "../types";
 import {
@@ -23,7 +22,6 @@ import {
   OMIT_SIDES_FOR_MULTIPLE_ELEMENTS,
   getTransformHandlesFromCoords,
   getTransformHandles,
-  getElementBounds,
   getCommonBounds,
 } from "../element";
 
@@ -62,11 +60,7 @@ import {
   TransformHandles,
   TransformHandleType,
 } from "../element/transformHandles";
-import {
+import { throttleRAF, isOnlyExportingSingleFrame } from "../utils";
-  viewportCoordsToSceneCoords,
-  throttleRAF,
-  isOnlyExportingSingleFrame,
-} from "../utils";
 import { UserIdleState } from "../types";
 import { FRAME_STYLE, THEME_FILTER } from "../constants";
 import {
@@ -418,10 +412,11 @@ const bootstrapCanvas = ({
 };
 
 const _renderInteractiveScene = ({
-  elements,
-  appState,
-  scale,
   canvas,
+  elements,
+  visibleElements,
+  scale,
+  appState,
   renderConfig,
 }: InteractiveSceneRenderConfig) => {
   if (canvas === null) {
@@ -444,17 +439,6 @@ const _renderInteractiveScene = ({
   let editingLinearElement: NonDeleted<ExcalidrawLinearElement> | undefined =
     undefined;
 
-  // FIXME I: memo?
-  const visibleElements = elements.filter((element) =>
-    isVisibleElement(element, normalizedWidth, normalizedHeight, {
-      zoom: appState.zoom,
-      offsetLeft: appState.offsetLeft,
-      offsetTop: appState.offsetTop,
-      scrollX: appState.scrollX,
-      scrollY: appState.scrollY,
-    }),
-  );
-
   visibleElements.forEach((element) => {
     // Getting the element using LinearElementEditor during collab mismatches version - being one head of visible elements due to
     // ShapeCache returns empty hence making sure that we get the
@@ -867,11 +851,12 @@ const _renderInteractiveScene = ({
 };
 
 const _renderStaticScene = ({
-  elements,
-  appState,
-  scale,
-  rc,
   canvas,
+  rc,
+  elements,
+  visibleElements,
+  scale,
+  appState,
   renderConfig,
 }: StaticSceneRenderConfig) => {
   if (canvas === null) {
@@ -910,17 +895,6 @@ const _renderStaticScene = ({
     );
   }
 
-  // Paint visible elements
-  const visibleElements = elements.filter((element) =>
-    isVisibleElement(element, normalizedWidth, normalizedHeight, {
-      zoom: appState.zoom,
-      offsetLeft: appState.offsetLeft,
-      offsetTop: appState.offsetTop,
-      scrollX: appState.scrollX,
-      scrollY: appState.scrollY,
-    }),
-  );
-
   const groupsToBeAddedToFrame = new Set<string>();
 
   visibleElements.forEach((element) => {
@@ -937,6 +911,7 @@ const _renderStaticScene = ({
     }
   });
 
+  // Paint visible elements
   visibleElements.forEach((element) => {
     try {
       // - when exporting the whole canvas, we DO NOT apply clipping
@@ -1353,42 +1328,6 @@ const renderLinkIcon = (
   }
 };
 
-export const isVisibleElement = (
-  element: ExcalidrawElement,
-  canvasWidth: number,
-  canvasHeight: number,
-  viewTransformations: {
-    zoom: Zoom;
-    offsetLeft: number;
-    offsetTop: number;
-    scrollX: number;
-    scrollY: number;
-  },
-) => {
-  const [x1, y1, x2, y2] = getElementBounds(element); // scene coordinates
-  const topLeftSceneCoords = viewportCoordsToSceneCoords(
-    {
-      clientX: viewTransformations.offsetLeft,
-      clientY: viewTransformations.offsetTop,
-    },
-    viewTransformations,
-  );
-  const bottomRightSceneCoords = viewportCoordsToSceneCoords(
-    {
-      clientX: viewTransformations.offsetLeft + canvasWidth,
-      clientY: viewTransformations.offsetTop + canvasHeight,
-    },
-    viewTransformations,
-  );
-
-  return (
-    topLeftSceneCoords.x <= x2 &&
-    topLeftSceneCoords.y <= y2 &&
-    bottomRightSceneCoords.x >= x1 &&
-    bottomRightSceneCoords.y >= y1
-  );
-};
-
 // This should be only called for exporting purposes
 export const renderSceneToSvg = (
   elements: readonly NonDeletedExcalidrawElement[],

src/scene/export.ts

@@ -12,6 +12,7 @@ import {
   updateImageCache,
 } from "../element/image";
 import Scene from "./Scene";
+import { isVisibleElement } from "../element/sizeHelpers";
 
 export const SVG_EXPORT_TAG = `<!-- svg-source:excalidraw -->`;
 
@@ -54,8 +55,29 @@ export const exportToCanvas = async (
 
   const onlyExportingSingleFrame = isOnlyExportingSingleFrame(elements);
 
+  const viewTransformations = {
+    zoom: appState.zoom,
+    offsetLeft: appState.offsetLeft,
+    offsetTop: appState.offsetTop,
+    scrollX: appState.scrollX,
+    scrollY: appState.scrollY,
+  };
+
+  const visibleElements = elements.filter((element) =>
+    isVisibleElement(
+      element,
+      appState.width,
+      appState.height,
+      viewTransformations,
+    ),
+  );
+
   renderStaticScene({
+    canvas,
+    rc: rough.canvas(canvas),
     elements,
+    visibleElements,
+    scale,
     appState: {
       ...appState,
       scrollX: -minX + (onlyExportingSingleFrame ? 0 : exportPadding),
@@ -64,9 +86,6 @@ export const exportToCanvas = async (
       shouldCacheIgnoreZoom: false,
       theme: appState.exportWithDarkMode ? "dark" : "light",
     },
-    scale,
-    rc: rough.canvas(canvas),
-    canvas,
     renderConfig: {
       imageCache,
       renderGrid: false,

src/scene/types.ts

@@ -40,17 +40,19 @@ export type RenderInteractiveSceneCallback = {
 };
 
 export type StaticSceneRenderConfig = {
-  elements: readonly NonDeletedExcalidrawElement[];
-  scale: number;
   canvas: HTMLCanvasElement | null;
   rc: RoughCanvas;
+  elements: readonly NonDeletedExcalidrawElement[];
+  visibleElements: readonly NonDeletedExcalidrawElement[];
+  scale: number;
   appState: StaticCanvasAppState;
   renderConfig: StaticCanvasRenderConfig;
 };
 
 export type InteractiveSceneRenderConfig = {
-  elements: readonly NonDeletedExcalidrawElement[];
   canvas: HTMLCanvasElement | null;
+  elements: readonly NonDeletedExcalidrawElement[];
+  visibleElements: readonly NonDeletedExcalidrawElement[];
   scale: number;
   appState: InteractiveCanvasAppState;
   renderConfig: InteractiveCanvasRenderConfig;

src/tests/__snapshots__/linearElementEditor.test.tsx.snap

@@ -5,7 +5,7 @@ exports[`Test Linear Elements Test bound text element should match styles for te
   class="excalidraw-wysiwyg"
   data-type="wysiwyg"
   dir="auto"
-  style="position: absolute; display: inline-block; min-height: 1em; margin: 0px; padding: 0px; border: 0px; outline: 0; resize: none; background: transparent; overflow: hidden; z-index: var(--zIndex-wysiwyg); word-break: break-word; white-space: pre-wrap; overflow-wrap: break-word; box-sizing: content-box; width: 10.5px; height: 25px; left: 35px; top: 7.5px; transform: translate(0px, 0px) scale(1) rotate(0deg); text-align: center; vertical-align: middle; color: rgb(30, 30, 30); opacity: 1; filter: var(--theme-filter); max-height: -7.5px; font: Emoji 20px 20px; line-height: 1.25; font-family: Virgil, Segoe UI Emoji;"
+  style="position: absolute; display: inline-block; min-height: 1em; margin: 0px; padding: 0px; border: 0px; outline: 0; resize: none; background: transparent; overflow: hidden; z-index: var(--zIndex-wysiwyg); word-break: break-word; white-space: pre-wrap; overflow-wrap: break-word; box-sizing: content-box; width: 10.5px; height: 25px; left: 35px; top: 7.5px; transform: translate(0px, 0px) scale(1) rotate(0deg); text-align: center; vertical-align: middle; color: rgb(30, 30, 30); opacity: 1; filter: var(--theme-filter); max-height: 992.5px; font: Emoji 20px 20px; line-height: 1.25; font-family: Virgil, Segoe UI Emoji;"
   tabindex="0"
   wrap="off"
 />

src/tests/linearElementEditor.test.tsx

@@ -25,7 +25,6 @@ import {
 import * as textElementUtils from "../element/textElement";
 import { ROUNDNESS, VERTICAL_ALIGN } from "../constants";
 
-// FIXME I: add specific tests for render of both components (when they should both, when just one, when just second, what is the order of renders - first static, then interactive, etc., all tests)
 const renderInteractiveScene = jest.spyOn(Renderer, "renderInteractiveScene");
 const renderStaticScene = jest.spyOn(Renderer, "renderStaticScene");
 
@@ -34,7 +33,6 @@ const font = "20px Cascadia, width: Segoe UI Emoji" as FontString;
 
 describe("Test Linear Elements", () => {
   let container: HTMLElement;
-  let canvas: HTMLCanvasElement;
   let interactiveCanvas: HTMLCanvasElement;
 
   beforeEach(async () => {
@@ -45,13 +43,10 @@ describe("Test Linear Elements", () => {
     renderStaticScene.mockClear();
     reseed(7);
     const comp = await render(<ExcalidrawApp />);
+    h.state.width = 1000;
+    h.state.height = 1000;
     container = comp.container;
-    canvas = container.querySelector("canvas.static")!;
-    canvas.width = 1000;
-    canvas.height = 1000;
     interactiveCanvas = container.querySelector("canvas.interactive")!;
-    interactiveCanvas.width = 1000;
-    interactiveCanvas.height = 1000;
   });
 
   const p1: Point = [20, 20];