Merge branch 'master' into ryan-di/mobile/new-layout

excalidraw-app/tests/MobileMenu.test.tsx

@@ -36,7 +36,7 @@ describe("Test MobileMenu", () => {
         },
         "isTouchScreen": false,
         "viewport": {
-          "isLandscape": false,
+          "isLandscape": true,
           "isMobile": true,
         },
       }

packages/common/src/constants.ts

@@ -347,15 +347,12 @@ export const DEFAULT_UI_OPTIONS: AppProps["UIOptions"] = {
 
 // breakpoints
 // -----------------------------------------------------------------------------
-// md screen
-export const MQ_MAX_WIDTH_LANDSCAPE = 1000;
-export const MQ_MAX_HEIGHT_LANDSCAPE = 500;
 
 // mobile: up to 699px
-export const MQ_MAX_WIDTH_MOBILE = 699;
+export const MQ_MAX_MOBILE = 599;
 
 // tablets
-export const MQ_MIN_TABLET = 600; // lower bound (excludes phones)
+export const MQ_MIN_TABLET = MQ_MAX_MOBILE + 1; // lower bound (excludes phones)
 export const MQ_MAX_TABLET = 1400; // upper bound (excludes laptops/desktops)
 
 // desktop/laptop

packages/element/src/renderElement.ts

@@ -1,7 +1,14 @@
 import rough from "roughjs/bin/rough";
 import { getStroke } from "perfect-freehand";
 
-import { isRightAngleRads } from "@excalidraw/math";
+import {
+  type GlobalPoint,
+  isRightAngleRads,
+  lineSegment,
+  pointFrom,
+  pointRotateRads,
+  type Radians,
+} from "@excalidraw/math";
 
 import {
   BOUND_TEXT_PADDING,
@@ -14,6 +21,7 @@ import {
   getFontString,
   isRTL,
   getVerticalOffset,
+  invariant,
 } from "@excalidraw/common";
 
 import type {
@@ -32,7 +40,7 @@ import type {
   InteractiveCanvasRenderConfig,
 } from "@excalidraw/excalidraw/scene/types";
 
-import { getElementAbsoluteCoords } from "./bounds";
+import { getElementAbsoluteCoords, getElementBounds } from "./bounds";
 import { getUncroppedImageElement } from "./cropElement";
 import { LinearElementEditor } from "./linearElementEditor";
 import {
@@ -1039,6 +1047,66 @@ export function getFreeDrawPath2D(element: ExcalidrawFreeDrawElement) {
 }
 
 export function getFreeDrawSvgPath(element: ExcalidrawFreeDrawElement) {
+  return getSvgPathFromStroke(getFreedrawOutlinePoints(element));
+}
+
+export function getFreedrawOutlineAsSegments(
+  element: ExcalidrawFreeDrawElement,
+  points: [number, number][],
+  elementsMap: ElementsMap,
+) {
+  const bounds = getElementBounds(
+    {
+      ...element,
+      angle: 0 as Radians,
+    },
+    elementsMap,
+  );
+  const center = pointFrom<GlobalPoint>(
+    (bounds[0] + bounds[2]) / 2,
+    (bounds[1] + bounds[3]) / 2,
+  );
+
+  invariant(points.length >= 2, "Freepath outline must have at least 2 points");
+
+  return points.slice(2).reduce(
+    (acc, curr) => {
+      acc.push(
+        lineSegment<GlobalPoint>(
+          acc[acc.length - 1][1],
+          pointRotateRads(
+            pointFrom<GlobalPoint>(curr[0] + element.x, curr[1] + element.y),
+            center,
+            element.angle,
+          ),
+        ),
+      );
+      return acc;
+    },
+    [
+      lineSegment<GlobalPoint>(
+        pointRotateRads(
+          pointFrom<GlobalPoint>(
+            points[0][0] + element.x,
+            points[0][1] + element.y,
+          ),
+          center,
+          element.angle,
+        ),
+        pointRotateRads(
+          pointFrom<GlobalPoint>(
+            points[1][0] + element.x,
+            points[1][1] + element.y,
+          ),
+          center,
+          element.angle,
+        ),
+      ),
+    ],
+  );
+}
+
+export function getFreedrawOutlinePoints(element: ExcalidrawFreeDrawElement) {
   // If input points are empty (should they ever be?) return a dot
   const inputPoints = element.simulatePressure
     ? element.points
@@ -1057,7 +1125,7 @@ export function getFreeDrawSvgPath(element: ExcalidrawFreeDrawElement) {
     last: !!element.lastCommittedPoint, // LastCommittedPoint is added on pointerup
   };
 
-  return getSvgPathFromStroke(getStroke(inputPoints as number[][], options));
+  return getStroke(inputPoints as number[][], options) as [number, number][];
 }
 
 function med(A: number[], B: number[]) {

packages/excalidraw/components/App.tsx

@@ -100,9 +100,7 @@ import {
   MINIMUM_ARROW_SIZE,
   DOUBLE_TAP_POSITION_THRESHOLD,
   isMobileOrTablet,
-  MQ_MAX_WIDTH_MOBILE,
-  MQ_MAX_HEIGHT_LANDSCAPE,
-  MQ_MAX_WIDTH_LANDSCAPE,
+  MQ_MAX_MOBILE,
   MQ_MIN_TABLET,
   MQ_MAX_TABLET,
 } from "@excalidraw/common";
@@ -2423,10 +2421,8 @@ class App extends React.Component<AppProps, AppState> {
   };
 
   private isMobileBreakpoint = (width: number, height: number) => {
-    return (
-      width <= MQ_MAX_WIDTH_MOBILE ||
-      (height < MQ_MAX_HEIGHT_LANDSCAPE && width < MQ_MAX_WIDTH_LANDSCAPE)
-    );
+    const minSide = Math.min(width, height);
+    return minSide <= MQ_MAX_MOBILE;
   };
 
   private isTabletBreakpoint = (editorWidth: number, editorHeight: number) => {
@@ -2442,14 +2438,14 @@ class App extends React.Component<AppProps, AppState> {
       return;
     }
 
-    const { clientWidth: viewportWidth, clientHeight: viewportHeight } =
-      document.body;
+    const { width: editorWidth, height: editorHeight } =
+      container.getBoundingClientRect();
 
     const prevViewportState = this.device.viewport;
 
     const nextViewportState = updateObject(prevViewportState, {
-      isLandscape: viewportWidth > viewportHeight,
-      isMobile: this.isMobileBreakpoint(viewportWidth, viewportHeight),
+      isLandscape: editorWidth > editorHeight,
+      isMobile: this.isMobileBreakpoint(editorWidth, editorHeight),
     });
 
     if (prevViewportState !== nextViewportState) {

packages/excalidraw/eraser/index.ts

@@ -1,11 +1,26 @@
 import { arrayToMap, easeOut, THEME } from "@excalidraw/common";
+
 import {
   computeBoundTextPosition,
+  distanceToElement,
+  doBoundsIntersect,
   getBoundTextElement,
+  getElementBounds,
+  getFreedrawOutlineAsSegments,
+  getFreedrawOutlinePoints,
   intersectElementWithLineSegment,
+  isArrowElement,
+  isFreeDrawElement,
+  isLineElement,
   isPointInElement,
 } from "@excalidraw/element";
-import { lineSegment, pointFrom } from "@excalidraw/math";
+import {
+  lineSegment,
+  lineSegmentsDistance,
+  pointFrom,
+  polygon,
+  polygonIncludesPointNonZero,
+} from "@excalidraw/math";
 
 import { getElementsInGroup } from "@excalidraw/element";
 
@@ -13,6 +28,8 @@ import { shouldTestInside } from "@excalidraw/element";
 import { hasBoundTextElement, isBoundToContainer } from "@excalidraw/element";
 import { getBoundTextElementId } from "@excalidraw/element";
 
+import type { Bounds } from "@excalidraw/element";
+
 import type { GlobalPoint, LineSegment } from "@excalidraw/math/types";
 import type { ElementsMap, ExcalidrawElement } from "@excalidraw/element/types";
 
@@ -96,6 +113,7 @@ export class EraserTrail extends AnimatedTrail {
           pathSegment,
           element,
           candidateElementsMap,
+          this.app.state.zoom.value,
         );
 
         if (intersects) {
@@ -131,6 +149,7 @@ export class EraserTrail extends AnimatedTrail {
           pathSegment,
           element,
           candidateElementsMap,
+          this.app.state.zoom.value,
         );
 
         if (intersects) {
@@ -180,8 +199,33 @@ const eraserTest = (
   pathSegment: LineSegment<GlobalPoint>,
   element: ExcalidrawElement,
   elementsMap: ElementsMap,
+  zoom: number,
 ): boolean => {
   const lastPoint = pathSegment[1];
+
+  // PERF: Do a quick bounds intersection test first because it's cheap
+  const threshold = isFreeDrawElement(element) ? 15 : element.strokeWidth / 2;
+  const segmentBounds = [
+    Math.min(pathSegment[0][0], pathSegment[1][0]) - threshold,
+    Math.min(pathSegment[0][1], pathSegment[1][1]) - threshold,
+    Math.max(pathSegment[0][0], pathSegment[1][0]) + threshold,
+    Math.max(pathSegment[0][1], pathSegment[1][1]) + threshold,
+  ] as Bounds;
+  const origElementBounds = getElementBounds(element, elementsMap);
+  const elementBounds: Bounds = [
+    origElementBounds[0] - threshold,
+    origElementBounds[1] - threshold,
+    origElementBounds[2] + threshold,
+    origElementBounds[3] + threshold,
+  ];
+
+  if (!doBoundsIntersect(segmentBounds, elementBounds)) {
+    return false;
+  }
+
+  // There are shapes where the inner area should trigger erasing
+  // even though the eraser path segment doesn't intersect with or
+  // get close to the shape's stroke
   if (
     shouldTestInside(element) &&
     isPointInElement(lastPoint, element, elementsMap)
@@ -189,6 +233,50 @@ const eraserTest = (
     return true;
   }
 
+  // Freedraw elements are tested for erasure by measuring the distance
+  // of the eraser path and the freedraw shape outline lines to a tolerance
+  // which offers a good visual precision at various zoom levels
+  if (isFreeDrawElement(element)) {
+    const outlinePoints = getFreedrawOutlinePoints(element);
+    const strokeSegments = getFreedrawOutlineAsSegments(
+      element,
+      outlinePoints,
+      elementsMap,
+    );
+    const tolerance = Math.max(2.25, 5 / zoom); // NOTE: Visually fine-tuned approximation
+
+    for (const seg of strokeSegments) {
+      if (lineSegmentsDistance(seg, pathSegment) <= tolerance) {
+        return true;
+      }
+    }
+
+    const poly = polygon(
+      ...(outlinePoints.map(([x, y]) =>
+        pointFrom<GlobalPoint>(element.x + x, element.y + y),
+      ) as GlobalPoint[]),
+    );
+
+    // PERF: Check only one point of the eraser segment. If the eraser segment
+    // start is inside the closed freedraw shape, the other point is either also
+    // inside or the eraser segment will intersect the shape outline anyway
+    if (polygonIncludesPointNonZero(pathSegment[0], poly)) {
+      return true;
+    }
+
+    return false;
+  } else if (
+    isArrowElement(element) ||
+    (isLineElement(element) && !element.polygon)
+  ) {
+    const tolerance = Math.max(
+      element.strokeWidth,
+      (element.strokeWidth * 2) / zoom,
+    );
+
+    return distanceToElement(element, elementsMap, lastPoint) <= tolerance;
+  }
+
   const boundTextElement = getBoundTextElement(element, elementsMap);
 
   return (

packages/math/src/segment.ts

@@ -177,3 +177,19 @@ export function lineSegmentIntersectionPoints<
 
   return candidate;
 }
+
+export function lineSegmentsDistance<Point extends GlobalPoint | LocalPoint>(
+  s1: LineSegment<Point>,
+  s2: LineSegment<Point>,
+): number {
+  if (lineSegmentIntersectionPoints(s1, s2)) {
+    return 0;
+  }
+
+  return Math.min(
+    distanceToLineSegment(s1[0], s2),
+    distanceToLineSegment(s1[1], s2),
+    distanceToLineSegment(s2[0], s1),
+    distanceToLineSegment(s2[1], s1),
+  );
+}