Bug 1403521 - Correct positioning of text-decoration lines in vertical writing modes. r=dholbert
authorJonathan Kew <jkew@mozilla.com>
Wed, 11 Oct 2017 14:01:53 +0100
changeset 385555 91bdf32e14f5d190b21ed75aa23cf53754399707
parent 385554 cab71dff90c4ab218a2bb008000f550222d419be
child 385556 2ec64f31c306f26ecc640520bd4a22fd0c9c70e1
push id32662
push userryanvm@gmail.com
push dateWed, 11 Oct 2017 21:53:47 +0000
treeherdermozilla-central@3d918ff5d634 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1403521
milestone58.0a1
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
Bug 1403521 - Correct positioning of text-decoration lines in vertical writing modes. r=dholbert
layout/generic/nsTextFrame.cpp
layout/generic/nsTextFrame.h
layout/painting/nsCSSRendering.cpp
layout/painting/nsCSSRendering.h
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -5669,16 +5669,17 @@ nsTextFrame::UnionAdditionalOverflow(nsP
     Float gfxWidth =
       (verticalRun ? aVisualOverflowRect->height
                    : aVisualOverflowRect->width) /
       appUnitsPerDevUnit;
     params.lineSize = Size(gfxWidth, underlineSize / appUnitsPerDevUnit);
     params.ascent = gfxFloat(mAscent) / appUnitsPerDevUnit;
     params.style = decorationStyle;
     params.vertical = verticalRun;
+    params.sidewaysLeft = mTextRun->IsSidewaysLeft();
 
     params.offset = underlineOffset / appUnitsPerDevUnit;
     params.decoration = NS_STYLE_TEXT_DECORATION_LINE_UNDERLINE;
     nsRect underlineRect =
       nsCSSRendering::GetTextDecorationRect(aPresContext, params);
     params.offset = maxAscent / appUnitsPerDevUnit;
     params.decoration = NS_STYLE_TEXT_DECORATION_LINE_OVERLINE;
     nsRect overlineRect =
@@ -5713,24 +5714,22 @@ nsTextFrame::UnionAdditionalOverflow(nsP
       gfxFloat gfxWidth = measure / appUnitsPerDevUnit;
       gfxFloat ascent = gfxFloat(GetLogicalBaseline(parentWM))
                           / appUnitsPerDevUnit;
       nscoord frameBStart = 0;
       if (parentWM.IsVerticalRL()) {
         frameBStart = GetSize().width;
         ascent = -ascent;
       }
-      // The decoration-line offsets need to be reversed for sideways-lr mode,
-      // so we will multiply the values from metrics by this factor.
-      gfxFloat decorationOffsetDir = mTextRun->IsSidewaysLeft() ? -1.0 : 1.0;
 
       nsCSSRendering::DecorationRectParams params;
       params.lineSize = Size(gfxWidth, 0);
       params.ascent = ascent;
       params.vertical = verticalDec;
+      params.sidewaysLeft = mTextRun->IsSidewaysLeft();
 
       nscoord topOrLeft(nscoord_MAX), bottomOrRight(nscoord_MIN);
       typedef gfxFont::Metrics Metrics;
       auto accumulateDecorationRect = [&](const LineDecoration& dec,
                                           gfxFloat Metrics::* lineSize,
                                           gfxFloat Metrics::* lineOffset) {
         params.style = dec.mStyle;
         // If the style is solid, let's include decoration line rect of solid
@@ -5742,17 +5741,17 @@ nsTextFrame::UnionAdditionalOverflow(nsP
 
         float inflation =
           GetInflationForTextDecorations(dec.mFrame, inflationMinFontSize);
         const Metrics metrics =
           GetFirstFontMetrics(GetFontGroupForFrame(dec.mFrame, inflation),
                               useVerticalMetrics);
 
         params.lineSize.height = metrics.*lineSize;
-        params.offset = decorationOffsetDir * metrics.*lineOffset;
+        params.offset = metrics.*lineOffset;
         const nsRect decorationRect =
           nsCSSRendering::GetTextDecorationRect(aPresContext, params) +
           (verticalDec ? nsPoint(frameBStart - dec.mBaselineOffset, 0)
                        : nsPoint(0, -dec.mBaselineOffset));
 
         if (verticalDec) {
           topOrLeft = std::min(decorationRect.x, topOrLeft);
           bottomOrRight = std::max(decorationRect.XMost(), bottomOrRight);
@@ -5926,31 +5925,31 @@ nsTextFrame::DrawSelectionDecorations(gf
                                       const TextRangeStyle &aRangeStyle,
                                       const Point& aPt,
                                       gfxFloat aICoordInFrame,
                                       gfxFloat aWidth,
                                       gfxFloat aAscent,
                                       const gfxFont::Metrics& aFontMetrics,
                                       DrawPathCallbacks* aCallbacks,
                                       bool aVertical,
-                                      gfxFloat aDecorationOffsetDir,
                                       uint8_t aDecoration)
 {
   PaintDecorationLineParams params;
   params.context = aContext;
   params.dirtyRect = aDirtyRect;
   params.pt = aPt;
   params.lineSize.width = aWidth;
   params.ascent = aAscent;
   params.offset = aDecoration == NS_STYLE_TEXT_DECORATION_LINE_UNDERLINE ?
                   aFontMetrics.underlineOffset : aFontMetrics.maxAscent;
   params.decoration = aDecoration;
   params.decorationType = DecorationType::Selection;
   params.callbacks = aCallbacks;
   params.vertical = aVertical;
+  params.sidewaysLeft = mTextRun->IsSidewaysLeft();
   params.descentLimit =
     ComputeDescentLimitForSelectionUnderline(aTextPaintStyle.PresContext(),
                                              aFontMetrics);
 
   float relativeSize;
 
   auto* textDrawer = aContext->GetTextDrawer();
 
@@ -6049,17 +6048,16 @@ nsTextFrame::DrawSelectionDecorations(gf
       params.offset = metrics.strikeoutOffset + 0.5;
       params.decoration = NS_STYLE_TEXT_DECORATION_LINE_LINE_THROUGH;
       break;
     }
     default:
       NS_WARNING("Requested selection decorations when there aren't any");
       return;
   }
-  params.offset *= aDecorationOffsetDir;
   params.lineSize.height *= relativeSize;
   params.icoordInFrame = (aVertical ? params.pt.y - aPt.y
                                     : params.pt.x - aPt.x) + aICoordInFrame;
   PaintDecorationLine(params);
 }
 
 /* static */
 bool
@@ -6613,17 +6611,16 @@ nsTextFrame::PaintTextSelectionDecoratio
   int32_t app = aParams.textPaintStyle->PresContext()->AppUnitsPerDevPixel();
   // XXX aTextBaselinePt is in AppUnits, shouldn't it be nsFloatPoint?
   Point pt;
   if (verticalRun) {
     pt.x = (aParams.textBaselinePt.x - mAscent) / app;
   } else {
     pt.y = (aParams.textBaselinePt.y - mAscent) / app;
   }
-  gfxFloat decorationOffsetDir = mTextRun->IsSidewaysLeft() ? -1.0 : 1.0;
   SelectionType nextSelectionType;
   TextRangeStyle selectedStyle;
 
   size_t selectionIndex = 0;
   auto* textDrawer = aParams.context->GetTextDrawer();
 
   while (iterator.GetNextSegment(&iOffset, &range, &hyphenWidth,
                                  &nextSelectionType, &selectedStyle)) {
@@ -6641,17 +6638,17 @@ nsTextFrame::PaintTextSelectionDecoratio
                (mTextRun->IsInlineReversed() ? advance : 0)) / app;
       }
       gfxFloat width = Abs(advance) / app;
       gfxFloat xInFrame = pt.x - (aParams.framePt.x / app);
       DrawSelectionDecorations(
         aParams.context, aParams.dirtyRect, aSelectionType,
         *aParams.textPaintStyle, selectedStyle, pt, xInFrame,
         width, mAscent / app, decorationMetrics, aParams.callbacks,
-        verticalRun, decorationOffsetDir, kDecoration);
+        verticalRun, kDecoration);
     }
     iterator.UpdateWithAdvance(advance);
     ++selectionIndex;
   }
 }
 
 bool
 nsTextFrame::PaintTextWithSelection(
@@ -7248,32 +7245,29 @@ nsTextFrame::DrawTextRunAndDecorations(R
     if (wm.IsVerticalRL()) {
       frameBStart += frameSize.width;
       ascent = -ascent;
     }
 
     nscoord inflationMinFontSize =
       nsLayoutUtils::InflationMinFontSizeFor(this);
 
-    // The decoration-line offsets need to be reversed for sideways-lr mode,
-    // so we will multiply the values from metrics by this factor.
-    gfxFloat decorationOffsetDir = mTextRun->IsSidewaysLeft() ? -1.0 : 1.0;
-
     PaintDecorationLineParams params;
     params.context = aParams.context;
     params.dirtyRect = aParams.dirtyRect;
     params.overrideColor = aParams.decorationOverrideColor;
     params.callbacks = aParams.callbacks;
     // pt is the physical point where the decoration is to be drawn,
     // relative to the frame; one of its coordinates will be updated below.
     params.pt = Point(x / app, y / app);
     Float& bCoord = verticalDec ? params.pt.x : params.pt.y;
     params.lineSize = Size(measure / app, 0);
     params.ascent = ascent;
     params.vertical = verticalDec;
+    params.sidewaysLeft = mTextRun->IsSidewaysLeft();
 
     // The matrix of the context may have been altered for text-combine-
     // upright. However, we want to draw decoration lines unscaled, thus
     // we need to revert the scaling here.
     gfxContextMatrixAutoSaveRestore scaledRestorer;
     if (StyleContext()->IsTextCombined()) {
       float scaleFactor = GetTextCombineScaleFactor(this);
       if (scaleFactor != 1.0f) {
@@ -7298,17 +7292,17 @@ nsTextFrame::DrawTextRunAndDecorations(R
       const Metrics metrics =
         GetFirstFontMetrics(GetFontGroupForFrame(dec.mFrame, inflation),
                             useVerticalMetrics);
 
       params.lineSize.height = metrics.*lineSize;
       bCoord = (frameBStart - dec.mBaselineOffset) / app;
 
       params.color = dec.mColor;
-      params.offset = decorationOffsetDir * metrics.*lineOffset;
+      params.offset = metrics.*lineOffset;
       params.style = dec.mStyle;
       PaintDecorationLine(params);
     };
 
     auto* textDrawer = aParams.context->GetTextDrawer();
 
     // Underlines
     if (textDrawer && aDecorations.mUnderlines.Length() > 0) {
@@ -7574,16 +7568,19 @@ nsTextFrame::CombineSelectionUnderlineRe
 
   nsCSSRendering::DecorationRectParams params;
   params.ascent = aPresContext->AppUnitsToGfxUnits(mAscent);
   params.offset = fontGroup->GetUnderlineOffset();
   params.descentLimit =
     ComputeDescentLimitForSelectionUnderline(aPresContext, metrics);
   params.vertical = verticalRun;
 
+  EnsureTextRun(nsTextFrame::eInflated);
+  params.sidewaysLeft = mTextRun ? mTextRun->IsSidewaysLeft() : false;
+
   UniquePtr<SelectionDetails> details = GetSelectionDetails();
   for (SelectionDetails* sd = details.get(); sd; sd = sd->mNext.get()) {
     if (sd->mStart == sd->mEnd ||
         sd->mSelectionType == SelectionType::eInvalid ||
         !(ToSelectionTypeMask(sd->mSelectionType) &
             kSelectionTypesWithDecorations) ||
         // URL strikeout does not use underline.
         sd->mSelectionType == SelectionType::eURLStrikeout) {
--- a/layout/generic/nsTextFrame.h
+++ b/layout/generic/nsTextFrame.h
@@ -828,17 +828,16 @@ protected:
                                 const TextRangeStyle& aRangeStyle,
                                 const Point& aPt,
                                 gfxFloat aICoordInFrame,
                                 gfxFloat aWidth,
                                 gfxFloat aAscent,
                                 const gfxFont::Metrics& aFontMetrics,
                                 DrawPathCallbacks* aCallbacks,
                                 bool aVertical,
-                                gfxFloat aDecorationOffsetDir,
                                 uint8_t aDecoration);
 
   struct PaintDecorationLineParams;
   void PaintDecorationLine(const PaintDecorationLineParams& aParams);
   /**
    * ComputeDescentLimitForSelectionUnderline() computes the most far position
    * where we can put selection underline.
    *
--- a/layout/painting/nsCSSRendering.cpp
+++ b/layout/painting/nsCSSRendering.cpp
@@ -4017,17 +4017,17 @@ nsCSSRendering::PaintDecorationLine(nsIF
           NS_STYLE_TEXT_DECORATION_STYLE_WAVY);
         return;
       }
 
       Point pt(rect.TopLeft());
       Float& ptICoord = aParams.vertical ? pt.y : pt.x;
       Float& ptBCoord = aParams.vertical ? pt.x : pt.y;
       if (aParams.vertical) {
-        ptBCoord += adv + lineThickness / 2.0;
+        ptBCoord += adv;
       }
       Float iCoordLimit = ptICoord + rectISize + lineThickness;
 
       const Float dirtyRectIMost = aParams.vertical ?
         aParams.dirtyRect.YMost() : aParams.dirtyRect.XMost();
       skipCycles = floor((iCoordLimit - dirtyRectIMost) / cycleLength);
       if (skipCycles > 0) {
         iCoordLimit -= skipCycles * cycleLength;
@@ -4217,50 +4217,89 @@ nsCSSRendering::GetTextDecorationRectInt
         // 2px wavy line can be used for different meaning in IME selections
         // at same time.
         r.height = std::max(suggestedMaxRectHeight, lineThickness * 2.0);
       }
     }
   }
 
   gfxFloat baseline = floor(bCoord + aParams.ascent + 0.5);
+
+  // Calculate adjusted offset based on writing-mode/orientation and thickness
+  // of decoration line. The input value aParams.offset is the nominal position
+  // (offset from baseline) where we would draw a single, infinitely-thin line;
+  // but for a wavy or double line, we'll need to move the bounding rect of the
+  // decoration outwards from the baseline so that an underline remains below
+  // the glyphs, and an overline above them, despite the increased block-dir
+  // extent of the decoration.
+  //
+  // So adjustments by r.Height() are used to make the wider line styles (wavy
+  // and double) "grow" in the appropriate direction compared to the basic
+  // single line.
+  //
+  // Note that at this point, the decoration rect is being calculated in line-
+  // relative coordinates, where 'x' is line-rightwards, and 'y' is line-
+  // upwards. We'll swap them to be physical coords at the end.
   gfxFloat offset = 0.0;
+
   switch (aParams.decoration) {
     case NS_STYLE_TEXT_DECORATION_LINE_UNDERLINE:
       offset = aParams.offset;
       if (canLiftUnderline) {
         if (descentLimit < -offset + r.Height()) {
           // If we can ignore the offset and the decoration line is overflowing,
           // we should align the bottom edge of the decoration line rect if it's
           // possible.  Otherwise, we should lift up the top edge of the rect as
           // far as possible.
           gfxFloat offsetBottomAligned = -descentLimit + r.Height();
           gfxFloat offsetTopAligned = 0.0;
           offset = std::min(offsetBottomAligned, offsetTopAligned);
         }
       }
       break;
+
     case NS_STYLE_TEXT_DECORATION_LINE_OVERLINE:
+      // For overline, we adjust the offset by lineThickness (the thickness of
+      // a single decoration line) because empirically it looks better to draw
+      // the overline just inside rather than outside the font's ascent, which
+      // is what nsTextFrame passes as aParams.offset (as fonts don't provide
+      // an explicit overline-offset).
       offset = aParams.offset - lineThickness + r.Height();
       break;
+
     case NS_STYLE_TEXT_DECORATION_LINE_LINE_THROUGH: {
+      // To maintain a consistent mid-point for line-through decorations,
+      // we adjust the offset by half of the decoration rect's height.
       gfxFloat extra = floor(r.Height() / 2.0 + 0.5);
       extra = std::max(extra, lineThickness);
       offset = aParams.offset - lineThickness + extra;
       break;
     }
+
     default:
       NS_ERROR("Invalid decoration value!");
   }
 
+  // Convert line-relative coordinate system (x = line-right, y = line-up)
+  // to physical coords, and move the decoration rect to the calculated
+  // offset from baseline.
   if (aParams.vertical) {
-    r.y = baseline + floor(offset + 0.5);
     Swap(r.x, r.y);
     Swap(r.width, r.height);
+    // line-upwards in vertical mode = physical-right, so we /add/ offset
+    // to baseline. Except in sideways-lr mode, where line-upwards will be
+    // physical leftwards.
+    if (aParams.sidewaysLeft) {
+      r.x = baseline - floor(offset + 0.5);
+    } else {
+      r.x = baseline + floor(offset - r.Width() + 0.5);
+    }
   } else {
+    // line-upwards in horizontal mode = physical-up, but our physical coord
+    // system works downwards, so we /subtract/ offset from baseline.
     r.y = baseline - floor(offset + 0.5);
   }
 
   return r;
 }
 
 #define MAX_BLUR_RADIUS 300
 #define MAX_SPREAD_RADIUS 50
--- a/layout/painting/nsCSSRendering.h
+++ b/layout/painting/nsCSSRendering.h
@@ -578,16 +578,17 @@ struct nsCSSRendering {
     // NS_STYLE_TEXT_DECORATION_LINE_UNDERLINE or
     // NS_STYLE_TEXT_DECORATION_LINE_OVERLINE or
     // NS_STYLE_TEXT_DECORATION_LINE_LINE_THROUGH.
     uint8_t decoration = NS_STYLE_TEXT_DECORATION_LINE_UNDERLINE;
     // The style of the decoration line such as
     // NS_STYLE_TEXT_DECORATION_STYLE_*.
     uint8_t style = NS_STYLE_TEXT_DECORATION_STYLE_NONE;
     bool vertical = false;
+    bool sidewaysLeft = false;
   };
 
   struct PaintDecorationLineParams : DecorationRectParams
   {
     // No need to paint outside this rect.
     Rect dirtyRect;
     // The top/left edge of the text.
     Point pt;