Bug 407059 - Part 4: Use gfxHarfbuzzShaper::GetGlyphHAdvance() for math operators. r=jfkthame

gfx/thebes/gfxFT2FontBase.cpp

@@ -117,7 +117,9 @@ gfxFT2FontBase::GetMetrics()
         new(&mMetrics) gfxFont::Metrics(); // zero initialize
         mSpaceGlyph = 0;
     } else {
-        gfxFT2LockedFace(this).GetMetrics(&mMetrics, &mSpaceGlyph);
+        gfxFT2LockedFace face(this);
+        mFUnitsConvFactor = face.XScale();
+        face.GetMetrics(&mMetrics, &mSpaceGlyph);
     }
 
     SanitizeMetrics(&mMetrics, false);

gfx/thebes/gfxFT2Fonts.cpp

@@ -426,9 +426,6 @@ gfxFT2Font::ShapeText(gfxContext      *aContext,
 
     if (!ok && gfxPlatform::GetPlatform()->UseHarfBuzzForScript(aScript)) {
         if (!mHarfBuzzShaper) {
-            gfxFT2LockedFace face(this);
-            mFUnitsConvFactor = face.XScale();
-
             mHarfBuzzShaper = new gfxHarfBuzzShaper(this);
         }
         ok = mHarfBuzzShaper->ShapeText(aContext, aText,

gfx/thebes/gfxFont.cpp

@@ -23,6 +23,7 @@
 #include "gfxTypes.h"
 #include "gfxContext.h"
 #include "gfxFontMissingGlyphs.h"
+#include "gfxHarfBuzzShaper.h"
 #include "gfxUserFontSet.h"
 #include "gfxPlatformFontList.h"
 #include "gfxScriptItemizer.h"
@@ -2027,6 +2028,28 @@ gfxFont::~gfxFont()
     }
 }
 
+gfxFloat
+gfxFont::GetGlyphHAdvance(gfxContext *aCtx, uint16_t aGID)
+{
+    if (ProvidesGlyphWidths()) {
+        return GetGlyphWidth(aCtx, aGID) / 65536.0;
+    }
+    if (mFUnitsConvFactor == 0.0f) {
+        GetMetrics();
+    }
+    NS_ASSERTION(mFUnitsConvFactor > 0.0f,
+                 "missing font unit conversion factor");
+    if (!mHarfBuzzShaper) {
+        mHarfBuzzShaper = new gfxHarfBuzzShaper(this);
+    }
+    gfxHarfBuzzShaper* shaper =
+        static_cast<gfxHarfBuzzShaper*>(mHarfBuzzShaper.get());
+    if (!shaper->Initialize() || !SetupCairoFont(aCtx)) {
+        return 0;
+    }
+    return shaper->GetGlyphHAdvance(aCtx, aGID) / 65536.0;
+}
+
 /*static*/
 PLDHashOperator
 gfxFont::AgeCacheEntry(CacheHashEntry *aEntry, void *aUserData)

gfx/thebes/gfxFont.h

@@ -43,6 +43,8 @@ class gfxTextRun;
 class gfxFont;
 class gfxFontFamily;
 class gfxFontGroup;
+class gfxGraphiteShaper;
+class gfxHarfBuzzShaper;
 class gfxUserFontSet;
 class gfxUserFontData;
 class gfxShapedText;
@@ -1457,6 +1459,10 @@ protected:
 
 /* a SPECIFIC single font family */
 class gfxFont {
+
+    friend class gfxHarfBuzzShaper;
+    friend class gfxGraphiteShaper;
+
 public:
     nsrefcnt AddRef(void) {
         NS_PRECONDITION(int32_t(mRefCnt) >= 0, "illegal refcnt");
@@ -1584,19 +1590,8 @@ public:
     virtual uint32_t GetGlyph(uint32_t unicode, uint32_t variation_selector) {
         return 0;
     }
-
+    // Return the horizontal advance of a glyph.
-    // subclasses may provide (possibly hinted) glyph widths (in font units);
+    gfxFloat GetGlyphHAdvance(gfxContext *aCtx, uint16_t aGID);
-    // if they do not override this, harfbuzz will use unhinted widths
-    // derived from the font tables
-    virtual bool ProvidesGlyphWidths() {
-        return false;
-    }
-
-    // The return value is interpreted as a horizontal advance in 16.16 fixed
-    // point format.
-    virtual int32_t GetGlyphWidth(gfxContext *aCtx, uint16_t aGID) {
-        return -1;
-    }
 
     // Return Azure GlyphRenderingOptions for drawing this font.
     virtual mozilla::TemporaryRef<mozilla::gfx::GlyphRenderingOptions>
@@ -1892,6 +1887,19 @@ public:
     }
 
 protected:
+    // subclasses may provide (possibly hinted) glyph widths (in font units);
+    // if they do not override this, harfbuzz will use unhinted widths
+    // derived from the font tables
+    virtual bool ProvidesGlyphWidths() {
+        return false;
+    }
+
+    // The return value is interpreted as a horizontal advance in 16.16 fixed
+    // point format.
+    virtual int32_t GetGlyphWidth(gfxContext *aCtx, uint16_t aGID) {
+        return -1;
+    }
+
     void AddGlyphChangeObserver(GlyphChangeObserver *aObserver);
     void RemoveGlyphChangeObserver(GlyphChangeObserver *aObserver);
 

gfx/thebes/gfxHarfBuzzShaper.cpp

@@ -188,10 +188,6 @@ hb_position_t
 gfxHarfBuzzShaper::GetGlyphHAdvance(gfxContext *aContext,
                                     hb_codepoint_t glyph) const
 {
-    if (mUseFontGlyphWidths) {
-        return mFont->GetGlyphWidth(aContext, glyph);
-    }
-
     // font did not implement GetHintedGlyphWidth, so get an unhinted value
     // directly from the font tables
 
@@ -211,13 +207,19 @@ gfxHarfBuzzShaper::GetGlyphHAdvance(gfxContext *aContext,
                         uint16_t(hmtx->metrics[glyph].advanceWidth));
 }
 
-static hb_position_t
+/* static */
-HBGetGlyphHAdvance(hb_font_t *font, void *font_data,
+hb_position_t
-                   hb_codepoint_t glyph, void *user_data)
+gfxHarfBuzzShaper::HBGetGlyphHAdvance(hb_font_t *font, void *font_data,
+                                      hb_codepoint_t glyph, void *user_data)
 {
     const gfxHarfBuzzShaper::FontCallbackData *fcd =
         static_cast<const gfxHarfBuzzShaper::FontCallbackData*>(font_data);
-    return fcd->mShaper->GetGlyphHAdvance(fcd->mContext, glyph);
+    gfxFont *gfxfont = fcd->mShaper->GetFont();
+    if (gfxfont->ProvidesGlyphWidths()) {
+        return gfxfont->GetGlyphWidth(fcd->mContext, glyph);
+    } else {
+        return fcd->mShaper->GetGlyphHAdvance(fcd->mContext, glyph);
+    }
 }
 
 static hb_bool_t
@@ -817,6 +819,121 @@ static hb_unicode_funcs_t * sHBUnicodeFuncs = nullptr;
 static const hb_script_t sMathScript =
     hb_ot_tag_to_script(HB_TAG('m','a','t','h'));
 
+bool
+gfxHarfBuzzShaper::Initialize()
+{
+    if (mInitialized) {
+        return mHBFont != nullptr;
+    }
+    mInitialized = true;
+    mCallbackData.mShaper = this;
+
+    mUseFontGlyphWidths = mFont->ProvidesGlyphWidths();
+
+    if (!sHBFontFuncs) {
+        // static function callback pointers, initialized by the first
+        // harfbuzz shaper used
+        sHBFontFuncs = hb_font_funcs_create();
+        hb_font_funcs_set_glyph_func(sHBFontFuncs, HBGetGlyph,
+                                     nullptr, nullptr);
+        hb_font_funcs_set_glyph_h_advance_func(sHBFontFuncs,
+                                               HBGetGlyphHAdvance,
+                                               nullptr, nullptr);
+        hb_font_funcs_set_glyph_contour_point_func(sHBFontFuncs,
+                                                   HBGetContourPoint,
+                                                   nullptr, nullptr);
+        hb_font_funcs_set_glyph_h_kerning_func(sHBFontFuncs,
+                                               HBGetHKerning,
+                                               nullptr, nullptr);
+
+        sHBUnicodeFuncs =
+            hb_unicode_funcs_create(hb_unicode_funcs_get_empty());
+        hb_unicode_funcs_set_mirroring_func(sHBUnicodeFuncs,
+                                            HBGetMirroring,
+                                            nullptr, nullptr);
+        hb_unicode_funcs_set_script_func(sHBUnicodeFuncs, HBGetScript,
+                                         nullptr, nullptr);
+        hb_unicode_funcs_set_general_category_func(sHBUnicodeFuncs,
+                                                   HBGetGeneralCategory,
+                                                   nullptr, nullptr);
+        hb_unicode_funcs_set_combining_class_func(sHBUnicodeFuncs,
+                                                  HBGetCombiningClass,
+                                                  nullptr, nullptr);
+        hb_unicode_funcs_set_eastasian_width_func(sHBUnicodeFuncs,
+                                                  HBGetEastAsianWidth,
+                                                  nullptr, nullptr);
+        hb_unicode_funcs_set_compose_func(sHBUnicodeFuncs,
+                                          HBUnicodeCompose,
+                                          nullptr, nullptr);
+        hb_unicode_funcs_set_decompose_func(sHBUnicodeFuncs,
+                                            HBUnicodeDecompose,
+                                            nullptr, nullptr);
+    }
+
+    gfxFontEntry *entry = mFont->GetFontEntry();
+    if (!mUseFontGetGlyph) {
+        // get the cmap table and find offset to our subtable
+        mCmapTable = entry->GetFontTable(TRUETYPE_TAG('c','m','a','p'));
+        if (!mCmapTable) {
+            NS_WARNING("failed to load cmap, glyphs will be missing");
+            return false;
+        }
+        uint32_t len;
+        const uint8_t* data = (const uint8_t*)hb_blob_get_data(mCmapTable, &len);
+        bool symbol;
+        mCmapFormat = gfxFontUtils::
+            FindPreferredSubtable(data, len,
+                                  &mSubtableOffset, &mUVSTableOffset,
+                                  &symbol);
+        if (mCmapFormat <= 0) {
+            return false;
+        }
+    }
+
+    if (!mUseFontGlyphWidths) {
+        // if font doesn't implement GetGlyphWidth, we will be reading
+        // the hmtx table directly;
+        // read mNumLongMetrics from hhea table without caching its blob,
+        // and preload/cache the hmtx table
+        gfxFontEntry::AutoTable hheaTable(entry, TRUETYPE_TAG('h','h','e','a'));
+        if (hheaTable) {
+            uint32_t len;
+            const HMetricsHeader* hhea =
+                reinterpret_cast<const HMetricsHeader*>
+                (hb_blob_get_data(hheaTable, &len));
+            if (len >= sizeof(HMetricsHeader)) {
+                mNumLongMetrics = hhea->numberOfHMetrics;
+                if (mNumLongMetrics > 0 &&
+                    int16_t(hhea->metricDataFormat) == 0) {
+                    // no point reading hmtx if number of entries is zero!
+                    // in that case, we won't be able to use this font
+                    // (this method will return FALSE below if mHmtx is null)
+                    mHmtxTable =
+                        entry->GetFontTable(TRUETYPE_TAG('h','m','t','x'));
+                    if (hb_blob_get_length(mHmtxTable) <
+                        mNumLongMetrics * sizeof(HLongMetric)) {
+                        // hmtx table is not large enough for the claimed
+                        // number of entries: invalid, do not use.
+                        hb_blob_destroy(mHmtxTable);
+                        mHmtxTable = nullptr;
+                    }
+                }
+            }
+        }
+        if (!mHmtxTable) {
+            return false;
+        }
+    }
+
+    mHBFont = hb_font_create(mHBFace);
+    hb_font_set_funcs(mHBFont, sHBFontFuncs, &mCallbackData, nullptr);
+    hb_font_set_ppem(mHBFont, mFont->GetAdjustedSize(), mFont->GetAdjustedSize());
+    uint32_t scale = FloatToFixed(mFont->GetAdjustedSize()); // 16.16 fixed-point
+    hb_font_set_scale(mHBFont, scale, scale);
+
+    return true;
+}
+
 bool
 gfxHarfBuzzShaper::ShapeText(gfxContext      *aContext,
                              const char16_t *aText,
@@ -831,112 +948,8 @@ gfxHarfBuzzShaper::ShapeText(gfxContext      *aContext,
     }
 
     mCallbackData.mContext = aContext;
-    gfxFontEntry *entry = mFont->GetFontEntry();
 
-    if (!mInitialized) {
+    if (!Initialize()) {
-        mInitialized = true;
-        mCallbackData.mShaper = this;
-
-        mUseFontGlyphWidths = mFont->ProvidesGlyphWidths();
-
-        if (!sHBFontFuncs) {
-            // static function callback pointers, initialized by the first
-            // harfbuzz shaper used
-            sHBFontFuncs = hb_font_funcs_create();
-            hb_font_funcs_set_glyph_func(sHBFontFuncs, HBGetGlyph,
-                                         nullptr, nullptr);
-            hb_font_funcs_set_glyph_h_advance_func(sHBFontFuncs,
-                                                   HBGetGlyphHAdvance,
-                                                   nullptr, nullptr);
-            hb_font_funcs_set_glyph_contour_point_func(sHBFontFuncs,
-                                                       HBGetContourPoint,
-                                                       nullptr, nullptr);
-            hb_font_funcs_set_glyph_h_kerning_func(sHBFontFuncs,
-                                                   HBGetHKerning,
-                                                   nullptr, nullptr);
-
-            sHBUnicodeFuncs =
-                hb_unicode_funcs_create(hb_unicode_funcs_get_empty());
-            hb_unicode_funcs_set_mirroring_func(sHBUnicodeFuncs,
-                                                HBGetMirroring,
-                                                nullptr, nullptr);
-            hb_unicode_funcs_set_script_func(sHBUnicodeFuncs, HBGetScript,
-                                             nullptr, nullptr);
-            hb_unicode_funcs_set_general_category_func(sHBUnicodeFuncs,
-                                                       HBGetGeneralCategory,
-                                                       nullptr, nullptr);
-            hb_unicode_funcs_set_combining_class_func(sHBUnicodeFuncs,
-                                                      HBGetCombiningClass,
-                                                      nullptr, nullptr);
-            hb_unicode_funcs_set_eastasian_width_func(sHBUnicodeFuncs,
-                                                      HBGetEastAsianWidth,
-                                                      nullptr, nullptr);
-            hb_unicode_funcs_set_compose_func(sHBUnicodeFuncs,
-                                              HBUnicodeCompose,
-                                              nullptr, nullptr);
-            hb_unicode_funcs_set_decompose_func(sHBUnicodeFuncs,
-                                                HBUnicodeDecompose,
-                                                nullptr, nullptr);
-        }
-
-        if (!mUseFontGetGlyph) {
-            // get the cmap table and find offset to our subtable
-            mCmapTable = entry->GetFontTable(TRUETYPE_TAG('c','m','a','p'));
-            if (!mCmapTable) {
-                NS_WARNING("failed to load cmap, glyphs will be missing");
-                return false;
-            }
-            uint32_t len;
-            const uint8_t* data = (const uint8_t*)hb_blob_get_data(mCmapTable, &len);
-            bool symbol;
-            mCmapFormat = gfxFontUtils::
-                FindPreferredSubtable(data, len,
-                                      &mSubtableOffset, &mUVSTableOffset,
-                                      &symbol);
-        }
-
-        if (!mUseFontGlyphWidths) {
-            // if font doesn't implement GetGlyphWidth, we will be reading
-            // the hmtx table directly;
-            // read mNumLongMetrics from hhea table without caching its blob,
-            // and preload/cache the hmtx table
-            gfxFontEntry::AutoTable hheaTable(entry, TRUETYPE_TAG('h','h','e','a'));
-            if (hheaTable) {
-                uint32_t len;
-                const HMetricsHeader* hhea =
-                    reinterpret_cast<const HMetricsHeader*>
-                        (hb_blob_get_data(hheaTable, &len));
-                if (len >= sizeof(HMetricsHeader)) {
-                    mNumLongMetrics = hhea->numberOfHMetrics;
-                    if (mNumLongMetrics > 0 &&
-                        int16_t(hhea->metricDataFormat) == 0) {
-                        // no point reading hmtx if number of entries is zero!
-                        // in that case, we won't be able to use this font
-                        // (this method will return FALSE below if mHmtx is null)
-                        mHmtxTable =
-                            entry->GetFontTable(TRUETYPE_TAG('h','m','t','x'));
-                        if (hb_blob_get_length(mHmtxTable) <
-                            mNumLongMetrics * sizeof(HLongMetric)) {
-                            // hmtx table is not large enough for the claimed
-                            // number of entries: invalid, do not use.
-                            hb_blob_destroy(mHmtxTable);
-                            mHmtxTable = nullptr;
-                        }
-                    }
-                }
-            }
-        }
-
-        mHBFont = hb_font_create(mHBFace);
-        hb_font_set_funcs(mHBFont, sHBFontFuncs, &mCallbackData, nullptr);
-        hb_font_set_ppem(mHBFont, mFont->GetAdjustedSize(), mFont->GetAdjustedSize());
-        uint32_t scale = FloatToFixed(mFont->GetAdjustedSize()); // 16.16 fixed-point
-        hb_font_set_scale(mHBFont, scale, scale);
-    }
-
-    if ((!mUseFontGetGlyph && mCmapFormat <= 0) ||
-        (!mUseFontGlyphWidths && !mHmtxTable)) {
-        // unable to shape with this font
         return false;
     }
 
@@ -945,6 +958,7 @@ gfxHarfBuzzShaper::ShapeText(gfxContext      *aContext,
     nsAutoTArray<hb_feature_t,20> features;
     nsDataHashtable<nsUint32HashKey,uint32_t> mergedFeatures;
 
+    gfxFontEntry *entry = mFont->GetFontEntry();
     if (MergeFontFeatures(style,
                           entry->mFeatureSettings,
                           aShapedText->DisableLigatures(),

gfx/thebes/gfxHarfBuzzShaper.h

@@ -24,6 +24,7 @@ public:
         gfxContext        *mContext;
     };
 
+    bool Initialize();
     virtual bool ShapeText(gfxContext      *aContext,
                            const char16_t *aText,
                            uint32_t         aOffset,
@@ -42,6 +43,11 @@ public:
     hb_position_t GetGlyphHAdvance(gfxContext *aContext,
                                    hb_codepoint_t glyph) const;
 
+    // get harfbuzz horizontal advance in 16.16 fixed point format.
+    static hb_position_t
+    HBGetGlyphHAdvance(hb_font_t *font, void *font_data,
+                       hb_codepoint_t glyph, void *user_data);
+
     hb_position_t GetHKerning(uint16_t aFirstGlyph,
                               uint16_t aSecondGlyph) const;
 

gfx/thebes/gfxPangoFonts.cpp

@@ -1624,10 +1624,7 @@ gfxFcFont::ShapeText(gfxContext      *aContext,
 
     if (!ok) {
         if (!mHarfBuzzShaper) {
-            gfxFT2LockedFace face(this);
             mHarfBuzzShaper = new gfxHarfBuzzShaper(this);
-            // Used by gfxHarfBuzzShaper, currently only for kerning
-            mFUnitsConvFactor = face.XScale();
         }
         ok = mHarfBuzzShaper->ShapeText(aContext, aText, aOffset, aLength,
                                         aScript, aShapedText);

layout/mathml/nsMathMLChar.cpp

@@ -542,9 +542,10 @@ nsOpenTypeTable::MakeTextRun(gfxContext*        aThebesContext,
                        false);
   gfxTextRun::DetailedGlyph detailedGlyph;
   detailedGlyph.mGlyphID = aGlyph.glyphID;
-  // We set the advance width to zero and this will be fixed in MeasureTextRun.
+  detailedGlyph.mAdvance =
-  // XXXfredw: We should use gfxHarfbuzzShaper::GetGlyphHAdvance()
+    NSToCoordRound(aAppUnitsPerDevPixel *
-  detailedGlyph.mAdvance = 0;
+                   aFontGroup->GetFontAt(0)->
+                   GetGlyphHAdvance(aThebesContext, aGlyph.glyphID));
   detailedGlyph.mXOffset = detailedGlyph.mYOffset = 0;
   gfxShapedText::CompressedGlyph g;
   g.SetComplex(true, true, 1);
@@ -1153,11 +1154,6 @@ MeasureTextRun(gfxContext* aThebesContext, gfxTextRun* aTextRun)
   bm.ascent = NSToCoordCeil(-metrics.mBoundingBox.Y());
   bm.descent = NSToCoordCeil(metrics.mBoundingBox.YMost());
   bm.width = NSToCoordRound(metrics.mAdvanceWidth);
-  if (bm.width == 0) {
-    // The advance width was not set in nsGlyphTable::MakeTextRun, so we use
-    // the right bearing instead.
-    bm.width = bm.rightBearing;
-  }
 
   return bm;
 }
@@ -1296,10 +1292,12 @@ StretchEnumContext::TryVariants(nsGlyphTable* aGlyphTable,
     if (ch.IsGlyphID()) {
       gfxFont* mathFont = aFontGroup->get()->GetFontAt(0);
       if (mathFont->GetFontEntry()->TryGetMathTable(mathFont)) {
-        // MeasureTextRun has set the advance width to the right bearing. We now
+        // MeasureTextRun should have set the advance width to the right
-        // subtract the italic correction, so that nsMathMLmmultiscripts will
+        // bearing for OpenType MATH fonts. We now subtract the italic
-        // place the scripts correctly.
+        // correction, so that nsMathMLmmultiscripts will place the scripts
-        // Note that STIX-Word does not provide italic corrections
+        // correctly.
+        // Note that STIX-Word does not provide italic corrections but its
+        // advance widths do not match right bearings.
         // (http://sourceforge.net/p/stixfonts/tracking/50/)
         gfxFloat italicCorrection;
         if (mathFont->GetFontEntry()->