From bd3c82f8db4391fc1d6d3338827356143fd598dd Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 24 Feb 2020 11:07:29 +0100 Subject: [PATCH] Fix non-trivial soft-hyphen line breaks The effect of the soft-hyphen needs to be updated once the final the break point has been found. This change cleans the logic by using two variables keeping track of soft-hyphen at current evaluated position and at last confirmed break point. Also adds tests for supression of soft-hyphens in the tight WrapAnywhere case. Fixes: QTBUG-35940 Fixes: QTBUG-44257 Change-Id: I7a89a8ef991b87691879bb7ce40cec4a3605fdd5 Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/text/qtextlayout.cpp | 29 ++-- .../gui/text/qtextlayout/tst_qtextlayout.cpp | 160 ++++++++++++++++-- 2 files changed, 164 insertions(+), 25 deletions(-) diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index a3e194f835..aaca1061b7 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -1667,7 +1667,8 @@ namespace { QFontEngine *previousGlyphFontEngine; QFixed minw; - QFixed softHyphenWidth; + QFixed currentSoftHyphenWidth; + QFixed commitedSoftHyphenWidth; QFixed rightBearing; QFixed minimumRightBearing; @@ -1681,7 +1682,7 @@ namespace { QFixed calculateNewWidth(const QScriptLine &line) const { return line.textWidth + tmpData.textWidth + spaceData.textWidth - + softHyphenWidth + negativeRightBearing(); + + (line.textWidth > 0 ? currentSoftHyphenWidth : QFixed()) + negativeRightBearing(); } inline glyph_t currentGlyph() const @@ -1755,6 +1756,7 @@ inline bool LineBreakHelper::checkFullOtherwiseExtend(QScriptLine &line) if (line.length && !manualWrap && (newWidth > line.width || glyphCount > maxGlyphs)) return true; + const QFixed oldTextWidth = line.textWidth; minw = qMax(minw, tmpData.textWidth); line += tmpData; line.textWidth += spaceData.textWidth; @@ -1765,6 +1767,11 @@ inline bool LineBreakHelper::checkFullOtherwiseExtend(QScriptLine &line) spaceData.textWidth = 0; spaceData.length = 0; + if (oldTextWidth != line.textWidth || currentSoftHyphenWidth > 0) { + commitedSoftHyphenWidth = currentSoftHyphenWidth; + currentSoftHyphenWidth = 0; + } + return false; } @@ -1837,7 +1844,6 @@ void QTextLine::layout_helper(int maxGlyphs) while (newItem < eng->layoutData->items.size()) { lbh.resetRightBearing(); - lbh.softHyphenWidth = 0; if (newItem != item) { item = newItem; const QScriptItem ¤t = eng->layoutData->items.at(item); @@ -1975,9 +1981,9 @@ void QTextLine::layout_helper(int maxGlyphs) } while (lbh.currentPosition < end); lbh.minw = qMax(lbh.tmpData.textWidth, lbh.minw); - if (lbh.currentPosition > 0 && lbh.currentPosition < end - && attributes[lbh.currentPosition].lineBreak - && eng->layoutData->string.at(lbh.currentPosition - 1).unicode() == QChar::SoftHyphen) { + if (lbh.currentPosition > 0 && lbh.currentPosition <= end + && (lbh.currentPosition == end || attributes[lbh.currentPosition].lineBreak) + && eng->layoutData->string.at(lbh.currentPosition - 1) == QChar::SoftHyphen) { // if we are splitting up a word because of // a soft hyphen then we ... // @@ -1994,10 +2000,7 @@ void QTextLine::layout_helper(int maxGlyphs) // want the soft-hyphen to slip into the next line // and thus become invisible again. // - if (line.length) - lbh.softHyphenWidth = lbh.glyphs.advances[lbh.logClusters[lbh.currentPosition - 1]]; - else if (breakany) - lbh.tmpData.textWidth += lbh.glyphs.advances[lbh.logClusters[lbh.currentPosition - 1]]; + lbh.currentSoftHyphenWidth = lbh.glyphs.advances[lbh.logClusters[lbh.currentPosition - 1]]; } if (sb_or_ws|breakany) { @@ -2023,6 +2026,7 @@ void QTextLine::layout_helper(int maxGlyphs) lbh.calculateRightBearing(); if (lbh.checkFullOtherwiseExtend(line)) { + // We are too wide to accept the next glyph with its bearing, so we restore the // right bearing to that of the previous glyph (the one that was already accepted), // so that the bearing can be be applied to the final width of the text below. @@ -2031,9 +2035,7 @@ void QTextLine::layout_helper(int maxGlyphs) else lbh.calculateRightBearingForPreviousGlyph(); - if (!breakany) { - line.textWidth += lbh.softHyphenWidth; - } + line.textWidth += lbh.commitedSoftHyphenWidth; goto found; } @@ -2045,6 +2047,7 @@ void QTextLine::layout_helper(int maxGlyphs) } LB_DEBUG("reached end of line"); lbh.checkFullOtherwiseExtend(line); + line.textWidth += lbh.commitedSoftHyphenWidth; found: line.textAdvance = line.textWidth; diff --git a/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp b/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp index 2dcca0209e..8466305832 100644 --- a/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp +++ b/tests/auto/gui/text/qtextlayout/tst_qtextlayout.cpp @@ -62,6 +62,7 @@ private slots: void lineBreaking(); #ifdef QT_BUILD_INTERNAL void simpleBoundingRect(); + void threeLineBoundingRect_data(); void threeLineBoundingRect(); void boundingRectWithLongLineAndNoWrap(); void forcedBreaks(); @@ -140,6 +141,7 @@ private slots: void showLineAndParagraphSeparatorsCrash(); void koreanWordWrap(); void tooManyDirectionalCharctersCrash_qtbug77819(); + void softHyphens(); private: QFont testFont; @@ -315,18 +317,49 @@ void tst_QTextLayout::simpleBoundingRect() QCOMPARE(layout.boundingRect(), QRectF(0, 0, width, QFontMetrics(testFont).height())); } +void tst_QTextLayout::threeLineBoundingRect_data() +{ + QTest::addColumn("wordBoundary1"); + QTest::addColumn("wordBoundary2"); + QTest::newRow("2x' '") << QChar(' ') << QChar(' '); + QTest::newRow("2x'\\n'") << QChar('\n') << QChar('\n'); + QTest::newRow("' ' + '\\n'") << QChar(' ') << QChar('\n'); + QTest::newRow("'\\n' + ' '") << QChar('\n') << QChar(' '); + QTest::newRow("2x'\\t'") << QChar('\t') << QChar('\t'); + QTest::newRow("2xsoft hyphen") << QChar(0xad) << QChar(0xad); + QTest::newRow("2x'-'") << QChar('-') << QChar('-'); + QTest::newRow("2x'/'") << QChar('/') << QChar('/'); + QTest::newRow("soft hyphen + ' '") << QChar(0xad) << QChar(' '); + QTest::newRow("soft hyphen + '\\n'") << QChar(0xad) << QChar('\n'); + QTest::newRow("soft hyphen + '-'") << QChar(0xad) << QChar('-'); + QTest::newRow("' ' + soft hyphen") << QChar(' ') << QChar(0xad); + QTest::newRow("'\\n' + soft hyphen") << QChar('\n') << QChar(0xad); + QTest::newRow("'-' + soft hyphen") << QChar('-') << QChar(0xad); +} + void tst_QTextLayout::threeLineBoundingRect() { /* stricter check. break text into three lines */ + QFETCH(QChar, wordBoundary1); + QFETCH(QChar, wordBoundary2); QString firstWord("hello"); - QString secondWord("world"); - QString thirdWord("test"); - QString text(firstWord + ' ' + secondWord + ' ' + thirdWord); + QString secondWord("test"); + QString thirdWord("world"); + QString text(firstWord + wordBoundary1 + secondWord + wordBoundary2 + thirdWord); - const int firstLineWidth = firstWord.length() * testFont.pixelSize(); - const int secondLineWidth = secondWord.length() * testFont.pixelSize(); - const int thirdLineWidth = thirdWord.length() * testFont.pixelSize(); + int firstLineWidth = firstWord.length() * testFont.pixelSize(); + int secondLineWidth = secondWord.length() * testFont.pixelSize(); + int thirdLineWidth = thirdWord.length() * testFont.pixelSize(); + // Trailing spaces do not count to line width: + if (!wordBoundary1.isSpace()) + firstLineWidth += testFont.pixelSize(); + if (!wordBoundary2.isSpace()) + secondLineWidth += testFont.pixelSize(); + // But trailing spaces do count to line length: + const int firstLineLength = firstWord.length() + 1; + const int secondLineLength = secondWord.length() + 1; + const int thirdLineLength = thirdWord.length(); const int longestLine = qMax(firstLineWidth, qMax(secondLineWidth, thirdLineWidth)); @@ -339,8 +372,7 @@ void tst_QTextLayout::threeLineBoundingRect() line.setLineWidth(firstLineWidth); line.setPosition(QPoint(0, y)); QCOMPARE(line.textStart(), pos); - // + 1 for trailing space - QCOMPARE(line.textLength(), firstWord.length() + 1); + QCOMPARE(line.textLength(), firstLineLength); QCOMPARE(qRound(line.naturalTextWidth()), firstLineWidth); pos += line.textLength(); @@ -349,9 +381,8 @@ void tst_QTextLayout::threeLineBoundingRect() line = layout.createLine(); line.setLineWidth(secondLineWidth); line.setPosition(QPoint(0, y)); - // + 1 for trailing space QCOMPARE(line.textStart(), pos); - QCOMPARE(line.textLength(), secondWord.length() + 1); + QCOMPARE(line.textLength(), secondLineLength); QCOMPARE(qRound(line.naturalTextWidth()), secondLineWidth); pos += line.textLength(); @@ -360,9 +391,8 @@ void tst_QTextLayout::threeLineBoundingRect() line = layout.createLine(); line.setLineWidth(secondLineWidth); line.setPosition(QPoint(0, y)); - // no trailing space here! QCOMPARE(line.textStart(), pos); - QCOMPARE(line.textLength(), thirdWord.length()); + QCOMPARE(line.textLength(), thirdLineLength); QCOMPARE(qRound(line.naturalTextWidth()), thirdLineWidth); y += qRound(line.ascent() + line.descent()); @@ -2352,5 +2382,111 @@ void tst_QTextLayout::tooManyDirectionalCharctersCrash_qtbug77819() tl.endLayout(); } +void tst_QTextLayout::softHyphens() +{ + QString text = QStringLiteral("xxxx\u00ad") + QStringLiteral("xxxx\u00ad"); + + QFont font; + font.setPixelSize(14); + font.setHintingPreference(QFont::PreferNoHinting); + const float xAdvance = QFontMetricsF(font).horizontalAdvance(QChar('x')); + const float shyAdvance = QFontMetricsF(font).horizontalAdvance(QChar::SoftHyphen); + if (xAdvance < (shyAdvance + 1.0f)) + QSKIP("Default font not suitable for this test."); + QTextLayout layout(text, font); + QTextOption option; + option.setWrapMode(QTextOption::WrapAtWordBoundaryOrAnywhere); + layout.setTextOption(option); + + // Loose fit + // xxxx- | + // xxxx- | + { + int pos = 0; + int y = 0; + layout.beginLayout(); + QTextLine line = layout.createLine(); + line.setLineWidth(qCeil(5 * xAdvance) + 1); + line.setPosition(QPoint(0, y)); + QCOMPARE(line.textStart(), pos); + QCOMPARE(line.textLength(), 5); + QVERIFY(qAbs(line.naturalTextWidth() - (4 * xAdvance + shyAdvance)) <= 1); + + pos += line.textLength(); + y += qRound(line.ascent() + line.descent()); + + line = layout.createLine(); + line.setLineWidth(qCeil(5 * xAdvance) + 1); + line.setPosition(QPoint(0, y)); + QCOMPARE(line.textStart(), pos); + QCOMPARE(line.textLength(), 5); + QVERIFY(qAbs(line.naturalTextWidth() - (4 * xAdvance + shyAdvance)) <= 1); + layout.endLayout(); + } + + // Tight fit + // xxxx-| + // xxxx-| + { + int pos = 0; + int y = 0; + layout.beginLayout(); + QTextLine line = layout.createLine(); + line.setLineWidth(qCeil(4 * xAdvance + shyAdvance) + 1); + line.setPosition(QPoint(0, y)); + QCOMPARE(line.textStart(), pos); + QCOMPARE(line.textLength(), 5); + QVERIFY(qAbs(line.naturalTextWidth() - (4 * xAdvance + shyAdvance)) <= 1); + + pos += line.textLength(); + y += qRound(line.ascent() + line.descent()); + + line = layout.createLine(); + line.setLineWidth(qCeil(4 * xAdvance + shyAdvance) + 1); + line.setPosition(QPoint(0, y)); + QCOMPARE(line.textStart(), pos); + QCOMPARE(line.textLength(), 5); + QVERIFY(qAbs(line.naturalTextWidth() - (4 * xAdvance + shyAdvance)) <= 1); + layout.endLayout(); + } + + // Very tight fit + // xxxx| + // xxxx| + // - | + { + int pos = 0; + int y = 0; + layout.beginLayout(); + QTextLine line = layout.createLine(); + line.setLineWidth(qCeil(4 * xAdvance) + 2); + line.setPosition(QPoint(0, y)); + QCOMPARE(line.textStart(), pos); + QCOMPARE(line.textLength(), 4); + QVERIFY(qAbs(line.naturalTextWidth() - 4 * xAdvance) <= 1); + + pos += line.textLength(); + y += qRound(line.ascent() + line.descent()); + + line = layout.createLine(); + line.setLineWidth(qCeil(4 * xAdvance) + 2); + line.setPosition(QPoint(0, y)); + QCOMPARE(line.textStart(), pos); + QCOMPARE(line.textLength(), 5); + QVERIFY(qAbs(line.naturalTextWidth() - 4 * xAdvance) <= 1); + + pos += line.textLength(); + y += qRound(line.ascent() + line.descent()); + + line = layout.createLine(); + line.setLineWidth(qCeil(4 * xAdvance) + 2); + line.setPosition(QPoint(0, y)); + QCOMPARE(line.textStart(), pos); + QCOMPARE(line.textLength(), 1); + QVERIFY(qAbs(line.naturalTextWidth() - shyAdvance) <= 1); + layout.endLayout(); + } +} + QTEST_MAIN(tst_QTextLayout) #include "tst_qtextlayout.moc"