Backed out changeset 4905048de8d1 (bug 1412355) for frequently failing reftest layout/reftests/writing-mode/1248248-1-orientation-break-glyphrun.html on Linux x64 debug and asan. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sat, 28 Oct 2017 16:39:59 +0200
changeset 442120 c0eb1f08953b31362483a415465d2964a67a5f0c
parent 442119 339e44e41f22670bd665d935ef56f2262f9360db
child 442121 ed73b55a33120d27efc862eb87d332e167c026f4
child 442130 1874e015f0ac68500ecc7dea1644a9ddd40ec5fb
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1412355, 1248248
milestone58.0a1
backs out4905048de8d11e8bec261886d68be9c3f99d312b
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
Backed out changeset 4905048de8d1 (bug 1412355) for frequently failing reftest layout/reftests/writing-mode/1248248-1-orientation-break-glyphrun.html on Linux x64 debug and asan. r=backout
gfx/thebes/gfxCoreTextShaper.cpp
gfx/thebes/gfxFT2Fonts.cpp
gfx/thebes/gfxFont.cpp
gfx/thebes/gfxFont.h
gfx/thebes/gfxGraphiteShaper.cpp
gfx/thebes/gfxHarfBuzzShaper.cpp
gfx/thebes/gfxTextRun.cpp
layout/generic/nsTextRunTransformations.cpp
layout/mathml/nsMathMLChar.cpp
--- a/gfx/thebes/gfxCoreTextShaper.cpp
+++ b/gfx/thebes/gfxCoreTextShaper.cpp
@@ -522,17 +522,18 @@ gfxCoreTextShaper::SetGlyphsFromRun(gfxS
                                                      glyphs[glyphStart]);
         } else {
             // collect all glyphs in a list to be assigned to the first char;
             // there must be at least one in the clump, and we already measured its advance,
             // hence the placement of the loop-exit test and the measurement of the next glyph
             while (true) {
                 gfxTextRun::DetailedGlyph *details = detailedGlyphs.AppendElement();
                 details->mGlyphID = glyphs[glyphStart];
-                details->mOffset.y = -positions[glyphStart].y * appUnitsPerDevUnit;
+                details->mXOffset = 0;
+                details->mYOffset = -positions[glyphStart].y * appUnitsPerDevUnit;
                 details->mAdvance = advance;
                 if (++glyphStart >= glyphEnd) {
                    break;
                 }
                 if (glyphStart < numGlyphs-1) {
                     toNextGlyph = positions[glyphStart+1].x - positions[glyphStart].x;
                 } else {
                     toNextGlyph = positions[0].x + runWidth - positions[glyphStart].x;
--- a/gfx/thebes/gfxFT2Fonts.cpp
+++ b/gfx/thebes/gfxFT2Fonts.cpp
@@ -146,16 +146,18 @@ gfxFT2Font::AddRange(const char16_t *aTe
             // gid = 0 only happens when the glyph is missing from the font
             aShapedText->SetMissingGlyph(aOffset, ch, this);
         } else {
             gfxTextRun::DetailedGlyph details;
             details.mGlyphID = gid;
             NS_ASSERTION(details.mGlyphID == gid,
                          "Seriously weird glyph ID detected!");
             details.mAdvance = advance;
+            details.mXOffset = 0;
+            details.mYOffset = 0;
             gfxShapedText::CompressedGlyph g;
             g.SetComplex(charGlyphs[aOffset].IsClusterStart(), true, 1);
             aShapedText->SetGlyphs(aOffset, g, &details);
         }
     }
 }
 
 gfxFT2Font::gfxFT2Font(const RefPtr<UnscaledFontFreeType>& aUnscaledFont,
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -680,16 +680,18 @@ gfxShapedText::SetMissingGlyph(uint32_t 
         details->mAdvance = 0;
     } else {
         gfxFloat width =
             std::max(aFont->GetMetrics(gfxFont::eHorizontal).aveCharWidth,
                      gfxFloat(gfxFontMissingGlyphs::GetDesiredMinWidth(aChar,
                                 mAppUnitsPerDevUnit)));
         details->mAdvance = uint32_t(width * mAppUnitsPerDevUnit);
     }
+    details->mXOffset = 0;
+    details->mYOffset = 0;
     GetCharacterGlyphs()[aIndex].SetMissing(1);
 }
 
 bool
 gfxShapedText::FilterIfIgnorable(uint32_t aIndex, uint32_t aCh)
 {
     if (IsIgnorable(aCh)) {
         // There are a few default-ignorables of Letter category (currently,
@@ -700,16 +702,18 @@ gfxShapedText::FilterIfIgnorable(uint32_
         if (GetGenCategory(aCh) == nsUGenCategory::kLetter &&
             aIndex + 1 < GetLength() &&
             !GetCharacterGlyphs()[aIndex + 1].IsClusterStart()) {
             return false;
         }
         DetailedGlyph *details = AllocateDetailedGlyphs(aIndex, 1);
         details->mGlyphID = aCh;
         details->mAdvance = 0;
+        details->mXOffset = 0;
+        details->mYOffset = 0;
         GetCharacterGlyphs()[aIndex].SetMissing(1);
         return true;
     }
     return false;
 }
 
 void
 gfxShapedText::AdjustAdvancesForSyntheticBold(float aSynBoldOffset,
@@ -724,17 +728,17 @@ gfxShapedText::AdjustAdvancesForSyntheti
              // simple glyphs ==> just add the advance
              int32_t advance = glyphData->GetSimpleAdvance() + synAppUnitOffset;
              if (CompressedGlyph::IsSimpleAdvance(advance)) {
                  glyphData->SetSimpleGlyph(advance, glyphData->GetSimpleGlyph());
              } else {
                  // rare case, tested by making this the default
                  uint32_t glyphIndex = glyphData->GetSimpleGlyph();
                  glyphData->SetComplex(true, true, 1);
-                 DetailedGlyph detail = { glyphIndex, advance, gfx::Point() };
+                 DetailedGlyph detail = {glyphIndex, advance, 0, 0};
                  SetGlyphs(i, *glyphData, &detail);
              }
          } else {
              // complex glyphs ==> add offset at cluster/ligature boundaries
              uint32_t detailedLength = glyphData->GetGlyphCount();
              if (detailedLength) {
                  DetailedGlyph *details = GetDetailedGlyphs(i);
                  if (!details) {
@@ -1932,17 +1936,24 @@ gfxFont::DrawGlyphs(const gfxShapedText*
                     }
                     if (glyphData->IsMissing()) {
                         if (!DrawMissingGlyph(aBuffer.mRunParams,
                                               aBuffer.mFontParams,
                                               details, *aPt)) {
                             return false;
                         }
                     } else {
-                        gfx::Point glyphPt(*aPt + details->mOffset);
+                        gfx::Point glyphPt(*aPt);
+                        if (aBuffer.mFontParams.isVerticalFont) {
+                            glyphPt.x += details->mYOffset;
+                            glyphPt.y += details->mXOffset;
+                        } else {
+                            glyphPt.x += details->mXOffset;
+                            glyphPt.y += details->mYOffset;
+                        }
                         DrawOneGlyph<FC>(details->mGlyphID, glyphPt, aBuffer,
                                          &emittedGlyphs);
                     }
                     if (!aBuffer.mRunParams.isRTL) {
                         inlineCoord += advance;
                     }
                 }
             }
@@ -2568,28 +2579,31 @@ gfxFont::Measure(const gfxTextRun *aText
             if (glyphCount > 0) {
                 const gfxTextRun::DetailedGlyph *details =
                     aTextRun->GetDetailedGlyphs(i);
                 NS_ASSERTION(details != nullptr,
                              "detailedGlyph record should not be missing!");
                 uint32_t j;
                 for (j = 0; j < glyphCount; ++j, ++details) {
                     uint32_t glyphIndex = details->mGlyphID;
-                    gfxPoint glyphPt(x + details->mOffset.x,
-                                     details->mOffset.y);
+                    gfxPoint glyphPt(x + details->mXOffset, details->mYOffset);
                     double advance = details->mAdvance;
                     gfxRect glyphRect;
                     if (glyphData->IsMissing() || !extents ||
                         !extents->GetTightGlyphExtentsAppUnits(this,
                                 aRefDrawTarget, 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 (orientation == eVertical) {
+                        Swap(glyphRect.x, glyphRect.y);
+                        Swap(glyphRect.width, glyphRect.height);
+                    }
                     if (isRTL) {
                         glyphRect -= gfxPoint(advance, 0);
                     }
                     glyphRect += glyphPt;
                     metrics.mBoundingBox = metrics.mBoundingBox.Union(glyphRect);
                     x += direction*advance;
                 }
             }
--- a/gfx/thebes/gfxFont.h
+++ b/gfx/thebes/gfxFont.h
@@ -952,27 +952,25 @@ public:
     virtual const CompressedGlyph *GetCharacterGlyphs() const = 0;
     virtual CompressedGlyph *GetCharacterGlyphs() = 0;
 
     /**
      * When the glyphs for a character don't fit into a CompressedGlyph record
      * in SimpleGlyph format, we use an array of DetailedGlyphs instead.
      */
     struct DetailedGlyph {
-        // The glyphID, or the Unicode character if this is a missing glyph
+        /** The glyphID, or the Unicode character
+         * if this is a missing glyph */
         uint32_t mGlyphID;
-        // The advance of the glyph, in appunits.
-        // mAdvance is in the text direction (RTL or LTR),
-        // and will normally be non-negative (although this is not guaranteed)
+        /** The advance, x-offset and y-offset of the glyph, in appunits
+         *  mAdvance is in the text direction (RTL or LTR)
+         *  mXOffset is always from left to right
+         *  mYOffset is always from top to bottom */   
         int32_t  mAdvance;
-        // The offset from the glyph's default position, in line-relative
-        // coordinates (so mOffset.x is an offset in the line-right direction,
-        // and mOffset.y is an offset in line-downwards direction).
-        // These values are in floating-point appUnits.
-        mozilla::gfx::Point mOffset;
+        float    mXOffset, mYOffset;
     };
 
     void SetGlyphs(uint32_t aCharIndex, CompressedGlyph aGlyph,
                    const DetailedGlyph *aGlyphs);
 
     void SetMissingGlyph(uint32_t aIndex, uint32_t aChar, gfxFont *aFont);
 
     void SetIsSpace(uint32_t aIndex) {
@@ -1080,17 +1078,17 @@ protected:
     // it to record specific characters that layout may need to detect.
     void EnsureComplexGlyph(uint32_t aIndex, CompressedGlyph& aGlyph)
     {
         MOZ_ASSERT(GetCharacterGlyphs() + aIndex == &aGlyph);
         if (aGlyph.IsSimpleGlyph()) {
             DetailedGlyph details = {
                 aGlyph.GetSimpleGlyph(),
                 (int32_t) aGlyph.GetSimpleAdvance(),
-                mozilla::gfx::Point()
+                0, 0
             };
             SetGlyphs(aIndex, CompressedGlyph().SetComplex(true, true, 1),
                       &details);
         }
     }
 
     // For characters whose glyph data does not fit the "simple" glyph criteria
     // in CompressedGlyph, we use a sorted array to store the association
--- a/gfx/thebes/gfxGraphiteShaper.cpp
+++ b/gfx/thebes/gfxGraphiteShaper.cpp
@@ -334,26 +334,27 @@ gfxGraphiteShaper::SetGlyphsFromSegment(
             charGlyphs[offs].SetSimpleGlyph(appAdvance, gids[c.baseGlyph]);
         } else {
             // not a one-to-one mapping with simple metrics: use DetailedGlyph
             AutoTArray<gfxShapedText::DetailedGlyph,8> details;
             float clusterLoc;
             for (uint32_t j = c.baseGlyph; j < c.baseGlyph + c.nGlyphs; ++j) {
                 gfxShapedText::DetailedGlyph* d = details.AppendElement();
                 d->mGlyphID = gids[j];
-                d->mOffset.y = roundY ? NSToIntRound(-yLocs[j]) * dev2appUnits
-                                      : -yLocs[j] * dev2appUnits;
+                d->mYOffset = roundY ? NSToIntRound(-yLocs[j]) * dev2appUnits
+                                     : -yLocs[j] * dev2appUnits;
                 if (j == c.baseGlyph) {
+                    d->mXOffset = 0;
                     d->mAdvance = appAdvance;
                     clusterLoc = xLocs[j];
                 } else {
                     float dx = rtl ? (xLocs[j] - clusterLoc) :
                                      (xLocs[j] - clusterLoc - adv);
-                    d->mOffset.x = roundX ? NSToIntRound(dx) * dev2appUnits
-                                          : dx * dev2appUnits;
+                    d->mXOffset = roundX ? NSToIntRound(dx) * dev2appUnits
+                                         : dx * dev2appUnits;
                     d->mAdvance = 0;
                 }
             }
             gfxShapedText::CompressedGlyph g;
             g.SetComplex(charGlyphs[offs].IsClusterStart(),
                          true, details.Length());
             aShapedText->SetGlyphs(aOffset + offs, g, details.Elements());
         }
--- a/gfx/thebes/gfxHarfBuzzShaper.cpp
+++ b/gfx/thebes/gfxHarfBuzzShaper.cpp
@@ -1731,29 +1731,22 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(gfxS
             // there must be at least one in the clump, and we already measured
             // its advance, hence the placement of the loop-exit test and the
             // measurement of the next glyph.
             while (1) {
                 gfxTextRun::DetailedGlyph* details =
                     detailedGlyphs.AppendElement();
                 details->mGlyphID = ginfo[glyphStart].codepoint;
 
+                details->mXOffset = iOffset;
                 details->mAdvance = advance;
 
-                if (aVertical) {
-                    details->mOffset.x = bPos -
-                        (roundB ? appUnitsPerDevUnit * FixedToIntRound(b_offset)
-                         : floor(hb2appUnits * b_offset + 0.5));
-                    details->mOffset.y = iOffset;
-                } else {
-                    details->mOffset.x = iOffset;
-                    details->mOffset.y = bPos -
-                        (roundB ? appUnitsPerDevUnit * FixedToIntRound(b_offset)
-                         : floor(hb2appUnits * b_offset + 0.5));
-                }
+                details->mYOffset = bPos -
+                    (roundB ? appUnitsPerDevUnit * FixedToIntRound(b_offset)
+                     : floor(hb2appUnits * b_offset + 0.5));
 
                 if (b_advance != 0) {
                     bPos -=
                         roundB ? appUnitsPerDevUnit * FixedToIntRound(b_advance)
                         : floor(hb2appUnits * b_advance + 0.5);
                 }
                 if (++glyphStart >= glyphEnd) {
                     break;
--- a/gfx/thebes/gfxTextRun.cpp
+++ b/gfx/thebes/gfxTextRun.cpp
@@ -2753,16 +2753,17 @@ gfxFontGroup::InitScriptRun(DrawTarget* 
                         if (gfxShapedText::CompressedGlyph::IsSimpleAdvance(advance)) {
                             aTextRun->GetCharacterGlyphs()[aOffset + index].
                                 SetSimpleGlyph(advance,
                                                mainFont->GetSpaceGlyph());
                         } else {
                             gfxTextRun::DetailedGlyph detailedGlyph;
                             detailedGlyph.mGlyphID = mainFont->GetSpaceGlyph();
                             detailedGlyph.mAdvance = advance;
+                            detailedGlyph.mXOffset = detailedGlyph.mYOffset = 0;
                             gfxShapedText::CompressedGlyph g;
                             g.SetComplex(true, true, 1);
                             aTextRun->SetGlyphs(aOffset + index,
                                                 g, &detailedGlyph);
                         }
                         continue;
                     }
                 }
--- a/layout/generic/nsTextRunTransformations.cpp
+++ b/layout/generic/nsTextRunTransformations.cpp
@@ -156,16 +156,18 @@ MergeCharactersInTextRun(gfxTextRun* aDe
     uint32_t stringEnd = iter.GetStringEnd();
     for (uint32_t k = iter.GetStringStart(); k < stringEnd; ++k) {
       const gfxTextRun::CompressedGlyph g = srcGlyphs[k];
       if (g.IsSimpleGlyph()) {
         if (!anyMissing) {
           gfxTextRun::DetailedGlyph details;
           details.mGlyphID = g.GetSimpleGlyph();
           details.mAdvance = g.GetSimpleAdvance();
+          details.mXOffset = 0;
+          details.mYOffset = 0;
           glyphs.AppendElement(details);
         }
       } else {
         if (g.IsMissing()) {
           anyMissing = true;
           glyphs.Clear();
         }
         if (g.GetGlyphCount() > 0) {
--- a/layout/mathml/nsMathMLChar.cpp
+++ b/layout/mathml/nsMathMLChar.cpp
@@ -583,16 +583,17 @@ nsOpenTypeTable::MakeTextRun(DrawTarget*
                               // We don't care about CSS writing mode here;
                               // math runs are assumed to be horizontal.
   gfxTextRun::DetailedGlyph detailedGlyph;
   detailedGlyph.mGlyphID = aGlyph.glyphID;
   detailedGlyph.mAdvance =
     NSToCoordRound(aAppUnitsPerDevPixel *
                    aFontGroup->GetFirstValidFont()->
                    GetGlyphHAdvance(aDrawTarget, aGlyph.glyphID));
+  detailedGlyph.mXOffset = detailedGlyph.mYOffset = 0;
   gfxShapedText::CompressedGlyph g;
   g.SetComplex(true, true, 1);
   textRun->SetGlyphs(0, g, &detailedGlyph);
 
   return textRun.forget();
 }
 
 // -----------------------------------------------------------------------------