[LayoutNG] Misc fixes in ShapingLineBreaker

third_party/WebKit/Source/platform/fonts/shaping/ShapingLineBreaker.cpp

 // Copyright 2017 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 "platform/fonts/shaping/ShapingLineBreaker.h"
 
 #include "platform/fonts/Font.h"
 #include "platform/fonts/shaping/HarfBuzzShaper.h"
 #include "platform/fonts/shaping/ShapeResult.h"
 #include "platform/fonts/shaping/ShapeResultInlineHeaders.h"
 #include "platform/text/TextBreakIterator.h"
 
 namespace blink {
 
-ShapingLineBreaker::ShapingLineBreaker(const HarfBuzzShaper* shaper,
-                                       const Font* font,
-                                       const ShapeResult* result,
-                                       const AtomicString locale,
-                                       LineBreakType break_type)
+ShapingLineBreaker::ShapingLineBreaker(
+    const HarfBuzzShaper* shaper,
+    const Font* font,
+    const ShapeResult* result,
+    const LazyLineBreakIterator* break_iterator)
     : shaper_(shaper),
       font_(font),
       result_(result),
-      locale_(locale),
-      break_type_(break_type) {
+      break_iterator_(break_iterator) {
   text_ = String(shaper->GetText(), shaper->TextLength());
 }
 
 namespace {
 
 unsigned PreviousSafeToBreakAfter(const UChar* text,
                                   unsigned start,
                                   unsigned offset) {
   // TODO(eae): This is quite incorrect. It should be changed to use the
   // HarfBuzzHarfBuzz safe to break info when available.
   for (; offset > start; offset--) {
     if (text[offset - 1] == kSpaceCharacter)
       break;
   }
   return offset;
 }
 
 unsigned NextSafeToBreakBefore(const UChar* text,
                                unsigned end,
                                unsigned offset) {
   // TODO(eae): This is quite incorrect. It should be changed to use the
   // HarfBuzzHarfBuzz safe to break info when available.
   for (; offset < end; offset++) {
     if (text[offset] == kSpaceCharacter)
       break;
   }
   return offset;
 }
 
+LayoutUnit FlipRtl(LayoutUnit value, TextDirection direction) {
+  return direction == TextDirection::kRtl ? -value : value;
+}
+
 }  // namespace
 
 // Shapes a line of text by finding a valid and appropriate break opportunity
 // based on the shaping results for the entire paragraph. Re-shapes the start
 // and end of the line as needed.
 //
 // Definitions:
 //   Candidate break opportunity: Ideal point to break, disregarding line
 //                                breaking rules. May be in the middle of a word
 //                                or inside a ligature.
@@ skipping 15 matching lines @@
 //   The candidate (or ideal) break opportunity would be at an offset of 10 as
 //   the break would happen at exactly 100px in that case.
 //   The previous valid break opportunity though is at an offset of 5.
 //   If we further assume that the font kerns with space then even though it's a
 //   valid break opportunity reshaping is required as the combined width of the
 //   two segments "Line " and "breaking" may be different from "Line breaking".
 PassRefPtr<ShapeResult> ShapingLineBreaker::ShapeLine(
     unsigned start,
     LayoutUnit available_space,
     unsigned* break_offset) {
+  unsigned range_start = result_->CharacterStartIndex();
+  unsigned range_end = range_start + result_->NumCharacters();
+  DCHECK_GE(start, range_start);
+  DCHECK_LT(start, range_end);
+
   // The start position in the original shape results.
-  LayoutUnit start_position = result_->SnappedStartPositionForOffset(start);
+  float start_position_float = result_->PositionForOffset(start - range_start);
+  LayoutUnit start_position = LayoutUnit::FromFloatFloor(start_position_float);
   TextDirection direction = result_->Direction();
 
+  // Find a candidate break opportunity by identifying the last offset before
+  // exceeding the available space and the determine the closest valid break
+  // preceding the candidate.
+  LayoutUnit end_position = LayoutUnit::FromFloatCeil(start_position_float) +
+                            FlipRtl(available_space, direction);
+  unsigned candidate_break =
+      result_->OffsetForPosition(end_position,
+                                 ShapeResult::kOutsideAsStartOrEnd) +
+      range_start;
+  DCHECK_GE(candidate_break, start);
+  unsigned break_opportunity =
+      break_iterator_->PreviousBreakOpportunity(candidate_break, start);
+  if (break_opportunity <= start) {
+    break_opportunity = break_iterator_->NextBreakOpportunity(
+        std::max(candidate_break, start + 1));
+  }
+  DCHECK_GT(break_opportunity, start);
+
   // If the start offset is not at a safe-to-break boundary the content between
   // the start and the next safe-to-break boundary needs to be reshaped and the
   // available space adjusted to take the reshaping into account.
   RefPtr<ShapeResult> line_start_result;
   unsigned first_safe =
       NextSafeToBreakBefore(shaper_->GetText(), shaper_->TextLength(), start);
-  if (first_safe != start) {
+  DCHECK_GE(first_safe, start);
+  // Reshape takes place only when first_safe is before the break opportunity.

[eae, 2017/05/12 18:45:49]

I considered this case and wanted to fix it but failed to create a test case
where it would fail which is why I left it out of the original version. Did you
manage to come up with a test for it? If so, yay!

[kojii, 2017/05/12 19:08:13]

Yes, the Han character in your ShapeLineArabicThaiHanLatinBreakAll() hit this
when I added some new cases. Thank you for the test!

+  // Otherwise reshape will be part of line_end_result.
+  if (first_safe != start && first_safe < break_opportunity) {
     LayoutUnit original_width =
-        result_->SnappedEndPositionForOffset(first_safe) - start_position;
+        FlipRtl(result_->SnappedEndPositionForOffset(first_safe - range_start) -
+                    start_position,
+                direction);
     line_start_result = shaper_->Shape(font_, direction, start, first_safe);
     available_space += line_start_result->SnappedWidth() - original_width;
   }
 
-  // Find a candidate break opportunity by identifying the last offset before
-  // exceeding the available space and the determine the closest valid break
-  // preceding the candidate.
-  LazyLineBreakIterator break_iterator(text_, locale_, break_type_);
-  LayoutUnit end_position = start_position + available_space;
-  unsigned candidate_break = result_->OffsetForPosition(end_position, false);
-  unsigned break_opportunity =
-      break_iterator.PreviousBreakOpportunity(candidate_break, start);
-  if (break_opportunity <= start) {
-    break_opportunity = break_iterator.NextBreakOpportunity(candidate_break);
-  }
-
   RefPtr<ShapeResult> line_end_result;
   unsigned last_safe = break_opportunity;
   while (break_opportunity > start) {
     // If the previous valid break opportunity is not at a safe-to-break
     // boundary reshape between the safe-to-break offset and the valid break
     // offset. If the resulting width exceeds the available space the
     // preceding boundary is tried until the available space is sufficient.
     unsigned previous_safe = std::max(
         PreviousSafeToBreakAfter(shaper_->GetText(), start, break_opportunity),
         start);
+    DCHECK_LE(previous_safe, break_opportunity);
     if (previous_safe != break_opportunity) {
       LayoutUnit safe_position =
-          result_->SnappedStartPositionForOffset(previous_safe);
-      while (break_opportunity > previous_safe && previous_safe > start) {
+          result_->SnappedStartPositionForOffset(previous_safe - range_start);
+      while (break_opportunity > previous_safe && previous_safe >= start) {
         line_end_result =
-            shaper_->Shape(font_, direction, previous_safe, break_opportunity);
-        if (safe_position + line_end_result->SnappedWidth() <= end_position)
+            shaper_->Shape(font_, direction, previous_safe,
+                           std::min(break_opportunity, range_end));
+        if (line_end_result->SnappedWidth() <=
+            FlipRtl(end_position - safe_position, direction))
           break;
+        // Doesn't fit after the reshape. Try previous break opportunity, or
+        // overflow if there were none.
+        unsigned previous_break_opportunity =
+            break_iterator_->PreviousBreakOpportunity(break_opportunity - 1,
+                                                      start);
+        if (previous_break_opportunity <= start)
+          break;
+        break_opportunity = previous_break_opportunity;
         line_end_result = nullptr;
-        break_opportunity = break_iterator.PreviousBreakOpportunity(
-            break_opportunity - 1, start);
       }
     }
 
     if (break_opportunity > start) {
       last_safe = previous_safe;
       break;
     }
 
     // No suitable break opportunity, not exceeding the available space,
     // found. Choose the next valid one even though it will overflow.
-    break_opportunity = break_iterator.NextBreakOpportunity(candidate_break);
+    break_opportunity = break_iterator_->NextBreakOpportunity(candidate_break);
   }
 
   // Create shape results for the line by copying from the re-shaped result (if
   // reshaping was needed) and the original shape results.
   RefPtr<ShapeResult> line_result = ShapeResult::Create(font_, 0, direction);
   unsigned max_length = std::numeric_limits<unsigned>::max();
   if (line_start_result)
     line_start_result->CopyRange(0, max_length, line_result.Get());
   if (last_safe > first_safe)
     result_->CopyRange(first_safe, last_safe, line_result.Get());
   if (line_end_result)
     line_end_result->CopyRange(last_safe, max_length, line_result.Get());
 
+  DCHECK_GT(break_opportunity, start);
+  DCHECK_EQ(std::min(break_opportunity, range_end) - start,
+            line_result->NumCharacters());
+
   *break_offset = break_opportunity;
   return line_result.Release();
 }
 
 }  // namespace blink