Bug 1528712 - Remove nsFont::featureValueLookup. r=jfkthame
☠☠ backed out by 8a4ad06c0575 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 18 Feb 2019 14:03:47 +0000
changeset 459876 2d5c4e71e258e24f1a70ed850c619f97e56f2c60
parent 459875 d981515b874bbece6fee9d6f486612d3ec430ef3
child 459877 17d8de567ea416b9a995fcec56897b1bc138b9c6
push id112018
push userbtara@mozilla.com
push dateTue, 19 Feb 2019 17:39:20 +0000
treeherdermozilla-inbound@8c7b302aa952 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame
bugs1528712
milestone67.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 1528712 - Remove nsFont::featureValueLookup. r=jfkthame It's a global object, it doesn't have to be stored in nsFont. Pass it from the caller like the user font set and co. Depends on D20141 Differential Revision: https://phabricator.services.mozilla.com/D20142
dom/canvas/CanvasRenderingContext2D.cpp
gfx/src/nsFont.cpp
gfx/src/nsFont.h
gfx/src/nsFontMetrics.cpp
gfx/src/nsFontMetrics.h
layout/base/PresShell.cpp
layout/base/nsLayoutUtils.cpp
layout/generic/MathMLTextRunFactory.cpp
layout/generic/nsPageFrame.cpp
layout/mathml/nsMathMLChar.cpp
layout/style/GeckoBindings.cpp
layout/style/GeckoBindings.h
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/properties.mako.rs
--- a/dom/canvas/CanvasRenderingContext2D.cpp
+++ b/dom/canvas/CanvasRenderingContext2D.cpp
@@ -2640,16 +2640,17 @@ class CanvasUserSpaceMetrics : public Us
   }
 
   virtual float GetExLength() const override {
     nsDeviceContext* dc = mPresContext->DeviceContext();
     nsFontMetrics::Params params;
     params.language = mFontLanguage;
     params.explicitLanguage = mExplicitLanguage;
     params.textPerf = mPresContext->GetTextPerfMetrics();
+    params.featureValueLookup = mPresContext->GetFontFeatureValuesLookup();
     RefPtr<nsFontMetrics> fontMetrics = dc->GetMetricsFor(mFont, params);
     return NSAppUnitsToFloatPixels(fontMetrics->XHeight(),
                                    AppUnitsPerCSSPixel());
   }
 
   virtual gfx::Size GetSize() const override { return Size(mSize); }
 
  private:
--- a/gfx/src/nsFont.cpp
+++ b/gfx/src/nsFont.cpp
@@ -46,35 +46,33 @@ nsFont::MaxDifference nsFont::CalcDiffer
       (languageOverride != aOther.languageOverride) ||
       (variantAlternates != aOther.variantAlternates) ||
       (variantCaps != aOther.variantCaps) ||
       (variantEastAsian != aOther.variantEastAsian) ||
       (variantLigatures != aOther.variantLigatures) ||
       (variantNumeric != aOther.variantNumeric) ||
       (variantPosition != aOther.variantPosition) ||
       (variantWidth != aOther.variantWidth) ||
-      (alternateValues != aOther.alternateValues) ||
-      (featureValueLookup != aOther.featureValueLookup)) {
+      (alternateValues != aOther.alternateValues)) {
     return MaxDifference::eLayoutAffecting;
   }
 
   if ((smoothing != aOther.smoothing) ||
       (fontSmoothingBackgroundColor != aOther.fontSmoothingBackgroundColor)) {
     return MaxDifference::eVisual;
   }
 
   return MaxDifference::eNone;
 }
 
 nsFont& nsFont::operator=(const nsFont& aOther) = default;
 
 void nsFont::CopyAlternates(const nsFont& aOther) {
   variantAlternates = aOther.variantAlternates;
   alternateValues = aOther.alternateValues;
-  featureValueLookup = aOther.featureValueLookup;
 }
 
 // mapping from bitflag to font feature tag/value pair
 //
 // these need to be kept in sync with the constants listed
 // in gfxFontConstants.h (e.g. NS_FONT_VARIANT_EAST_ASIAN_JIS78)
 
 // NS_FONT_VARIANT_EAST_ASIAN_xxx values
