From ae3b5b3ab4b9c268db8230cc2ac5edac3680a2d8 Mon Sep 17 00:00:00 2001 From: Eskil Abrahamsen Blomfeldt Date: Tue, 10 May 2011 15:08:29 +0200 Subject: [PATCH] Add function QGlyphRun::setRawData() To provide an optimized way of constructing QGlyphRun objects with no copying or allocation, we add function setRawData() (naming inspired by QByteArray::setRawData()). Data retrieved from QRawFont can be passed directly into this. The logic is now that the data pointers in QGlyphRunPrivate should always point to the current valid data and is what will be used in comparisons and drawing calls. The vectors are optimizations to avoid unnecessary copying if the user wants to use the QVector based API (which makes it easier to manage the memory.) This reflected in the functions that return QVectors, which will return the stored vector if and only if it is identical to the current pointer. Otherwise we will have to copy the memory. The internal addition operators in QGlyphRun have been removed since they really provide no real optimization and have an unclear definition if the two glyph runs are based on different fonts. Reviewed-by: Jiang Jiang (cherry picked from commit 86d88c5b719fd3d50336d9d8e7127b8045ee82ae) Change-Id: Id5bb55ee3d93afb32ffca850f53382e856df7b3e Reviewed-on: http://codereview.qt.nokia.com/342 Reviewed-by: Qt Sanity Bot Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/painting/qpainter.cpp | 15 ++-- src/gui/painting/qpainter_p.h | 2 +- src/gui/text/qglyphrun.cpp | 108 +++++++++++++++---------- src/gui/text/qglyphrun.h | 4 + src/gui/text/qglyphrun_p.h | 19 +++++ src/gui/text/qtextlayout.cpp | 16 +++- tests/auto/qglyphrun/tst_qglyphrun.cpp | 79 ++++++++++++++++++ 7 files changed, 190 insertions(+), 53 deletions(-) diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp index e7b5f18112e..b13ca94b306 100644 --- a/src/gui/painting/qpainter.cpp +++ b/src/gui/painting/qpainter.cpp @@ -5801,10 +5801,12 @@ void QPainter::drawGlyphRun(const QPointF &position, const QGlyphRun &glyphRun) if (!font.isValid()) return; - QVector glyphIndexes = glyphRun.glyphIndexes(); - QVector glyphPositions = glyphRun.positions(); + QGlyphRunPrivate *glyphRun_d = QGlyphRunPrivate::get(glyphRun); - int count = qMin(glyphIndexes.size(), glyphPositions.size()); + const quint32 *glyphIndexes = glyphRun_d->glyphIndexData; + const QPointF *glyphPositions = glyphRun_d->glyphPositionData; + + int count = qMin(glyphRun_d->glyphIndexDataSize, glyphRun_d->glyphPositionDataSize); QVarLengthArray fixedPointPositions(count); QRawFontPrivate *fontD = QRawFontPrivate::get(font); @@ -5818,17 +5820,18 @@ void QPainter::drawGlyphRun(const QPointF &position, const QGlyphRun &glyphRun) } for (int i=0; istate->transform().map(processedPosition); fixedPointPositions[i] = QFixedPoint::fromPointF(processedPosition); } - d->drawGlyphs(glyphIndexes.data(), fixedPointPositions.data(), count, font, glyphRun.overline(), + d->drawGlyphs(glyphIndexes, fixedPointPositions.data(), count, font, glyphRun.overline(), glyphRun.underline(), glyphRun.strikeOut()); } -void QPainterPrivate::drawGlyphs(quint32 *glyphArray, QFixedPoint *positions, int glyphCount, +void QPainterPrivate::drawGlyphs(const quint32 *glyphArray, QFixedPoint *positions, + int glyphCount, const QRawFont &font, bool overline, bool underline, bool strikeOut) { diff --git a/src/gui/painting/qpainter_p.h b/src/gui/painting/qpainter_p.h index 35cdf86ee01..79d4b4bbe03 100644 --- a/src/gui/painting/qpainter_p.h +++ b/src/gui/painting/qpainter_p.h @@ -232,7 +232,7 @@ public: void drawOpaqueBackground(const QPainterPath &path, DrawOperation operation); #if !defined(QT_NO_RAWFONT) - void drawGlyphs(quint32 *glyphArray, QFixedPoint *positionArray, int glyphCount, + void drawGlyphs(const quint32 *glyphArray, QFixedPoint *positionArray, int glyphCount, const QRawFont &font, bool overline = false, bool underline = false, bool strikeOut = false); #endif diff --git a/src/gui/text/qglyphrun.cpp b/src/gui/text/qglyphrun.cpp index 4a51e568057..18b6357863b 100644 --- a/src/gui/text/qglyphrun.cpp +++ b/src/gui/text/qglyphrun.cpp @@ -132,13 +132,27 @@ QGlyphRun &QGlyphRun::operator=(const QGlyphRun &other) */ bool QGlyphRun::operator==(const QGlyphRun &other) const { - return ((d == other.d) - || (d->glyphIndexes == other.d->glyphIndexes - && d->glyphPositions == other.d->glyphPositions - && d->overline == other.d->overline - && d->underline == other.d->underline - && d->strikeOut == other.d->strikeOut - && d->rawFont == other.d->rawFont)); + if (d == other.d) + return true; + + if ((d->glyphIndexDataSize != other.d->glyphIndexDataSize) + || (d->glyphPositionDataSize != other.d->glyphPositionDataSize)) { + return false; + } + + for (int i=0; iglyphIndexDataSize, d->glyphPositionDataSize); ++i) { + if (i < d->glyphIndexDataSize && d->glyphIndexData[i] != other.d->glyphIndexData[i]) + return false; + + if (i < d->glyphPositionDataSize && d->glyphPositionData[i] != other.d->glyphPositionData[i]) + return false; + } + + + return (d->overline == other.d->overline + && d->underline == other.d->underline + && d->strikeOut == other.d->strikeOut + && d->rawFont == other.d->rawFont); } /*! @@ -150,36 +164,6 @@ bool QGlyphRun::operator!=(const QGlyphRun &other) const return !(*this == other); } -/*! - \internal - - Adds together the lists of glyph indexes and positions in \a other and this QGlyphRun - object and returns the result. The font in the returned QGlyphRun will be the same as in - this QGlyphRun object. -*/ -QGlyphRun QGlyphRun::operator+(const QGlyphRun &other) const -{ - QGlyphRun ret(*this); - ret += other; - return ret; -} - -/*! - \internal - - Appends the glyph indexes and positions in \a other to this QGlyphRun object and returns - a reference to the current object. -*/ -QGlyphRun &QGlyphRun::operator+=(const QGlyphRun &other) -{ - detach(); - - d->glyphIndexes += other.d->glyphIndexes; - d->glyphPositions += other.d->glyphPositions; - - return *this; -} - /*! Returns the font selected for this QGlyphRun object. @@ -208,7 +192,13 @@ void QGlyphRun::setRawFont(const QRawFont &rawFont) */ QVector QGlyphRun::glyphIndexes() const { - return d->glyphIndexes; + if (d->glyphIndexes.constData() == d->glyphIndexData) { + return d->glyphIndexes; + } else { + QVector indexes(d->glyphIndexDataSize); + qMemCopy(indexes.data(), d->glyphIndexData, d->glyphIndexDataSize * sizeof(quint32)); + return indexes; + } } /*! @@ -218,7 +208,9 @@ QVector QGlyphRun::glyphIndexes() const void QGlyphRun::setGlyphIndexes(const QVector &glyphIndexes) { detach(); - d->glyphIndexes = glyphIndexes; + d->glyphIndexes = glyphIndexes; // Keep a reference to the QVector to avoid copying + d->glyphIndexData = glyphIndexes.constData(); + d->glyphIndexDataSize = glyphIndexes.size(); } /*! @@ -226,7 +218,14 @@ void QGlyphRun::setGlyphIndexes(const QVector &glyphIndexes) */ QVector QGlyphRun::positions() const { - return d->glyphPositions; + if (d->glyphPositions.constData() == d->glyphPositionData) { + return d->glyphPositions; + } else { + QVector glyphPositions(d->glyphPositionDataSize); + qMemCopy(glyphPositions.data(), d->glyphPositionData, + d->glyphPositionDataSize * sizeof(QPointF)); + return glyphPositions; + } } /*! @@ -236,7 +235,9 @@ QVector QGlyphRun::positions() const void QGlyphRun::setPositions(const QVector &positions) { detach(); - d->glyphPositions = positions; + d->glyphPositions = positions; // Keep a reference to the vector to avoid copying + d->glyphPositionData = positions.constData(); + d->glyphPositionDataSize = positions.size(); } /*! @@ -245,12 +246,33 @@ void QGlyphRun::setPositions(const QVector &positions) void QGlyphRun::clear() { detach(); - d->glyphPositions = QVector(); - d->glyphIndexes = QVector(); d->rawFont = QRawFont(); d->strikeOut = false; d->overline = false; d->underline = false; + + setPositions(QVector()); + setGlyphIndexes(QVector()); +} + +/*! + Sets the glyph indexes and positions of this QGlyphRun to use the first \a size + elements in the arrays \a glyphIndexArray and \a glyphPositionArray. The data is + \e not copied. The caller must guarantee that the arrays are not deleted as long + as this QGlyphRun and any copies of it exists. + + \sa setGlyphIndexes(), setPositions() +*/ +void QGlyphRun::setRawData(const quint32 *glyphIndexArray, const QPointF *glyphPositionArray, + int size) +{ + detach(); + d->glyphIndexes.clear(); + d->glyphPositions.clear(); + + d->glyphIndexData = glyphIndexArray; + d->glyphPositionData = glyphPositionArray; + d->glyphIndexDataSize = d->glyphPositionDataSize = size; } /*! diff --git a/src/gui/text/qglyphrun.h b/src/gui/text/qglyphrun.h index 99a1fc85935..b4f02f0d87d 100644 --- a/src/gui/text/qglyphrun.h +++ b/src/gui/text/qglyphrun.h @@ -66,6 +66,10 @@ public: QRawFont rawFont() const; void setRawFont(const QRawFont &rawFont); + void setRawData(const quint32 *glyphIndexArray, + const QPointF *glyphPositionArray, + int size); + QVector glyphIndexes() const; void setGlyphIndexes(const QVector &glyphIndexes); diff --git a/src/gui/text/qglyphrun_p.h b/src/gui/text/qglyphrun_p.h index 533679d96ad..1cb63b24035 100644 --- a/src/gui/text/qglyphrun_p.h +++ b/src/gui/text/qglyphrun_p.h @@ -71,6 +71,10 @@ public: : overline(false) , underline(false) , strikeOut(false) + , glyphIndexData(glyphIndexes.constData()) + , glyphIndexDataSize(0) + , glyphPositionData(glyphPositions.constData()) + , glyphPositionDataSize(0) { } @@ -82,6 +86,10 @@ public: , overline(other.overline) , underline(other.underline) , strikeOut(other.strikeOut) + , glyphIndexData(other.glyphIndexData) + , glyphIndexDataSize(other.glyphIndexDataSize) + , glyphPositionData(other.glyphPositionData) + , glyphPositionDataSize(other.glyphPositionDataSize) { } @@ -89,6 +97,17 @@ public: QVector glyphPositions; QRawFont rawFont; + const quint32 *glyphIndexData; + int glyphIndexDataSize; + + const QPointF *glyphPositionData; + int glyphPositionDataSize; + + static QGlyphRunPrivate *get(const QGlyphRun &glyphRun) + { + return glyphRun.d.data(); + } + uint overline : 1; uint underline : 1; uint strikeOut : 1; diff --git a/src/gui/text/qtextlayout.cpp b/src/gui/text/qtextlayout.cpp index add25cdad94..781dd23a583 100644 --- a/src/gui/text/qtextlayout.cpp +++ b/src/gui/text/qtextlayout.cpp @@ -2282,10 +2282,20 @@ QList QTextLine::glyphRuns(int from, int length) const glyphIndexes.setRawFont(font); QPair key(fontEngine, int(flags)); - if (!glyphsHash.contains(key)) + if (!glyphsHash.contains(key)) { glyphsHash.insert(key, glyphIndexes); - else - glyphsHash[key] += glyphIndexes; + } else { + QGlyphRun &glyphRun = glyphsHash[key]; + + QVector indexes = glyphRun.glyphIndexes(); + QVector positions = glyphRun.positions(); + + indexes += glyphIndexes.glyphIndexes(); + positions += glyphIndexes.positions(); + + glyphRun.setGlyphIndexes(indexes); + glyphRun.setPositions(positions); + } } } diff --git a/tests/auto/qglyphrun/tst_qglyphrun.cpp b/tests/auto/qglyphrun/tst_qglyphrun.cpp index 3ea84e3a4d7..a18a2ac8ddd 100644 --- a/tests/auto/qglyphrun/tst_qglyphrun.cpp +++ b/tests/auto/qglyphrun/tst_qglyphrun.cpp @@ -72,6 +72,8 @@ private slots: void drawUnderlinedText(); void drawRightToLeft(); void detach(); + void setRawData(); + void setRawDataAndGetAsVector(); private: int m_testFontId; @@ -284,6 +286,83 @@ void tst_QGlyphRun::drawExistingGlyphs() QCOMPARE(textLayoutDraw, drawGlyphs); } +void tst_QGlyphRun::setRawData() +{ + QGlyphRun glyphRun; + glyphRun.setRawFont(QRawFont::fromFont(m_testFont)); + glyphRun.setGlyphIndexes(QVector() << 2 << 2 << 2); + glyphRun.setPositions(QVector() << QPointF(2, 3) << QPointF(20, 3) << QPointF(10, 20)); + + QPixmap baseline(100, 50); + baseline.fill(Qt::white); + { + QPainter p(&baseline); + p.drawGlyphRun(QPointF(3, 2), glyphRun); + } + + QGlyphRun baselineCopied = glyphRun; + + quint32 glyphIndexArray[3] = { 2, 2, 2 }; + QPointF glyphPositionArray[3] = { QPointF(2, 3), QPointF(20, 3), QPointF(10, 20) }; + + glyphRun.setRawData(glyphIndexArray, glyphPositionArray, 3); + + QPixmap rawDataGlyphs(100, 50); + rawDataGlyphs.fill(Qt::white); + { + QPainter p(&rawDataGlyphs); + p.drawGlyphRun(QPointF(3, 2), glyphRun); + } + + quint32 otherGlyphIndexArray[1] = { 2 }; + QPointF otherGlyphPositionArray[1] = { QPointF(2, 3) }; + + glyphRun.setRawData(otherGlyphIndexArray, otherGlyphPositionArray, 1); + + QPixmap baselineCopiedPixmap(100, 50); + baselineCopiedPixmap.fill(Qt::white); + { + QPainter p(&baselineCopiedPixmap); + p.drawGlyphRun(QPointF(3, 2), baselineCopied); + } + +#if defined(DEBUG_SAVE_IMAGE) + baseline.save("setRawData_baseline.png"); + rawDataGlyphs.save("setRawData_rawDataGlyphs.png"); + baselineCopiedPixmap.save("setRawData_baselineCopiedPixmap.png"); +#endif + + QCOMPARE(rawDataGlyphs, baseline); + QCOMPARE(baselineCopiedPixmap, baseline); +} + +void tst_QGlyphRun::setRawDataAndGetAsVector() +{ + QVector glyphIndexArray; + glyphIndexArray << 3 << 2 << 1 << 4; + + QVector glyphPositionArray; + glyphPositionArray << QPointF(1, 2) << QPointF(3, 4) << QPointF(5, 6) << QPointF(7, 8); + + QGlyphRun glyphRun; + glyphRun.setRawData(glyphIndexArray.constData(), glyphPositionArray.constData(), 4); + + QVector glyphIndexes = glyphRun.glyphIndexes(); + QVector glyphPositions = glyphRun.positions(); + + QCOMPARE(glyphIndexes.size(), 4); + QCOMPARE(glyphPositions.size(), 4); + + QCOMPARE(glyphIndexes, glyphIndexArray); + QCOMPARE(glyphPositions, glyphPositionArray); + + QGlyphRun otherGlyphRun; + otherGlyphRun.setGlyphIndexes(glyphIndexArray); + otherGlyphRun.setPositions(glyphPositionArray); + + QCOMPARE(glyphRun, otherGlyphRun); +} + void tst_QGlyphRun::drawNonExistentGlyphs() { QVector glyphIndexes;