From 589a94ddb30499b0f3ecaf909a5b291d64d02dc7 Mon Sep 17 00:00:00 2001

From: Jiang Jiang <jiang.jiang@nokia.com>

Date: Mon, 20 Jun 2011 13:47:01 +0200

Subject: [PATCH] Fix fontconfig usage in X11 font database

We should do FcConfigSubstitute(0, pattern, FcMatchFont) on a

FcPattern for query because the family list in it will contain

almost all the families after FcConfigSubstitute(0, pattern,

FcMatchPattern), then the test in <match target="font"> will

almost always succeed. In general, FcMatchFont substitute

should only be done on the FcPattern that we got after

FcFontMatch() or FcFontRenderPrepare().

Based on the suggestion of fontconfig author Behdad Esfahbod,

this patch reorganized the tryPatternLoad logic so that it only

does the QFontEngine creation part, FcPattern *match is retrieved

outside of that function. In this way, the match pattern we got

can be either from FcFontMatch() or after FcFontRenderPrepare()

on one of the fonts we got from qt_fontSetForPattern(). Then we

don't need to duplicate the pattern and add all criterias back

with qt_addPatternProps(). It not only simplified the code a lot

but also fix the way we apply FcMatchFont substitution. This

substitution will either be done by FcFontMatch() or

FcFontRenderPrepare, both implicitly.

Task-number: QTBUG-2148, QTBUG-19947

Reviewed-by: Eskil

---

 src/gui/text/qfontdatabase_x11.cpp |   55 +++++++++++++++---------------------

 src/gui/text/qfontengine_x11.cpp   |   28 +++++-------------

 2 files changed, 31 insertions(+), 52 deletions(-)

diff --git a/src/gui/text/qfontdatabase_x11.cpp b/src/gui/text/qfontdatabase_x11.cpp

index 754334c..958daa2 100644

--- a/src/gui/text/qfontdatabase_x11.cpp

+++ b/src/gui/text/qfontdatabase_x11.cpp

@@ -1566,9 +1566,8 @@ static FcPattern *getFcPattern(const QFontPrivate *fp, int script, const QFontDe

     qt_addPatternProps(pattern, fp->screen, script, request);

-    FcDefaultSubstitute(pattern);

     FcConfigSubstitute(0, pattern, FcMatchPattern);

-    FcConfigSubstitute(0, pattern, FcMatchFont);

+    FcDefaultSubstitute(pattern);

     // these should only get added to the pattern _after_ substitution

     // append the default fallback font for the specified script

@@ -1606,35 +1605,20 @@ static void FcFontSetRemove(FcFontSet *fs, int at)

         memmove(fs->fonts + at, fs->fonts + at + 1, len);

 }

-static QFontEngine *tryPatternLoad(FcPattern *p, int screen,

-                                   const QFontDef &request, int script, FcPattern **matchedPattern = 0)

