Implement font fallback#187520
Conversation
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for fallback font families in WebTextStyle and updates WebStrutStyle to apply styles using WebTextStyle. It also adds a test verifying fallback font rendering. Feedback on these changes highlights that unconditionally quoting the font family and bypassing canonicalizeFontFamily can break generic font families, and that fallback fonts themselves should be canonicalized. Additionally, the reviewer noted that fontFamilyFallback was omitted in WebStrutStyle's usage of WebTextStyle, and pointed out leftover commented-out code.
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
mdebbar
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
auto label is removed for flutter/flutter/187520, Failed to enqueue flutter/flutter/187520 with HTTP 400: Pull request New changes require approval from someone other than mdebbar because they were the last pusher.. |
mdebbar
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
auto label is removed for flutter/flutter/187520, Failed to enqueue flutter/flutter/187520 with HTTP 400: Pull request New changes require approval from someone other than mdebbar because they were the last pusher.. |
harryterkelsen
left a comment
harryterkelsen
left a comment
There was a problem hiding this comment.
LGTM with a small question
|
|
||
| if (needsFallbacks) { | ||
| // The fallback font families provided by the caller. | ||
| if (fontFamilyFallback != null && fontFamilyFallback.isNotEmpty) { |
There was a problem hiding this comment.
Should we skip the caller-provided fallbacks if the main font family is a generic or Apple font?
There was a problem hiding this comment.
That's what we do already, unless I'm misunderstanding your question. This whole block of code is guarded by needsFallback which checks for exactly what you said.
mdebbar
added
CICD
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/187520, because - The status or check suite Linux linux_fuchsia_tests has failed. Please fix the issues identified (or deflake) before re-applying this label. |
mdebbar
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
flutter/flutter@5827d5f...3a0420c 2026-06-16 [email protected] Implement font fallback (flutter/flutter#187520) 2026-06-16 [email protected] Add FlatBuffers Verifier checks to Impeller asset loading (flutter/flutter#187878) 2026-06-16 [email protected] Roll Packages from aa964a3 to 8286d39 (1 revision) (flutter/flutter#188067) 2026-06-16 [email protected] Roll Skia from 9c2b83788409 to d7196b0b4939 (1 revision) (flutter/flutter#188066) 2026-06-16 [email protected] Roll Skia from ef17057bb776 to 9c2b83788409 (1 revision) (flutter/flutter#188061) 2026-06-16 [email protected] Roll Skia from 500025456bb5 to ef17057bb776 (1 revision) (flutter/flutter#188058) 2026-06-16 [email protected] Roll Skia from cb1035ff14bf to 500025456bb5 (5 revisions) (flutter/flutter#188057) 2026-06-16 [email protected] Roll Fuchsia Linux SDK from TbB86Po_HDe1dvXvT... to VeLhhlDcod09NR4Hb... (flutter/flutter#188055) 2026-06-16 [email protected] Roll Skia from 70acf6a5e7c9 to cb1035ff14bf (3 revisions) (flutter/flutter#188054) 2026-06-16 [email protected] [pv]skip non-tappable web view workaround on ios 26.4 (flutter/flutter#185424) 2026-06-16 [email protected] [web] RenderParagraph needs paint after a DPR change (flutter/flutter#186968) 2026-06-16 [email protected] Adds gamma correction to windows text. (flutter/flutter#187871) 2026-06-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add a platform view test to android_hardware_smoke_test (#187913)" (flutter/flutter#188051) 2026-06-15 [email protected] Add a platform view test to android_hardware_smoke_test (flutter/flutter#187913) 2026-06-15 [email protected] feat: linux_analyze in a workflow (flutter/flutter#187889) 2026-06-15 [email protected] [web] Changes to WebParagraph configuration (flutter/flutter#187188) 2026-06-15 [email protected] Fail gracefully on Android AVD lock errors during startup (flutter/flutter#187200) 2026-06-15 [email protected] [flutter_tools] Fix flakiness in widget_preview_detection_test (flutter/flutter#187938) 2026-06-15 [email protected] Exclude fuchsia-sdk/sdk/.build-id from the builder cache archive (flutter/flutter#187826) 2026-06-15 [email protected] Roll Skia from c8d9f80f13e4 to 70acf6a5e7c9 (4 revisions) (flutter/flutter#188020) 2026-06-15 [email protected] Roll Packages from b78ad83 to aa964a3 (7 revisions) (flutter/flutter#188021) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Pass the correct font line to Canvas2D I was only able to demonstrate it working on text fonts (Sans vs Serif). Part of flutter#172561 --------- Co-authored-by: Mouad Debbar <[email protected]>
3 participants
Pass the correct font line to Canvas2D
I was only able to demonstrate it working on text fonts (Sans vs Serif).
Part of #172561