Change CreateSubRun to copy entire array

This patch changes RunInfo::CreateSubRun() to copy the entire
array, then modify character_index. LayoutNG uses CreateSubRun
heavily, with much longer ShapeResult than the current layout
engine, and this function turned out to be hot.

perf_tests/layout/word-break-break-all.html gets ~10% faster,
~700ms to ~630ms.

Bug: 636993
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I869d1b90b1afe5cd84ad3b809ce8453f1f8feba4
Reviewed-on: https://chromium-review.googlesource.com/1088527
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565326}

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

@@ -563,6 +563,59 @@ TEST_F(HarfBuzzShaperTest, NegativeLetterSpacingToNegative) {
   EXPECT_GE(result->Bounds().MaxX(), char_width);
 }
 
+static struct GlyphDataRangeTestData {
+  const char16_t* text;
+  TextDirection direction;
+  unsigned run_index;
+  unsigned start_offset;
+  unsigned end_offset;
+  unsigned start_glyph;
+  unsigned end_glyph;
+} glyph_data_range_test_data[] = {
+    // Hebrew, taken from fast/text/selection/hebrew-selection.html
+    // The two code points form a grapheme cluster, which produces two glyphs.
+    // Character index array should be [0, 0].
+    {u"\u05E9\u05B0", TextDirection::kRtl, 0, 0, 1, 0, 2},
+#if !defined(OS_MACOSX)
+    // ZWJ tests taken from fast/text/international/zerowidthjoiner.html
+    // Character index array should be [6, 3, 3, 3, 0, 0, 0].
+    // Mac shapes differently and that glyph index expectations do not match.
+    {u"\u0639\u200D\u200D\u0639\u200D\u200D\u0639", TextDirection::kRtl, 0, 0,
+     1, 4, 7},
+    {u"\u0639\u200D\u200D\u0639\u200D\u200D\u0639", TextDirection::kRtl, 0, 2,
+     5, 1, 4},
+    {u"\u0639\u200D\u200D\u0639\u200D\u200D\u0639", TextDirection::kRtl, 0, 4,
+     7, 0, 1},
+#endif
+};
+
+std::ostream& operator<<(std::ostream& ostream,
+                         const GlyphDataRangeTestData& data) {
+  return ostream << data.text;
+}
+
+class GlyphDataRangeTest
+    : public HarfBuzzShaperTest,
+      public testing::WithParamInterface<GlyphDataRangeTestData> {};
+
+INSTANTIATE_TEST_CASE_P(HarfBuzzShaperTest,
+                        GlyphDataRangeTest,
+                        testing::ValuesIn(glyph_data_range_test_data));
+
+TEST_P(GlyphDataRangeTest, Data) {
+  auto data = GetParam();
+  String string(data.text);
+  HarfBuzzShaper shaper(string.Characters16(), string.length());
+  scoped_refptr<ShapeResult> result = shaper.Shape(&font, data.direction);
+
+  auto& run = TestInfo(result)->RunInfoForTesting(data.run_index);
+  auto glyphs = run.FindGlyphDataRange(data.start_offset, data.end_offset);
+  unsigned start_glyph = std::distance(run.glyph_data_.begin(), glyphs.begin);
+  EXPECT_EQ(data.start_glyph, start_glyph);
+  unsigned end_glyph = std::distance(run.glyph_data_.begin(), glyphs.end);
+  EXPECT_EQ(data.end_glyph, end_glyph);
+}
+
 static struct OffsetForPositionTestData {
   float position;
   unsigned offset_ltr;

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

@@ -42,6 +42,8 @@ namespace blink {
 
 class SimpleFontData;
 
+// This struct should be TriviallyCopyable so that std::copy() is equivalent to
+// memcpy.
 struct HarfBuzzRunGlyphData {
   DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
 
@@ -109,40 +111,67 @@ struct ShapeResult::RunInfo {
     return sizeof(this) + glyph_data_.size() * sizeof(HarfBuzzRunGlyphData);
   }
 
+  // Represents a range of HarfBuzzRunGlyphData. |begin| and |end| follow the
+  // iterator pattern; i.e., |begin| is lower or equal to |end| in the address
+  // space regardless of LTR/RTL. |begin| is inclusive, |end| is exclusive.
+  struct GlyphDataRange {
+    HarfBuzzRunGlyphData* begin;
+    HarfBuzzRunGlyphData* end;
+  };
+
+  // Find the range of HarfBuzzRunGlyphData for the specified character index
+  // range. This function uses binary search twice, hence O(2 log n).
+  GlyphDataRange FindGlyphDataRange(unsigned start_character_index,
+                                    unsigned end_character_index) {
+    const auto comparer = [](const HarfBuzzRunGlyphData& glyph_data,
+                             unsigned index) {
+      return glyph_data.character_index < index;
+    };
+    if (!Rtl()) {
+      HarfBuzzRunGlyphData* start_glyph =
+          std::lower_bound(glyph_data_.begin(), glyph_data_.end(),
+                           start_character_index, comparer);
+      if (UNLIKELY(start_glyph == glyph_data_.end()))
+        return {nullptr, nullptr};
+      HarfBuzzRunGlyphData* end_glyph = std::lower_bound(
+          start_glyph, glyph_data_.end(), end_character_index, comparer);
+      return {start_glyph, end_glyph};
+    }
+
+    // RTL needs to use reverse iterators because there maybe multiple glyphs
+    // for a character, and we want to find the first one in the logical order.
+    auto start_glyph =
+        std::lower_bound(glyph_data_.rbegin(), glyph_data_.rend(),
+                         start_character_index, comparer);
+    if (UNLIKELY(start_glyph == glyph_data_.rend()))
+      return {nullptr, nullptr};
+    auto end_glyph = std::lower_bound(start_glyph, glyph_data_.rend(),
+                                      end_character_index, comparer);
+    // Convert reverse iterators to pointers. Then increment to make |begin|
+    // inclusive and |end| exclusive.
+    return {&*end_glyph + 1, &*start_glyph + 1};
+  }
+
   // Creates a new RunInfo instance representing a subset of the current run.
   std::unique_ptr<RunInfo> CreateSubRun(unsigned start, unsigned end) {
     DCHECK(end > start);
     unsigned number_of_characters = std::min(end - start, num_characters_);
+    auto glyphs = FindGlyphDataRange(start, end);
+    unsigned number_of_glyphs = std::distance(glyphs.begin, glyphs.end);
 
-    // This is incorrect but is a resonable guess for the initial size of the
-    // glyph_data_ vector. It'll be resized once we've determined the number of
-    // glyphs in the sub run.
-    unsigned number_of_glyphs = number_of_characters;
     auto run = std::make_unique<RunInfo>(
         font_data_.get(), direction_, canvas_rotation_, script_,
         start_index_ + start, number_of_glyphs, number_of_characters);
 
-    number_of_glyphs = 0;
-    float total_advance = 0;
-    ForEachGlyphInRange(
-        0, start_index_ + start, start_index_ + end, 0,
-        [&](const HarfBuzzRunGlyphData& glyph_data, float, uint16_t) -> bool {
-          if (UNLIKELY(run->glyph_data_.size() == number_of_glyphs))
-            run->glyph_data_.resize(run->glyph_data_.size() * 2);
-          auto& sub_glyph = run->glyph_data_[number_of_glyphs++];
-          sub_glyph.glyph = glyph_data.glyph;
-          sub_glyph.character_index = glyph_data.character_index - start;
-          sub_glyph.safe_to_break_before = glyph_data.safe_to_break_before;
-          sub_glyph.advance = glyph_data.advance;
-          sub_glyph.offset = glyph_data.offset;
-          total_advance += glyph_data.advance;
-          return true;
-        });
+    static_assert(base::is_trivially_copyable<HarfBuzzRunGlyphData>::value,
+                  "HarfBuzzRunGlyphData should be trivially copyable");
+    std::copy(glyphs.begin, glyphs.end, run->glyph_data_.begin());
 
-    // Shrink glyph_data_ as the number of glyphs in the sub run is likely
-    // smaller than in the original.
-    DCHECK(run->glyph_data_.size() >= number_of_glyphs);
-    run->glyph_data_.resize(number_of_glyphs);
+    float total_advance = 0;
+    for (HarfBuzzRunGlyphData& glyph_data : run->glyph_data_) {
+      glyph_data.character_index -= start;
+      total_advance += glyph_data.advance;
+    }
 
     run->width_ = total_advance;
     run->num_characters_ = number_of_characters;

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

@@ -13,6 +13,11 @@ unsigned ShapeResultTestInfo::NumberOfRunsForTesting() const {
   return runs_.size();
 }
 
+ShapeResult::RunInfo& ShapeResultTestInfo::RunInfoForTesting(
+    unsigned run_index) const {
+  return *runs_[run_index];
+}
+
 bool ShapeResultTestInfo::RunInfoForTesting(unsigned run_index,
                                             unsigned& start_index,
                                             unsigned& num_characters,

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

@@ -15,6 +15,7 @@ namespace blink {
 class PLATFORM_EXPORT ShapeResultTestInfo : public ShapeResult {
  public:
   unsigned NumberOfRunsForTesting() const;
+  ShapeResult::RunInfo& RunInfoForTesting(unsigned run_index) const;
   bool RunInfoForTesting(unsigned run_index,
                          unsigned& start_index,
                          unsigned& num_glyphs,