+static QFontEngine *tryPatternLoad(FcPattern *match, int screen,

+                                   const QFontDef &request, int script)

 {

 #ifdef FONT_MATCH_DEBUG

     FcChar8 *fam;

-    FcPatternGetString(p, FC_FAMILY, 0, &fam;;

+    FcPatternGetString(match, FC_FAMILY, 0, &fam;;

     FM_DEBUG("==== trying %s\n", fam);

 #endif

     FM_DEBUG("passes charset test\n");

-    FcPattern *pattern = FcPatternDuplicate(p);

-    // add properties back in as the font selected from the

-    // list doesn't contain them.

-    qt_addPatternProps(pattern, screen, script, request);

-

-    FcConfigSubstitute(0, pattern, FcMatchPattern);

-    FcDefaultSubstitute(pattern);

-    FcResult res;

-    FcPattern *match = FcFontMatch(0, pattern, &res;;

-

-    if (matchedPattern)

-	*matchedPattern = 0;

     QFontEngineX11FT *engine = 0;

     if (!match) // probably no fonts available.

         goto done;

-    if (matchedPattern)

-	*matchedPattern = FcPatternDuplicate(match);

-

     if (script != QUnicodeTables::Common) {

         // skip font if it doesn't support the language we want

         if (specialChars[script]) {

@@ -1673,11 +1657,6 @@ static QFontEngine *tryPatternLoad(FcPattern *p, int screen,

         }

     }

 done:

-    FcPatternDestroy(pattern);

-    if (!engine && matchedPattern && *matchedPattern) {

-        FcPatternDestroy(*matchedPattern);

-        *matchedPattern = 0;

-    }

     return engine;

 }

@@ -1726,14 +1705,26 @@ static QFontEngine *loadFc(const QFontPrivate *fp, int script, const QFontDef &r

 #endif

     QFontEngine *fe = 0;

-    FcPattern *matchedPattern = 0;

-    fe = tryPatternLoad(pattern, fp->screen, request, script, &matchedPattern);

+    FcResult res;

+    FcPattern *match = FcFontMatch(0, pattern, &res;;

+    fe = tryPatternLoad(match, fp->screen, request, script);

     if (!fe) {

         FcFontSet *fs = qt_fontSetForPattern(pattern, request);

+        if (match) {

+            FcPatternDestroy(match);

+            match = 0;

+        }

+

         if (fs) {

-            for (int i = 0; !fe && i < fs->nfont; ++i)

-                fe = tryPatternLoad(fs->fonts[i], fp->screen, request, script, &matchedPattern);

+            for (int i = 0; !fe && i < fs->nfont; ++i) {

+                match = FcFontRenderPrepare(NULL, pattern, fs->fonts[i]);

+                fe = tryPatternLoad(match, fp->screen, request, script);

+                if (fe)

+                    break;

+                FcPatternDestroy(match);

+                match = 0;

+            }

             FcFontSetDestroy(fs);

         }

         FM_DEBUG("engine for script %d is %s\n", script, fe ? fe->fontDef.family.toLatin1().data(): "(null)");

@@ -1741,11 +1732,11 @@ static QFontEngine *loadFc(const QFontPrivate *fp, int script, const QFontDef &r

     if (fe

         && script == QUnicodeTables::Common

         && !(request.styleStrategy & QFont::NoFontMerging) && !fe->symbol) {

-        fe = new QFontEngineMultiFT(fe, matchedPattern, pattern, fp->screen, request);

+        fe = new QFontEngineMultiFT(fe, match, pattern, fp->screen, request);

     } else {

         FcPatternDestroy(pattern);

-        if (matchedPattern)

-            FcPatternDestroy(matchedPattern);

+        if (match)

+            FcPatternDestroy(match);

     }

     return fe;

 }

diff --git a/src/gui/text/qfontengine_x11.cpp b/src/gui/text/qfontengine_x11.cpp

index e3bfa5d..490866c 100644

--- a/src/gui/text/qfontengine_x11.cpp

+++ b/src/gui/text/qfontengine_x11.cpp

@@ -863,11 +863,8 @@ glyph_t QFontEngineXLFD::glyphIndexToFreetypeGlyphIndex(glyph_t g) const

 // Multi FT engine

 // ------------------------------------------------------------------

-static QFontEngine *engineForPattern(FcPattern *pattern, const QFontDef &request,

-                                     int screen)

+static QFontEngine *engineForPattern(FcPattern *match, const QFontDef &request, int screen)

 {

-    FcResult res;

-    FcPattern *match = FcFontMatch(0, pattern, &res;;

     QFontEngineX11FT *engine = new QFontEngineX11FT(match, request, screen);

     if (!engine->invalid())

         return engine;

@@ -879,9 +876,9 @@ static QFontEngine *engineForPattern(FcPattern *pattern, const QFontDef &request

 }

 QFontEngineMultiFT::QFontEngineMultiFT(QFontEngine *fe, FcPattern *matchedPattern, FcPattern *p, int s, const QFontDef &req)

-    : QFontEngineMulti(2), request(req), pattern(p), firstEnginePattern(matchedPattern), fontSet(0), screen(s)

+    : QFontEngineMulti(2), request(req), pattern(p), fontSet(0), screen(s)

 {

-

+    firstEnginePattern = FcPatternDuplicate(matchedPattern);

     engines[0] = fe;

     engines.at(0)->ref.ref();

     fontDef = engines[0]->fontDef;

@@ -907,8 +904,6 @@ void QFontEngineMultiFT::loadEngine(int at)

     extern QMutex *qt_fontdatabase_mutex();

     QMutexLocker locker(qt_fontdatabase_mutex());

-    extern void qt_addPatternProps(FcPattern *pattern, int screen, int script,

-                                   const QFontDef &request);

     extern QFontDef qt_FcPatternToQFontDef(FcPattern *pattern, const QFontDef &);

     extern FcFontSet *qt_fontSetForPattern(FcPattern *pattern, const QFontDef &request);

@@ -940,22 +935,18 @@ void QFontEngineMultiFT::loadEngine(int at)

     Q_ASSERT(at < engines.size());

     Q_ASSERT(engines.at(at) == 0);

-    FcPattern *pattern = FcPatternDuplicate(fontSet->fonts[at + firstFontIndex - 1]);

-    qt_addPatternProps(pattern, screen, QUnicodeTables::Common, request);

-

-    QFontDef fontDef = qt_FcPatternToQFontDef(pattern, this->request);

+    FcPattern *match = FcFontRenderPrepare(NULL, pattern, fontSet->fonts[at + firstFontIndex - 1]);

+    QFontDef fontDef = qt_FcPatternToQFontDef(match, this->request);

     // note: we use -1 for the script to make sure that we keep real

     // FT engines separate from Multi engines in the font cache

     QFontCache::Key key(fontDef, -1, screen);

     QFontEngine *fontEngine = QFontCache::instance()->findEngine(key);

     if (!fontEngine) {

-        FcConfigSubstitute(0, pattern, FcMatchPattern);

-        FcDefaultSubstitute(pattern);

-        fontEngine = engineForPattern(pattern, request, screen);

+        fontEngine = engineForPattern(match, request, screen);

         QFontCache::instance()->insertEngine(key, fontEngine);

     }

-    FcPatternDestroy(pattern);

+    FcPatternDestroy(match);

     fontEngine->ref.ref();

     engines[at] = fontEngine;

 }

@@ -1123,17 +1114,14 @@ QFontEngineX11FT::QFontEngineX11FT(FcPattern *pattern, const QFontDef &fd, int s

     }

 #endif

-    if (!init(face_id, antialias, defaultFormat)) {

-        FcPatternDestroy(pattern);

+    if (!init(face_id, antialias, defaultFormat))

         return;

-    }

     if (!freetype->charset) {

         FcCharSet *cs;

         FcPatternGetCharSet (pattern, FC_CHARSET, 0, &cs);

         freetype->charset = FcCharSetCopy(cs);

     }

-    FcPatternDestroy(pattern);

 }

 QFontEngineX11FT::~QFontEngineX11FT()

-- 

1.7.4.1