From d2aaf3394aaf2f6c77f3b083c1621495673a4287 Mon Sep 17 00:00:00 2001 From: Roozbeh Pournader Date: Tue, 14 Mar 2017 14:59:58 -0700 Subject: [PATCH] In greedy line breaking, repeat breaks until the line fits Previously, in greedy line breaking, when a line overflowed, we found the best line breaking candidate before it and broke the line there. But we didn't check to see if the remaining part now fits in a line. With this change, we now repeat checking for overflows, and break again until we have no breaking opportunity or the remaining text now fits in a line. Also found an issue with greedy line breaking and keeping the hyphenation edit for the next line which is now fixed. Test: Manual. The issue reported in the bug is now fixed. Bug: 34185255 Bug: https://code.google.com/p/android/issues/detail?id=231437 Bug: 33560754 Change-Id: I93bdd341e4f8e1257710e453e4938f224cb2a1ff --- .../src/flutter/include/minikin/LineBreaker.h | 6 +- .../src/flutter/libs/minikin/LineBreaker.cpp | 56 +++++++++++++++---- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/engine/src/flutter/include/minikin/LineBreaker.h b/engine/src/flutter/include/minikin/LineBreaker.h index ce8eb7d571..c91c0b3da7 100644 --- a/engine/src/flutter/include/minikin/LineBreaker.h +++ b/engine/src/flutter/include/minikin/LineBreaker.h @@ -191,8 +191,8 @@ class LineBreaker { struct Candidate { size_t offset; // offset to text buffer, in code units size_t prev; // index to previous break - ParaWidth preBreak; - ParaWidth postBreak; + ParaWidth preBreak; // width of text until this point, if we decide to not break here + ParaWidth postBreak; // width of text until this point, if we decide to break here float penalty; // penalty of this break (for example, hyphen penalty) float score; // best score found for this break size_t lineNumber; // only updated for non-constant line widths @@ -207,6 +207,7 @@ class LineBreaker { size_t preSpaceCount, size_t postSpaceCount, float penalty, HyphenationType hyph); void addCandidate(Candidate cand); + void pushGreedyBreak(); // push an actual break to the output. Takes care of setting flags for tab void pushBreak(int offset, float width, uint8_t hyphenEdit); @@ -248,6 +249,7 @@ class LineBreaker { size_t mBestBreak; float mBestScore; ParaWidth mPreBreak; // prebreak of last break + uint32_t mLastHyphenation; // hyphen edit of last break kept for next line int mFirstTabIndex; size_t mSpaceCount; }; diff --git a/engine/src/flutter/libs/minikin/LineBreaker.cpp b/engine/src/flutter/libs/minikin/LineBreaker.cpp index b8814b63d4..e75c7bf523 100644 --- a/engine/src/flutter/libs/minikin/LineBreaker.cpp +++ b/engine/src/flutter/libs/minikin/LineBreaker.cpp @@ -84,6 +84,7 @@ void LineBreaker::setText() { mBestBreak = 0; mBestScore = SCORE_INFTY; mPreBreak = 0; + mLastHyphenation = HyphenEdit::NO_EDIT; mFirstTabIndex = INT_MAX; mSpaceCount = 0; } @@ -269,24 +270,59 @@ void LineBreaker::addWordBreak(size_t offset, ParaWidth preBreak, ParaWidth post addCandidate(cand); } +// Helper method for addCandidate() +void LineBreaker::pushGreedyBreak() { + const Candidate& bestCandidate = mCandidates[mBestBreak]; + pushBreak(bestCandidate.offset, bestCandidate.postBreak - mPreBreak, + mLastHyphenation | HyphenEdit::editForThisLine(bestCandidate.hyphenType)); + mBestScore = SCORE_INFTY; +#if VERBOSE_DEBUG + ALOGD("break: %d %g", mBreaks.back(), mWidths.back()); +#endif + mLastBreak = mBestBreak; + mPreBreak = bestCandidate.preBreak; + mLastHyphenation = HyphenEdit::editForNextLine(bestCandidate.hyphenType); +} + // TODO performance: could avoid populating mCandidates if greedy only void LineBreaker::addCandidate(Candidate cand) { - size_t candIndex = mCandidates.size(); + const size_t candIndex = mCandidates.size(); mCandidates.push_back(cand); + + // mLastBreak is the index of the last line break we decided to do in mCandidates, + // and mPreBreak is its preBreak value. mBestBreak is the index of the best line breaking candidate + // we have found since then, and mBestScore is its penalty. if (cand.postBreak - mPreBreak > currentLineWidth()) { // This break would create an overfull line, pick the best break and break there (greedy) if (mBestBreak == mLastBreak) { + // No good break has been found since last break. Break here. mBestBreak = candIndex; } - pushBreak(mCandidates[mBestBreak].offset, mCandidates[mBestBreak].postBreak - mPreBreak, - HyphenEdit::editForThisLine(mCandidates[mBestBreak].hyphenType)); - mBestScore = SCORE_INFTY; -#if VERBOSE_DEBUG - ALOGD("break: %d %g", mBreaks.back(), mWidths.back()); -#endif - mLastBreak = mBestBreak; - mPreBreak = mCandidates[mBestBreak].preBreak; + pushGreedyBreak(); } + + while (mLastBreak != candIndex && cand.postBreak - mPreBreak > currentLineWidth()) { + // We should rarely come here. But if we are here, we have broken the line, but the + // remaining part still doesn't fit. We now need to break at the second best place after the + // last break, but we have not kept that information, so we need to go back and find it. + // + // In some really rare cases, postBreak - preBreak of a candidate itself may be over the + // current line width. We protect ourselves against an infinite loop in that case by + // checking that we have not broken the line at this candidate already. + for (size_t i = mLastBreak + 1; i < candIndex; i++) { + const float penalty = mCandidates[i].penalty; + if (penalty <= mBestScore) { + mBestBreak = i; + mBestScore = penalty; + } + } + if (mBestBreak == mLastBreak) { + // We didn't find anything good. Break here. + mBestBreak = candIndex; + } + pushGreedyBreak(); + } + if (cand.penalty <= mBestScore) { mBestBreak = candIndex; mBestScore = cand.penalty; @@ -329,7 +365,7 @@ void LineBreaker::computeBreaksGreedy() { size_t nCand = mCandidates.size(); if (nCand == 1 || mLastBreak != nCand - 1) { pushBreak(mCandidates[nCand - 1].offset, mCandidates[nCand - 1].postBreak - mPreBreak, - HyphenEdit::NO_EDIT); + mLastHyphenation); // don't need to update mBestScore, because we're done #if VERBOSE_DEBUG ALOGD("final break: %d %g", mBreaks.back(), mWidths.back());