bug 680402 - avoid calling SanitizeGlyphRuns repeatedly during textrun construction with multiple scripts. r=jdaggett
authorJonathan Kew <jfkthame@gmail.com>
Mon, 29 Aug 2011 09:21:53 +0100
changeset 76176 153a1de6cd67e8f803f75288f0105e8c2e3b4e0b
parent 76175 ab7182bc144e051366308c3a41b6eb589483f77a
child 76177 76e73aad0fabaeb8e26a3de4a5d6df4220de3050
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersjdaggett
bugs680402
milestone9.0a1
bug 680402 - avoid calling SanitizeGlyphRuns repeatedly during textrun construction with multiple scripts. r=jdaggett
gfx/thebes/gfxFont.cpp
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -2479,16 +2479,27 @@ gfxFontGroup::InitTextRun(gfxContext *aC
                     NS_ConvertUTF16toUTF8(aString + runStart, runLen).get()));
         }
 #endif
 
         InitScriptRun(aContext, aTextRun, aString, aLength,
                       runStart, runLimit, runScript);
     }
 
+    // It's possible for CoreText to omit glyph runs if it decides they contain
+    // only invisibles (e.g., U+FEFF, see reftest 474417-1). In this case, we
+    // need to eliminate them from the glyph run array to avoid drawing "partial
+    // ligatures" with the wrong font.
+    // We don't do this during InitScriptRun (or gfxFont::InitTextRun) because
+    // it will iterate back over all glyphruns in the textrun, which leads to
+    // pathologically-bad perf in the case where a textrun contains many script
+    // changes (see bug 680402) - we'd end up re-sanitizing all the earlier runs
+    // every time a new script subrun is processed.
+    aTextRun->SanitizeGlyphRuns();
+
     aTextRun->SortGlyphRuns();
 }
 
 void
 gfxFontGroup::InitScriptRun(gfxContext *aContext,
                             gfxTextRun *aTextRun,
                             const PRUnichar *aString,
                             PRUint32 aTotalLength,
@@ -2560,23 +2571,16 @@ gfxFontGroup::InitScriptRun(gfxContext *
                         aTextRun->SetMissingGlyph(index, ch);
                     }
                 }
             }
         }
 
         runStart += matchedLength;
     }
-
-    // It's possible for CoreText to omit glyph runs if it decides they contain
-    // only invisibles (e.g., U+FEFF, see reftest 474417-1). In this case, we
-    // need to eliminate them from the glyph run array to avoid drawing "partial
-    // ligatures" with the wrong font.
-    aTextRun->SanitizeGlyphRuns();
-
 }
 
 
 already_AddRefed<gfxFont>
 gfxFontGroup::FindFontForChar(PRUint32 aCh, PRUint32 aPrevCh,
                               PRInt32 aRunScript, gfxFont *aPrevMatchedFont,
                               PRUint8 *aMatchType)
 {
@@ -4040,16 +4044,19 @@ gfxTextRun::SortGlyphRuns()
             NS_ASSERTION(i == 0 ||
                          runs[i].mCharacterOffset !=
                          runs[i - 1].mCharacterOffset,
                          "Two fonts for the same run, glyph indices may not match the font");
         }
     }
 }
 
+// Note that SanitizeGlyphRuns scans all glyph runs in the textrun;
+// therefore we only call it once, at the end of textrun construction,
+// NOT incrementally as each glyph run is added (bug 680402).
 void
 gfxTextRun::SanitizeGlyphRuns()
 {
     if (mGlyphRuns.Length() <= 1)
         return;
 
     // If any glyph run starts with ligature-continuation characters, we need to advance it
     // to the first "real" character to avoid drawing partial ligature glyphs from wrong font