bug 631035 part 1 - optimize storage of DetailedGlyph records. r=roc a=blocking2.0
authorJonathan Kew <jfkthame@gmail.com>
Thu, 10 Feb 2011 06:50:47 +0000
changeset 62288 199cb628255440c7016a41284b14226160fa38db
parent 62287 f00a192644fd191747dd9cb62a890b709ff53ec9
child 62289 84cebd5b0f9317a305a7ab732ae834a5ab1367a9
push id18680
push userjkew@mozilla.com
push dateThu, 10 Feb 2011 06:52:32 +0000
treeherdermozilla-central@199cb6282554 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, blocking2
bugs631035
milestone2.0b12pre
first release with
nightly linux32
199cb6282554 / 4.0b12pre / 20110210030400 / files
nightly linux64
199cb6282554 / 4.0b12pre / 20110210030400 / files
nightly mac
199cb6282554 / 4.0b12pre / 20110210030400 / files
nightly win32
199cb6282554 / 4.0b12pre / 20110210030400 / files
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
bug 631035 part 1 - optimize storage of DetailedGlyph records. r=roc a=blocking2.0
gfx/thebes/gfxFont.cpp
gfx/thebes/gfxFont.h
gfx/thebes/gfxTextRunWordCache.cpp
layout/generic/nsTextRunTransformations.cpp
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -1172,60 +1172,65 @@ gfxFont::Draw(gfxTextRun *aTextRun, PRUi
                 doubleglyph->x =
                   ToDeviceUnits(glyphX + synBoldDevUnitOffsetAppUnits,
                                 devUnitsPerAppUnit);
                 doubleglyph->y = glyph->y;
             }
             
             glyphs.Flush(cr, aDrawToPath, isRTL);
         } else {
-            PRUint32 j;
             PRUint32 glyphCount = glyphData->GetGlyphCount();
-            const gfxTextRun::DetailedGlyph *details = aTextRun->GetDetailedGlyphs(i);
-            for (j = 0; j < glyphCount; ++j, ++details) {
-                double advance = details->mAdvance;
-                if (glyphData->IsMissing()) {
-                    // default ignorable characters will have zero advance width.
-                    // we don't have to draw the hexbox for them
-                    if (!aDrawToPath && advance > 0) {
-                        double glyphX = x;
+            if (glyphCount > 0) {
+                const gfxTextRun::DetailedGlyph *details =
+                    aTextRun->GetDetailedGlyphs(i);
+                NS_ASSERTION(details, "detailedGlyph should not be missing!");
+                for (PRUint32 j = 0; j < glyphCount; ++j, ++details) {
+                    double advance = details->mAdvance;
+                    if (glyphData->IsMissing()) {
+                        // default ignorable characters will have zero advance width.
+                        // we don't have to draw the hexbox for them
+                        if (!aDrawToPath && advance > 0) {
+                            double glyphX = x;
+                            if (isRTL) {
+                                glyphX -= advance;
+                            }
+                            gfxPoint pt(ToDeviceUnits(glyphX, devUnitsPerAppUnit),
+                                        ToDeviceUnits(y, devUnitsPerAppUnit));
+                            gfxFloat advanceDevUnits = ToDeviceUnits(advance, devUnitsPerAppUnit);
+                            gfxFloat height = GetMetrics().maxAscent;
+                            gfxRect glyphRect(pt.x, pt.y - height, advanceDevUnits, height);
+                            gfxFontMissingGlyphs::DrawMissingGlyph(aContext,
+                                                                   glyphRect,
+                                                                   details->mGlyphID);
+                        }
+                    } else {
+                        glyph = glyphs.AppendGlyph();
+                        glyph->index = details->mGlyphID;
+                        double glyphX = x + details->mXOffset;
                         if (isRTL) {
                             glyphX -= advance;
                         }
-                        gfxPoint pt(ToDeviceUnits(glyphX, devUnitsPerAppUnit),
-                                    ToDeviceUnits(y, devUnitsPerAppUnit));
-                        gfxFloat advanceDevUnits = ToDeviceUnits(advance, devUnitsPerAppUnit);
-                        gfxFloat height = GetMetrics().maxAscent;
-                        gfxRect glyphRect(pt.x, pt.y - height, advanceDevUnits, height);
-                        gfxFontMissingGlyphs::DrawMissingGlyph(aContext, glyphRect, details->mGlyphID);
-                    }
-                } else {
-                    glyph = glyphs.AppendGlyph();
-                    glyph->index = details->mGlyphID;
-                    double glyphX = x + details->mXOffset;
-                    if (isRTL) {
-                        glyphX -= advance;
+                        glyph->x = ToDeviceUnits(glyphX, devUnitsPerAppUnit);
+                        glyph->y = ToDeviceUnits(y + details->mYOffset, devUnitsPerAppUnit);
+
+                        // synthetic bolding by drawing with a one-pixel offset
+                        if (mSyntheticBoldOffset) {
+                            cairo_glyph_t *doubleglyph;
+                            doubleglyph = glyphs.AppendGlyph();
+                            doubleglyph->index = glyph->index;
+                            doubleglyph->x =
+                                ToDeviceUnits(glyphX + synBoldDevUnitOffsetAppUnits,
+                                              devUnitsPerAppUnit);
+                            doubleglyph->y = glyph->y;
+                        }
+
+                        glyphs.Flush(cr, aDrawToPath, isRTL);
                     }
-                    glyph->x = ToDeviceUnits(glyphX, devUnitsPerAppUnit);
-                    glyph->y = ToDeviceUnits(y + details->mYOffset, devUnitsPerAppUnit);
-
-                    // synthetic bolding by drawing with a one-pixel offset
-                    if (mSyntheticBoldOffset) {
-                        cairo_glyph_t *doubleglyph;
-                        doubleglyph = glyphs.AppendGlyph();
-                        doubleglyph->index = glyph->index;
-                        doubleglyph->x =
-                            ToDeviceUnits(glyphX + synBoldDevUnitOffsetAppUnits,
-                                          devUnitsPerAppUnit);
-                        doubleglyph->y = glyph->y;
-                    }
-
-                    glyphs.Flush(cr, aDrawToPath, isRTL);
+                    x += direction*advance;
                 }
-                x += direction*advance;
             }
         }
 
         if (aSpacing) {
             double space = aSpacing[i - aStart].mAfter;
             if (i + 1 < aEnd) {
                 space += aSpacing[i + 1 - aStart].mBefore;
             }
@@ -1243,37 +1248,16 @@ gfxFont::Draw(gfxTextRun *aTextRun, PRUi
     }
 
     // draw any remaining glyphs
     glyphs.Flush(cr, aDrawToPath, isRTL, PR_TRUE);
 
     *aPt = gfxPoint(x, y);
 }
 
-static PRInt32
-GetAdvanceForGlyphs(gfxTextRun *aTextRun, PRUint32 aStart, PRUint32 aEnd)
-{
-    const gfxTextRun::CompressedGlyph *glyphData = aTextRun->GetCharacterGlyphs() + aStart;
-    PRInt32 advance = 0;
-    PRUint32 i;
-    for (i = aStart; i < aEnd; ++i, ++glyphData) {
-        if (glyphData->IsSimpleGlyph()) {
-            advance += glyphData->GetSimpleAdvance();   
-        } else {
-            PRUint32 glyphCount = glyphData->GetGlyphCount();
-            const gfxTextRun::DetailedGlyph *details = aTextRun->GetDetailedGlyphs(i);
-            PRUint32 j;
-            for (j = 0; j < glyphCount; ++j, ++details) {
-                advance += details->mAdvance;
-            }
-        }
-    }
-    return advance;
-}
-
 static void
 UnionRange(gfxFloat aX, gfxFloat* aDestMin, gfxFloat* aDestMax)
 {
     *aDestMin = PR_MIN(*aDestMin, aX);
     *aDestMax = PR_MAX(*aDestMax, aX);
 }
 
 // We get precise glyph extents if the textrun creator requested them, or
@@ -1379,37 +1363,42 @@ gfxFont::Measure(gfxTextRun *aTextRun,
                     }
                     glyphRect.pos.x += x;
                     metrics.mBoundingBox = metrics.mBoundingBox.Union(glyphRect);
                 }
             }
             x += direction*advance;
         } else {
             PRUint32 glyphCount = glyphData->GetGlyphCount();
-            const gfxTextRun::DetailedGlyph *details = aTextRun->GetDetailedGlyphs(i);
-            PRUint32 j;
-            for (j = 0; j < glyphCount; ++j, ++details) {
-                PRUint32 glyphIndex = details->mGlyphID;
-                gfxPoint glyphPt(x + details->mXOffset, details->mYOffset);
-                double advance = details->mAdvance;
-                gfxRect glyphRect;
-                if (glyphData->IsMissing() || !extents ||
-                    !extents->GetTightGlyphExtentsAppUnits(this,
-                            aRefContext, glyphIndex, &glyphRect)) {
-                    // We might have failed to get glyph extents due to
-                    // OOM or something
-                    glyphRect = gfxRect(0, -metrics.mAscent,
-                        advance, metrics.mAscent + metrics.mDescent);
+            if (glyphCount > 0) {
+                const gfxTextRun::DetailedGlyph *details =
+                    aTextRun->GetDetailedGlyphs(i);
+                NS_ASSERTION(details != nsnull,
+                             "detaiedGlyph record should not be missing!");
+                PRUint32 j;
+                for (j = 0; j < glyphCount; ++j, ++details) {
+                    PRUint32 glyphIndex = details->mGlyphID;
+                    gfxPoint glyphPt(x + details->mXOffset, details->mYOffset);
+                    double advance = details->mAdvance;
+                    gfxRect glyphRect;
+                    if (glyphData->IsMissing() || !extents ||
+                        !extents->GetTightGlyphExtentsAppUnits(this,
+                                aRefContext, glyphIndex, &glyphRect)) {
+                        // We might have failed to get glyph extents due to
+                        // OOM or something
+                        glyphRect = gfxRect(0, -metrics.mAscent,
+                            advance, metrics.mAscent + metrics.mDescent);
+                    }
+                    if (isRTL) {
+                        glyphRect.pos.x -= advance;
+                    }
+                    glyphRect.pos.x += x;
+                    metrics.mBoundingBox = metrics.mBoundingBox.Union(glyphRect);
+                    x += direction*advance;
                 }
-                if (isRTL) {
-                    glyphRect.pos.x -= advance;
-                }
-                glyphRect.pos.x += x;
-                metrics.mBoundingBox = metrics.mBoundingBox.Union(glyphRect);
-                x += direction*advance;
             }
         }
         // Every other glyph type is ignored
         if (aSpacing) {
             double space = aSpacing[i - aStart].mAfter;
             if (i + 1 < aEnd) {
                 space += aSpacing[i + 1 - aStart].mBefore;
             }
@@ -3081,17 +3070,17 @@ gfxTextRun::Clone(const gfxTextRunFactor
     if (!mCharacterGlyphs)
         return nsnull;
 
     nsAutoPtr<gfxTextRun> textRun;
     textRun = gfxTextRun::Create(aParams, aText, aLength, aFontGroup, aFlags);
     if (!textRun)
         return nsnull;
 
-    textRun->CopyGlyphDataFrom(this, 0, mCharacterCount, 0, PR_FALSE);
+    textRun->CopyGlyphDataFrom(this, 0, mCharacterCount, 0);
     return textRun.forget();
 }
 
 PRBool
 gfxTextRun::SetPotentialLineBreaks(PRUint32 aStart, PRUint32 aLength,
                                    PRPackedBool *aBreakBefore,
                                    gfxContext *aRefContext)
 {
@@ -3129,17 +3118,17 @@ gfxTextRun::ComputeLigatureData(PRUint32
         NS_ASSERTION(i > 0, "Ligature at the start of the run??");
     }
     result.mLigatureStart = i;
     for (i = aPartStart + 1; i < mCharacterCount && !charGlyphs[i].IsLigatureGroupStart(); ++i) {
     }
     result.mLigatureEnd = i;
 
     PRInt32 ligatureWidth =
-        GetAdvanceForGlyphs(this, result.mLigatureStart, result.mLigatureEnd);
+        GetAdvanceForGlyphs(result.mLigatureStart, result.mLigatureEnd);
     // Count the number of started clusters we have seen
     PRUint32 totalClusterCount = 0;
     PRUint32 partClusterIndex = 0;
     PRUint32 partClusterCount = 0;
     for (i = result.mLigatureStart; i < result.mLigatureEnd; ++i) {
         // Treat the first character of the ligature as the start of a
         // cluster for our purposes of allocating ligature width to its
         // characters.
@@ -3190,16 +3179,42 @@ gfxTextRun::ComputePartialLigatureWidth(
                                         PropertyProvider *aProvider)
 {
     if (aPartStart >= aPartEnd)
         return 0;
     LigatureData data = ComputeLigatureData(aPartStart, aPartEnd, aProvider);
     return data.mPartWidth;
 }
 
+PRInt32
+gfxTextRun::GetAdvanceForGlyphs(PRUint32 aStart, PRUint32 aEnd)
+{
+    const CompressedGlyph *glyphData = mCharacterGlyphs + aStart;
+    PRInt32 advance = 0;
+    PRUint32 i;
+    for (i = aStart; i < aEnd; ++i, ++glyphData) {
+        if (glyphData->IsSimpleGlyph()) {
+            advance += glyphData->GetSimpleAdvance();   
+        } else {
+            PRUint32 glyphCount = glyphData->GetGlyphCount();
+            if (glyphCount == 0) {
+                continue;
+            }
+            const DetailedGlyph *details = GetDetailedGlyphs(i);
+            if (details) {
+                PRUint32 j;
+                for (j = 0; j < glyphCount; ++j, ++details) {
+                    advance += details->mAdvance;
+                }
+            }
+        }
+    }
+    return advance;
+}
+
 static void
 GetAdjustedSpacing(gfxTextRun *aTextRun, PRUint32 aStart, PRUint32 aEnd,
                    gfxTextRun::PropertyProvider *aProvider,
                    gfxTextRun::PropertyProvider::Spacing *aSpacing)
 {
     if (aStart >= aEnd)
         return;
 
@@ -3427,23 +3442,26 @@ gfxTextRun::AdjustAdvancesForSyntheticBo
                         PRUint32 glyphIndex = glyphData->GetSimpleGlyph();
                         glyphData->SetComplex(PR_TRUE, PR_TRUE, 1);
                         DetailedGlyph detail = {glyphIndex, advance, 0, 0};
                         SetGlyphs(i, *glyphData, &detail);
                     }
                 } else {
                     // complex glyphs ==> add offset at cluster/ligature boundaries
                     PRUint32 detailedLength = glyphData->GetGlyphCount();
-                    if (detailedLength && mDetailedGlyphs) {
-                        gfxTextRun::DetailedGlyph *details = mDetailedGlyphs[i].get();
-                        if (!details) continue;
-                        if (isRTL)
+                    if (detailedLength) {
+                        gfxTextRun::DetailedGlyph *details = GetDetailedGlyphs(i);
+                        if (!details) {
+                            continue;
+                        }
+                        if (isRTL) {
                             details[0].mAdvance += synAppUnitOffset;
-                        else
+                        } else {
                             details[detailedLength - 1].mAdvance += synAppUnitOffset;
+                        }
                     }
                 }
             }
         }
     }
 }
 
 void
@@ -3721,17 +3739,17 @@ gfxTextRun::BreakAndMeasureText(PRUint32
                     aborted = PR_TRUE;
                     break;
                 }
             }
         }
         
         gfxFloat charAdvance;
         if (i >= ligatureRunStart && i < ligatureRunEnd) {
-            charAdvance = GetAdvanceForGlyphs(this, i, i + 1);
+            charAdvance = GetAdvanceForGlyphs(i, i + 1);
             if (haveSpacing) {
                 PropertyProvider::Spacing *space = &spacingBuffer[i - bufferStart];
                 charAdvance += space->mBefore + space->mAfter;
             }
         } else {
             charAdvance = ComputePartialLigatureWidth(i, i + 1, aProvider);
         }
         
