Refactor OffsetForPosition and add a variation for ellipsis

This patch refactors ShapeResult::OffsetForPosition in:

1. Make the underlying function common, without "mode".
2. Unify LTR and RTL code path in lower-layer functions.
3. Add a variation OffsetToFit for 'text-overflow:
   ellipsis' that computes offset that can fit to the
   specified width.

While implementing 'text-overflow: ellipsis', which is
more sensitive to how many characters can fit, existing
functions turned out not to be accurate enough for the
requirement. The current code uses OffsetForPosition
then +1 or -1, which is not accurate when ligature/
joining occurs.

Maybe ShapingLineBreaker should move to the OffsetToFit
function but I have not done enough verifications yet.

Bug: 636993
Change-Id: Ic9a1865d4b772fc7ce5cf94424a0aa3f9e9af332
Reviewed-on: https://chromium-review.googlesource.com/1020760Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553982}

third_party/blink/renderer/platform/fonts/shaping/harf_buzz_shaper_test.cc

@@ -46,7 +46,6 @@ class HarfBuzzShaperTest : public testing::Test {
     scoped_refptr<ShapeResult> run2 = shape_result->SubRange(offset, length);
     shape_result = shape_result->SubRange(0, offset);
     run2->CopyRange(offset, length, shape_result.get());
-    // LOG(INFO) << shape_result->ToString();
     return shape_result;
   }
 
