Avoid double caching glyphs in raster/QPA/FreeType

Since FreeType has internal caching, it has had a special
code path in the raster engine which avoided using Qt's glyph
cache, as that would be redundant. However, when compiling
with QPA, we want the same behavior without being able to
access the QFontEngineFT class. To achieve this, I've made
a new abstraction and added it to QFontEngine which allows
the FT engine to provide direct pointers into the alpha maps
stored by FT. This requires a locking/unlocking mechanism.

Yes, the QFontEngine::alphaMap* API is slowly becoming a horrible
mess, but due to time constraints, the task of refactoring these
functions into a more flexible API needs to wait. I've added a
TODO comment for this.

Change-Id: I08237403c2967f8fb952258178676f33a87c0353
Reviewed-on: http://codereview.qt-project.org/5155
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Samuel Rødal <samuel.rodal@nokia.com>

diff --git a/src/gui/image/qimage.cpp b/src/gui/image/qimage.cpp
index 2dbb2f4..d0de953 100644 (file)
--- a/src/gui/image/qimage.cpp
+++ b/src/gui/image/qimage.cpp
@@ -69,6 +69,11 @@
 
 QT_BEGIN_NAMESPACE
 
+static inline bool isLocked(QImageData *data)
+{
+    return data != 0 && data->is_locked;
+}
+
 static inline bool checkPixelSize(const QImage::Format format)
 {
     switch (format) {
@@ -130,7 +135,7 @@ QImageData::QImageData()
       dpmx(qt_defaultDpiX() * 100 / qreal(2.54)),
       dpmy(qt_defaultDpiY() * 100 / qreal(2.54)),
       offset(0, 0), own_data(true), ro_data(false), has_alpha_clut(false),
-      is_cached(false), paintEngine(0)
+      is_cached(false), is_locked(false), paintEngine(0)
 {
 }
 
@@ -1022,7 +1027,7 @@ QImage::QImage(const char * const xpm[])
 QImage::QImage(const QImage &image)
     : QPaintDevice()
 {
-    if (image.paintingActive()) {
+    if (image.paintingActive() || isLocked(image.d)) {
         d = 0;
         operator=(image.copy());
     } else {
@@ -1054,7 +1059,7 @@ QImage::~QImage()
 
 QImage &QImage::operator=(const QImage &image)
 {
-    if (image.paintingActive()) {
+    if (image.paintingActive() || isLocked(image.d)) {
         operator=(image.copy());
     } else {
         if (image.d)

diff --git a/src/gui/image/qimage_p.h b/src/gui/image/qimage_p.h
index bc24778..77b9a7d 100644 (file)
--- a/src/gui/image/qimage_p.h
+++ b/src/gui/image/qimage_p.h
@@ -92,6 +92,7 @@ struct Q_GUI_EXPORT QImageData {        // internal image data
     uint ro_data : 1;
     uint has_alpha_clut : 1;
     uint is_cached : 1;
+    uint is_locked : 1;
 
     bool checkForAlphaPixels() const;
 

diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp
index 62701eb..e2e4ed6 100644 (file)
--- a/src/gui/painting/qpaintengine_raster.cpp
+++ b/src/gui/painting/qpaintengine_raster.cpp
@@ -54,10 +54,6 @@
 #include <qbitmap.h>
 #include <qmath.h>
 
-#if defined (Q_WS_X11)
-#  include <private/qfontengine_ft_p.h>
-#endif
-
 //   #include <private/qdatabuffer_p.h>
 //   #include <private/qpainter_p.h>
 #include <private/qmath_p.h>
@@ -88,8 +84,6 @@
 #  include <private/qpaintengine_mac_p.h>
 #elif defined(Q_OS_SYMBIAN) && defined(QT_NO_FREETYPE)
 #  include <private/qfontengine_s60_p.h>
-#elif defined(Q_WS_QPA)
-#  include <private/qfontengine_ft_p.h>
 #endif
 
 #if defined(Q_OS_WIN64)
@@ -2717,91 +2711,33 @@ bool QRasterPaintEngine::drawCachedGlyphs(int numGlyphs, const glyph_t *glyphs,
     Q_D(QRasterPaintEngine);
     QRasterPaintEngineState *s = state();
 
-#if !defined(QT_NO_FREETYPE)
-    if (fontEngine->type() == QFontEngine::Freetype) {
-        QFontEngineFT *fe = static_cast<QFontEngineFT *>(fontEngine);
-        QFontEngineFT::GlyphFormat neededFormat =
+    if (fontEngine->hasInternalCaching()) {
+        QFontEngine::GlyphFormat neededFormat =
             painter()->device()->devType() == QInternal::Widget
-            ? fe->defaultGlyphFormat()
-            : QFontEngineFT::Format_A8;
-
-        if (d_func()->mono_surface
-            || fe->isBitmapFont() // alphaPenBlt can handle mono, too
-            )
-            neededFormat = QFontEngineFT::Format_Mono;
-
-        if (neededFormat == QFontEngineFT::Format_None)
-            neededFormat = QFontEngineFT::Format_A8;
-
-        QFontEngineFT::QGlyphSet *gset = fe->defaultGlyphs();
-        if (s->matrix.type() >= QTransform::TxScale) {
-            if (s->matrix.isAffine())
-                gset = fe->loadTransformedGlyphSet(s->matrix);
-            else
-                gset = 0;
-        }
-
-        if (!gset || gset->outline_drawing
-            || !fe->loadGlyphs(gset, glyphs, numGlyphs, positions, neededFormat))
-            return false;
-
-        FT_Face lockedFace = 0;
+            ? QFontEngine::Format_None
+            : QFontEngine::Format_A8;
 
-        int depth;
-        switch (neededFormat) {
-        case QFontEngineFT::Format_Mono:
-            depth = 1;
-            break;
-        case QFontEngineFT::Format_A8:
-            depth = 8;
-            break;
-        case QFontEngineFT::Format_A32:
-            depth = 32;
-            break;
-        default:
-            Q_ASSERT(false);
-            depth = 0;
-        };
+        if (d_func()->mono_surface) // alphaPenBlt can handle mono, too
+            neededFormat = QFontEngine::Format_Mono;
 
         for (int i = 0; i < numGlyphs; i++) {
-            QFixed spp = fe->subPixelPositionForX(positions[i].x);
-            QFontEngineFT::Glyph *glyph = gset->getGlyph(glyphs[i], spp);
-
-            if (!glyph || glyph->format != neededFormat) {
-                if (!lockedFace)
-                    lockedFace = fe->lockFace();
-                glyph = fe->loadGlyph(gset, glyphs[i], spp, neededFormat);
-            }
+            QFixed spp = fontEngine->subPixelPositionForX(positions[i].x);
 
-            if (!glyph || !glyph->data)
+            QPoint offset;
+            QImage *alphaMap = fontEngine->lockedAlphaMapForGlyph(glyphs[i], spp, neededFormat, s->matrix,
+                                                                  &offset);
+            if (alphaMap == 0)
                 continue;
 
-            int pitch;
-            switch (neededFormat) {
-            case QFontEngineFT::Format_Mono:
-                pitch = ((glyph->width + 31) & ~31) >> 3;
-                break;
-            case QFontEngineFT::Format_A8:
-                pitch = (glyph->width + 3) & ~3;
-                break;
-            case QFontEngineFT::Format_A32:
-                pitch = glyph->width * 4;
-                break;
-            default:
-                Q_ASSERT(false);
-                pitch = 0;
-            };
-
-            alphaPenBlt(glyph->data, pitch, depth,
-                        qFloor(positions[i].x) + glyph->x,
-                        qFloor(positions[i].y) - glyph->y,
-                        glyph->width, glyph->height);
+            alphaPenBlt(alphaMap->bits(), alphaMap->bytesPerLine(), alphaMap->depth(),
+                        qFloor(positions[i].x) + offset.x(),
+                        qFloor(positions[i].y) + offset.y(),
+                        alphaMap->width(), alphaMap->height());
+
+            fontEngine->unlockAlphaMapForGlyph();
         }
-        if (lockedFace)
-            fe->unlockFace();
-    } else
-#endif
-    {
+
+    } else {
         QFontEngineGlyphCache::Type glyphType = fontEngine->glyphFormat >= 0 ? QFontEngineGlyphCache::Type(fontEngine->glyphFormat) : d->glyphCacheType;
 
         QImageTextureGlyphCache *cache =
@@ -3057,28 +2993,23 @@ void QRasterPaintEngine::drawTextItem(const QPointF &p, const QTextItem &textIte
 
     QFontEngine *fontEngine = ti.fontEngine;
 
-#if (defined(Q_WS_X11) || defined(Q_WS_QWS) || defined(Q_OS_SYMBIAN)) && !defined(QT_NO_FREETYPE)
+#if defined(Q_WS_X11) || defined(Q_WS_QWS) || defined(Q_OS_SYMBIAN)
 
-    if (fontEngine->type() != QFontEngine::Freetype) {
-        QPaintEngineEx::drawTextItem(p, ti);
-        return;
-    }
+    if (fontEngine->type() == QFontEngine::Freetype) {
+        QTransform matrix = s->matrix;
+        matrix.translate(p.x(), p.y());
 
-    QFontEngineFT *fe = static_cast<QFontEngineFT *>(fontEngine);
+        QVarLengthArray<QFixedPoint> positions;
+        QVarLengthArray<glyph_t> glyphs;
+        fontEngine->getGlyphPositions(ti.glyphs, matrix, ti.flags, glyphs, positions);
+        if (glyphs.size() == 0)
+            return;
 
-    QTransform matrix = s->matrix;
-    matrix.translate(p.x(), p.y());
+        if (!drawCachedGlyphs(glyphs.size(), glyphs.constData(), positions.constData(), fontEngine))
+            QPaintEngine::drawTextItem(p, ti);
 
-    QVarLengthArray<QFixedPoint> positions;
-    QVarLengthArray<glyph_t> glyphs;
-    fe->getGlyphPositions(ti.glyphs, matrix, ti.flags, glyphs, positions);
-    if (glyphs.size() == 0)
         return;
-
-    if (!drawCachedGlyphs(glyphs.size(), glyphs.constData(), positions.constData(), fontEngine))
-        QPaintEngine::drawTextItem(p, ti);
-
-    return;
+    }
 #endif
 #endif
 

diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp
index f7f3b0e..4f18aa3 100644 (file)
--- a/src/gui/text/qfontengine.cpp
+++ b/src/gui/text/qfontengine.cpp
@@ -636,6 +636,46 @@ QImage QFontEngine::alphaRGBMapForGlyph(glyph_t glyph, QFixed /*subPixelPosition
     return rgbMask;
 }
 
+QFixed QFontEngine::subPixelPositionForX(QFixed x)
+{
+    int m_subPixelPositionCount = 4;
+    if (!supportsSubPixelPositions())
+        return 0;
+
+    QFixed subPixelPosition;
+    if (x != 0) {
+        subPixelPosition = x - x.floor();
+        QFixed fraction = (subPixelPosition / QFixed::fromReal(1.0 / m_subPixelPositionCount)).floor();
+        subPixelPosition = fraction / QFixed(m_subPixelPositionCount);
+    }
+    return subPixelPosition;
+}
+
+QImage *QFontEngine::lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition,
+                                            QFontEngine::GlyphFormat neededFormat,
+                                            const QTransform &t, QPoint *offset)
+{
+    Q_ASSERT(currentlyLockedAlphaMap.isNull());
+    if (neededFormat == Format_None)
+        neededFormat = Format_A32;
+
+    if (neededFormat != Format_A32)
+        currentlyLockedAlphaMap = alphaMapForGlyph(glyph, subPixelPosition, t);
+    else
+        currentlyLockedAlphaMap = alphaRGBMapForGlyph(glyph, subPixelPosition, 0, 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)
 {
     glyph_metrics_t gm = boundingBox(glyph);

diff --git a/src/gui/text/qfontengine_ft.cpp b/src/gui/text/qfontengine_ft.cpp
index 1e7c3a2..a6fb0e8 100644 (file)
--- a/src/gui/text/qfontengine_ft.cpp
+++ b/src/gui/text/qfontengine_ft.cpp
@@ -44,6 +44,7 @@
 #include "qtextstream.h"
 #include "qvariant.h"
 #include "qfontengine_ft_p.h"
+#include "private/qimage_p.h"
 
 #ifndef QT_NO_FREETYPE
 
@@ -1368,21 +1369,6 @@ QFontEngineFT::QGlyphSet *QFontEngineFT::loadTransformedGlyphSet(const QTransfor
     return gs;
 }
 
-QFixed QFontEngineFT::subPixelPositionForX(QFixed x)
-{
-    int m_subPixelPositionCount = 4;
-    if (!supportsSubPixelPositions())
-        return 0;
-
-    QFixed subPixelPosition;
-    if (x != 0) {
-        subPixelPosition = x - x.floor();
-        QFixed fraction = (subPixelPosition / QFixed::fromReal(1.0 / m_subPixelPositionCount)).floor();
-        subPixelPosition = fraction / QFixed(m_subPixelPositionCount);
-    }
-    return subPixelPosition;
-}
-
 bool QFontEngineFT::loadGlyphs(QGlyphSet *gs, const glyph_t *glyphs, int num_glyphs,
                                const QFixedPoint *positions,
                                GlyphFormat format)
@@ -1811,6 +1797,91 @@ glyph_metrics_t QFontEngineFT::alphaMapBoundingBox(glyph_t glyph, QFixed subPixe
     return overall;
 }
 
+QImage *QFontEngineFT::lockedAlphaMapForGlyph(glyph_t glyphIndex, QFixed subPixelPosition,
+                                              QFontEngine::GlyphFormat neededFormat,
+                                              const QTransform &t, QPoint *offset)
+{
+    Q_ASSERT(currentlyLockedAlphaMap.isNull());
+    lockFace();
+
+    if (isBitmapFont())
+        neededFormat = Format_Mono;
+    else if (neededFormat == Format_None)
+        neededFormat = defaultFormat;
+
+    QFontEngineFT::QGlyphSet *gset = defaultGlyphs();
+    if (t.type() >= QTransform::TxScale) {
+        if (t.isAffine())
+            gset = loadTransformedGlyphSet(t);
+        else
+            gset = 0;
+    }
+
+    if (!gset || gset->outline_drawing || !loadGlyph(gset, glyphIndex, subPixelPosition,
+                                                     neededFormat)) {
+        unlockFace();
+        return QFontEngine::lockedAlphaMapForGlyph(glyphIndex, subPixelPosition, neededFormat, t,
+                                                   offset);
+    }
+
+    QImage::Format format;
+    switch (neededFormat) {
+    case QFontEngine::Format_Mono:
+        format = QImage::Format_Mono;
+        break;
+    case QFontEngine::Format_A8:
+        format = QImage::Format_Indexed8;
+        break;
+    case QFontEngine::Format_A32:
+        format = QImage::Format_ARGB32;
+        break;
+    default:
+        Q_ASSERT(false);
+        format = QImage::Format_Invalid;
+    };
+
+    QFontEngineFT::Glyph *glyph = gset->getGlyph(glyphIndex, subPixelPosition);
+    if (glyph == 0 || glyph->data == 0 || glyph->width == 0 || glyph->height == 0) {
+        unlockFace();
+        return 0;
+    }
+
+    int pitch;
+    switch (neededFormat) {
+    case QFontEngineFT::Format_Mono:
+        pitch = ((glyph->width + 31) & ~31) >> 3;
+        break;
+    case QFontEngineFT::Format_A8:
+        pitch = (glyph->width + 3) & ~3;
+        break;
+    case QFontEngineFT::Format_A32:
+        pitch = glyph->width * 4;
+        break;
+    default:
+        Q_ASSERT(false);
+        pitch = 0;
+    };
+
+    if (offset != 0)
+        *offset = QPoint(glyph->x, -glyph->y);
+
+
+    currentlyLockedAlphaMap = QImage(glyph->data, glyph->width, glyph->height, pitch, format);
+    Q_ASSERT(!currentlyLockedAlphaMap.isNull());
+
+    QImageData *data = currentlyLockedAlphaMap.data_ptr();
+    data->is_locked = true;
+
+    return &currentlyLockedAlphaMap;
+}
+
+void QFontEngineFT::unlockAlphaMapForGlyph()
+{
+    Q_ASSERT(!currentlyLockedAlphaMap.isNull());
+    unlockFace();
+    currentlyLockedAlphaMap = QImage();
+}
+
 QImage QFontEngineFT::alphaMapForGlyph(glyph_t g, QFixed subPixelPosition)
 {
     lockFace();

diff --git a/src/gui/text/qfontengine_ft_p.h b/src/gui/text/qfontengine_ft_p.h
index 62edaee..a236b79 100644 (file)
--- a/src/gui/text/qfontengine_ft_p.h
+++ b/src/gui/text/qfontengine_ft_p.h
@@ -265,6 +265,12 @@ private:
                                                 QFixed subPixelPosition,
                                                 const QTransform &matrix,
                                                 QFontEngine::GlyphFormat format);
+    virtual QImage *lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition,
+                                           GlyphFormat neededFormat, const QTransform &t,
+                                           QPoint *offset);
+    virtual bool hasInternalCaching() const { return true; }
+    virtual void unlockAlphaMapForGlyph();
+
     virtual void removeGlyphFromCache(glyph_t glyph);
 
     virtual int glyphCount() const;
@@ -292,7 +298,6 @@ private:
     inline Glyph *cachedGlyph(glyph_t g) const { return defaultGlyphSet.getGlyph(g, 0); }
 
     QGlyphSet *loadTransformedGlyphSet(const QTransform &matrix);
-    QFixed subPixelPositionForX(QFixed x);
     bool loadGlyphs(QGlyphSet *gs, const glyph_t *glyphs, int num_glyphs,
                     const QFixedPoint *positions,
                     GlyphFormat format = Format_Render);

diff --git a/src/gui/text/qfontengine_p.h b/src/gui/text/qfontengine_p.h
index a5858ff..f74302c 100644 (file)
--- a/src/gui/text/qfontengine_p.h
+++ b/src/gui/text/qfontengine_p.h
@@ -167,6 +167,7 @@ public:
     };
     virtual int synthesized() const { return 0; }
     virtual bool supportsSubPixelPositions() const { return false; }
+    QFixed subPixelPositionForX(QFixed x);
 
     virtual QFixed emSquareSize() const { return ascent(); }
 
@@ -195,11 +196,18 @@ public:
      * Create a qimage with the alpha values for the glyph.
      * Returns an image indexed_8 with index values ranging from 0=fully transparent to 255=opaque
      */
+    // ### Refactor this into a smaller and more flexible API.
     virtual QImage alphaMapForGlyph(glyph_t);
     virtual QImage alphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition);
     virtual QImage alphaMapForGlyph(glyph_t, const QTransform &t);
     virtual QImage alphaMapForGlyph(glyph_t, QFixed subPixelPosition, const QTransform &t);
     virtual QImage alphaRGBMapForGlyph(glyph_t, QFixed subPixelPosition, int margin, const QTransform &t);
+    virtual QImage *lockedAlphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition,
+                                           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*/)
     {
@@ -282,6 +290,7 @@ public:
 #endif
 
     int glyphFormat;
+    QImage currentlyLockedAlphaMap;
 
 protected:
     static const QVector<QRgb> &grayPalette();