engine
engine copied to clipboard
Expose `ui.Paragraph.debugGetFontAt`
Expose ui.Paragraph.debugGetFontAt
for debugging purposes. The goal is to allow developers to inspect the actual font used to render a glyph in debug mode.
Currently it always return null for canvaskit / skwasm, and it doesn't seem that the HTML renders will be able to support this.
Pre-launch Checklist
- [ ] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [ ] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [ ] I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
- [ ] I listed at least one issue that this PR fixes in the description above.
- [ ] I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
- [ ] I updated/added relevant documentation (doc comments with
///
). - [ ] I signed the CLA.
- [ ] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
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.
Changes reported for pull request #50256 at sha abf09f5f246fd1543a6f434786994cf82e747937
The implementation looks correct.
But I'd be concerned that this API will leak engine and Skia implementation details that are subject to change. Even if the API is only supported in debug mode, I'm suspicious that apps or tests may come to rely on behaviors that should not be part of the engine's interface.
For example, the test in the PR assumes that a TextStyle with an empty font family will be resolved to the "FlutterTest" font and that this font's internal font name is "MingLiU". I would prefer not to hardcode those assumptions.
Or as noted in the API's comments, the font family name returned by Skia to this API may be an internal alias for a fallback font.
Is there an issue associated with this PR? What kind of debugging purpose?
Yeah good point. Nevertheless I think the method will still be useful for debugging despite the return value may not be stable. I added a paragraph to the documentation to advise against using the method in tests or the actual application.
For example, the test in the PR assumes that a TextStyle with an empty font family will be resolved to the "FlutterTest" font and that this font's internal font name is "MingLiU". I would prefer not to hardcode those assumptions.
I think that's more of a hack on the font's part since I needed to disable freetype autohint but couldn't find a way to access the underlying SkFont
object.
Is there an issue associated with this PR? What kind of debugging purpose?
For example if someone uses different scripts in the same TextSpan
the vertical alignment of the text and the baseline location may look strange because the characters from different scripts may fallback to different typefaces. I think a debug method will help identify the case without having to build a custom engine to print out the font info.
Somewhat related: https://github.com/flutter/flutter/issues/139778
On second thought I think developers should be able to use this in tests:
- This uses public APIs on
SkFont
andSkTypeface
so these don't seem to be considered implementation details by skia. - If developers want to make sure the proper font is selected, there should be a way to do that. Right now the only way to verify that is to use golden tests, and that's usually more susceptible to changes.
- It's unlikely that people will write tests to verify the font is "27##fallback".
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.
not so sure about the part that mentions font loading now. I'll revise the API docs after taking a closer look.
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.
(triage) I spoke to @LongCatIsLooong and he said he wants to give this one another try soon.