Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]

- 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)
Expand Down
63 changes: 62 additions & 1 deletion src/shapes/Text/Text.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
});
});
});
35 changes: 19 additions & 16 deletions src/shapes/Text/Text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -824,48 +824,51 @@ 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 = !!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)) {
if (hasPrevChar && fontCache.has(previousChar)) {
previousWidth = fontCache.get(previousChar);
}
if (fontCache.has(_char)) {
kernedWidth = width = fontCache.get(_char);
}
if (stylesAreEqual && fontCache.has(couple)) {
coupleWidth = fontCache.get(couple)!;
kernedWidth = coupleWidth - previousWidth!;
}
if (
width === undefined ||
previousWidth === undefined ||
coupleWidth === undefined
stylesAreEqual &&
fontCache.has(couple) &&
previousWidth !== undefined
) {
coupleWidth = fontCache.get(couple)!;
kernedWidth = coupleWidth - previousWidth;
}
// 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);
if (width === undefined) {
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 {
Expand Down
Loading