From 29c03fe1d38eb5a1fc63308b07ca0f7bde7fe38e Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Fri, 26 Jun 2026 23:05:35 +0200 Subject: [PATCH 1/2] fix(text): avoid spurious measuring in _measureChar when only kerning-irrelevant entries are missing --- CHANGELOG.md | 1 + src/shapes/Text/Text.spec.ts | 63 +++++++++++++++++++++++++++++++++++- src/shapes/Text/Text.ts | 29 ++++++++--------- 3 files changed, 77 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d666469b30..02ac52185a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [next] +- fix(text): avoid spurious measuring in `_measureChar` when only kerning-irrelevant entries are missing [#11028](https://github.com/fabricjs/fabric.js/pull/11028) - 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..8326d1ffe14 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, @@ -1345,5 +1345,66 @@ describe('FabricText', () => { expect(cacheKeys?.next().value).not.toBe('undefineda'); expect(cacheKeys?.next().done).toBe(true); }); + + it('does not re-enter the measuring path when only kerning-irrelevant entries are missing', () => { + cache.clearFontCache(); + const text = new FabricText(''); + const style = text.getCompleteStyleDeclaration(0, 0); + // a different style so the previous char cannot form a kerning pair + const otherStyle = { ...style, fontFamily: 'OtherFont' }; + + // prime the cache for 'a' without a usable previous char + text._measureChar('a', style, undefined, style); + expect( + cache.charWidthsCache + .get(style.fontFamily.toLowerCase()) + ?.get(`${style.fontStyle}_${style.fontWeight}`) + ?.has('a'), + 'single char width is cached', + ).toBe(true); + + const setTextStylesSpy = vi.spyOn(text, '_setTextStyles'); + // second call: width is cached and no same-style kerning pair applies + const second = text._measureChar('a', style, 'b', otherStyle); + expect(second.width, 'returns the cached width').toBe( + text._measureChar('a', style, undefined, style).width, + ); + expect( + setTextStylesSpy, + 'does not touch the measuring context when nothing needs measuring', + ).not.toHaveBeenCalled(); + setTextStylesSpy.mockRestore(); + }); + + it('measures coupleWidth and previousWidth via measureText when both are undefined and same-style kerning pair applies', () => { + cache.clearFontCache(); + const text = new FabricText(''); + const style = text.getCompleteStyleDeclaration(0, 0); + + // prime only the single-character width for 'a'; 'b' and 'ba' stay uncached + text._measureChar('a', style, undefined, style); + + const setTextStylesSpy = vi.spyOn(text, '_setTextStyles'); + // 'a' width is cached, but previousWidth ('b') and coupleWidth ('ba') are not. + const result = text._measureChar('a', style, 'b', style); + + const fontCache = cache.getFontCache(style); + expect(fontCache.get('b'), 'previousChar is now cached').toBeTypeOf( + 'number', + ); + expect(fontCache.get('ba'), 'couple is now cached').toBeTypeOf('number'); + expect( + result.kernedWidth, + 'kernedWidth is coupleWidth - previousWidth', + ).toBe( + (fontCache.get('ba') - fontCache.get('b')) * + (style.fontSize / text.CACHE_FONT_SIZE), + ); + expect( + setTextStylesSpy, + 'does not touch the measuring context when nothing needs measuring', + ).toHaveBeenCalledOnce(); + setTextStylesSpy.mockRestore(); + }); }); }); diff --git a/src/shapes/Text/Text.ts b/src/shapes/Text/Text.ts index e2a858d9166..2d53c09a380 100644 --- a/src/shapes/Text/Text.ts +++ b/src/shapes/Text/Text.ts @@ -824,18 +824,19 @@ export class FabricText< ) { const fontCache = cache.getFontCache(charStyle), fontDeclaration = this._getFontDeclaration(charStyle), - couple = previousChar ? previousChar + _char : _char, + fontMultiplier = charStyle.fontSize / this.CACHE_FONT_SIZE; + const hasPrevChar = Boolean(previousChar), stylesAreEqual = - previousChar && + hasPrevChar && fontDeclaration === this._getFontDeclaration(prevCharStyle), - fontMultiplier = charStyle.fontSize / this.CACHE_FONT_SIZE; + couple = hasPrevChar ? previousChar + _char : _char; let width: number | undefined, coupleWidth: number | undefined, previousWidth: number | undefined, kernedWidth: number | undefined; - if (previousChar && fontCache.has(previousChar)) { - previousWidth = fontCache.get(previousChar); + if (hasPrevChar && fontCache.has(previousChar!)) { + previousWidth = fontCache.get(previousChar!); } if (fontCache.has(_char)) { kernedWidth = width = fontCache.get(_char); @@ -844,11 +845,9 @@ export class FabricText< coupleWidth = fontCache.get(couple)!; kernedWidth = coupleWidth - previousWidth!; } - if ( - width === undefined || - previousWidth === undefined || - coupleWidth === undefined - ) { + // only enter the measuring path when the single-character width is missing + // or a same-style kerning pair actually needs measuring + if (width === undefined || (stylesAreEqual && coupleWidth === undefined)) { const ctx = getMeasuringContext()!; // send a TRUE to specify measuring font size CACHE_FONT_SIZE this._setTextStyles(ctx, charStyle, true); @@ -856,16 +855,16 @@ export class FabricText< kernedWidth = width = ctx.measureText(_char).width; fontCache.set(_char, width); } - if (previousWidth === undefined && stylesAreEqual && previousChar) { - previousWidth = ctx.measureText(previousChar).width; - fontCache.set(previousChar, previousWidth); - } if (stylesAreEqual && coupleWidth === undefined) { + if (previousWidth === undefined) { + previousWidth = ctx.measureText(previousChar!).width; + fontCache.set(previousChar!, previousWidth); + } // we can measure the kerning couple and subtract the width of the previous character coupleWidth = ctx.measureText(couple).width; fontCache.set(couple, coupleWidth); // safe to use the non-null since if undefined we defined it before. - kernedWidth = coupleWidth - previousWidth!; + kernedWidth = coupleWidth - previousWidth; } } return { From 11e53308257b7fdfaa8de0839326369bf4d2dd2c Mon Sep 17 00:00:00 2001 From: Jiayi Hu Date: Mon, 29 Jun 2026 11:01:30 +0200 Subject: [PATCH 2/2] Improve boolean checks --- src/shapes/Text/Text.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/shapes/Text/Text.ts b/src/shapes/Text/Text.ts index 2d53c09a380..fda447499f3 100644 --- a/src/shapes/Text/Text.ts +++ b/src/shapes/Text/Text.ts @@ -825,7 +825,7 @@ export class FabricText< const fontCache = cache.getFontCache(charStyle), fontDeclaration = this._getFontDeclaration(charStyle), fontMultiplier = charStyle.fontSize / this.CACHE_FONT_SIZE; - const hasPrevChar = Boolean(previousChar), + const hasPrevChar = !!previousChar, stylesAreEqual = hasPrevChar && fontDeclaration === this._getFontDeclaration(prevCharStyle), @@ -835,15 +835,19 @@ export class FabricText< previousWidth: number | undefined, kernedWidth: number | undefined; - if (hasPrevChar && fontCache.has(previousChar!)) { - previousWidth = fontCache.get(previousChar!); + if (hasPrevChar && fontCache.has(previousChar)) { + previousWidth = fontCache.get(previousChar); } if (fontCache.has(_char)) { kernedWidth = width = fontCache.get(_char); } - if (stylesAreEqual && fontCache.has(couple)) { + if ( + stylesAreEqual && + fontCache.has(couple) && + previousWidth !== undefined + ) { coupleWidth = fontCache.get(couple)!; - kernedWidth = coupleWidth - previousWidth!; + kernedWidth = coupleWidth - previousWidth; } // only enter the measuring path when the single-character width is missing // or a same-style kerning pair actually needs measuring @@ -857,8 +861,8 @@ export class FabricText< } if (stylesAreEqual && coupleWidth === undefined) { if (previousWidth === undefined) { - previousWidth = ctx.measureText(previousChar!).width; - fontCache.set(previousChar!, previousWidth); + previousWidth = ctx.measureText(previousChar).width; + fontCache.set(previousChar, previousWidth); } // we can measure the kerning couple and subtract the width of the previous character coupleWidth = ctx.measureText(couple).width;