feat(editor): remove non-overlapping frame children on restore

packages/element/tests/frame.test.tsx

@@ -3,6 +3,7 @@ import {
   Excalidraw,
 } from "@excalidraw/excalidraw";
 import { arrayToMap } from "@excalidraw/common";
+import { degreesToRadians, pointFrom } from "@excalidraw/math";
 
 import { API } from "@excalidraw/excalidraw/tests/helpers/api";
 import { Keyboard, Pointer, UI } from "@excalidraw/excalidraw/tests/helpers/ui";
@@ -14,6 +15,8 @@ import {
 
 import { getSelectedElements } from "@excalidraw/excalidraw/scene";
 
+import type { Degrees } from "@excalidraw/math";
+
 import { elementOverlapsWithFrame } from "../src/frame";
 
 import type {
@@ -154,6 +157,108 @@ describe("adding elements to frames", () => {
     ).toBe(true);
   });
 
+  it("uses precise geometry (not the bounding box) to detect frame overlap", () => {
+    // a 300x200 frame at the origin, with shapes positioned just outside each
+    // corner so that their bounding box clips the frame but their actual
+    // outline does not. The precise `isElementIntersectingFrame` must report
+    // no overlap.
+    const overlapFrame = API.createElement({
+      type: "frame",
+      x: 0,
+      y: 0,
+      width: 300,
+      height: 200,
+    });
+    // clips the bottom-right corner; the rhombus tip stays out
+    const diamond = API.createElement({
+      type: "diamond",
+      x: 290,
+      y: 175,
+      width: 100,
+      height: 100,
+    });
+    // clips the bottom-left corner; rotated (NOT 45°, which would degenerate
+    // into an axis-aligned square whose shape fills its AABB)
+    const diamondRotated = API.createElement({
+      type: "diamond",
+      x: -50,
+      y: 195,
+      width: 100,
+      height: 100,
+      angle: degreesToRadians(45 as Degrees),
+    });
+    // clips the top-left corner; the curve doesn't reach the corner
+    const ellipse = API.createElement({
+      type: "ellipse",
+      x: -90,
+      y: -90,
+      width: 100,
+      height: 100,
+    });
+    // clips the top edge; the polyline weaves outside the frame
+    const line = API.createElement({
+      type: "line",
+      x: 130,
+      y: -20,
+      width: 293,
+      height: 183,
+      points: [
+        pointFrom(0, 0),
+        pointFrom(-165, -96),
+        pointFrom(-293, -16),
+        pointFrom(-162, 87),
+      ],
+    });
+    // clips the bottom-left corner; the stroke stays outside
+    const freedraw = API.createElement({
+      type: "freedraw",
+      x: -15,
+      y: 180,
+      width: 207,
+      height: 209,
+      points: [
+        pointFrom(0, 0),
+        pointFrom(-69, 99),
+        pointFrom(-33, 183),
+        pointFrom(48, 209),
+        pointFrom(124, 195),
+        pointFrom(138, 145),
+        pointFrom(123, 85),
+      ],
+    });
+    // clips the top-right corner, but its rounded corner is cut away from
+    // exactly that corner — so its actual outline stays outside
+    const roundedRect = API.createElement({
+      type: "rectangle",
+      x: 295,
+      y: -100,
+      width: 160,
+      height: 110,
+      roundness: { type: 3 },
+    });
+
+    API.setElements([
+      overlapFrame,
+      diamond,
+      diamondRotated,
+      ellipse,
+      line,
+      freedraw,
+      roundedRect,
+    ]);
+    const elementsMap = arrayToMap(h.elements);
+    const f = overlapFrame as ExcalidrawFrameLikeElement;
+
+    expect(elementOverlapsWithFrame(diamond, f, elementsMap)).toBe(false);
+    expect(elementOverlapsWithFrame(diamondRotated, f, elementsMap)).toBe(
+      false,
+    );
+    expect(elementOverlapsWithFrame(ellipse, f, elementsMap)).toBe(false);
+    expect(elementOverlapsWithFrame(line, f, elementsMap)).toBe(false);
+    expect(elementOverlapsWithFrame(freedraw, f, elementsMap)).toBe(false);
+    expect(elementOverlapsWithFrame(roundedRect, f, elementsMap)).toBe(false);
+  });
+
   it("should not add a newly created element to a frame behind a non-frame element", () => {
     const cover = API.createElement({
       id: "cover",

packages/excalidraw/data/restore.ts

@@ -54,6 +54,11 @@ import { getNormalizedDimensions } from "@excalidraw/element";
 
 import { isInvisiblySmallElement } from "@excalidraw/element";
 
+import {
+  elementOverlapsWithFrame,
+  isFrameLikeElement,
+} from "@excalidraw/element";
+
 import type { LocalPoint, Radians } from "@excalidraw/math";
 
 import type {
@@ -744,20 +749,78 @@ const repairBoundElement = (
 };
 
 /**
- * Remove an element's frameId if its containing frame is non-existent
+ * Remove an element's frameId if:
+ *  - its containing frame is non-existent (or isn't actually a frame), or
+ *  - the element no longer (precisely) overlaps its containing frame.
+ *
+ * Frame membership is treated atomically per group: a grouped element keeps its
+ * frameId as long as ANY element of its (outermost) group still overlaps the
+ * frame, so we never split a group across frame membership. Bound text inherits
+ * its container's geometry/membership.
  *
  * NOTE mutates elements.
  */
 const repairFrameMembership = (
   element: Mutable<ExcalidrawElement>,
   elementsMap: Map<string, Mutable<ExcalidrawElement>>,
+  /** elements bucketed by outermost groupId (see `restoreElements`) */
+  elementsByGroup: Map<string, Mutable<ExcalidrawElement>[]>,
+  /** memoizes the overlap decision per `${frameId}:${groupId}` */
+  checkedFrameGroups: Map<string, boolean>,
 ) => {
-  if (element.frameId) {
+  if (!element.frameId) {
-    const containingFrame = elementsMap.get(element.frameId);
+    return;
+  }
 
-    if (!containingFrame) {
+  const containingFrame = elementsMap.get(element.frameId);
-      element.frameId = null;
+
+  // frame no longer exists (or the frameId doesn't point to a frame)
+  if (!containingFrame || !isFrameLikeElement(containingFrame)) {
+    element.frameId = null;
+    return;
+  }
+
+  // don't recompute geometry for deleted elements
+  if (element.isDeleted) {
+    return;
+  }
+
+  // a bound text follows its container's geometry / membership
+  const geometryElement =
+    isTextElement(element) && element.containerId
+      ? elementsMap.get(element.containerId) ?? element
+      : element;
+
+  const outerGroupId =
+    geometryElement.groupIds[geometryElement.groupIds.length - 1];
+
+  let overlapsFrame: boolean;
+  if (outerGroupId) {
+    const cacheKey = `${containingFrame.id}:${outerGroupId}`;
+    const cached = checkedFrameGroups.get(cacheKey);
+    if (cached !== undefined) {
+      overlapsFrame = cached;
+    } else {
+      // group membership is atomic — keep the element as long as ANY element
+      // of its group still overlaps the frame, so we never split a group.
+      const groupElements = elementsByGroup.get(outerGroupId) ?? [
+        geometryElement,
+      ];
+      overlapsFrame = groupElements.some((groupElement) =>
+        elementOverlapsWithFrame(groupElement, containingFrame, elementsMap),
+      );
+      checkedFrameGroups.set(cacheKey, overlapsFrame);
     }
+  } else {
+    overlapsFrame = elementOverlapsWithFrame(
+      geometryElement,
+      containingFrame,
+      elementsMap,
+    );
+  }
+
+  if (!overlapsFrame) {
+    element.frameId = null;
   }
 };
 
@@ -836,9 +899,32 @@ export const restoreElements = <T extends ExcalidrawElement>(
 
   // repair binding. Mutates elements.
   const restoredElementsMap = arrayToMap(restoredElements);
+
+  // index elements by their outermost groupId once, so frame-membership repair
+  // can resolve a group's members in O(1) instead of scanning all elements per
+  // group (getElementsInGroup) — a big win on large, heavily-grouped scenes.
+  const elementsByGroup = new Map<string, Mutable<ExcalidrawElement>[]>();
+  for (const element of restoredElements) {
+    const outerGroupId = element.groupIds[element.groupIds.length - 1];
+    if (outerGroupId) {
+      const members = elementsByGroup.get(outerGroupId);
+      if (members) {
+        members.push(element);
+      } else {
+        elementsByGroup.set(outerGroupId, [element]);
+      }
+    }
+  }
+
+  const checkedFrameGroups = new Map<string, boolean>();
   for (const element of restoredElements) {
     if (element.frameId) {
-      repairFrameMembership(element, restoredElementsMap);
+      repairFrameMembership(
+        element,
+        restoredElementsMap,
+        elementsByGroup,
+        checkedFrameGroups,
+      );
     }
 
     if (isTextElement(element) && element.containerId) {

packages/excalidraw/tests/data/restore.test.ts

@@ -1,7 +1,12 @@
 import { pointFrom } from "@excalidraw/math";
 import { vi } from "vitest";
 
-import { DEFAULT_SIDEBAR, FONT_FAMILY, ROUNDNESS } from "@excalidraw/common";
+import {
+  arrayToMap,
+  DEFAULT_SIDEBAR,
+  FONT_FAMILY,
+  ROUNDNESS,
+} from "@excalidraw/common";
 
 import { newElementWith } from "@excalidraw/element";
 import * as sizeHelpers from "@excalidraw/element";
@@ -1140,3 +1145,201 @@ describe("repairing bindings", () => {
     ]);
   });
 });
+
+describe("repairing frame membership", () => {
+  const makeFrame = () =>
+    API.createElement({
+      type: "frame",
+      id: "frame",
+      x: 0,
+      y: 0,
+      width: 100,
+      height: 100,
+    });
+
+  it("keeps frameId for an element fully inside the frame", () => {
+    const frame = makeFrame();
+    const inside = API.createElement({
+      type: "rectangle",
+      id: "inside",
+      x: 10,
+      y: 10,
+      width: 20,
+      height: 20,
+      frameId: frame.id,
+    });
+
+    const restoredElementsMap = arrayToMap(
+      restore.restoreElements([frame, inside], null, { repairBindings: true }),
+    );
+    expect(restoredElementsMap.get("inside")?.frameId).toBe(frame.id);
+  });
+
+  it("keeps frameId for an element intersecting the frame border", () => {
+    const frame = makeFrame();
+    // spans x:[90,130] — crosses the frame's right edge (x=100)
+    const straddling = API.createElement({
+      type: "rectangle",
+      id: "straddling",
+      x: 90,
+      y: 10,
+      width: 40,
+      height: 20,
+      frameId: frame.id,
+    });
+
+    const restoredElementsMap = arrayToMap(
+      restore.restoreElements([frame, straddling], null, {
+        repairBindings: true,
+      }),
+    );
+    expect(restoredElementsMap.get("straddling")?.frameId).toBe(frame.id);
+  });
+
+  it("removes frameId for an element fully outside the frame", () => {
+    const frame = makeFrame();
+    const outside = API.createElement({
+      type: "rectangle",
+      id: "outside",
+      x: 200,
+      y: 200,
+      width: 20,
+      height: 20,
+      frameId: frame.id,
+    });
+
+    const restoredElementsMap = arrayToMap(
+      restore.restoreElements([frame, outside], null, { repairBindings: true }),
+    );
+    expect(restoredElementsMap.get("outside")?.frameId).toBe(null);
+  });
+
+  it("removes frameId when the containing frame no longer exists", () => {
+    const orphan = API.createElement({
+      type: "rectangle",
+      id: "orphan",
+      x: 10,
+      y: 10,
+      frameId: "non-existent-frame",
+    });
+
+    const restoredElementsMap = arrayToMap(
+      restore.restoreElements([orphan], null, { repairBindings: true }),
+    );
+    expect(restoredElementsMap.get("orphan")?.frameId).toBe(null);
+  });
+
+  it("does NOT touch frameId when repairBindings is off", () => {
+    const frame = makeFrame();
+    const outside = API.createElement({
+      type: "rectangle",
+      id: "outside",
+      x: 200,
+      y: 200,
+      width: 20,
+      height: 20,
+      frameId: frame.id,
+    });
+
+    const restoredElementsMap = arrayToMap(
+      restore.restoreElements([frame, outside], null, {
+        repairBindings: false,
+      }),
+    );
+    expect(restoredElementsMap.get("outside")?.frameId).toBe(frame.id);
+  });
+
+  it("keeps a group atomic: members stay if ANY group member overlaps", () => {
+    const frame = makeFrame();
+    const insideMember = API.createElement({
+      type: "rectangle",
+      id: "insideMember",
+      x: 10,
+      y: 10,
+      width: 20,
+      height: 20,
+      groupIds: ["g1"],
+      frameId: frame.id,
+    });
+    const outsideMember = API.createElement({
+      type: "rectangle",
+      id: "outsideMember",
+      x: 300,
+      y: 300,
+      width: 20,
+      height: 20,
+      groupIds: ["g1"],
+      frameId: frame.id,
+    });
+
+    const restoredElementsMap = arrayToMap(
+      restore.restoreElements([frame, insideMember, outsideMember], null, {
+        repairBindings: true,
+      }),
+    );
+    // the whole group overlaps (via insideMember), so neither is evicted
+    expect(restoredElementsMap.get("insideMember")?.frameId).toBe(frame.id);
+    expect(restoredElementsMap.get("outsideMember")?.frameId).toBe(frame.id);
+  });
+
+  it("removes frameId for a whole group when no member overlaps", () => {
+    const frame = makeFrame();
+    const a = API.createElement({
+      type: "rectangle",
+      id: "a",
+      x: 200,
+      y: 200,
+      width: 20,
+      height: 20,
+      groupIds: ["g1"],
+      frameId: frame.id,
+    });
+    const b = API.createElement({
+      type: "rectangle",
+      id: "b",
+      x: 300,
+      y: 300,
+      width: 20,
+      height: 20,
+      groupIds: ["g1"],
+      frameId: frame.id,
+    });
+
+    const restoredElementsMap = arrayToMap(
+      restore.restoreElements([frame, a, b], null, { repairBindings: true }),
+    );
+    expect(restoredElementsMap.get("a")?.frameId).toBe(null);
+    expect(restoredElementsMap.get("b")?.frameId).toBe(null);
+  });
+
+  it("bound text follows its container's frame membership", () => {
+    const frameOutside = makeFrame();
+    const container = API.createElement({
+      type: "rectangle",
+      id: "container",
+      x: 300,
+      y: 300,
+      width: 40,
+      height: 40,
+      frameId: frameOutside.id,
+      boundElements: [{ type: "text", id: "label" }],
+    });
+    const label = API.createElement({
+      type: "text",
+      id: "label",
+      x: 305,
+      y: 305,
+      containerId: container.id,
+      frameId: frameOutside.id,
+    });
+
+    const restoredElementsMap = arrayToMap(
+      restore.restoreElements([frameOutside, container, label], null, {
+        repairBindings: true,
+      }),
+    );
+    // container is fully outside -> both it and its bound text leave the frame
+    expect(restoredElementsMap.get("container")?.frameId).toBe(null);
+    expect(restoredElementsMap.get("label")?.frameId).toBe(null);
+  });
+});

packages/excalidraw/tests/helpers/svg.ts

@@ -0,0 +1,52 @@
+import type {
+  ExcalidrawElement,
+  NonDeletedExcalidrawElement,
+} from "@excalidraw/element/types";
+
+import { exportToSvg } from "../../scene/export";
+
+/**
+ * Test-only helper: render the supplied elements to an SVG string for visual
+ * debugging (e.g. to eyeball frame-membership / overlap geometry).
+ *
+ * Frames are drawn with their outline but are NOT clipped, and every element's
+ * `frameId` is ignored, so elements that fall outside a frame stay visible and
+ * within the exported viewBox instead of being clipped or cropped away.
+ *
+ * Pass `outFile` to also write the SVG to disk — open it in a browser to view.
+ *
+ * @example
+ *   await exportElementsToSVG([frame, diamond], {
+ *     outFile: "/tmp/frame-debug.svg",
+ *   });
+ */
+export const exportElementsToSVG = async (
+  elements: readonly ExcalidrawElement[],
+  opts?: { outFile?: string; padding?: number; background?: string },
+): Promise<string> => {
+  // ignore frame membership so nothing gets clipped/cropped out of view
+  const rootElements = elements.map(
+    (element) => ({ ...element, frameId: null } as NonDeletedExcalidrawElement),
+  );
+
+  const svg = await exportToSvg(
+    rootElements,
+    {
+      exportBackground: true,
+      viewBackgroundColor: opts?.background ?? "#ffffff",
+      exportPadding: opts?.padding ?? 24,
+      frameRendering: { enabled: true, name: true, outline: true, clip: false },
+    },
+    null,
+    { skipInliningFonts: true },
+  );
+
+  const svgString = svg.outerHTML;
+
+  if (opts?.outFile) {
+    const { writeFileSync } = await import("fs");
+    writeFileSync(opts.outFile, svgString);
+  }
+
+  return svgString;
+};