Change representation of collection of HarfBuzzRunGlyphData::offset

This patch introduces |ShapeResult::RunInfo::GlyphDataCollection| to represent a
collection of |HarfBuzzRunGlyphData| with optional offsets to reduce memory
usage, because of glyph offsets are often zero, particularly Latin runs.

This is done by following observations:
 - Most of |HarfBuzzRunGlyphData::offset| are zero == allocates collection of
 offset when there are non-zero offsets.
 - Collection of |HarfBuzzRunGlyphData::offset| is accessed per run == checks
 zero or non-zero collection before accessing collection

Bug: 965564
Change-Id: I345260a4e6d752bec9abbe21d32a2e31b07966f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627274
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664751}

third_party/blink/renderer/platform/BUILD.gn

@@ -1660,6 +1660,7 @@ jumbo_source_set("blink_platform_unittests_sources") {
     "fonts/shaping/harfbuzz_shaper_test.cc",
     "fonts/shaping/run_segmenter_test.cc",
     "fonts/shaping/shape_result_bloberizer_test.cc",
+    "fonts/shaping/shape_result_run_info_test.cc",
     "fonts/shaping/shape_result_test.cc",
     "fonts/shaping/shape_result_view_test.cc",
     "fonts/shaping/shaping_line_breaker_test.cc",

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

@@ -55,8 +55,8 @@ struct GlyphBoundsAccumulator {
 
   // Unite a glyph bounding box to |bounds|.
   template <bool is_horizontal_run>
-  void Unite(const HarfBuzzRunGlyphData& glyph_data,
-             FloatRect bounds_for_glyph) {
+  void Unite(FloatRect bounds_for_glyph,
+             ShapeResult::GlyphOffset glyph_offset) {
     if (UNLIKELY(bounds_for_glyph.IsEmpty()))
       return;
 
@@ -67,17 +67,17 @@ struct GlyphBoundsAccumulator {
       bounds_for_glyph.SetX(bounds_for_glyph.X() + origin);
     else
       bounds_for_glyph.SetY(bounds_for_glyph.Y() + origin);
-    bounds_for_glyph.Move(glyph_data.offset);
+    bounds_for_glyph.Move(glyph_offset);
 
     bounds.Unite(bounds_for_glyph);
   }
 
   // Non-template version of |Unite()|, see above.
   void Unite(bool is_horizontal_run,
-             const HarfBuzzRunGlyphData& glyph,
-             FloatRect bounds_for_glyph) {
-    is_horizontal_run ? Unite<true>(glyph, bounds_for_glyph)
-                      : Unite<false>(glyph, bounds_for_glyph);
+             FloatRect bounds_for_glyph,
+             ShapeResult::GlyphOffset glyph_offset) {
+    is_horizontal_run ? Unite<true>(bounds_for_glyph, glyph_offset)
+                      : Unite<false>(bounds_for_glyph, glyph_offset);
   }
 
   // Convert vertical run glyph bounding box to logical. Horizontal runs do not

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

@@ -333,6 +333,7 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
   String ToString() const;
   void ToString(StringBuilder*) const;
 
+  class GlyphOffset;
   struct RunInfo;
   RunInfo* InsertRunForTesting(unsigned start_index,
                                unsigned num_characters,
@@ -455,7 +456,7 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
   void InsertRun(scoped_refptr<ShapeResult::RunInfo>);
   void ReorderRtlRuns(unsigned run_size_before);
 
-  template <bool is_horizontal_run>
+  template <bool is_horizontal_run, bool has_non_zero_glyph_offsets>
   void ComputeRunInkBounds(const ShapeResult::RunInfo&,
                            float run_advance,
                            FloatRect* ink_bounds) const;
@@ -487,11 +488,27 @@ class PLATFORM_EXPORT ShapeResult : public RefCounted<ShapeResult> {
   // Tracks whether any runs contain glyphs with a y-offset != 0.
   unsigned has_vertical_offsets_ : 1;
 
+ private:
   friend class HarfBuzzShaper;
   friend class ShapeResultBuffer;
   friend class ShapeResultBloberizer;
   friend class ShapeResultView;
   friend class ShapeResultTest;
+
+  template <bool has_non_zero_glyph_offsets>
+  float ForEachGlyphImpl(float initial_advance,
+                         GlyphCallback,
+                         void* context,
+                         const RunInfo& run) const;
+
+  template <bool has_non_zero_glyph_offsets>
+  float ForEachGlyphImpl(float initial_advance,
+                         unsigned from,
+                         unsigned to,
+                         unsigned index_offset,
+                         GlyphCallback,
+                         void* context,
+                         const RunInfo& run) const;
 };
 
 PLATFORM_EXPORT std::ostream& operator<<(std::ostream&, const ShapeResult&);

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

+// Copyright (c) 2019 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 "third_party/blink/renderer/platform/fonts/shaping/shape_result_inline_headers.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+class ShapeResultRunInfoTest : public testing::Test {};
+
+TEST_F(ShapeResultRunInfoTest, CopyConstructor) {
+  ShapeResult::RunInfo::GlyphOffsetArray offsets(2);
+
+  ShapeResult::RunInfo::GlyphOffsetArray offsets2(offsets);
+  EXPECT_FALSE(offsets2.HasStorage());
+
+  offsets.SetAt(0, ShapeResult::GlyphOffset(1, 1));
+  ShapeResult::RunInfo::GlyphOffsetArray offsets3(offsets);
+  ASSERT_TRUE(offsets3.HasStorage());
+  EXPECT_EQ(ShapeResult::GlyphOffset(1, 1), offsets3.GetStorage()[0]);
+}
+
+TEST_F(ShapeResultRunInfoTest, CopyFromRange) {
+  ShapeResult::RunInfo::GlyphOffsetArray offsets(2);
+  HarfBuzzRunGlyphData glyhp_data[2];
+
+  ShapeResult::RunInfo::GlyphOffsetArray offsets2(2);
+  offsets2.CopyFromRange({&glyhp_data[0], &glyhp_data[2], nullptr});
+  EXPECT_FALSE(offsets2.HasStorage());
+
+  offsets.SetAt(0, ShapeResult::GlyphOffset(1, 1));
+  ASSERT_TRUE(offsets.HasStorage());
+
+  ShapeResult::RunInfo::GlyphOffsetArray offsets3(2);
+  offsets3.CopyFromRange(
+      {&glyhp_data[0], &glyhp_data[2], offsets.GetStorage()});
+  ASSERT_TRUE(offsets3.HasStorage());
+  EXPECT_EQ(ShapeResult::GlyphOffset(1, 1), offsets3.GetStorage()[0]);
+}
+
+TEST_F(ShapeResultRunInfoTest, GlyphOffsetArrayReverse) {
+  ShapeResult::RunInfo::GlyphOffsetArray offsets(2);
+
+  offsets.Reverse();
+  EXPECT_FALSE(offsets.HasStorage());
+
+  offsets.SetAt(0, ShapeResult::GlyphOffset(1, 1));
+  ASSERT_TRUE(offsets.HasStorage());
+  offsets.Reverse();
+  EXPECT_EQ(ShapeResult::GlyphOffset(), offsets.GetStorage()[0]);
+  EXPECT_EQ(ShapeResult::GlyphOffset(1, 1), offsets.GetStorage()[1]);
+}
+
+TEST_F(ShapeResultRunInfoTest, GlyphOffsetArraySetAddOffsetHeightAt) {
+  ShapeResult::RunInfo::GlyphOffsetArray offsets(2);
+
+  offsets.AddHeightAt(1, 1.5f);
+  ASSERT_TRUE(offsets.HasStorage());
+  EXPECT_EQ(ShapeResult::GlyphOffset(0, 1.5f), offsets.GetStorage()[1]);
+
+  offsets.AddHeightAt(1, 2.0f);
+  ASSERT_TRUE(offsets.HasStorage());
+  EXPECT_EQ(ShapeResult::GlyphOffset(0, 3.5f), offsets.GetStorage()[1]);
+}
+
+TEST_F(ShapeResultRunInfoTest, GlyphOffsetArraySetAddOffsetWidthAt) {
+  ShapeResult::RunInfo::GlyphOffsetArray offsets(2);
+
+  offsets.AddWidthAt(1, 1.5f);
+  ASSERT_TRUE(offsets.HasStorage());
+  EXPECT_EQ(ShapeResult::GlyphOffset(1.5f, 0), offsets.GetStorage()[1]);
+
+  offsets.AddWidthAt(1, 2.0f);
+  ASSERT_TRUE(offsets.HasStorage());
+  EXPECT_EQ(ShapeResult::GlyphOffset(3.5f, 0), offsets.GetStorage()[1]);
+}
+
+TEST_F(ShapeResultRunInfoTest, GlyphOffsetArraySetAt) {
+  ShapeResult::RunInfo::GlyphOffsetArray offsets(2);
+
+  offsets.SetAt(0, ShapeResult::GlyphOffset());
+  EXPECT_FALSE(offsets.HasStorage());
+
+  offsets.SetAt(1, ShapeResult::GlyphOffset(1, 1));
+  EXPECT_TRUE(offsets.HasStorage());
+}
+
+TEST_F(ShapeResultRunInfoTest, GlyphOffsetArrayShrink) {
+  ShapeResult::RunInfo::GlyphOffsetArray offsets(3);
+
+  offsets.Shrink(2);
+  EXPECT_FALSE(offsets.HasStorage());
+
+  offsets.SetAt(0, ShapeResult::GlyphOffset(1, 1));
+  ASSERT_TRUE(offsets.HasStorage());
+
+  offsets.Shrink(1);
+  ASSERT_TRUE(offsets.HasStorage());
+  EXPECT_EQ(ShapeResult::GlyphOffset(1, 1), offsets.GetStorage()[0]);
+}
+
+}  // namespace blink

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

@@ -2,13 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "third_party/blink/renderer/platform/fonts/shaping/shape_result.h"
+#include "third_party/blink/renderer/platform/fonts/shaping/shape_result_inline_headers.h"
 
 #include "base/containers/span.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/platform/fonts/font.h"
 #include "third_party/blink/renderer/platform/fonts/font_cache.h"
 #include "third_party/blink/renderer/platform/fonts/font_test_utilities.h"
+#include "third_party/blink/renderer/platform/fonts/shaping/shape_result_spacing.h"
 #include "third_party/blink/renderer/platform/fonts/shaping/shape_result_test_info.h"
 #include "third_party/blink/renderer/platform/testing/font_test_helpers.h"
 #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
@@ -35,6 +36,14 @@ class ShapeResultTest : public testing::Test {
   void TestCopyRangesLatin(const ShapeResult*) const;
   void TestCopyRangesArabic(const ShapeResult*) const;
 
+  static bool HasNonZeroGlyphOffsets(const ShapeResult& result) {
+    for (const auto& run : result.RunsOrParts()) {
+      if (run->glyph_data_.HasNonZeroOffsets())
+        return true;
+    }
+    return false;
+  }
+
   // Release the ShapeResults held inside an array of ShapeResult::ShapeRange
   // instances.
   static void ReleaseShapeRange(base::span<ShapeResult::ShapeRange> ranges) {
@@ -243,4 +252,23 @@ TEST_F(ShapeResultTest, CopyRangeArabicMultiRun) {
   TestCopyRangesArabic(result.get());
 }
 
+TEST_F(ShapeResultTest, ComputeInkBoundsWithZeroOffset) {
+  String string(u"abc");
+  HarfBuzzShaper shaper(string);
+  auto result = shaper.Shape(&font, TextDirection::kLtr);
+  EXPECT_FALSE(HasNonZeroGlyphOffsets(*result));
+  EXPECT_FALSE(result->ComputeInkBounds().IsZero());
+}
+
+// TDOO(yosin): We should use a font including U+0A81 or other code point
+// having non-zero glyph offset.
+TEST_F(ShapeResultTest, DISABLED_ComputeInkBoundsWithNonZeroOffset) {
+  // U+0A81 has non-zero glyph offset
+  String string(u"xy\u0A81z");
+  HarfBuzzShaper shaper(string);
+  auto result = shaper.Shape(&font, TextDirection::kLtr);
+  ASSERT_TRUE(HasNonZeroGlyphOffsets(*result));
+  EXPECT_FALSE(result->ComputeInkBounds().IsZero());
+}
+
 }  // namespace blink

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

@@ -43,6 +43,13 @@ struct ShapeResultView::RunInfoPart {
   const HarfBuzzRunGlyphData& GlyphAt(unsigned index) const {
     return *(range_.begin + index);
   }
+  template <bool has_non_zero_glyph_offsets>
+  ShapeResult::RunInfo::GlyphOffsetArray::iterator<has_non_zero_glyph_offsets>
+  GetGlyphOffsets() const {
+    return ShapeResult::RunInfo::GlyphOffsetArray::iterator<
+        has_non_zero_glyph_offsets>(range_);
+  }
+  bool HasGlyphOffsets() const { return range_.offsets; }
   // The end character index of |this| without considering offsets in
   // |ShapeResultView|. This is analogous to:
   //   GlyphAt(Rtl() ? -1 : NumGlyphs()).character_index
@@ -134,8 +141,7 @@ scoped_refptr<ShapeResult> ShapeResultView::CreateShapeResult() const {
         part->run_->font_data_.get(), part->run_->direction_,
         part->run_->canvas_rotation_, part->run_->script_, part->start_index_,
         part->NumGlyphs(), part->num_characters_);
-    std::copy(part->range_.begin, part->range_.end,
-              new_run->glyph_data_.begin());
+    new_run->glyph_data_.CopyFromRange(part->range_);
     for (HarfBuzzRunGlyphData& glyph_data : new_run->glyph_data_) {
       glyph_data.character_index -= part->offset_;
     }
@@ -343,26 +349,91 @@ void ShapeResultView::FallbackFonts(
   }
 }
 
+template <bool has_non_zero_glyph_offsets>
+float ShapeResultView::ForEachGlyphImpl(float initial_advance,
+                                        GlyphCallback glyph_callback,
+                                        void* context,
+                                        const RunInfoPart& part) const {
+  auto glyph_offsets = part.GetGlyphOffsets<has_non_zero_glyph_offsets>();
+  const auto& run = part.run_;
+  auto total_advance = initial_advance;
+  bool is_horizontal = HB_DIRECTION_IS_HORIZONTAL(run->direction_);
+  const SimpleFontData* font_data = run->font_data_.get();
+  const unsigned character_index_offset_for_glyph_data =
+      CharacterIndexOffsetForGlyphData(part);
+  for (const auto& glyph_data : part) {
+    unsigned character_index =
+        glyph_data.character_index + character_index_offset_for_glyph_data;
+    glyph_callback(context, character_index, glyph_data.glyph, *glyph_offsets,
+                   total_advance, is_horizontal, run->canvas_rotation_,
+                   font_data);
+    total_advance += glyph_data.advance;
+    ++glyph_offsets;
+  }
+  return total_advance;
+}
+
 float ShapeResultView::ForEachGlyph(float initial_advance,
                                     GlyphCallback glyph_callback,
                                     void* context) const {
   auto total_advance = initial_advance;
   for (const auto& part : parts_) {
-    const auto& run = part->run_;
-    bool is_horizontal = HB_DIRECTION_IS_HORIZONTAL(run->direction_);
-    const SimpleFontData* font_data = run->font_data_.get();
-    const unsigned character_index_offset_for_glyph_data =
-        CharacterIndexOffsetForGlyphData(*part);
-    for (const auto& glyph_data : *part) {
+    if (part->HasGlyphOffsets()) {
+      total_advance =
+          ForEachGlyphImpl<true>(total_advance, glyph_callback, context, *part);
+    } else {
+      total_advance = ForEachGlyphImpl<false>(total_advance, glyph_callback,
+                                              context, *part);
+    }
+  }
+  return total_advance;
+}
+
+template <bool has_non_zero_glyph_offsets>
+float ShapeResultView::ForEachGlyphImpl(float initial_advance,
+                                        unsigned from,
+                                        unsigned to,
+                                        unsigned index_offset,
+                                        GlyphCallback glyph_callback,
+                                        void* context,
+                                        const RunInfoPart& part) const {
+  auto glyph_offsets = part.GetGlyphOffsets<has_non_zero_glyph_offsets>();
+  auto total_advance = initial_advance;
+  const auto& run = part.run_;
+  bool is_horizontal = HB_DIRECTION_IS_HORIZONTAL(run->direction_);
+  const SimpleFontData* font_data = run->font_data_.get();
+  const unsigned character_index_offset_for_glyph_data =
+      CharacterIndexOffsetForGlyphData(part);
+  if (!run->Rtl()) {  // Left-to-right
+    for (const auto& glyph_data : part) {
       unsigned character_index =
           glyph_data.character_index + character_index_offset_for_glyph_data;
-      glyph_callback(context, character_index, glyph_data.glyph,
-                     glyph_data.offset, total_advance, is_horizontal,
-                     run->canvas_rotation_, font_data);
+      if (character_index >= to)
+        break;
+      if (character_index >= from) {
+        glyph_callback(context, character_index, glyph_data.glyph,
+                       *glyph_offsets, total_advance, is_horizontal,
+                       run->canvas_rotation_, font_data);
+      }
       total_advance += glyph_data.advance;
+      ++glyph_offsets;
     }
-  }
 
+  } else {  // Right-to-left
+    for (const auto& glyph_data : part) {
+      unsigned character_index =
+          glyph_data.character_index + character_index_offset_for_glyph_data;
+      if (character_index < from)
+        break;
+      if (character_index < to) {
+        glyph_callback(context, character_index, glyph_data.glyph,
+                       *glyph_offsets, total_advance, is_horizontal,
+                       run->canvas_rotation_, font_data);
+      }
+      total_advance += glyph_data.advance;
+      ++glyph_offsets;
+    }
+  }
   return total_advance;
 }
 
@@ -375,39 +446,14 @@ float ShapeResultView::ForEachGlyph(float initial_advance,
   auto total_advance = initial_advance;
 
   for (const auto& part : parts_) {
-    const auto& run = part->run_;
-    bool is_horizontal = HB_DIRECTION_IS_HORIZONTAL(run->direction_);
-    const SimpleFontData* font_data = run->font_data_.get();
-    const unsigned character_index_offset_for_glyph_data =
-        CharacterIndexOffsetForGlyphData(*part);
-
-    if (!run->Rtl()) {  // Left-to-right
-      for (const auto& glyph_data : *part) {
-        unsigned character_index =
-            glyph_data.character_index + character_index_offset_for_glyph_data;
-        if (character_index >= to)
-          break;
-        if (character_index >= from) {
-          glyph_callback(context, character_index, glyph_data.glyph,
-                         glyph_data.offset, total_advance, is_horizontal,
-                         run->canvas_rotation_, font_data);
-        }
-        total_advance += glyph_data.advance;
-      }
-
-    } else {  // Right-to-left
-      for (const auto& glyph_data : *part) {
-        unsigned character_index =
-            glyph_data.character_index + character_index_offset_for_glyph_data;
-        if (character_index < from)
-          break;
-        if (character_index < to) {
-          glyph_callback(context, character_index, glyph_data.glyph,
-                         glyph_data.offset, total_advance, is_horizontal,
-                         run->canvas_rotation_, font_data);
-        }
-        total_advance += glyph_data.advance;
-      }
+    if (part->HasGlyphOffsets()) {
+      total_advance =
+          ForEachGlyphImpl<true>(total_advance, from, to, index_offset,
+                                 glyph_callback, context, *part);
+    } else {
+      total_advance =
+          ForEachGlyphImpl<false>(total_advance, from, to, index_offset,
+                                  glyph_callback, context, *part);
     }
   }
   return total_advance;
@@ -499,7 +545,7 @@ float ShapeResultView::ForEachGraphemeClusters(const StringView& text,
   return advance_so_far;
 }
 
-template <bool is_horizontal_run>
+template <bool is_horizontal_run, bool has_non_zero_glyph_offsets>
 void ShapeResultView::ComputePartInkBounds(
     const ShapeResultView::RunInfoPart& part,
     float run_advance,
@@ -510,6 +556,7 @@ void ShapeResultView::ComputePartInkBounds(
   // https://bugs.chromium.org/p/skia/issues/detail?id=5328, and the cost to
   // prepare batching, which is normally much less than the benefit of
   // batching, is not ignorable unfortunately.
+  auto glyph_offsets = part.GetGlyphOffsets<has_non_zero_glyph_offsets>();
   const SimpleFontData& current_font_data = *part.run_->font_data_;
   unsigned num_glyphs = part.NumGlyphs();
 #if !defined(OS_MACOSX)
@@ -529,8 +576,9 @@ void ShapeResultView::ComputePartInkBounds(
 #else
     FloatRect glyph_bounds(bounds_list[j]);
 #endif
-    bounds.Unite<is_horizontal_run>(glyph_data, glyph_bounds);
+    bounds.Unite<is_horizontal_run>(glyph_bounds, *glyph_offsets);
     bounds.origin += glyph_data.advance;
+    ++glyph_offsets;
   }
 
   if (!is_horizontal_run)
@@ -543,10 +591,22 @@ FloatRect ShapeResultView::ComputeInkBounds() const {
 
   float run_advance = 0.0f;
   for (const auto& part : parts_) {
-    if (part->IsHorizontal())
-      ComputePartInkBounds<true>(*part.get(), run_advance, &ink_bounds);
-    else
-      ComputePartInkBounds<false>(*part.get(), run_advance, &ink_bounds);
+    if (part->HasGlyphOffsets()) {
+      if (part->IsHorizontal()) {
+        ComputePartInkBounds<true, true>(*part.get(), run_advance, &ink_bounds);
+      } else {
+        ComputePartInkBounds<false, true>(*part.get(), run_advance,
+                                          &ink_bounds);
+      }
+    } else {
+      if (part->IsHorizontal()) {
+        ComputePartInkBounds<true, false>(*part.get(), run_advance,
+                                          &ink_bounds);
+      } else {
+        ComputePartInkBounds<false, false>(*part.get(), run_advance,
+                                           &ink_bounds);
+      }
+    }
     run_advance += part->Width();
   }
 

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

@@ -157,7 +157,7 @@ class PLATFORM_EXPORT ShapeResultView final
 
   unsigned CharacterIndexOffsetForGlyphData(const RunInfoPart&) const;
 
-  template <bool is_horizontal_run>
+  template <bool is_horizontal_run, bool has_glyph_offsets>
   void ComputePartInkBounds(const ShapeResultView::RunInfoPart&,
                             float run_advance,
                             FloatRect* ink_bounds) const;
@@ -189,7 +189,23 @@ class PLATFORM_EXPORT ShapeResultView final
   float width_;
   Vector<std::unique_ptr<RunInfoPart>, 4> parts_;
 
+ private:
   friend class ShapeResult;
+
+  template <bool has_glyph_offsets>
+  float ForEachGlyphImpl(float initial_advance,
+                         GlyphCallback,
+                         void* context,
+                         const RunInfoPart& part) const;
+
+  template <bool has_glyph_offsets>
+  float ForEachGlyphImpl(float initial_advance,
+                         unsigned from,
+                         unsigned to,
+                         unsigned index_offset,
+                         GlyphCallback,
+                         void* context,
+                         const RunInfoPart& part) const;
 };
 
 }  // namespace blink