From 68d2010cfdd4a2dc2bef38741ba57a3dc0f20edb Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Sat, 31 May 2014 08:12:40 +0100 Subject: [PATCH] bug 1018551 - clean up redundancy in the font/shaper code. r=jdaggett --- gfx/thebes/gfxDWriteFonts.cpp | 36 --------------------------- gfx/thebes/gfxDWriteFonts.h | 8 ------ gfx/thebes/gfxFT2Fonts.cpp | 42 +++++++------------------------- gfx/thebes/gfxFT2Fonts.h | 3 +-- gfx/thebes/gfxFont.cpp | 35 +++++++++++++------------- gfx/thebes/gfxFont.h | 27 ++++++++------------ gfx/thebes/gfxGDIFont.cpp | 19 +++++---------- gfx/thebes/gfxGDIFont.h | 11 ++++----- gfx/thebes/gfxMacFont.cpp | 41 +++++++++++++------------------ gfx/thebes/gfxMacFont.h | 15 ++++++------ gfx/thebes/gfxPangoFonts.cpp | 46 ----------------------------------- 11 files changed, 72 insertions(+), 211 deletions(-) diff --git a/gfx/thebes/gfxDWriteFonts.cpp b/gfx/thebes/gfxDWriteFonts.cpp index 66f9cdd9cf85..80eb823a1197 100644 --- a/gfx/thebes/gfxDWriteFonts.cpp +++ b/gfx/thebes/gfxDWriteFonts.cpp @@ -7,9 +7,7 @@ #include "mozilla/MemoryReporting.h" -#include "gfxHarfBuzzShaper.h" #include -#include "gfxGraphiteShaper.h" #include "gfxDWriteFontList.h" #include "gfxContext.h" #include @@ -106,14 +104,6 @@ gfxDWriteFont::gfxDWriteFont(gfxFontEntry *aFontEntry, } ComputeMetrics(anAAOption); - - if (FontCanSupportGraphite()) { - mGraphiteShaper = new gfxGraphiteShaper(this); - } - - if (FontCanSupportHarfBuzz()) { - mHarfBuzzShaper = new gfxHarfBuzzShaper(this); - } } gfxDWriteFont::~gfxDWriteFont() @@ -134,32 +124,6 @@ gfxDWriteFont::CopyWithAntialiasOption(AntialiasOption anAAOption) &mStyle, mNeedsBold, anAAOption); } -bool -gfxDWriteFont::ShapeText(gfxContext *aContext, - const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping) -{ - bool ok = false; - - if (mGraphiteShaper && gfxPlatform::GetPlatform()->UseGraphiteShaping()) { - ok = mGraphiteShaper->ShapeText(aContext, aText, aOffset, aLength, - aScript, aShapedText); - } - - if (!ok && mHarfBuzzShaper) { - ok = mHarfBuzzShaper->ShapeText(aContext, aText, aOffset, aLength, - aScript, aShapedText); - } - - PostShapingFixup(aContext, aText, aOffset, aLength, aShapedText); - - return ok; -} - const gfxFont::Metrics& gfxDWriteFont::GetMetrics() { diff --git a/gfx/thebes/gfxDWriteFonts.h b/gfx/thebes/gfxDWriteFonts.h index 8ee972b553d3..a2adea8ebf2b 100644 --- a/gfx/thebes/gfxDWriteFonts.h +++ b/gfx/thebes/gfxDWriteFonts.h @@ -71,14 +71,6 @@ public: virtual cairo_scaled_font_t *GetCairoScaledFont(); protected: - virtual bool ShapeText(gfxContext *aContext, - const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping = false); - bool GetFakeMetricsForArialBlack(DWRITE_FONT_METRICS *aFontMetrics); void ComputeMetrics(AntialiasOption anAAOption); diff --git a/gfx/thebes/gfxFT2Fonts.cpp b/gfx/thebes/gfxFT2Fonts.cpp index cb5088bcacd4..fdc0e0e40cc2 100644 --- a/gfx/thebes/gfxFT2Fonts.cpp +++ b/gfx/thebes/gfxFT2Fonts.cpp @@ -24,8 +24,6 @@ #include "gfxFT2Utils.h" #include "gfxFT2FontList.h" #include -#include "gfxHarfBuzzShaper.h" -#include "gfxGraphiteShaper.h" #include "nsGkAtoms.h" #include "nsTArray.h" #include "nsUnicodeRange.h" @@ -44,42 +42,20 @@ */ bool -gfxFT2Font::ShapeText(gfxContext *aContext, +gfxFT2Font::ShapeText(gfxContext *aContext, const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping) + uint32_t aOffset, + uint32_t aLength, + int32_t aScript, + gfxShapedText *aShapedText) { - bool ok = false; - - if (FontCanSupportGraphite()) { - if (gfxPlatform::GetPlatform()->UseGraphiteShaping()) { - if (!mGraphiteShaper) { - mGraphiteShaper = new gfxGraphiteShaper(this); - } - ok = mGraphiteShaper->ShapeText(aContext, aText, - aOffset, aLength, - aScript, aShapedText); - } - } - - if (!ok) { - if (!mHarfBuzzShaper) { - mHarfBuzzShaper = new gfxHarfBuzzShaper(this); - } - ok = mHarfBuzzShaper->ShapeText(aContext, aText, - aOffset, aLength, - aScript, aShapedText); - } - - if (!ok) { + if (!gfxFont::ShapeText(aContext, aText, aOffset, aLength, aScript, + aShapedText)) { + // harfbuzz must have failed(?!), just render raw glyphs AddRange(aText, aOffset, aLength, aShapedText); + PostShapingFixup(aContext, aText, aOffset, aLength, aShapedText); } - PostShapingFixup(aContext, aText, aOffset, aLength, aShapedText); - return true; } diff --git a/gfx/thebes/gfxFT2Fonts.h b/gfx/thebes/gfxFT2Fonts.h index 871b9397f9a2..f0e390c5a60c 100644 --- a/gfx/thebes/gfxFT2Fonts.h +++ b/gfx/thebes/gfxFT2Fonts.h @@ -77,8 +77,7 @@ protected: uint32_t aOffset, uint32_t aLength, int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping); + gfxShapedText *aShapedText); void FillGlyphDataForChar(uint32_t ch, CachedGlyphData *gd); diff --git a/gfx/thebes/gfxFont.cpp b/gfx/thebes/gfxFont.cpp index 40a67889260a..36f8de70960a 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 "gfxGraphiteShaper.h" #include "gfxHarfBuzzShaper.h" #include "gfxUserFontSet.h" #include "gfxPlatformFontList.h" @@ -4074,8 +4075,7 @@ gfxFont::ShapeText(gfxContext *aContext, uint32_t aOffset, uint32_t aLength, int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping) + gfxShapedText *aShapedText) { nsDependentCSubstring ascii((const char*)aText, aLength); nsAutoString utf16; @@ -4084,7 +4084,7 @@ gfxFont::ShapeText(gfxContext *aContext, return false; } return ShapeText(aContext, utf16.BeginReading(), aOffset, aLength, - aScript, aShapedText, aPreferPlatformShaping); + aScript, aShapedText); } bool @@ -4093,30 +4093,29 @@ gfxFont::ShapeText(gfxContext *aContext, uint32_t aOffset, uint32_t aLength, int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping) + gfxShapedText *aShapedText) { bool ok = false; - if (mGraphiteShaper && gfxPlatform::GetPlatform()->UseGraphiteShaping()) { - ok = mGraphiteShaper->ShapeText(aContext, aText, aOffset, aLength, - aScript, aShapedText); + if (FontCanSupportGraphite()) { + if (gfxPlatform::GetPlatform()->UseGraphiteShaping()) { + if (!mGraphiteShaper) { + mGraphiteShaper = new gfxGraphiteShaper(this); + } + ok = mGraphiteShaper->ShapeText(aContext, aText, aOffset, aLength, + aScript, aShapedText); + } } - if (!ok && mHarfBuzzShaper && !aPreferPlatformShaping) { + if (!ok) { + if (!mHarfBuzzShaper) { + mHarfBuzzShaper = new gfxHarfBuzzShaper(this); + } ok = mHarfBuzzShaper->ShapeText(aContext, aText, aOffset, aLength, aScript, aShapedText); } - if (!ok) { - if (!mPlatformShaper) { - CreatePlatformShaper(); - } - if (mPlatformShaper) { - ok = mPlatformShaper->ShapeText(aContext, aText, aOffset, aLength, - aScript, aShapedText); - } - } + NS_WARN_IF_FALSE(ok, "shaper failed, expect scrambled or missing text"); PostShapingFixup(aContext, aText, aOffset, aLength, aShapedText); diff --git a/gfx/thebes/gfxFont.h b/gfx/thebes/gfxFont.h index 2138f2f857b3..fbbcadee7351 100644 --- a/gfx/thebes/gfxFont.h +++ b/gfx/thebes/gfxFont.h @@ -1479,12 +1479,12 @@ public: // Shape a piece of text and store the resulting glyph data into // aShapedText. Parameters aOffset/aLength indicate the range of // aShapedText to be updated; aLength is also the length of aText. - virtual bool ShapeText(gfxContext *aContext, + virtual bool ShapeText(gfxContext *aContext, const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText) = 0; + uint32_t aOffset, + uint32_t aLength, + int32_t aScript, + gfxShapedText *aShapedText) = 0; gfxFont *GetFont() const { return mFont; } @@ -1996,8 +1996,7 @@ protected: uint32_t aOffset, // dest offset in gfxShapedText uint32_t aLength, int32_t aScript, - gfxShapedText *aShapedText, // where to store the result - bool aPreferPlatformShaping = false); + gfxShapedText *aShapedText); // where to store the result // Call the appropriate shaper to generate glyphs for aText and store // them into aShapedText. @@ -2006,8 +2005,7 @@ protected: uint32_t aOffset, uint32_t aLength, int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping = false); + gfxShapedText *aShapedText); // Helper to adjust for synthetic bold and set character-type flags // in the shaped text; implementations of ShapeText should call this @@ -2166,19 +2164,14 @@ protected: // measurement by mathml code nsAutoPtr mNonAAFont; - // we may switch between these shapers on the fly, based on the script - // of the text run being shaped - nsAutoPtr mPlatformShaper; + // we create either or both of these shapers when needed, depending + // whether the font has graphite tables, and whether graphite shaping + // is actually enabled nsAutoPtr mHarfBuzzShaper; nsAutoPtr mGraphiteShaper; mozilla::RefPtr mAzureScaledFont; - // Create a default platform text shaper for this font. - // (TODO: This should become pure virtual once all font backends have - // been updated.) - virtual void CreatePlatformShaper() { } - // Helper for subclasses that want to initialize standard metrics from the // tables of sfnt (TrueType/OpenType) fonts. // This will use mFUnitsConvFactor if it is already set, else compute it diff --git a/gfx/thebes/gfxGDIFont.cpp b/gfx/thebes/gfxGDIFont.cpp index e5aedc9dcacd..f58d43426249 100644 --- a/gfx/thebes/gfxGDIFont.cpp +++ b/gfx/thebes/gfxGDIFont.cpp @@ -8,9 +8,7 @@ #include "mozilla/MemoryReporting.h" #include "mozilla/WindowsVersion.h" -#include "gfxHarfBuzzShaper.h" #include -#include "gfxGraphiteShaper.h" #include "gfxWindowsPlatform.h" #include "gfxContext.h" #include "mozilla/Preferences.h" @@ -52,10 +50,6 @@ gfxGDIFont::gfxGDIFont(GDIFontEntry *aFontEntry, mNeedsBold(aNeedsBold), mScriptCache(nullptr) { - if (FontCanSupportGraphite()) { - mGraphiteShaper = new gfxGraphiteShaper(this); - } - mHarfBuzzShaper = new gfxHarfBuzzShaper(this); } gfxGDIFont::~gfxGDIFont() @@ -83,13 +77,12 @@ gfxGDIFont::CopyWithAntialiasOption(AntialiasOption anAAOption) } bool -gfxGDIFont::ShapeText(gfxContext *aContext, +gfxGDIFont::ShapeText(gfxContext *aContext, const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping) + uint32_t aOffset, + uint32_t aLength, + int32_t aScript, + gfxShapedText *aShapedText) { if (!mMetrics) { Initialize(); @@ -108,7 +101,7 @@ gfxGDIFont::ShapeText(gfxContext *aContext, } return gfxFont::ShapeText(aContext, aText, aOffset, aLength, aScript, - aShapedText, aPreferPlatformShaping); + aShapedText); } const gfxFont::Metrics& diff --git a/gfx/thebes/gfxGDIFont.h b/gfx/thebes/gfxGDIFont.h index d6155f436210..c56cb17f98ff 100644 --- a/gfx/thebes/gfxGDIFont.h +++ b/gfx/thebes/gfxGDIFont.h @@ -72,13 +72,12 @@ public: protected: /* override to ensure the cairo font is set up properly */ - virtual bool ShapeText(gfxContext *aContext, + virtual bool ShapeText(gfxContext *aContext, const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping); + uint32_t aOffset, + uint32_t aLength, + int32_t aScript, + gfxShapedText *aShapedText); void Initialize(); // creates metrics and Cairo fonts diff --git a/gfx/thebes/gfxMacFont.cpp b/gfx/thebes/gfxMacFont.cpp index bf70a5637c18..8dd18d993e8e 100644 --- a/gfx/thebes/gfxMacFont.cpp +++ b/gfx/thebes/gfxMacFont.cpp @@ -8,9 +8,7 @@ #include "mozilla/MemoryReporting.h" #include "gfxCoreTextShaper.h" -#include "gfxHarfBuzzShaper.h" #include -#include "gfxGraphiteShaper.h" #include "gfxPlatformMac.h" #include "gfxContext.h" #include "gfxFontUtils.h" @@ -107,13 +105,6 @@ gfxMacFont::gfxMacFont(MacOSFontEntry *aFontEntry, const gfxFontStyle *aFontStyl NS_WARNING(warnBuf); #endif } - - if (FontCanSupportGraphite()) { - mGraphiteShaper = new gfxGraphiteShaper(this); - } - if (FontCanSupportHarfBuzz()) { - mHarfBuzzShaper = new gfxHarfBuzzShaper(this); - } } gfxMacFont::~gfxMacFont() @@ -127,29 +118,31 @@ gfxMacFont::~gfxMacFont() } bool -gfxMacFont::ShapeText(gfxContext *aContext, +gfxMacFont::ShapeText(gfxContext *aContext, const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping) + uint32_t aOffset, + uint32_t aLength, + int32_t aScript, + gfxShapedText *aShapedText) { if (!mIsValid) { NS_WARNING("invalid font! expect incorrect text rendering"); return false; } - bool requiresAAT = - static_cast(GetFontEntry())->RequiresAATLayout(); - return gfxFont::ShapeText(aContext, aText, aOffset, aLength, - aScript, aShapedText, requiresAAT); -} + if (static_cast(GetFontEntry())->RequiresAATLayout()) { + if (!mCoreTextShaper) { + mCoreTextShaper = new gfxCoreTextShaper(this); + } + if (mCoreTextShaper->ShapeText(aContext, aText, aOffset, aLength, + aScript, aShapedText)) { + PostShapingFixup(aContext, aText, aOffset, aLength, aShapedText); + return true; + } + } -void -gfxMacFont::CreatePlatformShaper() -{ - mPlatformShaper = new gfxCoreTextShaper(this); + return gfxFont::ShapeText(aContext, aText, aOffset, aLength, aScript, + aShapedText); } bool diff --git a/gfx/thebes/gfxMacFont.h b/gfx/thebes/gfxMacFont.h index 0e88c5d0f06b..3942213c0393 100644 --- a/gfx/thebes/gfxMacFont.h +++ b/gfx/thebes/gfxMacFont.h @@ -51,16 +51,13 @@ public: virtual FontType GetType() const { return FONT_TYPE_MAC; } protected: - virtual void CreatePlatformShaper(); - // override to prefer CoreText shaping with fonts that depend on AAT - virtual bool ShapeText(gfxContext *aContext, + virtual bool ShapeText(gfxContext *aContext, const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping = false); + uint32_t aOffset, + uint32_t aLength, + int32_t aScript, + gfxShapedText *aShapedText); void InitMetrics(); void InitMetricsFromPlatform(); @@ -76,6 +73,8 @@ protected: cairo_font_face_t *mFontFace; + nsAutoPtr mCoreTextShaper; + Metrics mMetrics; uint32_t mSpaceGlyph; }; diff --git a/gfx/thebes/gfxPangoFonts.cpp b/gfx/thebes/gfxPangoFonts.cpp index a519188f625f..c266c80e05f1 100644 --- a/gfx/thebes/gfxPangoFonts.cpp +++ b/gfx/thebes/gfxPangoFonts.cpp @@ -20,8 +20,6 @@ #include "gfxFT2Utils.h" #include "harfbuzz/hb.h" #include "harfbuzz/hb-ot.h" -#include "gfxHarfBuzzShaper.h" -#include "gfxGraphiteShaper.h" #include "nsUnicodeProperties.h" #include "nsUnicodeScriptCodes.h" #include "gfxFontconfigUtils.h" @@ -663,14 +661,6 @@ public: protected: virtual already_AddRefed GetSmallCapsFont(); - virtual bool ShapeText(gfxContext *aContext, - const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping); - private: gfxFcFont(cairo_scaled_font_t *aCairoFont, gfxFcFontEntry *aFontEntry, const gfxFontStyle *aFontStyle); @@ -1564,42 +1554,6 @@ gfxFcFont::GetSmallCapsFont() return font.forget(); } -bool -gfxFcFont::ShapeText(gfxContext *aContext, - const char16_t *aText, - uint32_t aOffset, - uint32_t aLength, - int32_t aScript, - gfxShapedText *aShapedText, - bool aPreferPlatformShaping) -{ - bool ok = false; - - if (FontCanSupportGraphite()) { - if (gfxPlatform::GetPlatform()->UseGraphiteShaping()) { - if (!mGraphiteShaper) { - mGraphiteShaper = new gfxGraphiteShaper(this); - } - ok = mGraphiteShaper->ShapeText(aContext, aText, aOffset, aLength, - aScript, aShapedText); - } - } - - if (!ok) { - if (!mHarfBuzzShaper) { - mHarfBuzzShaper = new gfxHarfBuzzShaper(this); - } - ok = mHarfBuzzShaper->ShapeText(aContext, aText, aOffset, aLength, - aScript, aShapedText); - } - - NS_WARN_IF_FALSE(ok, "shaper failed, expect scrambled or missing text"); - - PostShapingFixup(aContext, aText, aOffset, aLength, aShapedText); - - return ok; -} - /* static */ void gfxPangoFontGroup::Shutdown() {