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 <eskil.abrahamsen-blomfeldt@qt.io>

2 files changed, 165 insertions, 26 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 &current = 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<QChar>("wordBoundary1");
+    QTest::addColumn<QChar>("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);
-
-    const int firstLineWidth = firstWord.length() * testFont.pixelSize();
-    const int secondLineWidth = secondWord.length() * testFont.pixelSize();
-    const int thirdLineWidth = thirdWord.length() * testFont.pixelSize();
+    QString secondWord("test");
+    QString thirdWord("world");
+    QString text(firstWord + wordBoundary1 + secondWord + wordBoundary2 + thirdWord);
+
+    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"