Bug 1043358 - Fix up font caching in MathMLTextRunFactory. r=roc
authorJames Kitchener <jkitch.bug@internode.on.net>
Fri, 22 Aug 2014 03:48:00 -0400
changeset 201669 165df837579e1d6b321732349721279ee49c725f
parent 201668 123d4bd5a50bf17aabcea469b0eb70be5ad04fe5
child 201670 a019b75127fd362997330e7200619ebbeaf607db
push id27375
push userryanvm@gmail.com
push dateTue, 26 Aug 2014 19:56:59 +0000
treeherdermozilla-central@f9bfe115fee5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1043358
milestone34.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 1043358 - Fix up font caching in MathMLTextRunFactory. r=roc
layout/generic/MathMLTextRunFactory.cpp
layout/generic/nsTextFrame.cpp
--- a/layout/generic/MathMLTextRunFactory.cpp
+++ b/layout/generic/MathMLTextRunFactory.cpp
@@ -5,16 +5,18 @@
 
 #include "MathMLTextRunFactory.h"
 
 #include "mozilla/ArrayUtils.h"
  
 #include "nsStyleConsts.h"
 #include "nsStyleContext.h"
 #include "nsTextFrameUtils.h"
+#include "nsFontMetrics.h"
+#include "nsDeviceContext.h"
 
 using namespace mozilla;
 
 /*
   Entries for the mathvariant lookup tables.  mKey represents the Unicode
   character to be transformed and is used for searching the tables.
   mReplacement represents the mapped mathvariant Unicode character.
 */