@@ -181,17 +179,16 @@ void nsFont::AddFontFeaturesToStyle(gfxF
     setting.mValue = 1;
     setting.mTag = TRUETYPE_TAG('h', 'i', 's', 't');
     aStyle->featureSettings.AppendElement(setting);
   }
 
   // -- copy font-specific alternate info into style
   //    (this will be resolved after font-matching occurs)
   aStyle->alternateValues.AppendElements(alternateValues);
-  aStyle->featureValueLookup = featureValueLookup;
 
   // -- caps
   aStyle->variantCaps = variantCaps;
 
   // -- east-asian
   if (variantEastAsian) {
     AddFontFeaturesBitmask(variantEastAsian, NS_FONT_VARIANT_EAST_ASIAN_JIS78,
                            NS_FONT_VARIANT_EAST_ASIAN_RUBY, eastAsianDefaults,
--- a/gfx/src/nsFont.h
+++ b/gfx/src/nsFont.h
@@ -51,19 +51,16 @@ struct nsFont {
   nsTArray<gfxFontFeature> fontFeatureSettings;
 
   // Font variations from CSS font-variation-settings
   nsTArray<gfxFontVariation> fontVariationSettings;
 
   // -- list of value tags for font-specific alternate features
   nsTArray<gfxAlternateValue> alternateValues;
 
-  // -- object used to look these up once the font is matched
-  RefPtr<gfxFontFeatureValueSet> featureValueLookup;
-
   // The logical size of the font, in nscoord units
   nscoord size = 0;
 
   // 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
   // needs to be done; otherwise the value must be nonnegative.
   float sizeAdjust = -1.0f;
--- a/gfx/src/nsFontMetrics.cpp
+++ b/gfx/src/nsFontMetrics.cpp
@@ -119,16 +119,18 @@ nsFontMetrics::nsFontMetrics(const nsFon
   gfxFontStyle style(
       aFont.style, aFont.weight, aFont.stretch, gfxFloat(aFont.size) / mP2A,
       aParams.language, aParams.explicitLanguage, aFont.sizeAdjust,
       aFont.systemFont, mDeviceContext->IsPrinterContext(),
       aFont.synthesis & NS_FONT_SYNTHESIS_WEIGHT,
       aFont.synthesis & NS_FONT_SYNTHESIS_STYLE, aFont.languageOverride);
 
   aFont.AddFontFeaturesToStyle(&style, mOrientation == gfxFont::eVertical);
+  style.featureValueLookup = aParams.featureValueLookup;
+
   aFont.AddFontVariationsToStyle(&style);
 
   gfxFloat devToCssSize = gfxFloat(mP2A) / gfxFloat(AppUnitsPerCSSPixel());
   mFontGroup = gfxPlatform::GetPlatform()->CreateFontGroup(
       aFont.fontlist, &style, aParams.textPerf, aParams.userFontSet,
       devToCssSize);
 }
 
--- a/gfx/src/nsFontMetrics.h
+++ b/gfx/src/nsFontMetrics.h
@@ -49,16 +49,17 @@ class nsFontMetrics final {
   typedef mozilla::gfx::DrawTarget DrawTarget;
 
   struct MOZ_STACK_CLASS Params {
     nsAtom* language = nullptr;
     bool explicitLanguage = false;
     gfxFont::Orientation orientation = gfxFont::eHorizontal;
     gfxUserFontSet* userFontSet = nullptr;
     gfxTextPerfMetrics* textPerf = nullptr;
+    gfxFontFeatureValueSet* featureValueLookup = nullptr;
   };
 
   nsFontMetrics(const nsFont& aFont, const Params& aParams,
                 nsDeviceContext* aContext);
 
   // Used by stylo
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsFontMetrics)
 
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -9996,16 +9996,17 @@ void ReflowCountMgr::PaintCount(const ch
               devPixelOffset));
 
       // We don't care about the document language or user fonts here;
       // just get a default Latin font.
       nsFont font(eFamily_serif, nsPresContext::CSSPixelsToAppUnits(11));
       nsFontMetrics::Params params;
       params.language = nsGkAtoms::x_western;
       params.textPerf = aPresContext->GetTextPerfMetrics();
+      params.featureValueLookup = aPresContext->GetFontFeatureValuesLookup();
       RefPtr<nsFontMetrics> fm =
           aPresContext->DeviceContext()->GetMetricsFor(font, params);
 
       char buf[16];
       int len = SprintfLiteral(buf, "%d", counter->mCount);
       nscoord x = 0, y = fm->MaxAscent();
       nscoord width, height = fm->MaxHeight();
       fm->SetTextRunRTL(false);
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -4446,16 +4446,17 @@ already_AddRefed<nsFontMetrics> nsLayout
   params.explicitLanguage = styleFont->mExplicitLanguage;
   params.orientation = wm.IsVertical() && !wm.IsSideways()
                            ? gfxFont::eVertical
                            : gfxFont::eHorizontal;
   // pass the user font set object into the device context to
   // pass along to CreateFontGroup
   params.userFontSet = aPresContext->GetUserFontSet();
   params.textPerf = aPresContext->GetTextPerfMetrics();
+  params.featureValueLookup = aPresContext->GetFontFeatureValuesLookup();
 
   // When aInflation is 1.0 and we don't require width variant, avoid
   // making a local copy of the nsFont.
   // This also avoids running font.size through floats when it is large,
   // which would be lossy.  Fortunately, in such cases, aInflation is
   // guaranteed to be 1.0f.
   if (aInflation == 1.0f && aVariantWidth == NS_FONT_VARIANT_WIDTH_NORMAL) {
     return aPresContext->DeviceContext()->GetMetricsFor(styleFont->mFont,
@@ -9550,16 +9551,17 @@ already_AddRefed<nsFontMetrics> nsLayout
       aIsVertical ? gfxFont::eVertical : gfxFont::eHorizontal;
   nsFontMetrics::Params params;
   params.language = aStyleFont->mLanguage;
   params.explicitLanguage = aStyleFont->mExplicitLanguage;
   params.orientation = orientation;
   params.userFontSet =
       aUseUserFontSet ? aPresContext->GetUserFontSet() : nullptr;
   params.textPerf = aPresContext->GetTextPerfMetrics();
+  params.featureValueLookup = aPresContext->GetFontFeatureValuesLookup();
   return aPresContext->DeviceContext()->GetMetricsFor(font, params);
 }
 
 /* static */ void nsLayoutUtils::FixupNoneGeneric(
     nsFont* aFont, uint8_t aGenericFontID, const nsFont* aDefaultVariableFont) {
   bool useDocumentFonts = StaticPrefs::browser_display_use_document_fonts();
   if (aGenericFontID == kGenericFont_NONE ||
       (!useDocumentFonts && (aGenericFontID == kGenericFont_cursive ||
--- a/layout/generic/MathMLTextRunFactory.cpp
+++ b/layout/generic/MathMLTextRunFactory.cpp
@@ -635,16 +635,17 @@ void MathMLTextRunFactory::RebuildTextRu
   if (length) {
     font.size = NSToCoordRound(font.size * mFontInflation);
     nsPresContext* pc = styles[0]->mPresContext;
     nsFontMetrics::Params params;
     params.language = styles[0]->mLanguage;
     params.explicitLanguage = styles[0]->mExplicitLanguage;
     params.userFontSet = pc->GetUserFontSet();
     params.textPerf = pc->GetTextPerfMetrics();
+    params.featureValueLookup = pc->GetFontFeatureValuesLookup();
     RefPtr<nsFontMetrics> metrics =
         pc->DeviceContext()->GetMetricsFor(font, params);
     newFontGroup = metrics->GetThebesFontGroup();
   }
 
   if (!newFontGroup) {
     // If we can't get a new font group, fall back to the old one.  Rendering
     // will be incorrect, but not significantly so.
--- a/layout/generic/nsPageFrame.cpp
+++ b/layout/generic/nsPageFrame.cpp
@@ -580,16 +580,17 @@ void nsPageFrame::PaintHeaderFooter(gfxC
 
   DrawTargetAutoDisableSubpixelAntialiasing disable(
       aRenderingContext.GetDrawTarget(), aDisableSubpixelAA);
 
   // Get the FontMetrics to determine width.height of strings
   nsFontMetrics::Params params;
   params.userFontSet = pc->GetUserFontSet();
   params.textPerf = pc->GetTextPerfMetrics();
+  params.featureValueLookup = pc->GetFontFeatureValuesLookup();
   RefPtr<nsFontMetrics> fontMet =
       pc->DeviceContext()->GetMetricsFor(mPD->mHeadFootFont, params);
 
   nscoord ascent = 0;
   nscoord visibleHeight = 0;
   if (fontMet) {
     visibleHeight = fontMet->MaxHeight();
     ascent = fontMet->MaxAscent();
--- a/layout/mathml/nsMathMLChar.cpp
+++ b/layout/mathml/nsMathMLChar.cpp
@@ -878,16 +878,17 @@ bool nsMathMLChar::SetFontFamily(nsPresC
     nsFont font = aFont;
     font.fontlist = familyList;
     const nsStyleFont* styleFont = mComputedStyle->StyleFont();
     nsFontMetrics::Params params;
     params.language = styleFont->mLanguage;
     params.explicitLanguage = styleFont->mExplicitLanguage;
     params.userFontSet = aPresContext->GetUserFontSet();
     params.textPerf = aPresContext->GetTextPerfMetrics();
+    params.featureValueLookup = aPresContext->GetFontFeatureValuesLookup();
     RefPtr<nsFontMetrics> fm =
         aPresContext->DeviceContext()->GetMetricsFor(font, params);
     // Set the font if it is an unicode table
     // or if the same family name has been found
     gfxFont* firstFont = fm->GetThebesFontGroup()->GetFirstValidFont();
     FontFamilyList firstFontList(firstFont->GetFontEntry()->FamilyName(),
                                  eUnquotedName);
     if (aGlyphTable == &gGlyphTableList->mUnicodeTable ||
--- a/layout/style/GeckoBindings.cpp
+++ b/layout/style/GeckoBindings.cpp
@@ -1040,21 +1040,16 @@ nsTArray<unsigned int>* Gecko_AppendFeat
     uint32_t aAlternate, nsAtom* aName) {
   MOZ_ASSERT(NS_IsMainThread());
   static_assert(sizeof(unsigned int) == sizeof(uint32_t),
                 "sizeof unsigned int and uint32_t must be the same");
   return aFontFeatureValues->AppendFeatureValueHashEntry(
       nsAtomCString(aFamily), nsDependentAtomString(aName), aAlternate);
 }
 
-void Gecko_nsFont_SetFontFeatureValuesLookup(
-    nsFont* aFont, const RawGeckoPresContext* aPresContext) {
-  aFont->featureValueLookup = aPresContext->GetFontFeatureValuesLookup();
-}
-
 float Gecko_FontStretch_ToFloat(mozilla::FontStretch aStretch) {
   // Servo represents percentages with 1. being 100%.
   return aStretch.Percentage() / 100.0f;
 }
 
 void Gecko_FontStretch_SetFloat(mozilla::FontStretch* aStretch, float aFloat) {
   // Servo represents percentages with 1. being 100%.
   //
@@ -1088,35 +1083,30 @@ void Gecko_FontSlantStyle_Get(mozilla::F
 float Gecko_FontWeight_ToFloat(mozilla::FontWeight aWeight) {
   return aWeight.ToFloat();
 }
 
 void Gecko_FontWeight_SetFloat(mozilla::FontWeight* aWeight, float aFloat) {
   *aWeight = mozilla::FontWeight(aFloat);
 }
 
-void Gecko_nsFont_ResetFontFeatureValuesLookup(nsFont* aFont) {
-  aFont->featureValueLookup = nullptr;
-}
-
 void Gecko_ClearAlternateValues(nsFont* aFont, size_t aLength) {
   aFont->alternateValues.Clear();
   aFont->alternateValues.SetCapacity(aLength);
 }
 
 void Gecko_AppendAlternateValues(nsFont* aFont, uint32_t aAlternateName,
                                  nsAtom* aAtom) {
   aFont->alternateValues.AppendElement(
       gfxAlternateValue{aAlternateName, nsDependentAtomString(aAtom)});
 }
 
 void Gecko_CopyAlternateValuesFrom(nsFont* aDest, const nsFont* aSrc) {
   aDest->alternateValues.Clear();
   aDest->alternateValues.AppendElements(aSrc->alternateValues);
-  aDest->featureValueLookup = aSrc->featureValueLookup;
 }
 
 void Gecko_SetCounterStyleToName(CounterStylePtr* aPtr, nsAtom* aName) {
   RefPtr<nsAtom> name = already_AddRefed<nsAtom>(aName);
   *aPtr = name.forget();
 }
 
 void Gecko_SetCounterStyleToSymbols(CounterStylePtr* aPtr, uint8_t aSymbolsType,
--- a/layout/style/GeckoBindings.h
+++ b/layout/style/GeckoBindings.h
@@ -284,21 +284,16 @@ void Gecko_nsFont_Destroy(nsFont* dst);
 
 // The gfxFontFeatureValueSet returned from this function has zero reference.
 gfxFontFeatureValueSet* Gecko_ConstructFontFeatureValueSet();
 
 nsTArray<unsigned int>* Gecko_AppendFeatureValueHashEntry(
     gfxFontFeatureValueSet* value_set, nsAtom* family, uint32_t alternate,
     nsAtom* name);
 
-void Gecko_nsFont_SetFontFeatureValuesLookup(
-    nsFont* font, const RawGeckoPresContext* pres_context);
-
-void Gecko_nsFont_ResetFontFeatureValuesLookup(nsFont* font);
-
 // Font variant alternates
 void Gecko_ClearAlternateValues(nsFont* font, size_t length);
 
 void Gecko_AppendAlternateValues(nsFont* font, uint32_t alternate_name,
                                  nsAtom* atom);
 
 void Gecko_CopyAlternateValuesFrom(nsFont* dest, const nsFont* src);
 
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -2646,33 +2646,29 @@ fn static_assert() {
     }
 
     ${impl_simple("_moz_script_level", "mScriptLevel")}
     <% impl_simple_type_with_conversion("font_language_override", "mFont.languageOverride") %>
 
     pub fn set_font_variant_alternates(
         &mut self,
         v: values::computed::font::FontVariantAlternates,
-        device: &Device,
     ) {
         use crate::gecko_bindings::bindings::{Gecko_ClearAlternateValues, Gecko_AppendAlternateValues};
-        use crate::gecko_bindings::bindings::Gecko_nsFont_ResetFontFeatureValuesLookup;
-        use crate::gecko_bindings::bindings::Gecko_nsFont_SetFontFeatureValuesLookup;
         % for value in "normal swash stylistic ornaments annotation styleset character_variant historical".split():
             use crate::gecko_bindings::structs::NS_FONT_VARIANT_ALTERNATES_${value.upper()};
         % endfor
         use crate::values::specified::font::VariantAlternates;
 
         unsafe {
             Gecko_ClearAlternateValues(&mut self.gecko.mFont, v.len());
         }
 
         if v.0.is_empty() {
             self.gecko.mFont.variantAlternates = NS_FONT_VARIANT_ALTERNATES_NORMAL as u16;
-            unsafe { Gecko_nsFont_ResetFontFeatureValuesLookup(&mut self.gecko.mFont); }
             return;
         }
 
         for val in v.0.iter() {
             match *val {
                 % for value in "Swash Stylistic Ornaments Annotation".split():
                     VariantAlternates::${value}(ref ident) => {
                         self.gecko.mFont.variantAlternates |= NS_FONT_VARIANT_ALTERNATES_${value.upper()} as u16;
@@ -2695,20 +2691,16 @@ fn static_assert() {
                         }
                     },
                 % endfor
                 VariantAlternates::HistoricalForms => {
                     self.gecko.mFont.variantAlternates |= NS_FONT_VARIANT_ALTERNATES_HISTORICAL as u16;
                 }
             }
         }
-
-        unsafe {
-            Gecko_nsFont_SetFontFeatureValuesLookup(&mut self.gecko.mFont, device.pres_context());
-        }
     }
 
     #[allow(non_snake_case)]
     pub fn copy_font_variant_alternates_from(&mut self, other: &Self) {
         use crate::gecko_bindings::bindings::Gecko_CopyAlternateValuesFrom;
 
         self.gecko.mFont.variantAlternates = other.gecko.mFont.variantAlternates;
         unsafe {
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -3502,24 +3502,21 @@ impl<'a> StyleBuilder<'a> {
     pub fn set_${property.ident}(
         &mut self,
         value: longhands::${property.ident}::computed_value::T
     ) {
         % if not property.style_struct.inherited:
         self.modified_reset = true;
         % endif
 
-        <% props_need_device = ["font_variant_alternates"] %>
         self.${property.style_struct.ident}.mutate()
             .set_${property.ident}(
                 value,
                 % if property.logical:
                 self.writing_mode,
-                % elif product == "gecko" and property.ident in props_need_device:
-                self.device,
                 % endif
             );
     }
     % endif
     % endif
     % endfor
     <% del property %>