Refactor paint/font-engine shouldDrawCachedGlyphs and supportsTransformations

Some cruft had built up over time, and this is an attempt at cleaning up
the naming and use of these functions, and should not have any behavioral
effects.

The function supportsTransformations() has been renamed in QPaintEngineEx
to reflect its use, which is to decide if QPainter needs to pre-transform
the coordinates of the static text before asking the paint-engine to draw
it. The new name is requiresPretransformedGlyphPositions().

The OpenGL and CoreGraphics (Mac) paint engines keep their behavior of
not needing pre-transformed text, while the raster engine needs this
when using cached glyphs. The base-class implementation assumes that
all transforms that include a projection will need pre-transform,
which is also the case for the raster engine.

All decisions in the paint engines about whether or not to use the
glyph cache when drawing text are now deferred to the function
shouldDrawCachedGlyphs(), which has been refactored for the GL paint
engine(s) to share more logic. All implementations call the base
class implementation, which ensures that large font sizes will not
be cached. The raster engine will in addition ask the font engine
whether or not it can produce glyphs for the glyph-cache with the
given transform.

This is the only remaining instance of the supportsTransformations()
function, and will for all font engines except the CoreText engine
support affine transformations. The CoreText engine on the other hand
only supports translations (for now).

Change-Id: I8fb5e43e3de3ef62a526a79a6dfeda7f9546771d
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@digia.com>

14 files changed, 73 insertions, 65 deletions

diff --git a/src/gui/opengl/qopenglpaintengine.cpp b/src/gui/opengl/qopenglpaintengine.cpp
index 624eeaffd9..6238564eeb 100644
--- a/src/gui/opengl/qopenglpaintengine.cpp
+++ b/src/gui/opengl/qopenglpaintengine.cpp
@@ -1407,11 +1407,9 @@ void QOpenGL2PaintEngineEx::drawStaticTextItem(QStaticTextItem *textItem)
     ensureActive();
 
     QPainterState *s = state();
-    float det = s->matrix.determinant();
 
-    // don't try to cache huge fonts or vastly transformed fonts
     QFontEngine *fontEngine = textItem->fontEngine();