@@ -3812,17 +3830,17 @@ gfxTextRun::GetAdvanceWidth(PRUint32 aSt
                                spacingBuffer.Elements());
             for (i = 0; i < ligatureRunEnd - ligatureRunStart; ++i) {
                 PropertyProvider::Spacing *space = &spacingBuffer[i];
                 result += space->mBefore + space->mAfter;
             }
         }
     }
 
-    return result + GetAdvanceForGlyphs(this, ligatureRunStart, ligatureRunEnd);
+    return result + GetAdvanceForGlyphs(ligatureRunStart, ligatureRunEnd);
 }
 
 PRBool
 gfxTextRun::SetLineBreaks(PRUint32 aStart, PRUint32 aLength,
                           PRBool aLineBreakBefore, PRBool aLineBreakAfter,
                           gfxFloat *aAdvanceWidthDelta,
                           gfxContext *aRefContext)
 {
@@ -3971,32 +3989,30 @@ gfxTextRun::CountMissingGlyphs()
     return count;
 }
 
 gfxTextRun::DetailedGlyph *
 gfxTextRun::AllocateDetailedGlyphs(PRUint32 aIndex, PRUint32 aCount)
 {
     NS_ASSERTION(aIndex < mCharacterCount, "Index out of range");
 
-    if (!mCharacterGlyphs)
+    if (!mCharacterGlyphs) {
         return nsnull;
+    }
 
     if (!mDetailedGlyphs) {
-        mDetailedGlyphs = new nsAutoArrayPtr<DetailedGlyph>[mCharacterCount];
-        if (!mDetailedGlyphs) {
-            mCharacterGlyphs[aIndex].SetMissing(0);
-            return nsnull;
-        }
+        mDetailedGlyphs = new DetailedGlyphStore();
     }
-    DetailedGlyph *details = new DetailedGlyph[aCount];
+
+    DetailedGlyph *details = mDetailedGlyphs->Allocate(aIndex, aCount);
     if (!details) {
         mCharacterGlyphs[aIndex].SetMissing(0);
         return nsnull;
     }
-    mDetailedGlyphs[aIndex] = details;
+
     return details;
 }
 
 void
 gfxTextRun::SetGlyphs(PRUint32 aIndex, CompressedGlyph aGlyph,
                       const DetailedGlyph *aGlyphs)
 {
     NS_ASSERTION(!aGlyph.IsSimpleGlyph(), "Simple glyphs not handled here");
@@ -4058,75 +4074,46 @@ gfxTextRun::FilterIfIgnorable(PRUint32 a
             details->mYOffset = 0;
             mCharacterGlyphs[aIndex].SetMissing(1);
             return PR_TRUE;
         }
     }
     return PR_FALSE;
 }
 
