Glitchiness with Tab Characters #8591
Conversation
|
This library was originally built to support Android text layout APIs. Android would call I'd recommend having the |
10f337a to
c703c00
Compare
|
Thanks for explaining that @jason-simmons! I've updated the PR to use your approach and it seems to work just as well. I'm seeing a tangentially related crash when putting the cursor between tabs that I'm going to investigate before I finalize this PR. |
|
Unfortunately I'm having trouble tracking down the tab crash. I created a separate issue for it because it existed before this PR and isn't strictly related: flutter/flutter#31154 I'm happy to take a deeper look at it though if someone points me in the right direction. I'm unfamiliar with what could be causing it and unable to get a full stack trace for now. |
|
After taking a look at flutter/flutter#31158, I'm now thinking that it might be better to disable the tab handling logic in the Minikin LineBreaker: Flutter doesn't provide the Android StaticLayout tab stop behavior that Minikin's logic appears to be implementing. Removing the special handling of tabs in LineBreaker results in tabs acting like spaces for the purposes of line breaking and layout. |
|
@jason-simmons Ready for re-review. I removed the tab code altogether and it does seem to work fine. |
| postSpaceCount = mSpaceCount; | ||
| afterWord = i + 1; | ||
| } | ||
| if (isWordSpace(c)) |
There was a problem hiding this comment.
Add a "libtxt:" comment here saying that tab handling has been removed.
(This is intended to keep track of how our version of Minikin differs from the original)
There was a problem hiding this comment.
Got it. Thanks for the review.
currentLineWidth is the width passed into Paragraph layout, which comes from the maxIntrinsicWidth returned by a previous call to Paragraph layout. That width is calculated by Layout::measureText. postBreak is calculated from the character widths in the LineBreaker. A slight mismatch between these two widths may unnecessarily cause the insertion of desperate breaks in addWordBreak. Adding some slack to currentLineWidth works around this. Fixes flutter#8591
flutter/engine@b678709...06fea14 git log b678709..06fea14 --no-merges --oneline 06fea14 Glitchiness with Tab Characters (flutter/engine#8591) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff (cbracken@google.com), and stop the roller if necessary.
Closes flutter/flutter#30173
Entering tab characters into a text field is causing some glitchiness. It may suddenly cause a single line input to wrap, or the text to flash, etc. at random moments. Here is a gif from the issue:
It seems like this happens because
mTabWidthis unassigned and never set, so it contains random values from memory, sometimes zero and sometimes in the billions. It looks to me like the code that is supposed to set this variable is never called.My proposed solution in this PR is to hardcode the tab width to 2 and delete the code that's never called. This fixes the problem for me and seems to give some reasonable behavior for tab characters. Does anyone else know what the intention of the original code was? Or any opinions on the width of tabs or what should happen when a tab is entered?
For the record, here is the original commit that added this code in 2015: 01f5266144