From 4a5e0222a3e729b093f80db4fd60657d9ae41aca Mon Sep 17 00:00:00 2001
From: Gary Qian <garyq@google.com>
Date: Tue, 1 Aug 2017 12:31:37 -0700
Subject: [PATCH] Change handling of large y coord in
 GetGlyphPositionAtCoordinate() to match blink.

Change-Id: I6db6d5e98a0ef63e29cc4aac9a3aa38dfb18e59d
---
 engine/src/flutter/src/paragraph.cc                 | 13 ++++++++-----
 engine/src/flutter/src/paragraph.h                  | 11 ++++++++---
 engine/src/flutter/tests/txt/paragraph_unittests.cc |  1 +
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/engine/src/flutter/src/paragraph.cc b/engine/src/flutter/src/paragraph.cc
index 6b9f8ca5ae5..ad82dbd4f78 100644
--- a/engine/src/flutter/src/paragraph.cc
+++ b/engine/src/flutter/src/paragraph.cc
@@ -416,7 +416,7 @@ void Paragraph::Layout(double width, bool force) {
                                             run.style.height;
       if (max_line_spacing < temp_line_spacing) {
         max_line_spacing = temp_line_spacing;
-        // Record the alphabetic_baseline_:
+        // Record the alphabetic_baseline_ and idegraphic_baseline_:
         if (lines_ == 0) {
           alphabetic_baseline_ = -metrics.fAscent * run.style.height;
           // TODO(garyq): Properly implement ideographic_baseline_.
@@ -466,8 +466,8 @@ void Paragraph::Layout(double width, bool force) {
   postprocess_line();
   if (line_width != 0)
     line_widths_.push_back(line_width);
-
-  line_heights_.push_back(FLT_MAX);
+  line_heights_.push_back((line_heights_.empty() ? 0 : line_heights_.back()) +
+                          max_line_spacing + max_descent);
   glyph_single_line_position_x.push_back(glyph_single_line_position_x.back() +
                                          prev_char_advance);
   glyph_single_line_position_x.push_back(FLT_MAX);
@@ -830,22 +830,25 @@ size_t Paragraph::GetGlyphPositionAtCoordinate(
   size_t offset = 0;
   size_t y_index = 1;
   size_t prev_count = 0;
-  for (y_index = 1; y_index < line_heights_.size(); ++y_index) {
+  for (y_index = 1; y_index < line_heights_.size() - 2; ++y_index) {
     if (dy < line_heights_[y_index]) {
       offset += prev_count;
+      prev_count = glyph_position_x_[y_index - 1].size() - 3;
       break;
     } else {
       offset += prev_count;
       prev_count = glyph_position_x_[y_index].size() - 3;
     }
   }
+  if (y_index == line_heights_.size() - 2)
+    offset += prev_count;
   prev_count = 0;
   for (size_t x_index = 1; x_index < glyph_position_x_[y_index].size() - 1;
        ++x_index) {
     if (dx < glyph_position_x_[y_index][x_index] -
                  (using_glyph_center_as_boundary
                       ? (glyph_position_x_[y_index][x_index] -
-                         glyph_position_x_[y_index][x_index -1]) /
+                         glyph_position_x_[y_index][x_index - 1]) /
                             2.0f
                       : 0)) {
       break;
diff --git a/engine/src/flutter/src/paragraph.h b/engine/src/flutter/src/paragraph.h
index 21bb7227160..d0cc8fd05eb 100644
--- a/engine/src/flutter/src/paragraph.h
+++ b/engine/src/flutter/src/paragraph.h
@@ -86,11 +86,11 @@ class Paragraph {
   double GetMaxWidth() const;
 
   // Distance from top of paragraph to the Alphabetic baseline of the first
-  // line.
+  // line. Used for alphabetic fonts (A-Z, a-z, greek, etc.)
   double GetAlphabeticBaseline() const;
 
   // Distance from top of paragraph to the Ideographic baseline of the first
-  // line.
+  // line. Used for ideographic fonts (Chinese, Japanese, Korean, etc.)
   double GetIdeographicBaseline() const;
 
   // Returns the total width covered by the paragraph without linebreaking.
@@ -98,6 +98,8 @@ class Paragraph {
 
   // Currently, calculated similarly to as GetLayoutWidth(), however this is not
   // nessecarily 100% correct in all cases.
+  //
+  // Returns the actual max width of the longest line after Layout().
   double GetMinIntrinsicWidth() const;
 
   // Returns a vector of bounding boxes that enclose all text between start and
@@ -108,7 +110,10 @@ class Paragraph {
   // with the top left corner as the origin, and +y direction as down.
   //
   // When using_glyph_center_as_boundary == true, coords to the + direction of
-  // the center x-position of the glyph will be considered as the next glyph.
+  // the center x-position of the glyph will be considered as the next glyph. A
+  // typical use-case for this is when the cursor is meant to be on either side
+  // of any given character. This allows the transition border to be middle of
+  // each character.
   size_t GetGlyphPositionAtCoordinate(
       double dx,
       double dy,
diff --git a/engine/src/flutter/tests/txt/paragraph_unittests.cc b/engine/src/flutter/tests/txt/paragraph_unittests.cc
index b28ef56ce97..7e422673f7d 100644
--- a/engine/src/flutter/tests/txt/paragraph_unittests.cc
+++ b/engine/src/flutter/tests/txt/paragraph_unittests.cc
@@ -922,6 +922,7 @@ TEST_F(RenderTest, GetGlyphPositionAtCoordinateParagraph) {
   ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(1, 270), 54ull);
   ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(35, 90), 19ull);
   ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(10000, 10000), 77ull);
+  ASSERT_EQ(paragraph->GetGlyphPositionAtCoordinate(85, 10000), 74ull);
 }
 
 TEST_F(RenderTest, GetRectsForRangeParagraph) {