Fix bug in QTextLayout::min/maxWidth for WrapAtWordBoundaryOrAnywhere

In that specific wrapping mode, it will first try a normal word wrap. If
it doesn't fit within the specified line width it will discard the
result of that and try WrapAnywhere by calling layout_helper()
recursively. The problem was that at the point it called itself again it
had already adjusted eng->maxWidth:

        eng->maxWidth += line.textWidth;

This was not restored, but carried on to the recursive call to
layout_helper(), so the end result was that the maximumWidth would
accumulate text widths from parts of the same line twice.

Due to the same recursive behavior the minimumWidth also had a problem:
It always returned the width of the widest word because it took the
qMax() of the minimum widths of the two passes, (WordWrap and then
WrapAnywhere) effectively making the minimum width always be the width
of the widest word (even though it could wrap at finer granularity).

Pick-to: 5.15
Task-number: QTBUG-77337
Change-Id: Ie7e9c17b157506352c2da38cc7f4a8dfa1283966
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
bb10
Jan Arve Sæther 2020-04-29 18:18:51 +02:00
parent 0534aeffe9
commit 8d6d1d6fea
2 changed files with 68 additions and 20 deletions

View File

@ -1757,7 +1757,6 @@ inline bool LineBreakHelper::checkFullOtherwiseExtend(QScriptLine &line)
return true;
const QFixed oldTextWidth = line.textWidth;
minw = qMax(minw, tmpData.textWidth);
line += tmpData;
line.textWidth += spaceData.textWidth;
@ -1780,13 +1779,14 @@ inline bool LineBreakHelper::checkFullOtherwiseExtend(QScriptLine &line)
static inline void addNextCluster(int &pos, int end, QScriptLine &line, int &glyphCount,
const QScriptItem &current, const unsigned short *logClusters,
const QGlyphLayout &glyphs)
const QGlyphLayout &glyphs, QFixed *clusterWidth = nullptr)
{
int glyphPosition = logClusters[pos];
do { // got to the first next cluster
++pos;
++line.length;
} while (pos < end && logClusters[pos] == glyphPosition);
QFixed clusterWid = line.textWidth;
do { // calculate the textWidth for the rest of the current cluster.
if (!glyphs.attributes[glyphPosition].dontPrint)
line.textWidth += glyphs.advances[glyphPosition];
@ -1795,6 +1795,8 @@ static inline void addNextCluster(int &pos, int end, QScriptLine &line, int &gly
Q_ASSERT((pos == end && glyphPosition == current.num_glyphs) || logClusters[pos] == glyphPosition);
if (clusterWidth)
*clusterWidth += (line.textWidth - clusterWid);
++glyphCount;
}
@ -1821,6 +1823,7 @@ void QTextLine::layout_helper(int maxGlyphs)
QTextOption::WrapMode wrapMode = eng->option.wrapMode();
bool breakany = (wrapMode == QTextOption::WrapAnywhere);
const bool breakWordOrAny = breakany || (wrapMode == QTextOption::WrapAtWordBoundaryOrAnywhere);
lbh.manualWrap = (wrapMode == QTextOption::ManualWrap || wrapMode == QTextOption::NoWrap);
int item = -1;
@ -1957,9 +1960,10 @@ void QTextLine::layout_helper(int maxGlyphs)
lbh.whiteSpaceOrObject = false;
bool sb_or_ws = false;
lbh.saveCurrentGlyph();
QFixed accumulatedTextWidth;
do {
addNextCluster(lbh.currentPosition, end, lbh.tmpData, lbh.glyphCount,
current, lbh.logClusters, lbh.glyphs);
current, lbh.logClusters, lbh.glyphs, &accumulatedTextWidth);
// This is a hack to fix a regression caused by the introduction of the
// whitespace flag to non-breakable spaces and will cause the non-breakable
@ -1975,11 +1979,16 @@ void QTextLine::layout_helper(int maxGlyphs)
|| attributes[lbh.currentPosition].lineBreak) {
sb_or_ws = true;
break;
} else if (breakany && attributes[lbh.currentPosition].graphemeBoundary) {
break;
} else if (attributes[lbh.currentPosition].graphemeBoundary) {
if (breakWordOrAny) {
lbh.minw = qMax(accumulatedTextWidth, lbh.minw);
accumulatedTextWidth = 0;
}
if (breakany)
break;
}
} while (lbh.currentPosition < end);
lbh.minw = qMax(lbh.tmpData.textWidth, lbh.minw);
lbh.minw = qMax(accumulatedTextWidth, lbh.minw);
if (lbh.currentPosition > 0 && lbh.currentPosition <= end
&& (lbh.currentPosition == end || attributes[lbh.currentPosition].lineBreak)
@ -2106,6 +2115,20 @@ found:
line.descent.toReal(), line.textWidth.toReal(), lbh.spaceData.width.toReal());
LB_DEBUG(" : '%s'", eng->layoutData->string.mid(line.from, line.length).toUtf8().data());
const QFixed trailingSpace = (eng->option.flags() & QTextOption::IncludeTrailingSpaces
? lbh.spaceData.textWidth
: QFixed(0));
if (eng->option.wrapMode() == QTextOption::WrapAtWordBoundaryOrAnywhere) {
if ((lbh.maxGlyphs != INT_MAX && lbh.glyphCount > lbh.maxGlyphs)
|| (lbh.maxGlyphs == INT_MAX && line.textWidth > (line.width - trailingSpace))) {
eng->option.setWrapMode(QTextOption::WrapAnywhere);
layout_helper(lbh.maxGlyphs);
eng->option.setWrapMode(QTextOption::WrapAtWordBoundaryOrAnywhere);
return;
}
}
if (lbh.manualWrap) {
eng->minWidth = qMax(eng->minWidth, line.textWidth);
eng->maxWidth = qMax(eng->maxWidth, line.textWidth);
@ -2116,8 +2139,8 @@ found:
if (line.textWidth > 0 && item < eng->layoutData->items.size())
eng->maxWidth += lbh.spaceData.textWidth;
if (eng->option.flags() & QTextOption::IncludeTrailingSpaces)
line.textWidth += lbh.spaceData.textWidth;
line.textWidth += trailingSpace;
if (lbh.spaceData.length) {
line.trailingSpaces = lbh.spaceData.length;
line.hasTrailingSpaces = true;
@ -2125,18 +2148,6 @@ found:
line.justified = false;
line.gridfitted = false;
if (eng->option.wrapMode() == QTextOption::WrapAtWordBoundaryOrAnywhere) {
if ((lbh.maxGlyphs != INT_MAX && lbh.glyphCount > lbh.maxGlyphs)
|| (lbh.maxGlyphs == INT_MAX && line.textWidth > line.width)) {
eng->option.setWrapMode(QTextOption::WrapAnywhere);
line.length = 0;
line.textWidth = 0;
layout_helper(lbh.maxGlyphs);
eng->option.setWrapMode(QTextOption::WrapAtWordBoundaryOrAnywhere);
}
}
}
/*!

View File

@ -142,6 +142,7 @@ private slots:
void koreanWordWrap();
void tooManyDirectionalCharctersCrash_qtbug77819();
void softHyphens();
void min_maximumWidth();
private:
QFont testFont;
@ -2488,5 +2489,41 @@ void tst_QTextLayout::softHyphens()
}
}
void tst_QTextLayout::min_maximumWidth()
{
QString longString("lmong_long_crazy_87235982735_23857239682376923876923876-fuwhfhfw-names-AAAA-deeaois2019-03-03.and.more");
QTextLayout layout(longString, testFont);
for (int wrapMode = QTextOption::NoWrap; wrapMode <= QTextOption::WrapAtWordBoundaryOrAnywhere; ++wrapMode) {
QTextOption opt;
opt.setWrapMode((QTextOption::WrapMode)wrapMode);
layout.setTextOption(opt);
layout.beginLayout();
while (layout.createLine().isValid()) { }
layout.endLayout();
const qreal minWidth = layout.minimumWidth();
const qreal maxWidth = layout.maximumWidth();
// Try the layout from slightly wider than the widest (maxWidth)
// and narrow it down to slighly narrower than minWidth
// layout.maximumWidth() should return the same regardless
qreal width = qCeil(maxWidth/10)*10 + 10; // begin a bit wider
const qreal stepSize = 20;
while (width >= minWidth - stepSize) {
layout.beginLayout();
for (;;) {
QTextLine line = layout.createLine();
if (!line.isValid())
break;
line.setLineWidth(width);
}
layout.endLayout();
QCOMPARE(layout.minimumWidth(), minWidth);
QCOMPARE(layout.maximumWidth(), maxWidth);
width -= stepSize;
}
}
}
QTEST_MAIN(tst_QTextLayout)
#include "tst_qtextlayout.moc"