Use SkTextBlob::bounds instead of TextFrame::getBounds().#45148
Use SkTextBlob::bounds instead of TextFrame::getBounds().#45148matanlurey merged 3 commits intoflutter:mainfrom
SkTextBlob::bounds instead of TextFrame::getBounds().#45148Conversation
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
LGTM
If our text frame bounds computation is wrong, that is going to bubble up to more places too.
Can we either 1) make impeller::TextFrame use the bounds data from the SkTextFrame 2) fix the impeller::TextFrame computation?
|
I'm still learning the words, so I'm not sure what difference between a |
|
The impeller::TextFrame is the analog of the SkTextBlob |
|
(Not necessarily in this PR, FWIW) |
|
Actually in general TextFrame::GetBounds is super hot, so we might want to take the SkiaBounds in the constructor and use them cached, see #45131 (comment) |
|
Ah, sure! Filed flutter/flutter#133388. |
flutter/engine@683d7dc...b5f1821 2023-08-27 matanlurey@users.noreply.github.com Use `SkTextBlob::bounds` instead of `TextFrame::getBounds()`. (flutter/engine#45148) 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 jimgraham@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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
2 participants
Closes flutter/flutter#133157.