From 55b2011bcd3cfd93baf9db1a03a6672220c26413 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Tue, 23 Jan 2024 10:50:43 +0100 Subject: [PATCH] Fix clipped text when combining multiple writing systems When a QTextLine consists of multiple different scripts and the fonts had negative bearing, the background for a script item could overdraw the previous item's text, causing it to look clipped. This was because the background and text was drawn in a single pass, and moving the background drawing into its own pre-pass fixes the issue. [ChangeLog][QtGui] Fixed an issue where drawing text from different writing systems in the same line and including a background could cause parts of the text to be clipped. Pick-to: 6.6 6.5 Fixes: QTBUG-121040 Change-Id: I3f79e6d33c09a2a92853bc8752dbe11a0bea2dd0 Reviewed-by: Lars Knoll (cherry picked from commit 8be3c9f4867ce7982387b075739b8f55c5c45753) Reviewed-by: Qt Cherry-pick Bot --- src/gui/text/qtextlayout.cpp | 368 ++++++++++++---------- tests/baseline/text/tst_baseline_text.cpp | 21 ++ 2 files changed, 216 insertions(+), 173 deletions(-) diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index 8aea333e5ca..7e9dfbe3000 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -2228,20 +2228,20 @@ int QTextLine::textLength() const return eng->lines.at(index).length + eng->lines.at(index).trailingSpaces; } -static void setPenAndDrawBackground(QPainter *p, const QPen &defaultPen, const QTextCharFormat &chf, const QRectF &r) +static void drawBackground(QPainter *p, const QTextCharFormat &chf, const QRectF &r) { - QBrush c = chf.foreground(); - if (c.style() == Qt::NoBrush) { - p->setPen(defaultPen); - } - QBrush bg = chf.background(); if (bg.style() != Qt::NoBrush && !chf.property(SuppressBackground).toBool()) p->fillRect(r.toAlignedRect(), bg); - if (c.style() != Qt::NoBrush) { - p->setPen(QPen(c, 0)); - } +} +static void setPen(QPainter *p, const QPen &defaultPen, const QTextCharFormat &chf) +{ + QBrush c = chf.foreground(); + if (c.style() == Qt::NoBrush) + p->setPen(defaultPen); + else + p->setPen(QPen(c, 0)); } #if !defined(QT_NO_RAWFONT) @@ -2636,7 +2636,6 @@ void QTextLine::draw_internal(QPainter *p, const QPointF &origPos, Q_ASSERT(!eng->useRawFont); #endif const QScriptLine &line = eng->lines[index]; - QPen pen = p->pen(); bool noText = (selection && selection->format.property(SuppressText).toBool()); @@ -2648,8 +2647,7 @@ void QTextLine::draw_internal(QPainter *p, const QPointF &origPos, const qreal lineHeight = line.height().toReal(); QRectF r(origPos.x() + line.x.toReal(), origPos.y() + line.y.toReal(), lineHeight / 2, QFontMetrics(eng->font()).horizontalAdvance(u' ')); - setPenAndDrawBackground(p, QPen(), selection->format, r); - p->setPen(pen); + drawBackground(p, selection->format, r); } return; } @@ -2662,7 +2660,7 @@ void QTextLine::draw_internal(QPainter *p, const QPointF &origPos, else p->translate(origPos); - QTextLineItemIterator iterator(eng, index, pos, selection); + QFixed lineBase = line.base(); eng->clearDecorations(); eng->enableDelayDecorations(); @@ -2672,183 +2670,207 @@ void QTextLine::draw_internal(QPainter *p, const QPointF &origPos, const QTextFormatCollection *formatCollection = eng->formatCollection(); bool suppressColors = (eng->option.flags() & QTextOption::SuppressColors); - while (!iterator.atEnd()) { - QScriptItem &si = iterator.next(); - if (selection && selection->start >= 0 && iterator.isOutsideSelection()) - continue; + auto prepareFormat = [suppressColors, selection, this](QTextCharFormat &format, + QScriptItem *si) { + format.merge(eng->format(si)); - if (si.analysis.flags == QScriptAnalysis::LineOrParagraphSeparator - && !(eng->option.flags() & QTextOption::ShowLineAndParagraphSeparators)) - continue; + if (suppressColors) { + format.clearForeground(); + format.clearBackground(); + format.clearProperty(QTextFormat::TextUnderlineColor); + } + if (selection) + format.merge(selection->format); + }; - QFixed itemBaseLine = y; - QFont f = eng->font(si); - QTextCharFormat format; - if (formatCollection != nullptr) - format = formatCollection->defaultTextFormat(); + { + QTextLineItemIterator iterator(eng, index, pos, selection); + while (!iterator.atEnd()) { + QScriptItem &si = iterator.next(); - if (eng->hasFormats() || selection || formatCollection) { - format.merge(eng->format(&si)); - - if (suppressColors) { - format.clearForeground(); - format.clearBackground(); - format.clearProperty(QTextFormat::TextUnderlineColor); - } - if (selection) - format.merge(selection->format); - - setPenAndDrawBackground(p, pen, format, QRectF(iterator.x.toReal(), (y - lineBase).toReal(), - iterator.itemWidth.toReal(), line.height().toReal())); - - const qreal baseLineOffset = format.baselineOffset() / 100.0; - QTextCharFormat::VerticalAlignment valign = format.verticalAlignment(); - if (valign == QTextCharFormat::AlignSuperScript - || valign == QTextCharFormat::AlignSubScript - || !qFuzzyIsNull(baseLineOffset)) - { - QFontEngine *fe = f.d->engineForScript(si.analysis.script); - QFixed height = fe->ascent() + fe->descent(); - itemBaseLine -= height * QFixed::fromReal(baseLineOffset); - - if (valign == QTextCharFormat::AlignSubScript) - itemBaseLine += height * QFixed::fromReal(format.subScriptBaseline() / 100.0); - else if (valign == QTextCharFormat::AlignSuperScript) - itemBaseLine -= height * QFixed::fromReal(format.superScriptBaseline() / 100.0); + if (eng->hasFormats() || selection || formatCollection) { + QTextCharFormat format; + if (formatCollection != nullptr) + format = formatCollection->defaultTextFormat(); + prepareFormat(format, &si); + drawBackground(p, format, QRectF(iterator.x.toReal(), (y - lineBase).toReal(), + iterator.itemWidth.toReal(), line.height().toReal())); } } + } - if (si.analysis.flags >= QScriptAnalysis::TabOrObject) { + QPen pen = p->pen(); + { + QTextLineItemIterator iterator(eng, index, pos, selection); + while (!iterator.atEnd()) { + QScriptItem &si = iterator.next(); - if (eng->hasFormats()) { - p->save(); - if (si.analysis.flags == QScriptAnalysis::Object && QTextDocumentPrivate::get(eng->block)) { - QFixed itemY = y - si.ascent; - switch (format.verticalAlignment()) { - case QTextCharFormat::AlignTop: - itemY = y - lineBase; - break; - case QTextCharFormat::AlignMiddle: - itemY = y - lineBase + (line.height() - si.height()) / 2; - break; - case QTextCharFormat::AlignBottom: - itemY = y - lineBase + line.height() - si.height(); - break; - default: - break; - } + if (selection && selection->start >= 0 && iterator.isOutsideSelection()) + continue; - QRectF itemRect(iterator.x.toReal(), itemY.toReal(), iterator.itemWidth.toReal(), si.height().toReal()); + if (si.analysis.flags == QScriptAnalysis::LineOrParagraphSeparator + && !(eng->option.flags() & QTextOption::ShowLineAndParagraphSeparators)) + continue; - eng->docLayout()->drawInlineObject(p, itemRect, - QTextInlineObject(iterator.item, eng), - si.position + eng->block.position(), - format); - if (selection) { - QBrush bg = format.brushProperty(ObjectSelectionBrush); - if (bg.style() != Qt::NoBrush) { - QColor c = bg.color(); - c.setAlpha(128); - p->fillRect(itemRect, c); + QFixed itemBaseLine = y; + QFont f = eng->font(si); + QTextCharFormat format; + if (formatCollection != nullptr) + format = formatCollection->defaultTextFormat(); + + if (eng->hasFormats() || selection || formatCollection) { + prepareFormat(format, &si); + setPen(p, pen, format); + + const qreal baseLineOffset = format.baselineOffset() / 100.0; + QTextCharFormat::VerticalAlignment valign = format.verticalAlignment(); + if (valign == QTextCharFormat::AlignSuperScript + || valign == QTextCharFormat::AlignSubScript + || !qFuzzyIsNull(baseLineOffset)) + { + QFontEngine *fe = f.d->engineForScript(si.analysis.script); + QFixed height = fe->ascent() + fe->descent(); + itemBaseLine -= height * QFixed::fromReal(baseLineOffset); + + if (valign == QTextCharFormat::AlignSubScript) + itemBaseLine += height * QFixed::fromReal(format.subScriptBaseline() / 100.0); + else if (valign == QTextCharFormat::AlignSuperScript) + itemBaseLine -= height * QFixed::fromReal(format.superScriptBaseline() / 100.0); + } + } + + if (si.analysis.flags >= QScriptAnalysis::TabOrObject) { + + if (eng->hasFormats()) { + p->save(); + if (si.analysis.flags == QScriptAnalysis::Object && QTextDocumentPrivate::get(eng->block)) { + QFixed itemY = y - si.ascent; + switch (format.verticalAlignment()) { + case QTextCharFormat::AlignTop: + itemY = y - lineBase; + break; + case QTextCharFormat::AlignMiddle: + itemY = y - lineBase + (line.height() - si.height()) / 2; + break; + case QTextCharFormat::AlignBottom: + itemY = y - lineBase + line.height() - si.height(); + break; + default: + break; } - } - } else { // si.isTab - QFont f = eng->font(si); - QTextItemInt gf(si, &f, format); - gf.chars = nullptr; - gf.num_chars = 0; - gf.width = iterator.itemWidth; - QPainterPrivate::get(p)->drawTextItem(QPointF(iterator.x.toReal(), y.toReal()), gf, eng); - if (eng->option.flags() & QTextOption::ShowTabsAndSpaces) { - const QChar visualTab = QChar(QChar::VisualTabCharacter); - int w = QFontMetrics(f).horizontalAdvance(visualTab); - qreal x = iterator.itemWidth.toReal() - w; // Right-aligned - if (x < 0) - p->setClipRect(QRectF(iterator.x.toReal(), line.y.toReal(), - iterator.itemWidth.toReal(), line.height().toReal()), - Qt::IntersectClip); - else - x /= 2; // Centered - p->setFont(f); - p->drawText(QPointF(iterator.x.toReal() + x, - y.toReal()), visualTab); - } + QRectF itemRect(iterator.x.toReal(), itemY.toReal(), iterator.itemWidth.toReal(), si.height().toReal()); + + eng->docLayout()->drawInlineObject(p, itemRect, + QTextInlineObject(iterator.item, eng), + si.position + eng->block.position(), + format); + if (selection) { + QBrush bg = format.brushProperty(ObjectSelectionBrush); + if (bg.style() != Qt::NoBrush) { + QColor c = bg.color(); + c.setAlpha(128); + p->fillRect(itemRect, c); + } + } + } else { // si.isTab + QFont f = eng->font(si); + QTextItemInt gf(si, &f, format); + gf.chars = nullptr; + gf.num_chars = 0; + gf.width = iterator.itemWidth; + QPainterPrivate::get(p)->drawTextItem(QPointF(iterator.x.toReal(), y.toReal()), gf, eng); + if (eng->option.flags() & QTextOption::ShowTabsAndSpaces) { + const QChar visualTab = QChar(QChar::VisualTabCharacter); + int w = QFontMetrics(f).horizontalAdvance(visualTab); + qreal x = iterator.itemWidth.toReal() - w; // Right-aligned + if (x < 0) + p->setClipRect(QRectF(iterator.x.toReal(), line.y.toReal(), + iterator.itemWidth.toReal(), line.height().toReal()), + Qt::IntersectClip); + else + x /= 2; // Centered + p->setFont(f); + p->drawText(QPointF(iterator.x.toReal() + x, + y.toReal()), visualTab); + } + + } + p->restore(); } + + continue; + } + + unsigned short *logClusters = eng->logClusters(&si); + QGlyphLayout glyphs = eng->shapedGlyphs(&si); + + QTextItemInt gf(glyphs.mid(iterator.glyphsStart, iterator.glyphsEnd - iterator.glyphsStart), + &f, eng->layoutData->string.unicode() + iterator.itemStart, + iterator.itemEnd - iterator.itemStart, eng->fontEngine(si), format); + gf.logClusters = logClusters + iterator.itemStart - si.position; + gf.width = iterator.itemWidth; + gf.justified = line.justified; + gf.initWithScriptItem(si); + + Q_ASSERT(gf.fontEngine); + + 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); } - continue; - } - - unsigned short *logClusters = eng->logClusters(&si); - QGlyphLayout glyphs = eng->shapedGlyphs(&si); - - QTextItemInt gf(glyphs.mid(iterator.glyphsStart, iterator.glyphsEnd - iterator.glyphsStart), - &f, eng->layoutData->string.unicode() + iterator.itemStart, - iterator.itemEnd - iterator.itemStart, eng->fontEngine(si), format); - gf.logClusters = logClusters + iterator.itemStart - si.position; - gf.width = iterator.itemWidth; - gf.justified = line.justified; - gf.initWithScriptItem(si); - - Q_ASSERT(gf.fontEngine); - - 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 ((si.analysis.flags == QScriptAnalysis::Space + || si.analysis.flags == QScriptAnalysis::Nbsp) + && (eng->option.flags() & QTextOption::ShowTabsAndSpaces)) { + QBrush c = format.foreground(); + if (c.style() != Qt::NoBrush) + p->setPen(c.color()); + const QChar visualSpace = si.analysis.flags == QScriptAnalysis::Space ? u'\xb7' : u'\xb0'; + QFont oldFont = p->font(); + p->setFont(eng->font(si)); + p->drawText(QPointF(iterator.x.toReal(), itemBaseLine.toReal()), visualSpace); + p->setPen(pen); + p->setFont(oldFont); } - - 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 - || si.analysis.flags == QScriptAnalysis::Nbsp) - && (eng->option.flags() & QTextOption::ShowTabsAndSpaces)) { - QBrush c = format.foreground(); - if (c.style() != Qt::NoBrush) - p->setPen(c.color()); - const QChar visualSpace = si.analysis.flags == QScriptAnalysis::Space ? u'\xb7' : u'\xb0'; - QFont oldFont = p->font(); - p->setFont(eng->font(si)); - p->drawText(QPointF(iterator.x.toReal(), itemBaseLine.toReal()), visualSpace); - p->setPen(pen); - p->setFont(oldFont); } } eng->drawDecorations(p); diff --git a/tests/baseline/text/tst_baseline_text.cpp b/tests/baseline/text/tst_baseline_text.cpp index 0d2c2d47f50..67d87cf9ad2 100644 --- a/tests/baseline/text/tst_baseline_text.cpp +++ b/tests/baseline/text/tst_baseline_text.cpp @@ -17,6 +17,7 @@ public: private slots: void tst_render_data(); void tst_render(); + void tst_differentScriptsBackgrounds(); private: QDir htmlDir; @@ -81,6 +82,26 @@ void tst_Text::tst_render() QBASELINE_TEST(image); } +void tst_Text::tst_differentScriptsBackgrounds() +{ + QTextDocument textDocument; + textDocument.setPageSize(QSizeF(800, 600)); + textDocument.setHtml(QString::fromUtf8("イ雨ｴ")); + + QImage image(800, 600, QImage::Format_ARGB32); + image.fill(Qt::white); + + { + QPainter painter(&image); + + QAbstractTextDocumentLayout::PaintContext context; + context.palette.setColor(QPalette::Text, Qt::black); + textDocument.documentLayout()->draw(&painter, context); + } + + QBASELINE_CHECK(image, "tst_differentScriptsBackgrounds"); +} + #define main _realmain QTEST_MAIN(tst_Text)