Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf(): Rework constructors to avoid the extra perf cost of current setup #9891

Merged
merged 13 commits into from
May 30, 2024
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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't avoid rebuilding the object for Textbox -> IText -> Text because of how text measuring works. All the defaults needs to be assigned before Text base class execute the measurement of the text.

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
Loading