Fix menu mnemonic inside ligatures

The code to draw underlines on specific characters in widget menu could
not handle ligatures. Instead of using special code to handle this case
this patch changes the mnemonic underlines to use normal format-ranges
making the text engine deal with splitting ligatures as necessary.

Task-number: QTBUG-20960
Change-Id: I6159110eae7aa8c819af16ba4a393d758871e2e0
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@digia.com>

4 files changed, 52 insertions, 131 deletions

diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp
index 5d046caaa4..159acf40c6 100644
--- a/src/gui/painting/qpainter.cpp
+++ b/src/gui/painting/qpainter.cpp
@@ -7418,8 +7418,6 @@ void qt_format_text(const QFont &fnt, const QRectF &_r,
         tf |= Qt::TextDontPrint;
 
     uint maxUnderlines = 0;
-    int numUnderlines = 0;
-    QVarLengthArray<int, 32> underlinePositions(1);
 
     QFontMetricsF fm(fnt);
     QString text = str;
@@ -7450,10 +7448,13 @@ start_lengthVariant:
         }
     }
 
+    // no need to do extra work for underlines if we don't paint
+    if (tf & Qt::TextDontPrint)
+        maxUnderlines = 0;
+
+    QList<QTextLayout::FormatRange> underlineFormats;
     int length = offset - old_offset;
     if ((hidemnmemonic || showmnemonic) && maxUnderlines > 0) {
-        underlinePositions.resize(maxUnderlines + 1);
-
         QChar *cout = text.data() + old_offset;
         QChar *cout0 = cout;
         QChar *cin = cout;
@@ -7465,8 +7466,13 @@ start_lengthVariant:
                 --l;
                 if (!l)
                     break;
-                if (*cin != QLatin1Char('&') && !hidemnmemonic)
-                    underlinePositions[numUnderlines++] = cout - cout0;
+                if (*cin != QLatin1Char('&') && !hidemnmemonic) {
+                    QTextLayout::FormatRange range;
+                    range.start = cout - cout0;
+                    range.length = 1;
+                    range.format.setFontUnderline(true);
+                    underlineFormats.append(range);
+                }
             } else if (hidemnmemonic && *cin == QLatin1Char('(') && l >= 4 &&
                        cin[1] == QLatin1Char('&') && cin[2] != QLatin1Char('&') &&
                        cin[3] == QLatin1Char(')')) {
@@ -7486,11 +7492,6 @@ start_lengthVariant:
         }
     }
 
-    // no need to do extra work for underlines if we don't paint
-    if (tf & Qt::TextDontPrint)
-        numUnderlines = 0;
-
-    underlinePositions[numUnderlines] = -1;
     qreal height = 0;
     qreal width = 0;
 
@@ -7523,7 +7524,7 @@ start_lengthVariant:
         engine.forceJustification = true;
     QTextLayout textLayout(&engine);
     textLayout.setCacheEnabled(true);
-    textLayout.engine()->underlinePositions = underlinePositions.data();
+    textLayout.setAdditionalFormats(underlineFormats);
 
     if (finalText.isEmpty()) {
         height = fm.height();
diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp
index 2db8b3fc1b..452b263670 100644
--- a/src/gui/text/qtextengine.cpp
+++ b/src/gui/text/qtextengine.cpp
@@ -1406,7 +1406,6 @@ void QTextEngine::init(QTextEngine *e)
     e->minWidth = 0;
     e->maxWidth = 0;
 
-    e->underlinePositions = 0;
     e->specialData = 0;
     e->stackEngine = false;
 #ifndef QT_NO_RAWFONT
diff --git a/src/gui/text/qtextengine_p.h b/src/gui/text/qtextengine_p.h
index 13879912d3..8ea2d5efb9 100644
--- a/src/gui/text/qtextengine_p.h
+++ b/src/gui/text/qtextengine_p.h
@@ -563,8 +563,6 @@ public:
     uint useRawFont : 1;
 #endif
 
-    int *underlinePositions;
-
     mutable LayoutData *layoutData;
 
     ItemDecorationList underlineList;
diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp
index 9919ce6bb4..ad00396553 100644
--- a/src/gui/text/qtextlayout.cpp
+++ b/src/gui/text/qtextlayout.cpp
@@ -2021,79 +2021,6 @@ int QTextLine::textLength() const
     return eng->lines[index].length + eng->lines[index].trailingSpaces;
 }
 
