From b7d34cd5841e8733d1f3bc5e7eff4212a161c015 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=83=C2=A9d=C3=83=C2=A9ric=20Wang?= Date: Tue, 22 Apr 2014 08:44:04 -0400 Subject: [PATCH] Bug 407059 - Part 4: Use gfxHarfbuzzShaper::GetGlyphHAdvance() for math operators. r=jfkthame --- gfx/thebes/gfxFT2FontBase.cpp | 4 +- gfx/thebes/gfxFT2Fonts.cpp | 3 - gfx/thebes/gfxFont.cpp | 23 +++ gfx/thebes/gfxFont.h | 34 +++-- gfx/thebes/gfxHarfBuzzShaper.cpp | 240 ++++++++++++++++--------------- gfx/thebes/gfxHarfBuzzShaper.h | 6 + gfx/thebes/gfxPangoFonts.cpp | 3 - layout/mathml/nsMathMLChar.cpp | 22 ++- 8 files changed, 190 insertions(+), 145 deletions(-) diff --git a/gfx/thebes/gfxFT2FontBase.cpp b/gfx/thebes/gfxFT2FontBase.cpp index 0693b9aff38b..3755b5554bbc 100644 --- a/gfx/thebes/gfxFT2FontBase.cpp +++ b/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); diff --git a/gfx/thebes/gfxFT2Fonts.cpp b/gfx/thebes/gfxFT2Fonts.cpp index 6177548985ff..48836fa43b64 100644 --- a/gfx/thebes/gfxFT2Fonts.cpp +++ b/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, diff --git a/gfx/thebes/gfxFont.cpp b/gfx/thebes/gfxFont.cpp index c0aa3bb9c8c4..c4fcb932bf7d 100644 --- a/gfx/thebes/gfxFont.cpp +++ b/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(mHarfBuzzShaper.get()); + if (!shaper->Initialize() || !SetupCairoFont(aCtx)) { + return 0; + } + return shaper->GetGlyphHAdvance(aCtx, aGID) / 65536.0; +} + /*static*/ PLDHashOperator gfxFont::AgeCacheEntry(CacheHashEntry *aEntry, void *aUserData) diff --git a/gfx/thebes/gfxFont.h b/gfx/thebes/gfxFont.h index ac0443178cec..977b72fb72c0 100644 --- a/gfx/thebes/gfxFont.h +++ b/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; } - - // 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; - } + // Return the horizontal advance of a glyph. + gfxFloat GetGlyphHAdvance(gfxContext *aCtx, uint16_t aGID); // Return Azure GlyphRenderingOptions for drawing this font. virtual mozilla::TemporaryRef @@ -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); diff --git a/gfx/thebes/gfxHarfBuzzShaper.cpp b/gfx/thebes/gfxHarfBuzzShaper.cpp index 5c4b803490d9..84e4e4c845d3 100644 --- a/gfx/thebes/gfxHarfBuzzShaper.cpp +++ b/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 -HBGetGlyphHAdvance(hb_font_t *font, void *font_data, - hb_codepoint_t glyph, void *user_data) +/* static */ +hb_position_t +gfxHarfBuzzShaper::HBGetGlyphHAdvance(hb_font_t *font, void *font_data, + hb_codepoint_t glyph, void *user_data) { const gfxHarfBuzzShaper::FontCallbackData *fcd = static_cast(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 + (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) { - 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 - (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 + if (!Initialize()) { return false; } @@ -945,6 +958,7 @@ gfxHarfBuzzShaper::ShapeText(gfxContext *aContext, nsAutoTArray features; nsDataHashtable mergedFeatures; + gfxFontEntry *entry = mFont->GetFontEntry(); if (MergeFontFeatures(style, entry->mFeatureSettings, aShapedText->DisableLigatures(), diff --git a/gfx/thebes/gfxHarfBuzzShaper.h b/gfx/thebes/gfxHarfBuzzShaper.h index f5325f77291c..f82f70d75e6d 100644 --- a/gfx/thebes/gfxHarfBuzzShaper.h +++ b/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; diff --git a/gfx/thebes/gfxPangoFonts.cpp b/gfx/thebes/gfxPangoFonts.cpp index ff2654c28bc3..d1f0a54a356d 100644 --- a/gfx/thebes/gfxPangoFonts.cpp +++ b/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); diff --git a/layout/mathml/nsMathMLChar.cpp b/layout/mathml/nsMathMLChar.cpp index 8f8a0d0ba4b3..b97b5179181f 100644 --- a/layout/mathml/nsMathMLChar.cpp +++ b/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. - // XXXfredw: We should use gfxHarfbuzzShaper::GetGlyphHAdvance() - detailedGlyph.mAdvance = 0; + detailedGlyph.mAdvance = + NSToCoordRound(aAppUnitsPerDevPixel * + 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; } @@ -1302,10 +1298,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 - // subtract the italic correction, so that nsMathMLmmultiscripts will - // place the scripts correctly. - // Note that STIX-Word does not provide italic corrections + // MeasureTextRun should have set the advance width to the right + // bearing for OpenType MATH fonts. We now subtract the italic + // correction, so that nsMathMLmmultiscripts will place the scripts + // 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()->