Bug 1449605 - part 2 - Rename gfxFontEntry::IsBold() to SupportsBold() and make it smarter about variable fonts, to avoid inappropriate use of synthetic-bold effect. r=jwatt
authorJonathan Kew <jkew@mozilla.com>
Fri, 04 May 2018 10:19:55 +0100
changeset 473003 24583a2f0c84ca8a8fd08d8ce88fa7717d7c2a9d
parent 473002 6262a4f457d1fca0c5d59012994965f6217482d5
child 473004 3ce396c4e20f7d89c1f4bedc6a19c99ad1a47e26
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjwatt
bugs1449605
milestone61.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 1449605 - part 2 - Rename gfxFontEntry::IsBold() to SupportsBold() and make it smarter about variable fonts, to avoid inappropriate use of synthetic-bold effect. r=jwatt
gfx/thebes/gfxFont.h
gfx/thebes/gfxFontEntry.cpp
gfx/thebes/gfxFontEntry.h
--- a/gfx/thebes/gfxFont.h
+++ b/gfx/thebes/gfxFont.h
@@ -196,17 +196,17 @@ struct gfxFontStyle {
     // Adjust this style to simulate sub/superscript (as requested in the
     // variantSubSuper field) using size and baselineOffset instead.
     void AdjustForSubSuperscript(int32_t aAppUnitsPerDevPixel);
 
     // Should this style cause the given font entry to use synthetic bold?
     bool NeedsSyntheticBold(gfxFontEntry* aFontEntry) const {
         return weight.IsBold() &&
                allowSyntheticWeight &&
-               !aFontEntry->IsBold();
+               !aFontEntry->SupportsBold();
     }
 
     bool Equals(const gfxFontStyle& other) const {
         return
             (*reinterpret_cast<const uint64_t*>(&size) ==
              *reinterpret_cast<const uint64_t*>(&other.size)) &&
             (style == other.style) &&
             (weight == other.weight) &&
--- a/gfx/thebes/gfxFontEntry.cpp
+++ b/gfx/thebes/gfxFontEntry.cpp
@@ -74,17 +74,18 @@ gfxFontEntry::gfxFontEntry() :
     mSkipDefaultFeatureSpaceCheck(false),
     mGraphiteSpaceContextualsInitialized(false),
     mHasGraphiteSpaceContextuals(false),
     mSpaceGlyphIsInvisible(false),
     mSpaceGlyphIsInvisibleInitialized(false),
     mCheckedForGraphiteTables(false),
     mHasCmapTable(false),
     mGrFaceInitialized(false),
-    mCheckedForColorGlyph(false)
+    mCheckedForColorGlyph(false),
+    mCheckedForVariableWeight(false)
 {
     memset(&mDefaultSubSpaceFeatures, 0, sizeof(mDefaultSubSpaceFeatures));
     memset(&mNonDefaultSubSpaceFeatures, 0, sizeof(mNonDefaultSubSpaceFeatures));
 }
 
 gfxFontEntry::gfxFontEntry(const nsAString& aName, bool aIsStandardFace) :
     mName(aName),
     mFixedPitch(false),
@@ -103,17 +104,18 @@ gfxFontEntry::gfxFontEntry(const nsAStri
     mSkipDefaultFeatureSpaceCheck(false),
     mGraphiteSpaceContextualsInitialized(false),
     mHasGraphiteSpaceContextuals(false),
     mSpaceGlyphIsInvisible(false),
     mSpaceGlyphIsInvisibleInitialized(false),
     mCheckedForGraphiteTables(false),
     mHasCmapTable(false),
     mGrFaceInitialized(false),
-    mCheckedForColorGlyph(false)
+    mCheckedForColorGlyph(false),
+    mCheckedForVariableWeight(false)
 {
     memset(&mDefaultSubSpaceFeatures, 0, sizeof(mDefaultSubSpaceFeatures));
     memset(&mNonDefaultSubSpaceFeatures, 0, sizeof(mNonDefaultSubSpaceFeatures));
 }
 
 gfxFontEntry::~gfxFontEntry()
 {
     // Should not be dropped by stylo
@@ -1066,16 +1068,45 @@ gfxFontEntry::SetupVariationRanges()
 
         // case HB_TAG('i','t','a','l'): // XXX how to handle?
         default:
             continue;
         }
     }
 }
 
+bool
+gfxFontEntry::HasBoldVariableWeight()
+{
+    MOZ_ASSERT(!mIsUserFontContainer,
+               "should not be called for user-font containers!");
+
+    if (!gfxPlatform::GetPlatform()->HasVariationFontSupport()) {
+        return false;
+    }
+
+    if (!mCheckedForVariableWeight) {
+        if (HasVariations()) {
+            AutoTArray<gfxFontVariationAxis,4> axes;
+            GetVariationAxes(axes);
+            for (const auto& axis : axes) {
+                if (axis.mTag == HB_TAG('w','g','h','t') &&
+                    axis.mMaxValue >= 600.0f) {
+                    mRangeFlags |= RangeFlags::eBoldVariableWeight;
+                    break;
+                }
+            }
+        }
+        mCheckedForVariableWeight = true;
+    }
+
+    return (mRangeFlags & RangeFlags::eBoldVariableWeight) ==
+           RangeFlags::eBoldVariableWeight;
+}
+
 void
 gfxFontEntry::GetVariationsForStyle(nsTArray<gfxFontVariation>& aResult,
                                     const gfxFontStyle& aStyle)
 {
     if (!gfxPlatform::GetPlatform()->HasVariationFontSupport()) {
         return;
     }
 
@@ -1595,17 +1626,17 @@ gfxFontFamily::CheckForSimpleFamily()
         if (fe->Stretch() != firstStretch || fe->IsOblique()) {
             // simple families don't have varying font-stretch or oblique
             return;
         }
         if (!fe->Weight().IsSingle() || !fe->SlantStyle().IsSingle()) {
             return; // family with variation fonts is not considered "simple"
         }
         uint8_t faceIndex = (fe->IsItalic() ? kItalicMask : 0) |
-                            (fe->IsBold() ? kBoldMask : 0);
+                            (fe->SupportsBold() ? kBoldMask : 0);
         if (faces[faceIndex]) {
             return; // two faces resolve to the same slot; family isn't "simple"
         }
         faces[faceIndex] = fe;
     }
 
     // we have successfully slotted the available faces into the standard
     // 4-face framework
@@ -1668,17 +1699,17 @@ CalcStyleMatch(gfxFontEntry *aFontEntry,
         } else {
             rank += 2000.0f; // the font supports the exact weight wanted
         }
     } else {
         // if no font to match, prefer non-bold, non-italic fonts
         if (aFontEntry->IsUpright()) {
             rank += 2000.0f;
         }
-        if (!aFontEntry->IsBold()) {
+        if (aFontEntry->Weight().Min() <= FontWeight(500)) {
             rank += 1000.0f;
         }
     }
 
     return rank;
 }
 
 #define RANK_MATCHED_CMAP   10000.0f
