From 10f869e2b21534a8c58bd97e21e193fab2a4e2d1 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Sun, 17 Dec 2023 09:44:12 +0200 Subject: [PATCH 1/3] perf(): reduce setCoords calls --- src/LayoutManager/LayoutManager.ts | 4 ++++ src/brushes/PencilBrush.ts | 5 ++--- src/canvas/Canvas.ts | 17 +++++------------ src/canvas/SelectableCanvas.ts | 9 +++++++-- src/canvas/StaticCanvas.ts | 16 +--------------- src/controls/polyControl.ts | 2 ++ src/controls/wrapWithFixedAnchor.ts | 1 + src/shapes/ActiveSelection.ts | 10 +++------- src/shapes/Group.ts | 22 +--------------------- src/shapes/Object/InteractiveObject.ts | 1 - src/shapes/Text/Text.ts | 2 +- test/unit/canvas_static.js | 21 --------------------- 12 files changed, 27 insertions(+), 83 deletions(-) diff --git a/src/LayoutManager/LayoutManager.ts b/src/LayoutManager/LayoutManager.ts index 958512fad14..5ef5453a308 100644 --- a/src/LayoutManager/LayoutManager.ts +++ b/src/LayoutManager/LayoutManager.ts @@ -251,6 +251,8 @@ export class LayoutManager { target.setPositionByOrigin(nextCenter, CENTER, CENTER); // invalidate target.setCoords(); + (target.subTargetCheck || target.interactive) && + target.forEachObject((object) => object.setCoords()); target.set('dirty', true); } } @@ -286,6 +288,8 @@ export class LayoutManager { left: object.left + offset.x, top: object.top + offset.y, }); + // TODO: should we delete aCoords/oCoords here so they are not stale if subTargetCheck is false? + // or should we apply offset instead of recalculating aCoords? } protected onAfterLayout( diff --git a/src/brushes/PencilBrush.ts b/src/brushes/PencilBrush.ts index a18f8410fe4..d8e2b16fa3c 100644 --- a/src/brushes/PencilBrush.ts +++ b/src/brushes/PencilBrush.ts @@ -288,13 +288,12 @@ export class PencilBrush extends BaseBrush { const path = this.createPath(pathData); this.canvas.clearContext(this.canvas.contextTop); - this.canvas.fire('before:path:created', { path: path }); + this.canvas.fire('before:path:created', { path }); this.canvas.add(path); this.canvas.requestRenderAll(); - path.setCoords(); this._resetShadow(); // fire event 'path' created - this.canvas.fire('path:created', { path: path }); + this.canvas.fire('path:created', { path }); } } diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index ba0a0efbd78..e3ff71e2802 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -1332,7 +1332,6 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { this.setCursor(this.defaultCursor); return; } - let hoverCursor = target.hoverCursor || this.hoverCursor; const activeSelection = isActiveSelection(this._activeObject) ? this._activeObject : null, @@ -1345,17 +1344,11 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { target.findControl(this.getViewportPoint(e)); if (!corner) { - if ((target as Group).subTargetCheck) { - // hoverCursor should come from top-most subTarget, - // so we walk the array backwards - this.targets - .concat() - .reverse() - .map((_target) => { - hoverCursor = _target.hoverCursor || hoverCursor; - }); - } - this.setCursor(hoverCursor); + // hoverCursor should come from top-most subTarget + const hoverCursor = + (target as Group).subTargetCheck && + this.targets.find((target) => target.hoverCursor)?.hoverCursor; + this.setCursor(hoverCursor || target.hoverCursor || this.hoverCursor); } else { const control = corner.control; this.setCursor(control.cursorStyleHandler(e, control, target)); diff --git a/src/canvas/SelectableCanvas.ts b/src/canvas/SelectableCanvas.ts index 019731c1069..4f0a62dfdb4 100644 --- a/src/canvas/SelectableCanvas.ts +++ b/src/canvas/SelectableCanvas.ts @@ -189,6 +189,7 @@ export class SelectableCanvas /** * Keep track of the subTargets for Mouse Events + * Sorted in descending z index order, top most object first * @type FabricObject[] */ targets: FabricObject[] = []; @@ -836,7 +837,10 @@ export class SelectableCanvas while (i--) { const target = objects[i]; if (this._checkTarget(target, pointer)) { - if (isCollection(target) && target.subTargetCheck) { + if ( + isCollection(target) && + (target.subTargetCheck || target.interactive) + ) { const subTarget = this._searchPossibleTargets( target._objects as FabricObject[], pointer @@ -1132,9 +1136,10 @@ export class SelectableCanvas if (isActiveSelection(object) && prevActiveObject !== object) { object.set('canvas', this); - object.setCoords(); } + object.setCoords(); + return true; } diff --git a/src/canvas/StaticCanvas.ts b/src/canvas/StaticCanvas.ts index 7ca095dbbdb..632ac2c3ebc 100644 --- a/src/canvas/StaticCanvas.ts +++ b/src/canvas/StaticCanvas.ts @@ -379,21 +379,7 @@ export class StaticCanvas< * @param {Array} vpt a Canvas 2D API transform matrix */ setViewportTransform(vpt: TMat2D) { - const backgroundObject = this.backgroundImage, - overlayObject = this.overlayImage, - len = this._objects.length; - - this.viewportTransform = vpt; - for (let i = 0; i < len; i++) { - const object = this._objects[i]; - object.group || object.setCoords(); - } - if (backgroundObject) { - backgroundObject.setCoords(); - } - if (overlayObject) { - overlayObject.setCoords(); - } + this.viewportTransform = [...vpt]; this.calcViewportBoundaries(); this.renderOnAddRemove && this.requestRenderAll(); } diff --git a/src/controls/polyControl.ts b/src/controls/polyControl.ts index 40a37baa275..9ee96509804 100644 --- a/src/controls/polyControl.ts +++ b/src/controls/polyControl.ts @@ -91,6 +91,8 @@ export const factoryPolyActionHandler = ( poly.left -= diff.x; poly.top -= diff.y; + actionPerformed && poly.setCoords(); + return actionPerformed; }; }; diff --git a/src/controls/wrapWithFixedAnchor.ts b/src/controls/wrapWithFixedAnchor.ts index 787add10448..085a9083c36 100644 --- a/src/controls/wrapWithFixedAnchor.ts +++ b/src/controls/wrapWithFixedAnchor.ts @@ -21,6 +21,7 @@ export function wrapWithFixedAnchor( transform.originX, transform.originY ); + actionPerformed && target.setCoords(); return actionPerformed; }) as TransformActionHandler; } diff --git a/src/shapes/ActiveSelection.ts b/src/shapes/ActiveSelection.ts index 19ff25a0f65..b1f4405dd33 100644 --- a/src/shapes/ActiveSelection.ts +++ b/src/shapes/ActiveSelection.ts @@ -20,6 +20,8 @@ export interface ActiveSelectionOptions extends GroupProps { const activeSelectionDefaultValues: Partial> = { multiSelectionStacking: 'canvas-stacking', + subTargetCheck: true, + interactive: false, }; /** @@ -68,13 +70,6 @@ export class ActiveSelection extends Group { }); } - /** - * @private - */ - _shouldSetNestedCoords() { - return true; - } - /** * @private * @override we don't want the selection monitor to be active @@ -149,6 +144,7 @@ export class ActiveSelection extends Group { // the object will be in the objects array of both the ActiveSelection and the Group // but referenced in the group's _activeObjects so that it won't be rendered twice. this._enterGroup(object, removeParentTransform); + object.setCoords(); } /** diff --git a/src/shapes/Group.ts b/src/shapes/Group.ts index 0c971a249a3..b7cda6bb9f2 100644 --- a/src/shapes/Group.ts +++ b/src/shapes/Group.ts @@ -91,7 +91,6 @@ export class Group /** * Used to allow targeting of object inside groups. * set to true if you want to select an object inside a group.\ - * **REQUIRES** `subTargetCheck` set to true * This will be not removed but slowly replaced with a method setInteractive * that will take care of enabling subTargetCheck and necessary object events. * There is too much attached to group interactivity to just be evaluated by a @@ -284,13 +283,6 @@ export class Group return this; } - /** - * @private - */ - _shouldSetNestedCoords() { - return this.subTargetCheck; - } - /** * Remove all objects * @returns {FabricObject[]} removed objects @@ -365,7 +357,7 @@ export class Group ) ); } - this._shouldSetNestedCoords() && object.setCoords(); + object._set('group', this); object._set('canvas', this.canvas); this._watchObject(true, object); @@ -412,7 +404,6 @@ export class Group object.calcTransformMatrix() ) ); - object.setCoords(); } this._watchObject(false, object); const index = @@ -489,16 +480,6 @@ export class Group this._drawClipPath(ctx, this.clipPath); } - /** - * @override - * @return {Boolean} - */ - setCoords() { - super.setCoords(); - this._shouldSetNestedCoords() && - this.forEachObject((object) => object.setCoords()); - } - triggerLayout(options: ImperativeLayoutOptions = {}) { this.layoutManager.performLayout({ target: this, @@ -689,7 +670,6 @@ export class Group target: group, targets: group.getObjects(), }); - group.setCoords(); return group; }); } diff --git a/src/shapes/Object/InteractiveObject.ts b/src/shapes/Object/InteractiveObject.ts index 795f1de4a1e..8b05bd69f47 100644 --- a/src/shapes/Object/InteractiveObject.ts +++ b/src/shapes/Object/InteractiveObject.ts @@ -534,7 +534,6 @@ export class InteractiveFabricObject< ctx.strokeStyle = options.cornerStrokeColor; } this._setLineDash(ctx, options.cornerDashArray); - this.setCoords(); this.forEachControl((control, key) => { if (control.getVisibility(this, key)) { const p = this.oCoords[key]; diff --git a/src/shapes/Text/Text.ts b/src/shapes/Text/Text.ts index 4dcd8312d83..6578873e27a 100644 --- a/src/shapes/Text/Text.ts +++ b/src/shapes/Text/Text.ts @@ -427,7 +427,6 @@ export class FabricText< this.setPathInfo(); } this.initDimensions(); - this.setCoords(); } /** @@ -1694,6 +1693,7 @@ export class FabricText< } if (this._forceClearCache) { this.initDimensions(); + this.setCoords(); } super.render(ctx); } diff --git a/test/unit/canvas_static.js b/test/unit/canvas_static.js index 4645f9ae780..1bffd448952 100644 --- a/test/unit/canvas_static.js +++ b/test/unit/canvas_static.js @@ -1714,27 +1714,6 @@ canvas.viewportTransform = fabric.StaticCanvas.getDefaults().viewportTransform; }); - QUnit.test('setViewportTransform calls objects setCoords', function(assert) { - var vpt = [2, 0, 0, 2, 50, 50]; - assert.deepEqual(canvas.viewportTransform, [1, 0, 0, 1, 0, 0], 'initial viewport is identity matrix'); - var rect = new fabric.Rect({ width: 10, heigth: 10 }); - var rectBg = new fabric.Rect({ width: 10, heigth: 10 }); - var rectOverlay = new fabric.Rect({ width: 10, heigth: 10 }); - canvas.add(rect); - canvas.cancelRequestedRender(); - rectBg.canvas = canvas; - canvas.backgroundImage = rectBg; - rectOverlay.canvas = canvas; - canvas.overlayImage = rectOverlay; - assert.deepEqual(new fabric.Point(rect.oCoords.tl), new fabric.Point(0,0), 'rect oCoords are set for normal viewport'); - assert.equal(rectBg.oCoords, undefined, 'rectBg oCoords are not set'); - assert.equal(rectOverlay.oCoords, undefined, 'rectOverlay oCoords are not set'); - canvas.setViewportTransform(vpt); - assert.deepEqual(new fabric.Point(rect.oCoords.tl), new fabric.Point(50,50), 'rect oCoords are set'); - assert.deepEqual(new fabric.Point(rectBg.oCoords.tl), new fabric.Point(50,50), 'rectBg oCoords are set'); - assert.deepEqual(new fabric.Point(rectOverlay.oCoords.tl), new fabric.Point(50,50), 'rectOverlay oCoords are set'); - }); - QUnit.test('getZoom', function(assert) { assert.ok(typeof canvas.getZoom === 'function'); var vpt = [2, 0, 0, 2, 50, 50]; From ec6b63e4864db4fbc889109ae51dfaf90ed995b5 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sun, 17 Dec 2023 09:29:13 +0000 Subject: [PATCH 2/3] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe6bbae6675..6df7d281655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ - refactor(): Separate defaults for base fabric object vs interactive object. Also some moving around of variables [#9474](https://github.com/fabricjs/fabric.js/pull/9474) - refactor(): Change how LayoutManager handles restoring groups [#9522](https://github.com/fabricjs/fabric.js/pull/9522) - fix(BaseConfiguration): set `devicePixelRatio` from window [#9470](https://github.com/fabricjs/fabric.js/pull/9470) +- perf(): reduce setCoords calls [#9550](https://github.com/fabricjs/fabric.js/pull/9550) - fix(): bubble dirty flag for group only when true [#9540](https://github.com/fabricjs/fabric.js/pull/9540) - test() Backport a test to capture a failing text style situation [#9531](https://github.com/fabricjs/fabric.js/pull/9531) From b9ba21b69eee8373b05e32f42f8234afabfaaa05 Mon Sep 17 00:00:00 2001 From: ShaMan123 Date: Mon, 8 Apr 2024 11:51:56 +0300 Subject: [PATCH 3/3] setCoords in `_performTransformAction` --- src/canvas/Canvas.ts | 21 +++++++++------------ src/controls/polyControl.ts | 2 -- src/controls/wrapWithFixedAnchor.ts | 1 - 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/canvas/Canvas.ts b/src/canvas/Canvas.ts index e3ff71e2802..6bc7b7191b5 100644 --- a/src/canvas/Canvas.ts +++ b/src/canvas/Canvas.ts @@ -1304,19 +1304,16 @@ export class Canvas extends SelectableCanvas implements CanvasOptions { transform: Transform, pointer: Point ) { - const x = pointer.x, - y = pointer.y, - action = transform.action, - actionHandler = transform.actionHandler; - let actionPerformed = false; - // this object could be created from the function in the control handlers - - if (actionHandler) { - actionPerformed = actionHandler(e, transform, x, y); - } + const { action, actionHandler, target } = transform; + + const actionPerformed = + !!actionHandler && actionHandler(e, transform, pointer.x, pointer.y); + // This call needs to recurse down + actionPerformed && target.setCoords(); + if (action === 'drag' && actionPerformed) { - transform.target.isMoving = true; - this.setCursor(transform.target.moveCursor || this.moveCursor); + target.isMoving = true; + this.setCursor(target.moveCursor || this.moveCursor); } transform.actionPerformed = transform.actionPerformed || actionPerformed; } diff --git a/src/controls/polyControl.ts b/src/controls/polyControl.ts index 9ee96509804..40a37baa275 100644 --- a/src/controls/polyControl.ts +++ b/src/controls/polyControl.ts @@ -91,8 +91,6 @@ export const factoryPolyActionHandler = ( poly.left -= diff.x; poly.top -= diff.y; - actionPerformed && poly.setCoords(); - return actionPerformed; }; }; diff --git a/src/controls/wrapWithFixedAnchor.ts b/src/controls/wrapWithFixedAnchor.ts index 085a9083c36..787add10448 100644 --- a/src/controls/wrapWithFixedAnchor.ts +++ b/src/controls/wrapWithFixedAnchor.ts @@ -21,7 +21,6 @@ export function wrapWithFixedAnchor( transform.originX, transform.originY ); - actionPerformed && target.setCoords(); return actionPerformed; }) as TransformActionHandler; }