Refactor lockedAlphaMapForGlyph

Simply return a Glyph pointer and not a QImage to avoid allocating and
deleting lots of d pointers for QImage when drawing text. Saves one
new/delete pair per glyph drawn and speeds up text drawing by 10% for
relatively large glyphs (probably more for smaller ones).

The qtext::paintLayoutToPixmap() benchmark shows a 16% improvement
in performance with this change.

Renamed the method to glyphData().

Change-Id: I7a353de521e4f4321c770fb1ac6043d33f6f332c
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>

src/gui/painting/qpaintengine_raster.cpp

@@ -2908,19 +2908,34 @@ bool QRasterPaintEngine::drawCachedGlyphs(int numGlyphs, const glyph_t *glyphs,
         for (int i = 0; i < numGlyphs; i++) {
             QFixed spp = fontEngine->subPixelPositionForX(positions[i].x);
 
-            QPoint offset;
+            const QFontEngine::Glyph *alphaMap = fontEngine->glyphData(glyphs[i], spp, neededFormat, s->matrix);
-            const QImage *alphaMap = fontEngine->lockedAlphaMapForGlyph(glyphs[i], spp, neededFormat, s->matrix,
+            if (!alphaMap)
-                                                                        &offset);
-            if (alphaMap == 0 || alphaMap->isNull())
                 continue;
 
-            alphaPenBlt(alphaMap->constBits(), alphaMap->bytesPerLine(), alphaMap->depth(),
+            int depth;
-                        qFloor(positions[i].x) + offset.x(),
+            int bytesPerLine;
-                        qRound(positions[i].y) + offset.y(),
+            switch (alphaMap->format) {
-                        alphaMap->width(), alphaMap->height(),
+            case QFontEngine::Format_Mono:
-                        fontEngine->expectsGammaCorrectedBlending());
+                depth = 1;
+                bytesPerLine = ((alphaMap->width + 31) & ~31) >> 3;
+                break;
+            case QFontEngine::Format_A8:
+                depth = 8;
+                bytesPerLine = (alphaMap->width + 3) & ~3;
+                break;
+            case QFontEngine::Format_A32:
+                depth = 32;
+                bytesPerLine = alphaMap->width * 4;
+                break;
+            default:
+                Q_UNREACHABLE();
+            };
 
-            fontEngine->unlockAlphaMapForGlyph();
+            alphaPenBlt(alphaMap->data, bytesPerLine, depth,
+                        qFloor(positions[i].x) + alphaMap->x,
+                        qRound(positions[i].y) - alphaMap->y,
+                        alphaMap->width, alphaMap->height,
+                        fontEngine->expectsGammaCorrectedBlending());
         }
 
     } else {

src/gui/text/qfontengine.cpp

@@ -923,29 +923,10 @@ QFixed QFontEngine::subPixelPositionForX(QFixed x) const
     return subPixelPosition;
 }
 
-QImage *QFontEngine::lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition,
+QFontEngine::Glyph *QFontEngine::glyphData(glyph_t, QFixed,
-                                            QFontEngine::GlyphFormat neededFormat,
+                                           QFontEngine::GlyphFormat, const QTransform &)
-                                            const QTransform &t, QPoint *offset)
 {
-    Q_ASSERT(currentlyLockedAlphaMap.isNull());
+    return nullptr;
-    if (neededFormat == Format_None)
-        neededFormat = Format_A32;
-
-    if (neededFormat != Format_A32)
-        currentlyLockedAlphaMap = alphaMapForGlyph(glyph, subPixelPosition, t);
-    else
-        currentlyLockedAlphaMap = alphaRGBMapForGlyph(glyph, subPixelPosition, t);
-
-    if (offset != 0)
-        *offset = QPoint(0, 0);
-
-    return &currentlyLockedAlphaMap;
-}
-
-void QFontEngine::unlockAlphaMapForGlyph()
-{
-    Q_ASSERT(!currentlyLockedAlphaMap.isNull());
-    currentlyLockedAlphaMap = QImage();
 }
 
 QImage QFontEngine::alphaMapForGlyph(glyph_t glyph)

src/gui/text/qfontengine_p.h

@@ -124,6 +124,22 @@ public:
     };
     Q_DECLARE_FLAGS(ShaperFlags, ShaperFlag)
 
+    /* Used with the Freetype font engine. We don't cache glyphs that are too large anyway, so we can make this struct rather small */
+    struct Glyph {
+        Glyph() = default;
+        ~Glyph() { delete [] data; }
+        short linearAdvance = 0;
+        unsigned char width = 0;
+        unsigned char height = 0;
+        short x = 0;
+        short y = 0;
+        short advance = 0;
+        signed char format = 0;
+        uchar *data = nullptr;
+    private:
+        Q_DISABLE_COPY(Glyph);
+    };
+
     virtual ~QFontEngine();
 
     inline Type type() const { return m_type; }
@@ -191,11 +207,7 @@ public:
     virtual QImage alphaMapForGlyph(glyph_t, QFixed subPixelPosition, const QTransform &t);
     virtual QImage alphaRGBMapForGlyph(glyph_t, QFixed subPixelPosition, const QTransform &t);
     virtual QImage bitmapForGlyph(glyph_t, QFixed subPixelPosition, const QTransform &t, const QColor &color = QColor());