--- a/gfx/thebes/gfxFontEntry.h
+++ b/gfx/thebes/gfxFontEntry.h
@@ -153,17 +153,17 @@ public:
     SlantStyleRange SlantStyle() const { return mStyleRange; }
 
     bool IsUserFont() const { return mIsDataUserFont || mIsLocalUserFont; }
     bool IsLocalUserFont() const { return mIsLocalUserFont; }
     bool IsFixedPitch() const { return mFixedPitch; }
     bool IsItalic() const { return SlantStyle().Min().IsItalic(); }
     bool IsOblique() const { return SlantStyle().Min().IsOblique(); }
     bool IsUpright() const { return SlantStyle().Min().IsNormal(); }
-    bool IsBold() const { return Weight().Max().IsBold(); } // bold == weights 600 and above
+    inline bool SupportsBold(); // defined below, because of RangeFlags use
     bool IgnoreGDEF() const { return mIgnoreGDEF; }
     bool IgnoreGSUB() const { return mIgnoreGSUB; }
 
     // Return whether the face corresponds to "normal" CSS style properties:
     //    font-style: normal;
     //    font-weight: normal;
     //    font-stretch: normal;
     // If this is false, we might want to fall back to a different face and
@@ -373,16 +373,18 @@ public:
     virtual bool HasVariations() = 0;
 
     virtual void
     GetVariationAxes(nsTArray<gfxFontVariationAxis>& aVariationAxes) = 0;
 
     virtual void
     GetVariationInstances(nsTArray<gfxFontVariationInstance>& aInstances) = 0;
 
