Bug 423270. Make gfxTextRunWordCache handle cases where a space and another character combine to form a ligature. r=vlad
authorroc+@cs.cmu.edu
Sun, 06 Apr 2008 03:12:21 -0700
changeset 13959 f5870c780e33a8d9cff5021e2f7fb7f3124e0169
parent 13958 e322204aafb2fd3249dc3faed7c8e15fc0ca698f
child 13960 a09999d69bad0f3efa52a91fe787a8ddf834a31f
push idunknown
push userunknown
push dateunknown
reviewersvlad
bugs423270
milestone1.9pre
Bug 423270. Make gfxTextRunWordCache handle cases where a space and another character combine to form a ligature. r=vlad
gfx/thebes/src/gfxTextRunWordCache.cpp
--- a/gfx/thebes/src/gfxTextRunWordCache.cpp
+++ b/gfx/thebes/src/gfxTextRunWordCache.cpp
@@ -337,36 +337,39 @@ TextRunWordCache::FinishTextRun(gfxTextR
         gfxTextRun *source = word->mSourceTextRun;
         if (!source) {
             source = aNewRun;
         }
         // If the word starts inside a cluster we don't want this word
         // in the cache, so we'll remove the associated cache entry
         PRBool wordStartsInsideCluster =
             !source->IsClusterStart(word->mSourceOffset);
+        PRBool wordStartsInsideLigature =
+            !source->IsLigatureGroupStart(word->mSourceOffset);
         if (source == aNewRun) {
             // We created a cache entry for this word based on the assumption
             // that the word matches GetFontAt(0). If this assumption is false,
             // we need to remove that cache entry and replace it with an entry
             // keyed off the fontgroup.
             PRBool rekeyWithFontGroup =
                 GetWordFontOrGroup(aNewRun, word->mSourceOffset, word->mLength) != font;
-            if (!aSuccessful || wordStartsInsideCluster || rekeyWithFontGroup) {
+            if (!aSuccessful || rekeyWithFontGroup ||
+                wordStartsInsideCluster || wordStartsInsideLigature) {
                 // We need to remove the current placeholder cache entry
                 CacheHashKey key(aTextRun, font, word->mDestOffset, word->mLength,
                                  word->mHash);
                 NS_ASSERTION(mCache.GetEntry(key),
                              "This entry should have been added previously!");
                 mCache.RemoveEntry(key);
 #ifdef DEBUG
                 --aTextRun->mCachedWords;
 #endif
                 PR_LOG(gWordCacheLog, PR_LOG_DEBUG, ("%p(%d-%d,%d): removed using font", aTextRun, word->mDestOffset, word->mLength, word->mHash));
                 
-                if (aSuccessful && !wordStartsInsideCluster) {
+                if (aSuccessful && !wordStartsInsideCluster && !wordStartsInsideLigature) {
                     key.mFontOrGroup = fontGroup;
                     CacheHashEntry *groupEntry = mCache.PutEntry(key);
                     if (groupEntry) {
 #ifdef DEBUG
                         ++aTextRun->mCachedWords;
 #endif
                         PR_LOG(gWordCacheLog, PR_LOG_DEBUG, ("%p(%d-%d,%d): added using fontgroup", aTextRun, word->mDestOffset, word->mLength, word->mHash));
                         groupEntry->mTextRun = aTextRun;
@@ -382,33 +385,36 @@ TextRunWordCache::FinishTextRun(gfxTextR
             // Copy the word. If the source is aNewRun, then
             // allow CopyGlyphDataFrom to steal the internal data of
             // aNewRun since that's only temporary anyway.
             PRUint32 sourceOffset = word->mSourceOffset;
             PRUint32 destOffset = word->mDestOffset;
             PRUint32 length = word->mLength;
             nsAutoPtr<gfxTextRun> tmpTextRun;
             PRBool stealData = source == aNewRun;
-            if (wordStartsInsideCluster) {
+            if (wordStartsInsideCluster || wordStartsInsideLigature) {
                 NS_ASSERTION(sourceOffset > 0, "How can the first character be inside a cluster?");
-                if (destOffset > 0 && IsBoundarySpace(aTextRun->GetChar(destOffset - 1))) {
+                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
                     // to that character.
                     --sourceOffset;
                     --destOffset;
                     ++length;
                 } else {
-                    // URK! This case sucks! We have combining marks
-                    // at the start of the text. We had to prepend a space
-                    // just so we could detect these are combining marks
-                    // (so we can keep this "word" out of the cache).
-                    // But now the data in aNewRun is no use to us. We
-                    // need to find out what the platform would do
-                    // if the marks were at the start of the text.
+                    // URK! This case sucks! We have combining marks or
+                    // part of a ligature at the start of the text. We
+                    // had to prepend a space just so we could detect this
+                    // situation (so we can keep this "word" out of the
+                    // cache). But now the data in aNewRun is no use to us.
+                    // 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());
                     source = tmpTextRun;
                     sourceOffset = 0;
                     stealData = PR_TRUE;
                 }
             }