-static void
-ClearCharacters(gfxTextRun::CompressedGlyph *aGlyphs, PRUint32 aLength)
-{
-    memset(aGlyphs, 0, sizeof(gfxTextRun::CompressedGlyph)*aLength);
-}
-
 void
 gfxTextRun::CopyGlyphDataFrom(gfxTextRun *aSource, PRUint32 aStart,
-                              PRUint32 aLength, PRUint32 aDest,
-                              PRBool aStealData)
+                              PRUint32 aLength, PRUint32 aDest)
 {
     NS_ASSERTION(aStart + aLength <= aSource->GetLength(),
                  "Source substring out of range");
     NS_ASSERTION(aDest + aLength <= GetLength(),
                  "Destination substring out of range");
 
-    PRUint32 i;
-    // Copy base character data
-    for (i = 0; i < aLength; ++i) {
+    // Copy base glyph data, and DetailedGlyph data where present
+    for (PRUint32 i = 0; i < aLength; ++i) {
         CompressedGlyph g = aSource->mCharacterGlyphs[i + aStart];
         g.SetCanBreakBefore(mCharacterGlyphs[i + aDest].CanBreakBefore());
-        mCharacterGlyphs[i + aDest] = g;
-        if (aStealData) {
-            aSource->mCharacterGlyphs[i + aStart] = CompressedGlyph();
-        }
-    }
-
-    // Copy detailed glyphs
-    if (aSource->mDetailedGlyphs) {
-        for (i = 0; i < aLength; ++i) {
-            DetailedGlyph *details = aSource->mDetailedGlyphs[i + aStart];
-            if (details) {
-                if (aStealData) {
-                    if (!mDetailedGlyphs) {
-                        mDetailedGlyphs = new nsAutoArrayPtr<DetailedGlyph>[mCharacterCount];
-                        if (!mDetailedGlyphs) {
-                            ClearCharacters(&mCharacterGlyphs[aDest], aLength);
-                            return;
-                        }
-                    }        
-                    mDetailedGlyphs[i + aDest] = details;
-                    aSource->mDetailedGlyphs[i + aStart].forget();
+        if (!g.IsSimpleGlyph()) {
+            PRUint32 count = g.GetGlyphCount();
+            if (count > 0) {
+                DetailedGlyph *dst = AllocateDetailedGlyphs(i + aDest, count);
+                if (dst) {
+                    DetailedGlyph *src = aSource->GetDetailedGlyphs(i + aStart);
+                    if (src) {
+                        ::memcpy(dst, src, count * sizeof(DetailedGlyph));
+                    } else {
+                        g.SetMissing(0);
+                    }
                 } else {
-                    PRUint32 glyphCount = mCharacterGlyphs[i + aDest].GetGlyphCount();
-                    DetailedGlyph *dest = AllocateDetailedGlyphs(i + aDest, glyphCount);
-                    if (!dest) {
-                        ClearCharacters(&mCharacterGlyphs[aDest], aLength);
-                        return;
-                    }
-                    memcpy(dest, details, sizeof(DetailedGlyph)*glyphCount);
+                    g.SetMissing(0);
                 }
-            } else if (mDetailedGlyphs) {
-                mDetailedGlyphs[i + aDest] = nsnull;
             }
         }
-    } else if (mDetailedGlyphs) {
-        for (i = 0; i < aLength; ++i) {
-            mDetailedGlyphs[i + aDest] = nsnull;
-        }
+        mCharacterGlyphs[i + aDest] = g;
     }
 
     // Copy glyph runs
     GlyphRunIterator iter(aSource, aStart, aLength);
 #ifdef DEBUG
     gfxFont *lastFont = nsnull;
 #endif
     while (iter.NextRun()) {
@@ -4172,17 +4159,17 @@ gfxTextRun::SetSpaceGlyph(gfxFont *aFont
         };
         static const PRUint8 space = ' ';
         nsAutoPtr<gfxTextRun> textRun;
         textRun = mFontGroup->MakeTextRun(&space, 1, &params,
             gfxTextRunFactory::TEXT_IS_8BIT | gfxTextRunFactory::TEXT_IS_ASCII |
             gfxTextRunFactory::TEXT_IS_PERSISTENT);
         if (!textRun || !textRun->mCharacterGlyphs)
             return;
-        CopyGlyphDataFrom(textRun, 0, 1, aCharIndex, PR_TRUE);
+        CopyGlyphDataFrom(textRun, 0, 1, aCharIndex);
         return;
     }
 
     AddGlyphRun(aFont, aCharIndex);
     CompressedGlyph g;
     g.SetSimpleGlyph(spaceWidthAppUnits, spaceGlyph);
     SetSimpleGlyph(aCharIndex, g);
 }
@@ -4219,20 +4206,25 @@ gfxTextRun::FetchGlyphExtents(gfxContext
                         }
 #ifdef DEBUG_TEXT_RUN_STORAGE_METRICS
                         ++gGlyphExtentsSetupEagerSimple;
 #endif
                         font->SetupGlyphExtents(aRefContext, glyphIndex, PR_FALSE, extents);
                     }
                 }
             } else if (!glyphData->IsMissing()) {
-                PRUint32 k;
                 PRUint32 glyphCount = glyphData->GetGlyphCount();
+                if (glyphCount == 0) {
+                    continue;
+                }
                 const gfxTextRun::DetailedGlyph *details = GetDetailedGlyphs(j);
-                for (k = 0; k < glyphCount; ++k, ++details) {
+                if (!details) {
+                    continue;
+                }
+                for (PRUint32 k = 0; k < glyphCount; ++k, ++details) {
                     PRUint32 glyphIndex = details->mGlyphID;
                     if (!extents->IsGlyphKnownWithTightExtents(glyphIndex)) {
                         if (!fontIsSetup) {
                             font->SetupCairoFont(aRefContext);
                             fontIsSetup = PR_TRUE;
                         }
 #ifdef DEBUG_TEXT_RUN_STORAGE_METRICS
                         ++gGlyphExtentsSetupEagerTight;
--- a/gfx/thebes/gfxFont.h
+++ b/gfx/thebes/gfxFont.h
@@ -1900,19 +1900,16 @@ public:
      * simple glyph or has no associated glyphs. If non-null the data is copied,
      * the caller retains ownership.
      */
     void SetSimpleGlyph(PRUint32 aCharIndex, CompressedGlyph aGlyph) {
         NS_ASSERTION(aGlyph.IsSimpleGlyph(), "Should be a simple glyph here");
         if (mCharacterGlyphs) {
             mCharacterGlyphs[aCharIndex] = aGlyph;
         }
-        if (mDetailedGlyphs) {
-            mDetailedGlyphs[aCharIndex] = nsnull;
-        }
     }
     void SetGlyphs(PRUint32 aCharIndex, CompressedGlyph aGlyph,
                    const DetailedGlyph *aGlyphs);
     void SetMissingGlyph(PRUint32 aCharIndex, PRUint32 aUnicodeChar);
     void SetSpaceGlyph(gfxFont *aFont, gfxContext *aContext, PRUint32 aCharIndex);
 
     // If the character at aIndex is default-ignorable, set the glyph
     // to be invisible-missing and return TRUE, else return FALSE
@@ -1924,35 +1921,35 @@ public:
      * that some glyph extents might not be fetched due to OOM or other
      * errors.
      */
     void FetchGlyphExtents(gfxContext *aRefContext);
 
     // API for access to the raw glyph data, needed by gfxFont::Draw
     // and gfxFont::GetBoundingBox
     const CompressedGlyph *GetCharacterGlyphs() { return mCharacterGlyphs; }
-    const DetailedGlyph *GetDetailedGlyphs(PRUint32 aCharIndex) {
-        return mDetailedGlyphs ? mDetailedGlyphs[aCharIndex].get() : nsnull;
+    DetailedGlyph *GetDetailedGlyphs(PRUint32 aCharIndex) {
+        if (!mDetailedGlyphs) {
+            return nsnull;
+        }
+        return mDetailedGlyphs->Get(aCharIndex);
     }
-    PRBool HasDetailedGlyphs() { return mDetailedGlyphs.get() != nsnull; }
+    PRBool HasDetailedGlyphs() { return mDetailedGlyphs != nsnull; }
     PRUint32 CountMissingGlyphs();
     const GlyphRun *GetGlyphRuns(PRUint32 *aNumGlyphRuns) {
         *aNumGlyphRuns = mGlyphRuns.Length();
         return mGlyphRuns.Elements();
     }
     // Returns the index of the GlyphRun containing the given offset.
     // Returns mGlyphRuns.Length() when aOffset is mCharacterCount.
     PRUint32 FindFirstGlyphRunContaining(PRUint32 aOffset);
     // Copy glyph data for a range of characters from aSource to this
-    // textrun. If aStealData is true then we actually steal the glyph data,
-    // setting the data in aSource to "missing". aDest should be in the last
-    // glyphrun.
+    // textrun.
     virtual void CopyGlyphDataFrom(gfxTextRun *aSource, PRUint32 aStart,
-                                   PRUint32 aLength, PRUint32 aDest,
-                                   PRBool aStealData);
+                                   PRUint32 aLength, PRUint32 aDest);
 
     nsExpirationState *GetExpirationState() { return &mExpirationState; }
 
     struct LigatureData {
         // textrun offsets of the start and end of the containing ligature
         PRUint32 mLigatureStart;
         PRUint32 mLigatureEnd;
         // appunits advance to the start of the ligature part within the ligature;
@@ -2004,16 +2001,19 @@ protected:
                                             PRUint32 aFlags);
 
 private:
     // **** general helpers **** 
 
     // Allocate aCount DetailedGlyphs for the given index
     DetailedGlyph *AllocateDetailedGlyphs(PRUint32 aCharIndex, PRUint32 aCount);
 
+    // Get the total advance for a range of glyphs.
+    PRInt32 GetAdvanceForGlyphs(PRUint32 aStart, PRUint32 aEnd);
+
     // Spacing for characters outside the range aSpacingStart/aSpacingEnd
     // is assumed to be zero; such characters are not passed to aProvider.
     // This is useful to protect aProvider from being passed character indices
     // it is not currently able to handle.
     PRBool GetAdjustedSpacingArray(PRUint32 aStart, PRUint32 aEnd,
                                    PropertyProvider *aProvider,
                                    PRUint32 aSpacingStart, PRUint32 aSpacingEnd,
                                    nsTArray<PropertyProvider::Spacing> *aSpacing);
@@ -2057,17 +2057,128 @@ private:
                     PRUint32 aSpacingStart, PRUint32 aSpacingEnd);
 
     // All our glyph data is in logical order, not visual.
     // mCharacterGlyphs is allocated by the factory that creates the textrun,
     // to avoid the possibility of failure during the constructor;
     // however, ownership passes to the textrun during construction and so
     // it must be deleted in the destructor.
     CompressedGlyph*                               mCharacterGlyphs;
-    nsAutoArrayPtr<nsAutoArrayPtr<DetailedGlyph> > mDetailedGlyphs; // only non-null if needed
+
+    // For characters whose glyph data does not fit the "simple" glyph criteria
+    // in CompressedGlyph, we use a sorted array to store the association
+    // between the source character offset and an index into an array 
+    // DetailedGlyphs. The CompressedGlyph record includes a count of
+    // the number of DetailedGlyph records that belong to the character,
+    // starting at the given index.
+    class DetailedGlyphStore {
+    public:
+        DetailedGlyphStore()
+            : mLastUsed(0)
+        { }
+
+        // This is optimized for the most common calling patterns:
+        // we rarely need random access to the records, access is most commonly
+        // sequential through the textRun, so we record the last-used index
+        // and check whether the caller wants the same record again, or the
+        // next; if not, it's most likely we're starting over from the start
+        // of the run, so we check the first entry before resorting to binary
+        // search as a last resort.
+        DetailedGlyph* Get(PRUint32 aOffset) {
+            NS_ASSERTION(mOffsetToIndex.Length() > 0,
+                         "no detailed glyph records!");
+            DetailedGlyph* details = mDetails.Elements();
+            // check common cases (fwd iteration, initial entry, etc) first
+            if (mLastUsed < mOffsetToIndex.Length() - 1 &&
+                aOffset == mOffsetToIndex[mLastUsed + 1].mOffset) {
+                ++mLastUsed;
+            } else if (aOffset == mOffsetToIndex[0].mOffset) {
+                mLastUsed = 0;
+            } else if (aOffset == mOffsetToIndex[mLastUsed].mOffset) {
+                // do nothing
+            } else if (mLastUsed > 0 &&
+                       aOffset == mOffsetToIndex[mLastUsed - 1].mOffset) {
+                --mLastUsed;
+            } else {
+                mLastUsed =
+                    mOffsetToIndex.BinaryIndexOf(aOffset, CompareToOffset());
+            }
+            NS_ASSERTION(mLastUsed != nsTArray<DGRec>::NoIndex,
+                         "detailed glyph record missing!");
+            return details + mOffsetToIndex[mLastUsed].mIndex;
+        }
+
+        DetailedGlyph* Allocate(PRUint32 aOffset, PRUint32 aCount) {
+            PRUint32 detailIndex = mDetails.Length();
+            DetailedGlyph *details = mDetails.AppendElements(aCount);
+            if (!details) {
+                return nsnull;
+            }
+            // We normally set up glyph records sequentially, so the common case
+            // here is to append new records to the mOffsetToIndex array;
+            // test for that before falling back to the InsertElementSorted
+            // method.
+            if (mOffsetToIndex.Length() == 0 ||
+                aOffset > mOffsetToIndex[mLastUsed].mOffset) {
+                if (!mOffsetToIndex.AppendElement(DGRec(aOffset, detailIndex))) {
+                    return nsnull;
+                }
+            } else {
+                if (!mOffsetToIndex.InsertElementSorted(DGRec(aOffset, detailIndex),
+                                                        CompareRecordOffsets())) {
+                    return nsnull;
+                }
+            }
+            return details;
+        }
+
+    private:
+        struct DGRec {
+            DGRec(const PRUint32& aOffset, const PRUint32& aIndex)
+                : mOffset(aOffset), mIndex(aIndex) { }
+            PRUint32 mOffset; // source character offset in the textrun
+            PRUint32 mIndex;  // index where this char's DetailedGlyphs begin
+        };
+
+        struct CompareToOffset {
+            PRBool Equals(const DGRec& a, const PRUint32& b) const {
+                return a.mOffset == b;
+            }
+            PRBool LessThan(const DGRec& a, const PRUint32& b) const {
+                return a.mOffset < b;
+            }
+        };
+
+        struct CompareRecordOffsets {
+            PRBool Equals(const DGRec& a, const DGRec& b) const {
+                return a.mOffset == b.mOffset;
+            }
+            PRBool LessThan(const DGRec& a, const DGRec& b) const {
+                return a.mOffset < b.mOffset;
+            }
+        };
+
+        // Concatenated array of all the DetailedGlyph records needed for the
+        // textRun; individual character offsets are associated with indexes
+        // into this array via the mOffsetToIndex table.
+        nsTArray<DetailedGlyph>     mDetails;
+
+        // For each character offset that needs DetailedGlyphs, we record the
+        // index in mDetails where the list of glyphs begins. This array is
+        // sorted by mOffset.
+        nsTArray<DGRec>             mOffsetToIndex;
+
+        // Records the most recently used index into mOffsetToIndex, so that
+        // we can support sequential access more quickly than just doing
+        // a binary search each time.
+        nsTArray<DGRec>::index_type mLastUsed;
+    };
+
+    nsAutoPtr<DetailedGlyphStore>   mDetailedGlyphs;
+
     // XXX this should be changed to a GlyphRun plus a maybe-null GlyphRun*,
     // for smaller size especially in the super-common one-glyphrun case
     nsAutoTArray<GlyphRun,1>                       mGlyphRuns;
     // When TEXT_IS_8BIT is set, we use mSingle, otherwise we use mDouble.
     // When TEXT_IS_PERSISTENT is set, we don't own the text, otherwise we
     // own the text. When we own the text, it's allocated fused with the
     // mCharacterGlyphs array, and therefore need not be explicitly deleted.
     // This text is not null-terminated.
--- a/gfx/thebes/gfxTextRunWordCache.cpp
+++ b/gfx/thebes/gfxTextRunWordCache.cpp
@@ -393,17 +393,17 @@ TextRunWordCache::LookupWord(gfxTextRun 
 
     if (existingEntry) {
         if (aDeferredWords) {
             DeferredWord word = { existingEntry->mTextRun,
                   existingEntry->mWordOffset, aStart, aEnd - aStart, aHash };
             aDeferredWords->AppendElement(word);
         } else {
             aTextRun->CopyGlyphDataFrom(existingEntry->mTextRun,
-                existingEntry->mWordOffset, aEnd - aStart, aStart, PR_FALSE);
+                existingEntry->mWordOffset, aEnd - aStart, aStart);
         }
         return PR_TRUE;
     }
 
 #ifdef DEBUG
     ++aTextRun->mCachedWords;
 #endif
     // Set up the cache entry so that if later in this textrun we hit this
@@ -496,24 +496,21 @@ TextRunWordCache::FinishTextRun(gfxTextR
                         groupEntry->mHashedByFont = PR_FALSE;
                         NS_ASSERTION(mCache.GetEntry(key),
                                      "We should find the thing we just added!");
                     }
                 }
             }
         }
         if (aSuccessful) {
-            // Copy the word. If the source is aNewRun, then
-            // allow CopyGlyphDataFrom to steal the internal data of
-            // aNewRun since that's only temporary anyway.
+            // Copy the word.
             PRUint32 sourceOffset = word->mSourceOffset;
             PRUint32 destOffset = word->mDestOffset;
             PRUint32 length = word->mLength;
             nsAutoPtr<gfxTextRun> tmpTextRun;
-            PRBool stealData = source == aNewRun;
             if (wordStartsInsideCluster || wordStartsInsideLigature) {
                 NS_ASSERTION(sourceOffset > 0, "How can the first character be inside a cluster?");
                 if (wordStartsInsideCluster && destOffset > 0 &&
                     IsBoundarySpace(aTextRun->GetChar(destOffset - 1))) {
                     // The first character of the word formed a cluster
                     // with the preceding space.
                     // We should copy over data for the preceding space
                     // as well. The glyphs have probably been attached
@@ -530,31 +527,30 @@ TextRunWordCache::FinishTextRun(gfxTextR
                     // We need to find out what the platform would do
                     // if the characters were at the start of the text.
                     tmpTextRun = aNewRun->GetFontGroup()->MakeTextRun(
                         source->GetTextUnicode() + sourceOffset, length, aParams,
                         aNewRun->GetFlags());
                     if (tmpTextRun) {
                         source = tmpTextRun;
                         sourceOffset = 0;
-                        stealData = PR_TRUE;
                     } else {
                         // If we failed to create the temporary run (OOM),
                         // skip the word, as if aSuccessful had been FALSE.
                         // (In practice this is only likely to occur if
                         // we're on the verge of an OOM crash anyhow.
                         // But ignoring gfxFontGroup::MakeTextRun() failure
                         // is bad because it means we'd be using an invalid
                         // source pointer.)
                         continue;
                     }
                 }
             }
             aTextRun->CopyGlyphDataFrom(source, sourceOffset, length,
-                destOffset, stealData);
+                                        destOffset);
             // Fill in additional spaces
             PRUint32 endCharIndex;
             if (i + 1 < aDeferredWords.Length()) {
                 endCharIndex = aDeferredWords[i + 1].mDestOffset;
             } else {
                 endCharIndex = aTextRun->GetLength();
             }
             PRUint32 charIndex;
--- a/layout/generic/nsTextRunTransformations.cpp
+++ b/layout/generic/nsTextRunTransformations.cpp
@@ -304,17 +304,17 @@ nsFontVariantTextRunFactory::RebuildText
       // (and also child will be shaped appropriately)
       NS_ASSERTION(canBreakBeforeArray.Length() == i - runStart,
                    "lost some break-before values?");
       child->SetPotentialLineBreaks(0, canBreakBeforeArray.Length(),
           canBreakBeforeArray.Elements(), aRefContext);
       if (transformedChild) {
         transformedChild->FinishSettingProperties(aRefContext);
       }
-      aTextRun->CopyGlyphDataFrom(child, 0, child->GetLength(), runStart, PR_FALSE);
+      aTextRun->CopyGlyphDataFrom(child, 0, child->GetLength(), runStart);
 
       runStart = i;
       styleArray.Clear();
       canBreakBeforeArray.Clear();
     }
 
     if (i < length) {
       runIsLowercase = isLowercase;
@@ -422,11 +422,11 @@ nsCaseTransformTextRunFactory::RebuildTe
   if (extraCharsCount > 0) {
     // Now merge multiple characters into one multi-glyph character as required
     MergeCharactersInTextRun(aTextRun, child, charsToMergeArray.Elements());
   } else {
     // No merging to do, so just copy; this produces a more optimized textrun.
     // We can't steal the data because the child may be cached and stealing
     // the data would break the cache.
     aTextRun->ResetGlyphRuns();
-    aTextRun->CopyGlyphDataFrom(child, 0, child->GetLength(), 0, PR_FALSE);
+    aTextRun->CopyGlyphDataFrom(child, 0, child->GetLength(), 0);
   }
 }