Pass LinearTextFlag to SkFont - iOS13 letter spacing#13321
Pass LinearTextFlag to SkFont - iOS13 letter spacing#13321GaryQian merged 3 commits intoflutter:masterfrom
Conversation
|
Actually, this is WIP. This breaks tons of txt_unittests by a tiny subpixel amount. Need to verify if this is ok. cc @Rusino Heads up! This fix is quite important though as without it, iOS 13 is very ugly. |
|
It is possible this may also require goldens/scuba updates if the changes are acceptable. |
|
After reviewing the layout results, it seems this would introduce regressions in many cases and Will explore more specific or alternative solutions. |
|
Modified to pass the LinearTextFlag to SkFont. Will need to verify this works as expected and maintains fixing ios13. |
| MinikinPaint* paint = reinterpret_cast<MinikinPaint*>(fontData); | ||
| float advance = paint->font->GetHorizontalAdvance(glyph, *paint); | ||
| return 256 * advance + 0.5; | ||
| return 256 * advance; |
There was a problem hiding this comment.
I could not figure out why there is a + 0.5. I have removed it. This constant seems to have no positive effect on the layout and would break a large suite of tests for no discernible reason.
There was a problem hiding this comment.
I will be putting the 0.5 back in, as the framework tests/goldens will fully pass with this constant addition. This may be to mirror harfbuzz's implementation. In any case, with the + 0.5, we no longer need to manually roll, though it will require many more small adjustments throughout txt_unittests
There was a problem hiding this comment.
the 0.5 was to make the rounding round both up and down instead of flooring
|
Passing LinearTextFlag causes the if statement in Skia |
GaryQian
changed the title
|
This will require a manual engine roll due to sub-pixel differences in the text framework tests. |
|
Confirmed this fixes iOS 13 letter spacing bug and handles bold properly. |
|
This no longer needs a manual roll. |
GaryQian
changed the title
[email protected]:flutter/engine.git/compare/869b74eb4eca...728e473 git log 869b74e..728e473 --no-merges --oneline 2019-10-28 [email protected] Pass LinearTextFlat to SkFont - iOS13 letter spacing (flutter/engine#13321) 2019-10-28 [email protected] Roll src/third_party/skia 428b5de64a08..18f5b1a6dd77 (1 commits) (flutter/engine#13387) 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 [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
[email protected]:flutter/engine.git/compare/869b74eb4eca...728e473 git log 869b74e..728e473 --no-merges --oneline 2019-10-28 [email protected] Pass LinearTextFlat to SkFont - iOS13 letter spacing (flutter/engine#13321) 2019-10-28 [email protected] Roll src/third_party/skia 428b5de64a08..18f5b1a6dd77 (1 commits) (flutter/engine#13387) 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 [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
3 participants
Call hb_font_set_funcs again since harfbuzz is now modern.
This fixes flutter/flutter#41101