Fixed remaining issues in subpixel positioning with FreeType

1. Merge QStaticText and QGlyphs drawing code of raster engine
   into glyph drawing code for normal QTextItem, it simplify
   the caching system (we don't use QImageTextureGlyphCache for
   raster anymore, just glyph set cache in FreeType engine is
   enough), and fix some regressions in QStaticText and QGlyphs
   drawing.

2. Fix subpixel positioning support for OpenGL texture glyph
   cache. Including a transform handling regression introduced
   by the initial patch.

3. Disable subpixel positioning for bitmap fonts (mono format).

After this change, we only have two code paths for glyph rendering
with FreeType: raster will always use the simple code path (with
QFontEngine_FT cache) while GL will always use QTextureGlyphCache.

Reviewed-by: Eskil

6 files changed, 142 insertions, 142 deletions

diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp
index 3af98d626a..d6cd98b3d2 100644
--- a/src/gui/painting/qpaintengine_raster.cpp
+++ b/src/gui/painting/qpaintengine_raster.cpp
@@ -3076,65 +3076,147 @@ void QRasterPaintEngine::alphaPenBlt(const void* src, int bpl, int depth, int rx
         blend(current, spans, &s->penData);
 }
 
-void QRasterPaintEngine::drawCachedGlyphs(int numGlyphs, const glyph_t *glyphs,
+bool QRasterPaintEngine::drawCachedGlyphs(int numGlyphs, const glyph_t *glyphs,
                                           const QFixedPoint *positions, QFontEngine *fontEngine)
 {
     Q_D(QRasterPaintEngine);
     QRasterPaintEngineState *s = state();
 
-    QFontEngineGlyphCache::Type glyphType = fontEngine->glyphFormat >= 0 ? QFontEngineGlyphCache::Type(fontEngine->glyphFormat) : d->glyphCacheType;
+#if !defined(QT_NO_FREETYPE)
+    if (fontEngine->type() == QFontEngine::Freetype) {
+        QFontEngineFT *fe = static_cast<QFontEngineFT *>(fontEngine);
+        QFontEngineFT::GlyphFormat neededFormat = fe->defaultGlyphFormat();
 
-    QImageTextureGlyphCache *cache =
-        static_cast<QImageTextureGlyphCache *>(fontEngine->glyphCache(0, glyphType, s->matrix));
-    if (!cache) {
-        cache = new QImageTextureGlyphCache(glyphType, s->matrix);
-        fontEngine->setGlyphCache(0, cache);
-    }
+        if (d_func()->mono_surface
+            || fe->isBitmapFont() // alphaPenBlt can handle mono, too
+            )
+            neededFormat = QFontEngineFT::Format_Mono;
 
-    cache->populate(fontEngine, numGlyphs, glyphs, positions);
-    cache->fillInPendingGlyphs();
+        if (neededFormat == QFontEngineFT::Format_None)
+            neededFormat = QFontEngineFT::Format_A8;
 
-    const QImage &image = cache->image();
-    int bpl = image.bytesPerLine();
+        QFontEngineFT::QGlyphSet *gset = fe->defaultGlyphs();
+        if (s->matrix.type() >= QTransform::TxScale) {
+            if (s->matrix.isAffine())
+                gset = fe->loadTransformedGlyphSet(s->matrix);
+            else
+                gset = 0;
+        }
 
-    int depth = image.depth();
-    int rightShift = 0;
-    int leftShift = 0;
-    if (depth == 32)
-        leftShift = 2; // multiply by 4
-    else if (depth == 1)
-        rightShift = 3; // divide by 8
+        if (!gset || gset->outline_drawing
+            || !fe->loadGlyphs(gset, glyphs, numGlyphs, positions, neededFormat))
+            return false;
 
-    int margin = cache->glyphMargin();
+        FT_Face lockedFace = 0;
 
-    bool supportsSubPixelPositions = fontEngine->supportsSubPixelPositions();
+        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;
+        };
 
-    const uchar *bits = image.bits();
-    for (int i=0; i<numGlyphs; ++i) {
+        for (int i = 0; i < numGlyphs; i++) {
+            QFixed spp = fe->subPixelPositionForX(positions[i].x);
+            QFontEngineFT::Glyph *glyph = gset->getGlyph(glyphs[i], spp);
 
-        QFixed subPixelPosition;
-        if (supportsSubPixelPositions)
-            subPixelPosition = cache->subPixelPositionForX(positions[i].x);
-        QTextureGlyphCache::GlyphAndSubPixelPosition glyph(glyphs[i], subPixelPosition);
-        const QTextureGlyphCache::Coord &c = cache->coords[glyph];
-        if (c.isNull())
-            continue;
+            if (!glyph || glyph->format != neededFormat) {
+                if (!lockedFace)
+                    lockedFace = fe->lockFace();
+                glyph = fe->loadGlyph(gset, glyphs[i], spp, neededFormat);
+            }
+
+            if (!glyph || !glyph->data)
+                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);
+        }
+        if (lockedFace)
+            fe->unlockFace();
+    } else
+#endif
+    {
+        QFontEngineGlyphCache::Type glyphType = fontEngine->glyphFormat >= 0 ? QFontEngineGlyphCache::Type(fontEngine->glyphFormat) : d->glyphCacheType;
 
-        int x = qFloor(positions[i].x) + c.baseLineX - margin;
-        int y = qFloor(positions[i].y) - c.baseLineY - margin;
+        QImageTextureGlyphCache *cache =
+            static_cast<QImageTextureGlyphCache *>(fontEngine->glyphCache(0, glyphType, s->matrix));
+        if (!cache) {
+            cache = new QImageTextureGlyphCache(glyphType, s->matrix);
+            fontEngine->setGlyphCache(0, cache);
+        }
 
-//         printf("drawing [%d %d %d %d] baseline [%d %d], glyph: %d, to: %d %d, pos: %d %d\n",
-//                c.x, c.y,
-//                c.w, c.h,
-//                c.baseLineX, c.baseLineY,
-//                glyphs[i],
-//                x, y,
-//                positions[i].x.toInt(), positions[i].y.toInt());
+        cache->populate(fontEngine, numGlyphs, glyphs, positions);
+        cache->fillInPendingGlyphs();
 
-        alphaPenBlt(bits + ((c.x << leftShift) >> rightShift) + c.y * bpl, bpl, depth, x, y, c.w, c.h);
-    }
+        const QImage &image = cache->image();
+        int bpl = image.bytesPerLine();
 
-    return;
+        int depth = image.depth();
+        int rightShift = 0;
+        int leftShift = 0;
+        if (depth == 32)
+            leftShift = 2; // multiply by 4
+        else if (depth == 1)
+            rightShift = 3; // divide by 8
+
+        int margin = cache->glyphMargin();
+
+        bool supportsSubPixelPositions = fontEngine->supportsSubPixelPositions();
+
+        const uchar *bits = image.bits();
+        for (int i=0; i<numGlyphs; ++i) {
+
+            QFixed subPixelPosition;
+            if (supportsSubPixelPositions)
+                subPixelPosition = cache->subPixelPositionForX(positions[i].x);
+            QTextureGlyphCache::GlyphAndSubPixelPosition glyph(glyphs[i], subPixelPosition);
+            const QTextureGlyphCache::Coord &c = cache->coords[glyph];
+            if (c.isNull())
+                continue;
+
+            int x = qFloor(positions[i].x) + c.baseLineX - margin;
+            int y = qFloor(positions[i].y) - c.baseLineY - margin;
+
+            // printf("drawing [%d %d %d %d] baseline [%d %d], glyph: %d, to: %d %d, pos: %d %d\n",
+            //        c.x, c.y,
+            //        c.w, c.h,
+            //        c.baseLineX, c.baseLineY,
+            //        glyphs[i],
+            //        x, y,
+            //        positions[i].x.toInt(), positions[i].y.toInt());
+
+            alphaPenBlt(bits + ((c.x << leftShift) >> rightShift) + c.y * bpl, bpl, depth, x, y, c.w, c.h);
+        }
+    }
+    return true;
 }
 
 #if defined(Q_OS_SYMBIAN) && defined(QT_NO_FREETYPE)
