fix(editor): handle invalid points on restore (#11321)
* fix: handle invalid points on restore * move isValidPoint to @excalidraw/math
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { isFiniteNumber, pointFrom } from "@excalidraw/math";
|
||||
import { isFiniteNumber, isValidPoint, pointFrom } from "@excalidraw/math";
|
||||
|
||||
import {
|
||||
type CombineBrandsIfNeeded,
|
||||
@@ -98,6 +98,67 @@ type RestoredAppState = Omit<
|
||||
|
||||
const MAX_ARROW_PX = 75_000;
|
||||
|
||||
const restoreLinearElementPoints = (
|
||||
points: unknown,
|
||||
width: unknown,
|
||||
height: unknown,
|
||||
): LocalPoint[] => {
|
||||
const restoredPoints = Array.isArray(points)
|
||||
? points.reduce<LocalPoint[]>((acc, point) => {
|
||||
if (isValidPoint(point)) {
|
||||
acc.push(pointFrom<LocalPoint>(point[0], point[1]));
|
||||
}
|
||||
return acc;
|
||||
}, [])
|
||||
: [];
|
||||
|
||||
return restoredPoints.length < 2
|
||||
? [
|
||||
pointFrom<LocalPoint>(0, 0),
|
||||
pointFrom<LocalPoint>(
|
||||
isFiniteNumber(width) ? width : 0,
|
||||
isFiniteNumber(height) ? height : 0,
|
||||
),
|
||||
]
|
||||
: restoredPoints;
|
||||
};
|
||||
|
||||
const restoreFreedrawPoints = (
|
||||
points: unknown,
|
||||
pressures: unknown,
|
||||
): {
|
||||
points: LocalPoint[];
|
||||
pressures: number[];
|
||||
} => {
|
||||
if (!Array.isArray(points)) {
|
||||
return {
|
||||
points: [],
|
||||
pressures: [],
|
||||
};
|
||||
}
|
||||
|
||||
const pressureValues: readonly unknown[] = Array.isArray(pressures)
|
||||
? pressures
|
||||
: [];
|
||||
const restoredPoints: LocalPoint[] = [];
|
||||
const restoredPressures: number[] = [];
|
||||
|
||||
points.forEach((point, index) => {
|
||||
if (isValidPoint(point)) {
|
||||
restoredPoints.push(pointFrom<LocalPoint>(point[0], point[1]));
|
||||
if (index in pressureValues) {
|
||||
const pressure = pressureValues[index];
|
||||
restoredPressures.push(isFiniteNumber(pressure) ? pressure : 0.5);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
return {
|
||||
points: restoredPoints,
|
||||
pressures: restoredPressures,
|
||||
};
|
||||
};
|
||||
|
||||
export const AllowedExcalidrawActiveTools: Record<
|
||||
AppState["activeTool"]["type"],
|
||||
boolean
|
||||
@@ -414,10 +475,15 @@ export const restoreElement = (
|
||||
|
||||
return element;
|
||||
case "freedraw": {
|
||||
const { points, pressures } = restoreFreedrawPoints(
|
||||
element.points,
|
||||
element.pressures,
|
||||
);
|
||||
|
||||
return restoreElementWithProperties(element, {
|
||||
points: element.points,
|
||||
points,
|
||||
simulatePressure: element.simulatePressure,
|
||||
pressures: element.pressures,
|
||||
pressures,
|
||||
});
|
||||
}
|
||||
case "image":
|
||||
@@ -435,14 +501,20 @@ export const restoreElement = (
|
||||
const endArrowhead = normalizeArrowhead(element.endArrowhead);
|
||||
let x = element.x;
|
||||
let y = element.y;
|
||||
let points = // migrate old arrow model to new one
|
||||
!Array.isArray(element.points) || element.points.length < 2
|
||||
? [pointFrom(0, 0), pointFrom(element.width, element.height)]
|
||||
: element.points;
|
||||
let points = restoreLinearElementPoints(
|
||||
element.points,
|
||||
element.width,
|
||||
element.height,
|
||||
);
|
||||
|
||||
if (points[0][0] !== 0 || points[0][1] !== 0) {
|
||||
({ points, x, y } =
|
||||
LinearElementEditor.getNormalizeElementPointsAndCoords(element));
|
||||
LinearElementEditor.getNormalizeElementPointsAndCoords({
|
||||
...element,
|
||||
points,
|
||||
x: x ?? 0,
|
||||
y: y ?? 0,
|
||||
} as ExcalidrawLinearElement));
|
||||
}
|
||||
|
||||
return restoreElementWithProperties(element, {
|
||||
@@ -456,7 +528,7 @@ export const restoreElement = (
|
||||
y,
|
||||
...(isLineElement(element)
|
||||
? {
|
||||
polygon: isValidPolygon(element.points)
|
||||
polygon: isValidPolygon(points)
|
||||
? element.polygon ?? false
|
||||
: false,
|
||||
}
|
||||
@@ -471,22 +543,29 @@ export const restoreElement = (
|
||||
: normalizeArrowhead(element.endArrowhead);
|
||||
const x = element.x as number | undefined;
|
||||
const y = element.y as number | undefined;
|
||||
const points: readonly LocalPoint[] | undefined = // migrate old arrow model to new one
|
||||
!Array.isArray(element.points) || element.points.length < 2
|
||||
? [pointFrom(0, 0), pointFrom(element.width, element.height)]
|
||||
: element.points;
|
||||
const points = restoreLinearElementPoints(
|
||||
element.points,
|
||||
element.width,
|
||||
element.height,
|
||||
);
|
||||
const elementWithRestoredPoints = {
|
||||
...element,
|
||||
points,
|
||||
x: x ?? 0,
|
||||
y: y ?? 0,
|
||||
} as ExcalidrawArrowElement;
|
||||
|
||||
const base = {
|
||||
type: element.type,
|
||||
startBinding: repairBinding(
|
||||
element as ExcalidrawArrowElement,
|
||||
elementWithRestoredPoints,
|
||||
element.startBinding,
|
||||
targetElementsMap,
|
||||
existingElementsMap,
|
||||
"start",
|
||||
),
|
||||
endBinding: repairBinding(
|
||||
element as ExcalidrawArrowElement,
|
||||
elementWithRestoredPoints,
|
||||
element.endBinding,
|
||||
targetElementsMap,
|
||||
existingElementsMap,
|
||||
|
||||
@@ -160,6 +160,39 @@ describe("restoreElements", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("should restore only valid freedraw points and keep pressures aligned", () => {
|
||||
const freedrawElement = API.createElement({
|
||||
type: "freedraw",
|
||||
id: "id-freedraw-invalid-points",
|
||||
points: [pointFrom(0, 0), pointFrom(10, 10)],
|
||||
});
|
||||
|
||||
const restoredFreedraw = restore.restoreElements(
|
||||
[
|
||||
{
|
||||
...freedrawElement,
|
||||
simulatePressure: false,
|
||||
points: [
|
||||
pointFrom(0, 0),
|
||||
[Infinity, 10],
|
||||
null,
|
||||
pointFrom(20, 20),
|
||||
[NaN, 30],
|
||||
[40, null],
|
||||
],
|
||||
pressures: [0.1, 0.2, 0.3, 0.4, 0.5, 0.6],
|
||||
} as any,
|
||||
],
|
||||
null,
|
||||
)[0] as ExcalidrawFreeDrawElement;
|
||||
|
||||
expect(restoredFreedraw.points).toEqual([
|
||||
pointFrom(0, 0),
|
||||
pointFrom(20, 20),
|
||||
]);
|
||||
expect(restoredFreedraw.pressures).toEqual([0.1, 0.4]);
|
||||
});
|
||||
|
||||
it("should restore line and draw elements correctly", () => {
|
||||
const lineElement = API.createElement({ type: "line", id: "id-line01" });
|
||||
|
||||
@@ -400,6 +433,52 @@ describe("restoreElements", () => {
|
||||
expect(restoredLine.points).toMatchObject(expectedLinePoints);
|
||||
});
|
||||
|
||||
it("should restore only valid linear points", () => {
|
||||
const lineElement: any = API.createElement({
|
||||
type: "line",
|
||||
x: 10,
|
||||
y: 20,
|
||||
width: 100,
|
||||
height: 200,
|
||||
});
|
||||
const arrowElement: any = API.createElement({
|
||||
type: "arrow",
|
||||
width: 100,
|
||||
height: 200,
|
||||
});
|
||||
|
||||
lineElement.points = [
|
||||
[2, 3],
|
||||
null,
|
||||
[Infinity, 4],
|
||||
[5, 7],
|
||||
[NaN, 8],
|
||||
[9, null],
|
||||
];
|
||||
arrowElement.points = [
|
||||
[null, 0],
|
||||
[Infinity, 4],
|
||||
];
|
||||
|
||||
const restoredElements = restore.restoreElements(
|
||||
[lineElement, arrowElement],
|
||||
null,
|
||||
);
|
||||
const restoredLine = restoredElements[0] as ExcalidrawLinearElement;
|
||||
const restoredArrow = restoredElements[1] as ExcalidrawArrowElement;
|
||||
|
||||
expect(restoredLine.points).toEqual([pointFrom(0, 0), pointFrom(3, 4)]);
|
||||
expect(restoredLine.x).toBe(12);
|
||||
expect(restoredLine.y).toBe(23);
|
||||
expect(restoredLine.width).toBe(3);
|
||||
expect(restoredLine.height).toBe(4);
|
||||
|
||||
expect(restoredArrow.points).toEqual([
|
||||
pointFrom(0, 0),
|
||||
pointFrom(100, 200),
|
||||
]);
|
||||
});
|
||||
|
||||
it("when the number of points of a line is greater or equal 2", () => {
|
||||
const lineElement_0 = API.createElement({
|
||||
type: "line",
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { degreesToRadians } from "./angle";
|
||||
import { PRECISION } from "./utils";
|
||||
import { isFiniteNumber, PRECISION } from "./utils";
|
||||
import { vectorFromPoint, vectorScale } from "./vector";
|
||||
|
||||
import type {
|
||||
@@ -253,3 +253,12 @@ export const isPointWithinBounds = <P extends GlobalPoint | LocalPoint>(
|
||||
q[1] >= Math.min(p[1], r[1])
|
||||
);
|
||||
};
|
||||
|
||||
export const isValidPoint = (point: unknown): point is LocalPoint => {
|
||||
return (
|
||||
Array.isArray(point) &&
|
||||
point.length === 2 &&
|
||||
isFiniteNumber(point[0]) &&
|
||||
isFiniteNumber(point[1])
|
||||
);
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user