Fill a looped curve with the selected background color (#1315)

package-lock.json

@@ -13437,9 +13437,9 @@
       }
     },
     "roughjs": {
-      "version": "4.0.4",
+      "version": "4.1.3",
-      "resolved": "https://registry.npmjs.org/roughjs/-/roughjs-4.0.4.tgz",
+      "resolved": "https://registry.npmjs.org/roughjs/-/roughjs-4.1.3.tgz",
-      "integrity": "sha512-rXmMGcALUlYIFKBbn9aWuxznPKOtnx9bouVC407/uneUNx0mT/4Mo2Z4TUieoCOT+rWmHnOQqVT1FvoN+L3baA=="
+      "integrity": "sha512-tpmMIBuiPTImvvyFr/ZYwHqIRJU+a2KmHvqAIfiPG0jIx8xmVuIU3QqL0UQ0jDxwfIJJJYEobgaYtkvUai2+/A=="
     },
     "rsvp": {
       "version": "4.8.5",

package.json

@@ -27,7 +27,7 @@
     "react": "16.13.1",
     "react-dom": "16.13.1",
     "react-scripts": "3.4.1",
-    "roughjs": "4.0.4",
+    "roughjs": "4.1.3",
     "socket.io-client": "2.3.0"
   },
   "devDependencies": {

src/actions/actionFinalize.tsx

@@ -7,6 +7,7 @@ import { done } from "../components/icons";
 import { t } from "../i18n";
 import { register } from "./register";
 import { mutateElement } from "../element/mutateElement";
+import { isPathALoop } from "../math";
 
 export const actionFinalize = register({
   name: "finalize",
@@ -32,6 +33,23 @@ export const actionFinalize = register({
         newElements = newElements.slice(0, -1);
       }
 
+      // If the multi point line closes the loop,
+      // set the last point to first point.
+      // This ensures that loop remains closed at different scales.
+      if (appState.multiElement.type === "line") {
+        if (isPathALoop(appState.multiElement.points)) {
+          const linePoints = appState.multiElement.points;
+          const firstPoint = linePoints[0];
+          mutateElement(appState.multiElement, {
+            points: linePoints.map((point, i) =>
+              i === linePoints.length - 1
+                ? ([firstPoint[0], firstPoint[1]] as const)
+                : point,
+            ),
+          });
+        }
+      }
+
       if (!appState.elementLocked) {
         appState.selectedElementIds[appState.multiElement.id] = true;
       }

src/components/App.tsx

@@ -54,13 +54,14 @@ import { renderScene } from "../renderer";
 import { AppState, GestureEvent, Gesture } from "../types";
 import { ExcalidrawElement, ExcalidrawTextElement } from "../element/types";
 
+import { distance2d, isPathALoop } from "../math";
+
 import {
   isWritableElement,
   isInputLike,
   isToolIcon,
   debounce,
   distance,
-  distance2d,
   resetCursor,
   viewportCoordsToSceneCoords,
   sceneCoordsToViewportCoords,
@@ -97,7 +98,7 @@ import {
   POINTER_BUTTON,
   DRAGGING_THRESHOLD,
   TEXT_TO_CENTER_SNAP_THRESHOLD,
-  ARROW_CONFIRM_THRESHOLD,
+  LINE_CONFIRM_THRESHOLD,
 } from "../constants";
 import { LayerUI } from "./LayerUI";
 import { ScrollBars, SceneState } from "../scene/types";
@@ -1456,7 +1457,7 @@ export class App extends React.Component<any, AppState> {
         //  threshold, add a point
         if (
           distance2d(x - rx, y - ry, lastPoint[0], lastPoint[1]) >=
-          ARROW_CONFIRM_THRESHOLD
+          LINE_CONFIRM_THRESHOLD
         ) {
           mutateElement(multiElement, {
             points: [...points, [x - rx, y - ry]],
@@ -1477,13 +1478,16 @@ export class App extends React.Component<any, AppState> {
             y - ry,
             lastCommittedPoint[0],
             lastCommittedPoint[1],
-          ) < ARROW_CONFIRM_THRESHOLD
+          ) < LINE_CONFIRM_THRESHOLD
         ) {
           document.documentElement.style.cursor = CURSOR_TYPE.POINTER;
           mutateElement(multiElement, {
             points: points.slice(0, -1),
           });
         } else {
+          if (isPathALoop(points)) {
+            document.documentElement.style.cursor = CURSOR_TYPE.POINTER;
+          }
           // update last uncommitted point
           mutateElement(multiElement, {
             points: [...points.slice(0, -1), [x - rx, y - ry]],
@@ -1875,6 +1879,16 @@ export class App extends React.Component<any, AppState> {
       if (this.state.multiElement) {
         const { multiElement } = this.state;
 
+        // finalize if completing a loop
+        if (multiElement.type === "line" && isPathALoop(multiElement.points)) {
+          mutateElement(multiElement, {
+            lastCommittedPoint:
+              multiElement.points[multiElement.points.length - 1],
+          });
+          this.actionManager.executeAction(actionFinalize);
+          return;
+        }
+
         const { x: rx, y: ry, lastCommittedPoint } = multiElement;
 
         // clicking inside commit zone → finalize arrow
@@ -1886,11 +1900,12 @@ export class App extends React.Component<any, AppState> {
             y - ry,
             lastCommittedPoint[0],
             lastCommittedPoint[1],
-          ) < ARROW_CONFIRM_THRESHOLD
+          ) < LINE_CONFIRM_THRESHOLD
         ) {
           this.actionManager.executeAction(actionFinalize);
           return;
         }
+
         this.setState((prevState) => ({
           selectedElementIds: {
             ...prevState.selectedElementIds,

src/constants.ts

@@ -1,5 +1,5 @@
 export const DRAGGING_THRESHOLD = 10; // 10px
-export const ARROW_CONFIRM_THRESHOLD = 10; // 10px
+export const LINE_CONFIRM_THRESHOLD = 10; // 10px
 export const ELEMENT_SHIFT_TRANSLATE_AMOUNT = 5;
 export const ELEMENT_TRANSLATE_AMOUNT = 1;
 export const TEXT_TO_CENTER_SNAP_THRESHOLD = 30;

src/element/bounds.ts

@@ -1,6 +1,6 @@
 import { ExcalidrawElement, ExcalidrawLinearElement } from "./types";
 import { rotate } from "../math";
-import { Drawable } from "roughjs/bin/core";
+import { Drawable, Op } from "roughjs/bin/core";
 import { Point } from "../types";
 import { getShapeForElement } from "../renderer/renderElement";
 import { isLinearElement } from "./typeChecks";
@@ -36,6 +36,15 @@ export function getDiamondPoints(element: ExcalidrawElement) {
   return [topX, topY, rightX, rightY, bottomX, bottomY, leftX, leftY];
 }
 
+export function getCurvePathOps(shape: Drawable): Op[] {
+  for (const set of shape.sets) {
+    if (set.type === "path") {
+      return set.ops;
+    }
+  }
+  return shape.sets[0].ops;
+}
+
 export function getLinearElementAbsoluteBounds(
   element: ExcalidrawLinearElement,
 ): [number, number, number, number] {
@@ -63,7 +72,7 @@ export function getLinearElementAbsoluteBounds(
   const shape = getShapeForElement(element) as Drawable[];
 
   // first element is always the curve
-  const ops = shape[0].sets[0].ops;
+  const ops = getCurvePathOps(shape[0]);
 
   let currentP: Point = [0, 0];
 
@@ -128,7 +137,7 @@ export function getArrowPoints(
   element: ExcalidrawLinearElement,
   shape: Drawable[],
 ) {
-  const ops = shape[0].sets[0].ops;
+  const ops = getCurvePathOps(shape[0]);
 
   const data = ops[ops.length - 1].data;
   const p3 = [data[4], data[5]] as Point;

src/element/collision.ts

@@ -1,23 +1,35 @@
-import { distanceBetweenPointAndSegment } from "../math";
+import {
+  distanceBetweenPointAndSegment,
+  isPathALoop,
+  rotate,
+  isPointInPolygon,
+} from "../math";
+import { getPointsOnBezierCurves } from "roughjs/bin/geometry";
 
 import { NonDeletedExcalidrawElement } from "./types";
 
-import { getDiamondPoints, getElementAbsoluteCoords } from "./bounds";
+import {
+  getDiamondPoints,
+  getElementAbsoluteCoords,
+  getCurvePathOps,
+} from "./bounds";
 import { Point } from "../types";
-import { Drawable, OpSet } from "roughjs/bin/core";
+import { Drawable } from "roughjs/bin/core";
 import { AppState } from "../types";
 import { getShapeForElement } from "../renderer/renderElement";
 import { isLinearElement } from "./typeChecks";
-import { rotate } from "../math";
 
 function isElementDraggableFromInside(
   element: NonDeletedExcalidrawElement,
   appState: AppState,
 ): boolean {
-  return (
+  const dragFromInside =
     element.backgroundColor !== "transparent" ||
-    appState.selectedElementIds[element.id]
+    appState.selectedElementIds[element.id];
-  );
+  if (element.type === "line") {
+    return dragFromInside && isPathALoop(element.points);
+  }
+  return dragFromInside;
 }
 
 export function hitTest(
@@ -178,9 +190,18 @@ export function hitTest(
     const relX = x - element.x;
     const relY = y - element.y;
 
+    if (isElementDraggableFromInside(element, appState)) {
+      const hit = shape.some((subshape) =>
+        hitTestCurveInside(subshape, relX, relY, lineThreshold),
+      );
+      if (hit) {
+        return true;
+      }
+    }
+
     // hit thest all "subshapes" of the linear element
     return shape.some((subshape) =>
-      hitTestRoughShape(subshape.sets, relX, relY, lineThreshold),
+      hitTestRoughShape(subshape, relX, relY, lineThreshold),
     );
   } else if (element.type === "text") {
     return x >= x1 && x <= x2 && y >= y1 && y <= y2;
@@ -224,14 +245,41 @@ const pointInBezierEquation = (
   return false;
 };
 
+const hitTestCurveInside = (
+  drawable: Drawable,
+  x: number,
+  y: number,
+  lineThreshold: number,
+) => {
+  const ops = getCurvePathOps(drawable);
+  const points: Point[] = [];
+  for (const operation of ops) {
+    if (operation.op === "move") {
+      if (points.length) {
+        break;
+      }
+      points.push([operation.data[0], operation.data[1]]);
+    } else if (operation.op === "bcurveTo") {
+      points.push([operation.data[0], operation.data[1]]);
+      points.push([operation.data[2], operation.data[3]]);
+      points.push([operation.data[4], operation.data[5]]);
+    }
+  }
+  if (points.length >= 4) {
+    const polygonPoints = getPointsOnBezierCurves(points as any, 50);
+    return isPointInPolygon(polygonPoints, x, y);
+  }
+  return false;
+};
+
 const hitTestRoughShape = (
-  opSet: OpSet[],
+  drawable: Drawable,
   x: number,
   y: number,
   lineThreshold: number,
 ) => {
   // read operations from first opSet
-  const ops = opSet[0].ops;
+  const ops = getCurvePathOps(drawable);
 
   // set start position as (0,0) just in case
   // move operation does not exist (unlikely but it is worth safekeeping it)

src/math.ts

@@ -1,4 +1,5 @@
 import { Point } from "./types";
+import { LINE_CONFIRM_THRESHOLD } from "./constants";
 
 // https://stackoverflow.com/a/6853926/232122
 export function distanceBetweenPointAndSegment(
@@ -144,3 +145,109 @@ export const getPointOnAPath = (point: Point, path: Point[]) => {
 
   return null;
 };
+
+export function distance2d(x1: number, y1: number, x2: number, y2: number) {
+  const xd = x2 - x1;
+  const yd = y2 - y1;
+  return Math.hypot(xd, yd);
+}
+
+// Checks if the first and last point are close enough
+// to be considered a loop
+export function isPathALoop(points: Point[]): boolean {
+  if (points.length >= 3) {
+    const [firstPoint, lastPoint] = [points[0], points[points.length - 1]];
+    return (
+      distance2d(firstPoint[0], firstPoint[1], lastPoint[0], lastPoint[1]) <=
+      LINE_CONFIRM_THRESHOLD
+    );
+  }
+  return false;
+}
+
+// Draw a line from the point to the right till infiinty
+// Check how many lines of the polygon does this infinite line intersects with
+// If the number of intersections is odd, point is in the polygon
+export function isPointInPolygon(
+  points: Point[],
+  x: number,
+  y: number,
+): boolean {
+  const vertices = points.length;
+
+  // There must be at least 3 vertices in polygon
+  if (vertices < 3) {
+    return false;
+  }
+  const extreme: Point = [Number.MAX_SAFE_INTEGER, y];
+  const p: Point = [x, y];
+  let count = 0;
+  for (let i = 0; i < vertices; i++) {
+    const current = points[i];
+    const next = points[(i + 1) % vertices];
+    if (doIntersect(current, next, p, extreme)) {
+      if (orientation(current, p, next) === 0) {
+        return onSegment(current, p, next);
+      }
+      count++;
+    }
+  }
+  // true if count is off
+  return count % 2 === 1;
+}
+
+// Check if q lies on the line segment pr
+function onSegment(p: Point, q: Point, r: Point) {
+  return (
+    q[0] <= Math.max(p[0], r[0]) &&
+    q[0] >= Math.min(p[0], r[0]) &&
+    q[1] <= Math.max(p[1], r[1]) &&
+    q[1] >= Math.min(p[1], r[1])
+  );
+}
+
+// For the ordered points p, q, r, return
+// 0 if p, q, r are collinear
+// 1 if Clockwise
+// 2 if counterclickwise
+function orientation(p: Point, q: Point, r: Point) {
+  const val = (q[1] - p[1]) * (r[0] - q[0]) - (q[0] - p[0]) * (r[1] - q[1]);
+  if (val === 0) {
+    return 0;
+  }
+  return val > 0 ? 1 : 2;
+}
+
+// Check is p1q1 intersects with p2q2
+function doIntersect(p1: Point, q1: Point, p2: Point, q2: Point) {
+  const o1 = orientation(p1, q1, p2);
+  const o2 = orientation(p1, q1, q2);
+  const o3 = orientation(p2, q2, p1);
+  const o4 = orientation(p2, q2, q1);
+
+  if (o1 !== o2 && o3 !== o4) {
+    return true;
+  }
+
+  // p1, q1 and p2 are colinear and p2 lies on segment p1q1
+  if (o1 === 0 && onSegment(p1, p2, q1)) {
+    return true;
+  }
+
+  // p1, q1 and p2 are colinear and q2 lies on segment p1q1
+  if (o2 === 0 && onSegment(p1, q2, q1)) {
+    return true;
+  }
+
+  // p2, q2 and p1 are colinear and p1 lies on segment p2q2
+  if (o3 === 0 && onSegment(p2, p1, q2)) {
+    return true;
+  }
+
+  // p2, q2 and q1 are colinear and q1 lies on segment p2q2
+  if (o4 === 0 && onSegment(p2, q1, q2)) {
+    return true;
+  }
+
+  return false;
+}

src/renderer/renderElement.ts

@@ -10,11 +10,12 @@ import {
   getElementAbsoluteCoords,
 } from "../element/bounds";
 import { RoughCanvas } from "roughjs/bin/canvas";
-import { Drawable } from "roughjs/bin/core";
+import { Drawable, Options } from "roughjs/bin/core";
 import { RoughSVG } from "roughjs/bin/svg";
 import { RoughGenerator } from "roughjs/bin/generator";
 import { SceneState } from "../scene/types";
 import { SVG_NS, distance } from "../utils";
+import { isPathALoop } from "../math";
 import rough from "roughjs/bin/rough";
 
 const CANVAS_PADDING = 20;
@@ -226,16 +227,29 @@ function generateElement(
         break;
       case "line":
       case "arrow": {
-        const options = {
+        const options: Options = {
           stroke: element.strokeColor,
           strokeWidth: element.strokeWidth,
           roughness: element.roughness,
           seed: element.seed,
         };
+
         // points array can be empty in the beginning, so it is important to add
         // initial position to it
         const points = element.points.length ? element.points : [[0, 0]];
 
+        // If shape is a line and is a closed shape,
+        // fill the shape if a color is set.
+        if (element.type === "line") {
+          if (isPathALoop(element.points)) {
+            options.fillStyle = element.fillStyle;
+            options.fill =
+              element.backgroundColor === "transparent"
+                ? undefined
+                : element.backgroundColor;
+          }
+        }
+
         // curve is always the first element
         // this simplifies finding the curve for an element
         shape = [generator.curve(points as [number, number][], options)];

src/scene/comparisons.ts

@@ -7,7 +7,10 @@ import { getElementAbsoluteCoords, hitTest } from "../element";
 import { AppState } from "../types";
 
 export const hasBackground = (type: string) =>
-  type === "rectangle" || type === "ellipse" || type === "diamond";
+  type === "rectangle" ||
+  type === "ellipse" ||
+  type === "diamond" ||
+  type === "line";
 
 export const hasStroke = (type: string) =>
   type === "rectangle" ||

src/utils.ts

@@ -134,12 +134,6 @@ export function distance(x: number, y: number) {
   return Math.abs(x - y);
 }
 
-export function distance2d(x1: number, y1: number, x2: number, y2: number) {
-  const xd = x2 - x1;
-  const yd = y2 - y1;
-  return Math.hypot(xd, yd);
-}
-
 export function resetCursor() {
   document.documentElement.style.cursor = "";
 }