@@ -3445,92 +3527,10 @@ void QRasterPaintEngine::drawTextItem(const QPointF &p, const QTextItem &textIte
     if (glyphs.size() == 0)
         return;
 
-    // only use subpixel antialiasing when drawing to widgets
-    QFontEngineFT::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.data(), glyphs.size(), positions, neededFormat))
-    {
+    if (!drawCachedGlyphs(glyphs.size(), glyphs.constData(), positions.constData(), ti.fontEngine))
         QPaintEngine::drawTextItem(p, ti);
-        return;
-    }
-
-    FT_Face lockedFace = 0;
-
-    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;
-    };
 
-    for(int i = 0; i < glyphs.size(); 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);
-        }
-
-        if (!glyph || !glyph->data)
-            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);
-    }
-    if (lockedFace)
-        fe->unlockFace();
     return;
-
 #endif
 #endif
 
diff --git a/src/gui/painting/qpaintengine_raster_p.h b/src/gui/painting/qpaintengine_raster_p.h
index 7f902a8ceb..52f51fab16 100644
--- a/src/gui/painting/qpaintengine_raster_p.h
+++ b/src/gui/painting/qpaintengine_raster_p.h
@@ -261,7 +261,7 @@ private:
     void fillRect(const QRectF &rect, QSpanData *data);
     void drawBitmap(const QPointF &pos, const QImage &image, QSpanData *fill);
 
