diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index 8eb2a9e2cd..dc6d287828 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -8437,21 +8437,78 @@ class App extends React.Component { const elements = this.scene.getElementsIncludingDeleted(); for (const element of elements) { - if ( + const isInSelection = selectedElementIds.has(element.id) || // case: the state.selectedElementIds might not have been // updated yet by the time this mousemove event is fired (element.id === hitElement?.id && - pointerDownState.hit.wasAddedToSelection) + pointerDownState.hit.wasAddedToSelection); + // NOTE (mtolmacs): This is a temporary fix for very large scenes + if ( + Math.abs(element.x) > 1e7 || + Math.abs(element.x) > 1e7 || + Math.abs(element.width) > 1e7 || + Math.abs(element.height) > 1e7 ) { + console.error( + `Alt+dragging element in scene with invalid dimensions`, + element.x, + element.y, + element.width, + element.height, + isInSelection, + ); + + return; + } + + if (isInSelection) { const duplicatedElement = duplicateElement( this.state.editingGroupId, groupIdMap, element, ); + + // NOTE (mtolmacs): This is a temporary fix for very large scenes + if ( + Math.abs(duplicatedElement.x) > 1e7 || + Math.abs(duplicatedElement.x) > 1e7 || + Math.abs(duplicatedElement.width) > 1e7 || + Math.abs(duplicatedElement.height) > 1e7 + ) { + console.error( + `Alt+dragging duplicated element with invalid dimensions`, + duplicatedElement.x, + duplicatedElement.y, + duplicatedElement.width, + duplicatedElement.height, + ); + + return; + } + const origElement = pointerDownState.originalElements.get( element.id, )!; + + // NOTE (mtolmacs): This is a temporary fix for very large scenes + if ( + Math.abs(origElement.x) > 1e7 || + Math.abs(origElement.x) > 1e7 || + Math.abs(origElement.width) > 1e7 || + Math.abs(origElement.height) > 1e7 + ) { + console.error( + `Alt+dragging duplicated element with invalid dimensions`, + origElement.x, + origElement.y, + origElement.width, + origElement.height, + ); + + return; + } + mutateElement(duplicatedElement, { x: origElement.x, y: origElement.y, diff --git a/packages/excalidraw/data/restore.ts b/packages/excalidraw/data/restore.ts index 257a2208ac..c4e45b025a 100644 --- a/packages/excalidraw/data/restore.ts +++ b/packages/excalidraw/data/restore.ts @@ -8,6 +8,7 @@ import type { ExcalidrawTextElement, FixedPointBinding, FontFamilyValues, + NonDeletedSceneElementsMap, OrderedExcalidrawElement, PointBinding, StrokeRoundness, @@ -60,6 +61,10 @@ import { import type { LocalPoint, Radians } from "@excalidraw/math"; import { isFiniteNumber, pointFrom } from "@excalidraw/math"; import { detectLineHeight } from "../element/textMeasurements"; +import { + updateElbowArrowPoints, + validateElbowPoints, +} from "../element/elbowArrow"; type RestoredAppState = Omit< AppState, @@ -206,24 +211,6 @@ const restoreElementWithProperties = < "customData" in extra ? extra.customData : element.customData; } - // NOTE (mtolmacs): This is a temporary check to detect extremely large - // element position or sizing - if ( - element.x < -1e6 || - element.x > 1e6 || - element.y < -1e6 || - element.y > 1e6 || - element.width < -1e6 || - element.width > 1e6 || - element.height < -1e6 || - element.height > 1e6 - ) { - console.error( - "Restore element with properties size or position is too large", - { element }, - ); - } - return { // spread the original element properties to not lose unknown ones // for forward-compatibility @@ -240,21 +227,6 @@ const restoreElement = ( ): typeof element | null => { element = { ...element }; - // NOTE (mtolmacs): This is a temporary check to detect extremely large - // element position or sizing - if ( - element.x < -1e6 || - element.x > 1e6 || - element.y < -1e6 || - element.y > 1e6 || - element.width < -1e6 || - element.width > 1e6 || - element.height < -1e6 || - element.height > 1e6 - ) { - console.error("Restore element size or position is too large", { element }); - } - switch (element.type) { case "text": // temp fix: cleanup legacy obsidian-excalidraw attribute else it'll @@ -596,7 +568,73 @@ export const restoreElements = ( } } - return restoredElements; + // NOTE (mtolmacs): Temporary fix for extremely large arrows + // Need to iterate again so we have attached text nodes in elementsMap + return restoredElements.map((element) => { + if ( + isElbowArrow(element) && + element.startBinding == null && + element.endBinding == null && + !validateElbowPoints(element.points) + ) { + return { + ...element, + ...updateElbowArrowPoints( + element, + restoredElementsMap as NonDeletedSceneElementsMap, + { + points: [ + pointFrom(0, 0), + element.points[element.points.length - 1], + ], + }, + ), + index: element.index, + }; + } + + if ( + isElbowArrow(element) && + element.startBinding && + element.endBinding && + element.startBinding.elementId === element.endBinding.elementId && + element.points.length > 1 && + element.points.some( + ([rx, ry]) => Math.abs(rx) > 1e6 || Math.abs(ry) > 1e6, + ) + ) { + console.error("Fixing self-bound elbow arrow", element.id); + const boundElement = restoredElementsMap.get( + element.startBinding.elementId, + ); + if (!boundElement) { + console.error( + "Bound element not found", + element.startBinding.elementId, + ); + return element; + } + + return { + ...element, + x: boundElement.x + boundElement.width / 2, + y: boundElement.y - 5, + width: boundElement.width, + height: boundElement.height, + points: [ + pointFrom(0, 0), + pointFrom(0, -10), + pointFrom(boundElement.width / 2 + 5, -10), + pointFrom( + boundElement.width / 2 + 5, + boundElement.height / 2 + 5, + ), + ], + }; + } + + return element; + }); }; const coalesceAppStateValue = < diff --git a/packages/excalidraw/element/binding.ts b/packages/excalidraw/element/binding.ts index b679ae9d01..8de9254fac 100644 --- a/packages/excalidraw/element/binding.ts +++ b/packages/excalidraw/element/binding.ts @@ -943,7 +943,10 @@ export const bindPointToSnapToElementOutline = ( ), )[0]; const currentDistance = pointDistance(p, center); - const fullDistance = pointDistance(intersection, center); + const fullDistance = Math.max( + pointDistance(intersection ?? p, center), + 1e-5, + ); const ratio = currentDistance / fullDistance; switch (true) { @@ -954,10 +957,10 @@ export const bindPointToSnapToElementOutline = ( return pointFromVector( vectorScale( - vectorNormalize(vectorFromPoint(p, intersection)), + vectorNormalize(vectorFromPoint(p, intersection ?? center)), ratio > 1 ? FIXED_BINDING_DISTANCE : -FIXED_BINDING_DISTANCE, ), - intersection, + intersection ?? center, ); default: diff --git a/packages/excalidraw/element/bounds.ts b/packages/excalidraw/element/bounds.ts index e4be1dfbd3..06f9770239 100644 --- a/packages/excalidraw/element/bounds.ts +++ b/packages/excalidraw/element/bounds.ts @@ -40,6 +40,7 @@ import { pointRotateRads, } from "@excalidraw/math"; import type { Mutable } from "../utility-types"; +import { getCurvePathOps } from "@excalidraw/utils/geometry/shape"; export type RectangleBox = { x: number; @@ -367,15 +368,6 @@ export const getDiamondPoints = (element: ExcalidrawElement) => { return [topX, topY, rightX, rightY, bottomX, bottomY, leftX, leftY]; }; -export const getCurvePathOps = (shape: Drawable): Op[] => { - for (const set of shape.sets) { - if (set.type === "path") { - return set.ops; - } - } - return shape.sets[0].ops; -}; - // reference: https://eliot-jones.com/2019/12/cubic-bezier-curve-bounding-boxes const getBezierValueForT = ( t: number, @@ -583,6 +575,10 @@ export const getArrowheadPoints = ( position: "start" | "end", arrowhead: Arrowhead, ) => { + if (shape.length < 1) { + return null; + } + const ops = getCurvePathOps(shape[0]); if (ops.length < 1) { return null; diff --git a/packages/excalidraw/element/elbowArrow.ts b/packages/excalidraw/element/elbowArrow.ts index 703cea0d58..a965d2f93c 100644 --- a/packages/excalidraw/element/elbowArrow.ts +++ b/packages/excalidraw/element/elbowArrow.ts @@ -1038,7 +1038,13 @@ export const updateElbowArrowPoints = ( // Short circuit on no-op to avoid huge performance hit if ( updates.startBinding === arrow.startBinding && - updates.endBinding === arrow.endBinding + updates.endBinding === arrow.endBinding && + (updates.points ?? []).every((p, i) => + pointsEqual( + p, + arrow.points[i] ?? pointFrom(Infinity, Infinity), + ), + ) ) { return {}; } @@ -2034,7 +2040,6 @@ const normalizeArrowElementUpdate = ( } => { const offsetX = global[0][0]; const offsetY = global[0][1]; - let points = global.map((p) => pointTranslate( p, @@ -2240,7 +2245,7 @@ const getHoveredElements = ( const gridAddressesEqual = (a: GridAddress, b: GridAddress): boolean => a[0] === b[0] && a[1] === b[1]; -const validateElbowPoints =

( +export const validateElbowPoints =

( points: readonly P[], tolerance: number = DEDUP_TRESHOLD, ) => diff --git a/packages/excalidraw/element/linearElementEditor.ts b/packages/excalidraw/element/linearElementEditor.ts index 4bf1e988bb..b616268a67 100644 --- a/packages/excalidraw/element/linearElementEditor.ts +++ b/packages/excalidraw/element/linearElementEditor.ts @@ -14,11 +14,7 @@ import type { } from "./types"; import { getElementAbsoluteCoords, getLockedLinearCursorAlignSize } from "."; import type { Bounds } from "./bounds"; -import { - getCurvePathOps, - getElementPointsCoords, - getMinMaxXYFromCurvePathOps, -} from "./bounds"; +import { getElementPointsCoords, getMinMaxXYFromCurvePathOps } from "./bounds"; import type { AppState, PointerCoords, @@ -53,11 +49,9 @@ import { pointFrom, pointRotateRads, pointsEqual, - vector, type GlobalPoint, type LocalPoint, pointDistance, - pointTranslate, vectorFromPoint, } from "@excalidraw/math"; import { @@ -69,6 +63,7 @@ import { } from "../shapes"; import { getGridPoint } from "../snapping"; import { headingIsHorizontal, vectorToHeading } from "./heading"; +import { getCurvePathOps } from "@excalidraw/utils/geometry/shape"; const editorMidPointsCache: { version: number | null; @@ -1273,34 +1268,28 @@ export class LinearElementEditor { // 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. - let offsetX = 0; - let offsetY = 0; + const [deltaX, deltaY] = + targetPoints.find(({ index }) => index === 0)?.point ?? + pointFrom(0, 0); + const [offsetX, offsetY] = pointFrom( + deltaX - points[0][0], + deltaY - points[0][1], + ); - const selectedOriginPoint = targetPoints.find(({ index }) => index === 0); + const nextPoints = isElbowArrow(element) + ? [ + targetPoints.find((t) => t.index === 0)?.point ?? points[0], + targetPoints.find((t) => t.index === points.length - 1)?.point ?? + points[points.length - 1], + ] + : points.map((p, idx) => { + const current = targetPoints.find((t) => t.index === idx)?.point ?? p; - if (selectedOriginPoint) { - offsetX = - selectedOriginPoint.point[0] + points[selectedOriginPoint.index][0]; - offsetY = - selectedOriginPoint.point[1] + points[selectedOriginPoint.index][1]; - } - - const nextPoints: LocalPoint[] = points.map((p, idx) => { - const selectedPointData = targetPoints.find((t) => t.index === idx); - if (selectedPointData) { - if (selectedPointData.index === 0) { - return p; - } - - const deltaX = - selectedPointData.point[0] - points[selectedPointData.index][0]; - const deltaY = - selectedPointData.point[1] - points[selectedPointData.index][1]; - - return pointFrom(p[0] + deltaX - offsetX, p[1] + deltaY - offsetY); - } - return offsetX || offsetY ? pointFrom(p[0] - offsetX, p[1] - offsetY) : p; - }); + return pointFrom( + current[0] - offsetX, + current[1] - offsetY, + ); + }); LinearElementEditor._updatePoints( element, @@ -1451,14 +1440,6 @@ export class LinearElementEditor { } updates.points = Array.from(nextPoints); - updates.points[0] = pointTranslate( - updates.points[0], - vector(offsetX, offsetY), - ); - updates.points[updates.points.length - 1] = pointTranslate( - updates.points[updates.points.length - 1], - vector(offsetX, offsetY), - ); mutateElement(element, updates, true, { isDragging: options?.isDragging, diff --git a/packages/excalidraw/element/resizeElements.ts b/packages/excalidraw/element/resizeElements.ts index f71a0bc7a9..b86ba02791 100644 --- a/packages/excalidraw/element/resizeElements.ts +++ b/packages/excalidraw/element/resizeElements.ts @@ -769,32 +769,12 @@ const getResizedOrigin = ( y: y - (newHeight - prevHeight) / 2, }; case "east-side": - // NOTE (mtolmacs): Reverting this for a short period to test if it is - // the cause of the megasized elbow arrows showing up. - if ( - Math.abs( - y + - ((prevWidth - newWidth) / 2) * Math.sin(angle) + - (prevHeight - newHeight) / 2, - ) > 1e6 - ) { - console.error( - "getResizedOrigin() new calculation creates extremely large (> 1e6) y value where the old calculation resulted in", - { - result: - y + - (newHeight - prevHeight) / 2 + - ((prevWidth - newWidth) / 2) * Math.sin(angle), - }, - ); - } - return { x: x + ((prevWidth - newWidth) / 2) * (Math.cos(angle) + 1), y: y + - (newHeight - prevHeight) / 2 + - ((prevWidth - newWidth) / 2) * Math.sin(angle), + ((prevWidth - newWidth) / 2) * Math.sin(angle) + + (prevHeight - newHeight) / 2, }; case "west-side": return { diff --git a/packages/excalidraw/renderer/staticScene.ts b/packages/excalidraw/renderer/staticScene.ts index 21fca4590d..90ed8af430 100644 --- a/packages/excalidraw/renderer/staticScene.ts +++ b/packages/excalidraw/renderer/staticScene.ts @@ -351,7 +351,14 @@ const _renderStaticScene = ({ renderLinkIcon(element, context, appState, elementsMap); } } catch (error: any) { - console.error(error); + console.error( + error, + element.id, + element.x, + element.y, + element.width, + element.height, + ); } }); diff --git a/packages/excalidraw/scene/Shape.ts b/packages/excalidraw/scene/Shape.ts index 0fef0b2b2b..9c49db0c21 100644 --- a/packages/excalidraw/scene/Shape.ts +++ b/packages/excalidraw/scene/Shape.ts @@ -430,12 +430,26 @@ export const _generateElementShape = ( : [pointFrom(0, 0)]; if (isElbowArrow(element)) { - shape = [ - generator.path( - generateElbowArrowShape(points, 16), - generateRoughOptions(element, true), - ), - ]; + // NOTE (mtolmacs): Temporary fix for extremely big arrow shapes + if ( + !points.every( + (point) => Math.abs(point[0]) <= 1e6 && Math.abs(point[1]) <= 1e6, + ) + ) { + console.error( + `Elbow arrow with extreme point positions detected. Arrow not rendered.`, + element.id, + JSON.stringify(points), + ); + shape = []; + } else { + shape = [ + generator.path( + generateElbowArrowShape(points, 16), + generateRoughOptions(element, true), + ), + ]; + } } else if (!element.roundness) { // curve is always the first element // this simplifies finding the curve for an element diff --git a/packages/utils/geometry/shape.ts b/packages/utils/geometry/shape.ts index 2ba04a2b2f..10fc06e310 100644 --- a/packages/utils/geometry/shape.ts +++ b/packages/utils/geometry/shape.ts @@ -192,6 +192,11 @@ export const getEllipseShape = ( }; export const getCurvePathOps = (shape: Drawable): Op[] => { + // NOTE (mtolmacs): Temporary fix for extremely large elements + if (!shape) { + return []; + } + for (const set of shape.sets) { if (set.type === "path") { return set.ops;