+    bool HasBoldVariableWeight();
+
     // Set up the entry's weight/stretch/style ranges according to axes found
     // by GetVariationAxes (for installed fonts; do NOT call this for user
     // fonts, where the ranges are provided by @font-face descriptors).
     void SetupVariationRanges();
 
     // Get variation axis settings that should be used to implement a particular
     // font style using this resource.
     void GetVariationsForStyle(nsTArray<gfxFontVariation>& aResult,
@@ -428,20 +430,27 @@ public:
     // properties. When the @font-face rule omitted one or more of these
     // descriptors, it is treated as the initial value for font-matching (and
     // so that is what we record in the font entry), but when rendering the
     // range is NOT clamped.
     enum class RangeFlags : uint8_t {
         eNoFlags        = 0,
         eAutoWeight     = (1 << 0),
         eAutoStretch    = (1 << 1),
-        eAutoSlantStyle = (1 << 2)
+        eAutoSlantStyle = (1 << 2),
+        // Flag to record whether the face has a variable "wght" axis
+        // that supports "bold" values, used to disable the application
+        // of synthetic-bold effects.
+        eBoldVariableWeight = (1 << 3)
     };
     RangeFlags       mRangeFlags = RangeFlags::eNoFlags;
 
+    // NOTE that there are currently exactly 24 one-bit flags defined here,
+    // so together with the 8-bit RangeFlags above, this packs neatly to a
+    // 32-bit boundary. Worth considering if further flags are wanted.
     bool             mFixedPitch  : 1;
     bool             mIsBadUnderlineFont : 1;
     bool             mIsUserFontContainer : 1; // userfont entry
     bool             mIsDataUserFont : 1;      // platform font entry (data)
     bool             mIsLocalUserFont : 1;     // platform font entry (local)
     bool             mStandardFace : 1;
     bool             mIgnoreGDEF  : 1;
     bool             mIgnoreGSUB  : 1;
@@ -455,16 +464,17 @@ public:
     bool             mHasGraphiteSpaceContextuals : 1;
     bool             mSpaceGlyphIsInvisible : 1;
     bool             mSpaceGlyphIsInvisibleInitialized : 1;
     bool             mHasGraphiteTables : 1;
     bool             mCheckedForGraphiteTables : 1;
     bool             mHasCmapTable : 1;
     bool             mGrFaceInitialized : 1;
     bool             mCheckedForColorGlyph : 1;
+    bool             mCheckedForVariableWeight : 1;
 
 protected:
     friend class gfxPlatformFontList;
     friend class gfxMacPlatformFontList;
     friend class gfxUserFcFontEntry;
     friend class gfxFontFamily;
     friend class gfxSingleFaceMacFontFamily;
     friend class gfxUserFontEntry;
@@ -637,16 +647,29 @@ private:
     mozilla::UniquePtr<nsTHashtable<FontTableHashEntry> > mFontTableCache;
 
     gfxFontEntry(const gfxFontEntry&);
     gfxFontEntry& operator=(const gfxFontEntry&);
 };
 
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(gfxFontEntry::RangeFlags)
 
+inline bool
+gfxFontEntry::SupportsBold()
+{
+    // bold == weights 600 and above
+    // We return true if the face has a max weight descriptor >= 600,
+    // OR if it's a user font with auto-weight (no descriptor) and has
+    // a weight axis that supports values >= 600
+    return Weight().Max().IsBold() ||
+           ((mRangeFlags & RangeFlags::eAutoWeight) ==
+               RangeFlags::eAutoWeight &&
+            HasBoldVariableWeight());
+}
+
 // used when iterating over all fonts looking for a match for a given character
 struct GlobalFontMatch {
     GlobalFontMatch(const uint32_t aCharacter,
                     const gfxFontStyle *aStyle) :
         mCh(aCharacter), mStyle(aStyle),
         mMatchRank(0.0f), mCount(0), mCmapsTested(0)
         {