Bug 1427641 - patch 8 - Refactor gfxFT2FontBase glyph-width code so that we properly respect variations when getting character widths during InitMetrics. r=lsalzman
authorJonathan Kew <jkew@mozilla.com>
Tue, 02 Jan 2018 13:29:54 +0000
changeset 449890 a85c5795cc6f0db71e13288e849ef47b2f225270
parent 449889 944248bbe7adaaf6eea61d22d163309fd99057a7
child 449891 ceb61da04fd81da9f9f835479e90359ccc50ebf7
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman
bugs1427641
milestone59.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 1427641 - patch 8 - Refactor gfxFT2FontBase glyph-width code so that we properly respect variations when getting character widths during InitMetrics. r=lsalzman
gfx/thebes/gfxFT2FontBase.cpp
gfx/thebes/gfxFT2FontBase.h
--- a/gfx/thebes/gfxFT2FontBase.cpp
+++ b/gfx/thebes/gfxFT2FontBase.cpp
@@ -158,16 +158,32 @@ gfxFT2FontBase::GetCharExtents(char aCha
 {
     FT_UInt gid = GetGlyph(aChar);
     if (gid) {
         GetGlyphExtents(gid, aExtents);
     }
     return gid;
 }
 
+/**
+ * Get glyph id and width for a simple character.
+ * The return value is the glyph id of that glyph or zero if no such glyph
+ * exists.  aWidth is only set when this returns a non-zero glyph id.
+ * This is just for use during initialization, and doesn't use the width cache.
+ */
+uint32_t
+gfxFT2FontBase::GetCharWidth(char aChar, gfxFloat* aWidth)
+{
+    FT_UInt gid = GetGlyph(aChar);
+    if (gid) {
+        *aWidth = FLOAT_FROM_16_16(GetFTGlyphAdvance(gid));
+    }
+    return gid;
+}
+
 void
 gfxFT2FontBase::InitMetrics()
 {
     mFUnitsConvFactor = 0.0;
 
     if (MOZ_UNLIKELY(GetStyle()->size <= 0.0) ||
         MOZ_UNLIKELY(GetStyle()->sizeAdjust == 0.0)) {
         memset(&mMetrics, 0, sizeof(mMetrics)); // zero initialize
@@ -371,34 +387,35 @@ gfxFT2FontBase::InitMetrics()
     } else {
         mMetrics.capHeight = mMetrics.maxAscent;
     }
 
     // Release the face lock to safely load glyphs with GetCharExtents if
     // necessary without recursively locking.
     cairo_ft_scaled_font_unlock_face(GetCairoScaledFont());
 
-    cairo_text_extents_t extents;
-    mSpaceGlyph = GetCharExtents(' ', &extents);
+    gfxFloat width;
+    mSpaceGlyph = GetCharWidth(' ', &width);
     if (mSpaceGlyph) {
-        mMetrics.spaceWidth = extents.x_advance;
+        mMetrics.spaceWidth = width;
     } else {
         mMetrics.spaceWidth = mMetrics.maxAdvance; // guess
     }
 
-    if (GetCharExtents('0', &extents)) {
-        mMetrics.zeroOrAveCharWidth = extents.x_advance;
+    if (GetCharWidth('0', &width)) {
+        mMetrics.zeroOrAveCharWidth = width;
     } else {
         mMetrics.zeroOrAveCharWidth = 0.0;
     }
 
     // Prefering a measured x over sxHeight because sxHeight doesn't consider
     // hinting, but maybe the x extents are not quite right in some fancy
     // script fonts.  CSS 2.1 suggests possibly using the height of an "o",
     // which would have a more consistent glyph across fonts.
+    cairo_text_extents_t extents;
     if (GetCharExtents('x', &extents) && extents.y_bearing < 0.0) {
         mMetrics.xHeight = -extents.y_bearing;
         mMetrics.aveCharWidth =
             std::max(mMetrics.aveCharWidth, extents.x_advance);
     }
 
     if (GetCharExtents('H', &extents) && extents.y_bearing < 0.0) {
         mMetrics.capHeight = -extents.y_bearing;
@@ -485,29 +502,19 @@ gfxFT2FontBase::GetGlyph(uint32_t unicod
             return GetGlyph(unicode);
         }
         return 0;
     }
 
     return GetGlyph(unicode);
 }
 
-int32_t
-gfxFT2FontBase::GetGlyphWidth(DrawTarget& aDrawTarget, uint16_t aGID)
+FT_Fixed
+gfxFT2FontBase::GetFTGlyphAdvance(uint16_t aGID)
 {
-    if (!mGlyphWidths) {
-        mGlyphWidths =
-            mozilla::MakeUnique<nsDataHashtable<nsUint32HashKey,int32_t>>(128);
-    }
-
-    int32_t width;
-    if (mGlyphWidths->Get(aGID, &width)) {
-        return width;
-    }
-
     gfxFT2LockedFace face(this);
     int32_t flags =
         gfxPlatform::GetPlatform()->FontHintingEnabled()
             ? FT_LOAD_ADVANCE_ONLY
             : FT_LOAD_ADVANCE_ONLY | FT_LOAD_NO_AUTOHINT | FT_LOAD_NO_HINTING;
     FT_Fixed advance = 0;
     // FT_Get_Advance is not reliable with variations until FreeType 2.8.2,
     // so fall back to calling FT_Load_Glyph and reading the glyph slot's
@@ -537,17 +544,33 @@ gfxFT2FontBase::GetGlyphWidth(DrawTarget
         // This is the embolden "strength" used by FT_GlyphSlot_Embolden,
         // converted from 26.6 to 16.16
         FT_Fixed strength = 1024 *
             FT_MulFix(face.get()->units_per_EM,
                       face.get()->size->metrics.y_scale) / 24;
         advance += strength;
     }
 
-    width = advance;
+    return advance;
+}
+
+int32_t
+gfxFT2FontBase::GetGlyphWidth(DrawTarget& aDrawTarget, uint16_t aGID)
+{
+    if (!mGlyphWidths) {
+        mGlyphWidths =
+            mozilla::MakeUnique<nsDataHashtable<nsUint32HashKey,int32_t>>(128);
+    }
+
+    int32_t width;
+    if (mGlyphWidths->Get(aGID, &width)) {
+        return width;
+    }
+
+    width = GetFTGlyphAdvance(aGID);
     mGlyphWidths->Put(aGID, width);
 
     return width;
 }
 
 bool
 gfxFT2FontBase::SetupCairoFont(DrawTarget* aDrawTarget)
 {
--- a/gfx/thebes/gfxFT2FontBase.h
+++ b/gfx/thebes/gfxFT2FontBase.h
@@ -39,16 +39,18 @@ public:
     virtual FontType GetType() const override { return FONT_TYPE_FT2; }
 
     static void SetupVarCoords(FT_Face aFace,
                                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);
+    FT_Fixed GetFTGlyphAdvance(uint16_t aGID);
     void InitMetrics();
 
 protected:
     virtual const Metrics& GetHorizontalMetrics() override;
 
     uint32_t mSpaceGlyph;
     Metrics mMetrics;
     bool    mEmbolden;