engineData should be cached solely based on QFontDef

Each QFontPrivate points to a QFontEngineData instance, which is
essentially a mapping from different scripts to QFontEngines. Let's
say we have QFont("Waree") and trying to use that for one text in
Thai and another text in English, there should be only one
QFontEngineData: [ QUnicodeTables::Common -> QFontEngineMulti(1),
QUnicodeTables::Thai -> QFontEngineMulti(2) ]. If we cache
QFontEngineData using QFontCache::Key (which includes QFontDef,
script and screen) as the key, then we will create two
QFontEngineData: [ QUnicodeTables::Common -> QFontEngineMulti(1) ]
and [ QUnicodeTables::Thai -> QFontEngineMulti(2) ], so it will be
pointless to have QFontEngineData at all.

This bug was introduced in a 2005 refactoring (512f0e8c in history
repo).

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

diff --git a/src/gui/text/qfont.cpp b/src/gui/text/qfont.cpp
index 23fe5ca..804d3f9 100644 (file)
--- a/src/gui/text/qfont.cpp
+++ b/src/gui/text/qfont.cpp
@@ -2696,9 +2696,9 @@ void QFontCache::clear()
 }
 
 
-QFontEngineData *QFontCache::findEngineData(const Key &key) const
+QFontEngineData *QFontCache::findEngineData(const QFontDef &def) const
 {
-    EngineDataCache::ConstIterator it = engineDataCache.find(key),
+    EngineDataCache::ConstIterator it = engineDataCache.find(def),
                                   end = engineDataCache.end();
     if (it == end) return 0;
 
@@ -2706,11 +2706,11 @@ QFontEngineData *QFontCache::findEngineData(const Key &key) const
     return it.value();
 }
 
-void QFontCache::insertEngineData(const Key &key, QFontEngineData *engineData)
+void QFontCache::insertEngineData(const QFontDef &def, QFontEngineData *engineData)
 {
     FC_DEBUG("QFontCache: inserting new engine data %p", engineData);
 
-    engineDataCache.insert(key, engineData);
+    engineDataCache.insert(def, engineData);
     increaseCost(sizeof(QFontEngineData));
 }
 

diff --git a/src/gui/text/qfont_p.h b/src/gui/text/qfont_p.h
index f4641ff..2d4f3de 100644 (file)
--- a/src/gui/text/qfont_p.h
+++ b/src/gui/text/qfont_p.h
@@ -222,11 +222,11 @@ public:
     };
 
     // QFontEngineData cache
-    typedef QMap<Key,QFontEngineData*> EngineDataCache;
+    typedef QMap<QFontDef, QFontEngineData*> EngineDataCache;
     EngineDataCache engineDataCache;
 
-    QFontEngineData *findEngineData(const Key &key) const;
-    void insertEngineData(const Key &key, QFontEngineData *engineData);
+    QFontEngineData *findEngineData(const QFontDef &def) const;
+    void insertEngineData(const QFontDef &def, QFontEngineData *engineData);
 
     // QFontEngine cache
     struct Engine {

diff --git a/src/gui/text/qfontdatabase.cpp b/src/gui/text/qfontdatabase.cpp
index 52cfb8a..c80ded6 100644 (file)
--- a/src/gui/text/qfontdatabase.cpp
+++ b/src/gui/text/qfontdatabase.cpp
@@ -759,14 +759,14 @@ static void initFontDef(const QtFontDesc &desc, const QFontDef &request, QFontDe
     fontDef->ignorePitch   = false;
 }
 
-static void getEngineData(const QFontPrivate *d, const QFontCache::Key &key)
+static void getEngineData(const QFontPrivate *d, const QFontDef &def)
 {
     // look for the requested font in the engine data cache
-    d->engineData = QFontCache::instance()->findEngineData(key);
+    d->engineData = QFontCache::instance()->findEngineData(def);
     if (!d->engineData) {
         // create a new one
         d->engineData = new QFontEngineData;
-        QFontCache::instance()->insertEngineData(key, d->engineData);
+        QFontCache::instance()->insertEngineData(def, d->engineData);
     } else {
         d->engineData->ref.ref();
     }

diff --git a/src/gui/text/qfontdatabase_qpa.cpp b/src/gui/text/qfontdatabase_qpa.cpp
index 47d16b4..55b9023 100644 (file)
--- a/src/gui/text/qfontdatabase_qpa.cpp
+++ b/src/gui/text/qfontdatabase_qpa.cpp
@@ -348,7 +348,7 @@ void QFontDatabase::load(const QFontPrivate *d, int script)
     QFontCache::Key key(req, script, multi ? 1 : 0);
 
     if (!d->engineData)
-        getEngineData(d, key);
+        getEngineData(d, req);
 
     // the cached engineData could have already loaded the engine we want
     if (d->engineData->engines[script])