refactor: cleanup renderScene (#5573)

* refactor: cleanup renderScene

* pass object instead of individual params

src/components/App.tsx

@@ -1165,7 +1165,23 @@ class App extends React.Component<AppProps, AppState> {
         ),
       );
     }
+    this.renderScene();
+    this.history.record(this.state, this.scene.getElementsIncludingDeleted());
 
+    // Do not notify consumers if we're still loading the scene. Among other
+    // potential issues, this fixes a case where the tab isn't focused during
+    // init, which would trigger onChange with empty elements, which would then
+    // override whatever is in localStorage currently.
+    if (!this.state.isLoading) {
+      this.props.onChange?.(
+        this.scene.getElementsIncludingDeleted(),
+        this.state,
+        this.files,
+      );
+    }
+  }
+
+  private renderScene = () => {
     const cursorButton: {
       [id: string]: string | undefined;
     } = {};
@@ -1202,6 +1218,7 @@ class App extends React.Component<AppProps, AppState> {
       );
       cursorButton[socketId] = user.button;
     });
+
     const renderingElements = this.scene
       .getNonDeletedElements()
       .filter((element) => {
@@ -1223,13 +1240,13 @@ class App extends React.Component<AppProps, AppState> {
       });
 
     renderScene(
-      renderingElements,
-      this.state,
-      this.state.selectionElement,
-      window.devicePixelRatio,
-      this.rc!,
-      this.canvas!,
       {
+        elements: renderingElements,
+        appState: this.state,
+        scale: window.devicePixelRatio,
+        rc: this.rc!,
+        canvas: this.canvas!,
+        renderConfig: {
           scrollX: this.state.scrollX,
           scrollY: this.state.scrollY,
           viewBackgroundColor: this.state.viewBackgroundColor,
@@ -1245,7 +1262,7 @@ class App extends React.Component<AppProps, AppState> {
           isExporting: false,
           renderScrollbars: !this.device.isMobile,
         },
-      ({ atLeastOneVisibleElement, scrollBars }) => {
+        callback: ({ atLeastOneVisibleElement, scrollBars }) => {
           if (scrollBars) {
             currentScrollBars = scrollBars;
           }
@@ -1260,27 +1277,14 @@ class App extends React.Component<AppProps, AppState> {
 
           this.scheduleImageRefresh();
         },
+      },
       THROTTLE_NEXT_RENDER && window.EXCALIDRAW_THROTTLE_RENDER === true,
     );
 
     if (!THROTTLE_NEXT_RENDER) {
       THROTTLE_NEXT_RENDER = true;
     }
-
-    this.history.record(this.state, this.scene.getElementsIncludingDeleted());
-
-    // Do not notify consumers if we're still loading the scene. Among other
-    // potential issues, this fixes a case where the tab isn't focused during
-    // init, which would trigger onChange with empty elements, which would then
-    // override whatever is in localStorage currently.
-    if (!this.state.isLoading) {
-      this.props.onChange?.(
-        this.scene.getElementsIncludingDeleted(),
-        this.state,
-        this.files,
-      );
-    }
-  }
+  };
 
   private onScroll = debounce(() => {
     const { offsetTop, offsetLeft } = this.getCanvasOffsets();

src/renderer/renderScene.ts

@@ -284,20 +284,26 @@ const renderLinearElementPointHighlight = (
   context.restore();
 };
 
-export const _renderScene = (
-  elements: readonly NonDeletedExcalidrawElement[],
-  appState: AppState,
-  selectionElement: NonDeletedExcalidrawElement | null,
-  scale: number,
-  rc: RoughCanvas,
-  canvas: HTMLCanvasElement,
-  renderConfig: RenderConfig,
+export const _renderScene = ({
+  elements,
+  appState,
+  scale,
+  rc,
+  canvas,
+  renderConfig,
+}: {
+  elements: readonly NonDeletedExcalidrawElement[];
+  appState: AppState;
+  scale: number;
+  rc: RoughCanvas;
+  canvas: HTMLCanvasElement;
+  renderConfig: RenderConfig;
+}) =>
   // extra options passed to the renderer
-) => {
+  {
     if (canvas === null) {
       return { atLeastOneVisibleElement: false };
     }
-
     const {
       renderScrollbars = true,
       renderSelection = true,
@@ -389,9 +395,9 @@ export const _renderScene = (
     }
 
     // Paint selection element
-  if (selectionElement) {
+    if (appState.selectionElement) {
       try {
-      renderElement(selectionElement, rc, context, renderConfig);
+        renderElement(appState.selectionElement, rc, context, renderConfig);
       } catch (error: any) {
         console.error(error);
       }
@@ -503,7 +509,9 @@ export const _renderScene = (
             context,
             renderConfig,
             selection,
-          isSingleLinearElementSelected ? DEFAULT_SPACING * 2 : DEFAULT_SPACING,
+            isSingleLinearElementSelected
+              ? DEFAULT_SPACING * 2
+              : DEFAULT_SPACING,
           ),
         );
       }
@@ -706,70 +714,45 @@ export const _renderScene = (
 
     context.restore();
     return { atLeastOneVisibleElement: visibleElements.length > 0, scrollBars };
-};
+  };
 
 const renderSceneThrottled = throttleRAF(
-  (
-    elements: readonly NonDeletedExcalidrawElement[],
-    appState: AppState,
-    selectionElement: NonDeletedExcalidrawElement | null,
-    scale: number,
-    rc: RoughCanvas,
-    canvas: HTMLCanvasElement,
-    renderConfig: RenderConfig,
-    callback?: (data: ReturnType<typeof _renderScene>) => void,
-  ) => {
-    const ret = _renderScene(
-      elements,
-      appState,
-      selectionElement,
-      scale,
-      rc,
-      canvas,
-      renderConfig,
-    );
-    callback?.(ret);
+  (config: {
+    elements: readonly NonDeletedExcalidrawElement[];
+    appState: AppState;
+    scale: number;
+    rc: RoughCanvas;
+    canvas: HTMLCanvasElement;
+    renderConfig: RenderConfig;
+    callback?: (data: ReturnType<typeof _renderScene>) => void;
+  }) => {
+    const ret = _renderScene(config);
+    config.callback?.(ret);
   },
   { trailing: true },
 );
 
 /** renderScene throttled to animation framerate */
 export const renderScene = <T extends boolean = false>(
-  elements: readonly NonDeletedExcalidrawElement[],
-  appState: AppState,
-  selectionElement: NonDeletedExcalidrawElement | null,
-  scale: number,
-  rc: RoughCanvas,
-  canvas: HTMLCanvasElement,
-  renderConfig: RenderConfig,
-  callback?: (data: ReturnType<typeof _renderScene>) => void,
+  config: {
+    elements: readonly NonDeletedExcalidrawElement[];
+    appState: AppState;
+    scale: number;
+    rc: RoughCanvas;
+    canvas: HTMLCanvasElement;
+    renderConfig: RenderConfig;
+    callback?: (data: ReturnType<typeof _renderScene>) => void;
+  },
   /** Whether to throttle rendering. Defaults to false.
    * When throttling, no value is returned. Use the callback instead. */
   throttle?: T,
 ): T extends true ? void : ReturnType<typeof _renderScene> => {
   if (throttle) {
-    renderSceneThrottled(
-      elements,
-      appState,
-      selectionElement,
-      scale,
-      rc,
-      canvas,
-      renderConfig,
-      callback,
-    );
+    renderSceneThrottled(config);
     return undefined as T extends true ? void : ReturnType<typeof _renderScene>;
   }
-  const ret = _renderScene(
-    elements,
-    appState,
-    selectionElement,
-    scale,
-    rc,
-    canvas,
-    renderConfig,
-  );
-  callback?.(ret);
+  const ret = _renderScene(config);
+  config.callback?.(ret);
   return ret as T extends true ? void : ReturnType<typeof _renderScene>;
 };
 

src/scene/export.ts

@@ -51,7 +51,13 @@ export const exportToCanvas = async (
     files,
   });
 
-  renderScene(elements, appState, null, scale, rough.canvas(canvas), canvas, {
+  renderScene({
+    elements,
+    appState,
+    scale,
+    rc: rough.canvas(canvas),
+    canvas,
+    renderConfig: {
       viewBackgroundColor: exportBackground ? viewBackgroundColor : null,
       scrollX: -minX + exportPadding,
       scrollY: -minY + exportPadding,
@@ -67,6 +73,7 @@ export const exportToCanvas = async (
       renderSelection: false,
       renderGrid: false,
       isExporting: true,
+    },
   });
 
   return canvas;