Merge QFontEngineMultiBasicImpl and QFontEngineMulti

Minor refactoring also improves the behavior and the code readability.

Change-Id: Id89dd224f4132a4c0dfbc16f414ef42cc8d8a4da
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@digia.com>

src/gui/text/qfontdatabase.h

@@ -159,7 +159,7 @@ private:
     friend class QFontPrivate;
     friend class QFontDialog;
     friend class QFontDialogPrivate;
-    friend class QFontEngineMultiBasicImpl;
+    friend class QFontEngineMulti;
 
     QFontDatabasePrivate *d;
 };

src/gui/text/qfontengine.cpp

@@ -1554,34 +1554,103 @@ static inline uchar highByte(glyph_t glyph)
 static inline glyph_t stripped(glyph_t glyph)
 { return glyph & 0x00ffffff; }
 
-QFontEngineMulti::QFontEngineMulti(int engineCount)
+QFontEngineMulti::QFontEngineMulti(QFontEngine *engine, int script, const QStringList &fallbackFamilies)
-    : QFontEngine(Multi)
+    : QFontEngine(Multi),
+      m_fallbackFamilies(fallbackFamilies),
+      m_script(script),
+      m_fallbackFamiliesQueried(!m_fallbackFamilies.isEmpty())
 {
-    engines.fill(0, engineCount);
+    Q_ASSERT(engine && engine->type() != QFontEngine::Multi);
-    cache_cost = 0;
+
+    if (m_fallbackFamilies.isEmpty()) {
+        // defer obtaining the fallback families until loadEngine(1)
+        m_fallbackFamilies << QString();
+    }
+
+    m_engines.fill(0, m_fallbackFamilies.size() + 1);
+
+    engine->ref.ref();
+    m_engines[0] = engine;
+
+    fontDef = engine->fontDef;
+    cache_cost = engine->cache_cost;
 }
 
 QFontEngineMulti::~QFontEngineMulti()
 {
-    for (int i = 0; i < engines.size(); ++i) {
+    for (int i = 0; i < m_engines.size(); ++i) {
-        QFontEngine *fontEngine = engines.at(i);
+        QFontEngine *fontEngine = m_engines.at(i);
         if (fontEngine && !fontEngine->ref.deref())
             delete fontEngine;
     }
 }
 
+void QFontEngineMulti::ensureFallbackFamiliesQueried()
+{
+    if (QPlatformIntegration *integration = QGuiApplicationPrivate::platformIntegration()) {
+        const QStringList fallbackFamilies = integration->fontDatabase()->fallbacksForFamily(fontDef.family,
+                                                                                             QFont::Style(fontDef.style),
+                                                                                             QFont::AnyStyle,
+                                                                                             QChar::Script(m_script));
+        setFallbackFamiliesList(fallbackFamilies);
+    }
+}
+
+void QFontEngineMulti::setFallbackFamiliesList(const QStringList &fallbackFamilies)
+{
+    Q_ASSERT(!m_fallbackFamiliesQueried);
+
+    m_fallbackFamilies = fallbackFamilies;
+
+    m_engines.resize(m_fallbackFamilies.size() + 1);
+
+    if (m_fallbackFamilies.isEmpty()) {
+        // turns out we lied about having any fallback at all
+        QFontEngine *engine = m_engines.at(0);
+        engine->ref.ref();
+        m_engines[1] = engine;
+        m_fallbackFamilies << fontDef.family;
+    }
+
+    m_fallbackFamiliesQueried = true;
+}
+
+void QFontEngineMulti::ensureEngineAt(int at)
+{
+    Q_ASSERT(m_fallbackFamiliesQueried);
+    Q_ASSERT(at < m_engines.size());
+    if (!m_engines.at(at)) {
+        QFontEngine *engine = loadEngine(at);
+        if (!engine)
+            engine = new QFontEngineBox(fontDef.pixelSize);
+        Q_ASSERT(engine && engine->type() != QFontEngine::Multi);
+        engine->ref.ref();
+        m_engines[at] = engine;
+    }
+}
+
+QFontEngine *QFontEngineMulti::loadEngine(int at)
+{
+    QFontDef request(fontDef);
+    request.styleStrategy |= QFont::NoFontMerging;
+    request.family = fallbackFamilyAt(at - 1);
+
+    return QFontDatabase::findFont(m_script, /*fontprivate = */0, request, /*multi = */false);
+}
+
 glyph_t QFontEngineMulti::glyphIndex(uint ucs4) const
 {
     glyph_t glyph = engine(0)->glyphIndex(ucs4);
     if (glyph == 0 && ucs4 != QChar::LineSeparator) {
-        const_cast<QFontEngineMulti *>(this)->ensureFallbackFamiliesQueried();
+        if (!m_fallbackFamiliesQueried)
-        for (int x = 1, n = qMin(engines.size(), 256); x < n; ++x) {
+            const_cast<QFontEngineMulti *>(this)->ensureFallbackFamiliesQueried();
-            QFontEngine *engine = engines.at(x);
+        for (int x = 1, n = qMin(m_engines.size(), 256); x < n; ++x) {
+            QFontEngine *engine = m_engines.at(x);
             if (!engine) {
                 if (!shouldLoadFontEngineForCharacter(x, ucs4))
                     continue;
-                const_cast<QFontEngineMulti *>(this)->loadEngine(x);
+                const_cast<QFontEngineMulti *>(this)->ensureEngineAt(x);
-                engine = engines.at(x);
+                engine = m_engines.at(x);
             }
             Q_ASSERT(engine != 0);
             if (engine->type() == Box)
@@ -1606,20 +1675,22 @@ bool QFontEngineMulti::stringToCMap(const QChar *str, int len,
     if (!engine(0)->stringToCMap(str, len, glyphs, nglyphs, flags))
         return false;
 
-    const_cast<QFontEngineMulti *>(this)->ensureFallbackFamiliesQueried();
     int glyph_pos = 0;
     QStringIterator it(str, str + len);
     while (it.hasNext()) {
         const uint ucs4 = it.peekNext();
         if (glyphs->glyphs[glyph_pos] == 0 && ucs4 != QChar::LineSeparator) {
-            for (int x = 1, n = qMin(engines.size(), 256); x < n; ++x) {
+            if (!m_fallbackFamiliesQueried)
-                if (engines.at(x) == 0 && !shouldLoadFontEngineForCharacter(x, ucs4))
+                const_cast<QFontEngineMulti *>(this)->ensureFallbackFamiliesQueried();
-                    continue;
+            for (int x = 1, n = qMin(m_engines.size(), 256); x < n; ++x) {
-
+                QFontEngine *engine = m_engines.at(x);
-                QFontEngine *engine = engines.at(x);
                 if (!engine) {
-                    const_cast<QFontEngineMulti *>(this)->loadEngine(x);
+                    if (!shouldLoadFontEngineForCharacter(x, ucs4))
-                    engine = engines.at(x);
+                        continue;
+                    const_cast<QFontEngineMulti *>(this)->ensureEngineAt(x);
+                    engine = m_engines.at(x);
+                    if (!engine)
+                        continue;
                 }
                 Q_ASSERT(engine != 0);
                 if (engine->type() == Box)
@@ -1874,7 +1945,6 @@ void QFontEngineMulti::doKerning(QGlyphLayout *glyphs, QFontEngine::ShaperFlags
 glyph_metrics_t QFontEngineMulti::boundingBox(glyph_t glyph)
 {
     const int which = highByte(glyph);
-    Q_ASSERT(which < engines.size());
     return engine(which)->boundingBox(stripped(glyph));
 }
 
@@ -1953,110 +2023,33 @@ bool QFontEngineMulti::canRender(const QChar *string, int len) const
 QImage QFontEngineMulti::alphaMapForGlyph(glyph_t glyph)
 {
     const int which = highByte(glyph);
-    Q_ASSERT(which < engines.size());
     return engine(which)->alphaMapForGlyph(stripped(glyph));
 }
 
 QImage QFontEngineMulti::alphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition)
 {
     const int which = highByte(glyph);
-    Q_ASSERT(which < engines.size());
     return engine(which)->alphaMapForGlyph(stripped(glyph), subPixelPosition);
 }
 
 QImage QFontEngineMulti::alphaMapForGlyph(glyph_t glyph, const QTransform &t)
 {
     const int which = highByte(glyph);
-    Q_ASSERT(which < engines.size());
     return engine(which)->alphaMapForGlyph(stripped(glyph), t);
 }
 
 QImage QFontEngineMulti::alphaMapForGlyph(glyph_t glyph, QFixed subPixelPosition, const QTransform &t)
 {
     const int which = highByte(glyph);
-    Q_ASSERT(which < engines.size());
     return engine(which)->alphaMapForGlyph(stripped(glyph), subPixelPosition, t);
 }
 
 QImage QFontEngineMulti::alphaRGBMapForGlyph(glyph_t glyph, QFixed subPixelPosition, const QTransform &t)
 {
     const int which = highByte(glyph);
-    Q_ASSERT(which < engines.size());
     return engine(which)->alphaRGBMapForGlyph(stripped(glyph), subPixelPosition, t);
 }
 
-/*
-    Creates a new multi engine.
-
-    This function takes ownership of the QFontEngine, increasing it's refcount.
-*/
-QFontEngineMultiBasicImpl::QFontEngineMultiBasicImpl(QFontEngine *fe, int _script, const QStringList &fallbacks)
-    : QFontEngineMulti(fallbacks.size() + 1),
-      fallbackFamilies(fallbacks), script(_script)
-    , fallbacksQueried(true)
-{
-    init(fe);
-}
-
-QFontEngineMultiBasicImpl::QFontEngineMultiBasicImpl(QFontEngine *fe, int _script)
-    : QFontEngineMulti(2)
-    , script(_script)
-    , fallbacksQueried(false)
-{
-    fallbackFamilies << QString();
-    init(fe);
-}
-
-void QFontEngineMultiBasicImpl::init(QFontEngine *fe)
-{
-    Q_ASSERT(fe && fe->type() != QFontEngine::Multi);
-    engines[0] = fe;
-    fe->ref.ref();
-    fontDef = engines[0]->fontDef;
-    cache_cost = fe->cache_cost;
-}
-
-void QFontEngineMultiBasicImpl::loadEngine(int at)
-{
-    ensureFallbackFamiliesQueried();
-    Q_ASSERT(at < engines.size());
-    Q_ASSERT(engines.at(at) == 0);
-    QFontDef request = fontDef;
-    request.styleStrategy |= QFont::NoFontMerging;
-    request.family = fallbackFamilies.at(at-1);
-    engines[at] = QFontDatabase::findFont(script,
-                                          /*fontprivate = */0,
-                                          request, /*multi = */false);
-    Q_ASSERT(engines[at]);
-    engines[at]->ref.ref();
-    engines[at]->fontDef = request;
-}
-void QFontEngineMultiBasicImpl::ensureFallbackFamiliesQueried()
-{
-    if (fallbacksQueried)
-        return;
-    QStringList fallbacks = QGuiApplicationPrivate::instance()->platformIntegration()->fontDatabase()->fallbacksForFamily(engine(0)->fontDef.family, QFont::Style(engine(0)->fontDef.style)
-                                                                                                                          , QFont::AnyStyle, QChar::Script(script));
-    setFallbackFamiliesList(fallbacks);
-}
-
-void QFontEngineMultiBasicImpl::setFallbackFamiliesList(const QStringList &fallbacks)
-{
-    // Original FontEngine to restore after the fill.
-    QFontEngine *fe = engines[0];
-    fallbackFamilies = fallbacks;
-    if (!fallbackFamilies.isEmpty()) {
-        engines.fill(0, fallbackFamilies.size() + 1);
-        engines[0] = fe;
-    } else {
-        // Turns out we lied about having any fallback at all.
-        fallbackFamilies << fe->fontDef.family;
-        engines[1] = fe;
-        fe->ref.ref();
-    }
-    fallbacksQueried = true;
-}
-
 /*
   This is used indirectly by Qt WebKit when using QTextLayout::setRawFont
 
@@ -2065,7 +2058,7 @@ void QFontEngineMultiBasicImpl::setFallbackFamiliesList(const QStringList &fallb
   the same raw font over and over again, we want to cache the corresponding multi font engine
   as it may contain fallback font engines already.
 */
-QFontEngine* QFontEngineMultiBasicImpl::createMultiFontEngine(QFontEngine *fe, int script)
+QFontEngine *QFontEngineMulti::createMultiFontEngine(QFontEngine *fe, int script)
 {
     QFontEngine *engine = 0;
     QFontCache::Key key(fe->fontDef, script, /*multi = */true);

src/gui/text/qfontengine_p.h

@@ -385,7 +385,7 @@ private:
 class Q_GUI_EXPORT QFontEngineMulti : public QFontEngine
 {
 public:
-    explicit QFontEngineMulti(int engineCount);
+    explicit QFontEngineMulti(QFontEngine *engine, int script, const QStringList &fallbackFamilies = QStringList());
     ~QFontEngineMulti();
 
     virtual glyph_t glyphIndex(uint ucs4) const;
@@ -418,46 +418,28 @@ public:
 
     virtual bool canRender(const QChar *string, int len) const;
 
-    QFontEngine *engine(int at) const
+    inline int fallbackFamilyCount() const { return m_fallbackFamilies.size(); }
-    {Q_ASSERT(at < engines.size()); return engines.at(at); }
+    inline QString fallbackFamilyAt(int at) const { return m_fallbackFamilies.at(at); }
 
-    inline void ensureEngineAt(int at)
+    void setFallbackFamiliesList(const QStringList &fallbackFamilies);
-    {
-        if (at >= engines.size() || engines.at(at) == 0)
-            loadEngine(at);
-    }
 
-    virtual bool shouldLoadFontEngineForCharacter(int at, uint ucs4) const;
+    inline QFontEngine *engine(int at) const
-    virtual void setFallbackFamiliesList(const QStringList &) {}
+    { Q_ASSERT(at < m_engines.size()); return m_engines.at(at); }
+
+    void ensureEngineAt(int at);
+
+    static QFontEngine *createMultiFontEngine(QFontEngine *fe, int script);
 
 protected:
-    friend class QRawFont;
-    virtual void loadEngine(int at) = 0;
-    virtual void ensureFallbackFamiliesQueried() {}
-    QVector<QFontEngine *> engines;
-};
-
-class Q_GUI_EXPORT QFontEngineMultiBasicImpl : public QFontEngineMulti
-{
-public:
-    QFontEngineMultiBasicImpl(QFontEngine *fe, int script, const QStringList &fallbacks);
-    QFontEngineMultiBasicImpl(QFontEngine *fe, int script);
-
-    void loadEngine(int at);
-    static QFontEngine* createMultiFontEngine(QFontEngine *fe, int script);
-
-    int fallbackFamilyCount() const { return fallbackFamilies.size(); }
-    QString fallbackFamilyAt(int at) const { return fallbackFamilies.at(at); }
-
     virtual void ensureFallbackFamiliesQueried();
-    virtual void setFallbackFamiliesList(const QStringList &fallbacks);
+    virtual bool shouldLoadFontEngineForCharacter(int at, uint ucs4) const;
+    virtual QFontEngine *loadEngine(int at);
 
 private:
-    void init(QFontEngine *fe);
+    QVector<QFontEngine *> m_engines;
-
+    QStringList m_fallbackFamilies;
-    mutable QStringList fallbackFamilies;
+    const int m_script;
-    int script;
+    bool m_fallbackFamiliesQueried;
-    mutable bool fallbacksQueried;
 };
 
 class QTestFontEngine : public QFontEngineBox

src/gui/text/qplatformfontdatabase.cpp

@@ -314,7 +314,7 @@ void QPlatformFontDatabase::invalidate()
 */
 QFontEngineMulti *QPlatformFontDatabase::fontEngineMulti(QFontEngine *fontEngine, QChar::Script script)
 {
-    return new QFontEngineMultiBasicImpl(fontEngine, script);
+    return new QFontEngineMulti(fontEngine, script);
 }
 
 /*!

src/gui/text/qtextengine.cpp

@@ -1950,7 +1950,7 @@ QFontEngine *QTextEngine::fontEngine(const QScriptItem &si, QFixed *ascent, QFix
         if (feCache.prevFontEngine && feCache.prevFontEngine->type() == QFontEngine::Multi && feCache.prevScript == script) {
             engine = feCache.prevFontEngine;
         } else {
-            engine = QFontEngineMultiBasicImpl::createMultiFontEngine(rawFont.d->fontEngine, script);
+            engine = QFontEngineMulti::createMultiFontEngine(rawFont.d->fontEngine, script);
             feCache.prevFontEngine = engine;
             feCache.prevScript = script;
             engine->ref.ref();
@@ -1965,7 +1965,7 @@ QFontEngine *QTextEngine::fontEngine(const QScriptItem &si, QFixed *ascent, QFix
             } else {
                 QFontEngine *scEngine = rawFont.d->fontEngine->cloneWithSize(smallCapsFraction * rawFont.pixelSize());
                 scEngine->ref.ref();
-                scaledEngine = QFontEngineMultiBasicImpl::createMultiFontEngine(scEngine, script);
+                scaledEngine = QFontEngineMulti::createMultiFontEngine(scEngine, script);
                 scaledEngine->ref.ref();
                 feCache.prevScaledFontEngine = scaledEngine;
                 // If scEngine is not ref'ed by scaledEngine, make sure it is deallocated and not leaked.

src/platformsupport/fontdatabases/fontconfig/qfontenginemultifontconfig.cpp

@@ -38,7 +38,7 @@
 QT_BEGIN_NAMESPACE
 
 QFontEngineMultiFontConfig::QFontEngineMultiFontConfig(QFontEngine *fe, int script)
-    : QFontEngineMultiBasicImpl(fe, script)
+    : QFontEngineMulti(fe, script)
 {
 }
 
@@ -67,8 +67,8 @@ bool QFontEngineMultiFontConfig::shouldLoadFontEngineForCharacter(int at, uint u
 FcPattern * QFontEngineMultiFontConfig::getMatchPatternForFallback(int fallBackIndex) const
 {
     Q_ASSERT(fallBackIndex < fallbackFamilyCount());
-    if (engines.size() - 1 > cachedMatchPatterns.size())
+    if (fallbackFamilyCount() > cachedMatchPatterns.size())
-        cachedMatchPatterns.resize(engines.size() - 1);
+        cachedMatchPatterns.resize(fallbackFamilyCount());
     FcPattern *ret = cachedMatchPatterns.at(fallBackIndex);
     if (ret)
         return ret;

src/platformsupport/fontdatabases/fontconfig/qfontenginemultifontconfig_p.h

@@ -50,7 +50,7 @@
 
 QT_BEGIN_NAMESPACE
 
-class QFontEngineMultiFontConfig : public QFontEngineMultiBasicImpl
+class QFontEngineMultiFontConfig : public QFontEngineMulti
 {
 public:
     explicit QFontEngineMultiFontConfig(QFontEngine *fe, int script);

src/plugins/platforms/windows/qwindowsfontdatabase.cpp

@@ -1087,7 +1087,7 @@ QFontEngineMulti *QWindowsFontDatabase::fontEngineMulti(QFontEngine *fontEngine,
     if (script == QChar::Script_Common)
         return new QWindowsMultiFontEngine(fontEngine, script);
     // ### as long as fallbacksForFamily() does not take script parameter into account,
-    // prefer QFontEngineMultiBasicImpl's loadEngine() implementation for complex scripts
+    // prefer QFontEngineMulti's loadEngine() implementation for complex scripts
     return QPlatformFontDatabase::fontEngineMulti(fontEngine, script);
 }
 

src/plugins/platforms/windows/qwindowsfontengine.cpp

@@ -1302,17 +1302,13 @@ void QWindowsFontEngine::initFontInfo(const QFontDef &request,
     Will probably be superseded by a common Free Type font engine in Qt 5.X.
 */
 QWindowsMultiFontEngine::QWindowsMultiFontEngine(QFontEngine *fe, int script)
-    : QFontEngineMultiBasicImpl(fe, script)
+    : QFontEngineMulti(fe, script)
 {
 }
 
-void QWindowsMultiFontEngine::loadEngine(int at)
+QFontEngine *QWindowsMultiFontEngine::loadEngine(int at)
 {
-    ensureFallbackFamiliesQueried();
+    QFontEngine *fontEngine = engine(0);
-    Q_ASSERT(at < engines.size());
-    Q_ASSERT(engines.at(at) == 0);
-
-    QFontEngine *fontEngine = engines.at(0);
     QSharedPointer<QWindowsFontEngineData> data;
     LOGFONT lf;
 
@@ -1345,23 +1341,18 @@ void QWindowsMultiFontEngine::loadEngine(int at)
         if (FAILED(hr)) {
             qErrnoWarning("%s: CreateFontFromLOGFONT failed", __FUNCTION__);
         } else {
+            Q_ASSERT(directWriteFont);
             IDWriteFontFace *directWriteFontFace = NULL;
             HRESULT hr = directWriteFont->CreateFontFace(&directWriteFontFace);
             if (SUCCEEDED(hr)) {
+                Q_ASSERT(directWriteFontFace);
                 QWindowsFontEngineDirectWrite *fedw = new QWindowsFontEngineDirectWrite(directWriteFontFace,
                                                                                         fontEngine->fontDef.pixelSize,
                                                                                         data);
-                fedw->fontDef = fontDef;
+                return fedw;
-                fedw->fontDef.family = fam;
-                fedw->ref.ref();
-                engines[at] = fedw;
-
-                qCDebug(lcQpaFonts) << __FUNCTION__ << at << fam;
-                return;
             } else {
                 qErrnoWarning("%s: CreateFontFace failed", __FUNCTION__);
             }
-
         }
     }
 #endif
@@ -1375,13 +1366,8 @@ void QWindowsMultiFontEngine::loadEngine(int at)
         hfont = (HFONT)GetStockObject(ANSI_VAR_FONT);
         stockFont = true;
     }
-    engines[at] = new QWindowsFontEngine(fam, hfont, stockFont, lf, data);
-    engines[at]->ref.ref();
-    engines[at]->fontDef = fontDef;
-    engines[at]->fontDef.family = fam;
-    qCDebug(lcQpaFonts) << __FUNCTION__ << at << fam;
 
-    // TODO: increase cost in QFontCache for the font engine loaded here
+    return new QWindowsFontEngine(fam, hfont, stockFont, lf, data);
 }
 
 bool QWindowsFontEngine::supportsTransformation(const QTransform &transform) const

src/plugins/platforms/windows/qwindowsfontengine.h

@@ -163,13 +163,12 @@ private:
     mutable int designAdvancesSize;
 };
 
-
+class QWindowsMultiFontEngine : public QFontEngineMulti
-class QWindowsMultiFontEngine : public QFontEngineMultiBasicImpl
 {
 public:
     explicit QWindowsMultiFontEngine(QFontEngine *fe, int script);
 
-    void loadEngine(int at);
+    QFontEngine *loadEngine(int at) Q_DECL_OVERRIDE;
 };
 
 QT_END_NAMESPACE