RubyNG: Cleanup of ComputeAnnotationOverflow()

- Do not use NGLineHeightMetrics
  The return value is unrelated to ascent/descent.

- Do not use a single field for two difference purpose; overflow and
  space

- Rename variables named |*_block_start/end| to |*_over/under| in
  ComputeAnnotationOverflow().
  All variables representing offsets are in line-relative coordination
  system. |*_block_start/end| are confusing.

This CL has no behavior changes.

Bug: 1069817
Change-Id: Ida0f3abdb462fa0b35d4b0a9a299a1e1c1e5c442
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2260478
Auto-Submit: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781668}

third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc

@@ -380,7 +380,7 @@ void NGInlineLayoutAlgorithm::CreateLine(
                          exclusion_space);
   }
 
-  NGLineHeightMetrics annotation_metrics = NGLineHeightMetrics::Zero();
+  NGAnnotationMetrics annotation_metrics;
   if (Node().HasRuby() &&
       !RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) {
     annotation_metrics = ComputeAnnotationOverflow(line_box_, line_box_metrics,
@@ -421,33 +421,38 @@ void NGInlineLayoutAlgorithm::CreateLine(
     annotation_metrics = ComputeAnnotationOverflow(
         line_box_, line_box_metrics, LayoutUnit(), line_info->LineStyle());
   }
-  LayoutUnit annotation_block_start;
-  LayoutUnit annotation_block_end;
+  LayoutUnit annotation_overflow_block_start;
+  LayoutUnit annotation_overflow_block_end;
+  LayoutUnit annotation_space_block_start;
+  LayoutUnit annotation_space_block_end;
   if (!IsFlippedLinesWritingMode(line_info->LineStyle().GetWritingMode())) {
-    annotation_block_start = annotation_metrics.ascent;
-    annotation_block_end = annotation_metrics.descent;
+    annotation_overflow_block_start = annotation_metrics.overflow_over;
+    annotation_overflow_block_end = annotation_metrics.overflow_under;
+    annotation_space_block_start = annotation_metrics.space_over;
+    annotation_space_block_end = annotation_metrics.space_under;
   } else {
-    annotation_block_start = annotation_metrics.descent;
-    annotation_block_end = annotation_metrics.ascent;
+    annotation_overflow_block_start = annotation_metrics.overflow_under;
+    annotation_overflow_block_end = annotation_metrics.overflow_over;
+    annotation_space_block_start = annotation_metrics.space_under;
+    annotation_space_block_end = annotation_metrics.space_over;
   }
 
-  LayoutUnit block_offset_shift = annotation_block_start.ClampNegativeToZero();
+  LayoutUnit block_offset_shift = annotation_overflow_block_start;
   // If the previous line has block-end annotation overflow and this line has
   // block-start annotation space, shift up the block offset of this line.
   if (ConstraintSpace().BlockStartAnnotationSpace() < LayoutUnit() &&
-      annotation_block_start < LayoutUnit()) {
+      annotation_space_block_start) {
     const LayoutUnit overflow = -ConstraintSpace().BlockStartAnnotationSpace();
-    const LayoutUnit space = -annotation_block_start;
-    block_offset_shift = -std::min(space, overflow);
+    block_offset_shift = -std::min(annotation_space_block_start, overflow);
   }
 
   // If this line has block-start annotation overflow and the previous line has
   // block-end annotation space, borrow the block-end space of the previous line
   // and shift down the block offset by |overflow - space|.
-  if (annotation_block_start > LayoutUnit() &&
+  if (annotation_overflow_block_start &&
       ConstraintSpace().BlockStartAnnotationSpace() > LayoutUnit()) {
-    block_offset_shift =
-        (annotation_block_start - ConstraintSpace().BlockStartAnnotationSpace())
+    block_offset_shift = (annotation_overflow_block_start -
+                          ConstraintSpace().BlockStartAnnotationSpace())
                              .ClampNegativeToZero();
   }
 
@@ -458,10 +463,10 @@ void NGInlineLayoutAlgorithm::CreateLine(
   container_builder_.SetMetrics(line_box_metrics);
   container_builder_.SetBfcBlockOffset(line_info->BfcOffset().block_offset +
                                        block_offset_shift);
-  if (annotation_block_end > LayoutUnit())
-    container_builder_.SetAnnotationOverflow(annotation_block_end);
-  else if (annotation_block_end < LayoutUnit())
-    container_builder_.SetBlockEndAnnotationSpace(-annotation_block_end);
+  if (annotation_overflow_block_end)
+    container_builder_.SetAnnotationOverflow(annotation_overflow_block_end);
+  else if (annotation_space_block_end)
+    container_builder_.SetBlockEndAnnotationSpace(annotation_space_block_end);
 }
 
 void NGInlineLayoutAlgorithm::PlaceControlItem(const NGInlineItem& item,

third_party/blink/renderer/core/layout/ng/inline/ng_ruby_utils.cc

@@ -217,29 +217,28 @@ LayoutUnit CommitPendingEndOverhang(NGLineInfo* line_info) {
   return end_overhang;
 }
 
-NGLineHeightMetrics ComputeAnnotationOverflow(
+NGAnnotationMetrics ComputeAnnotationOverflow(
     const NGLogicalLineItems& logical_line,
     const NGLineHeightMetrics& line_box_metrics,
-    LayoutUnit line_block_start,
+    LayoutUnit line_over,
     const ComputedStyle& line_style) {
   DCHECK(RuntimeEnabledFeatures::LayoutNGRubyEnabled());
   // Min/max position of content without line-height.
-  LayoutUnit content_block_start = line_block_start + line_box_metrics.ascent;
-  LayoutUnit content_block_end = content_block_start;
+  LayoutUnit content_over = line_over + line_box_metrics.ascent;
+  LayoutUnit content_under = content_over;
 
   // Min/max position of annotations.
-  LayoutUnit annotation_block_start = content_block_start;
-  LayoutUnit annotation_block_end = content_block_start;
+  LayoutUnit annotation_over = content_over;
+  LayoutUnit annotation_under = content_over;
 
-  const LayoutUnit line_block_end =
-      line_block_start + line_box_metrics.LineHeight();
+  const LayoutUnit line_under = line_over + line_box_metrics.LineHeight();
   bool has_over_emphasis = false;
   bool has_under_emphasis = false;
   for (const NGLogicalLineItem& item : logical_line) {
     if (item.HasInFlowFragment()) {
       if (!item.IsControl()) {
-        content_block_start = std::min(content_block_start, item.BlockOffset());
-        content_block_end = std::max(content_block_end, item.BlockEndOffset());
+        content_over = std::min(content_over, item.BlockOffset());
+        content_under = std::max(content_under, item.BlockEndOffset());
       }
       if (const auto* style = item.Style()) {
         if (style->GetTextEmphasisMark() != TextEmphasisMark::kNone) {
@@ -260,49 +259,47 @@ NGLineHeightMetrics ComputeAnnotationOverflow(
     if (IsFlippedLinesWritingMode(line_style.GetWritingMode()))
       overflow = -overflow;
     if (overflow < LayoutUnit()) {
-      annotation_block_start = std::min(
-          annotation_block_start, item.rect.offset.block_offset + overflow);
+      annotation_over =
+          std::min(annotation_over, item.rect.offset.block_offset + overflow);
     } else if (overflow > LayoutUnit()) {
-      const LayoutUnit block_end =
+      const LayoutUnit logical_bottom =
           item.rect.offset.block_offset +
           layout_result->PhysicalFragment()
               .Size()
               .ConvertToLogical(line_style.GetWritingMode())
               .block_size;
-      annotation_block_end =
-          std::max(annotation_block_end, block_end + overflow);
+      annotation_under = std::max(annotation_under, logical_bottom + overflow);
     }
   }
 
   // Probably this is an empty line. We should secure font-size space.
   const LayoutUnit font_size(line_style.ComputedFontSize());
-  if (content_block_end - content_block_start < font_size) {
+  if (content_under - content_over < font_size) {
     LayoutUnit half_leading = (line_box_metrics.LineHeight() - font_size) / 2;
     half_leading = half_leading.ClampNegativeToZero();
-    content_block_start = line_block_start + half_leading;
-    content_block_end = line_block_end - half_leading;
+    content_over = line_over + half_leading;
+    content_under = line_under - half_leading;
   }
 
   // Don't provide annotation space if text-emphasis exists.
-  // TODO(layout-dev): If the text-emphasis is in
-  // [line_block_start, line_block_end], this line can provide annotation space.
+  // TODO(layout-dev): If the text-emphasis is in [line_over, line_under],
+  // this line can provide annotation space.
   if (has_over_emphasis)
-    content_block_start = line_block_start;
+    content_over = line_over;
   if (has_under_emphasis)
-    content_block_end = line_block_end;
+    content_under = line_under;
 
+  const LayoutUnit overflow_over =
+      (line_over - annotation_over).ClampNegativeToZero();
+  const LayoutUnit overflow_under =
+      (annotation_under - line_under).ClampNegativeToZero();
+  return {overflow_over, overflow_under,
           // With some fonts, text fragment sizes can exceed line-height.
-  // We should not handle them as annotation overflow.
-  content_block_start = std::max(content_block_start, line_block_start);
-  content_block_end = std::min(content_block_end, line_block_end);
-
-  const LayoutUnit content_or_annotation_block_start =
-      std::min(content_block_start, annotation_block_start);
-  const LayoutUnit content_or_annotation_block_end =
-      std::max(content_block_end, annotation_block_end);
-  return NGLineHeightMetrics(
-      line_block_start - content_or_annotation_block_start,
-      content_or_annotation_block_end - line_block_end);
+          // We need ClampNegativeToZero().
+          overflow_over ? LayoutUnit()
+                        : (content_over - line_over).ClampNegativeToZero(),
+          overflow_under ? LayoutUnit()
+                         : (line_under - content_under).ClampNegativeToZero()};
 }
 
 }  // namespace blink

third_party/blink/renderer/core/layout/ng/inline/ng_ruby_utils.h

@@ -55,19 +55,29 @@ bool CanApplyStartOverhang(const NGLineInfo& line_info,
 // in |line_info|
 LayoutUnit CommitPendingEndOverhang(NGLineInfo* line_info);
 
-// Compute over/under annotation overflow/space for the specified line.
+// Stores ComputeAnnotationOverflow() results.
 //
-// Return value:
-//   .ascent > 0: The amount of annotation overflow at the line-top side
-//   .ascent < 0: The amount of annotation space which the next line at the
-//                line-top side can consume.
-//   .descent > 0: The amount of annotation overflow at the line-bottom side.
-//   .descent < 0: The amount of annotation space which the next line at the
-//                 line-bottom side can consume.
-NGLineHeightMetrics ComputeAnnotationOverflow(
+// |overflow_over| and |space_over| are exclusive. Only one of them can be
+// non-zero. |overflow_under| and |space_under| are exclusive too.
+// All fields never be negative.
+struct NGAnnotationMetrics {
+  // The amount of annotation overflow at the line-over side.
+  LayoutUnit overflow_over;
+  // The amount of annotation overflow at the line-under side.
+  LayoutUnit overflow_under;
+  // The amount of annotation space which the next line at the line-over
+  // side can consume.
+  LayoutUnit space_over;
+  // The amount of annotation space which the next line at the line-under
+  // side can consume.
+  LayoutUnit space_under;
+};
+
+// Compute over/under annotation overflow/space for the specified line.
+NGAnnotationMetrics ComputeAnnotationOverflow(
     const NGLogicalLineItems& logical_line,
     const NGLineHeightMetrics& line_box_metrics,
-    LayoutUnit line_block_start,
+    LayoutUnit line_over,
     const ComputedStyle& line_style);
 
 }  // namespace blink