fix: make bounds independent of scene (excalidraw#7679)

* fix: make bounds independent of scene

* pass only elements to getCommonBounds

* lint

* pass elementsMap to getVisibleAndNonSelectedElements

packages/excalidraw/components/App.tsx

@@ -953,6 +953,7 @@ class App extends React.Component<AppProps, AppState> {
             normalizedWidth,
             normalizedHeight,
             this.state,
+            this.scene.getNonDeletedElementsMap(),
           );
           const hasBeenInitialized = this.initializedEmbeds.has(el.id);
 
@@ -1287,6 +1288,7 @@ class App extends React.Component<AppProps, AppState> {
             scrollY: this.state.scrollY,
             zoom: this.state.zoom,
           },
+          this.scene.getNonDeletedElementsMap(),
         )
       ) {
         // if frame not visible, don't render its name

packages/excalidraw/element/bounds.test.ts

@@ -62,37 +62,58 @@ describe("getElementAbsoluteCoords", () => {
 
 describe("getElementBounds", () => {
   it("rectangle", () => {
-    const [x1, y1, x2, y2] = getElementBounds(
-      _ce({ x: 40, y: 30, w: 20, h: 10, a: Math.PI / 4, t: "rectangle" }),
-    );
+    const element = _ce({
+      x: 40,
+      y: 30,
+      w: 20,
+      h: 10,
+      a: Math.PI / 4,
+      t: "rectangle",
+    });
+    const [x1, y1, x2, y2] = getElementBounds(element, arrayToMap([element]));
     expect(x1).toEqual(39.39339828220179);
     expect(y1).toEqual(24.393398282201787);
     expect(x2).toEqual(60.60660171779821);
     expect(y2).toEqual(45.60660171779821);
   });
 
   it("diamond", () => {
-    const [x1, y1, x2, y2] = getElementBounds(
-      _ce({ x: 40, y: 30, w: 20, h: 10, a: Math.PI / 4, t: "diamond" }),
-    );
+    const element = _ce({
+      x: 40,
+      y: 30,
+      w: 20,
+      h: 10,
+      a: Math.PI / 4,
+      t: "diamond",
+    });
+
+    const [x1, y1, x2, y2] = getElementBounds(element, arrayToMap([element]));
+
     expect(x1).toEqual(42.928932188134524);
     expect(y1).toEqual(27.928932188134524);
     expect(x2).toEqual(57.071067811865476);
     expect(y2).toEqual(42.071067811865476);
   });
 
   it("ellipse", () => {
-    const [x1, y1, x2, y2] = getElementBounds(
-      _ce({ x: 40, y: 30, w: 20, h: 10, a: Math.PI / 4, t: "ellipse" }),
-    );
+    const element = _ce({
+      x: 40,
+      y: 30,
+      w: 20,
+      h: 10,
+      a: Math.PI / 4,
+      t: "ellipse",
+    });
+
+    const [x1, y1, x2, y2] = getElementBounds(element, arrayToMap([element]));
     expect(x1).toEqual(42.09430584957905);
     expect(y1).toEqual(27.09430584957905);
     expect(x2).toEqual(57.90569415042095);
     expect(y2).toEqual(42.90569415042095);
   });
 
   it("curved line", () => {
-    const [x1, y1, x2, y2] = getElementBounds({
+    const element = {
       ..._ce({
         t: "line",
         x: 449.58203125,
@@ -106,7 +127,9 @@ describe("getElementBounds", () => {
         [67.33984375, 92.48828125] as [number, number],
         [-102.7890625, 52.15625] as [number, number],
       ],
-    } as ExcalidrawLinearElement);
+    } as ExcalidrawLinearElement;
+
+    const [x1, y1, x2, y2] = getElementBounds(element, arrayToMap([element]));
     expect(x1).toEqual(360.3176068760539);
     expect(y1).toEqual(185.90654264413516);
     expect(x2).toEqual(480.87005902729743);

packages/excalidraw/element/bounds.ts

@@ -5,7 +5,6 @@ import {
   ExcalidrawFreeDrawElement,
   NonDeleted,
   ExcalidrawTextElementWithContainer,
-  ElementsMapOrArray,
   ElementsMap,
 } from "./types";
 import { distance2d, rotate, rotatePoint } from "../math";
@@ -25,7 +24,7 @@ import { getBoundTextElement, getContainerElement } from "./textElement";
 import { LinearElementEditor } from "./linearElementEditor";
 import { Mutable } from "../utility-types";
 import { ShapeCache } from "../scene/ShapeCache";
-import Scene from "../scene/Scene";
+import { arrayToMap } from "../utils";
 
 export type RectangleBox = {
   x: number;
@@ -63,7 +62,7 @@ export class ElementBounds {
     }
   >();
 
-  static getBounds(element: ExcalidrawElement) {
+  static getBounds(element: ExcalidrawElement, elementsMap: ElementsMap) {
     const cachedBounds = ElementBounds.boundsCache.get(element);
 
     if (
@@ -75,23 +74,12 @@ export class ElementBounds {
     ) {
       return cachedBounds.bounds;
     }
-    const scene = Scene.getScene(element);
-    const bounds = ElementBounds.calculateBounds(
-      element,
-      scene?.getNonDeletedElementsMap() || new Map(),
-    );
+    const bounds = ElementBounds.calculateBounds(element, elementsMap);
 
-    // hack to ensure that downstream checks could retrieve element Scene
-    // so as to have correctly calculated bounds
-    // FIXME remove when we get rid of all the id:Scene / element:Scene mapping
-    const shouldCache = !!scene;
-
-    if (shouldCache) {
-      ElementBounds.boundsCache.set(element, {
-        version: element.version,
-        bounds,
-      });
-    }
+    ElementBounds.boundsCache.set(element, {
+      version: element.version,
+      bounds,
+    });
 
     return bounds;
   }
@@ -748,11 +736,17 @@ const getLinearElementRotatedBounds = (
   return coords;
 };
 
-export const getElementBounds = (element: ExcalidrawElement): Bounds => {
-  return ElementBounds.getBounds(element);
+export const getElementBounds = (
+  element: ExcalidrawElement,
+  elementsMap: ElementsMap,
+): Bounds => {
+  return ElementBounds.getBounds(element, elementsMap);
 };
-export const getCommonBounds = (elements: ElementsMapOrArray): Bounds => {
-  if ("size" in elements ? !elements.size : !elements.length) {
+
+export const getCommonBounds = (
+  elements: readonly ExcalidrawElement[],
+): Bounds => {
+  if (!elements.length) {
     return [0, 0, 0, 0];
   }
 
@@ -761,8 +755,10 @@ export const getCommonBounds = (elements: ElementsMapOrArray): Bounds => {
   let minY = Infinity;
   let maxY = -Infinity;
 
+  const elementsMap = arrayToMap(elements);
+
   elements.forEach((element) => {
-    const [x1, y1, x2, y2] = getElementBounds(element);
+    const [x1, y1, x2, y2] = getElementBounds(element, elementsMap);
     minX = Math.min(minX, x1);
     minY = Math.min(minY, y1);
     maxX = Math.max(maxX, x2);
@@ -868,9 +864,9 @@ export const getClosestElementBounds = (
 
   let minDistance = Infinity;
   let closestElement = elements[0];
-
+  const elementsMap = arrayToMap(elements);
   elements.forEach((element) => {
-    const [x1, y1, x2, y2] = getElementBounds(element);
+    const [x1, y1, x2, y2] = getElementBounds(element, elementsMap);
     const distance = distance2d((x1 + x2) / 2, (y1 + y2) / 2, from.x, from.y);
 
     if (distance < minDistance) {
@@ -879,7 +875,7 @@ export const getClosestElementBounds = (
     }
   });
 
-  return getElementBounds(closestElement);
+  return getElementBounds(closestElement, elementsMap);
 };
 
 export interface BoundingBox {

packages/excalidraw/element/sizeHelpers.ts

@@ -1,4 +1,4 @@
-import { ExcalidrawElement } from "./types";
+import { ElementsMap, ExcalidrawElement } from "./types";
 import { mutateElement } from "./mutateElement";
 import { isFreeDrawElement, isLinearElement } from "./typeChecks";
 import { SHIFT_LOCKING_ANGLE } from "../constants";
@@ -26,8 +26,9 @@ export const isElementInViewport = (
     scrollX: number;
     scrollY: number;
   },
+  elementsMap: ElementsMap,
 ) => {
-  const [x1, y1, x2, y2] = getElementBounds(element); // scene coordinates
+  const [x1, y1, x2, y2] = getElementBounds(element, elementsMap); // scene coordinates
   const topLeftSceneCoords = viewportCoordsToSceneCoords(
     {
       clientX: viewTransformations.offsetLeft,

packages/excalidraw/renderer/renderScene.ts

@@ -886,7 +886,7 @@ const _renderInteractiveScene = ({
   let scrollBars;
   if (renderConfig.renderScrollbars) {
     scrollBars = getScrollBars(
-      elementsMap,
+      visibleElements,
       normalizedWidth,
       normalizedHeight,
       appState,

packages/excalidraw/scene/Renderer.ts

@@ -40,13 +40,19 @@ export class Renderer {
       const visibleElements: NonDeletedExcalidrawElement[] = [];
       for (const element of elementsMap.values()) {
         if (
-          isElementInViewport(element, width, height, {
-            zoom,
-            offsetLeft,
-            offsetTop,
-            scrollX,
-            scrollY,
-          })
+          isElementInViewport(
+            element,
+            width,
+            height,
+            {
+              zoom,
+              offsetLeft,
+              offsetTop,
+              scrollX,
+              scrollY,
+            },
+            elementsMap,
+          )
         ) {
           visibleElements.push(element);
         }

packages/excalidraw/scene/scrollbars.ts

@@ -1,20 +1,21 @@
 import { getCommonBounds } from "../element";
 import { InteractiveCanvasAppState } from "../types";
-import { RenderableElementsMap, ScrollBars } from "./types";
+import { ScrollBars } from "./types";
 import { getGlobalCSSVariable } from "../utils";
 import { getLanguage } from "../i18n";
+import { ExcalidrawElement } from "../element/types";
 
 export const SCROLLBAR_MARGIN = 4;
 export const SCROLLBAR_WIDTH = 6;
 export const SCROLLBAR_COLOR = "rgba(0,0,0,0.3)";
 
 export const getScrollBars = (
-  elements: RenderableElementsMap,
+  elements: readonly ExcalidrawElement[],
   viewportWidth: number,
   viewportHeight: number,
   appState: InteractiveCanvasAppState,
 ): ScrollBars => {
-  if (!elements.size) {
+  if (!elements.length) {
     return {
       horizontal: null,
       vertical: null,

packages/excalidraw/scene/selection.ts

@@ -52,12 +52,17 @@ export const getElementsWithinSelection = (
     getElementAbsoluteCoords(selection, elementsMap);
 
   let elementsInSelection = elements.filter((element) => {
-    let [elementX1, elementY1, elementX2, elementY2] =
-      getElementBounds(element);
+    let [elementX1, elementY1, elementX2, elementY2] = getElementBounds(
+      element,
+      elementsMap,
+    );
 
     const containingFrame = getContainingFrame(element);
     if (containingFrame) {
-      const [fx1, fy1, fx2, fy2] = getElementBounds(containingFrame);
+      const [fx1, fy1, fx2, fy2] = getElementBounds(
+        containingFrame,
+        elementsMap,
+      );
 
       elementX1 = Math.max(fx1, elementX1);
       elementY1 = Math.max(fy1, elementY1);
@@ -97,6 +102,7 @@ export const getVisibleAndNonSelectedElements = (
   elements: readonly NonDeletedExcalidrawElement[],
   selectedElements: readonly NonDeletedExcalidrawElement[],
   appState: AppState,
+  elementsMap: ElementsMap,
 ) => {
   const selectedElementsSet = new Set(
     selectedElements.map((element) => element.id),
@@ -107,6 +113,7 @@ export const getVisibleAndNonSelectedElements = (
       appState.width,
       appState.height,
       appState,
+      elementsMap,
     );
 
     return !selectedElementsSet.has(element.id) && isVisible;

packages/excalidraw/snapping.ts

@@ -269,6 +269,7 @@ const getReferenceElements = (
   elements: readonly NonDeletedExcalidrawElement[],
   selectedElements: NonDeletedExcalidrawElement[],
   appState: AppState,
+  elementsMap: ElementsMap,
 ) => {
   const selectedFrames = selectedElements
     .filter((element) => isFrameLikeElement(element))
@@ -278,6 +279,7 @@ const getReferenceElements = (
     elements,
     selectedElements,
     appState,
+    elementsMap,
   ).filter(
     (element) => !(element.frameId && selectedFrames.includes(element.frameId)),
   );
@@ -293,6 +295,7 @@ export const getVisibleGaps = (
     elements,
     selectedElements,
     appState,
+    elementsMap,
   );
 
   const referenceBounds = getMaximumGroups(referenceElements, elementsMap)
@@ -580,6 +583,7 @@ export const getReferenceSnapPoints = (
     elements,
     selectedElements,
     appState,
+    elementsMap,
   );
   return getMaximumGroups(referenceElements, elementsMap)
     .filter(
@@ -1296,6 +1300,7 @@ export const getSnapLinesAtPointer = (
     elements,
     [],
     appState,
+    elementsMap,
   );
 
   const snapDistance = getSnapDistance(appState.zoom.value);