Line up underlines if fallback fonts are used (QTBUG-21832)

Change-Id: Icecc514f6c47c0576af8cabd39cdc0987f8d93fa
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@nokia.com>

6 files changed, 200 insertions, 44 deletions

diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp
index 973dabc4cb..eafbe87b31 100644
--- a/src/gui/painting/qpainter.cpp
+++ b/src/gui/painting/qpainter.cpp
@@ -90,7 +90,7 @@ void qt_format_text(const QFont &font,
                     const QRectF &_r, int tf, const QTextOption *option, const QString& str, QRectF *brect,
                     int tabstops, int* tabarray, int tabarraylen,
                     QPainter *painter);
-static void drawTextItemDecoration(QPainter *painter, const QPointF &pos, const QFontEngine *fe,
+static void drawTextItemDecoration(QPainter *painter, const QPointF &pos, const QFontEngine *fe, QTextEngine *textEngine,
                                    QTextCharFormat::UnderlineStyle underlineStyle,
                                    QTextItem::RenderFlags flags, qreal width,
                                    const QTextCharFormat &charFormat);
@@ -5641,6 +5641,7 @@ void QPainterPrivate::drawGlyphs(const quint32 *glyphArray, QFixedPoint *positio
 
     drawTextItemDecoration(q, QPointF(leftMost.toReal(), baseLine.toReal()),
                            fontEngine,
+                           0, // textEngine
                            (underline
                               ? QTextCharFormat::SingleUnderline
                               : QTextCharFormat::NoUnderline),
@@ -6177,7 +6178,7 @@ static QPixmap generateWavyPixmap(qreal maxRadius, const QPen &pen)
     return pixmap;
 }
 
-static void drawTextItemDecoration(QPainter *painter, const QPointF &pos, const QFontEngine *fe,
+static void drawTextItemDecoration(QPainter *painter, const QPointF &pos, const QFontEngine *fe, QTextEngine *textEngine,
                                    QTextCharFormat::UnderlineStyle underlineStyle,
                                    QTextItem::RenderFlags flags, qreal width,
                                    const QTextCharFormat &charFormat)
@@ -6222,15 +6223,17 @@ static void drawTextItemDecoration(QPainter *painter, const QPointF &pos, const
         painter->fillRect(pos.x(), 0, qCeil(width), qMin(wave.height(), descent), wave);
         painter->restore();
     } else if (underlineStyle != QTextCharFormat::NoUnderline) {
-        QLineF underLine(line.x1(), underlinePos, line.x2(), underlinePos);
-
         QColor uc = charFormat.underlineColor();
         if (uc.isValid())
             pen.setColor(uc);
 
         pen.setStyle((Qt::PenStyle)(underlineStyle));
         painter->setPen(pen);
-        painter->drawLine(underLine);
+        QLineF underline(line.x1(), underlinePos, line.x2(), underlinePos);
+        if (textEngine)
+            textEngine->addUnderline(painter, underline);
+        else
+            painter->drawLine(underline);
     }
 
     pen.setStyle(Qt::SolidLine);
@@ -6240,14 +6243,20 @@ static void drawTextItemDecoration(QPainter *painter, const QPointF &pos, const
         QLineF strikeOutLine = line;
         strikeOutLine.translate(0., - fe->ascent().toReal() / 3.);
         painter->setPen(pen);
-        painter->drawLine(strikeOutLine);
+        if (textEngine)
+            textEngine->addStrikeOut(painter, strikeOutLine);
+        else
+            painter->drawLine(strikeOutLine);
     }
 
     if (flags & QTextItem::Overline) {
-        QLineF overLine = line;
-        overLine.translate(0., - fe->ascent().toReal());
+        QLineF overline = line;
+        overline.translate(0., - fe->ascent().toReal());
         painter->setPen(pen);
-        painter->drawLine(overLine);
+        if (textEngine)
+            textEngine->addOverline(painter, overline);
+        else
+            painter->drawLine(overline);
     }
 
     painter->setPen(oldPen);
@@ -6272,7 +6281,7 @@ Q_GUI_EXPORT void qt_draw_decoration_for_glyphs(QPainter *painter, const glyph_t
 
         // We don't support glyphs that do not share a common baseline. If this turns out to
         // be a relevant use case, then we need to find clusters of glyphs that share a baseline
-        // and do a drawTextItemDecorations call per cluster.
+        // and do a drawTextItemDecoration call per cluster.
         if (i == 0 || baseLine < positions[i].y)
             baseLine = positions[i].y;
 
@@ -6293,12 +6302,20 @@ Q_GUI_EXPORT void qt_draw_decoration_for_glyphs(QPainter *painter, const glyph_t
 
     drawTextItemDecoration(painter, QPointF(leftMost.toReal(), baseLine.toReal()),
                            fontEngine,
+                           0, // textEngine
                            font.underline() ? QTextCharFormat::SingleUnderline
                                             : QTextCharFormat::NoUnderline, flags,
                            width.toReal(), charFormat);
 }
 
-void QPainter::drawTextItem(const QPointF &p, const QTextItem &_ti)
+void QPainter::drawTextItem(const QPointF &p, const QTextItem &ti)
+{
+    Q_D(QPainter);
+
+    d->drawTextItem(p, ti, static_cast<QTextEngine *>(0));
+}
+
+void QPainterPrivate::drawTextItem(const QPointF &p, const QTextItem &_ti, QTextEngine *textEngine)
 {
 #ifdef QT_DEBUG_DRAW
     if (qt_show_painter_debug_output)
@@ -6306,35 +6323,35 @@ void QPainter::drawTextItem(const QPointF &p, const QTextItem &_ti)
                p.x(), p.y(), qPrintable(_ti.text()));
 #endif
 
-    Q_D(QPainter);
+    Q_Q(QPainter);
 
-    if (!d->engine)
+    if (!engine)
         return;
 
 #ifndef QT_NO_DEBUG
-    qt_painter_thread_test(d->device->devType(),
+    qt_painter_thread_test(device->devType(),
                            "text and fonts",
                            QFontDatabase::supportsThreadedFontRendering());
 #endif
 
     QTextItemInt &ti = const_cast<QTextItemInt &>(static_cast<const QTextItemInt &>(_ti));
 
-    if (!d->extended && d->state->bgMode == Qt::OpaqueMode) {
+    if (!extended && state->bgMode == Qt::OpaqueMode) {
         QRectF rect(p.x(), p.y() - ti.ascent.toReal(), ti.width.toReal(), (ti.ascent + ti.descent + 1).toReal());
-        fillRect(rect, d->state->bgBrush);
+        q->fillRect(rect, state->bgBrush);
     }
 
-    if (pen().style() == Qt::NoPen)
+    if (q->pen().style() == Qt::NoPen)
         return;
 
-    const RenderHints oldRenderHints = d->state->renderHints;
-    if (!d->state->renderHints & QPainter::Antialiasing && d->state->matrix.type() >= QTransform::TxScale) {
+    const QPainter::RenderHints oldRenderHints = state->renderHints;
+    if (!state->renderHints & QPainter::Antialiasing && state->matrix.type() >= QTransform::TxScale) {
         // draw antialias decoration (underline/overline/strikeout) with
         // transformed text
 
         bool aa = true;
-        const QTransform &m = d->state->matrix;
-        if (d->state->matrix.type() < QTransform::TxShear) {
+        const QTransform &m = state->matrix;
+        if (state->matrix.type() < QTransform::TxShear) {
             bool isPlain90DegreeRotation =
                 (qFuzzyIsNull(m.m11())
                  && qFuzzyIsNull(m.m12() - qreal(1))
@@ -6357,11 +6374,11 @@ void QPainter::drawTextItem(const QPointF &p, const QTextItem &_ti)
             aa = !isPlain90DegreeRotation;
         }
         if (aa)
-            setRenderHint(QPainter::Antialiasing, true);
+            q->setRenderHint(QPainter::Antialiasing, true);
     }
 
-    if (!d->extended)
-        d->updateState(d->state);
+    if (!extended)
+        updateState(state);
 
     if (!ti.glyphs.numGlyphs) {
         // nothing to do
@@ -6397,7 +6414,7 @@ void QPainter::drawTextItem(const QPointF &p, const QTextItem &_ti)
             if (rtl)
                 x -= ti2.width.toReal();
 
-            d->engine->drawTextItem(QPointF(x, y), ti2);
+            engine->drawTextItem(QPointF(x, y), ti2);
 
             if (!rtl)
                 x += ti2.width.toReal();
@@ -6424,10 +6441,10 @@ void QPainter::drawTextItem(const QPointF &p, const QTextItem &_ti)
         if (rtl)
             x -= ti2.width.toReal();
 
-        if (d->extended)
-            d->extended->drawTextItem(QPointF(x, y), ti2);
+        if (extended)
+            extended->drawTextItem(QPointF(x, y), ti2);
         else
-            d->engine->drawTextItem(QPointF(x,y), ti2);
+            engine->drawTextItem(QPointF(x,y), ti2);
 
         // reset the high byte for all glyphs
         const int hi = which << 24;
@@ -6435,20 +6452,20 @@ void QPainter::drawTextItem(const QPointF &p, const QTextItem &_ti)
             glyphs.glyphs[i] = hi | glyphs.glyphs[i];
 
     } else {
-        if (d->extended)
-            d->extended->drawTextItem(p, ti);
+        if (extended)
+            extended->drawTextItem(p, ti);
         else
-            d->engine->drawTextItem(p, ti);
+            engine->drawTextItem(p, ti);
     }
-    drawTextItemDecoration(this, p, ti.fontEngine, ti.underlineStyle, ti.flags, ti.width.toReal(),
-                           ti.charFormat);
+    drawTextItemDecoration(q, p, ti.fontEngine, textEngine, ti.underlineStyle,
+                           ti.flags, ti.width.toReal(), ti.charFormat);
 
-    if (d->state->renderHints != oldRenderHints) {
-        d->state->renderHints = oldRenderHints;
-        if (d->extended)
-            d->extended->renderHintsChanged();
+    if (state->renderHints != oldRenderHints) {
+        state->renderHints = oldRenderHints;
+        if (extended)
+            extended->renderHintsChanged();
         else
-            d->state->dirtyFlags |= QPaintEngine::DirtyHints;
+            state->dirtyFlags |= QPaintEngine::DirtyHints;
     }
 }
 
@@ -7545,11 +7562,12 @@ start_lengthVariant:
 
         for (int i = 0; i < textLayout.lineCount(); i++) {
             QTextLine line = textLayout.lineAt(i);
+            QTextEngine *eng = textLayout.engine();
+            eng->enableDelayDecorations();
 
             qreal advance = line.horizontalAdvance();
             xoff = 0;
             if (tf & Qt::AlignRight) {
-                QTextEngine *eng = textLayout.engine();
                 xoff = r.width() - advance -
                     eng->leadingSpaceWidth(eng->lines[line.lineNumber()]).toReal();
             }
@@ -7557,6 +7575,7 @@ start_lengthVariant:
                 xoff = (r.width() - advance) / 2;
 
             line.draw(painter, QPointF(r.x() + xoff, r.y() + yoff));
+            eng->drawDecorations(painter);
         }
 
         if (restore) {
diff --git a/src/gui/painting/qpainter.h b/src/gui/painting/qpainter.h
index 67b05eeca5..97c10a2764 100644
--- a/src/gui/painting/qpainter.h
+++ b/src/gui/painting/qpainter.h
@@ -74,6 +74,7 @@ class QPainterPrivate;
 class QPen;
 class QPolygon;
 class QTextItem;
+class QTextEngine;
 class QMatrix;
 class QTransform;
 class QStaticText;
@@ -487,6 +488,7 @@ private:
     friend class QRasterPaintEngine;
     friend class QAlphaPaintEngine;
     friend class QPreviewPaintEngine;
+    friend class QTextEngine;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(QPainter::RenderHints)
diff --git a/src/gui/painting/qpainter_p.h b/src/gui/painting/qpainter_p.h
index fecf8bd960..3327860ac9 100644
--- a/src/gui/painting/qpainter_p.h
+++ b/src/gui/painting/qpainter_p.h
@@ -230,6 +230,7 @@ public:
     void draw_helper(const QPainterPath &path, DrawOperation operation = StrokeAndFillDraw);
     void drawStretchedGradient(const QPainterPath &path, DrawOperation operation);
     void drawOpaqueBackground(const QPainterPath &path, DrawOperation operation);
+    void drawTextItem(const QPointF &p, const QTextItem &_ti, QTextEngine *textEngine);
 
 #if !defined(QT_NO_RAWFONT)
     void drawGlyphs(const quint32 *glyphArray, QFixedPoint *positionArray, int glyphCount,
diff --git a/src/gui/text/qtextengine.cpp b/src/gui/text/qtextengine.cpp
index dae02def07..3d58d91169 100644
--- a/src/gui/text/qtextengine.cpp
+++ b/src/gui/text/qtextengine.cpp
@@ -1174,6 +1174,7 @@ static void init(QTextEngine *e)
     e->cacheGlyphs = false;
     e->forceJustification = false;
     e->visualMovement = false;
+    e->delayDecorations = false;
 
     e->layoutData = 0;
 
@@ -2913,6 +2914,104 @@ int QTextEngine::positionAfterVisualMovement(int pos, QTextCursor::MoveOperation
     return pos;
 }
 
+void QTextEngine::addItemDecoration(QPainter *painter, const QLineF &line, ItemDecorationList *decorationList)
+{
+    if (delayDecorations) {
+        decorationList->append(ItemDecoration(line.x1(), line.x2(), line.y1(), painter->pen()));
+    } else {
+        painter->drawLine(line);
+    }
+}
+
+void QTextEngine::addUnderline(QPainter *painter, const QLineF &line)
+{
+    // qDebug() << "Adding underline:" << line;
+    addItemDecoration(painter, line, &underlineList);
+}
+
+void QTextEngine::addStrikeOut(QPainter *painter, const QLineF &line)
+{
+    addItemDecoration(painter, line, &strikeOutList);
+}
+
+void QTextEngine::addOverline(QPainter *painter, const QLineF &line)
+{
+    addItemDecoration(painter, line, &overlineList);
+}
+
+void QTextEngine::drawItemDecorationList(QPainter *painter, const ItemDecorationList &decorationList)
+{
+    // qDebug() << "Drawing" << decorationList.size() << "decorations";
+    if (decorationList.isEmpty())
+        return;
+
+    foreach (const ItemDecoration decoration, decorationList) {
+        painter->setPen(decoration.pen);
+        QLineF line(decoration.x1, decoration.y, decoration.x2, decoration.y);
+        painter->drawLine(line);
+    }
+}
+
+void QTextEngine::drawDecorations(QPainter *painter)
+{
+    QPen oldPen = painter->pen();
+
+    adjustUnderlines();
+    drawItemDecorationList(painter, underlineList);
+    drawItemDecorationList(painter, strikeOutList);
+    drawItemDecorationList(painter, overlineList);
+
+    painter->setPen(oldPen);
+    clearDecorations();
+}
+
+void QTextEngine::clearDecorations()
+{
+    underlineList.clear();
+    strikeOutList.clear();
+    overlineList.clear();
+}
+
+void QTextEngine::adjustUnderlines()
+{
+    // qDebug() << __PRETTY_FUNCTION__ << underlineList.count() << "underlines";
+    if (underlineList.isEmpty())
+        return;
+
+    ItemDecorationList::iterator start = underlineList.begin();
+    ItemDecorationList::iterator end   = underlineList.end();
+    ItemDecorationList::iterator it = start;
+    qreal underlinePos = start->y;
+    qreal penWidth = start->pen.widthF();
+    qreal lastLineEnd = start->x1;
+
+    while (it != end) {
+        if (qFuzzyCompare(lastLineEnd, it->x1)) { // no gap between underlines
+            underlinePos = qMax(underlinePos, it->y);
+            penWidth = qMax(penWidth, it->pen.widthF());
+        } else { // gap between this and the last underline
+            adjustUnderlines(start, it, underlinePos, penWidth);
+            start = it;
+            underlinePos = start->y;
+            penWidth = start->pen.widthF();
+        }
+        lastLineEnd = it->x2;
+        ++it;
+    }
+
+    adjustUnderlines(start, end, underlinePos, penWidth);
+}
+
+void QTextEngine::adjustUnderlines(ItemDecorationList::iterator start,
+                                   ItemDecorationList::iterator end,
+                                   qreal underlinePos, qreal penWidth)
+{
+    for (ItemDecorationList::iterator it = start; it != end; ++it) {
+        it->y = underlinePos;
+        it->pen.setWidth(penWidth);
+    }
+}
+
 QStackTextEngine::QStackTextEngine(const QString &string, const QFont &f)
     : QTextEngine(string, f),
       _layoutData(string, _memory, MemSize)
diff --git a/src/gui/text/qtextengine_p.h b/src/gui/text/qtextengine_p.h
index 6f1fd713f1..03581eb6a2 100644
--- a/src/gui/text/qtextengine_p.h
+++ b/src/gui/text/qtextengine_p.h
@@ -446,6 +446,18 @@ public:
         bool reallocate(int totalGlyphs);
     };
 
+    struct ItemDecoration {
+        ItemDecoration(qreal x1, qreal x2, qreal y, const QPen &pen):
+            x1(x1), x2(x2), y(y), pen(pen) {}
+
+        qreal x1;
+        qreal x2;
+        qreal y;
+        QPen pen;
+    };
+
+    typedef QList<ItemDecoration> ItemDecorationList;
+
     QTextEngine(LayoutData *data);
     QTextEngine();
     QTextEngine(const QString &str, const QFont &f);
@@ -597,6 +609,7 @@ public:
     uint stackEngine : 1;
     uint forceJustification : 1;
     uint visualMovement : 1;
+    uint delayDecorations: 1;
 #ifndef QT_NO_RAWFONT
     uint useRawFont : 1;
 #endif
@@ -605,6 +618,10 @@ public:
 
     mutable LayoutData *layoutData;
 
+    ItemDecorationList underlineList;
+    ItemDecorationList strikeOutList;
+    ItemDecorationList overlineList;
+
     inline bool hasFormats() const { return (block.docHandle() || specialData); }
     inline bool visualCursorMovement() const
     {
@@ -639,7 +656,22 @@ public:
     void insertionPointsForLine(int lineNum, QVector<int> &insertionPoints);
     void resetFontEngineCache();
 
+    void enableDelayDecorations(bool enable = true) { delayDecorations = enable; }
+
+    void addUnderline(QPainter *painter, const QLineF &line);
+    void addStrikeOut(QPainter *painter, const QLineF &line);
+    void addOverline(QPainter *painter, const QLineF &line);
+
+    void drawDecorations(QPainter *painter);
+    void clearDecorations();
+    void adjustUnderlines();
+
 private:
+    void addItemDecoration(QPainter *painter, const QLineF &line, ItemDecorationList *decorationList);
+    void adjustUnderlines(ItemDecorationList::iterator start,
+                          ItemDecorationList::iterator end,
+                          qreal underlinePos, qreal penWidth);
+    void drawItemDecorationList(QPainter *painter, const ItemDecorationList &decorationList);
     void setBoundary(int strPos) const;
     void addRequiredBoundaries() const;
     void shapeText(int item) const;
diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp
index d5b05a8957..56098b0bdb 100644
--- a/src/gui/text/qtextlayout.cpp
+++ b/src/gui/text/qtextlayout.cpp
@@ -60,6 +60,7 @@
 #include <qdebug.h>
 
 #include "qfontengine_p.h"
+#include <private/qpainter_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -2062,7 +2063,7 @@ static void drawMenuText(QPainter *p, QFixed x, QFixed y, const QScriptItem &si,
         if (rtl)
             x -= w;
         if (gf.num_chars)
-            p->drawTextItem(QPointF(x.toReal(), y.toReal()), gf);
+            QPainterPrivate::get(p)->drawTextItem(QPointF(x.toReal(), y.toReal()), gf, eng);
         if (!rtl)
             x += w;
         if (ul && *ul != -1 && *ul < end) {
@@ -2083,7 +2084,7 @@ static void drawMenuText(QPainter *p, QFixed x, QFixed y, const QScriptItem &si,
             gf.underlineStyle = QTextCharFormat::SingleUnderline;
             if (rtl)
                 x -= w;
-            p->drawTextItem(QPointF(x.toReal(), y.toReal()), gf);
+            QPainterPrivate::get(p)->drawTextItem(QPointF(x.toReal(), y.toReal()), gf, eng);
             if (!rtl)
                 x += w;
             gf.underlineStyle = QTextCharFormat::NoUnderline;
@@ -2379,6 +2380,8 @@ void QTextLine::draw(QPainter *p, const QPointF &pos, const QTextLayout::FormatR
 
     QTextLineItemIterator iterator(eng, index, pos, selection);
     QFixed lineBase = line.base();
+    eng->clearDecorations();
+    eng->enableDelayDecorations();
 
     const QFixed y = QFixed::fromReal(pos.y()) + line.y + lineBase;
 
@@ -2451,7 +2454,7 @@ void QTextLine::draw(QPainter *p, const QPointF &pos, const QTextLayout::FormatR
                     gf.chars = 0;
                     gf.num_chars = 0;
                     gf.width = iterator.itemWidth;
-                    p->drawTextItem(QPointF(iterator.x.toReal(), y.toReal()), gf);
+                    QPainterPrivate::get(p)->drawTextItem(QPointF(iterator.x.toReal(), y.toReal()), gf, eng);
                     if (eng->option.flags() & QTextOption::ShowTabsAndSpaces) {
                         QChar visualTab(0x2192);
                         int w = QFontMetrics(f).width(visualTab);
@@ -2529,7 +2532,7 @@ void QTextLine::draw(QPainter *p, const QPointF &pos, const QTextLayout::FormatR
             } else {
                 if (noText)
                     gf.glyphs.numGlyphs = 0; // slightly less elegant than it should be
-                p->drawTextItem(pos, gf);
+                QPainterPrivate::get(p)->drawTextItem(pos, gf, eng);
             }
         }
         if (si.analysis.flags == QScriptAnalysis::Space
@@ -2542,7 +2545,7 @@ void QTextLine::draw(QPainter *p, const QPointF &pos, const QTextLayout::FormatR
             p->setPen(pen);
         }
     }
-
+    eng->drawDecorations(p);
 
     if (eng->hasFormats())
         p->setPen(pen);