diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d666469b30..adc637744fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- fix(text): skip `initDimensions` in `set` when a `textLayoutProperty` value did not change [#11029](https://github.com/fabricjs/fabric.js/pull/11029) - chore(deps-dev): bump rolldown from 1.1.0 to 1.1.2 [#11025](https://github.com/fabricjs/fabric.js/pull/11025) - fix(sandbox): restore vanilla startup with pnpm [#11012](https://github.com/fabricjs/fabric.js/pull/11012) - fix(): nested duplicated clipPath causes infinite recursion [#10774](https://github.com/fabricjs/fabric.js/pull/10774) diff --git a/src/shapes/Text/Text.spec.ts b/src/shapes/Text/Text.spec.ts index 50250519d6e..87f800d53a2 100644 --- a/src/shapes/Text/Text.spec.ts +++ b/src/shapes/Text/Text.spec.ts @@ -4,7 +4,7 @@ import { config } from '../../config'; import { Path } from '../Path'; import { FabricText } from './Text'; -import { describe, expect, it, afterEach } from 'vitest'; +import { describe, expect, it, afterEach, vi } from 'vitest'; import { FabricObject, getFabricDocument, @@ -1346,4 +1346,60 @@ describe('FabricText', () => { expect(cacheKeys?.next().done).toBe(true); }); }); + + describe('set & initDimensions', () => { + it('does not call initDimensions when a textLayoutProperty is set to the same value', () => { + const text = new FabricText('hello'); + const initSpy = vi.spyOn(text, 'initDimensions'); + const setCoordsSpy = vi.spyOn(text, 'setCoords'); + + // set to the same value it already has + text.set('text', 'hello'); + expect( + initSpy, + 'initDimensions is not called for a no-op set', + ).not.toHaveBeenCalled(); + expect(setCoordsSpy).not.toHaveBeenCalled(); + + // changing the value does trigger it + text.set('text', 'world'); + expect( + initSpy, + 'initDimensions is called when value changes', + ).toHaveBeenCalledTimes(1); + expect( + setCoordsSpy, + 'setCoords is called when value changes', + ).toHaveBeenCalledTimes(1); + + initSpy.mockRestore(); + setCoordsSpy.mockRestore(); + }); + + it('does not call initDimensions when textLayoutProperties are set to the same values via object', () => { + const text = new FabricText('hello', { fontSize: 40, charSpacing: 0 }); + const initSpy = vi.spyOn(text, 'initDimensions'); + const setCoordsSpy = vi.spyOn(text, 'setCoords'); + + text.set({ text: 'hello', fontSize: 40, charSpacing: 0 }); + expect( + initSpy, + 'no initDimensions for no-op object set', + ).not.toHaveBeenCalled(); + expect(setCoordsSpy).not.toHaveBeenCalled(); + + text.set({ text: 'hello', fontSize: 50, charSpacing: 0 }); + expect( + initSpy, + 'initDimensions runs when at least one value changes', + ).toHaveBeenCalledTimes(1); + expect( + setCoordsSpy, + 'setCoords runs when at least one value changes', + ).toHaveBeenCalledTimes(1); + + initSpy.mockRestore(); + setCoordsSpy.mockRestore(); + }); + }); }); diff --git a/src/shapes/Text/Text.ts b/src/shapes/Text/Text.ts index e2a858d9166..b5ad494e79b 100644 --- a/src/shapes/Text/Text.ts +++ b/src/shapes/Text/Text.ts @@ -1806,21 +1806,27 @@ export class FabricText< set(key: string | any, value?: any) { const { textLayoutProperties } = this.constructor as typeof FabricText; - super.set(key, value); + // detect layout-relevant changes before super.set mutates the instance, + // so initDimensions is only requested when a value actually changed. let needsDims = false; let isAddingPath = false; if (typeof key === 'object') { for (const _key in key) { - if (_key === 'path') { - this.setPathInfo(); - } - needsDims = needsDims || textLayoutProperties.includes(_key); isAddingPath = isAddingPath || _key === 'path'; + needsDims = + needsDims || + (textLayoutProperties.includes(_key) && + this[_key as keyof this] !== key[_key]); } } else { - needsDims = textLayoutProperties.includes(key); isAddingPath = key === 'path'; + needsDims = + textLayoutProperties.includes(key) && this[key as keyof this] !== value; } + + super.set(key, value); + + // setPathInfo must run after super.set so it reads the new path if (isAddingPath) { this.setPathInfo(); }