Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit ccb2c3e

Browse files
yjbanovyjbanov
committed
fix family fallback
1 parent 6bfb21d commit ccb2c3e

File tree

2 files changed

+90
-38
lines changed

2 files changed

+90
-38
lines changed

‎lib/web_ui/lib/src/engine/canvaskit/text.dart‎

Lines changed: 65 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -62,21 +62,34 @@ class CkParagraphStyle implements ui.ParagraphStyle {
6262
skTextStyle.fontSize = fontSize;
6363
}
6464

65-
if (fontFamily == null ||
66-
!skiaFontCollection.registeredFamilies.contains(fontFamily)) {
67-
fontFamily = 'Roboto';
68-
}
69-
skTextStyle.fontFamilies = [fontFamily];
65+
skTextStyle.fontFamilies = [fontFamilyOrFallback(fontFamily)];
7066

7167
return skTextStyle;
7268
}
7369

70+
static String fontFamilyOrFallback(String? requestedFontFamily) {
71+
if (requestedFontFamily == null ||
72+
!isRegisteredFontFamily(requestedFontFamily)) {
73+
return 'Roboto';
74+
} else {
75+
return requestedFontFamily;
76+
}
77+
}
78+
79+
static bool isRegisteredFontFamily(String fontFamily) {
80+
return skiaFontCollection.registeredFamilies.contains(fontFamily);
81+
}
82+
83+
static bool isNotRegisteredFontFamily(String fontFamily) {
84+
return !isRegisteredFontFamily(fontFamily);
85+
}
86+
7487
static SkStrutStyleProperties toSkStrutStyleProperties(ui.StrutStyle value) {
7588
EngineStrutStyle style = value as EngineStrutStyle;
7689
final SkStrutStyleProperties skStrutStyle = SkStrutStyleProperties();
7790
if (style._fontFamily != null) {
7891
String fontFamily = style._fontFamily!;
79-
if (!skiaFontCollection.registeredFamilies.contains(fontFamily)) {
92+
if (!isRegisteredFontFamily(fontFamily)) {
8093
fontFamily = 'Roboto';
8194
}
8295
final List<String> fontFamilies = <String>[fontFamily];
@@ -167,7 +180,7 @@ class CkParagraphStyle implements ui.ParagraphStyle {
167180

168181
CkTextStyle getTextStyle() {
169182
return CkTextStyle(
170-
fontFamily: _fontFamily,
183+
fontFamily: fontFamilyOrFallback(_fontFamily),
171184
fontSize: _fontSize,
172185
fontWeight: _fontWeight,
173186
fontStyle: _fontStyle,
@@ -279,19 +292,22 @@ class CkTextStyle implements ui.TextStyle {
279292
properties.locale = locale.toLanguageTag();
280293
}
281294

282-
if (fontFamily == null ||
283-
!skiaFontCollection.registeredFamilies.contains(fontFamily)) {
284-
fontFamily = 'Roboto';
285-
}
286-
287-
List<String> fontFamilies = <String>[fontFamily];
288-
if (fontFamilyFallback != null &&
289-
!fontFamilyFallback.every((font) => fontFamily == font)) {
290-
fontFamilies.addAll(fontFamilyFallback);
295+
final bool hasFontFamilyFallback = fontFamilyFallback != null && fontFamilyFallback.isNotEmpty;
296+
if (fontFamily != null && hasFontFamilyFallback) {
297+
properties.fontFamilies = <String>[
298+
fontFamily,
299+
...fontFamilyFallback,
300+
];
301+
} else if (fontFamily != null) {
302+
properties.fontFamilies = <String>[fontFamily];
303+
} else if (hasFontFamilyFallback) {
304+
properties.fontFamilies = fontFamilyFallback;
305+
} else {
306+
// All cases where some font family is specified should have been
307+
// exhausted above.
308+
assert(fontFamily == null && !hasFontFamilyFallback);
291309
}
292310

293-
properties.fontFamilies = fontFamilies;
294-
295311
if (fontWeight != null || fontStyle != null) {
296312
properties.fontStyle = toSkFontStyle(fontWeight, fontStyle);
297313
}
@@ -352,6 +368,24 @@ class CkTextStyle implements ui.TextStyle {
352368
/// The values in this text style are used unless [other] specifically
353369
/// overrides it.
354370
CkTextStyle mergeWith(CkTextStyle other) {
371+
final String? effectiveFontFamily = other.fontFamily ?? fontFamily;
372+
List<String>? effectiveFontFamilyFallback = other.fontFamilyFallback ?? fontFamilyFallback;
373+
374+
if (effectiveFontFamily == null || CkParagraphStyle.isNotRegisteredFontFamily(effectiveFontFamily)) {
375+
// If the effective family is not registered, we must make sure that we have a good fallback.
376+
if (effectiveFontFamilyFallback == null || effectiveFontFamilyFallback.isEmpty) {
377+
effectiveFontFamilyFallback = <String>['Roboto'];
378+
} else {
379+
final bool allFallbacksUnregistered = effectiveFontFamilyFallback.every(CkParagraphStyle.isNotRegisteredFontFamily);
380+
if (allFallbacksUnregistered) {
381+
effectiveFontFamilyFallback = <String>[
382+
...effectiveFontFamilyFallback,
383+
'Roboto',
384+
];
385+
}
386+
}
387+
}
388+
355389
return CkTextStyle(
356390
color: other.color ?? color,
357391
decoration: other.decoration ?? decoration,
@@ -361,8 +395,8 @@ class CkTextStyle implements ui.TextStyle {
361395
fontWeight: other.fontWeight ?? fontWeight,
362396
fontStyle: other.fontStyle ?? fontStyle,
363397
textBaseline: other.textBaseline ?? textBaseline,
364-
fontFamily: other.fontFamily ?? fontFamily,
365-
fontFamilyFallback: other.fontFamilyFallback ?? fontFamilyFallback,
398+
fontFamily: effectiveFontFamily,
399+
fontFamilyFallback: effectiveFontFamilyFallback,
366400
fontSize: other.fontSize ?? fontSize,
367401
letterSpacing: other.letterSpacing ?? letterSpacing,
368402
wordSpacing: other.wordSpacing ?? wordSpacing,
@@ -609,7 +643,9 @@ class CkParagraphBuilder implements ui.ParagraphBuilder {
609643
_paragraphBuilder = canvasKit.ParagraphBuilder.MakeFromFontProvider(
610644
style.skParagraphStyle,
611645
skiaFontCollection.fontProvider,
612-
);
646+
) {
647+
_styleStack.add(_style.getTextStyle());
648+
}
613649

614650
@override
615651
void addPlaceholder(
@@ -687,13 +723,19 @@ class CkParagraphBuilder implements ui.ParagraphBuilder {
687723

688724
@override
689725
void pop() {
726+
if (_styleStack.length <= 1) {
727+
// The top-level text style is paragraph-level. We don't pop it off.
728+
return;
729+
}
690730
_commands.add(const _ParagraphCommand.pop());
691731
_styleStack.removeLast();
692732
_paragraphBuilder.pop();
693733
}
694734

695-
CkTextStyle _peekStyle() =>
696-
_styleStack.isEmpty ? _style.getTextStyle() : _styleStack.last;
735+
CkTextStyle _peekStyle() {
736+
assert(_styleStack.isNotEmpty);
737+
return _styleStack.last;
738+
}
697739

698740
// Used as the paint for background or foreground in the text style when
699741
// the other one is not specified. CanvasKit either both background and

‎lib/web_ui/test/canvaskit/canvas_golden_test.dart‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -348,19 +348,21 @@ void testMain() {
348348
drawText('multiline', layoutWidth: 50);
349349
drawText('max lines', paragraphMaxLines: 1, layoutWidth: 50);
350350
drawText('ellipsis', paragraphMaxLines: 1, paragraphEllipsis: '...', layoutWidth: 60);
351-
// TODO(yjbanov): the text seems to fail to inherit the paragraph font family, rendering "World!" incorrectly.
352-
drawText('paragraph font family (BROKEN)', paragraphFontFamily: 'Ahem');
351+
drawText('paragraph font family', paragraphFontFamily: 'Ahem');
353352
drawText('paragraph font size', paragraphFontSize: 22);
354353
// TODO(yjbanov): paragraphHeight seems to have no effect, but maybe I'm using it wrong.
355-
drawText('paragraph height (BROKEN)', layoutWidth: 50, paragraphHeight: 1.5);
354+
drawText('paragraph height', layoutWidth: 50, paragraphHeight: 1.5);
356355
// TODO(yjbanov): paragraphTextHeightBehavior seems to have no effect. Unsure how to use it.
357-
drawText('paragraph text height behavior (BROKEN)', layoutWidth: 50, paragraphHeight: 1.5, paragraphTextHeightBehavior: ui.TextHeightBehavior(
356+
drawText('paragraph text height behavior', layoutWidth: 50, paragraphHeight: 1.5, paragraphTextHeightBehavior: ui.TextHeightBehavior(
358357
applyHeightToFirstAscent: false,
359358
applyHeightToLastDescent: false,
360359
));
361360
drawText('paragraph weight', paragraphFontWeight: ui.FontWeight.w900);
362-
// TODO(yjbanov): not sure how to test locale
363-
drawText('locale (TODO)', paragraphLocale: ui.Locale('ru', 'RU'));
361+
// TODO(yjbanov): test paragraph locale after CJK font support
362+
drawText('locale', outerText: '次 化 刃 直 入 令', innerText: '', paragraphLocale: const ui.Locale('zh', 'CN'));
363+
drawText('locale', outerText: '次 化 刃 直 入 令', innerText: '', paragraphLocale: const ui.Locale('zh', 'TW'));
364+
drawText('locale', outerText: '次 化 刃 直 入 令', innerText: '', paragraphLocale: const ui.Locale('ja'));
365+
drawText('locale', outerText: '次 化 刃 直 入 令', innerText: '', paragraphLocale: const ui.Locale('ko'));
364366

365367
// Test text style properties
366368
drawText('color', color: const ui.Color(0xFF009900));
@@ -372,19 +374,22 @@ void testMain() {
372374
drawText('baseline', textBaseline: ui.TextBaseline.ideographic);
373375
drawText('font family', textFontFamily: 'Ahem');
374376
drawText('non-existent family', textFontFamily: 'DoesNotExist');
375-
// TODO(yjbanov): the fallback is broken
376-
drawText('family fallback (BROKEN)', textFontFamily: 'DoesNotExist', fontFamilyFallback: <String>['Ahem']);
377+
drawText('family fallback', textFontFamily: 'DoesNotExist', fontFamilyFallback: <String>['Ahem']);
377378
drawText('font size', fontSize: 24);
378-
drawText('letter spacing', letterSpacing: 5);
379-
drawText('word spacing', innerText: 'Beautiful World!', wordSpacing: 25);
380-
drawText('height', height: 2);
381379

382380
canvas.restore();
383381
canvas.save();
384382
canvas.translate(columnWidth + 30, 10);
385383

386-
// TODO(yjbanov): not sure how to test locale
387-
drawText('locale (TODO)', locale: ui.Locale('ru', 'RU'));
384+
drawText('letter spacing', letterSpacing: 5);
385+
drawText('word spacing', innerText: 'Beautiful World!', wordSpacing: 25);
386+
drawText('height', height: 2);
387+
// TODO(yjbanov): test text locale after CJK font support
388+
drawText('locale', outerText: '次 化 刃 直 入 令', innerText: '', locale: const ui.Locale('zh', 'CN'));
389+
drawText('locale', outerText: '次 化 刃 直 入 令', innerText: '', locale: const ui.Locale('zh', 'TW'));
390+
drawText('locale', outerText: '次 化 刃 直 入 令', innerText: '', locale: const ui.Locale('ja'));
391+
drawText('locale', outerText: '次 化 刃 直 入 令', innerText: '', locale: const ui.Locale('ko'));
392+
388393
drawText('background', background: CkPaint()..color = const ui.Color(0xFF00FF00));
389394
drawText('foreground', foreground: CkPaint()..color = const ui.Color(0xFF0000FF));
390395
drawText(
@@ -431,8 +436,13 @@ void testMain() {
431436
fontFeatures: <ui.FontFeature>[const ui.FontFeature.oldstyleFigures()],
432437
);
433438

434-
final CkPicture originalPicture = recorder.endRecording();
435-
await matchPictureGolden('canvaskit_text_styles.png', originalPicture, region: region);
439+
final CkPicture picture = recorder.endRecording();
440+
await matchPictureGolden(
441+
'canvaskit_text_styles.png',
442+
picture,
443+
region: region,
444+
// write: true,
445+
);
436446
});
437447
// TODO: https://github.com/flutter/flutter/issues/60040
438448
// TODO: https://github.com/flutter/flutter/issues/71520

0 commit comments

Comments
 (0)