@@ -516,105 +518,108 @@ MathVariant(uint32_t aCh, uint8_t aMathV
 
 }
 
 void
 MathMLTextRunFactory::RebuildTextRun(nsTransformedTextRun* aTextRun,
                                      gfxContext* aRefContext)
 {
   gfxFontGroup* fontGroup = aTextRun->GetFontGroup();
-  gfxFontStyle fontStyle = *fontGroup->GetStyle();
 
   nsAutoString convertedString;
   nsAutoTArray<bool,50> charsToMergeArray;
   nsAutoTArray<bool,50> deletedCharsArray;
   nsAutoTArray<nsStyleContext*,50> styleArray;
   nsAutoTArray<uint8_t,50> canBreakBeforeArray;
   bool mergeNeeded = false;
 
   bool singleCharMI =
     aTextRun->GetFlags() & nsTextFrameUtils::TEXT_IS_SINGLE_CHAR_MI;
 
   uint32_t length = aTextRun->GetLength();
   const char16_t* str = aTextRun->mString.BeginReading();
   nsRefPtr<nsStyleContext>* styles = aTextRun->mStyles.Elements();
-
-  if (mSSTYScriptLevel && length) {
-    bool found = false;
-    // We respect ssty settings explicitly set by the user
-    for (uint32_t i = 0; i < fontStyle.featureSettings.Length(); i++) {
-      if (fontStyle.featureSettings[i].mTag == TRUETYPE_TAG('s','s','t','y')) {
-        found = true;
-        break;
-      }
-    }
-    if (!found) {
-      uint8_t sstyLevel = 0;
-      float scriptScaling = pow(styles[0]->StyleFont()->mScriptSizeMultiplier,
-                                mSSTYScriptLevel);
-      static_assert(NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER < 1,
-                    "Shouldn't it make things smaller?");
-      /*
-        An SSTY level of 2 is set if the scaling factor is less than or equal
-        to halfway between that for a scriptlevel of 1 (0.71) and that of a
-        scriptlevel of 2 (0.71^2), assuming the default script size multiplier.
-        An SSTY level of 1 is set if the script scaling factor is less than 
-        or equal that for a scriptlevel of 1 assuming the default script size
-        multiplier.
+  nsFont font;
+  if (length) {
+    font = styles[0]->StyleFont()->mFont;
 
-        User specified values of script size multiplier will change the scaling
-        factor which mSSTYScriptLevel values correspond to.
-
-        In the event that the script size multiplier actually makes things
-        larger, no change is made.
+    if (mSSTYScriptLevel) {
+      bool found = false;
+      // We respect ssty settings explicitly set by the user
+      for (uint32_t i = 0; i < font.fontFeatureSettings.Length(); i++) {
+        if (font.fontFeatureSettings[i].mTag == TRUETYPE_TAG('s', 's', 't', 'y')) {
+          found = true;
+          break;
+        }
+      }
+      if (!found) {
+        uint8_t sstyLevel = 0;
+        float scriptScaling = pow(styles[0]->StyleFont()->mScriptSizeMultiplier,
+                                  mSSTYScriptLevel);
+        static_assert(NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER < 1,
+                      "Shouldn't it make things smaller?");
+        /*
+          An SSTY level of 2 is set if the scaling factor is less than or equal
+          to halfway between that for a scriptlevel of 1 (0.71) and that of a
+          scriptlevel of 2 (0.71^2), assuming the default script size multiplier.
+          An SSTY level of 1 is set if the script scaling factor is less than
+          or equal that for a scriptlevel of 1 assuming the default script size
+          multiplier.
 
-        If the user doesn't want this to happen, all they need to do is set
-        style="font-feature-settings: 'ssty' 0"
-      */
-      if (scriptScaling <= (NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER +
-                            (NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER*
-                             NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER))/2) {
-        // Currently only the first two ssty settings are used, so two is large
-        // as we go
-        sstyLevel = 2;
-      } else if (scriptScaling <= NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER) {
-        sstyLevel = 1;
-      }
-      if (sstyLevel) {
-        gfxFontFeature settingSSTY;
-        settingSSTY.mTag = TRUETYPE_TAG('s','s','t','y');
-        settingSSTY.mValue = sstyLevel;
-        fontStyle.featureSettings.AppendElement(settingSSTY);
+          User specified values of script size multiplier will change the scaling
+          factor which mSSTYScriptLevel values correspond to.
+
+          In the event that the script size multiplier actually makes things
+          larger, no change is made.
+
+          To opt out of this change, add the following to the stylesheet:
+          "font-feature-settings: 'ssty' 0"
+        */
+        if (scriptScaling <= (NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER +
+                              (NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER *
+                               NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER))/2) {
+          // Currently only the first two ssty settings are used, so two is large
+          // as we go
+          sstyLevel = 2;
+        } else if (scriptScaling <= NS_MATHML_DEFAULT_SCRIPT_SIZE_MULTIPLIER) {
+          sstyLevel = 1;
+        }
+        if (sstyLevel) {
+          gfxFontFeature settingSSTY;
+          settingSSTY.mTag = TRUETYPE_TAG('s','s','t','y');
+          settingSSTY.mValue = sstyLevel;
+          font.fontFeatureSettings.AppendElement(settingSSTY);
+        }
       }
     }
   }
 
-  uint8_t mathVar;
+  uint8_t mathVar = NS_MATHML_MATHVARIANT_NONE;
   bool doMathvariantStyling = true;
 
   for (uint32_t i = 0; i < length; ++i) {
     int extraChars = 0;
     nsStyleContext* styleContext = styles[i];
     mathVar = styleContext->StyleFont()->mMathVariant;
 
     if (singleCharMI && mathVar == NS_MATHML_MATHVARIANT_NONE) {
       // If the user has explicitly set a non-default value for fontstyle or
       // fontweight, the italic mathvariant behaviour of <mi> is disabled
       // This overrides the initial values specified in fontStyle, to avoid
       // inconsistencies in which attributes allow CSS changes and which do not.
       if (mFlags & MATH_FONT_WEIGHT_BOLD) {
-        fontStyle.weight = NS_FONT_WEIGHT_BOLD;
+        font.weight = NS_FONT_WEIGHT_BOLD;
         if (mFlags & MATH_FONT_STYLING_NORMAL) {
-          fontStyle.style = NS_FONT_STYLE_NORMAL;
+          font.style = NS_FONT_STYLE_NORMAL;
         } else {
-          fontStyle.style = NS_FONT_STYLE_ITALIC;
+          font.style = NS_FONT_STYLE_ITALIC;
         }
       } else if (mFlags & MATH_FONT_STYLING_NORMAL) {
-        fontStyle.style = NS_FONT_STYLE_NORMAL;
-        fontStyle.weight = NS_FONT_WEIGHT_NORMAL;
+        font.style = NS_FONT_STYLE_NORMAL;
+        font.weight = NS_FONT_WEIGHT_NORMAL;
       } else {
         mathVar = NS_MATHML_MATHVARIANT_ITALIC;
       }
     }
 
     uint32_t ch = str[i];
     if (NS_IS_HIGH_SURROGATE(ch) && i < length - 1 &&
         NS_IS_LOW_SURROGATE(str[i + 1])) {
@@ -679,36 +684,53 @@ MathMLTextRunFactory::RebuildTextRun(nsT
   gfxTextRunFactory::Parameters innerParams =
       GetParametersForInner(aTextRun, &flags, aRefContext);
 
   nsAutoPtr<nsTransformedTextRun> transformedChild;
   nsAutoPtr<gfxTextRun> cachedChild;
   gfxTextRun* child;
 
   if (mathVar == NS_MATHML_MATHVARIANT_BOLD && doMathvariantStyling) {
-    fontStyle.style = NS_FONT_STYLE_NORMAL;
-    fontStyle.weight = NS_FONT_WEIGHT_BOLD;
+    font.style = NS_FONT_STYLE_NORMAL;
+    font.weight = NS_FONT_WEIGHT_BOLD;
   } else if (mathVar == NS_MATHML_MATHVARIANT_ITALIC && doMathvariantStyling) {
-    fontStyle.style = NS_FONT_STYLE_ITALIC;
-    fontStyle.weight = NS_FONT_WEIGHT_NORMAL;
+    font.style = NS_FONT_STYLE_ITALIC;
+    font.weight = NS_FONT_WEIGHT_NORMAL;
   } else if (mathVar == NS_MATHML_MATHVARIANT_BOLD_ITALIC &&
              doMathvariantStyling) {
-    fontStyle.style = NS_FONT_STYLE_ITALIC;
-    fontStyle.weight = NS_FONT_WEIGHT_BOLD;
+    font.style = NS_FONT_STYLE_ITALIC;
+    font.weight = NS_FONT_WEIGHT_BOLD;
   } else if (mathVar != NS_MATHML_MATHVARIANT_NONE) {
     // Mathvariant overrides fontstyle and fontweight
     // Need to check to see if mathvariant is actually applied as this function
     // is used for other purposes.
-    fontStyle.style = NS_FONT_STYLE_NORMAL;
-    fontStyle.weight = NS_FONT_WEIGHT_NORMAL;
+    font.style = NS_FONT_STYLE_NORMAL;
+    font.weight = NS_FONT_WEIGHT_NORMAL;
   }
-  nsRefPtr<gfxFontGroup> newFontGroup = fontGroup->Copy(&fontStyle);
+  gfxFontGroup* newFontGroup = nullptr;
 
-  if (!newFontGroup)
-    return;
+  // Get the correct gfxFontGroup that corresponds to the earlier font changes.
+  if (length) {
+    nsPresContext* pc = styles[0]->PresContext();
+    nsRefPtr<nsFontMetrics> metrics;
+    pc->DeviceContext()->GetMetricsFor(font,
+                                       styles[0]->StyleFont()->mLanguage,
+                                       pc->GetUserFontSet(),
+                                       pc->GetTextPerfMetrics(),
+                                       *getter_AddRefs(metrics));
+    if (metrics) {
+      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.
+    newFontGroup = fontGroup;
+  }
 
   if (mInnerTransformingTextRunFactory) {
     transformedChild = mInnerTransformingTextRunFactory->MakeTextRun(
         convertedString.BeginReading(), convertedString.Length(),
         &innerParams, newFontGroup, flags, styleArray.Elements(), false);
     child = transformedChild.get();
   } else {
     cachedChild = newFontGroup->MakeTextRun(
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -1905,17 +1905,19 @@ BuildTextRunsScanner::BuildTextRunForFra
     nsTextFrameUtils::CompressionMode compression =
       CSSWhitespaceToCompressionMode[textStyle->mWhiteSpace];
     if (enabledJustification && !textStyle->WhiteSpaceIsSignificant()) {
       textFlags |= gfxTextRunFactory::TEXT_ENABLE_SPACING;
     }
     fontStyle = f->StyleFont();
     nsIFrame* parent = mLineContainer->GetParent();
     if (NS_MATHML_MATHVARIANT_NONE != fontStyle->mMathVariant) {
-      anyMathMLStyling = true;
+      if (NS_MATHML_MATHVARIANT_NORMAL != fontStyle->mMathVariant) {
+        anyMathMLStyling = true;
+      }
     } else if (mLineContainer->GetStateBits() & NS_FRAME_IS_IN_SINGLE_CHAR_MI) {
       textFlags |= nsTextFrameUtils::TEXT_IS_SINGLE_CHAR_MI;
       anyMathMLStyling = true;
       // Test for fontstyle attribute as StyleFont() may not be accurate
       // To be consistent in terms of ignoring CSS style changes, fontweight
       // gets checked too.
       if (parent) {
         nsIContent* content = parent->GetContent();