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 3af98d6..d6cd98b 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 7f902a8..52f51fa 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 16ef09b..94828fb 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 eab02e1..f0db805 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 e90a12f..6c6781c 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 63fd9a7..054a57e 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)