add clippings bounds tolerance, improve group related checks

src/components/App.tsx

@@ -6868,10 +6868,13 @@ class App extends React.Component<AppProps, AppState> {
               topLayerFrame &&
               !this.state.selectedElementIds[topLayerFrame.id]
             ) {
+              const processedGroupIds = new Map<string, boolean>();
               const elementsToAdd = selectedElements.filter(
                 (element) =>
                   element.frameId !== topLayerFrame.id &&
-                  isElementInFrame(element, nextElements, this.state),
+                  isElementInFrame(element, nextElements, this.state, {
+                    processedGroupIds,
+                  }),
               );
 
               if (this.state.editingGroupId) {

src/frame.ts

@@ -1,8 +1,4 @@
-import {
-  getCommonBounds,
-  getElementAbsoluteCoords,
-  isTextElement,
-} from "./element";
+import { getCommonBounds, getElementBounds, isTextElement } from "./element";
 import {
   ExcalidrawElement,
   ExcalidrawFrameElement,
@@ -56,6 +52,7 @@ export const bindElementsToFramesAfterDuplication = (
   }
 };
 
+// --------------------------- Frame Geometry ---------------------------------
 export function isElementIntersectingFrame(
   element: ExcalidrawElement,
   frame: ExcalidrawFrameElement,
@@ -85,36 +82,27 @@ export const getElementsCompletelyInFrame = (
       element.frameId === frame.id,
   );
 
-export const isElementContainingFrame = (
-  elements: readonly ExcalidrawElement[],
-  element: ExcalidrawElement,
-  frame: ExcalidrawFrameElement,
-) => {
-  return getElementsWithinSelection(elements, element).some(
-    (e) => e.id === frame.id,
-  );
-};
-
 export const getElementsIntersectingFrame = (
   elements: readonly ExcalidrawElement[],
   frame: ExcalidrawFrameElement,
 ) => elements.filter((element) => isElementIntersectingFrame(element, frame));
 
-export const elementsAreInFrameBounds = (
+export const elementsAreInBounds = (
   elements: readonly ExcalidrawElement[],
-  frame: ExcalidrawFrameElement,
+  element: ExcalidrawElement,
+  tolerance = 0,
 ) => {
-  const [selectionX1, selectionY1, selectionX2, selectionY2] =
-    getElementAbsoluteCoords(frame);
-
   const [elementX1, elementY1, elementX2, elementY2] =
+    getElementBounds(element);
+
+  const [elementsX1, elementsY1, elementsX2, elementsY2] =
     getCommonBounds(elements);
 
   return (
-    selectionX1 <= elementX1 &&
-    selectionY1 <= elementY1 &&
-    selectionX2 >= elementX2 &&
-    selectionY2 >= elementY2
+    elementX1 <= elementsX1 - tolerance &&
+    elementY1 <= elementsY1 - tolerance &&
+    elementX2 >= elementsX2 + tolerance &&
+    elementY2 >= elementsY2 + tolerance
   );
 };
 
@@ -123,9 +111,12 @@ export const elementOverlapsWithFrame = (
   frame: ExcalidrawFrameElement,
 ) => {
   return (
-    elementsAreInFrameBounds([element], frame) ||
-    isElementIntersectingFrame(element, frame) ||
-    isElementContainingFrame([frame], element, frame)
+    // frame contains element
+    elementsAreInBounds([element], frame) ||
+    // element contains frame
+    (elementsAreInBounds([frame], element) && element.frameId === frame.id) ||
+    // element intersects with frame
+    isElementIntersectingFrame(element, frame)
   );
 };
 
@@ -136,7 +127,7 @@ export const isCursorInFrame = (
   },
   frame: NonDeleted<ExcalidrawFrameElement>,
 ) => {
-  const [fx1, fy1, fx2, fy2] = getElementAbsoluteCoords(frame);
+  const [fx1, fy1, fx2, fy2] = getElementBounds(frame);
 
   return isPointWithinBounds(
     [fx1, fy1],
@@ -160,7 +151,7 @@ export const groupsAreAtLeastIntersectingTheFrame = (
 
   return !!elementsInGroup.find(
     (element) =>
-      elementsAreInFrameBounds([element], frame) ||
+      elementsAreInBounds([element], frame) ||
       isElementIntersectingFrame(element, frame),
   );
 };
@@ -181,7 +172,7 @@ export const groupsAreCompletelyOutOfFrame = (
   return (
     elementsInGroup.find(
       (element) =>
-        elementsAreInFrameBounds([element], frame) ||
+        elementsAreInBounds([element], frame) ||
         isElementIntersectingFrame(element, frame),
     ) === undefined
   );
@@ -249,12 +240,18 @@ export const getElementsInResizingFrame = (
   const prevElementsInFrame = getFrameChildren(allElements, frame.id);
   const nextElementsInFrame = new Set<ExcalidrawElement>(prevElementsInFrame);
 
-  const elementsCompletelyInFrame = new Set([
-    ...getElementsCompletelyInFrame(allElements, frame),
-    ...prevElementsInFrame.filter((element) =>
-      isElementContainingFrame(allElements, element, frame),
-    ),
-  ]);
+  const elementsCompletelyInFrame = new Set<ExcalidrawElement>(
+    getElementsCompletelyInFrame(allElements, frame),
+  );
+
+  for (const element of prevElementsInFrame) {
+    if (!elementsCompletelyInFrame.has(element)) {
+      // element contains the frame
+      if (elementsAreInBounds([frame], element)) {
+        elementsCompletelyInFrame.add(element);
+      }
+    }
+  }
 
   const elementsNotCompletelyInFrame = prevElementsInFrame.filter(
     (element) => !elementsCompletelyInFrame.has(element),
@@ -321,7 +318,7 @@ export const getElementsInResizingFrame = (
     if (isSelected) {
       const elementsInGroup = getElementsInGroup(allElements, id);
 
-      if (elementsAreInFrameBounds(elementsInGroup, frame)) {
+      if (elementsAreInBounds(elementsInGroup, frame)) {
         for (const element of elementsInGroup) {
           nextElementsInFrame.add(element);
         }
@@ -509,12 +506,15 @@ export const updateFrameMembershipOfSelectedElements = (
   }
 
   const elementsToRemove = new Set<ExcalidrawElement>();
+  const processedGroupIds = new Map<string, boolean>();
 
   elementsToFilter.forEach((element) => {
     if (
       element.frameId &&
       !isFrameElement(element) &&
-      !isElementInFrame(element, allElements, appState)
+      !isElementInFrame(element, allElements, appState, {
+        processedGroupIds,
+      })
     ) {
       elementsToRemove.add(element);
     }
@@ -576,27 +576,36 @@ export const getTargetFrame = (
     : getContainingFrame(_element);
 };
 
-// TODO: this a huge bottleneck for large scenes, optimise
 // given an element, return if the element is in some frame
 export const isElementInFrame = (
   element: ExcalidrawElement,
   allElements: ExcalidrawElementsIncludingDeleted,
   appState: StaticCanvasAppState,
-  targetFrame?: ExcalidrawFrameElement,
+  opts?: {
+    targetFrame?: ExcalidrawFrameElement;
+    processedGroupIds?: Map<string, boolean>;
+  },
 ) => {
-  const frame = targetFrame ?? getTargetFrame(element, appState);
+  const frame = opts?.targetFrame ?? getTargetFrame(element, appState);
   const _element = isTextElement(element)
     ? getContainerElement(element) || element
     : element;
 
+  const groupsInFrame = (yes: boolean) => {
+    if (opts?.processedGroupIds) {
+      _element.groupIds.forEach((gid) => {
+        opts.processedGroupIds?.set(gid, yes);
+      });
+    }
+  };
+
   if (frame) {
     // 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.
+    // For an element that's already in a frame, if it's not being selected
+    // and its frame is not being selected, it has to be in its containing frame.
     if (
-      !appState.selectedElementIds[element.id] ||
-      !appState.selectedElementsAreBeingDragged
+      !appState.selectedElementIds[element.id] &&
+      !appState.selectedElementIds[frame.id]
     ) {
       return true;
     }
@@ -605,8 +614,21 @@ export const isElementInFrame = (
       return elementOverlapsWithFrame(_element, frame);
     }
 
+    for (const gid of _element.groupIds) {
+      if (opts?.processedGroupIds?.has(gid)) {
+        return opts.processedGroupIds.get(gid);
+      }
+    }
+
     const allElementsInGroup = new Set(
-      _element.groupIds.flatMap((gid) => getElementsInGroup(allElements, gid)),
+      _element.groupIds
+        .filter((gid) => {
+          if (opts?.processedGroupIds) {
+            return !opts.processedGroupIds.has(gid);
+          }
+          return true;
+        })
+        .flatMap((gid) => getElementsInGroup(allElements, gid)),
     );
 
     if (appState.editingGroupId && appState.selectedElementsAreBeingDragged) {
@@ -627,16 +649,22 @@ export const isElementInFrame = (
 
     for (const elementInGroup of allElementsInGroup) {
       if (isFrameElement(elementInGroup)) {
+        groupsInFrame(false);
         return false;
       }
     }
 
     for (const elementInGroup of allElementsInGroup) {
       if (elementOverlapsWithFrame(elementInGroup, frame)) {
+        groupsInFrame(true);
         return true;
       }
     }
   }
 
+  if (_element.groupIds.length > 0) {
+    groupsInFrame(false);
+  }
+
   return false;
 };

src/groups.ts

@@ -232,6 +232,8 @@ export const selectGroupsFromGivenElements = (
     selectedGroupIds: {},
   };
 
+  const processedGroupIds = new Set<string>();
+
   for (const element of elements) {
     let groupIds = element.groupIds;
     if (appState.editingGroupId) {
@@ -242,10 +244,13 @@ export const selectGroupsFromGivenElements = (
     }
     if (groupIds.length > 0) {
       const groupId = groupIds[groupIds.length - 1];
-      nextAppState = {
-        ...nextAppState,
-        ...selectGroup(groupId, nextAppState, elements),
-      };
+      if (!processedGroupIds.has(groupId)) {
+        nextAppState = {
+          ...nextAppState,
+          ...selectGroup(groupId, nextAppState, elements),
+        };
+        processedGroupIds.add(groupId);
+      }
     }
   }
 

src/renderer/renderScene.ts

@@ -71,6 +71,7 @@ import { renderSnaps } from "./renderSnaps";
 import {
   isEmbeddableElement,
   isFrameElement,
+  isFreeDrawElement,
   isLinearElement,
 } from "../element/typeChecks";
 import {
@@ -78,7 +79,7 @@ import {
   createPlaceholderEmbeddableLabel,
 } from "../element/embeddable";
 import {
-  elementsAreInFrameBounds,
+  elementsAreInBounds,
   getTargetFrame,
   isElementInFrame,
 } from "../frame";
@@ -981,6 +982,7 @@ const _renderStaticScene = ({
     }
   };
 
+  const processedGroupIds = new Map<string, boolean>();
   for (const element of visibleElementsToRender) {
     const frameId = element.frameId || appState.frameToHighlight?.id;
 
@@ -994,8 +996,17 @@ const _renderStaticScene = ({
       // only clip elements that are not completely in the target frame
       if (
         targetFrame &&
-        !elementsAreInFrameBounds([element], targetFrame) &&
-        isElementInFrame(element, elements, appState)
+        !elementsAreInBounds(
+          [element],
+          targetFrame,
+          isFreeDrawElement(element)
+            ? element.strokeWidth * 8
+            : element.roughness * (isLinearElement(element) ? 8 : 4),
+        ) &&
+        isElementInFrame(element, elements, appState, {
+          targetFrame,
+          processedGroupIds,
+        })
       ) {
         context.save();
         frameClip(targetFrame, context, renderConfig, appState);
@@ -1112,7 +1123,7 @@ const renderTransformHandles = (
 
 const renderSelectionBorder = (
   context: CanvasRenderingContext2D,
-  appState: InteractiveCanvasAppState,
+  appState: InteractiveCanvasAppState | StaticCanvasAppState,
   elementProperties: {
     angle: number;
     elementX1: number;
@@ -1277,6 +1288,23 @@ const renderFrameHighlight = (
   context.restore();
 };
 
+const getSelectionFromElements = (elements: ExcalidrawElement[]) => {
+  const [elementX1, elementY1, elementX2, elementY2] =
+    getCommonBounds(elements);
+  return {
+    angle: 0,
+    elementX1,
+    elementX2,
+    elementY1,
+    elementY2,
+    selectionColors: ["rgb(0,118,255)"],
+    dashed: false,
+    cx: elementX1 + (elementX2 - elementX1) / 2,
+    cy: elementY1 + (elementY2 - elementY1) / 2,
+    activeEmbeddable: false,
+  };
+};
+
 const renderElementsBoxHighlight = (
   context: CanvasRenderingContext2D,
   appState: InteractiveCanvasAppState,
@@ -1290,37 +1318,28 @@ const renderElementsBoxHighlight = (
     (element) => element.groupIds.length > 0,
   );
 
-  const getSelectionFromElements = (elements: ExcalidrawElement[]) => {
-    const [elementX1, elementY1, elementX2, elementY2] =
-      getCommonBounds(elements);
-    return {
-      angle: 0,
-      elementX1,
-      elementX2,
-      elementY1,
-      elementY2,
-      selectionColors: ["rgb(0,118,255)"],
-      dashed: false,
-      cx: elementX1 + (elementX2 - elementX1) / 2,
-      cy: elementY1 + (elementY2 - elementY1) / 2,
-      activeEmbeddable: false,
-    };
-  };
+  const processedGroupIds = new Set<string>();
 
   const getSelectionForGroupId = (groupId: GroupId) => {
-    const groupElements = getElementsInGroup(elements, groupId);
-    return getSelectionFromElements(groupElements);
+    if (!processedGroupIds.has(groupId)) {
+      const groupElements = getElementsInGroup(elements, groupId);
+      processedGroupIds.add(groupId);
+      return getSelectionFromElements(groupElements);
+    }
+
+    return null;
   };
 
   Object.entries(selectGroupsFromGivenElements(elementsInGroups, appState))
     .filter(([id, isSelected]) => isSelected)
     .map(([id, isSelected]) => id)
     .map((groupId) => getSelectionForGroupId(groupId))
+    .filter((selection) => selection)
     .concat(
       individualElements.map((element) => getSelectionFromElements([element])),
     )
     .forEach((selection) =>
-      renderSelectionBorder(context, appState, selection),
+      renderSelectionBorder(context, appState, selection!),
     );
 };