Fix 'text-underline-position: under' to use em height ascent/descent

third_party/WebKit/Source/core/paint/InlineTextBoxPainter.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/paint/InlineTextBoxPainter.h"
 
 #include "core/editing/CompositionUnderline.h"
 #include "core/editing/Editor.h"
 #include "core/editing/markers/DocumentMarkerController.h"
 #include "core/frame/LocalFrame.h"
@@ skipping 25 matching lines @@
   const unsigned end_offset =
       std::min(marker.EndOffset() - text_box.Start(), text_box.Len());
   return std::make_pair(start_offset, end_offset);
 }
 }
 
 enum class ResolvedUnderlinePosition { kRoman, kUnder, kOver };
 
 static ResolvedUnderlinePosition ResolveUnderlinePosition(
     const ComputedStyle& style,
-    const InlineTextBox* inline_text_box) {
+    FontBaseline baseline_type) {
   // |auto| should resolve to |under| to avoid drawing through glyphs in
   // scripts where it would not be appropriate (e.g., ideographs.)
   // However, this has performance implications. For now, we only work with
   // vertical text.
-  switch (inline_text_box->Root().BaselineType()) {
-    default:
-      NOTREACHED();
-    // Fall though.
+  switch (baseline_type) {
     case kAlphabeticBaseline:
       switch (style.GetTextUnderlinePosition()) {
-        default:
-          NOTREACHED();
-        // Fall though.
         case kTextUnderlinePositionAuto:
           return ResolvedUnderlinePosition::kRoman;
         case kTextUnderlinePositionUnder:
           return ResolvedUnderlinePosition::kUnder;
       }
       break;
     case kIdeographicBaseline:
       // Compute language-appropriate default underline position.
       // https://drafts.csswg.org/css-text-decor-3/#default-stylesheet
       UScriptCode script = style.GetFontDescription().GetScript();
       if (script == USCRIPT_KATAKANA_OR_HIRAGANA || script == USCRIPT_HANGUL)
         return ResolvedUnderlinePosition::kOver;
       return ResolvedUnderlinePosition::kUnder;
   }
+  NOTREACHED();
+  return ResolvedUnderlinePosition::kRoman;
 }
 
 static LineLayoutItem EnclosingUnderlineObject(
     const InlineTextBox* inline_text_box) {
   bool first_line = inline_text_box->IsFirstLineStyle();
   for (LineLayoutItem current = inline_text_box->Parent()->GetLineLayoutItem();
        ;) {
     if (current.IsLayoutBlock())
       return current;
     if (!current.IsLayoutInline() || current.IsRubyText())
       return nullptr;
 
     const ComputedStyle& style_to_use = current.StyleRef(first_line);
     if (style_to_use.GetTextDecoration() & kTextDecorationUnderline)
       return current;
 
     current = current.Parent();
     if (!current)
       return current;
 
     if (Node* node = current.GetNode()) {
       if (isHTMLAnchorElement(node) || node->HasTagName(HTMLNames::fontTag))
         return current;
     }
   }
 }
 
-static int ComputeUnderlineOffsetForUnder(const ComputedStyle& style,
-                                          const InlineTextBox* inline_text_box,
-                                          bool is_overline = false) {
+static int ComputeUnderlineOffsetForUnder(
+    const ComputedStyle& style,
+    const InlineTextBox* inline_text_box,
+    LineLayoutItem decorating_box,
+    float text_decoration_thickness,
+    LineVerticalPositionType position_type) {
   const RootInlineBox& root = inline_text_box->Root();
-  LineLayoutItem decoration_object = EnclosingUnderlineObject(inline_text_box);
-  LayoutUnit offset;
-  if (style.IsFlippedLinesWritingMode()) {
-    LayoutUnit position = inline_text_box->LogicalTop();
-    offset =
-        position - root.MinLogicalTopForUnderline(decoration_object, position);
-  } else {
-    LayoutUnit position = inline_text_box->LogicalBottom();
-    offset = root.MaxLogicalBottomForUnderline(decoration_object, position) -
-             position;
-  }
-  if (is_overline)
-    return std::min(-offset, LayoutUnit()).ToInt();
-  return (inline_text_box->LogicalHeight() + std::max(offset, LayoutUnit()))
-      .ToInt();
-}
+  FontBaseline baseline_type = root.BaselineType();
+  LayoutUnit offset = inline_text_box->OffsetTo(position_type, baseline_type);
 
-static int ComputeOverlineOffset(const ComputedStyle& style,
-                                 const InlineTextBox* inline_text_box) {
-  return ComputeUnderlineOffsetForUnder(style, inline_text_box, true);
+  // Compute offset to the farthest position of the decorating box.
+  LayoutUnit logical_top = inline_text_box->LogicalTop();
+  LayoutUnit position = logical_top + offset;
+  LayoutUnit farthest = root.FarthestPositionForUnderline(
+      decorating_box, position_type, baseline_type, position);
+  int offset_int = (farthest - logical_top).ToInt();

[eae, 2017/05/01 16:21:48]

Is flooring the desired behavior here? If so please use .Floor() instead.

What happens if offset_int is negative?

[kojii, 2017/05/02 22:56:52]

Tested Round() and Floor() but Floor() looks better results. Probably in small
size, Floor() matches to font rendering engine.

Added comments.

+
+  // Gaps are not needed for TextTop because it generally has internal
+  // leadings.
+  if (position_type == LineVerticalPositionType::TextTop)
+    return offset_int;
+  return !IsOverSide(position_type) ? offset_int + 1 : offset_int - 1;
 }
 
 static int ComputeUnderlineOffsetForRoman(
     const FontMetrics& font_metrics,
     const float text_decoration_thickness) {
   // Compute the gap between the font and the underline. Use at least one
   // pixel gap, if underline is thick then use a bigger gap.
   int gap = 0;
 
   // Underline position of zero means draw underline on Baseline Position,
   // in Blink we need at least 1-pixel gap to adding following check.
   // Positive underline Position means underline should be drawn above baseline
   // and negative value means drawing below baseline, negating the value as in
   // Blink downward Y-increases.
 
   if (font_metrics.UnderlinePosition())
     gap = -font_metrics.UnderlinePosition();
   else
     gap = std::max<int>(1, ceilf(text_decoration_thickness / 2.f));
 
   // Position underline near the alphabetic baseline.
   return font_metrics.Ascent() + gap;
 }
 
 static int ComputeUnderlineOffset(ResolvedUnderlinePosition underline_position,
                                   const ComputedStyle& style,
                                   const FontMetrics& font_metrics,
                                   const InlineTextBox* inline_text_box,
+                                  LineLayoutItem decorating_box,
                                   const float text_decoration_thickness) {
   switch (underline_position) {
     default:
       NOTREACHED();
     // Fall through.
     case ResolvedUnderlinePosition::kRoman:
       return ComputeUnderlineOffsetForRoman(font_metrics,
                                             text_decoration_thickness);
     case ResolvedUnderlinePosition::kUnder:
       // Position underline at the under edge of the lowest element's
       // content box.
-      return ComputeUnderlineOffsetForUnder(style, inline_text_box);
+      return ComputeUnderlineOffsetForUnder(
+          style, inline_text_box, decorating_box, text_decoration_thickness,
+          LineVerticalPositionType::BottomOfEmHeight);
   }
 }
 
 static bool ShouldSetDecorationAntialias(
     const Vector<AppliedTextDecoration>& decorations) {
   for (const AppliedTextDecoration& decoration : decorations) {
     TextDecorationStyle decoration_style = decoration.Style();
     if (decoration_style == kTextDecorationStyleDotted ||
         decoration_style == kTextDecorationStyleDashed)
       return true;
@@ skipping 51 matching lines @@
   STACK_ALLOCATED();
 
   LayoutUnit width;
   FloatPoint local_origin;
   bool antialias;
   float baseline;
   const ComputedStyle* style;
   const SimpleFontData* font_data;
   float thickness;
   float double_offset;
+  FontBaseline baseline_type;
+  ResolvedUnderlinePosition underline_position;
+  // Decorating box: https://drafts.csswg.org/css-text-decor-3/#decorating-box
+  LineLayoutItem decorating_box;
 };
 
 class AppliedDecorationPainter final {
   STACK_ALLOCATED();
 
  public:
   AppliedDecorationPainter(GraphicsContext& context,
                            const DecorationInfo& decoration_info,
                            float start_point_y_offset,
                            const AppliedTextDecoration& decoration,
@@ skipping 256 matching lines @@
 
 static void RestoreContextFromDecoration(GraphicsContext& context,
                                          const LayoutTextCombine* combined_text,
                                          const LayoutRect& box_rect) {
   if (combined_text) {
     context.ConcatCTM(
         TextPainter::Rotation(box_rect, TextPainter::kCounterclockwise));
   }
 }
 
+static float ComputeDecorationThickness(const ComputedStyle* style,
+                                        const SimpleFontData* font_data) {
+  // Set the thick of the line to be 10% (or something else ?)of the computed
+  // font size and not less than 1px.  Using computedFontSize should take care
+  // of zoom as well.
+
+  // Update Underline thickness, in case we have Faulty Font Metrics calculating
+  // underline thickness by old method.
+  float text_decoration_thickness = 0.0;
+  int font_height_int = 0;
+  if (font_data) {
+    text_decoration_thickness =
+        font_data->GetFontMetrics().UnderlineThickness();
+    font_height_int = (int)(font_data->GetFontMetrics().FloatHeight() + 0.5);

[eae, 2017/05/01 16:21:48]

Please add a comment explaining this logic (especially why as to why we're doing
it this way rather than using round) and try to avoid c-style casts.

[kojii, 2017/05/02 22:56:52]

Done, didn't pay much attention when I moved this code...Height() should be ok,
changed.

+  }
+  if ((text_decoration_thickness == 0.f) ||
+      (text_decoration_thickness >= (font_height_int >> 1))) {
+    text_decoration_thickness = std::max(1.f, style->ComputedFontSize() / 10.f);
+  }
+  return text_decoration_thickness;
+}
+
 static void ComputeDecorationInfo(
     DecorationInfo& decoration_info,
     const InlineTextBox& box,
     const LayoutPoint& box_origin,
     const Vector<AppliedTextDecoration>& decorations) {
   LayoutPoint local_origin = LayoutPoint(box_origin);
   LayoutUnit width = box.LogicalWidth();
   if (box.Truncation() != kCNoTruncation) {
     bool ltr = box.IsLeftToRightDirection();
     bool flow_is_ltr =
         box.GetLineLayoutItem().Style()->IsLeftToRightDirection();
     width = LayoutUnit(box.GetLineLayoutItem().Width(
         ltr == flow_is_ltr ? box.Start() : box.Start() + box.Truncation(),
         ltr == flow_is_ltr ? box.Truncation() : box.Len() - box.Truncation(),
         box.TextPos(), flow_is_ltr ? TextDirection::kLtr : TextDirection::kRtl,
         box.IsFirstLineStyle()));
     if (!flow_is_ltr) {
       local_origin.Move(box.LogicalWidth() - width, LayoutUnit());
     }
   }
   decoration_info.width = width;
   decoration_info.local_origin = FloatPoint(local_origin);
 
   decoration_info.antialias = ShouldSetDecorationAntialias(decorations);
 
   decoration_info.style =
       LineLayoutAPIShim::LayoutObjectFrom(box.GetLineLayoutItem())
           ->Style(box.IsFirstLineStyle());
+  decoration_info.baseline_type = box.Root().BaselineType();
+  decoration_info.underline_position = ResolveUnderlinePosition(
+      *decoration_info.style, decoration_info.baseline_type);
+
   decoration_info.font_data = decoration_info.style->GetFont().PrimaryFont();
   DCHECK(decoration_info.font_data);
   decoration_info.baseline =
       decoration_info.font_data
           ? decoration_info.font_data->GetFontMetrics().FloatAscent()
           : 0;
 
-  // Set the thick of the line to be 10% (or something else ?)of the computed
-  // font size and not less than 1px.  Using computedFontSize should take care
-  // of zoom as well.
-
-  // Update Underline thickness, in case we have Faulty Font Metrics calculating
-  // underline thickness by old method.
-  float text_decoration_thickness = 0.0;
-  int font_height_int = 0;
-  if (decoration_info.font_data) {
-    text_decoration_thickness =
-        decoration_info.font_data->GetFontMetrics().UnderlineThickness();
-    font_height_int =
-        (int)(decoration_info.font_data->GetFontMetrics().FloatHeight() + 0.5);
+  if (decoration_info.underline_position == ResolvedUnderlinePosition::kRoman) {
+    decoration_info.thickness = ComputeDecorationThickness(
+        decoration_info.style, decoration_info.font_data);
+  } else {
+    // Compute decorating box. Position and thickness are computed from the
+    // decorating box.
+    // Only for non-Roman for now for the performance implications.
+    // https:// drafts.csswg.org/css-text-decor-3/#decorating-box
+    decoration_info.decorating_box = EnclosingUnderlineObject(&box);
+    if (decoration_info.decorating_box) {
+      decoration_info.thickness = ComputeDecorationThickness(
+          decoration_info.decorating_box.Style(),
+          decoration_info.decorating_box.Style()->GetFont().PrimaryFont());
+    } else {
+      decoration_info.thickness = ComputeDecorationThickness(
+          decoration_info.style, decoration_info.font_data);
+    }
   }
-  if ((text_decoration_thickness == 0.f) ||
-      (text_decoration_thickness >= (font_height_int >> 1))) {
-    text_decoration_thickness =
-        std::max(1.f, decoration_info.style->ComputedFontSize() / 10.f);
-  }
-  decoration_info.thickness = text_decoration_thickness;
 
   // Offset between lines - always non-zero, so lines never cross each other.
-  decoration_info.double_offset = text_decoration_thickness + 1.f;
+  decoration_info.double_offset = decoration_info.thickness + 1.f;
 }
 
 static void PaintDecorationsExceptLineThrough(
     TextPainter& text_painter,
     bool& has_line_through_decoration,
     const InlineTextBox& box,
     const DecorationInfo& decoration_info,
     const PaintInfo& paint_info,
     const Vector<AppliedTextDecoration>& decorations) {
   GraphicsContext& context = paint_info.context;
   GraphicsContextStateSaver state_saver(context);
   context.SetStrokeThickness(decoration_info.thickness);
   bool skip_intercepts =
       decoration_info.style->GetTextDecorationSkip() & kTextDecorationSkipInk;
 
   // text-underline-position may flip underline and overline.
   ResolvedUnderlinePosition underline_position =
-      ResolveUnderlinePosition(*decoration_info.style, &box);
+      decoration_info.underline_position;
   bool flip_underline_and_overline = false;
   if (underline_position == ResolvedUnderlinePosition::kOver) {
     flip_underline_and_overline = true;
     underline_position = ResolvedUnderlinePosition::kUnder;
   }
 
   for (const AppliedTextDecoration& decoration : decorations) {
     TextDecoration lines = decoration.Lines();
     if (flip_underline_and_overline) {
       lines = static_cast<TextDecoration>(
           lines ^ (kTextDecorationUnderline | kTextDecorationOverline));
     }
     if ((lines & kTextDecorationUnderline) && decoration_info.font_data) {
-      const int underline_offset =
-          ComputeUnderlineOffset(underline_position, *decoration_info.style,
-                                 decoration_info.font_data->GetFontMetrics(),
-                                 &box, decoration_info.thickness);
+      const int underline_offset = ComputeUnderlineOffset(
+          underline_position, *decoration_info.style,
+          decoration_info.font_data->GetFontMetrics(), &box,
+          decoration_info.decorating_box, decoration_info.thickness);
       AppliedDecorationPainter decoration_painter(
           context, decoration_info, underline_offset, decoration,
           decoration_info.double_offset, 1);
       if (skip_intercepts) {
         text_painter.ClipDecorationsStripe(
             -decoration_info.baseline +
                 decoration_painter.DecorationBounds().Y() -
                 decoration_info.local_origin.Y(),
             decoration_painter.DecorationBounds().Height(),
             decoration_info.thickness);
       }
       decoration_painter.Paint();
     }
     if (lines & kTextDecorationOverline) {
-      const int overline_offset =
-          ComputeOverlineOffset(*decoration_info.style, &box);
+      const int overline_offset = ComputeUnderlineOffsetForUnder(
+          *decoration_info.style, &box, decoration_info.decorating_box,
+          decoration_info.thickness,
+          flip_underline_and_overline ? LineVerticalPositionType::TopOfEmHeight
+                                      : LineVerticalPositionType::TextTop);
       AppliedDecorationPainter decoration_painter(
           context, decoration_info, overline_offset, decoration,
           -decoration_info.double_offset, 1);
       if (skip_intercepts) {
         text_painter.ClipDecorationsStripe(
             -decoration_info.baseline +
                 decoration_painter.DecorationBounds().Y() -
                 decoration_info.local_origin.Y(),
             decoration_painter.DecorationBounds().Height(),
             decoration_info.thickness);
@@ skipping 940 matching lines @@
 
   LayoutRect box_rect(box_origin, LayoutSize(inline_text_box_.LogicalWidth(),
                                              inline_text_box_.LogicalHeight()));
   context.Clip(FloatRect(box_rect));
   context.DrawHighlightForText(font, run, FloatPoint(box_origin),
                                box_rect.Height().ToInt(), color,
                                paint_offsets.first, paint_offsets.second);
 }
 
 }  // namespace blink