From 9b556afc5a2507077dff54c18a2b19c7df3c4575 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Tue, 16 Sep 2014 14:18:36 +0200 Subject: [PATCH] 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 --- src/gui/painting/qpainter.cpp | 25 +++--- src/gui/text/qtextengine.cpp | 1 - src/gui/text/qtextengine_p.h | 2 - src/gui/text/qtextlayout.cpp | 153 +++++++++------------------------- 4 files changed, 51 insertions(+), 130 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 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 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); + 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); - } + 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();