@@ -570,29 +569,33 @@ static struct OffsetForPositionTestData {
   unsigned offset_rtl;
   unsigned hit_test_ltr;
   unsigned hit_test_rtl;
-} offset_for_position_tet_data[] = {
+  unsigned fit_ltr_ltr;
+  unsigned fit_ltr_rtl;
+  unsigned fit_rtl_ltr;
+  unsigned fit_rtl_rtl;
+} offset_for_position_fixed_pitch_test_data[] = {
     // The left edge.
-    {-1, 0, 5, 0, 5},
-    {0, 0, 5, 0, 5},
+    {-1, 0, 5, 0, 5, 0, 0, 5, 5},
+    {0, 0, 5, 0, 5, 0, 0, 5, 5},
     // Hit test should round to the nearest glyph at the middle of a glyph.
-    {4, 0, 4, 0, 5},
-    {6, 0, 4, 1, 4},
+    {4, 0, 4, 0, 5, 0, 1, 5, 4},
+    {6, 0, 4, 1, 4, 0, 1, 5, 4},
     // Glyph boundary between the 1st and the 2nd glyph.
     // Avoid testing "10.0" to avoid rounding differences on Windows.
-    {9.99, 0, 4, 1, 4},
-    {10.01, 1, 3, 1, 4},
+    {9.9, 0, 4, 1, 4, 0, 1, 5, 4},
+    {10.1, 1, 3, 1, 4, 1, 2, 4, 3},
     // Run boundary is at position 20. The 1st run has 2 characters.
-    {14, 1, 3, 1, 4},
-    {16, 1, 3, 2, 3},
-    {20.01, 2, 2, 2, 3},
-    {24, 2, 2, 2, 3},
-    {26, 2, 2, 3, 2},
+    {14, 1, 3, 1, 4, 1, 2, 4, 3},
+    {16, 1, 3, 2, 3, 1, 2, 4, 3},
+    {20.1, 2, 2, 2, 3, 2, 3, 3, 2},
+    {24, 2, 2, 2, 3, 2, 3, 3, 2},
+    {26, 2, 2, 3, 2, 2, 3, 3, 2},
     // The end of the ShapeResult. The result has 5 characters.
-    {44, 4, 0, 4, 1},
-    {46, 4, 0, 5, 0},
-    {50, 5, 0, 5, 0},
+    {44, 4, 0, 4, 1, 4, 5, 1, 0},
+    {46, 4, 0, 5, 0, 4, 5, 1, 0},
+    {50, 5, 0, 5, 0, 5, 5, 0, 0},
     // Beyond the right edge of the ShapeResult.
-    {51, 5, 0, 5, 0},
+    {51, 5, 0, 5, 0, 5, 5, 0, 0},
 };
 
 std::ostream& operator<<(std::ostream& ostream,
@@ -604,9 +607,10 @@ class OffsetForPositionTest
     : public HarfBuzzShaperTest,
       public testing::WithParamInterface<OffsetForPositionTestData> {};
 
-INSTANTIATE_TEST_CASE_P(HarfBuzzShaperTest,
-                        OffsetForPositionTest,
-                        testing::ValuesIn(offset_for_position_tet_data));
+INSTANTIATE_TEST_CASE_P(
+    HarfBuzzShaperTest,
+    OffsetForPositionTest,
+    testing::ValuesIn(offset_for_position_fixed_pitch_test_data));
 
 TEST_P(OffsetForPositionTest, Data) {
   auto data = GetParam();
@@ -617,10 +621,18 @@ TEST_P(OffsetForPositionTest, Data) {
       SplitRun(shaper.Shape(&ahem, TextDirection::kLtr), 2);
   EXPECT_EQ(data.offset_ltr, result->OffsetForPosition(data.position, false));
   EXPECT_EQ(data.hit_test_ltr, result->OffsetForPosition(data.position, true));
+  EXPECT_EQ(data.fit_ltr_ltr,
+            result->OffsetToFit(data.position, TextDirection::kLtr));
+  EXPECT_EQ(data.fit_ltr_rtl,
+            result->OffsetToFit(data.position, TextDirection::kRtl));
 
   result = SplitRun(shaper.Shape(&ahem, TextDirection::kRtl), 3);
   EXPECT_EQ(data.offset_rtl, result->OffsetForPosition(data.position, false));
   EXPECT_EQ(data.hit_test_rtl, result->OffsetForPosition(data.position, true));
+  EXPECT_EQ(data.fit_rtl_ltr,
+            result->OffsetToFit(data.position, TextDirection::kLtr));
+  EXPECT_EQ(data.fit_rtl_rtl,
+            result->OffsetToFit(data.position, TextDirection::kRtl));
 }
 
 TEST_F(HarfBuzzShaperTest, PositionForOffsetLatin) {

third_party/blink/renderer/platform/fonts/shaping/shape_result.cc

@@ -135,57 +135,35 @@ float ShapeResult::RunInfo::XPositionForOffset(
   return position;
 }
 
-static bool TargetPastEdge(bool rtl, float target_x, float next_x) {
-  // In LTR, the edge belongs to the character on right.
-  if (!rtl)
-    return target_x < next_x;
-
-  // In RTL, the edge belongs to the character on left.
-  return target_x <= next_x;
-}
-
-int ShapeResult::RunInfo::CharacterIndexForXPosition(
+void ShapeResult::RunInfo::CharacterIndexForXPosition(
     float target_x,
-    bool include_partial_glyphs) const {
+    GlyphIndexResult* result) const {
   DCHECK(target_x >= 0 && target_x <= width_);
-  if (target_x <= 0)
-    return !Rtl() ? 0 : num_characters_;
   const unsigned num_glyphs = glyph_data_.size();
   float current_x = 0;
-  float current_advance = 0;
   unsigned glyph_index = 0;
-  unsigned prev_character_index = num_characters_;  // used only when rtl()
 
-  while (glyph_index < num_glyphs) {
-    float prev_advance = current_advance;
+  while (true) {
     unsigned current_character_index = glyph_data_[glyph_index].character_index;
-    current_advance = glyph_data_[glyph_index].advance;
-    while (glyph_index < num_glyphs - 1 &&
+    float current_advance = glyph_data_[glyph_index].advance;
+    unsigned next_glyph_index = glyph_index + 1;
+    while (next_glyph_index < num_glyphs &&
            current_character_index ==
-               glyph_data_[glyph_index + 1].character_index)
-      current_advance += glyph_data_[++glyph_index].advance;
-    float next_x;
-    if (include_partial_glyphs) {
-      // For hit testing, find the closest caret point by incuding
-      // end-half of the previous character and start-half of the current
-      // character.
-      current_advance = current_advance / 2.0;
-      next_x = current_x + prev_advance + current_advance;
-      // When include_partial_glyphs, "<=" or "<" is not a big deal because
-      // |next_x| is not at the character boundary.
-      if (target_x <= next_x)
-        return Rtl() ? prev_character_index : current_character_index;
-    } else {
-      next_x = current_x + current_advance;
-      if (TargetPastEdge(Rtl(), target_x, next_x))
-        return current_character_index;
+               glyph_data_[next_glyph_index].character_index)
+      current_advance += glyph_data_[next_glyph_index++].advance;
+    float next_x = current_x + current_advance;
+    if (target_x < next_x || next_glyph_index == num_glyphs) {
+      result->glyph_index = glyph_index;
+      result->next_glyph_index = next_glyph_index;
+      result->character_index = current_character_index;
+      result->origin_x = current_x;
+      result->advance = current_advance;
+      return;
     }
     current_x = next_x;
-    prev_character_index = current_character_index;
-    ++glyph_index;
+    glyph_index = next_glyph_index;
   }
-
-  return Rtl() ? 0 : num_characters_;
+  NOTREACHED();
 }
 
 void ShapeResult::RunInfo::SetGlyphAndPositions(unsigned index,
@@ -321,50 +299,107 @@ unsigned ShapeResult::PreviousSafeToBreakOffset(unsigned index) const {
   return StartIndexForResult();
 }
 
+// Returns the offset of the character of |result| for LTR.
+unsigned ShapeResult::OffsetLtr(const GlyphIndexResult& result) const {
+  DCHECK(IsLtr(Direction()));
+  return result.characters_on_left_runs + result.character_index;
+}
+
+// Returns the offset of the character of |result| for RTL.
+unsigned ShapeResult::OffsetRtl(const GlyphIndexResult& result, float x) const {
+  DCHECK(IsRtl(Direction()));
+  if (!result.IsInRun())
+    return NumCharacters() - result.characters_on_left_runs;
+  // In RTL, the boundary belongs to the left character. This subtle difference
+  // allows round trips between OffsetForPoint and PointForOffset.
+  if (UNLIKELY(x == result.origin_x))
+    return OffsetLeftRtl(result);
+  return NumCharacters() - result.characters_on_left_runs -
+         runs_[result.run_index]->num_characters_ + result.character_index;
+}
+
+// Returns the offset of the character on the right of |result| for LTR.
+unsigned ShapeResult::OffsetRightLtr(const GlyphIndexResult& result) const {
+  DCHECK(IsLtr(Direction()));
+  if (result.run_index >= runs_.size())
+    return NumCharacters();
+  const RunInfo& run = *runs_[result.run_index];
+  return result.characters_on_left_runs +
+         (result.next_glyph_index < run.glyph_data_.size()
+              ? run.glyph_data_[result.next_glyph_index].character_index
+              : run.num_characters_);
+}
+
+// Returns the offset of the character on the left of |result| for RTL.
+unsigned ShapeResult::OffsetLeftRtl(const GlyphIndexResult& result) const {
+  DCHECK(IsRtl(Direction()));
+  if (!result.glyph_index)
+    return NumCharacters() - result.characters_on_left_runs;
+  const RunInfo& run = *runs_[result.run_index];
+  return NumCharacters() - result.characters_on_left_runs -
+         run.num_characters_ +
+         run.glyph_data_[result.glyph_index - 1].character_index;
+}
+
 // If the position is outside of the result, returns the start or the end offset
 // depends on the position.
-unsigned ShapeResult::OffsetForPosition(float target_x,
-                                        bool include_partial_glyphs) const {
+void ShapeResult::OffsetForPosition(float target_x,
+                                    GlyphIndexResult* result) const {
+  if (target_x <= 0)
+    return;
+
   unsigned characters_so_far = 0;
   float current_x = 0;
-
-  if (Rtl()) {
-    if (target_x <= 0)
-      return num_characters_;
-    characters_so_far = num_characters_;
-    for (unsigned i = 0; i < runs_.size(); ++i) {
-      if (!runs_[i])
-        continue;
-      characters_so_far -= runs_[i]->num_characters_;
-      float next_x = current_x + runs_[i]->width_;
-      float offset_for_run = target_x - current_x;
-      if (offset_for_run >= 0 && offset_for_run <= runs_[i]->width_) {
-        // The x value in question is within this script run.
-        const unsigned index = runs_[i]->CharacterIndexForXPosition(
-            offset_for_run, include_partial_glyphs);
-        return characters_so_far + index;
-      }
-      current_x = next_x;
-    }
-  } else {
-    if (target_x <= 0)
-      return 0;
-    for (unsigned i = 0; i < runs_.size(); ++i) {
-      if (!runs_[i])
-        continue;
-      float next_x = current_x + runs_[i]->width_;
-      float offset_for_run = target_x - current_x;
-      if (offset_for_run >= 0 && offset_for_run <= runs_[i]->width_) {
-        const unsigned index = runs_[i]->CharacterIndexForXPosition(
-            offset_for_run, include_partial_glyphs);
-        return characters_so_far + index;
-      }
-      characters_so_far += runs_[i]->num_characters_;
-      current_x = next_x;
+  for (unsigned i = 0; i < runs_.size(); ++i) {
+    const RunInfo* run = runs_[i].get();
+    if (!run)
+      continue;
+    float next_x = current_x + run->width_;
+    float offset_for_run = target_x - current_x;
+    if (offset_for_run >= 0 && offset_for_run < run->width_) {
+      // The x value in question is within this script run.
+      run->CharacterIndexForXPosition(offset_for_run, result);
+      result->run_index = i;
+      result->characters_on_left_runs = characters_so_far;
+      result->origin_x += current_x;
+      DCHECK_LE(result->characters_on_left_runs + result->character_index,
+                NumCharacters());
+      return;
     }
+    characters_so_far += run->num_characters_;
+    current_x = next_x;
+  }
+
+  result->run_index = runs_.size();
+  result->characters_on_left_runs = characters_so_far;
+}
+
+unsigned ShapeResult::OffsetForPosition(float x) const {
+  GlyphIndexResult result;
+  OffsetForPosition(x, &result);
+  return IsLtr(Direction()) ? OffsetLtr(result) : OffsetRtl(result, x);
+}
+
+unsigned ShapeResult::OffsetForHitTest(float x) const {
+  GlyphIndexResult result;
+  OffsetForPosition(x, &result);
+  if (IsLtr(Direction())) {
+    if (result.IsInRun() && x > result.origin_x + result.advance / 2)
+      return OffsetRightLtr(result);
+    return OffsetLtr(result);
   }
+  if (result.IsInRun() && x <= result.origin_x + result.advance / 2)
+    return OffsetLeftRtl(result);
+  return OffsetRtl(result, x);
+}
 
-  return characters_so_far;
+unsigned ShapeResult::OffsetToFit(float x, TextDirection line_direction) const {
+  GlyphIndexResult result;
+  OffsetForPosition(x, &result);
+  if (IsLtr(line_direction)) {
+    return IsLtr(Direction()) ? OffsetLtr(result) : OffsetLeftRtl(result);
+  }
+  return IsRtl(Direction()) ? OffsetRtl(result, x) : OffsetRightLtr(result);
 }
 
 float ShapeResult::PositionForOffset(

third_party/blink/renderer/platform/fonts/shaping/shape_result.h

@@ -112,7 +112,20 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
   unsigned NextSafeToBreakOffset(unsigned offset) const;
   unsigned PreviousSafeToBreakOffset(unsigned offset) const;
 
-  unsigned OffsetForPosition(float target_x, bool include_partial_glyphs) const;
+  // Returns the offset whose (origin, origin+advance) contains |x|.
+  unsigned OffsetForPosition(float x) const;
+  // Returns the offset whose glyph boundary is nearest to |x|. Depends on
+  // whether |x| is on the left-half or the right-half of the glyph, it
+  // determines the left-boundary or the right-boundary, then computes the
+  // offset from the bidi direction.
+  unsigned OffsetForHitTest(float x) const;
+  // Returns the offset that can fit to between |x| and the left or the right
+  // edge. The side of the edge is determined by |line_direction|.
+  unsigned OffsetToFit(float x, TextDirection line_direction) const;
+  unsigned OffsetForPosition(float x, bool include_partial_glyphs) const {
+    return !include_partial_glyphs ? OffsetForPosition(x) : OffsetForHitTest(x);
+  }
+
   float PositionForOffset(unsigned offset,
                           AdjustMidCluster = AdjustMidCluster::kToEnd) const;
   LayoutUnit SnappedStartPositionForOffset(unsigned offset) const {
@@ -169,6 +182,33 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
     return base::AdoptRef(new ShapeResult(other));
   }
 
+  struct GlyphIndexResult {
+    STACK_ALLOCATED();
+
+    unsigned run_index = 0;
+    // The total number of characters of runs_[0..run_index - 1].
+    unsigned characters_on_left_runs = 0;
+    unsigned character_index = 0;
+    unsigned glyph_index = 0;
+    // |next_glyph_index| may not be |glyph_index| + 1 when a cluster is of
+    // multiple glyphs; i.e., ligatures or combining glyphs.
+    unsigned next_glyph_index = 0;
+    // The glyph origin of the glyph.
+    float origin_x = 0;
+    // The advance of the glyph.
+    float advance = 0;
+
+    // True if the position was found on a run. False otherwise.
+    bool IsInRun() const { return next_glyph_index; }
+  };
+
+  unsigned OffsetLtr(const GlyphIndexResult&) const;
+  unsigned OffsetRtl(const GlyphIndexResult&, float x) const;
+  unsigned OffsetRightLtr(const GlyphIndexResult&) const;
+  unsigned OffsetLeftRtl(const GlyphIndexResult&) const;
+
+  void OffsetForPosition(float target_x, GlyphIndexResult*) const;
+
   template <typename TextContainerType>
   void ApplySpacingImpl(ShapeResultSpacing<TextContainerType>&,
                         int text_start_offset = 0);

third_party/blink/renderer/platform/fonts/shaping/shape_result_inline_headers.h

@@ -87,7 +87,7 @@ struct ShapeResult::RunInfo {
   unsigned PreviousSafeToBreakOffset(unsigned) const;
   float XPositionForVisualOffset(unsigned, AdjustMidCluster) const;
   float XPositionForOffset(unsigned, AdjustMidCluster) const;
-  int CharacterIndexForXPosition(float, bool include_partial_glyphs) const;
+  void CharacterIndexForXPosition(float, GlyphIndexResult*) const;
   void SetGlyphAndPositions(unsigned index,
                             uint16_t glyph_id,
                             float advance,