Bug 1478716 - Ensure we only call FT_Get_MM_Var once per face (and cache the result in the font entry), to avoid being bitten by freetype bug 52955 on Ubuntu. r=lsalzman
authorJonathan Kew <jkew@mozilla.com>
Wed, 01 Aug 2018 22:39:05 +0100
changeset 429719 aee91e7da807e596dc18e0286500dcfd98b05c09
parent 429718 6e508c90723834f3e242d06e063a114ea1bba637
child 429720 8f9c9580f68754c4617c7a8e674cf99cf917caac
push id34372
push usernerli@mozilla.com
push dateThu, 02 Aug 2018 08:55:28 +0000
treeherdermozilla-central@bd79b07f57a3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman
bugs1478716, 52955
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1478716 - Ensure we only call FT_Get_MM_Var once per face (and cache the result in the font entry), to avoid being bitten by freetype bug 52955 on Ubuntu. r=lsalzman
gfx/thebes/gfxFT2FontBase.cpp
gfx/thebes/gfxFT2FontBase.h
gfx/thebes/gfxFT2FontList.cpp
gfx/thebes/gfxFT2FontList.h
gfx/thebes/gfxFcPlatformFontList.cpp
gfx/thebes/gfxFcPlatformFontList.h
gfx/thebes/gfxFontEntry.h
--- a/gfx/thebes/gfxFT2FontBase.cpp
+++ b/gfx/thebes/gfxFT2FontBase.cpp
@@ -228,17 +228,17 @@ gfxFT2FontBase::InitMetrics()
         SanitizeMetrics(&mMetrics, false);
         return;
     }
 
     if (face->face_flags & FT_FACE_FLAG_MULTIPLE_MASTERS) {
         // Resolve variations from entry (descriptor) and style (property)
         AutoTArray<gfxFontVariation,8> settings;
         mFontEntry->GetVariationsForStyle(settings, mStyle);
-        SetupVarCoords(face, settings, &mCoords);
+        SetupVarCoords(mFontEntry->GetMMVar(), settings, &mCoords);
         if (!mCoords.IsEmpty()) {
 #if MOZ_TREE_FREETYPE
             FT_Set_Var_Design_Coordinates(face, mCoords.Length(), mCoords.Elements());
 #else
             typedef FT_Error (*SetCoordsFunc)(FT_Face, FT_UInt, FT_Fixed*);
             static SetCoordsFunc setCoords;
             static bool firstTime = true;
             if (firstTime) {
@@ -633,51 +633,30 @@ gfxFT2FontBase::SetupCairoFont(DrawTarge
     return true;
 }
 
 // For variation fonts, figure out the variation coordinates to be applied
 // for each axis, in freetype's order (which may not match the order of
 // axes in mStyle.variationSettings, so we need to search by axis tag).
 /*static*/
 void
-gfxFT2FontBase::SetupVarCoords(FT_Face aFace,
+gfxFT2FontBase::SetupVarCoords(FT_MM_Var* aMMVar,
                                const nsTArray<gfxFontVariation>& aVariations,
                                nsTArray<FT_Fixed>* aCoords)
 {
     aCoords->TruncateLength(0);
-    if (aFace->face_flags & FT_FACE_FLAG_MULTIPLE_MASTERS) {
-        typedef FT_Error (*GetVarFunc)(FT_Face, FT_MM_Var**);
-        typedef FT_Error (*DoneVarFunc)(FT_Library, FT_MM_Var*);
-#if MOZ_TREE_FREETYPE
-        GetVarFunc getVar = &FT_Get_MM_Var;
-        DoneVarFunc doneVar = &FT_Done_MM_Var;
-#else
-        static GetVarFunc getVar;
-        static DoneVarFunc doneVar;
-        static bool firstTime = true;
-        if (firstTime) {
-            firstTime = false;
-            getVar = (GetVarFunc)dlsym(RTLD_DEFAULT, "FT_Get_MM_Var");
-            doneVar = (DoneVarFunc)dlsym(RTLD_DEFAULT, "FT_Done_MM_Var");
-        }
-#endif
-        FT_MM_Var* ftVar;
-        if (getVar && FT_Err_Ok == (*getVar)(aFace, &ftVar)) {
-            for (unsigned i = 0; i < ftVar->num_axis; ++i) {
-                aCoords->AppendElement(ftVar->axis[i].def);
-                for (const auto& v : aVariations) {
-                    if (ftVar->axis[i].tag == v.mTag) {
-                        FT_Fixed val = v.mValue * 0x10000;
-                        val = std::min(val, ftVar->axis[i].maximum);
-                        val = std::max(val, ftVar->axis[i].minimum);
-                        (*aCoords)[i] = val;
-                        break;
-                    }
-                }
-            }
-            if (doneVar) {
-                (*doneVar)(aFace->glyph->library, ftVar);
-            } else {
-                free(ftVar);
+    if (!aMMVar) {
+        return;
+    }
+
+    for (unsigned i = 0; i < aMMVar->num_axis; ++i) {
+        aCoords->AppendElement(aMMVar->axis[i].def);
+        for (const auto& v : aVariations) {
+            if (aMMVar->axis[i].tag == v.mTag) {
+                FT_Fixed val = v.mValue * 0x10000;
+                val = std::min(val, aMMVar->axis[i].maximum);
+                val = std::max(val, aMMVar->axis[i].minimum);
+                (*aCoords)[i] = val;
+                break;
             }
         }
     }
 }
--- a/gfx/thebes/gfxFT2FontBase.h
+++ b/gfx/thebes/gfxFT2FontBase.h
@@ -32,17 +32,17 @@ public:
     virtual bool ProvidesGlyphWidths() const override { return true; }
     virtual int32_t GetGlyphWidth(DrawTarget& aDrawTarget,
                                   uint16_t aGID) override;
 
     virtual bool SetupCairoFont(DrawTarget* aDrawTarget) override;
 
     virtual FontType GetType() const override { return FONT_TYPE_FT2; }
 
-    static void SetupVarCoords(FT_Face aFace,
+    static void SetupVarCoords(FT_MM_Var* aMMVar,
                                const nsTArray<gfxFontVariation>& aVariations,
                                nsTArray<FT_Fixed>* aCoords);
 
 private:
     uint32_t GetCharExtents(char aChar, cairo_text_extents_t* aExtents);
     uint32_t GetCharWidth(char aChar, gfxFloat* aWidth);
 
     // Get advance of a single glyph from FreeType, and return true;
--- a/gfx/thebes/gfxFT2FontList.cpp
+++ b/gfx/thebes/gfxFT2FontList.cpp
@@ -201,16 +201,20 @@ FT2FontEntry::CreateScaledFont(const gfx
     NS_ASSERTION(cairo_scaled_font_status(scaledFont) == CAIRO_STATUS_SUCCESS,
                  "Failed to make scaled font");
 
     return scaledFont;
 }
 
 FT2FontEntry::~FT2FontEntry()
 {
+    if (mMMVar) {
+        FT_Done_MM_Var(mFTFace->glyph->library, mMMVar);
+    }
+
     // Do nothing for mFTFace here since FTFontDestroyFunc is called by cairo.
     mFTFace = nullptr;
 
 #ifndef ANDROID
     if (mFontFace) {
         cairo_font_face_destroy(mFontFace);
         mFontFace = nullptr;
     }
@@ -468,17 +472,17 @@ FT2FontEntry::CairoFontFace(const gfxFon
         (mFTFace->face_flags & FT_FACE_FLAG_MULTIPLE_MASTERS)) {
         int flags = gfxPlatform::GetPlatform()->FontHintingEnabled() ?
                     FT_LOAD_DEFAULT :
                     (FT_LOAD_NO_AUTOHINT | FT_LOAD_NO_HINTING);
         // Resolve variations from entry (descriptor) and style (property)
         AutoTArray<gfxFontVariation,8> settings;
         GetVariationsForStyle(settings, aStyle ? *aStyle : gfxFontStyle());
         AutoTArray<FT_Fixed,8> coords;
-        gfxFT2FontBase::SetupVarCoords(mFTFace, settings, &coords);
+        gfxFT2FontBase::SetupVarCoords(GetMMVar(), settings, &coords);
         // Create a separate FT_Face because we need to apply custom
         // variation settings to it.
         FT_Face ftFace;
         if (!mFilename.IsEmpty() && mFilename[0] == '/') {
             ftFace = Factory::NewFTFace(nullptr, mFilename.get(), mFTFontIndex);
         } else {
             auto ufd = reinterpret_cast<FTUserFontData*>(
                 cairo_font_face_get_user_data(mFontFace, &sFTUserFontDataKey));
@@ -641,45 +645,52 @@ FT2FontEntry::HasVariations()
 }
 
 void
 FT2FontEntry::GetVariationAxes(nsTArray<gfxFontVariationAxis>& aAxes)
 {
     if (!HasVariations()) {
         return;
     }
-    AutoFTFace face(this);
-    if (!face) {
-        return;
-    }
-    FT_MM_Var* mmVar;
-    if (FT_Err_Ok != (FT_Get_MM_Var(face, &mmVar))) {
+    FT_MM_Var* mmVar = GetMMVar();
+    if (!mmVar) {
         return;
     }
     gfxFT2Utils::GetVariationAxes(mmVar, aAxes);
-    FT_Done_MM_Var(FT_Face(face)->glyph->library, mmVar);
 }
 
 void
 FT2FontEntry::GetVariationInstances(
     nsTArray<gfxFontVariationInstance>& aInstances)
 {
     if (!HasVariations()) {
         return;
     }
-    AutoFTFace face(this);
-    if (!face) {
-        return;
-    }
-    FT_MM_Var* mmVar;
-    if (FT_Err_Ok != (FT_Get_MM_Var(face, &mmVar))) {
+    FT_MM_Var* mmVar = GetMMVar();
+    if (!mmVar) {
         return;
     }
     gfxFT2Utils::GetVariationInstances(this, mmVar, aInstances);
-    FT_Done_MM_Var(FT_Face(face)->glyph->library, mmVar);
+}
+
+FT_MM_Var*
+FT2FontEntry::GetMMVar()
+{
+    if (mMMVarInitialized) {
+        return mMMVar;
+    }
+    mMMVarInitialized = true;
+    AutoFTFace face(this);
+    if (!face) {
+        return nullptr;
+    }
+    if (FT_Err_Ok != FT_Get_MM_Var(face, &mMMVar)) {
+        mMMVar = nullptr;
+    }
+    return mMMVar;
 }
 
 void
 FT2FontEntry::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                                      FontListSizes* aSizes) const
 {
     gfxFontEntry::AddSizeOfExcludingThis(aMallocSizeOf, aSizes);
     aSizes->mFontListSize +=
--- a/gfx/thebes/gfxFT2FontList.h
+++ b/gfx/thebes/gfxFT2FontList.h
@@ -89,31 +89,36 @@ public:
     bool HasVariations() override;
     void GetVariationAxes(nsTArray<gfxFontVariationAxis>& aVariationAxes) override;
     void GetVariationInstances(nsTArray<gfxFontVariationInstance>& aInstances) override;
 
     // Check for various kinds of brokenness, and set flags on the entry
     // accordingly so that we avoid using bad font tables
     void CheckForBrokenFont(gfxFontFamily *aFamily);
 
+    FT_MM_Var* GetMMVar() override;
+
     virtual void AddSizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf,
                                         FontListSizes* aSizes) const override;
     virtual void AddSizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf,
                                         FontListSizes* aSizes) const override;
 
     FT_Face mFTFace;
     cairo_font_face_t *mFontFace;
 
+    FT_MM_Var* mMMVar = nullptr;
+
     nsCString mFilename;
     uint8_t   mFTFontIndex;
 
     mozilla::ThreadSafeWeakPtr<mozilla::gfx::UnscaledFontFreeType> mUnscaledFont;
 
     bool mHasVariations = false;
     bool mHasVariationsInitialized = false;
+    bool mMMVarInitialized = false;
 };
 
 class FT2FontFamily : public gfxFontFamily
 {
 public:
     explicit FT2FontFamily(const nsAString& aName) :
         gfxFontFamily(aName) { }
 
--- a/gfx/thebes/gfxFcPlatformFontList.cpp
+++ b/gfx/thebes/gfxFcPlatformFontList.cpp
@@ -779,17 +779,17 @@ gfxFontconfigFontEntry::CreateScaledFont
     }
 
     AutoTArray<FT_Fixed,8> coords;
     if (HasVariations()) {
         FT_Face ftFace = GetFTFace();
         if (ftFace) {
             AutoTArray<gfxFontVariation,8> settings;
             GetVariationsForStyle(settings, *aStyle);
-            gfxFT2FontBase::SetupVarCoords(ftFace, settings, &coords);
+            gfxFT2FontBase::SetupVarCoords(GetMMVar(), settings, &coords);
         }
     }
 
     cairo_font_face_t *face =
         cairo_ft_font_face_create_for_pattern(aRenderPattern,
                                               coords.Elements(),
                                               coords.Length());
 
--- a/gfx/thebes/gfxFcPlatformFontList.h
+++ b/gfx/thebes/gfxFcPlatformFontList.h
@@ -112,17 +112,17 @@ public:
 
     FcPattern* GetPattern() { return mFontPattern; }
 
     nsresult ReadCMAP(FontInfoData *aFontInfoData = nullptr) override;
     bool TestCharacterMap(uint32_t aCh) override;
 
     FT_Face GetFTFace();
 
-    FT_MM_Var* GetMMVar();
+    FT_MM_Var* GetMMVar() override;
 
     bool HasVariations() override;
     void GetVariationAxes(nsTArray<gfxFontVariationAxis>& aAxes) override;
     void GetVariationInstances(nsTArray<gfxFontVariationInstance>& aInstances) override;
 
     hb_blob_t* GetFontTable(uint32_t aTableTag) override;
 
     void ForgetHBFace() override;
--- a/gfx/thebes/gfxFontEntry.h
+++ b/gfx/thebes/gfxFontEntry.h
@@ -22,16 +22,17 @@
 #include "harfbuzz/hb.h"
 #include "mozilla/FontPropertyTypes.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/WeakPtr.h"
 #include <math.h>
 
 typedef struct gr_face gr_face;
+typedef struct FT_MM_Var_ FT_MM_Var;
 
 #ifdef DEBUG
 #include <stdio.h>
 #endif
 
 struct gfxFontStyle;
 class gfxContext;
 class gfxFont;
@@ -392,16 +393,19 @@ public:
     // Get variation axis settings that should be used to implement a particular
     // font style using this resource.
     void GetVariationsForStyle(nsTArray<gfxFontVariation>& aResult,
                                const gfxFontStyle& aStyle);
 
     // Get the font's list of features (if any) for DevTools support.
     void GetFeatureInfo(nsTArray<gfxFontFeatureInfo>& aFeatureInfo);
 
+    // This is only called on platforms where we use FreeType.
+    virtual FT_MM_Var* GetMMVar() { return nullptr; }
+
     nsString         mName;
     nsString         mFamilyName;
 
     RefPtr<gfxCharacterMap> mCharacterMap;
 
     mozilla::UniquePtr<uint8_t[]> mUVSData;
     mozilla::UniquePtr<gfxUserFontData> mUserFontData;
     mozilla::UniquePtr<gfxSVGGlyphs> mSVGGlyphs;