chore: Logging and fixing extremely large scenes (#9225)

This commit is contained in:
Márk Tolmács 2025-03-05 23:06:01 +01:00 committed by GitHub
parent 70c3e921bb
commit a9e2d2348b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 207 additions and 121 deletions

View file

@ -8437,21 +8437,78 @@ class App extends React.Component<AppProps, AppState> {
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,

View file

@ -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<LocalPoint>(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<LocalPoint>(0, 0),
pointFrom<LocalPoint>(0, -10),
pointFrom<LocalPoint>(boundElement.width / 2 + 5, -10),
pointFrom<LocalPoint>(
boundElement.width / 2 + 5,
boundElement.height / 2 + 5,
),
],
};
}
return element;
});
};
const coalesceAppStateValue = <

View file

@ -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:

View file

@ -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;

View file

@ -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<LocalPoint>(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<GlobalPoint, LocalPoint>(
p,
@ -2240,7 +2245,7 @@ const getHoveredElements = (
const gridAddressesEqual = (a: GridAddress, b: GridAddress): boolean =>
a[0] === b[0] && a[1] === b[1];
const validateElbowPoints = <P extends GlobalPoint | LocalPoint>(
export const validateElbowPoints = <P extends GlobalPoint | LocalPoint>(
points: readonly P[],
tolerance: number = DEDUP_TRESHOLD,
) =>

View file

@ -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<LocalPoint>(0, 0);
const [offsetX, offsetY] = pointFrom<LocalPoint>(
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<LocalPoint>(
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,

View file

@ -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 {

View file

@ -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,
);
}
});

View file

@ -430,12 +430,26 @@ export const _generateElementShape = (
: [pointFrom<LocalPoint>(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

View file

@ -192,6 +192,11 @@ export const getEllipseShape = <Point extends GlobalPoint | LocalPoint>(
};
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;