-
Notifications
You must be signed in to change notification settings - Fork 6k
[skwasm] Use test fonts while in debugEmulateFlutterTesterEnvironment mode #51630
Conversation
ditman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but is the test lying in a comment?
| final ui.LineMetrics? metrics = paragraph.getLineMetricsAt(0); | ||
| expect(metrics, isNotNull); | ||
|
|
||
| // Ahem's 'X' character is a square, so it's the font size (10.0) * 4 characters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the fallback font now FlutterTest? (_testFonts.first), or am I misunderstanding the testing fallback?
(If it's not Ahem, then I guess this comment is a little lie.)
PS: is there any way of adding an expectation of what font was eventually used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think FlutterTest is actually Ahem. It's the same font with a different name. Why do we have a FlutterTest font that is actually Ahem? I don't really know the historical reason. I'm just trying to match the behavior of CanvasKit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked up some more context, which can be found in this PR: #39809
The FlutterTest is almost Ahem, with a few modifications. I'm changing the comment to be more accurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, and in answer to your second question, no. The font is substituted under the hood in a way that isn't directly visible to a client via dart:ui. So we're doing measurements instead.
flutter/engine@bc4fa43...8a51e12 2024-03-22 skia-flutter-autoroll@skia.org Roll Skia from 10a460af3308 to 2e99e3a5445c (1 revision) (flutter/engine#51632) 2024-03-22 skia-flutter-autoroll@skia.org Roll Dart SDK from a959c7def805 to ad26f4885bf4 (1 revision) (flutter/engine#51631) 2024-03-22 1961493+harryterkelsen@users.noreply.github.com [web] Add ability to customize font fallback download URL (flutter/engine#51569) 2024-03-22 jacksongardner@google.com [skwasm] Use test fonts while in debugEmulateFlutterTesterEnvironment mode (flutter/engine#51630) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jonahwilliams@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
This brings the behavior in line with the other renderers. The framework sets this bit to make sure we render only using the Ahem font.