From d56bb4087f9b8ad5e253e52a51ea01ab0346f9c0 Mon Sep 17 00:00:00 2001
From: dwelle <5153846+dwelle@users.noreply.github.com>
Date: Sun, 25 May 2025 22:57:32 +0200
Subject: [PATCH] break polygon on restore/finalize if invalid & prevent
 creation

---
 packages/element/src/linearElementEditor.ts   | 19 +++++++-------
 packages/element/src/shapes.ts                | 11 +++++++-
 packages/element/src/typeChecks.ts            | 25 +++++++++++++++++++
 .../excalidraw/actions/actionFinalize.tsx     | 14 ++++++++++-
 .../excalidraw/actions/actionProperties.tsx   | 20 +++++++--------
 packages/excalidraw/data/restore.ts           |  8 ++++--
 6 files changed, 73 insertions(+), 24 deletions(-)

diff --git a/packages/element/src/linearElementEditor.ts b/packages/element/src/linearElementEditor.ts
index 1273745d0..ae752eeae 100644
--- a/packages/element/src/linearElementEditor.ts
+++ b/packages/element/src/linearElementEditor.ts
@@ -1290,16 +1290,17 @@ export class LinearElementEditor {
       app.state.editingLinearElement?.lastUncommittedPoint ===
       element.points[element.points.length - 1];
 
+    const isPolygon = isLineElement(element) && element.polygon;
+
     // break polygon if deleting start/end point
-    if (isLineElement(element) && element.polygon) {
-      if (
-        pointIndices.includes(0) ||
-        (pointIndices.includes(element.points.length - 1) &&
-          // don't disable polygon if cleaning up uncommitted point
-          !isUncommittedPoint)
-      ) {
-        app.scene.mutateElement(element, { polygon: false });
-      }
+    if (
+      isPolygon &&
+      // don't disable polygon if cleaning up uncommitted point
+      !isUncommittedPoint &&
+      (pointIndices.includes(0) ||
+        pointIndices.includes(element.points.length - 1))
+    ) {
+      app.scene.mutateElement(element, { polygon: false });
     }
 
     const nextPoints = element.points.filter((_, idx) => {
diff --git a/packages/element/src/shapes.ts b/packages/element/src/shapes.ts
index cc6481908..83bfd74ec 100644
--- a/packages/element/src/shapes.ts
+++ b/packages/element/src/shapes.ts
@@ -36,6 +36,8 @@ import { ShapeCache } from "./ShapeCache";
 
 import { getElementAbsoluteCoords, type Bounds } from "./bounds";
 
+import { canBecomePolygon } from "./typeChecks";
+
 import type {
   ElementsMap,
   ExcalidrawElement,
@@ -402,10 +404,17 @@ export const isPathALoop = (
 export const toggleLinePolygonState = (
   element: ExcalidrawLineElement,
   nextPolygonState: boolean,
-) => {
+): {
+  polygon: ExcalidrawLineElement["polygon"];
+  points: ExcalidrawLineElement["points"];
+} | null => {
   const updatedPoints = [...element.points];
 
   if (nextPolygonState) {
+    if (!canBecomePolygon(element.points)) {
+      return null;
+    }
+
     const firstPoint = updatedPoints[0];
     const lastPoint = updatedPoints[updatedPoints.length - 1];
 
diff --git a/packages/element/src/typeChecks.ts b/packages/element/src/typeChecks.ts
index 963c4a730..37974f1f5 100644
--- a/packages/element/src/typeChecks.ts
+++ b/packages/element/src/typeChecks.ts
@@ -1,5 +1,7 @@
 import { ROUNDNESS, assertNever } from "@excalidraw/common";
 
+import { pointsEqual } from "@excalidraw/math";
+
 import type { ElementOrToolType } from "@excalidraw/excalidraw/types";
 
 import type { MarkNonNullable } from "@excalidraw/common/utility-types";
@@ -379,3 +381,26 @@ export const getLinearElementSubType = (
   }
   return "line";
 };
+
+/**
+ * Checks if current element points meet all the conditions for polygon=true
+ * (this isn't a element type check, for that use isLineElement).
+ *
+ * If you want to check if points *can* be turned into a polygon, use
+ *  canBecomePolygon(points).
+ */
+export const isValidPolygon = (
+  points: ExcalidrawLineElement["points"],
+): boolean => {
+  return points.length > 3 && pointsEqual(points[0], points[points.length - 1]);
+};
+
+export const canBecomePolygon = (
+  points: ExcalidrawLineElement["points"],
+): boolean => {
+  return (
+    points.length > 3 ||
+    // 3-point polygons can't have all points in a single line
+    (points.length === 3 && !pointsEqual(points[0], points[points.length - 1]))
+  );
+};
diff --git a/packages/excalidraw/actions/actionFinalize.tsx b/packages/excalidraw/actions/actionFinalize.tsx
index ea033073d..e1c044f1e 100644
--- a/packages/excalidraw/actions/actionFinalize.tsx
+++ b/packages/excalidraw/actions/actionFinalize.tsx
@@ -5,7 +5,7 @@ import {
   bindOrUnbindLinearElement,
   isBindingEnabled,
 } from "@excalidraw/element/binding";
-import { LinearElementEditor } from "@excalidraw/element/linearElementEditor";
+import { isValidPolygon, LinearElementEditor } from "@excalidraw/element";
 
 import {
   isBindingElement,
@@ -99,6 +99,12 @@ export const actionFinalize = register({
             scene,
           );
         }
+        if (isLineElement(element) && !isValidPolygon(element.points)) {
+          scene.mutateElement(element, {
+            polygon: false,
+          });
+        }
+
         return {
           elements:
             element.points.length < 2 || isInvisiblySmallElement(element)
@@ -198,6 +204,12 @@ export const actionFinalize = register({
           }
         }
 
+        if (isLineElement(element) && !isValidPolygon(element.points)) {
+          scene.mutateElement(element, {
+            polygon: false,
+          });
+        }
+
         if (
           isBindingElement(element) &&
           !isLoop &&
diff --git a/packages/excalidraw/actions/actionProperties.tsx b/packages/excalidraw/actions/actionProperties.tsx
index ab971b886..30c9e7434 100644
--- a/packages/excalidraw/actions/actionProperties.tsx
+++ b/packages/excalidraw/actions/actionProperties.tsx
@@ -24,7 +24,7 @@ import {
   reduceToCommonValue,
 } from "@excalidraw/common";
 
-import { getNonDeletedElements } from "@excalidraw/element";
+import { canBecomePolygon, getNonDeletedElements } from "@excalidraw/element";
 
 import {
   bindLinearElement,
@@ -364,18 +364,16 @@ export const actionChangeBackgroundColor = register({
       };
     }
 
-    const selectedElements = app.scene.getSelectedElements(appState);
-    const shouldEnablePolygon = selectedElements.every((el) =>
-      isLineElement(el),
-    );
-
     let nextElements;
 
-    if (
-      shouldEnablePolygon &&
-      value.currentItemBackgroundColor &&
-      !isTransparent(value.currentItemBackgroundColor)
-    ) {
+    const selectedElements = app.scene.getSelectedElements(appState);
+    const shouldEnablePolygon =
+      !isTransparent(value.currentItemBackgroundColor) &&
+      selectedElements.every(
+        (el) => isLineElement(el) && canBecomePolygon(el.points),
+      );
+
+    if (shouldEnablePolygon) {
       const selectedElementsMap = arrayToMap(selectedElements);
       nextElements = elements.map((el) => {
         if (selectedElementsMap.has(el.id) && isLineElement(el)) {
diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts
index e282767c3..a609c0a0e 100644
--- a/packages/excalidraw/data/restore.ts
+++ b/packages/excalidraw/data/restore.ts
@@ -18,7 +18,7 @@ import {
   normalizeLink,
   getLineHeight,
 } from "@excalidraw/common";
-import { getNonDeletedElements } from "@excalidraw/element";
+import { getNonDeletedElements, isValidPolygon } from "@excalidraw/element";
 import { normalizeFixedPoint } from "@excalidraw/element";
 import {
   updateElbowArrowPoints,
@@ -342,7 +342,11 @@ const restoreElement = (
         x,
         y,
         ...(isLineElement(element)
-          ? { polygon: element.polygon ?? false }
+          ? {
+              polygon: isValidPolygon(element.points)
+                ? element.polygon ?? false
+                : false,
+            }
           : {}),
         ...getSizeFromPoints(points),
       });