From 7e900e65dae41789caf7ef3655b67bcb5cffef8a Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Sun, 19 May 2024 17:08:41 +0200 Subject: [PATCH 1/5] perf(calcTransformMatrix): replace cache key string with array --- src/shapes/Object/ObjectGeometry.ts | 56 ++++++++++++----------------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/src/shapes/Object/ObjectGeometry.ts b/src/shapes/Object/ObjectGeometry.ts index fa7e6c0c320..c3b30169bf0 100644 --- a/src/shapes/Object/ObjectGeometry.ts +++ b/src/shapes/Object/ObjectGeometry.ts @@ -25,9 +25,10 @@ import type { StaticCanvas } from '../../canvas/StaticCanvas'; import { ObjectOrigin } from './ObjectOrigin'; import type { ObjectEvents } from '../../EventTypeDefs'; import type { ControlProps } from './types/ControlProps'; +import { resolveOrigin } from '../../util/misc/resolveOrigin'; type TMatrixCache = { - key: string; + key: number[]; value: TMat2D; }; @@ -437,40 +438,29 @@ export class ObjectGeometry this.aCoords = this.calcACoords(); } - transformMatrixKey(skipGroup = false): string { - const sep = '_'; - let prefix = ''; + transformMatrixKey(skipGroup = false): number[] { + let prefix: number[] = []; if (!skipGroup && this.group) { - prefix = this.group.transformMatrixKey(skipGroup) + sep; + prefix = this.group.transformMatrixKey(skipGroup); } - return ( - prefix + - this.top + - sep + - this.left + - sep + - this.scaleX + - sep + - this.scaleY + - sep + - this.skewX + - sep + - this.skewY + - sep + - this.angle + - sep + - this.originX + - sep + - this.originY + - sep + - this.width + - sep + - this.height + - sep + - this.strokeWidth + - this.flipX + - this.flipY + prefix.push( + this.top, + this.left, + this.scaleX, + this.scaleY, + this.skewX, + this.skewY, + this.angle, + resolveOrigin(this.originX), + resolveOrigin(this.originY), + this.width, + this.height, + +this.flipX, + +this.flipY, + this.strokeWidth ); + + return prefix; } /** @@ -487,7 +477,7 @@ export class ObjectGeometry } const key = this.transformMatrixKey(skipGroup), cache = this.matrixCache; - if (cache && cache.key === key) { + if (cache && cache.key.every((x, i) => x === key[i])) { return cache.value; } if (this.group) { From 001367a18835bad081e8e2808f98fa78da78fc19 Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Sun, 19 May 2024 17:24:30 +0200 Subject: [PATCH 2/5] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0be4ecfc020..e64b3e44c6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- perf(ObjectGeometry): replace cache key string with array [#9887](https://github.com/fabricjs/fabric.js/pull/9887) - fix(util): restore old composeMatrix code for performances improvement [#9851](https://github.com/fabricjs/fabric.js/pull/9851) - fix(Control): corner coords definition order [#9884](https://github.com/fabricjs/fabric.js/pull/9884) - fix(Polyline): safeguard points arg from options [#9855](https://github.com/fabricjs/fabric.js/pull/9855) From 0296c17ae976d6f965db3fb7b16902ff66437274 Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Mon, 20 May 2024 23:42:44 +0200 Subject: [PATCH 3/5] Sort properties by likelihood of being changed --- src/shapes/Object/ObjectGeometry.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shapes/Object/ObjectGeometry.ts b/src/shapes/Object/ObjectGeometry.ts index c3b30169bf0..05f2cbabeed 100644 --- a/src/shapes/Object/ObjectGeometry.ts +++ b/src/shapes/Object/ObjectGeometry.ts @@ -446,18 +446,18 @@ export class ObjectGeometry prefix.push( this.top, this.left, + this.width, + this.height, this.scaleX, this.scaleY, + this.angle, + this.strokeWidth, this.skewX, this.skewY, - this.angle, - resolveOrigin(this.originX), - resolveOrigin(this.originY), - this.width, - this.height, +this.flipX, +this.flipY, - this.strokeWidth + resolveOrigin(this.originX), + resolveOrigin(this.originY) ); return prefix; From fe3bd226269ea95e18bf0e2a795cec0c35211fb4 Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Thu, 23 May 2024 23:40:15 +0200 Subject: [PATCH 4/5] Add benchmark --- src/benchmarks/transformMatrixKey.mjs | 97 +++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 src/benchmarks/transformMatrixKey.mjs diff --git a/src/benchmarks/transformMatrixKey.mjs b/src/benchmarks/transformMatrixKey.mjs new file mode 100644 index 00000000000..a3bae78774c --- /dev/null +++ b/src/benchmarks/transformMatrixKey.mjs @@ -0,0 +1,97 @@ +import { FabricObject, Group } from '../../dist/index.mjs'; + +// Swapping of calcCornerCoords in #9377 + +// OLD CODE FOR REFERENCE AND IMPLEMENTATION TEST + +class OldObject extends FabricObject { + transformMatrixKey(skipGroup = false) { + const sep = '_'; + let prefix = ''; + if (!skipGroup && this.group) { + prefix = this.group.transformMatrixKey(skipGroup) + sep; + } + return ( + prefix + + this.top + + sep + + this.left + + sep + + this.scaleX + + sep + + this.scaleY + + sep + + this.skewX + + sep + + this.skewY + + sep + + this.angle + + sep + + this.originX + + sep + + this.originY + + sep + + this.width + + sep + + this.height + + sep + + this.strokeWidth + + this.flipX + + this.flipY + ); + } +} + +class OldGroup extends Group { + transformMatrixKey(skipGroup = false) { + return OldObject.prototype.transformMatrixKey.call(this, skipGroup); + } +} + +// END OF OLD CODE + +const newComplexObject = new FabricObject({ width: 100, height: 100 }); +const newComplexGroup = new Group([newComplexObject]); +new Group([newComplexGroup]); + +const oldComplexObject = new OldObject({ width: 100, height: 100 }); +const oldComplexGroup = new OldGroup([oldComplexObject]); +new OldGroup([oldComplexGroup]); + +const benchmark = (callback) => { + const start = Date.now(); + callback(); + return Date.now() - start; +}; + +const complexNew = benchmark(() => { + for (let i = 0; i < 1_000_000; i++) { + newComplexObject.transformMatrixKey(); + } +}); + +const complexOld = benchmark(() => { + for (let i = 0; i < 1_000_000; i++) { + oldComplexObject.transformMatrixKey(); + } +}); + +console.log({ complexOld, complexNew }); + +const newSimpleObject = new FabricObject({ width: 100, height: 100 }); + +const oldSimpleObject = new OldObject({ width: 100, height: 100 }); + +const simpleNew = benchmark(() => { + for (let i = 0; i < 1_000_000; i++) { + newSimpleObject.transformMatrixKey(); + } +}); + +const simpleOld = benchmark(() => { + for (let i = 0; i < 1_000_000; i++) { + oldSimpleObject.transformMatrixKey(); + } +}); + +console.log({ simpleOld, simpleNew }); From 59e9517cffd579f0c4e4839d593c35fdf6aa5531 Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Sun, 26 May 2024 23:32:04 +0200 Subject: [PATCH 5/5] Fix test --- test/unit/object_geometry.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/unit/object_geometry.js b/test/unit/object_geometry.js index 77b860e8cab..69ee6ab5dae 100644 --- a/test/unit/object_geometry.js +++ b/test/unit/object_geometry.js @@ -343,11 +343,11 @@ var key3 = cObj.transformMatrixKey(); cObj.width = 5; var key4 = cObj.transformMatrixKey(); - assert.notEqual(key1, key2, 'keys are different'); - assert.equal(key1, key3, 'keys are equal'); - assert.notEqual(key4, key2, 'keys are different'); - assert.notEqual(key4, key1, 'keys are different'); - assert.notEqual(key4, key3, 'keys are different'); + assert.notDeepEqual(key1, key2, 'keys are different'); + assert.deepEqual(key1, key3, 'keys are equal'); + assert.notDeepEqual(key4, key2, 'keys are different'); + assert.notDeepEqual(key4, key1, 'keys are different'); + assert.notDeepEqual(key4, key3, 'keys are different'); }); QUnit.test('transformMatrixKey depends from originX/originY', function(assert) { @@ -358,9 +358,9 @@ var key2 = cObj.transformMatrixKey(); cObj.originY = 'center'; var key3 = cObj.transformMatrixKey(); - assert.notEqual(key1, key2, 'keys are different origins 1'); - assert.notEqual(key1, key3, 'keys are different origins 2'); - assert.notEqual(key2, key3, 'keys are different origins 3'); + assert.notDeepEqual(key1, key2, 'keys are different origins 1'); + assert.notDeepEqual(key1, key3, 'keys are different origins 2'); + assert.notDeepEqual(key2, key3, 'keys are different origins 3'); }); QUnit.test('isOnScreen with object that include canvas', function(assert) {