Skip to content

Commit

Permalink
Fix(): path parsing performance regression (#10123)
Browse files Browse the repository at this point in the history
  • Loading branch information
asturur authored Sep 7, 2024
1 parent 1e02da0 commit d495607
Show file tree
Hide file tree
Showing 12 changed files with 769 additions and 223 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## [next]

- Fix(): path parsing performance [#10123](https://github.com/fabricjs/fabric.js/pull/10123)

## [6.4.1]

- fix(): Package.json had wrong path to types for extensions [#10115](https://github.com/fabricjs/fabric.js/pull/10115)
Expand Down
69 changes: 69 additions & 0 deletions src/benchmarks/boundingBoxFromPoint.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/* eslint-disable no-restricted-syntax */
import { util, Point } from '../../dist/index.mjs';

const makeBBoxOld = (points) => {
if (points.length === 0) {
return {
left: 0,
top: 0,
width: 0,
height: 0,
};
}

const { min, max } = points.reduce(
({ min, max }, curr) => {
return {
min: min.min(curr),
max: max.max(curr),
};
},
{ min: new Point(points[0]), max: new Point(points[0]) },
);

const size = max.subtract(min);

return {
left: min.x,
top: min.y,
width: size.x,
height: size.y,
};
};

const size = 10000;
const arraySize = 6000;

const points = new Array(arraySize)
.fill(0)
.map(
() => new Point((Math.random() - 0.5) * 1000, (Math.random() - 0.5) * 1000),
);

const benchmark = (callback) => {
const start = Date.now();
callback();
return Date.now() - start;
};

const fabric = benchmark(() => {
for (let i = 0; i < size; i++) {
util.makeBoundingBoxFromPoints(points);
}
});

const older = benchmark(() => {
for (let i = 0; i < size; i++) {
makeBBoxOld(points);
}
});

console.log({
fabric,
older,
});

/**
* On Node 20 macbook pro m1
* { fabric: 139, older: 1089 }
*/
8 changes: 6 additions & 2 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,13 @@ class BaseConfiguration {

/**
* If disabled boundsOfCurveCache is not used. For apps that make heavy usage of pencil drawing probably disabling it is better
* @default true
* With the standard behaviour of fabric to translate all curves in absolute commands and by not subtracting the starting point from
* the curve is very hard to hit any cache.
* Enable only if you know why it could be useful.
* Candidate for removal/simplification
* @default false
*/
cachesBoundsOfCurve = true;
cachesBoundsOfCurve = false;

/**
* Map of font files
Expand Down
2 changes: 0 additions & 2 deletions src/parser/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ export const reNum = String.raw`(?:[-+]?(?:\d*\.\d+|\d+\.?)(?:[eE][-+]?\d+)?)`;

export const svgNS = 'http://www.w3.org/2000/svg';

export const commaWsp = String.raw`(?:\s+,?\s*|,\s*|$)`;

export const reFontDeclaration = new RegExp(
'(normal|italic)?\\s*(normal|small-caps)?\\s*' +
'(normal|bold|bolder|lighter|100|200|300|400|500|600|700|800|900)?\\s*(' +
Expand Down
2 changes: 1 addition & 1 deletion src/shapes/Path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ export class Path<
case 'L': // lineto, absolute
x = command[1];
y = command[2];
bounds.push(new Point(subpathStartX, subpathStartY), new Point(x, y));
bounds.push({ x: subpathStartX, y: subpathStartY }, { x, y });
break;

case 'M': // moveTo, absolute
Expand Down
39 changes: 15 additions & 24 deletions src/util/misc/boundingBoxFromPoints.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { XY } from '../../Point';
import { Point } from '../../Point';
import type { TBBox } from '../../typedefs';

/**
Expand All @@ -8,31 +7,23 @@ import type { TBBox } from '../../typedefs';
* @return {Object} Object with left, top, width, height properties
*/
export const makeBoundingBoxFromPoints = (points: XY[]): TBBox => {
if (points.length === 0) {
return {
left: 0,
top: 0,
width: 0,
height: 0,
};
}

const { min, max } = points.reduce(
({ min, max }, curr) => {
return {
min: min.min(curr),
max: max.max(curr),
};
},
{ min: new Point(points[0]), max: new Point(points[0]) },
);
let left = 0,
top = 0,
width = 0,
height = 0;

const size = max.subtract(min);
for (let i = 0, len = points.length; i < len; i++) {
const { x, y } = points[i];
if (x > width || !i) width = x;
if (x < left || !i) left = x;
if (y > height || !i) height = y;
if (y < top || !i) top = y;
}

return {
left: min.x,
top: min.y,
width: size.x,
height: size.y,
left,
top,
width: width - left,
height: height - top,
};
};
Loading

0 comments on commit d495607

Please sign in to comment.