-    virtual QImage *lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition,
+    virtual Glyph *glyphData(glyph_t glyph, QFixed subPixelPosition, GlyphFormat neededFormat, const QTransform &t);
-                                           GlyphFormat neededFormat,
-                                           const QTransform &t = QTransform(),
-                                           QPoint *offset = 0);
-    virtual void unlockAlphaMapForGlyph();
     virtual bool hasInternalCaching() const { return false; }
 
     virtual glyph_metrics_t alphaMapBoundingBox(glyph_t glyph, QFixed /*subPixelPosition*/, const QTransform &matrix, GlyphFormat /*format*/)
@@ -346,7 +358,6 @@ public:
     void loadKerningPairs(QFixed scalingFactor);
 
     GlyphFormat glyphFormat;
-    QImage currentlyLockedAlphaMap;
     int m_subPixelPositionCount; // Number of positions within a single pixel for this cache
 
     inline QVariant userData() const { return m_userData; }

src/platformsupport/fontdatabases/freetype/qfontengine_ft.cpp

@@ -106,7 +106,7 @@ static bool ft_getSfntTable(void *user_data, uint tag, uchar *buffer, uint *leng
     return result;
 }
 
-static QFontEngineFT::Glyph emptyGlyph = {0, 0, 0, 0, 0, 0, 0, 0};
+static QFontEngineFT::Glyph emptyGlyph;
 
 static const QFontEngine::HintStyle ftInitialDefaultHintStyle =
 #ifdef Q_OS_WIN
@@ -556,11 +556,6 @@ void QFreetypeFace::addBitmapToPath(FT_GlyphSlot slot, const QFixedPoint &point,
                        slot->bitmap.buffer, slot->bitmap.pitch, slot->bitmap.width, slot->bitmap.rows, path);
 }
 
-QFontEngineFT::Glyph::~Glyph()
-{
-    delete [] data;
-}
-
 struct LcdFilterDummy
 {
     static inline void filterPixel(uchar &, uchar &, uchar &)
@@ -1986,11 +1981,10 @@ static inline QImage alphaMapFromGlyphData(QFontEngineFT::Glyph *glyph, QFontEng
     return img;
 }
 
-QImage *QFontEngineFT::lockedAlphaMapForGlyph(glyph_t glyphIndex, QFixed subPixelPosition,
+QFontEngine::Glyph *QFontEngineFT::glyphData(glyph_t glyphIndex, QFixed subPixelPosition,
-                                              QFontEngine::GlyphFormat neededFormat,
+                                             QFontEngine::GlyphFormat neededFormat, const QTransform &t)
-                                              const QTransform &t, QPoint *offset)
 {
-    Q_ASSERT(currentlyLockedAlphaMap.isNull());
+    Q_ASSERT(cacheEnabled);
 
     if (isBitmapFont())
         neededFormat = Format_Mono;
@@ -2000,33 +1994,10 @@ QImage *QFontEngineFT::lockedAlphaMapForGlyph(glyph_t glyphIndex, QFixed subPixe
         neededFormat = Format_A8;
 
     Glyph *glyph = loadGlyphFor(glyphIndex, subPixelPosition, neededFormat, t);
-
+    if (!glyph || !glyph->width || !glyph->height)
-    if (offset != 0 && glyph != 0)
-        *offset = QPoint(glyph->x, -glyph->y);
-
-    currentlyLockedAlphaMap = alphaMapFromGlyphData(glyph, neededFormat);
-
-    const bool glyphHasGeometry = glyph != nullptr && glyph->height != 0 && glyph->width != 0;
-    if (!cacheEnabled && glyph != &emptyGlyph) {
-        currentlyLockedAlphaMap = currentlyLockedAlphaMap.copy();
-        delete glyph;
-    }
-
-    if (!glyphHasGeometry)
         return nullptr;
 
-    if (currentlyLockedAlphaMap.isNull())
+    return glyph;
-        return QFontEngine::lockedAlphaMapForGlyph(glyphIndex, subPixelPosition, neededFormat, t, offset);
-
-    QImageData *data = currentlyLockedAlphaMap.data_ptr();
-    data->is_locked = true;
-
-    return &currentlyLockedAlphaMap;
-}
-
-void QFontEngineFT::unlockAlphaMapForGlyph()
-{
-    QFontEngine::unlockAlphaMapForGlyph();
 }
 
 static inline bool is2dRotation(const QTransform &t)

src/platformsupport/fontdatabases/freetype/qfontengine_ft_p.h

@@ -129,20 +129,6 @@ private:
 class QFontEngineFT : public QFontEngine
 {
 public:
-
-    /* we don't cache glyphs that are too large anyway, so we can make this struct rather small */
-    struct Glyph {
-        ~Glyph();
-        int linearAdvance : 22;
-        unsigned char width;
-        unsigned char height;
-        short x;
-        short y;
-        short advance;
-        signed char format;
-        uchar *data;
-    };
-
     struct GlyphInfo {
         int             linearAdvance;
         unsigned short  width;
@@ -241,11 +227,9 @@ private:
                                         QFixed subPixelPosition,
                                         const QTransform &matrix,
                                         QFontEngine::GlyphFormat format) override;
-    QImage *lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition,
+    Glyph *glyphData(glyph_t glyph, QFixed subPixelPosition,
-                                   GlyphFormat neededFormat, const QTransform &t,
+                     GlyphFormat neededFormat, const QTransform &t) override;
-                                   QPoint *offset) override;
     bool hasInternalCaching() const override { return cacheEnabled; }
-    void unlockAlphaMapForGlyph() override;
     bool expectsGammaCorrectedBlending() const override;
 
     void removeGlyphFromCache(glyph_t glyph) override;