-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Improve TextPainter.layout caching
#118128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ui.Paragraph.layout calls.ui.Paragraph.layout calls, make it less destructive
ui.Paragraph.layout calls, make it less destructiveui.Paragraph.layout caching
ui.Paragraph.layout cachingTextPainter.layout caching
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.
we really should stop doing this
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.
also the comment is just wrong (it's been wrong for a long time). we looked at doing what it says, but we decided not to, many years ago. if we're going to make a breaking change anyway, we might as well remove this at the same time...
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.
based on the conversation in #31707, it doesn't seem to be safe to remove this hack yet, since widths/maxIntrinsicWidths are reported in floats and the input widths are doubles
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.
After removing the rounding in the framework and skparagraph, and tuning down the harfbuzz scale from 1^16 to 1^10, with this change there are only 50 framework failures and nothing seems to be overflowing. So I guess that's doable but I'd like to try to do that in a separate PR.
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.
@Hixie still this breaks 500+ scuba tests. But I'm not sure how I can write a migration guide for this one.
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.
(nit: extraneous spaces after //)
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.
not sure what this comment means
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.
removed. I thought if the text doesn't contain hard breaks the extra width will be distributed among the whitespaces in that line, but turns out the end of line is counted as a hard break so when maxWidth > maxIntrinsicWidth it's the same as TextAlign.start.
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.
for clarity it might be good to say 0.0 and 1.0 instead of 0 and 1 when they're doubles
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.
this might be clearer?
| final List<TextBox> rawBoxes = layout._inlinePlaceholderBoxes | |
| ??= layout.paragraph.getBoxesForPlaceholders(); | |
| layout._inlinePlaceholderBoxes ??= layout.paragraph.getBoxesForPlaceholders(); | |
| final List<TextBox> rawBoxes = layout._inlinePlaceholderBoxes; |
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.
_inlinePlaceholderBoxes's type is List<TextBox>? so I would have to use !.
Roll Flutter from 216605b to ee162e4 (53 revisions) flutter/flutter@216605b...ee162e4 2023-05-26 [email protected] Roll Packages from 995bfc5 to 9f8dcc5 (4 revisions) (flutter/flutter#127671) 2023-05-26 [email protected] Roll Flutter Engine from 474fcbbe445e to 8573f3b63a1f (1 revision) (flutter/flutter#127647) 2023-05-26 [email protected] Roll Flutter Engine from f63fcf5a274e to 474fcbbe445e (1 revision) (flutter/flutter#127644) 2023-05-26 [email protected] Roll Flutter Engine from b6fcbe31ba1a to f63fcf5a274e (1 revision) (flutter/flutter#127642) 2023-05-26 [email protected] Roll Flutter Engine from bec5da5bff7c to b6fcbe31ba1a (1 revision) (flutter/flutter#127641) 2023-05-26 [email protected] Roll Flutter Engine from 593d4d6b1f9b to bec5da5bff7c (2 revisions) (flutter/flutter#127638) 2023-05-26 [email protected] Roll Flutter Engine from f4dc96aef71b to 593d4d6b1f9b (1 revision) (flutter/flutter#127635) 2023-05-26 [email protected] Roll Flutter Engine from ff04d2fdd12b to f4dc96aef71b (2 revisions) (flutter/flutter#127633) 2023-05-26 [email protected] Remove a bad assert from tooltip implementation (flutter/flutter#127629) 2023-05-26 [email protected] Remove rounding from TextPainter (flutter/flutter#127099) 2023-05-26 [email protected] Adds `TestDisplay` API for testing `Display` features (flutter/flutter#127525) 2023-05-25 [email protected] Roll Flutter Engine from 6a4e675ab0b4 to ff04d2fdd12b (3 revisions) (flutter/flutter#127630) 2023-05-25 [email protected] Roll Flutter Engine from 99f2653cbb79 to 6a4e675ab0b4 (2 revisions) (flutter/flutter#127625) 2023-05-25 [email protected] Append $flutter/osx sdk property to existing platforms/targets relying on xcode (flutter/flutter#127537) 2023-05-25 [email protected] Indent markdown value on CP issue template (flutter/flutter#127623) 2023-05-25 [email protected] Roll Flutter Engine from 6ff02c17268f to 99f2653cbb79 (6 revisions) (flutter/flutter#127619) 2023-05-25 [email protected] `ProcessResultMatcher` created and used in test (flutter/flutter#127414) 2023-05-25 [email protected] Fix yaml lint issue (flutter/flutter#127600) 2023-05-25 [email protected] Roll Flutter Engine from 515a4bfc4b6e to 6ff02c17268f (1 revision) (flutter/flutter#127609) 2023-05-25 [email protected] `SearchBar` should not be impacted by overall `InputDecorationTheme` (flutter/flutter#127465) 2023-05-25 [email protected] Roll Flutter Engine from 195009b91abd to 515a4bfc4b6e (4 revisions) (flutter/flutter#127601) 2023-05-25 [email protected] Roll Flutter Engine from fb463217d1f3 to 195009b91abd (1 revision) (flutter/flutter#127591) 2023-05-25 [email protected] avoid incorrect usage of TimelineTask (flutter/flutter#127527) 2023-05-25 [email protected] Add `ScrollNotificationObserver` sample (flutter/flutter#127023) 2023-05-25 [email protected] Roll Packages from fba97fa to 995bfc5 (3 revisions) (flutter/flutter#127590) 2023-05-25 [email protected] Roll Flutter Engine from a74d9d1f4776 to fb463217d1f3 (1 revision) (flutter/flutter#127577) 2023-05-25 [email protected] Roll Flutter Engine from a7f026b03e35 to a74d9d1f4776 (1 revision) (flutter/flutter#127565) 2023-05-25 [email protected] Roll Flutter Engine from fb38bb6b1bc1 to a7f026b03e35 (1 revision) (flutter/flutter#127563) 2023-05-25 [email protected] Roll Flutter Engine from 2be736c390ee to fb38bb6b1bc1 (1 revision) (flutter/flutter#127560) 2023-05-25 [email protected] Remove Tooltip mouse tracker listener & update hovering/MouseRegion logic & animation (flutter/flutter#119199) 2023-05-25 [email protected] Roll Flutter Engine from 37ebad2d5c27 to 2be736c390ee (2 revisions) (flutter/flutter#127554) 2023-05-25 [email protected] Roll Flutter Engine from 62a83490ee60 to 37ebad2d5c27 (1 revision) (flutter/flutter#127551) 2023-05-25 [email protected] Roll Flutter Engine from 1bed04a4375a to 62a83490ee60 (3 revisions) (flutter/flutter#127546) 2023-05-25 [email protected] Roll Flutter Engine from 6a361090a367 to 1bed04a4375a (2 revisions) (flutter/flutter#127544) 2023-05-25 [email protected] Roll Flutter Engine from 9ba461efd3fe to 6a361090a367 (3 revisions) (flutter/flutter#127542) 2023-05-24 [email protected] Revert "Update labeler action wildcards" (flutter/flutter#127541) 2023-05-24 [email protected] [web] ui.platformViewRegistry => ui_web.platformViewRegistry (flutter/flutter#127493) 2023-05-24 [email protected] Roll Flutter Engine from c641f6307f08 to 9ba461efd3fe (6 revisions) (flutter/flutter#127536) 2023-05-24 [email protected] Add missing parameters in `TextFormField` (flutter/flutter#127020) 2023-05-24 [email protected] Update labeler action wildcards (flutter/flutter#127524) 2023-05-24 [email protected] Improve `TextPainter.layout` caching (flutter/flutter#118128) 2023-05-24 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 2713f7303c96cb1e69627957ec16eea0fd7f94a4 to 0776a679364a9a16110aac8d0f40f5e11009e327 (flutter/flutter#127533) 2023-05-24 [email protected] Fix ScrollPosition overscroll precision error (flutter/flutter#127321) 2023-05-24 [email protected] `Slider.onChangeStart` & `Slider.onChangeEnd` are not called on keyboard shortcuts (flutter/flutter#126896) 2023-05-24 [email protected] Manually roll Flutter Engine from 0c41b02cd5a6 to c641f6307f08 (flutter/flutter#127514) 2023-05-24 [email protected] Update CP template to include timeline and changelog (flutter/flutter#127507) ...
Improves
TextPainter.layoutcaching when only the input constraints change:TextPainter._layoutParagraph: now double layout is only needed whenTextAlignis not left, and the inputmaxWidth == double.infinity.ui.Paragraph.layoutwhen it's guaranteed that there's no soft line breaks before/after the layout call.This doesn't introduce new APIs but may slightly shift text rendered on screen.
This reduces the number of
layoutcalls but since shaping results are already cached so it only skips the relatively cheap line-breaking process when possible.528 scuba failures but all of them seem reasonable.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.