Bug 1655364 - Rename gfxShapedText::SetGlyphs to SetDetailedGlyphs and make it just set glyph information, not clobber character-type flags. r=heycam

Differential Revision: https://phabricator.services.mozilla.com/D85187

gfx/thebes/gfxCoreTextShaper.cpp

@@ -517,11 +517,9 @@ nsresult gfxCoreTextShaper::SetGlyphsFromRun(gfxShapedText* aShapedText,
         advance = int32_t(toNextGlyph * appUnitsPerDevUnit);
       }
 
-      bool isClusterStart = charGlyphs[baseCharIndex].IsClusterStart();
-      aShapedText->SetGlyphs(aOffset + baseCharIndex,
-                             CompressedGlyph::MakeComplex(
-                                 isClusterStart, true, detailedGlyphs.Length()),
-                             detailedGlyphs.Elements());
+      aShapedText->SetDetailedGlyphs(aOffset + baseCharIndex,
+                                     detailedGlyphs.Length(),
+                                     detailedGlyphs.Elements());
 
       detailedGlyphs.Clear();
     }

gfx/thebes/gfxFT2Fonts.cpp

@@ -144,10 +144,7 @@ void gfxFT2Font::AddRange(const char16_t* aText, uint32_t aOffset,
       NS_ASSERTION(details.mGlyphID == gid,
                    "Seriously weird glyph ID detected!");
       details.mAdvance = advance;
-      bool isClusterStart = charGlyphs[aOffset].IsClusterStart();
-      aShapedText->SetGlyphs(
-          aOffset, CompressedGlyph::MakeComplex(isClusterStart, true, 1),
-          &details);
+      aShapedText->SetDetailedGlyphs(aOffset, 1, &details);
     }
   }
 }

gfx/thebes/gfxFont.cpp

@@ -630,18 +630,19 @@ gfxShapedText::DetailedGlyph* gfxShapedText::AllocateDetailedGlyphs(
   return mDetailedGlyphs->Allocate(aIndex, aCount);
 }
 
-void gfxShapedText::SetGlyphs(uint32_t aIndex, CompressedGlyph aGlyph,
-                              const DetailedGlyph* aGlyphs) {
-  NS_ASSERTION(!aGlyph.IsSimpleGlyph(), "Simple glyphs not handled here");
-  NS_ASSERTION(aIndex > 0 || aGlyph.IsLigatureGroupStart(),
-               "First character can't be a ligature continuation!");
+void gfxShapedText::SetDetailedGlyphs(uint32_t aIndex, uint32_t aGlyphCount,
+                                      const DetailedGlyph* aGlyphs) {
+  CompressedGlyph& g = GetCharacterGlyphs()[aIndex];
 
-  uint32_t glyphCount = aGlyph.GetGlyphCount();
-  if (glyphCount > 0) {
-    DetailedGlyph* details = AllocateDetailedGlyphs(aIndex, glyphCount);
-    memcpy(details, aGlyphs, sizeof(DetailedGlyph) * glyphCount);
+  MOZ_ASSERT(aIndex > 0 || g.IsLigatureGroupStart(),
+             "First character can't be a ligature continuation!");
+
+  if (aGlyphCount > 0) {
+    DetailedGlyph* details = AllocateDetailedGlyphs(aIndex, aGlyphCount);
+    memcpy(details, aGlyphs, sizeof(DetailedGlyph) * aGlyphCount);
   }
-  GetCharacterGlyphs()[aIndex] = aGlyph;
+
+  g.SetGlyphCount(aGlyphCount);
 }
 
 #define ZWNJ 0x200C
@@ -712,9 +713,11 @@ void gfxShapedText::AdjustAdvancesForSyntheticBold(float aSynBoldOffset,
         } else {
           // rare case, tested by making this the default
           uint32_t glyphIndex = glyphData->GetSimpleGlyph();
-          glyphData->SetComplex(true, true, 1);
+          // convert the simple CompressedGlyph to an empty complex record
+          glyphData->SetComplex(true, true, 0);
+          // then set its details (glyph ID with its new advance)
           DetailedGlyph detail = {glyphIndex, advance, gfx::Point()};
-          SetGlyphs(i, *glyphData, &detail);
+          SetDetailedGlyphs(i, 1, &detail);
         }
       }
     } else {

gfx/thebes/gfxFont.h

@@ -909,6 +909,12 @@ class gfxShapedText {
       NS_ASSERTION(!IsSimpleGlyph(), "Expected non-simple-glyph");
       return (mValue & GLYPH_COUNT_MASK) >> GLYPH_COUNT_SHIFT;
     }
+    void SetGlyphCount(uint32_t aGlyphCount) {
+      MOZ_ASSERT(!IsSimpleGlyph(), "Expected non-simple-glyph");
+      MOZ_ASSERT(GetGlyphCount() == 0, "Glyph count already set");
+      MOZ_ASSERT(aGlyphCount <= 0xffff, "Glyph count out of range");
+      mValue |= FLAG_NOT_MISSING | (aGlyphCount << GLYPH_COUNT_SHIFT);
+    }
 
     void SetIsSpace() { mValue |= FLAG_CHAR_IS_SPACE; }
     void SetIsTab() {
@@ -955,8 +961,10 @@ class gfxShapedText {
     mozilla::gfx::Point mOffset;
   };
 
-  void SetGlyphs(uint32_t aCharIndex, CompressedGlyph aGlyph,
-                 const DetailedGlyph* aGlyphs);
+  // Store DetailedGlyph records for the given index. (This does not modify
+  // the associated CompressedGlyph character-type or break flags.)
+  void SetDetailedGlyphs(uint32_t aIndex, uint32_t aGlyphCount,
+                         const DetailedGlyph* aGlyphs);
 
   void SetMissingGlyph(uint32_t aIndex, uint32_t aChar, gfxFont* aFont);
 
@@ -1056,7 +1064,8 @@ class gfxShapedText {
       DetailedGlyph details = {aGlyph.GetSimpleGlyph(),
                                (int32_t)aGlyph.GetSimpleAdvance(),
                                mozilla::gfx::Point()};
-      SetGlyphs(aIndex, CompressedGlyph().SetComplex(true, true, 1), &details);
+      aGlyph.SetComplex(true, true, 0);
+      SetDetailedGlyphs(aIndex, 1, &details);
     }
   }
 

gfx/thebes/gfxGraphiteShaper.cpp

@@ -422,11 +422,8 @@ nsresult gfxGraphiteShaper::SetGlyphsFromSegment(
           d->mAdvance = 0;
         }
       }
