Iterate through potential grapheme cluster lengths in text painter#24797
Merged
GaryQian merged 23 commits intoflutter:masterfrom Dec 18, 2018
Merged
Iterate through potential grapheme cluster lengths in text painter#24797GaryQian merged 23 commits intoflutter:masterfrom
GaryQian merged 23 commits intoflutter:masterfrom
Conversation
GaryQian
added
a: text input
zoechi
added
the
framework
Contributor
Author
|
The whitespace hadnling responsibility has been shifted to LibTxt in flutter/engine#7164. This PR can focus on just the emoji/grapheme clusters only. Will require an engine roll once it lands for the tests in here to pass. |
Hixie
reviewed
Dec 11, 2018
xster
reviewed
Dec 18, 2018
Member
xster
left a comment
xster
left a comment
There was a problem hiding this comment.
LGTM.
Doesn't have to be in this PR but it would be great to add a benchmark test for text operations so we have a baseline and changes are quantifiable.
Contributor
Author
|
Filed an issue to benchmark the text field. #25523 |
GaryQian
added a commit
to GaryQian/flutter
that referenced
this pull request
Dec 18, 2018
…inter (flutter#24797)" This reverts commit f8964ae.
Contributor
Author
|
Reverting for now as it causes an unforeseen and untested iOS issue |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since we cannot yet access the extended grapheme clusters in a dart string, we cannot know how much to offset the index such that we can draw a minimal rectangle around the nearest character.
This uses a loop to double the size of the index offset when it cannot find a glyph within the range when the glyph is defined by an extended grapheme cluster.
We also perform the same search when the current code unit is whitespace. LibTxt may truncate the layout early on trailing whitespace in order to properly align text (particularly centered text), so we need to obtain the last drawn character in order to find a bounding box.
Fixes #24266, #24456
Partially fixes #24203, #22627, #24802
Overall, this fixes the inability to move the caret into emoji/zwj strings on the first line. On the second+ lines, the caret still has a different bug where you are able to move it within a single glyph and edit the individual components of a zwj glyph.