-    if (shouldDrawCachedGlyphs(fontEngine, s->matrix) && det >= 0.25f && det <= 4.f) {
+    if (shouldDrawCachedGlyphs(fontEngine, s->matrix)) {
         QFontEngineGlyphCache::Type glyphType = fontEngine->glyphFormat >= 0
                                                 ? QFontEngineGlyphCache::Type(textItem->fontEngine()->glyphFormat)
                                                 : d->glyphCacheType;
@@ -1461,13 +1459,6 @@ void QOpenGL2PaintEngineEx::drawTextItem(const QPointF &p, const QTextItem &text
 
     QTransform::TransformationType txtype = s->matrix.type();
 
-    float det = s->matrix.determinant();
-    bool drawCached = txtype < QTransform::TxProject;
-
-    // don't try to cache huge fonts or vastly transformed fonts
-    if (!shouldDrawCachedGlyphs(ti.fontEngine, s->matrix) || det < 0.25f || det > 4.f)
-        drawCached = false;
-
     QFontEngineGlyphCache::Type glyphType = ti.fontEngine->glyphFormat >= 0
                                             ? QFontEngineGlyphCache::Type(ti.fontEngine->glyphFormat)
                                             : d->glyphCacheType;
@@ -1482,7 +1473,7 @@ void QOpenGL2PaintEngineEx::drawTextItem(const QPointF &p, const QTextItem &text
         }
     }
 
-    if (drawCached) {
+    if (shouldDrawCachedGlyphs(ti.fontEngine, s->matrix)) {
         QVarLengthArray<QFixedPoint> positions;
         QVarLengthArray<glyph_t> glyphs;
         QTransform matrix = QTransform::fromTranslate(p.x(), p.y());
@@ -1531,6 +1522,16 @@ namespace {
 
 // #define QT_OPENGL_DRAWCACHEDGLYPHS_INDEX_ARRAY_VBO
 
+bool QOpenGL2PaintEngineEx::shouldDrawCachedGlyphs(QFontEngine *fontEngine, const QTransform &t) const
+{
+    float det = t.determinant();
+
+    // Don't try to cache huge fonts or vastly transformed fonts
+    return t.type() < QTransform::TxProject
+        && QPaintEngineEx::shouldDrawCachedGlyphs(fontEngine, t)
+        && det >= 0.25f && det <= 4.f;
+}
+
 void QOpenGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyphType,
                                                 QStaticTextItem *staticTextItem)
 {
diff --git a/src/gui/opengl/qopenglpaintengine_p.h b/src/gui/opengl/qopenglpaintengine_p.h
index e06d9f561d..125db00238 100644
--- a/src/gui/opengl/qopenglpaintengine_p.h
+++ b/src/gui/opengl/qopenglpaintengine_p.h
@@ -157,7 +157,8 @@ public:
     void setRenderTextActive(bool);
 
     bool isNativePaintingActive() const;
-    bool supportsTransformations(QFontEngine *, const QTransform &) const { return true; }
+    bool requiresPretransformedGlyphPositions(QFontEngine *, const QTransform &) const { return false; }
+    bool shouldDrawCachedGlyphs(QFontEngine *, const QTransform &) const;
 
 private:
     Q_DISABLE_COPY(QOpenGL2PaintEngineEx)
diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp
index 0e9129f8c0..53fdd42b8a 100644
--- a/src/gui/painting/qpaintengine_raster.cpp
+++ b/src/gui/painting/qpaintengine_raster.cpp
@@ -3010,13 +3010,15 @@ void QRasterPaintEngine::drawStaticTextItem(QStaticTextItem *textItem)
     ensurePen();
     ensureRasterState();
 
+    QTransform matrix = state()->matrix;
+
     QFontEngine *fontEngine = textItem->fontEngine();
-    if (!supportsTransformations(fontEngine)) {
+    if (shouldDrawCachedGlyphs(fontEngine, matrix)) {
         drawCachedGlyphs(textItem->numGlyphs, textItem->glyphs, textItem->glyphPositions,
                          fontEngine);
-    } else if (state()->matrix.type() < QTransform::TxProject) {
+    } else if (matrix.type() < QTransform::TxProject) {
         bool invertible;
-        QTransform invMat = state()->matrix.inverted(&invertible);
+        QTransform invMat = matrix.inverted(&invertible);
         if (!invertible)
             return;
 
@@ -3055,7 +3057,7 @@ void QRasterPaintEngine::drawTextItem(const QPointF &p, const QTextItem &textIte
     QRasterPaintEngineState *s = state();
     QTransform matrix = s->matrix;
 
-    if (!supportsTransformations(ti.fontEngine)) {
+    if (shouldDrawCachedGlyphs(ti.fontEngine, matrix)) {
         QVarLengthArray<QFixedPoint> positions;
         QVarLengthArray<glyph_t> glyphs;
 
@@ -3299,21 +3301,25 @@ void QRasterPaintEngine::releaseDC(HDC) const
 /*!
     \internal
 */
-bool QRasterPaintEngine::supportsTransformations(QFontEngine *fontEngine) const
+bool QRasterPaintEngine::requiresPretransformedGlyphPositions(QFontEngine *fontEngine, const QTransform &m) const
 {
-    const QTransform &m = state()->matrix;
-    return supportsTransformations(fontEngine, m);
+    // Cached glyphs always require pretransformed positions
+    if (shouldDrawCachedGlyphs(fontEngine, m))
+        return true;
+
+    // Otherwise let the base-class decide based on the transform
+    return QPaintEngineEx::requiresPretransformedGlyphPositions(fontEngine, m);
 }
 
-/*!
-    \internal
-*/
-bool QRasterPaintEngine::supportsTransformations(QFontEngine *fontEngine, const QTransform &m) const
+bool QRasterPaintEngine::shouldDrawCachedGlyphs(QFontEngine *fontEngine, const QTransform &m) const
 {
-    if (fontEngine->supportsTransformations(m))
-        return true;
+    // The font engine might not support filling the glyph cache
+    // with the given transform applied, in which case we need to
+    // fall back to the QPainterPath code-path.
+    if (!fontEngine->supportsTransformation(m))
+        return false;
 
-    return !shouldDrawCachedGlyphs(fontEngine, m);
+    return QPaintEngineEx::shouldDrawCachedGlyphs(fontEngine, m);
 }
 
 /*!
diff --git a/src/gui/painting/qpaintengine_raster_p.h b/src/gui/painting/qpaintengine_raster_p.h
index 8fb72edabd..440ddd9ef9 100644
--- a/src/gui/painting/qpaintengine_raster_p.h
+++ b/src/gui/painting/qpaintengine_raster_p.h
@@ -231,8 +231,8 @@ public:
 
     QPoint coordinateOffset() const;
 
-    bool supportsTransformations(QFontEngine *fontEngine) const;
-    bool supportsTransformations(QFontEngine *fontEngine, const QTransform &m) const;
+    bool requiresPretransformedGlyphPositions(QFontEngine *fontEngine, const QTransform &m) const;
+    bool shouldDrawCachedGlyphs(QFontEngine *fontEngine, const QTransform &m) const;
 
 protected:
     QRasterPaintEngine(QRasterPaintEnginePrivate &d, QPaintDevice *);
diff --git a/src/gui/painting/qpaintengineex.cpp b/src/gui/painting/qpaintengineex.cpp
index 2c41ab9ff2..872197ab75 100644
--- a/src/gui/painting/qpaintengineex.cpp
+++ b/src/gui/painting/qpaintengineex.cpp
@@ -1081,14 +1081,9 @@ void QPaintEngineEx::drawStaticTextItem(QStaticTextItem *staticTextItem)
     }
 }
 
-bool QPaintEngineEx::supportsTransformations(QFontEngine *fontEngine, const QTransform &m) const
+bool QPaintEngineEx::requiresPretransformedGlyphPositions(QFontEngine *, const QTransform &t) const
 {
-    Q_UNUSED(fontEngine);
-
-    if (!m.isAffine())
-        return true;
-
-    return false;
+    return t.type() >= QTransform::TxProject;
 }
 
 bool QPaintEngineEx::shouldDrawCachedGlyphs(QFontEngine *fontEngine, const QTransform &m) const
diff --git a/src/gui/painting/qpaintengineex_p.h b/src/gui/painting/qpaintengineex_p.h
index de43b7782c..257bc7a65a 100644
--- a/src/gui/painting/qpaintengineex_p.h
+++ b/src/gui/painting/qpaintengineex_p.h
@@ -164,7 +164,7 @@ public:
         IsEmulationEngine = 0x02    // If set, this object is a QEmulationEngine.
     };
     virtual uint flags() const {return 0;}
-    virtual bool supportsTransformations(QFontEngine *fontEngine, const QTransform &m) const;
+    virtual bool requiresPretransformedGlyphPositions(QFontEngine *fontEngine, const QTransform &m) const;
     virtual bool shouldDrawCachedGlyphs(QFontEngine *fontEngine, const QTransform &m) const;
 
 protected:
diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp
index 9d5d4ebc95..1a97839ef8 100644
--- a/src/gui/painting/qpainter.cpp
+++ b/src/gui/painting/qpainter.cpp
@@ -5552,13 +5552,13 @@ void QPainter::drawGlyphRun(const QPointF &position, const QGlyphRun &glyphRun)
     QVarLengthArray<QFixedPoint, 128> fixedPointPositions(count);
 
     QRawFontPrivate *fontD = QRawFontPrivate::get(font);
-    bool supportsTransformations = d->extended
-        ? d->extended->supportsTransformations(fontD->fontEngine, d->state->matrix)
-        : d->engine->type() == QPaintEngine::CoreGraphics || d->state->matrix.isAffine();
+    bool engineRequiresPretransformedGlyphPositions = d->extended
+        ? d->extended->requiresPretransformedGlyphPositions(fontD->fontEngine, d->state->matrix)
+        : d->engine->type() != QPaintEngine::CoreGraphics && !d->state->matrix.isAffine();
 
     for (int i=0; i<count; ++i) {
         QPointF processedPosition = position + glyphPositions[i];
-        if (!supportsTransformations)
+        if (engineRequiresPretransformedGlyphPositions)
             processedPosition = d->state->transform().map(processedPosition);
         fixedPointPositions[i] = QFixedPoint::fromPointF(processedPosition);
     }
@@ -5741,14 +5741,18 @@ void QPainter::drawStaticText(const QPointF &topLeftPosition, const QStaticText
     QFontEngine *fe = staticText_d->font.d->engineForScript(QChar::Script_Common);
     if (fe->type() == QFontEngine::Multi)
         fe = static_cast<QFontEngineMulti *>(fe)->engine(0);
-    bool supportsTransformations = d->extended->supportsTransformations(fe,
-                                                                        d->state->matrix);
-    if (supportsTransformations && !staticText_d->untransformedCoordinates) {
-        staticText_d->untransformedCoordinates = true;
-        staticText_d->needsRelayout = true;
-    } else if (!supportsTransformations && staticText_d->untransformedCoordinates) {
+
+    bool engineRequiresPretransform = d->extended->requiresPretransformedGlyphPositions(fe, d->state->matrix);
+    if (staticText_d->untransformedCoordinates && engineRequiresPretransform) {
+        // The coordinates are untransformed, and the engine can't deal with that
+        // nativly, so we have to pre-transform the static text.
         staticText_d->untransformedCoordinates = false;
         staticText_d->needsRelayout = true;
+    } else if (!staticText_d->untransformedCoordinates && !engineRequiresPretransform) {
+        // The coordinates are already transformed, but the engine can handle that
+        // nativly, so undo the transform of the static text.
+        staticText_d->untransformedCoordinates = true;
+        staticText_d->needsRelayout = true;
     }
 
     // Don't recalculate entire layout because of translation, rather add the dx and dy
diff --git a/src/gui/text/qfontengine.cpp b/src/gui/text/qfontengine.cpp
index 0f0ad290fd..0b727619cd 100644
--- a/src/gui/text/qfontengine.cpp
+++ b/src/gui/text/qfontengine.cpp
@@ -268,9 +268,9 @@ QFixed QFontEngine::averageCharWidth() const
     return bb.xoff;
 }
 
-bool QFontEngine::supportsTransformations(const QTransform &transform) const
+bool QFontEngine::supportsTransformation(const QTransform &transform) const
 {
-    return (transform.type() >= QTransform::TxProject);
+    return transform.type() <= QTransform::TxProject;
 }
 
 void QFontEngine::getGlyphPositions(const QGlyphLayout &glyphs, const QTransform &matrix, QTextItem::RenderFlags flags,
diff --git a/src/gui/text/qfontengine_p.h b/src/gui/text/qfontengine_p.h
index 3321ca3b93..741b1c2bb7 100644
--- a/src/gui/text/qfontengine_p.h
+++ b/src/gui/text/qfontengine_p.h
@@ -241,7 +241,7 @@ public:
         return canRender(utf16, utf16len);
     }
 
-    virtual bool supportsTransformations(const QTransform &transform) const;
+    virtual bool supportsTransformation(const QTransform &transform) const;
 
     virtual Type type() const = 0;
 
diff --git a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
index 7284efa53b..ebff94da77 100644
--- a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
+++ b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp
@@ -626,6 +626,16 @@ bool QGL2PaintEngineEx::isNativePaintingActive() const {
     return d->nativePaintingActive;
 }
 
+bool QGL2PaintEngineEx::shouldDrawCachedGlyphs(QFontEngine *fontEngine, const QTransform &t) const
+{
+    float det = t.determinant();
+
+    // Don't try to cache huge fonts or vastly transformed fonts
+    return t.type() < QTransform::TxProject
+        && QPaintEngineEx::shouldDrawCachedGlyphs(fontEngine, t)
+        && det >= 0.25f && det <= 4.f;
+}
+
 void QGL2PaintEngineExPrivate::transferMode(EngineMode newMode)
 {
     if (newMode == mode)
@@ -1439,11 +1449,10 @@ void QGL2PaintEngineEx::drawStaticTextItem(QStaticTextItem *textItem)
     ensureActive();
 
     QPainterState *s = state();
-    float det = s->matrix.determinant();
 
     // don't try to cache huge fonts or vastly transformed fonts
     QFontEngine *fontEngine = textItem->fontEngine();
-    if (shouldDrawCachedGlyphs(fontEngine, s->matrix) && det >= 0.25f && det <= 4.f) {
+    if (shouldDrawCachedGlyphs(fontEngine, s->matrix)) {
         QFontEngineGlyphCache::Type glyphType = fontEngine->glyphFormat >= 0
                                                 ? QFontEngineGlyphCache::Type(textItem->fontEngine()->glyphFormat)
                                                 : d->glyphCacheType;
@@ -1497,13 +1506,6 @@ void QGL2PaintEngineEx::drawTextItem(const QPointF &p, const QTextItem &textItem
 
     QTransform::TransformationType txtype = s->matrix.type();
 
-    float det = s->matrix.determinant();
-    bool drawCached = txtype < QTransform::TxProject;
-
-    // don't try to cache huge fonts or vastly transformed fonts
-    if (!shouldDrawCachedGlyphs(ti.fontEngine, s->matrix) || det < 0.25f || det > 4.f)
-        drawCached = false;
-
     QFontEngineGlyphCache::Type glyphType = ti.fontEngine->glyphFormat >= 0
                                             ? QFontEngineGlyphCache::Type(ti.fontEngine->glyphFormat)
                                             : d->glyphCacheType;
@@ -1519,7 +1521,7 @@ void QGL2PaintEngineEx::drawTextItem(const QPointF &p, const QTextItem &textItem
         }
     }
 
-    if (drawCached) {
+    if (shouldDrawCachedGlyphs(ti.fontEngine, s->matrix)) {
         QVarLengthArray<QFixedPoint> positions;
         QVarLengthArray<glyph_t> glyphs;
         QTransform matrix = QTransform::fromTranslate(p.x(), p.y());
diff --git a/src/opengl/gl2paintengineex/qpaintengineex_opengl2_p.h b/src/opengl/gl2paintengineex/qpaintengineex_opengl2_p.h
index f2dc5af768..dd09046bc9 100644
--- a/src/opengl/gl2paintengineex/qpaintengineex_opengl2_p.h
+++ b/src/opengl/gl2paintengineex/qpaintengineex_opengl2_p.h
@@ -155,7 +155,8 @@ public:
     void setRenderTextActive(bool);
 
     bool isNativePaintingActive() const;
-    bool supportsTransformations(QFontEngine *, const QTransform &) const { return true; }
+    bool requiresPretransformedGlyphPositions(QFontEngine *, const QTransform &) const { return false; }
+    bool shouldDrawCachedGlyphs(QFontEngine *, const QTransform &) const;
 private:
     Q_DISABLE_COPY(QGL2PaintEngineEx)
 };
diff --git a/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm b/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
index 1087d18891..ace4c982fd 100644
--- a/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
+++ b/src/platformsupport/fontdatabases/mac/qfontengine_coretext.mm
@@ -600,9 +600,9 @@ QFontEngine *QCoreTextFontEngine::cloneWithSize(qreal pixelSize) const
     return new QCoreTextFontEngine(cgFont, newFontDef);
 }
 
-bool QCoreTextFontEngine::supportsTransformations(const QTransform &transform) const
+bool QCoreTextFontEngine::supportsTransformation(const QTransform &transform) const
 {
-    return transform.type() > QTransform::TxTranslate;
+    return transform.type() <= QTransform::TxTranslate;
 }
 
 QT_END_NAMESPACE
diff --git a/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h b/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
index 6c5c557b13..bd52234f41 100644
--- a/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
+++ b/src/platformsupport/fontdatabases/mac/qfontengine_coretext_p.h
@@ -103,7 +103,7 @@ public:
     virtual qreal minLeftBearing() const;
     virtual QFixed emSquareSize() const;
 
-    bool supportsTransformations(const QTransform &transform) const;
+    bool supportsTransformation(const QTransform &transform) const;
 
     virtual QFontEngine *cloneWithSize(qreal pixelSize) const;
     virtual int glyphMargin(QFontEngineGlyphCache::Type type) { Q_UNUSED(type); return 0; }
diff --git a/src/plugins/platforms/cocoa/qpaintengine_mac_p.h b/src/plugins/platforms/cocoa/qpaintengine_mac_p.h
index 120ceae5c6..ef9878a98a 100644
--- a/src/plugins/platforms/cocoa/qpaintengine_mac_p.h
+++ b/src/plugins/platforms/cocoa/qpaintengine_mac_p.h
@@ -122,8 +122,6 @@ public:
     void drawPolygon(const QPoint *points, int pointCount, PolygonDrawMode mode)
     { QPaintEngine::drawPolygon(points, pointCount, mode); }
 
-    bool supportsTransformations(qreal, const QTransform &) const { return true; };
-
 protected:
     friend class QMacPrintEngine;
     friend class QMacPrintEnginePrivate;