Add space metrics tracking for trailing whitespace "ghost" runs. #7791
Conversation
third_party/txt/src/txt/paragraph.cc
Outdated
| // centering/layout/etc). See above declaration of 'ghost_run_count' for | ||
| // more details. | ||
| run_index++; | ||
| bool is_ghost_run = false; |
There was a problem hiding this comment.
Would it make sense to add the is_ghost flag to the BidiRun class?
There was a problem hiding this comment.
So a bidi run can actually be split into multiple parts, where the beginning half is a normal run and the last half is span of trailing whitespaces. Only once we split it into line_runs do we reach the level where a 'ghost' run can be defined.
For example, a line_range can have the range [0, 20, 40) where [20, 40) are whitespace. A corresponding bidi run can then be [0, 40), which should be broken into [0, 20) and [20, 40) where the first run is normal and the second is a 'ghost'.
Thus, we can't blanket a is_ghost var onto a whole BidiRun
There was a problem hiding this comment.
line_runs is a list of BidiRun instances. I was thinking that it would be simpler to add a flag to the BidiRun struct to track the ghost runs instead of keeping state about which line_runs indexes are ghost runs.
Or maybe a better option would be to create a LineRun struct that includes all the fields of BidiRun plus the ghost flag.
|
The existing tests for the old behavior actually tests this new feature quite well. I modified them to assert the new behavior. |
|
Waiting for buildbots to succeed and flutter/flutter to go green |
Fixes flutter/flutter#26658.
This adds the concept of a "ghost" run representing trailing whitespace where it does not impact the layout of visible glyphs, but does have metrics as if it existed in GetRectsForRange().
Previously, these space metrics were discarded.
This allows trailing spaces to appear at the end of center and right aligned text without changing the alignment.
Will require a manual engine roll as it breaks one test in the framework specifically testing for this (flutter/flutter#27808)