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 <eskil.abrahamsen-blomfeldt@digia.com>

2 files changed, 41 insertions, 39 deletions

diff --git a/src/gui/text/qrawfont.cpp b/src/gui/text/qrawfont.cpp
index dee1abcfbc..f1191bc4b9 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<quint32> QRawFont::glyphIndexesForString(const QString &text) const
 {
-    if (!d->isValid())
-        return QVector<quint32>();
+    QVector<quint32> 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<quint32>();
         }
     }
 
-    QVector<quint32> glyphIndexes;
-    for (int i=0; i<nglyphs; ++i)
-        glyphIndexes.append(glyphs.glyphs[i]);
-
+    glyphIndexes.resize(numGlyphs);
     return glyphIndexes;
 }
 
@@ -491,38 +490,32 @@ QVector<quint32> 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<QPointF> QRawFont::advancesForGlyphIndexes(const QVector<quint32> &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<QPointF> QRawFont::advancesForGlyphIndexes(const QVector<quint32> &glyphIndexes) const
-{
-    if (!d->isValid())
-        return QVector<QPointF>();
-
-    int numGlyphs = glyphIndexes.size();
-    QVarLengthGlyphLayoutArray glyphs(numGlyphs);
-    memcpy(glyphs.glyphs, glyphIndexes.data(), numGlyphs * sizeof(quint32));
-
-    d->fontEngine->recalcAdvances(&glyphs, 0);
-
-    QVector<QPointF> advances;
-    for (int i=0; i<numGlyphs; ++i)
-        advances.append(QPointF(glyphs.advances_x[i].toReal(), glyphs.advances_y[i].toReal()));
-
-    return advances;
-}
 
 /*!
    Returns the QRawFont's advances for each of the \a glyphIndexes in pixel units. The advances
@@ -535,7 +528,8 @@ QVector<QPointF> QRawFont::advancesForGlyphIndexes(const QVector<quint32> &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 1dbde27c73..98314809e8 100644
--- a/src/gui/text/qrawfont.h
+++ b/src/gui/text/qrawfont.h
@@ -94,7 +94,7 @@ public:
     int weight() const;
 
     QVector<quint32> glyphIndexesForString(const QString &text) const;
-    QVector<QPointF> advancesForGlyphIndexes(const QVector<quint32> &glyphIndexes) const;
+    inline QVector<QPointF> advancesForGlyphIndexes(const QVector<quint32> &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<QPointF> QRawFont::advancesForGlyphIndexes(const QVector<quint32> &glyphIndexes) const
+{
+    QVector<QPointF> advances(glyphIndexes.size());
+    if (advancesForGlyphIndexes(glyphIndexes.constData(), advances.data(), glyphIndexes.size()))
+        return advances;
+    return QVector<QPointF>();
+}
+
 QT_END_NAMESPACE
 
 QT_END_HEADER