Bug 1056516 - use AutoTArray for hyphenBuffer in BreakAndMeasureText. r=jfkthame
☠☠ backed out by a8d497b09753 ☠ ☠
authorjeremychen@mozilla.com <jeremychen@mozilla.com>
Fri, 10 Mar 2017 00:30:10 +0800
changeset 346880 aaec04f764815ee50d8e8ff2e4ee24f59ec50209
parent 346879 c593315237b83e79fc0bc23422f897b5d58f903d
child 346881 41d39e1e440b7f2d39d134a5c5c74381cbe6e848
push id31480
push usercbook@mozilla.com
push dateFri, 10 Mar 2017 10:37:06 +0000
treeherdermozilla-central@e18d3dd20e8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame
bugs1056516
milestone55.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 1056516 - use AutoTArray for hyphenBuffer in BreakAndMeasureText. r=jfkthame MozReview-Commit-ID: 1fQGiosLYG
gfx/thebes/gfxTextRun.cpp
layout/generic/nsTextFrame.cpp
--- a/gfx/thebes/gfxTextRun.cpp
+++ b/gfx/thebes/gfxTextRun.cpp
@@ -862,23 +862,27 @@ gfxTextRun::BreakAndMeasureText(uint32_t
 
     Range bufferRange(aStart, aStart +
         std::min<uint32_t>(aMaxLength, MEASUREMENT_BUFFER_SIZE));
     PropertyProvider::Spacing spacingBuffer[MEASUREMENT_BUFFER_SIZE];
     bool haveSpacing = aProvider && (mFlags & gfxTextRunFactory::TEXT_ENABLE_SPACING) != 0;
     if (haveSpacing) {
         GetAdjustedSpacing(this, bufferRange, aProvider, spacingBuffer);
     }
-    bool hyphenBuffer[MEASUREMENT_BUFFER_SIZE];
+    AutoTArray<bool, 4096> hyphenBuffer;
     bool haveHyphenation = aProvider &&
         (aProvider->GetHyphensOption() == StyleHyphens::Auto ||
          (aProvider->GetHyphensOption() == StyleHyphens::Manual &&
           (mFlags & gfxTextRunFactory::TEXT_ENABLE_HYPHEN_BREAKS) != 0));
     if (haveHyphenation) {
-        aProvider->GetHyphenationBreaks(bufferRange, hyphenBuffer);
+        if (hyphenBuffer.AppendElements(bufferRange.Length(), fallible)) {
+            aProvider->GetHyphenationBreaks(bufferRange, hyphenBuffer.Elements());
+        } else {
+            haveHyphenation = false;
+        }
     }
 
     gfxFloat width = 0;
     gfxFloat advance = 0;
     // The number of space characters that can be trimmed or hang at a soft-wrap
     uint32_t trimmableChars = 0;
     // The amount of space removed by ignoring trimmableChars
     gfxFloat trimmableAdvance = 0;
@@ -891,36 +895,51 @@ gfxTextRun::BreakAndMeasureText(uint32_t
 
     Range ligatureRange(aStart, end);
     ShrinkToLigatureBoundaries(&ligatureRange);
 
     uint32_t i;
     for (i = aStart; i < end; ++i) {
         if (i >= bufferRange.end) {
             // Fetch more spacing and hyphenation data
+            uint32_t oldHyphenBufferLength = hyphenBuffer.Length();
             bufferRange.start = i;
             bufferRange.end = std::min(aStart + aMaxLength,
                                        i + MEASUREMENT_BUFFER_SIZE);
+            // For spacing, we always overwrite the old data with the newly
+            // fetched one. However, for hyphenation, hyphenation data sometimes
+            // depends on the context in every word (if "hyphens: auto" is set).
+            // To ensure we get enough information between neighboring buffers,
+            // we grow the hyphenBuffer instead of overwrite it.
+            // NOTE that this means bufferRange does not correspond to the
+            // entire hyphenBuffer, but only to the most recently added portion.
+            // Therefore, we need to add the old length to hyphenBuffer.Elements()
+            // when getting more data.
             if (haveSpacing) {
                 GetAdjustedSpacing(this, bufferRange, aProvider, spacingBuffer);
             }
             if (haveHyphenation) {
-                aProvider->GetHyphenationBreaks(bufferRange, hyphenBuffer);
+                if (hyphenBuffer.AppendElements(bufferRange.Length(), fallible)) {
+                    aProvider->GetHyphenationBreaks(
+                        bufferRange, hyphenBuffer.Elements() + oldHyphenBufferLength);
+                } else {
+                    haveHyphenation = false;
+                }
             }
         }
 
         // There can't be a word-wrap break opportunity at the beginning of the
         // line: if the width is too small for even one character to fit, it
         // could be the first and last break opportunity on the line, and that
         // would trigger an infinite loop.
         if (aSuppressBreak != eSuppressAllBreaks &&
             (aSuppressBreak != eSuppressInitialBreak || i > aStart)) {
             bool atNaturalBreak = mCharacterGlyphs[i].CanBreakBefore() == 1;
             bool atHyphenationBreak = !atNaturalBreak &&
-                haveHyphenation && hyphenBuffer[i - bufferRange.start];
+                haveHyphenation && hyphenBuffer[i - aStart];
             bool atBreak = atNaturalBreak || atHyphenationBreak;
             bool wordWrapping =
                 aCanWordWrap && mCharacterGlyphs[i].IsClusterStart() &&
                 *aBreakPriority <= gfxBreakPriority::eWordWrapBreak;
 
             if (atBreak || wordWrapping) {
                 gfxFloat hyphenatedAdvance = advance;
                 if (atHyphenationBreak) {
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -3627,17 +3627,18 @@ PropertyProvider::GetHyphenationBreaks(R
     if (run.IsSkipped()) {
       // Check if there's a soft hyphen which would let us hyphenate before
       // the next non-skipped character. Don't look at soft hyphens followed
       // by other skipped characters, we won't use them.
       allowHyphenBreakBeforeNextChar =
         mFrag->CharAt(run.GetOriginalOffset() + run.GetRunLength() - 1) == CH_SHY;
     } else {
       int32_t runOffsetInSubstring = run.GetSkippedOffset() - aRange.start;
-      memset(aBreakBefore + runOffsetInSubstring, false, run.GetRunLength()*sizeof(bool));
+      memset(aBreakBefore + runOffsetInSubstring, false,
+             run.GetRunLength() * sizeof(bool));
       // Don't allow hyphen breaks at the start of the line
       aBreakBefore[runOffsetInSubstring] = allowHyphenBreakBeforeNextChar &&
           (!(mFrame->GetStateBits() & TEXT_START_OF_LINE) ||
            run.GetSkippedOffset() > mStart.GetSkippedOffset());
       allowHyphenBreakBeforeNextChar = false;
     }
   }
 
@@ -8383,39 +8384,38 @@ nsTextFrame::AddInlineMinISizeForFlow(ns
       aData->OptionallyBreak();
     }
     aData->mCurrentLine += provider.GetFontMetrics()->EmHeight();
     aData->mTrailingWhitespace = 0;
     return;
   }
 
   AutoTArray<bool,BIG_TEXT_NODE_SIZE> hyphBuffer;
-  bool *hyphBreakBefore = nullptr;
   if (hyphenating) {
-    hyphBreakBefore = hyphBuffer.AppendElements(flowEndInTextRun - start,
-                                                fallible);
-    if (hyphBreakBefore) {
+    if (hyphBuffer.AppendElements(flowEndInTextRun - start, fallible)) {
       provider.GetHyphenationBreaks(Range(start, flowEndInTextRun),
-                                    hyphBreakBefore);
+                                    hyphBuffer.Elements());
+    } else {
+      hyphenating = false;
     }
   }
 
   for (uint32_t i = start, wordStart = start; i <= flowEndInTextRun; ++i) {
     bool preformattedNewline = false;
     bool preformattedTab = false;
     if (i < flowEndInTextRun) {
       // XXXldb Shouldn't we be including the newline as part of the
       // segment that it ends rather than part of the segment that it
       // starts?
       preformattedNewline = preformatNewlines && textRun->CharIsNewline(i);
       preformattedTab = preformatTabs && textRun->CharIsTab(i);
       if (!textRun->CanBreakLineBefore(i) &&
           !preformattedNewline &&
           !preformattedTab &&
-          (!hyphBreakBefore || !hyphBreakBefore[i - start]))
+          (!hyphenating || !hyphBuffer[i - start]))
       {
         // we can't break here (and it's not the end of the flow)
         continue;
       }
     }
 
     if (i > wordStart) {
       nscoord width = NSToCoordCeilClamped(
@@ -8452,18 +8452,18 @@ nsTextFrame::AddInlineMinISizeForFlow(ns
         AdvanceToNextTab(aData->mCurrentLine, this, textRun, tabWidth);
       aData->mCurrentLine = nscoord(afterTab + spacing.mAfter);
       wordStart = i + 1;
     } else if (i < flowEndInTextRun ||
         (i == textRun->GetLength() &&
          (textRun->GetFlags() & nsTextFrameUtils::TEXT_HAS_TRAILING_BREAK))) {
       if (preformattedNewline) {
         aData->ForceBreak();
-      } else if (i < flowEndInTextRun && hyphBreakBefore &&
-                 hyphBreakBefore[i - start]) {
+      } else if (i < flowEndInTextRun && hyphenating &&
+                 hyphBuffer[i - start]) {
         aData->OptionallyBreak(NSToCoordRound(provider.GetHyphenWidth()));
       } else {
         aData->OptionallyBreak();
       }
       wordStart = i;
     }
   }