Skip to content

Commit

Permalink
perf(): Rework constructors to avoid the extra perf cost of current s…
Browse files Browse the repository at this point in the history
…etup (#9891)
  • Loading branch information
asturur authored May 30, 2024
1 parent 01b9a40 commit 1025c81
Show file tree
Hide file tree
Showing 21 changed files with 130 additions and 39 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [next]

- perf(): Rework constructors to avoid the extra perf cost of current setup [#9891](https://github.com/fabricjs/fabric.js/pull/9891)
- perf(): Remove redundant matrix multiplication in multiplayTransformMatrixArray [#9893](https://github.com/fabricjs/fabric.js/pull/9893)
- test(): Convert Animation tests to jest [#9892](https://github.com/fabricjs/fabric.js/pull/9892)
- perf(ObjectGeometry): replace cache key string with array [#9887](https://github.com/fabricjs/fabric.js/pull/9887)
Expand Down
4 changes: 4 additions & 0 deletions fabric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ export {
*/
FabricObject as Object,
} from './src/shapes/Object/FabricObject';
/**
* Exported so we can tweak default values
*/
export { FabricObject as BaseFabricObject } from './src/shapes/Object/Object';

export type {
TFabricObjectProps,
Expand Down
2 changes: 1 addition & 1 deletion src/Shadow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export class Shadow {
constructor(arg0: string | Partial<TClassProperties<Shadow>>) {
const options: Partial<TClassProperties<Shadow>> =
typeof arg0 === 'string' ? Shadow.parseShadow(arg0) : arg0;
Object.assign(this, (this.constructor as typeof Shadow).ownDefaults);
Object.assign(this, Shadow.ownDefaults);
for (const prop in options) {
// @ts-expect-error for loops are so messy in TS
this[prop] = options[prop];
Expand Down
8 changes: 4 additions & 4 deletions src/shapes/ActiveSelection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ export class ActiveSelection extends Group {

constructor(
objects: FabricObject[] = [],
options: Partial<ActiveSelectionOptions> = {}
{ layoutManager, ...options }: Partial<ActiveSelectionOptions> = {}
) {
super(objects, {
...options,
layoutManager:
options.layoutManager ?? new ActiveSelectionLayoutManager(),
layoutManager: layoutManager ?? new ActiveSelectionLayoutManager(),
});
Object.assign(this, ActiveSelection.ownDefaults);
this.setOptions(options);
}

/**
Expand Down
10 changes: 10 additions & 0 deletions src/shapes/Circle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ export class Circle<
};
}

/**
* Constructor
* @param {Object} [options] Options object
*/
constructor(options?: Props) {
super();
Object.assign(this, Circle.ownDefaults);
this.setOptions(options);
}

/**
* @private
* @param {String} key
Expand Down
10 changes: 10 additions & 0 deletions src/shapes/Ellipse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ export class Ellipse<
};
}

/**
* Constructor
* @param {Object} [options] Options object
*/
constructor(options?: Props) {
super();
Object.assign(this, Ellipse.ownDefaults);
this.setOptions(options);
}

/**
* @private
* @param {String} key
Expand Down
5 changes: 3 additions & 2 deletions src/shapes/Group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,9 @@ export class Group
* @param {Object} [options] Options object
*/
constructor(objects: FabricObject[] = [], options: Partial<GroupProps> = {}) {
// @ts-expect-error options error
super(options);
super();
Object.assign(this, Group.ownDefaults);
this.setOptions(options);
this._objects = [...objects]; // Avoid unwanted mutations of Collection to affect the caller

this.__objectSelectionTracker = this.__objectSelectionMonitor.bind(
Expand Down
3 changes: 1 addition & 2 deletions src/shapes/IText/IText.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,13 +217,12 @@ export class IText<
}

/**
* Constructor
* @param {String} text Text string
* @param {Object} [options] Options object
*/
constructor(text: string, options?: Props) {
super(text, options);
super(text, { ...IText.ownDefaults, ...options } as Props);
this.initBehavior();
}

Expand Down
7 changes: 5 additions & 2 deletions src/shapes/Image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,11 @@ export class FabricImage<
*/
constructor(elementId: string, options?: Props);
constructor(element: ImageSource, options?: Props);
constructor(arg0: ImageSource | string, options: Props = {} as Props) {
super({ filters: [], ...options });
constructor(arg0: ImageSource | string, options?: Props) {
super();
this.filters = [];
Object.assign(this, FabricImage.ownDefaults);
this.setOptions(options);
this.cacheKey = `texture${uid()}`;
this.setElement(
typeof arg0 === 'string'
Expand Down
10 changes: 8 additions & 2 deletions src/shapes/Line.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,14 @@ export class Line<
* @param {Object} [options] Options object
* @return {Line} thisArg
*/
constructor([x1, y1, x2, y2] = [0, 0, 0, 0], options: Props = {} as Props) {
super({ ...options, x1, y1, x2, y2 });
constructor([x1, y1, x2, y2] = [0, 0, 0, 0], options: Partial<Props> = {}) {
super();
Object.assign(this, Line.ownDefaults);
this.setOptions(options);
this.x1 = x1;
this.x2 = x2;
this.y1 = y1;
this.y2 = y2;
this._setWidthHeight();
const { left, top } = options;
typeof left === 'number' && this.set(LEFT, left);
Expand Down
25 changes: 24 additions & 1 deletion src/shapes/Object/InteractiveObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,34 @@ export class InteractiveFabricObject<
static getDefaults(): Record<string, any> {
return {
...super.getDefaults(),
controls: createObjectDefaultControls(),
...InteractiveFabricObject.ownDefaults,
};
}

/**
* Constructor
* @param {Object} [options] Options object
*/
constructor(options?: Props) {
super();
Object.assign(
this,
(this.constructor as typeof InteractiveFabricObject).createControls(),
InteractiveFabricObject.ownDefaults
);
this.setOptions(options);
}

/**
* Creates the default control object.
* If you prefer to have on instance of controls shared among all objects
* make this function return an empty object and add controls to the ownDefaults
* @param {Object} [options] Options object
*/
static createControls(): { controls: Record<string, Control> } {
return { controls: createObjectDefaultControls() };
}

/**
* Update width and height of the canvas for cache
* returns true or false if canvas needed resize.
Expand Down
9 changes: 3 additions & 6 deletions src/shapes/Object/Object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ export class FabricObject<
static ownDefaults = fabricObjectDefaultValues;

static getDefaults(): Record<string, any> {
return { ...FabricObject.ownDefaults };
return FabricObject.ownDefaults;
}

/**
Expand Down Expand Up @@ -333,12 +333,9 @@ export class FabricObject<
* Constructor
* @param {Object} [options] Options object
*/
constructor(options: Props = {} as Props) {
constructor(options?: Props) {
super();
Object.assign(
this,
(this.constructor as typeof FabricObject).getDefaults()
);
Object.assign(this, FabricObject.ownDefaults);
this.setOptions(options);
}

Expand Down
5 changes: 4 additions & 1 deletion src/shapes/Path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,12 @@ export class Path<
*/
constructor(
path: TComplexPathData | string,
// todo: evaluate this spread here
{ path: _, left, top, ...options }: Partial<Props> = {}
) {
super(options as Props);
super();
Object.assign(this, Path.ownDefaults);
this.setOptions(options);
this._setPath(path || [], true);
typeof left === 'number' && this.set(LEFT, left);
typeof top === 'number' && this.set(TOP, top);
Expand Down
7 changes: 0 additions & 7 deletions src/shapes/Polygon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@ export class Polygon extends Polyline {

static type = 'Polygon';

static getDefaults(): Record<string, any> {
return {
...super.getDefaults(),
...Polyline.ownDefaults,
};
}

protected isOpen() {
return false;
}
Expand Down
6 changes: 5 additions & 1 deletion src/shapes/Polyline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export class Polyline<
...Polyline.ownDefaults,
};
}

/**
* A list of properties that if changed trigger a recalculation of dimensions
* @todo check if you really need to recalculate for all cases
Expand Down Expand Up @@ -108,7 +109,10 @@ export class Polyline<
* });
*/
constructor(points: XY[] = [], options: Props = {} as Props) {
super({ ...options, points });
super();
Object.assign(this, Polyline.ownDefaults);
this.setOptions(options);
this.points = points;
const { left, top } = options;
this.initialized = true;
this.setBoundingBox(true);
Expand Down
8 changes: 4 additions & 4 deletions src/shapes/Rect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ export class Rect<
/**
* Constructor
* @param {Object} [options] Options object
* @return {Object} thisArg
*/
constructor(options: Props) {
super(options);
constructor(options?: Props) {
super();
Object.assign(this, Rect.ownDefaults);
this.setOptions(options);
this._initRxRy();
}

/**
* Initializes rx/ry attributes
* @private
Expand Down
10 changes: 8 additions & 2 deletions src/shapes/Text/Text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,14 @@ export class FabricText<
return { ...super.getDefaults(), ...FabricText.ownDefaults };
}

constructor(text: string, options: Props = {} as Props) {
super({ ...options, text, styles: options?.styles || {} });
constructor(text: string, options?: Props) {
super();
Object.assign(this, FabricText.ownDefaults);
this.setOptions(options);
if (!this.styles) {
this.styles = {};
}
this.text = text;
this.initialized = true;
if (this.path) {
this.setPathInfo();
Expand Down
20 changes: 19 additions & 1 deletion src/shapes/Textbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { TextStyleDeclaration } from './Text/StyledText';
import type { SerializedITextProps, ITextProps } from './IText/IText';
import type { ITextEvents } from './IText/ITextBehavior';
import type { TextLinesInfo } from './Text/Text';
import type { Control } from '../controls/Control';

// @TODO: Many things here are configuration related and shouldn't be on the class nor prototype
// regexes, list of properties that are not suppose to change by instances, magic consts.
Expand Down Expand Up @@ -97,11 +98,28 @@ export class Textbox<
static getDefaults(): Record<string, any> {
return {
...super.getDefaults(),
controls: createTextboxDefaultControls(),
...Textbox.ownDefaults,
};
}

/**
* Constructor
* @param {String} text Text string
* @param {Object} [options] Options object
*/
constructor(text: string, options?: Props) {
super(text, { ...Textbox.ownDefaults, ...options } as Props);
}

/**
* Creates the default control object.
* If you prefer to have on instance of controls shared among all objects
* make this function return an empty object and add controls to the ownDefaults object
*/
static createControls(): { controls: Record<string, Control> } {
return { controls: createTextboxDefaultControls() };
}

/**
* Unlike superclass's version of this function, Textbox does not update
* its width.
Expand Down
10 changes: 10 additions & 0 deletions src/shapes/Triangle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ export class Triangle<
return { ...super.getDefaults(), ...Triangle.ownDefaults };
}

/**
* Constructor
* @param {Object} [options] Options object
*/
constructor(options?: Props) {
super();
Object.assign(this, Triangle.ownDefaults);
this.setOptions(options);
}

/**
* @private
* @param {CanvasRenderingContext2D} ctx Context to render on
Expand Down
7 changes: 5 additions & 2 deletions test/unit/canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,11 @@

function initActiveSelection(canvas, activeObject, target, multiSelectionStacking) {
fabric.classRegistry.setClass(class TextActiveSelection extends fabric.ActiveSelection {
static getDefaults() {
return {...super.getDefaults(),multiSelectionStacking}
static ownDefaults = {
multiSelectionStacking,
}
constructor(objects, options) {
super(objects, { ...TextActiveSelection.ownDefaults, ...options })
}
});
canvas.setActiveObject(activeObject);
Expand Down
2 changes: 1 addition & 1 deletion test/unit/object_interactivity.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
// set size for bottom left corner and have different results for bl than normal setCornerCoords test
QUnit.test('corner coords: custom control size', function(assert) {
//set custom corner size
const sharedControls = fabric.Object.getDefaults().controls;
const sharedControls = fabric.FabricObject.createControls().controls;
sharedControls.bl.sizeX = 30;
sharedControls.bl.sizeY = 10;

Expand Down

0 comments on commit 1025c81

Please sign in to comment.