Bug 1706077 - Move setting of 'opsz' axis from nsFont into gfxFont initialization as it depends on data from both the style and the specific gfxFontEntry. r=layout-reviewers,emilio

Differential Revision: https://phabricator.services.mozilla.com/D112631

gfx/src/nsFont.cpp

@@ -280,8 +280,7 @@ void nsFont::AddFontVariationsToStyle(gfxFontStyle* aStyle) const {
   const uint32_t kTagOpsz = TRUETYPE_TAG('o', 'p', 's', 'z');
   if (opticalSizing == NS_FONT_OPTICAL_SIZING_AUTO &&
       !fontVariationSettings.Contains(kTagOpsz, VariationTagComparator())) {
-    gfxFontVariation opsz = {kTagOpsz, size.ToCSSPixels()};
-    aStyle->variationSettings.AppendElement(opsz);
+    aStyle->autoOpticalSize = size.ToCSSPixels();
   }
 
   // Add in arbitrary values from font-variation-settings

gfx/thebes/gfxDWriteFontList.cpp

@@ -579,12 +579,18 @@ bool gfxDWriteFontEntry::HasVariations() {
     return mHasVariations;
   }
   mHasVariationsInitialized = true;
+  mHasVariations = false;
+
+  if (!gfxPlatform::GetPlatform()->HasVariationFontSupport()) {
+    return mHasVariations;
+  }
+
   if (!mFontFace) {
     // CreateFontFace will initialize the mFontFace field, and also
     // mFontFace5 if available on the current DWrite version.
     RefPtr<IDWriteFontFace> fontFace;
     if (NS_FAILED(CreateFontFace(getter_AddRefs(fontFace)))) {
-      return false;
+      return mHasVariations;
     }
   }
   if (mFontFace5) {
@@ -656,7 +662,8 @@ gfxFont* gfxDWriteFontEntry::CreateFontInstance(
   RefPtr<UnscaledFontDWrite> unscaledFont(unscaledFontPtr);
   if (!unscaledFont) {
     RefPtr<IDWriteFontFace> fontFace;
-    nsresult rv = CreateFontFace(getter_AddRefs(fontFace), nullptr, sims);
+    nsresult rv =
+        CreateFontFace(getter_AddRefs(fontFace), nullptr, sims, nullptr);
     if (NS_FAILED(rv)) {
       return nullptr;
     }
@@ -668,10 +675,18 @@ gfxFont* gfxDWriteFontEntry::CreateFontInstance(
     unscaledFontPtr = unscaledFont;
   }
   RefPtr<IDWriteFontFace> fontFace;
-  if (HasVariations() && !aFontStyle->variationSettings.IsEmpty()) {
-    nsresult rv = CreateFontFace(getter_AddRefs(fontFace), aFontStyle, sims);
-    if (NS_FAILED(rv)) {
-      return nullptr;
+  if (HasVariations()) {
+    // Get the variation settings needed to instantiate the fontEntry
+    // for a particular fontStyle.
+    AutoTArray<gfxFontVariation, 4> vars;
+    GetVariationsForStyle(vars, *aFontStyle);
+
+    if (!vars.IsEmpty()) {
+      nsresult rv =
+          CreateFontFace(getter_AddRefs(fontFace), aFontStyle, sims, &vars);
+      if (NS_FAILED(rv)) {
+        return nullptr;
+      }
     }
   }
   return new gfxDWriteFont(unscaledFont, this, aFontStyle, fontFace);
@@ -679,7 +694,8 @@ gfxFont* gfxDWriteFontEntry::CreateFontInstance(
 
 nsresult gfxDWriteFontEntry::CreateFontFace(
     IDWriteFontFace** aFontFace, const gfxFontStyle* aFontStyle,
-    DWRITE_FONT_SIMULATIONS aSimulations) {
+    DWRITE_FONT_SIMULATIONS aSimulations,
+    const nsTArray<gfxFontVariation>* aVariations) {
   // Convert an OpenType font tag from our uint32_t representation
   // (as constructed by TRUETYPE_TAG(...)) to the order DWrite wants.
   auto makeDWriteAxisTag = [](uint32_t aTag) {
@@ -742,15 +758,9 @@ nsresult gfxDWriteFontEntry::CreateFontFace(
     if (SUCCEEDED(hr) && resource) {
       AutoTArray<DWRITE_FONT_AXIS_VALUE, 4> fontAxisValues;
 
-      // Get the variation settings needed to instantiate the fontEntry
-      // for a particular fontStyle, or use default style if no aFontStyle
-      // was passed (e.g. instantiating a face just to read font tables).
-      AutoTArray<gfxFontVariation, 4> vars;
-      GetVariationsForStyle(vars, aFontStyle ? *aFontStyle : gfxFontStyle());
-
       // Copy variation settings to DWrite's type.
-      if (!vars.IsEmpty()) {
-        for (const auto& v : vars) {
+      if (aVariations) {
+        for (const auto& v : *aVariations) {
           DWRITE_FONT_AXIS_VALUE axisValue = {makeDWriteAxisTag(v.mTag),
                                               v.mValue};
           fontAxisValues.AppendElement(axisValue);

gfx/thebes/gfxDWriteFontList.h

@@ -223,7 +223,8 @@ class gfxDWriteFontEntry final : public gfxFontEntry {
 
   nsresult CreateFontFace(
       IDWriteFontFace** aFontFace, const gfxFontStyle* aFontStyle = nullptr,
-      DWRITE_FONT_SIMULATIONS aSimulations = DWRITE_FONT_SIMULATIONS_NONE);
+      DWRITE_FONT_SIMULATIONS aSimulations = DWRITE_FONT_SIMULATIONS_NONE,
+      const nsTArray<gfxFontVariation>* aVariations = nullptr);
 
   static bool InitLogFont(IDWriteFont* aFont, LOGFONTW* aLogFont);
 

gfx/thebes/gfxFT2FontList.cpp

@@ -174,27 +174,29 @@ gfxFont* FT2FontEntry::CreateFontInstance(const gfxFontStyle* aStyle) {
     return nullptr;
   }
 
-  // If variations are present, we will not use our cached mFTFace
-  // but always create a new one as it will have custom variation
-  // coordinates applied.
-  if ((!mVariationSettings.IsEmpty() ||
-       (aStyle && !aStyle->variationSettings.IsEmpty())) &&
-      (face->GetFace()->face_flags & FT_FACE_FLAG_MULTIPLE_MASTERS)) {
-    // Create a separate FT_Face because we need to apply custom
-    // variation settings to it.
-    RefPtr<SharedFTFace> varFace;
-    if (!mFilename.IsEmpty() && mFilename[0] == '/') {
-      varFace =
-          Factory::NewSharedFTFace(nullptr, mFilename.get(), mFTFontIndex);
-    } else {
-      varFace = face->GetData()->CloneFace(mFTFontIndex);
-    }
-    if (varFace) {
-      // Resolve variations from entry (descriptor) and style (property)
-      AutoTArray<gfxFontVariation, 8> settings;
-      GetVariationsForStyle(settings, aStyle ? *aStyle : gfxFontStyle());
-      gfxFT2FontBase::SetupVarCoords(GetMMVar(), settings, varFace->GetFace());
-      face = std::move(varFace);
+  if (face->GetFace()->face_flags & FT_FACE_FLAG_MULTIPLE_MASTERS) {
+    // Resolve variations from entry (descriptor) and style (property)
+    AutoTArray<gfxFontVariation, 8> settings;
+    GetVariationsForStyle(settings, aStyle ? *aStyle : gfxFontStyle());
+
+    // If variations are present, we will not use our cached mFTFace
+    // but always create a new one as it will have custom variation
+    // coordinates applied.
+    if (!settings.IsEmpty()) {
+      // Create a separate FT_Face because we need to apply custom
+      // variation settings to it.
+      RefPtr<SharedFTFace> varFace;
+      if (!mFilename.IsEmpty() && mFilename[0] == '/') {
+        varFace =
+            Factory::NewSharedFTFace(nullptr, mFilename.get(), mFTFontIndex);
+      } else {
+        varFace = face->GetData()->CloneFace(mFTFontIndex);
+      }
+      if (varFace) {
+        gfxFT2FontBase::SetupVarCoords(GetMMVar(), settings,
+                                       varFace->GetFace());
+        face = std::move(varFace);
+      }
     }
   }
 

gfx/thebes/gfxFont.h

@@ -126,6 +126,9 @@ struct gfxFontStyle {
   // The logical size of the font, in pixels
   gfxFloat size;
 
+  // The optical size value to apply (if supported); negative means none.
+  float autoOpticalSize = -1.0f;
+
   // The aspect-value (ie., the ratio actualsize:actualxheight) that any
   // actual physical font created from this font structure must have when
   // rendering or measuring a string. A value of -1.0 means no adjustment
@@ -235,6 +238,8 @@ struct gfxFontStyle {
            (featureValueLookup == other.featureValueLookup) &&
            (variationSettings == other.variationSettings) &&
            (languageOverride == other.languageOverride) &&
+           mozilla::NumbersAreBitwiseIdentical(autoOpticalSize,
+                                               other.autoOpticalSize) &&
            (fontSmoothingBackgroundColor == other.fontSmoothingBackgroundColor);
   }
 };

gfx/thebes/gfxFontEntry.cpp

@@ -1271,6 +1271,10 @@ void gfxFontEntry::SetupVariationRanges() {
 }
 
 void gfxFontEntry::CheckForVariationAxes() {
+  if (mCheckedForVariationAxes) {
+    return;
+  }
+  mCheckedForVariationAxes = true;
   if (HasVariations()) {
     AutoTArray<gfxFontVariationAxis, 4> axes;
     GetVariationAxes(axes);
@@ -1280,42 +1284,34 @@ void gfxFontEntry::CheckForVariationAxes() {
       } else if (axis.mTag == HB_TAG('i', 't', 'a', 'l') &&
                  axis.mMaxValue >= 1.0f) {
         mRangeFlags |= RangeFlags::eItalicVariation;
+      } else if (axis.mTag == HB_TAG('o', 'p', 's', 'z')) {
+        mRangeFlags |= RangeFlags::eOpticalSize;
       }
     }
   }
-  mCheckedForVariationAxes = true;
 }
 
 bool gfxFontEntry::HasBoldVariableWeight() {
   MOZ_ASSERT(!mIsUserFontContainer,
              "should not be called for user-font containers!");
-
-  if (!gfxPlatform::GetPlatform()->HasVariationFontSupport()) {
-    return false;
-  }
-
-  if (!mCheckedForVariationAxes) {
-    CheckForVariationAxes();
-  }
-
+  CheckForVariationAxes();
   return bool(mRangeFlags & RangeFlags::eBoldVariableWeight);
 }
 
 bool gfxFontEntry::HasItalicVariation() {
   MOZ_ASSERT(!mIsUserFontContainer,
              "should not be called for user-font containers!");
-
-  if (!gfxPlatform::GetPlatform()->HasVariationFontSupport()) {
-    return false;
-  }
-
-  if (!mCheckedForVariationAxes) {
-    CheckForVariationAxes();
-  }
-
+  CheckForVariationAxes();
   return bool(mRangeFlags & RangeFlags::eItalicVariation);
 }
 
+bool gfxFontEntry::HasOpticalSize() {
+  MOZ_ASSERT(!mIsUserFontContainer,
+             "should not be called for user-font containers!");
+  CheckForVariationAxes();
+  return bool(mRangeFlags & RangeFlags::eOpticalSize);
+}
+
 void gfxFontEntry::GetVariationsForStyle(nsTArray<gfxFontVariation>& aResult,
                                          const gfxFontStyle& aStyle) {
   if (!gfxPlatform::GetPlatform()->HasVariationFontSupport() ||
@@ -1374,12 +1370,13 @@ void gfxFontEntry::GetVariationsForStyle(nsTArray<gfxFontVariation>& aResult,
     aResult.AppendElement(gfxFontVariation{HB_TAG('s', 'l', 'n', 't'), -angle});
   }
 
+  struct TagEquals {
+    bool Equals(const gfxFontVariation& aIter, uint32_t aTag) const {
+      return aIter.mTag == aTag;
+    }
+  };
+
   auto replaceOrAppend = [&aResult](const gfxFontVariation& aSetting) {
-    struct TagEquals {
-      bool Equals(const gfxFontVariation& aIter, uint32_t aTag) const {
-        return aIter.mTag == aTag;
-      }
-    };
     auto index = aResult.IndexOf(aSetting.mTag, 0, TagEquals());
     if (index == aResult.NoIndex) {
       aResult.AppendElement(aSetting);
@@ -1400,6 +1397,16 @@ void gfxFontEntry::GetVariationsForStyle(nsTArray<gfxFontVariation>& aResult,
   for (const auto& v : aStyle.variationSettings) {
     replaceOrAppend(v);
   }
+
+  // If there's no explicit opsz in the settings, apply 'auto' value.
+  if (HasOpticalSize() && aStyle.autoOpticalSize >= 0.0f) {
+    const uint32_t kOpszTag = HB_TAG('o', 'p', 's', 'z');
+    auto index = aResult.IndexOf(kOpszTag, 0, TagEquals());
+    if (index == aResult.NoIndex) {
+      float value = aStyle.autoOpticalSize * mSizeAdjust;
+      aResult.AppendElement(gfxFontVariation{kOpszTag, value});
+    }
+  }
 }
 
 size_t gfxFontEntry::FontTableHashEntry::SizeOfExcludingThis(

gfx/thebes/gfxFontEntry.h

@@ -431,6 +431,8 @@ class gfxFontEntry {
 
   bool HasBoldVariableWeight();
   bool HasItalicVariation();
+  bool HasOpticalSize();
+
   void CheckForVariationAxes();
 
   // Set up the entry's weight/stretch/style ranges according to axes found
@@ -527,7 +529,10 @@ class gfxFontEntry {
     // properties to the variation axes (though they can still be
     // explicitly set using font-variation-settings).
     eNonCSSWeight = (1 << 5),
-    eNonCSSStretch = (1 << 6)
+    eNonCSSStretch = (1 << 6),
+
+    // Whether the font has an 'opsz' axis.
+    eOpticalSize = (1 << 7)
   };
   RangeFlags mRangeFlags = RangeFlags::eNoFlags;
 

gfx/thebes/gfxMacFont.cpp

@@ -53,45 +53,46 @@ gfxMacFont::gfxMacFont(const RefPtr<UnscaledFontMac>& aUnscaledFont,
     AutoTArray<gfxFontVariation, 4> vars;
     aFontEntry->GetVariationsForStyle(vars, *aFontStyle);
 
-    // Because of a Core Text bug, we need to ensure that if the font has
-    // an 'opsz' axis, it is always explicitly set, and NOT to the font's
-    // default value. (See bug 1457417, bug 1478720.)
-    // We record the result of searching the font's axes in the font entry,
-    // so that this only has to be done by the first instance created for
-    // a given font resource.
-    const uint32_t kOpszTag = HB_TAG('o', 'p', 's', 'z');
-    const float kOpszFudgeAmount = 0.01f;
+    if (aFontEntry->HasOpticalSize()) {
+      // Because of a Core Text bug, we need to ensure that if the font has
+      // an 'opsz' axis, it is always explicitly set, and NOT to the font's
+      // default value. (See bug 1457417, bug 1478720.)
+      // We record the result of searching the font's axes in the font entry,
+      // so that this only has to be done by the first instance created for
+      // a given font resource.
+      const uint32_t kOpszTag = HB_TAG('o', 'p', 's', 'z');
+      const float kOpszFudgeAmount = 0.01f;
 
-    if (!aFontEntry->mCheckedForOpszAxis) {
-      aFontEntry->mCheckedForOpszAxis = true;
-      AutoTArray<gfxFontVariationAxis, 4> axes;
-      aFontEntry->GetVariationAxes(axes);
-      auto index = axes.IndexOf(kOpszTag, 0, TagEquals<gfxFontVariationAxis>());
-      if (index == axes.NoIndex) {
-        aFontEntry->mHasOpszAxis = false;
-      } else {
-        const auto& axis = axes[index];
-        aFontEntry->mHasOpszAxis = true;
-        aFontEntry->mOpszAxis = axis;
-        // Pick a slightly-adjusted version of the default that we'll
-        // use to work around Core Text's habit of ignoring any attempt
-        // to explicitly set the default value.
-        aFontEntry->mAdjustedDefaultOpsz =
-            axis.mDefaultValue == axis.mMinValue
-                ? axis.mDefaultValue + kOpszFudgeAmount
-                : axis.mDefaultValue - kOpszFudgeAmount;
+      // Record the opsz axis details in the font entry, if not already done.
+      if (!aFontEntry->mOpszAxis.mTag) {
+        AutoTArray<gfxFontVariationAxis, 4> axes;
+        aFontEntry->GetVariationAxes(axes);
+        auto index =
+            axes.IndexOf(kOpszTag, 0, TagEquals<gfxFontVariationAxis>());
+        MOZ_ASSERT(index != axes.NoIndex);
+        if (index != axes.NoIndex) {
+          const auto& axis = axes[index];
+          aFontEntry->mOpszAxis = axis;
+          // Pick a slightly-adjusted version of the default that we'll
+          // use to work around Core Text's habit of ignoring any attempt
+          // to explicitly set the default value.
+          aFontEntry->mAdjustedDefaultOpsz =
+              axis.mDefaultValue == axis.mMinValue
+                  ? axis.mDefaultValue + kOpszFudgeAmount
+                  : axis.mDefaultValue - kOpszFudgeAmount;
+        }
       }
-    }
 
-    // Add 'opsz' if not present, or tweak its value if it looks too close
-    // to the default (after clamping to the font's available range).
-    if (aFontEntry->mHasOpszAxis) {
+      // Add 'opsz' if not present, or tweak its value if it looks too close
+      // to the default (after clamping to the font's available range).
       auto index = vars.IndexOf(kOpszTag, 0, TagEquals<gfxFontVariation>());
       if (index == vars.NoIndex) {
-        gfxFontVariation opsz{kOpszTag, aFontEntry->mAdjustedDefaultOpsz};
-        vars.AppendElement(opsz);
+        // No explicit opsz; set to the font's default.
+        vars.AppendElement(
+            gfxFontVariation{kOpszTag, aFontEntry->mAdjustedDefaultOpsz});
       } else {
-        // Figure out a "safe" value that Core Text won't ignore.
+        // An 'opsz' value was already present; use it, but adjust if necessary
+        // to a "safe" value that Core Text won't ignore.
         auto& value = vars[index].mValue;
         auto& axis = aFontEntry->mOpszAxis;
         value = fmin(fmax(value, axis.mMinValue), axis.mMaxValue);

gfx/thebes/gfxMacPlatformFontList.h

@@ -105,8 +105,6 @@ class MacOSFontEntry final : public gfxFontEntry {
   // These fields are used by gfxMacFont, but stored in the font entry so
   // that only a single font instance needs to inspect the available
   // variations.
-  bool mCheckedForOpszAxis;
-  bool mHasOpszAxis;
   gfxFontVariationAxis mOpszAxis;
   float mAdjustedDefaultOpsz;
 

gfx/thebes/gfxMacPlatformFontList.mm

@@ -329,9 +329,9 @@ MacOSFontEntry::MacOSFontEntry(const nsACString& aPostscriptName, WeightRange aW
       mHasVariations(false),
       mHasVariationsInitialized(false),
       mHasAATSmallCaps(false),
-      mHasAATSmallCapsInitialized(false),
-      mCheckedForOpszAxis(false) {
+      mHasAATSmallCapsInitialized(false) {
   mWeightRange = aWeight;
+  mOpszAxis.mTag = 0;
 }
 
 MacOSFontEntry::MacOSFontEntry(const nsACString& aPostscriptName, CGFontRef aFontRef,
@@ -347,8 +347,7 @@ MacOSFontEntry::MacOSFontEntry(const nsACString& aPostscriptName, CGFontRef aFon
       mHasVariations(false),
       mHasVariationsInitialized(false),
       mHasAATSmallCaps(false),
-      mHasAATSmallCapsInitialized(false),
-      mCheckedForOpszAxis(false) {
+      mHasAATSmallCapsInitialized(false) {
   mFontRef = aFontRef;
   mFontRefInitialized = true;
   ::CFRetain(mFontRef);
@@ -357,6 +356,7 @@ MacOSFontEntry::MacOSFontEntry(const nsACString& aPostscriptName, CGFontRef aFon
   mStretchRange = aStretch;
   mFixedPitch = false;  // xxx - do we need this for downloaded fonts?
   mStyleRange = aStyle;
+  mOpszAxis.mTag = 0;
 
   NS_ASSERTION(!(aIsDataUserFont && aIsLocalUserFont),
                "userfont is either a data font or a local font");