perf: reduce unnecessary frame clippings (#8980)

* reduce unnecessary frame clippings

* further optim

packages/excalidraw/frame.ts

@@ -95,12 +95,11 @@ export const getElementsCompletelyInFrame = (
   );
 
 export const isElementContainingFrame = (
-  elements: readonly ExcalidrawElement[],
   element: ExcalidrawElement,
   frame: ExcalidrawFrameLikeElement,
   elementsMap: ElementsMap,
 ) => {
-  return getElementsWithinSelection(elements, element, elementsMap).some(
+  return getElementsWithinSelection([frame], element, elementsMap).some(
     (e) => e.id === frame.id,
   );
 };
@@ -144,7 +143,7 @@ export const elementOverlapsWithFrame = (
   return (
     elementsAreInFrameBounds([element], frame, elementsMap) ||
     isElementIntersectingFrame(element, frame, elementsMap) ||
-    isElementContainingFrame([frame], element, frame, elementsMap)
+    isElementContainingFrame(element, frame, elementsMap)
   );
 };
 
@@ -283,7 +282,7 @@ export const getElementsInResizingFrame = (
   const elementsCompletelyInFrame = new Set([
     ...getElementsCompletelyInFrame(allElements, frame, elementsMap),
     ...prevElementsInFrame.filter((element) =>
-      isElementContainingFrame(allElements, element, frame, elementsMap),
+      isElementContainingFrame(element, frame, elementsMap),
     ),
   ]);
 
@@ -695,61 +694,150 @@ export const isElementInFrame = (
   element: ExcalidrawElement,
   allElementsMap: ElementsMap,
   appState: StaticCanvasAppState,
+  opts?: {
+    targetFrame?: ExcalidrawFrameLikeElement;
+    checkedGroups?: Map<string, boolean>;
+  },
 ) => {
-  const frame = getTargetFrame(element, allElementsMap, appState);
+  const frame =
+    opts?.targetFrame ?? getTargetFrame(element, allElementsMap, appState);
+
+  if (!frame) {
+    return false;
+  }
+
   const _element = isTextElement(element)
     ? getContainerElement(element, allElementsMap) || element
     : element;
 
-  if (frame) {
+  const setGroupsInFrame = (isInFrame: boolean) => {
-    // Perf improvement:
+    if (opts?.checkedGroups) {
-    // For an element that's already in a frame, if it's not being dragged
+      _element.groupIds.forEach((groupId) => {
-    // then there is no need to refer to geometry (which, yes, is slow) to check if it's in a frame.
+        opts.checkedGroups?.set(groupId, isInFrame);
-    // It has to be in its containing frame.
+      });
-    if (
-      !appState.selectedElementIds[element.id] ||
-      !appState.selectedElementsAreBeingDragged
-    ) {
-      return true;
     }
+  };
+
+  // Perf improvement:
+  // For an element that's already in a frame, if it's not being dragged
+  // then there is no need to refer to geometry (which, yes, is slow) to check if it's in a frame.
+  // It has to be in its containing frame.
+  if (
+    !appState.selectedElementIds[element.id] ||
+    !appState.selectedElementsAreBeingDragged
+  ) {
+    return true;
+  }
+
+  if (_element.groupIds.length === 0) {
+    return elementOverlapsWithFrame(_element, frame, allElementsMap);
+  }
 
-    if (_element.groupIds.length === 0) {
+  for (const gid of _element.groupIds) {
-      return elementOverlapsWithFrame(_element, frame, allElementsMap);
+    if (opts?.checkedGroups?.has(gid)) {
+      return opts.checkedGroups.get(gid)!!;
     }
+  }
 
-    const allElementsInGroup = new Set(
+  const allElementsInGroup = new Set(
-      _element.groupIds.flatMap((gid) =>
+    _element.groupIds
-        getElementsInGroup(allElementsMap, gid),
+      .filter((gid) => {
-      ),
+        if (opts?.checkedGroups) {
+          return !opts.checkedGroups.has(gid);
+        }
+        return true;
+      })
+      .flatMap((gid) => getElementsInGroup(allElementsMap, gid)),
+  );
+
+  if (appState.editingGroupId && appState.selectedElementsAreBeingDragged) {
+    const selectedElements = new Set(
+      getSelectedElements(allElementsMap, appState),
     );
 
-    if (appState.editingGroupId && appState.selectedElementsAreBeingDragged) {
+    const editingGroupOverlapsFrame = appState.frameToHighlight !== null;
-      const selectedElements = new Set(
-        getSelectedElements(allElementsMap, appState),
-      );
 
-      const editingGroupOverlapsFrame = appState.frameToHighlight !== null;
+    if (editingGroupOverlapsFrame) {
+      return true;
+    }
 
-      if (editingGroupOverlapsFrame) {
+    selectedElements.forEach((selectedElement) => {
-        return true;
+      allElementsInGroup.delete(selectedElement);
-      }
+    });
+  }
 
-      selectedElements.forEach((selectedElement) => {
+  for (const elementInGroup of allElementsInGroup) {
-        allElementsInGroup.delete(selectedElement);
+    if (isFrameLikeElement(elementInGroup)) {
-      });
+      setGroupsInFrame(false);
+      return false;
     }
+  }
 
-    for (const elementInGroup of allElementsInGroup) {
+  for (const elementInGroup of allElementsInGroup) {
-      if (isFrameLikeElement(elementInGroup)) {
+    if (elementOverlapsWithFrame(elementInGroup, frame, allElementsMap)) {
-        return false;
+      setGroupsInFrame(true);
-      }
+      return true;
     }
+  }
 
-    for (const elementInGroup of allElementsInGroup) {
+  return false;
-      if (elementOverlapsWithFrame(elementInGroup, frame, allElementsMap)) {
+};
-        return true;
+
+export const shouldApplyFrameClip = (
+  element: ExcalidrawElement,
+  frame: ExcalidrawFrameLikeElement,
+  appState: StaticCanvasAppState,
+  elementsMap: ElementsMap,
+  checkedGroups?: Map<string, boolean>,
+) => {
+  if (!appState.frameRendering || !appState.frameRendering.clip) {
+    return false;
+  }
+
+  // for individual elements, only clip when the element is
+  // a. overlapping with the frame, or
+  // b. containing the frame, for example when an element is used as a background
+  //    and is therefore bigger than the frame and completely contains the frame
+  const shouldClipElementItself =
+    isElementIntersectingFrame(element, frame, elementsMap) ||
+    isElementContainingFrame(element, frame, elementsMap);
+
+  if (shouldClipElementItself) {
+    for (const groupId of element.groupIds) {
+      checkedGroups?.set(groupId, true);
+    }
+
+    return true;
+  }
+
+  // if an element is outside the frame, but is part of a group that has some elements
+  // "in" the frame, we should clip the element
+  if (
+    !shouldClipElementItself &&
+    element.groupIds.length > 0 &&
+    !elementsAreInFrameBounds([element], frame, elementsMap)
+  ) {
+    let shouldClip = false;
+
+    // if no elements are being dragged, we can skip the geometry check
+    // because we know if the element is in the given frame or not
+    if (!appState.selectedElementsAreBeingDragged) {
+      shouldClip = element.frameId === frame.id;
+      for (const groupId of element.groupIds) {
+        checkedGroups?.set(groupId, shouldClip);
       }
+    } else {
+      shouldClip = isElementInFrame(element, elementsMap, appState, {
+        targetFrame: frame,
+        checkedGroups,
+      });
     }
+
+    for (const groupId of element.groupIds) {
+      checkedGroups?.set(groupId, shouldClip);
+    }
+
+    return shouldClip;
   }
 
   return false;

packages/excalidraw/renderer/staticScene.ts

@@ -4,7 +4,7 @@ import { getElementAbsoluteCoords } from "../element";
 import {
   elementOverlapsWithFrame,
   getTargetFrame,
-  isElementInFrame,
+  shouldApplyFrameClip,
 } from "../frame";
 import {
   isEmbeddableElement,
@@ -273,6 +273,8 @@ const _renderStaticScene = ({
     }
   });
 
+  const inFrameGroupsMap = new Map<string, boolean>();
+
   // Paint visible elements
   visibleElements
     .filter((el) => !isIframeLikeElement(el))
@@ -297,9 +299,16 @@ const _renderStaticScene = ({
           appState.frameRendering.clip
         ) {
           const frame = getTargetFrame(element, elementsMap, appState);
-
+          if (
-          // TODO do we need to check isElementInFrame here?
+            frame &&
-          if (frame && isElementInFrame(element, elementsMap, appState)) {
+            shouldApplyFrameClip(
+              element,
+              frame,
+              appState,
+              elementsMap,
+              inFrameGroupsMap,
+            )
+          ) {
             frameClip(frame, context, renderConfig, appState);
           }
           renderElement(
@@ -400,7 +409,16 @@ const _renderStaticScene = ({
 
           const frame = getTargetFrame(element, elementsMap, appState);
 
-          if (frame && isElementInFrame(element, elementsMap, appState)) {
+          if (
+            frame &&
+            shouldApplyFrameClip(
+              element,
+              frame,
+              appState,
+              elementsMap,
+              inFrameGroupsMap,
+            )
+          ) {
             frameClip(frame, context, renderConfig, appState);
           }
           render();