-static void drawMenuText(QPainter *p, QFixed x, QFixed y, const QScriptItem &si, QTextItemInt &gf, QTextEngine *eng,
-                         int start, int glyph_start)
-{
-    int ge = glyph_start + gf.glyphs.numGlyphs;
-    int gs = glyph_start;
-    int end = start + gf.num_chars;
-    unsigned short *logClusters = eng->logClusters(&si);
-    QGlyphLayout glyphs = eng->shapedGlyphs(&si);
-    QFixed orig_width = gf.width;
-
-    int *ul = eng->underlinePositions;
-    if (ul)
-        while (*ul != -1 && *ul < start)
-            ++ul;
-    bool rtl = si.analysis.bidiLevel % 2;
-    if (rtl)
-        x += si.width;
-
-    do {
-        int gtmp = ge;
-        int stmp = end;
-        if (ul && *ul != -1 && *ul < end) {
-            stmp = *ul;
-            gtmp = logClusters[*ul-si.position];
-        }
-
-        gf.glyphs = glyphs.mid(gs, gtmp - gs);
-        gf.num_chars = stmp - start;
-        gf.chars = eng->layoutData->string.unicode() + start;
-        QFixed w = 0;
-        while (gs < gtmp) {
-            w += glyphs.effectiveAdvance(gs);
-            ++gs;
-        }
-        start = stmp;
-        gf.width = w;
-        if (rtl)
-            x -= w;
-        if (gf.num_chars)
-            QPainterPrivate::get(p)->drawTextItem(QPointF(x.toReal(), y.toReal()), gf, eng);
-        if (!rtl)
-            x += w;
-        if (ul && *ul != -1 && *ul < end) {
-            // draw underline
-            gtmp = (*ul == end-1) ? ge : logClusters[*ul+1-si.position];
-            ++stmp;
-            gf.glyphs = glyphs.mid(gs, gtmp - gs);
-            gf.num_chars = stmp - start;
-            gf.chars = eng->layoutData->string.unicode() + start;
-            gf.logClusters = logClusters + start - si.position;
-            w = 0;
-            while (gs < gtmp) {
-                w += glyphs.effectiveAdvance(gs);
-                ++gs;
-            }
-            ++start;
-            gf.width = w;
-            gf.underlineStyle = QTextCharFormat::SingleUnderline;
-            if (rtl)
-                x -= w;
-            QPainterPrivate::get(p)->drawTextItem(QPointF(x.toReal(), y.toReal()), gf, eng);
-            if (!rtl)
-                x += w;
-            gf.underlineStyle = QTextCharFormat::NoUnderline;
-            ++gf.chars;
-            ++ul;
-        }
-    } while (gs < ge);
-
-    gf.width = orig_width;
-}
-
-
 static void setPenAndDrawBackground(QPainter *p, const QPen &defaultPen, const QTextCharFormat &chf, const QRectF &r)
 {
     QBrush c = chf.foreground();
@@ -2483,52 +2410,48 @@ void QTextLine::draw(QPainter *p, const QPointF &pos, const QTextLayout::FormatR
 
         Q_ASSERT(gf.fontEngine);
 
-        if (eng->underlinePositions) {
-            // can't have selections in this case
-            drawMenuText(p, iterator.x, itemBaseLine, si, gf, eng, iterator.itemStart, iterator.glyphsStart);
-        } else {
-            QPointF pos(iterator.x.toReal(), itemBaseLine.toReal());
-            if (format.penProperty(QTextFormat::TextOutline).style() != Qt::NoPen) {
-                QPainterPath path;
-                path.setFillRule(Qt::WindingFill);
-
-                if (gf.glyphs.numGlyphs)
-                    gf.fontEngine->addOutlineToPath(pos.x(), pos.y(), gf.glyphs, &path, gf.flags);
-                if (gf.flags) {
-                    const QFontEngine *fe = gf.fontEngine;
-                    const qreal lw = fe->lineThickness().toReal();
-                    if (gf.flags & QTextItem::Underline) {
-                        qreal offs = fe->underlinePosition().toReal();
-                        path.addRect(pos.x(), pos.y() + offs, gf.width.toReal(), lw);
-                    }
-                    if (gf.flags & QTextItem::Overline) {
-                        qreal offs = fe->ascent().toReal() + 1;
-                        path.addRect(pos.x(), pos.y() - offs, gf.width.toReal(), lw);
-                    }
-                    if (gf.flags & QTextItem::StrikeOut) {
-                        qreal offs = fe->ascent().toReal() / 3;
-                        path.addRect(pos.x(), pos.y() - offs, gf.width.toReal(), lw);
-                    }
+        QPointF pos(iterator.x.toReal(), itemBaseLine.toReal());
+        if (format.penProperty(QTextFormat::TextOutline).style() != Qt::NoPen) {
+            QPainterPath path;
+            path.setFillRule(Qt::WindingFill);
+
+            if (gf.glyphs.numGlyphs)
+                gf.fontEngine->addOutlineToPath(pos.x(), pos.y(), gf.glyphs, &path, gf.flags);
+            if (gf.flags) {
+                const QFontEngine *fe = gf.fontEngine;
+                const qreal lw = fe->lineThickness().toReal();
+                if (gf.flags & QTextItem::Underline) {
+                    qreal offs = fe->underlinePosition().toReal();
+                    path.addRect(pos.x(), pos.y() + offs, gf.width.toReal(), lw);
+                }
+                if (gf.flags & QTextItem::Overline) {
+                    qreal offs = fe->ascent().toReal() + 1;
+                    path.addRect(pos.x(), pos.y() - offs, gf.width.toReal(), lw);
+                }
+                if (gf.flags & QTextItem::StrikeOut) {
+                    qreal offs = fe->ascent().toReal() / 3;
+                    path.addRect(pos.x(), pos.y() - offs, gf.width.toReal(), lw);
                 }
-
-                p->save();
-                p->setRenderHint(QPainter::Antialiasing);
-                //Currently QPen with a Qt::NoPen style still returns a default
-                //QBrush which != Qt::NoBrush so we need this specialcase to reset it
-                if (p->pen().style() == Qt::NoPen)
-                    p->setBrush(Qt::NoBrush);
-                else
-                    p->setBrush(p->pen().brush());
-
-                p->setPen(format.textOutline());
-                p->drawPath(path);
-                p->restore();
-            } else {
-                if (noText)
-                    gf.glyphs.numGlyphs = 0; // slightly less elegant than it should be
-                QPainterPrivate::get(p)->drawTextItem(pos, gf, eng);
             }
+
+            p->save();
+            p->setRenderHint(QPainter::Antialiasing);
+            //Currently QPen with a Qt::NoPen style still returns a default
+            //QBrush which != Qt::NoBrush so we need this specialcase to reset it
+            if (p->pen().style() == Qt::NoPen)
+                p->setBrush(Qt::NoBrush);
+            else
+                p->setBrush(p->pen().brush());
+
+            p->setPen(format.textOutline());
+            p->drawPath(path);
+            p->restore();
+        } else {
+            if (noText)
+                gf.glyphs.numGlyphs = 0; // slightly less elegant than it should be
+            QPainterPrivate::get(p)->drawTextItem(pos, gf, eng);
         }
+
         if (si.analysis.flags == QScriptAnalysis::Space
             && (eng->option.flags() & QTextOption::ShowTabsAndSpaces)) {
             QBrush c = format.foreground();