feat: line polygons (#9477)

* Loop Lock/Unlock * fixed condition. 4 line points are required for the action to be available * extracted updateLoopLock to improve readability. Removed unnecessary SVG attributes * lint + added loopLock to restore.ts * added loopLock to newElement, updated test snapshots * lint * dislocate enpoint when breaking the loop. * change icon & turn into a state style button * POC: auto-transform to polygon on bg set * keep polygon icon constant * do not split points on de-polygonizing & highlight overlapping points * rewrite color picker to support no (mixed) colors & fix focus handling * refactor * tweak point rendering inside line editor * do not disable polygon when creating new points via alt * auto-enable polygon when aligning start/end points * TBD: remove bg color when disabling polygon * TBD: only show polygon button for enabled polygons * fix polygon behavior when adding/removing/moving points within line editor * convert to polygon when creating line * labels tweak * add to command palette * loopLock -> polygon * restore `polygon` state on type conversions * update snapshots * naming * break polygon on restore/finalize if invalid & prevent creation * snapshots * fix: merge issue and forgotten debug * snaps * do not merge points for 3-point lines --------- Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>
2025-09-15 05:20:27 +02:00 · 2025-05-26 11:14:55 +02:00
parent 4dc205537c
commit 87c87a9fb1
26 changed files with 555 additions and 121 deletions
--- a/packages/element/src/linearElementEditor.ts
+++ b/packages/element/src/linearElementEditor.ts
@@ -63,10 +63,13 @@ import {
  getControlPointsForBezierCurve,
  mapIntervalToBezierT,
  getBezierXY,
+  toggleLinePolygonState,
 } from "./shapes";

 import { getLockedLinearCursorAlignSize } from "./sizeHelpers";

+import { isLineElement } from "./typeChecks";
+
 import type { Scene } from "./Scene";

 import type { Bounds } from "./bounds";
@@ -85,6 +88,35 @@ import type {
  PointsPositionUpdates,
 } from "./types";

+/**
+ * Normalizes line points so that the start point is at [0,0]. This is
+ * expected in various parts of the codebase.
+ *
+ * Also returns the offsets - [0,0] if no normalization needed.
+ *
+ * @private
+ */
+const getNormalizedPoints = ({
+  points,
+}: {
+  points: ExcalidrawLinearElement["points"];
+}): {
+  points: LocalPoint[];
+  offsetX: number;
+  offsetY: number;
+} => {
+  const offsetX = points[0][0];
+  const offsetY = points[0][1];
+
+  return {
+    points: points.map((p) => {
+      return pointFrom(p[0] - offsetX, p[1] - offsetY);
+    }),
+    offsetX,
+    offsetY,
+  };
+};
+
 export class LinearElementEditor {
  public readonly elementId: ExcalidrawElement["id"] & {
    _brand: "excalidrawLinearElementId";
@@ -127,7 +159,11 @@ export class LinearElementEditor {
    };
    if (!pointsEqual(element.points[0], pointFrom(0, 0))) {
      console.error("Linear element is not normalized", Error().stack);
-      LinearElementEditor.normalizePoints(element, elementsMap);
+      mutateElement(
+        element,
+        elementsMap,
+        LinearElementEditor.getNormalizeElementPointsAndCoords(element),
+      );
    }
    this.selectedPointsIndices = null;
    this.lastUncommittedPoint = null;
@@ -459,6 +495,18 @@ export class LinearElementEditor {
          selectedPoint === element.points.length - 1
        ) {
          if (isPathALoop(element.points, appState.zoom.value)) {
+            if (isLineElement(element)) {
+              scene.mutateElement(
+                element,
+                {
+                  ...toggleLinePolygonState(element, true),
+                },
+                {
+                  informMutation: false,
+                  isDragging: false,
+                },
+              );
+            }
            LinearElementEditor.movePoints(
              element,
              scene,
@@ -946,9 +994,7 @@ export class LinearElementEditor {

    if (!event.altKey) {
      if (lastPoint === lastUncommittedPoint) {
-        LinearElementEditor.deletePoints(element, app.scene, [
-          points.length - 1,
-        ]);
+        LinearElementEditor.deletePoints(element, app, [points.length - 1]);
      }
      return {
        ...appState.editingLinearElement,
@@ -999,7 +1045,7 @@ export class LinearElementEditor {
        ]),
      );
    } else {
-      LinearElementEditor.addPoints(element, app.scene, [{ point: newPoint }]);
+      LinearElementEditor.addPoints(element, app.scene, [newPoint]);
    }
    return {
      ...appState.editingLinearElement,
@@ -1142,40 +1188,23 @@ export class LinearElementEditor {

  /**
   * Normalizes line points so that the start point is at [0,0]. This is
-   * expected in various parts of the codebase. Also returns new x/y to account
-   * for the potential normalization.
+   * expected in various parts of the codebase.
+   *
+   * Also returns normalized x and y coords to account for the normalization
+   * of the points.
   */
-  static getNormalizedPoints(element: ExcalidrawLinearElement): {
-    points: LocalPoint[];
-    x: number;
-    y: number;
-  } {
-    const { points } = element;
-
-    const offsetX = points[0][0];
-    const offsetY = points[0][1];
+  static getNormalizeElementPointsAndCoords(element: ExcalidrawLinearElement) {
+    const { points, offsetX, offsetY } = getNormalizedPoints(element);

    return {
-      points: points.map((p) => {
-        return pointFrom(p[0] - offsetX, p[1] - offsetY);
-      }),
+      points,
      x: element.x + offsetX,
      y: element.y + offsetY,
    };
  }
+
  // element-mutating methods
  // ---------------------------------------------------------------------------
-  static normalizePoints(
-    element: NonDeleted<ExcalidrawLinearElement>,
-    elementsMap: ElementsMap,
-  ) {
-    mutateElement(
-      element,
-      elementsMap,
-      LinearElementEditor.getNormalizedPoints(element),
-    );
-  }
-
  static duplicateSelectedPoints(appState: AppState, scene: Scene): AppState {
    invariant(
      appState.editingLinearElement,
@@ -1254,41 +1283,47 @@ export class LinearElementEditor {

  static deletePoints(
    element: NonDeleted<ExcalidrawLinearElement>,
-    scene: Scene,
+    app: AppClassProperties,
    pointIndices: readonly number[],
  ) {
-    let offsetX = 0;
-    let offsetY = 0;
+    const isUncommittedPoint =
+      app.state.editingLinearElement?.lastUncommittedPoint ===
+      element.points[element.points.length - 1];

-    const isDeletingOriginPoint = pointIndices.includes(0);
+    const isPolygon = isLineElement(element) && element.polygon;

-    // if deleting first point, make the next to be [0,0] and recalculate
-    // positions of the rest with respect to it
-    if (isDeletingOriginPoint) {
-      const firstNonDeletedPoint = element.points.find((point, idx) => {
-        return !pointIndices.includes(idx);
-      });
-      if (firstNonDeletedPoint) {
-        offsetX = firstNonDeletedPoint[0];
-        offsetY = firstNonDeletedPoint[1];
-      }
+    // break polygon if deleting start/end point
+    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.reduce((acc: LocalPoint[], p, idx) => {
-      if (!pointIndices.includes(idx)) {
-        acc.push(
-          !acc.length
-            ? pointFrom(0, 0)
-            : pointFrom(p[0] - offsetX, p[1] - offsetY),
-        );
-      }
-      return acc;
-    }, []);
+    const nextPoints = element.points.filter((_, idx) => {
+      return !pointIndices.includes(idx);
+    });
+
+    if (isUncommittedPoint && isLineElement(element) && element.polygon) {
+      nextPoints[0] = pointFrom(
+        nextPoints[nextPoints.length - 1][0],
+        nextPoints[nextPoints.length - 1][1],
+      );
+    }
+
+    const {
+      points: normalizedPoints,
+      offsetX,
+      offsetY,
+    } = getNormalizedPoints({ points: nextPoints });

    LinearElementEditor._updatePoints(
      element,
-      scene,
-      nextPoints,
+      app.scene,
+      normalizedPoints,
      offsetX,
      offsetY,
    );
@@ -1297,16 +1332,27 @@ export class LinearElementEditor {
  static addPoints(
    element: NonDeleted<ExcalidrawLinearElement>,
    scene: Scene,
-    targetPoints: { point: LocalPoint }[],
+    addedPoints: LocalPoint[],
  ) {
-    const offsetX = 0;
-    const offsetY = 0;
+    const nextPoints = [...element.points, ...addedPoints];
+
+    if (isLineElement(element) && element.polygon) {
+      nextPoints[0] = pointFrom(
+        nextPoints[nextPoints.length - 1][0],
+        nextPoints[nextPoints.length - 1][1],
+      );
+    }
+
+    const {
+      points: normalizedPoints,
+      offsetX,
+      offsetY,
+    } = getNormalizedPoints({ points: nextPoints });

-    const nextPoints = [...element.points, ...targetPoints.map((x) => x.point)];
    LinearElementEditor._updatePoints(
      element,
      scene,
-      nextPoints,
+      normalizedPoints,
      offsetX,
      offsetY,
    );
@@ -1323,17 +1369,37 @@ export class LinearElementEditor {
  ) {
    const { points } = element;

+    // if polygon, move start and end points together
+    if (isLineElement(element) && element.polygon) {
+      const firstPointUpdate = pointUpdates.get(0);
+      const lastPointUpdate = pointUpdates.get(points.length - 1);
+
+      if (firstPointUpdate) {
+        pointUpdates.set(points.length - 1, {
+          point: pointFrom(
+            firstPointUpdate.point[0],
+            firstPointUpdate.point[1],
+          ),
+          isDragging: firstPointUpdate.isDragging,
+        });
+      } else if (lastPointUpdate) {
+        pointUpdates.set(0, {
+          point: pointFrom(lastPointUpdate.point[0], lastPointUpdate.point[1]),
+          isDragging: lastPointUpdate.isDragging,
+        });
+      }
+    }
+
    // in case we're moving start point, instead of modifying its position
    // which would break the invariant of it being at [0,0], we move
    // all the other points in the opposite direction by delta to
    // offset it. We do the same with actual element.x/y position, so
    // this hacks are completely transparent to the user.
-    const [deltaX, deltaY] =
+
+    const updatedOriginPoint =
      pointUpdates.get(0)?.point ?? pointFrom<LocalPoint>(0, 0);
-    const [offsetX, offsetY] = pointFrom<LocalPoint>(
-      deltaX - points[0][0],
-      deltaY - points[0][1],
-    );
+
+    const [offsetX, offsetY] = updatedOriginPoint;

    const nextPoints = isElbowArrow(element)
      ? [
@@ -1503,6 +1569,7 @@ export class LinearElementEditor {
        isDragging: options?.isDragging ?? false,
      });
    } else {
+      // TODO do we need to get precise coords here just to calc centers?
      const nextCoords = getElementPointsCoords(element, nextPoints);
      const prevCoords = getElementPointsCoords(element, element.points);
      const nextCenterX = (nextCoords[0] + nextCoords[2]) / 2;
@@ -1511,7 +1578,7 @@ export class LinearElementEditor {
      const prevCenterY = (prevCoords[1] + prevCoords[3]) / 2;
      const dX = prevCenterX - nextCenterX;
      const dY = prevCenterY - nextCenterY;
-      const rotated = pointRotateRads(
+      const rotatedOffset = pointRotateRads(
        pointFrom(offsetX, offsetY),
        pointFrom(dX, dY),
        element.angle,
@@ -1519,8 +1586,8 @@ export class LinearElementEditor {
      scene.mutateElement(element, {
        ...otherUpdates,
        points: nextPoints,
-        x: element.x + rotated[0],
-        y: element.y + rotated[1],
+        x: element.x + rotatedOffset[0],
+        y: element.y + rotatedOffset[1],
      });
    }
  }
--- a/packages/element/src/newElement.ts
+++ b/packages/element/src/newElement.ts
@@ -25,6 +25,8 @@ import { getBoundTextMaxWidth } from "./textElement";
 import { normalizeText, measureText } from "./textMeasurements";
 import { wrapText } from "./textWrapping";

+import { isLineElement } from "./typeChecks";
+
 import type {
  ExcalidrawElement,
  ExcalidrawImageElement,
@@ -45,6 +47,7 @@ import type {
  ElementsMap,
  ExcalidrawArrowElement,
  ExcalidrawElbowArrowElement,
+  ExcalidrawLineElement,
 } from "./types";

 export type ElementConstructorOpts = MarkOptional<
@@ -457,9 +460,10 @@ export const newLinearElement = (
  opts: {
    type: ExcalidrawLinearElement["type"];
    points?: ExcalidrawLinearElement["points"];
+    polygon?: ExcalidrawLineElement["polygon"];
  } & ElementConstructorOpts,
 ): NonDeleted<ExcalidrawLinearElement> => {
-  return {
+  const element = {
    ..._newElementBase<ExcalidrawLinearElement>(opts.type, opts),
    points: opts.points || [],
    lastCommittedPoint: null,
@@ -468,6 +472,17 @@ export const newLinearElement = (
    startArrowhead: null,
    endArrowhead: null,
  };
+
+  if (isLineElement(element)) {
+    const lineElement: NonDeleted<ExcalidrawLineElement> = {
+      ...element,
+      polygon: opts.polygon ?? false,
+    };
+
+    return lineElement;
+  }
+
+  return element;
 };

 export const newArrowElement = <T extends boolean>(
--- a/packages/element/src/shapes.ts
+++ b/packages/element/src/shapes.ts
@@ -5,6 +5,7 @@ import {
  ROUNDNESS,
  invariant,
  elementCenterPoint,
+  LINE_POLYGON_POINT_MERGE_DISTANCE,
 } from "@excalidraw/common";
 import {
  isPoint,
@@ -35,10 +36,13 @@ import { ShapeCache } from "./ShapeCache";

 import { getElementAbsoluteCoords, type Bounds } from "./bounds";

+import { canBecomePolygon } from "./typeChecks";
+
 import type {
  ElementsMap,
  ExcalidrawElement,
  ExcalidrawLinearElement,
+  ExcalidrawLineElement,
  NonDeleted,
 } from "./types";

@@ -396,3 +400,47 @@ export const isPathALoop = (
  }
  return false;
 };
+
+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];
+
+    const distance = Math.hypot(
+      firstPoint[0] - lastPoint[0],
+      firstPoint[1] - lastPoint[1],
+    );
+
+    if (
+      distance > LINE_POLYGON_POINT_MERGE_DISTANCE ||
+      updatedPoints.length < 4
+    ) {
+      updatedPoints.push(pointFrom(firstPoint[0], firstPoint[1]));
+    } else {
+      updatedPoints[updatedPoints.length - 1] = pointFrom(
+        firstPoint[0],
+        firstPoint[1],
+      );
+    }
+  }
+
+  // TODO: satisfies ElementUpdate<ExcalidrawLineElement>
+  const ret = {
+    polygon: nextPolygonState,
+    points: updatedPoints,
+  };
+
+  return ret;
+};
--- 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";
@@ -25,6 +27,7 @@ import type {
  ExcalidrawMagicFrameElement,
  ExcalidrawArrowElement,
  ExcalidrawElbowArrowElement,
+  ExcalidrawLineElement,
  PointBinding,
  FixedPointBinding,
  ExcalidrawFlowchartNodeElement,
@@ -108,6 +111,12 @@ export const isLinearElement = (
  return element != null && isLinearElementType(element.type);
 };

+export const isLineElement = (
+  element?: ExcalidrawElement | null,
+): element is ExcalidrawLineElement => {
+  return element != null && element.type === "line";
+};
+
 export const isArrowElement = (
  element?: ExcalidrawElement | null,
 ): element is ExcalidrawArrowElement => {
@@ -372,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]))
+  );
+};
--- a/packages/element/src/types.ts
+++ b/packages/element/src/types.ts
@@ -296,8 +296,10 @@ export type FixedPointBinding = Merge<
  }
 >;

+type Index = number;
+
 export type PointsPositionUpdates = Map<
-  number,
+  Index,
  { point: LocalPoint; isDragging?: boolean }
 >;

@@ -326,10 +328,16 @@ export type ExcalidrawLinearElement = _ExcalidrawElementBase &
    endArrowhead: Arrowhead | null;
  }>;

+export type ExcalidrawLineElement = ExcalidrawLinearElement &
+  Readonly<{
+    type: "line";
+    polygon: boolean;
+  }>;
+
 export type FixedSegment = {
  start: LocalPoint;
  end: LocalPoint;
-  index: number;
+  index: Index;
 };

 export type ExcalidrawArrowElement = ExcalidrawLinearElement &