From 3e030a9652e7a95ed28e24ec258a7895e8bc1e34 Mon Sep 17 00:00:00 2001 From: Konstantin Ritt Date: Thu, 11 Oct 2012 16:19:34 +0300 Subject: [PATCH] QRawFont: improve performance and safety of glyphIndexesForString() As of 98c1eb1750498cdff9d3b26658e5e5be9c026c92, partially initialized QGlyphLayout is ok for stringToCMap() if GlyphIndicesOnly flag is set, thus we can use the glyphIndexes buffer directly and avoid copying. Also add some checks to guarantee we're not falling into an undefined behavior for the empty text or NULL buffer. Change-Id: I662953703e4c65edbebabbe4b753972417d963f3 Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/gui/text/qrawfont.cpp | 70 ++++++++++++--------------- src/gui/text/qrawfont.h | 10 +++- tests/auto/gui/text/qrawfont/tst_qrawfont.cpp | 45 +++++++++++++++++ 3 files changed, 86 insertions(+), 39 deletions(-) diff --git a/src/gui/text/qrawfont.cpp b/src/gui/text/qrawfont.cpp index dee1abc..f1191bc 100644 --- a/src/gui/text/qrawfont.cpp +++ b/src/gui/text/qrawfont.cpp @@ -388,10 +388,7 @@ qreal QRawFont::unitsPerEm() const */ qreal QRawFont::lineThickness() const { - if (!isValid()) - return 0.0; - - return d->fontEngine->lineThickness().toReal(); + return d->isValid() ? d->fontEngine->lineThickness().toReal() : 0.0; } /*! @@ -400,10 +397,7 @@ qreal QRawFont::lineThickness() const */ qreal QRawFont::underlinePosition() const { - if (!isValid()) - return 0.0; - - return d->fontEngine->underlinePosition().toReal(); + return d->isValid() ? d->fontEngine->underlinePosition().toReal() : 0.0; } /*! @@ -459,23 +453,28 @@ int QRawFont::weight() const */ QVector QRawFont::glyphIndexesForString(const QString &text) const { - if (!d->isValid()) - return QVector(); + QVector glyphIndexes; + if (!d->isValid() || text.isEmpty()) + return glyphIndexes; + + int numGlyphs = text.size(); + glyphIndexes.resize(numGlyphs); - int nglyphs = text.size(); - QVarLengthGlyphLayoutArray glyphs(nglyphs); - if (!glyphIndexesForChars(text.data(), text.size(), glyphs.glyphs, &nglyphs)) { - glyphs.resize(nglyphs); - if (!glyphIndexesForChars(text.data(), text.size(), glyphs.glyphs, &nglyphs)) { + QGlyphLayout glyphs; + glyphs.numGlyphs = numGlyphs; + glyphs.glyphs = glyphIndexes.data(); + if (!d->fontEngine->stringToCMap(text.data(), text.size(), &glyphs, &numGlyphs, QFontEngine::GlyphIndicesOnly)) { + glyphIndexes.resize(numGlyphs); + + glyphs.numGlyphs = numGlyphs; + glyphs.glyphs = glyphIndexes.data(); + if (!d->fontEngine->stringToCMap(text.data(), text.size(), &glyphs, &numGlyphs, QFontEngine::GlyphIndicesOnly)) { Q_ASSERT_X(false, Q_FUNC_INFO, "stringToCMap shouldn't fail twice"); return QVector(); } } - QVector glyphIndexes; - for (int i=0; i QRawFont::glyphIndexesForString(const QString &text) const */ bool QRawFont::glyphIndexesForChars(const QChar *chars, int numChars, quint32 *glyphIndexes, int *numGlyphs) const { - if (!d->isValid()) + Q_ASSERT(numGlyphs); + if (!d->isValid() || numChars <= 0) { + *numGlyphs = 0; + return false; + } + + if (*numGlyphs <= 0 || !glyphIndexes) { + *numGlyphs = numChars; return false; + } QGlyphLayout glyphs; + glyphs.numGlyphs = *numGlyphs; glyphs.glyphs = glyphIndexes; return d->fontEngine->stringToCMap(chars, numChars, &glyphs, numGlyphs, QFontEngine::GlyphIndicesOnly); } /*! + \fn QVector QRawFont::advancesForGlyphIndexes(const QVector &glyphIndexes) const + Returns the QRawFont's advances for each of the \a glyphIndexes in pixel units. The advances give the distance from the position of a given glyph to where the next glyph should be drawn to make it appear as if the two glyphs are unspaced. \sa QTextLine::horizontalAdvance(), QFontMetricsF::width() */ -QVector QRawFont::advancesForGlyphIndexes(const QVector &glyphIndexes) const -{ - if (!d->isValid()) - return QVector(); - - int numGlyphs = glyphIndexes.size(); - QVarLengthGlyphLayoutArray glyphs(numGlyphs); - memcpy(glyphs.glyphs, glyphIndexes.data(), numGlyphs * sizeof(quint32)); - - d->fontEngine->recalcAdvances(&glyphs, 0); - - QVector advances; - for (int i=0; i QRawFont::advancesForGlyphIndexes(const QVector &glyph */ bool QRawFont::advancesForGlyphIndexes(const quint32 *glyphIndexes, QPointF *advances, int numGlyphs) const { - if (!d->isValid()) + Q_ASSERT(glyphIndexes && advances); + if (!d->isValid() || numGlyphs <= 0) return false; QGlyphLayout glyphs; diff --git a/src/gui/text/qrawfont.h b/src/gui/text/qrawfont.h index 1dbde27..9831480 100644 --- a/src/gui/text/qrawfont.h +++ b/src/gui/text/qrawfont.h @@ -94,7 +94,7 @@ public: int weight() const; QVector glyphIndexesForString(const QString &text) const; - QVector advancesForGlyphIndexes(const QVector &glyphIndexes) const; + inline QVector advancesForGlyphIndexes(const QVector &glyphIndexes) const; bool glyphIndexesForChars(const QChar *chars, int numChars, quint32 *glyphIndexes, int *numGlyphs) const; bool advancesForGlyphIndexes(const quint32 *glyphIndexes, QPointF *advances, int numGlyphs) const; @@ -147,6 +147,14 @@ private: Q_DECLARE_SHARED(QRawFont) +inline QVector QRawFont::advancesForGlyphIndexes(const QVector &glyphIndexes) const +{ + QVector advances(glyphIndexes.size()); + if (advancesForGlyphIndexes(glyphIndexes.constData(), advances.data(), glyphIndexes.size())) + return advances; + return QVector(); +} + QT_END_NAMESPACE QT_END_HEADER diff --git a/tests/auto/gui/text/qrawfont/tst_qrawfont.cpp b/tests/auto/gui/text/qrawfont/tst_qrawfont.cpp index 7a25454..c26964e 100644 --- a/tests/auto/gui/text/qrawfont/tst_qrawfont.cpp +++ b/tests/auto/gui/text/qrawfont/tst_qrawfont.cpp @@ -267,6 +267,24 @@ void tst_QRawFont::glyphIndices() expectedGlyphIndices << 44 << 83 << 83 << 70 << 69 << 86; QCOMPARE(glyphIndices, expectedGlyphIndices); + + glyphIndices = font.glyphIndexesForString(QString()); + QVERIFY(glyphIndices.isEmpty()); + + QString str(QLatin1String("Foobar")); + int numGlyphs = str.size(); + glyphIndices.resize(numGlyphs); + + QVERIFY(!font.glyphIndexesForChars(str.constData(), 0, glyphIndices.data(), &numGlyphs)); + QCOMPARE(numGlyphs, 0); + + QVERIFY(!font.glyphIndexesForChars(str.constData(), str.size(), glyphIndices.data(), &numGlyphs)); + QCOMPARE(numGlyphs, str.size()); + + QVERIFY(font.glyphIndexesForChars(str.constData(), str.size(), glyphIndices.data(), &numGlyphs)); + QCOMPARE(numGlyphs, str.size()); + + QCOMPARE(glyphIndices, expectedGlyphIndices); } void tst_QRawFont::advances_data() @@ -310,6 +328,33 @@ void tst_QRawFont::advances() QVERIFY(qFuzzyIsNull(advances.at(i).y())); } + + advances = font.advancesForGlyphIndexes(QVector()); + QVERIFY(advances.isEmpty()); + + int numGlyphs = glyphIndices.size(); + advances.resize(numGlyphs); + + QVERIFY(!font.advancesForGlyphIndexes(glyphIndices.constData(), advances.data(), 0)); + + QVERIFY(font.advancesForGlyphIndexes(glyphIndices.constData(), advances.data(), numGlyphs)); + + for (int i=0; ifontEngine->type() == QFontEngine::Freetype + && (hintingPreference == QFont::PreferFullHinting || hintingPreference == QFont::PreferDefaultHinting) + && (i == 0 || i == 5)) { + QEXPECT_FAIL("", "Advance for some glyphs is not the expected with Windows Freetype engine (9 instead of 8)", Continue); + } +#endif + QVERIFY(qFuzzyCompare(qRound(advances.at(i).x()), 8.0)); + if (supportsSubPixelPositions) + QVERIFY(advances.at(i).x() > 8.0); + + QVERIFY(qFuzzyIsNull(advances.at(i).y())); + } } void tst_QRawFont::textLayout() -- 2.7.4