-      bool isClusterStart = charGlyphs[offs].IsClusterStart();
-      aShapedText->SetGlyphs(
-          aOffset + offs,
-          CompressedGlyph::MakeComplex(isClusterStart, true, details.Length()),
-          details.Elements());
+      aShapedText->SetDetailedGlyphs(aOffset + offs, details.Length(),
+                                     details.Elements());
     }
 
     // check unexpected offset

gfx/thebes/gfxHarfBuzzShaper.cpp

@@ -1687,11 +1687,9 @@ nsresult gfxHarfBuzzShaper::SetGlyphsFromRun(gfxShapedText* aShapedText,
         }
       }
 
-      bool isClusterStart = charGlyphs[baseCharIndex].IsClusterStart();
-      aShapedText->SetGlyphs(aOffset + baseCharIndex,
-                             CompressedGlyph::MakeComplex(
-                                 isClusterStart, true, detailedGlyphs.Length()),
-                             detailedGlyphs.Elements());
+      aShapedText->SetDetailedGlyphs(aOffset + baseCharIndex,
+                                     detailedGlyphs.Length(),
+                                     detailedGlyphs.Elements());
 
       detailedGlyphs.Clear();
     }

gfx/thebes/gfxTextRun.cpp

@@ -1434,13 +1434,12 @@ void gfxTextRun::CopyGlyphDataFrom(gfxShapedWord* aShapedWord,
   if (aShapedWord->HasDetailedGlyphs()) {
     for (uint32_t i = 0; i < wordLen; ++i, ++aOffset) {
       const CompressedGlyph& g = wordGlyphs[i];
-      if (g.IsSimpleGlyph()) {
-        charGlyphs[aOffset] = g;
-      } else {
+      if (!g.IsSimpleGlyph()) {
         const DetailedGlyph* details =
             g.GetGlyphCount() > 0 ? aShapedWord->GetDetailedGlyphs(i) : nullptr;
-        SetGlyphs(aOffset, g, details);
+        SetDetailedGlyphs(aOffset, g.GetGlyphCount(), details);
       }
+      charGlyphs[aOffset] = g;
     }
   } else {
     memcpy(charGlyphs + aOffset, wordGlyphs, wordLen * sizeof(CompressedGlyph));
@@ -2844,8 +2843,7 @@ void gfxFontGroup::InitScriptRun(DrawTarget* aDrawTarget, gfxTextRun* aTextRun,
               gfxTextRun::DetailedGlyph detailedGlyph;
               detailedGlyph.mGlyphID = mainFont->GetSpaceGlyph();
               detailedGlyph.mAdvance = advance;
-              CompressedGlyph g = CompressedGlyph::MakeComplex(true, true, 1);
-              aTextRun->SetGlyphs(aOffset + index, g, &detailedGlyph);
+              aTextRun->SetDetailedGlyphs(aOffset + index, 1, &detailedGlyph);
             }
             continue;
           }

layout/generic/nsTextRunTransformations.cpp

@@ -130,6 +130,8 @@ void MergeCharactersInTextRun(gfxTextRun* aDest, gfxTextRun* aSrc,
   AutoTArray<gfxTextRun::DetailedGlyph, 2> glyphs;
   const gfxTextRun::CompressedGlyph continuationGlyph =
       gfxTextRun::CompressedGlyph::MakeComplex(false, false, 0);
+  const gfxTextRun::CompressedGlyph* srcGlyphs = aSrc->GetCharacterGlyphs();
+  gfxTextRun::CompressedGlyph* destGlyphs = aDest->GetCharacterGlyphs();
   while (iter.NextRun()) {
     const gfxTextRun::GlyphRun* run = iter.GetGlyphRun();
     aDest->AddGlyphRun(run->mFont, run->mMatchType, offset, false,
@@ -137,7 +139,6 @@ void MergeCharactersInTextRun(gfxTextRun* aDest, gfxTextRun* aSrc,
 
     bool anyMissing = false;
     uint32_t mergeRunStart = iter.GetStringStart();
-    const gfxTextRun::CompressedGlyph* srcGlyphs = aSrc->GetCharacterGlyphs();
     gfxTextRun::CompressedGlyph mergedGlyph = srcGlyphs[mergeRunStart];
     uint32_t stringEnd = iter.GetStringEnd();
     for (uint32_t k = iter.GetStringStart(); k < stringEnd; ++k) {
@@ -182,11 +183,11 @@ void MergeCharactersInTextRun(gfxTextRun* aDest, gfxTextRun* aSrc,
                                  mergedGlyph.IsLigatureGroupStart(),
                                  glyphs.Length());
         }
-        aDest->SetGlyphs(offset, mergedGlyph, glyphs.Elements());
-        ++offset;
+        aDest->SetDetailedGlyphs(offset, glyphs.Length(), glyphs.Elements());
+        destGlyphs[offset++] = mergedGlyph;
 
         while (offset < aDest->GetLength() && aDeletedChars[offset]) {
-          aDest->SetGlyphs(offset++, continuationGlyph, nullptr);
+          destGlyphs[offset++] = continuationGlyph;
         }
       }
 

layout/mathml/nsMathMLChar.cpp

@@ -528,9 +528,7 @@ already_AddRefed<gfxTextRun> nsOpenTypeTable::MakeTextRun(
   detailedGlyph.mAdvance = NSToCoordRound(
       aAppUnitsPerDevPixel * aFontGroup->GetFirstValidFont()->GetGlyphHAdvance(
                                  aDrawTarget, aGlyph.glyphID));
-  textRun->SetGlyphs(0,
-                     gfxShapedText::CompressedGlyph::MakeComplex(true, true, 1),
-                     &detailedGlyph);
+  textRun->SetDetailedGlyphs(0, 1, &detailedGlyph);
 
   return textRun.forget();
 }

layout/reftests/text-decoration/reftest.list

@@ -114,8 +114,7 @@ fuzzy-if(skiaContent,0-4,0-2) == underline-select-1.html underline-select-1-ref.
 fuzzy-if(Android,0-238,0-36) == vertical-mode-decorations-2.html vertical-mode-decorations-2-ref.html
 != 1415214.html 1415214-notref.html
 test-pref(layout.css.text-decoration-thickness.enabled,false) == text-decoration-shorthands-001.html text-decoration-shorthands-001-ref.html
-# fails because of bug 1572302
-test-pref(layout.css.text-decoration-skip-ink.enabled,true) test-pref(layout.css.text-underline-offset.enabled,true) fails HTTP(..) == skip-ink-multiline-position.html skip-ink-multiline-position-ref.html
+test-pref(layout.css.text-decoration-skip-ink.enabled,true) test-pref(layout.css.text-underline-offset.enabled,true) HTTP(..) == skip-ink-multiline-position.html skip-ink-multiline-position-ref.html
 test-pref(layout.css.text-decoration-skip-ink.enabled,true) == skip-ink-vertical-align.html skip-ink-vertical-align-ref.html
 test-pref(layout.css.text-decoration-skip-ink.enabled,true) != skip-ink-vertical-align-2.html skip-ink-vertical-align-2-notref.html
 pref(layout.css.text-decoration-skip-ink.enabled,true) fuzzy(0-94,0-4) == skip-ink-cjk-1.html skip-ink-cjk-1-ref.html

layout/reftests/text/1655364-1-ref.html

@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<div style="font:300px Consolas, monospace;width:3ch;text-align:right">ab<br>cd</div>

layout/reftests/text/1655364-1.html

@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<div style="font:300px Consolas, monospace;width:3ch;text-align:right">ab cd</div>

layout/reftests/text/reftest.list

@@ -192,6 +192,7 @@ fails-if(Android) == 1463020-letter-spacing-text-transform-2.html 1463020-letter
 == 1507661-spurious-hyphenation-after-explicit.html 1507661-spurious-hyphenation-after-explicit-ref.html
 fuzzy-if(!webrender,12-66,288-1660) fails-if(gtkWidget&&!webrender) == 1522857-1.html 1522857-1-ref.html # antialiasing fuzz in non-webrender cases
 != 1637405-pua-shaping-1.html 1637405-pua-shaping-1-notref.html
+fuzzy-if(!webrender,0-42,0-1553) fuzzy-if(gtkWidget&&!webrender,0-255,0-50) == 1655364-1.html 1655364-1-ref.html
 
 # ensure emoji chars don't render blank (bug 715798, bug 779042);
 # should at least render hexboxes if there's no font support