-    void drawCachedGlyphs(int numGlyphs, const glyph_t *glyphs, const QFixedPoint *positions,
+    bool drawCachedGlyphs(int numGlyphs, const glyph_t *glyphs, const QFixedPoint *positions,
                           QFontEngine *fontEngine);
 
 #if defined(Q_OS_SYMBIAN) && defined(QT_NO_FREETYPE)
diff --git a/src/gui/painting/qpaintengine_x11.cpp b/src/gui/painting/qpaintengine_x11.cpp
index 16ef09bb45..94828fba53 100644
--- a/src/gui/painting/qpaintengine_x11.cpp
+++ b/src/gui/painting/qpaintengine_x11.cpp
@@ -2385,7 +2385,7 @@ void QX11PaintEngine::drawFreetype(const QPointF &p, const QTextItemInt &ti)
         set = ft->loadTransformedGlyphSet(d->matrix);
 
     if (!set || set->outline_drawing
-        || !ft->loadGlyphs(set, glyphs.data(), glyphs.size(), positions, QFontEngineFT::Format_Render))
+        || !ft->loadGlyphs(set, glyphs.constData(), glyphs.size(), positions.constData(), QFontEngineFT::Format_Render))
     {
         QPaintEngine::drawTextItem(p, ti);
         return;
diff --git a/src/gui/painting/qtextureglyphcache.cpp b/src/gui/painting/qtextureglyphcache.cpp
index eab02e1d25..f0db805e1a 100644
--- a/src/gui/painting/qtextureglyphcache.cpp
+++ b/src/gui/painting/qtextureglyphcache.cpp
@@ -316,11 +316,11 @@ QImage QTextureGlyphCache::textureMapForGlyph(glyph_t g, QFixed subPixelPosition
 
         QFontEngineFT *ft = static_cast<QFontEngineFT*> (m_current_fontengine);
         QFontEngineFT::QGlyphSet *gset = ft->loadTransformedGlyphSet(m_transform);
-        QFontEngineFT::Glyph *glyph = NULL;
-        if (gset)
-            glyph = ft->loadGlyph(gset, g, subPixelPosition, format);
+        QFixedPoint positions[1];
+        positions[0].x = subPixelPosition;
 
-        if (glyph) {
+        if (gset && ft->loadGlyphs(gset, &g, 1, positions, format)) {
+            QFontEngineFT::Glyph *glyph = gset->getGlyph(g, subPixelPosition);
             const int bytesPerLine = (format == QFontEngineFT::Format_Mono ? ((glyph->width + 31) & ~31) >> 3
                                : (glyph->width + 3) & ~3);
             return QImage(glyph->data, glyph->width, glyph->height, bytesPerLine, imageFormat);
@@ -367,7 +367,7 @@ void QImageTextureGlyphCache::createTextureData(int width, int height)
 
 int QImageTextureGlyphCache::glyphMargin() const
 {
-#if defined(Q_WS_MAC) && defined(QT_MAC_USE_COCOA)
+#if (defined(Q_WS_MAC) && defined(QT_MAC_USE_COCOA)) || defined(Q_WS_X11)
     return 0;
 #else
     return m_type == QFontEngineGlyphCache::Raster_RGBMask ? 2 : 0;
diff --git a/src/gui/text/qfontengine_ft.cpp b/src/gui/text/qfontengine_ft.cpp
index e90a12f447..6c6781cd17 100644
--- a/src/gui/text/qfontengine_ft.cpp
+++ b/src/gui/text/qfontengine_ft.cpp
@@ -944,7 +944,7 @@ QFontEngineFT::Glyph *QFontEngineFT::loadGlyph(QGlyphSet *set, uint glyph,
     FT_Face face = freetype->face;
 
     FT_Vector v;
-    v.x = FT_Pos((subPixelPosition).toReal() * 64);
+    v.x = format == Format_Mono ? 0 : FT_Pos(subPixelPosition.toReal() * 64);
     v.y = 0;
     FT_Set_Transform(face, &freetype->matrix, &v);
 
@@ -1049,7 +1049,7 @@ QFontEngineFT::Glyph *QFontEngineFT::loadGlyph(QGlyphSet *set, uint glyph,
 
     int hpixels = TRUNC(right - left);
     // subpixel position requires one more pixel
-    if (subPixelPosition > 0)
+    if (subPixelPosition > 0 && format != Format_Mono)
         hpixels++;
 
     if (hsubpixel)
@@ -1440,8 +1440,8 @@ QFixed QFontEngineFT::subPixelPositionForX(QFixed x)
     return subPixelPosition;
 }
 
-bool QFontEngineFT::loadGlyphs(QGlyphSet *gs, glyph_t *glyphs, int num_glyphs,
-                               QVarLengthArray<QFixedPoint> &positions,
+bool QFontEngineFT::loadGlyphs(QGlyphSet *gs, const glyph_t *glyphs, int num_glyphs,
+                               const QFixedPoint *positions,
                                GlyphFormat format)
 {
     FT_Face face = 0;
@@ -1866,16 +1866,16 @@ glyph_metrics_t QFontEngineFT::alphaMapBoundingBox(glyph_t glyph, const QTransfo
     return overall;
 }
 
-QImage QFontEngineFT::alphaMapForGlyph(glyph_t g)
+QImage QFontEngineFT::alphaMapForGlyph(glyph_t g, QFixed subPixelPosition)
 {
     lockFace();
 
     GlyphFormat glyph_format = antialias ? Format_A8 : Format_Mono;
 
-    Glyph *glyph = defaultGlyphSet.outline_drawing ? 0 : loadGlyph(g, 0, glyph_format);
+    Glyph *glyph = defaultGlyphSet.outline_drawing ? 0 : loadGlyph(g, subPixelPosition, glyph_format);
     if (!glyph) {
         unlockFace();
-        return QFontEngine::alphaMapForGlyph(g);
+        return QFontEngine::alphaMapForGlyph(g, subPixelPosition);
     }
 
     const int pitch = antialias ? (glyph->width + 3) & ~3 : ((glyph->width + 31)/32) * 4;
diff --git a/src/gui/text/qfontengine_ft_p.h b/src/gui/text/qfontengine_ft_p.h
index 63fd9a7634..054a57e035 100644
--- a/src/gui/text/qfontengine_ft_p.h
+++ b/src/gui/text/qfontengine_ft_p.h
@@ -254,7 +254,7 @@ private:
     virtual glyph_metrics_t boundingBox(glyph_t glyph, const QTransform &matrix);
 
     virtual void recalcAdvances(QGlyphLayout *glyphs, QTextEngine::ShaperFlags flags) const;
-    virtual QImage alphaMapForGlyph(glyph_t);
+    virtual QImage alphaMapForGlyph(glyph_t, QFixed);
     virtual QImage alphaRGBMapForGlyph(glyph_t, QFixed subPixelPosition, int margin, const QTransform &t);
     virtual glyph_metrics_t alphaMapBoundingBox(glyph_t glyph, const QTransform &matrix,
                                                 QFontEngine::GlyphFormat format);
@@ -286,8 +286,8 @@ private:
 
     QGlyphSet *loadTransformedGlyphSet(const QTransform &matrix);
     QFixed subPixelPositionForX(QFixed x);
-    bool loadGlyphs(QGlyphSet *gs, glyph_t *glyphs, int num_glyphs,
-                    QVarLengthArray<QFixedPoint> &positions,
+    bool loadGlyphs(QGlyphSet *gs, const glyph_t *glyphs, int num_glyphs,
+                    const QFixedPoint *positions,
                     GlyphFormat format = Format_Render);
 
 #if defined(Q_WS_QWS) || defined(Q_OS_SYMBIAN)