Add support for shaping a substring to HarfBuzzShaper

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

Index: third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp
index 9b9ec1033bc296e83db31077fc5334de445f4dab..83ae9dd0a5ca55152f46a7d285111c7905264d76 100644
--- a/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp
+++ b/third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzShaperTest.cpp
@@ -44,7 +44,7 @@ static inline String to16Bit(const char* text, unsigned length) {
 TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsLatin) {
   String latinCommon = to16Bit("ABC DEF.", 8);
   HarfBuzzShaper shaper(latinCommon.characters16(), 8, TextDirection::kLtr);
-  RefPtr<ShapeResult> result = shaper.shapeResult(&font);
+  RefPtr<ShapeResult> result = shaper.shape(&font);
 
   ASSERT_EQ(1u, testInfo(result)->numberOfRunsForTesting());
   ASSERT_TRUE(
@@ -57,7 +57,7 @@ TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsLatin) {
 TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsLeadingCommon) {
   String leadingCommon = to16Bit("... test", 8);
   HarfBuzzShaper shaper(leadingCommon.characters16(), 8, TextDirection::kLtr);
-  RefPtr<ShapeResult> result = shaper.shapeResult(&font);
+  RefPtr<ShapeResult> result = shaper.shape(&font);
 
   ASSERT_EQ(1u, testInfo(result)->numberOfRunsForTesting());
   ASSERT_TRUE(
@@ -82,7 +82,7 @@ TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsUnicodeVariants) {
   };
   for (auto& test : testlist) {
     HarfBuzzShaper shaper(test.string, test.length, TextDirection::kLtr);
-    RefPtr<ShapeResult> result = shaper.shapeResult(&font);
+    RefPtr<ShapeResult> result = shaper.shape(&font);
 
     EXPECT_EQ(1u, testInfo(result)->numberOfRunsForTesting()) << test.name;
     ASSERT_TRUE(
@@ -111,7 +111,7 @@ TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsDevanagariCommon) {
   String devanagariCommonLatin(devanagariCommonString, 6);
   HarfBuzzShaper shaper(devanagariCommonLatin.characters16(), 6,
                         TextDirection::kLtr);
-  RefPtr<ShapeResult> result = shaper.shapeResult(&font);
+  RefPtr<ShapeResult> result = shaper.shape(&font);
 
   ASSERT_EQ(2u, testInfo(result)->numberOfRunsForTesting());
   ASSERT_TRUE(
@@ -131,7 +131,7 @@ TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsDevanagariCommonLatinCommon) {
   UChar devanagariCommonLatinString[] = {0x915, 0x94d, 0x930, 0x20,
                                          0x61,  0x62,  0x2E};
   HarfBuzzShaper shaper(devanagariCommonLatinString, 7, TextDirection::kLtr);
-  RefPtr<ShapeResult> result = shaper.shapeResult(&font);
+  RefPtr<ShapeResult> result = shaper.shape(&font);
 
   ASSERT_EQ(3u, testInfo(result)->numberOfRunsForTesting());
   ASSERT_TRUE(
@@ -156,7 +156,7 @@ TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsDevanagariCommonLatinCommon) {
 TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsArabicThaiHanLatin) {
   UChar mixedString[] = {0x628, 0x64A, 0x629, 0xE20, 0x65E5, 0x62};
   HarfBuzzShaper shaper(mixedString, 6, TextDirection::kLtr);
-  RefPtr<ShapeResult> result = shaper.shapeResult(&font);
+  RefPtr<ShapeResult> result = shaper.shape(&font);
 
   ASSERT_EQ(4u, testInfo(result)->numberOfRunsForTesting());
   ASSERT_TRUE(
@@ -187,19 +187,19 @@ TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsArabicThaiHanLatin) {
 TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsArabicThaiHanLatinTwice) {
   UChar mixedString[] = {0x628, 0x64A, 0x629, 0xE20, 0x65E5, 0x62};
   HarfBuzzShaper shaper(mixedString, 6, TextDirection::kLtr);
-  RefPtr<ShapeResult> result = shaper.shapeResult(&font);
+  RefPtr<ShapeResult> result = shaper.shape(&font);
   ASSERT_EQ(4u, testInfo(result)->numberOfRunsForTesting());
 
   // Shape again on the same shape object and check the number of runs.
   // Should be equal if no state was retained between shape calls.
-  RefPtr<ShapeResult> result2 = shaper.shapeResult(&font);
+  RefPtr<ShapeResult> result2 = shaper.shape(&font);
   ASSERT_EQ(4u, testInfo(result2)->numberOfRunsForTesting());
 }
 
 TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsArabic) {
   UChar arabicString[] = {0x628, 0x64A, 0x629};
   HarfBuzzShaper shaper(arabicString, 3, TextDirection::kRtl);
-  RefPtr<ShapeResult> result = shaper.shapeResult(&font);
+  RefPtr<ShapeResult> result = shaper.shape(&font);
 
   ASSERT_EQ(1u, testInfo(result)->numberOfRunsForTesting());
   ASSERT_TRUE(
@@ -209,4 +209,51 @@ TEST_F(HarfBuzzShaperTest, ResolveCandidateRunsArabic) {
   EXPECT_EQ(HB_SCRIPT_ARABIC, script);
 }
 
+TEST_F(HarfBuzzShaperTest, ShapeLatinSegment) {
+  String string = to16Bit("Hello World!", 12);

[drott, 2017/02/08 15:52:59]

Thinking about this more, see also my comment above: I don't think it's
generally safe to use arbitrary indices into a run here, but only safe-to-break
positions would work reliably. If you have for example a string beginning with
'e + combining cedilla + combining breve + remaining text' and use start index 1
or 2, the shaping for the orphaned combining marks will either fail or look
different from the e with them and procude a different shape result.

Perhaps we need something like an API that can tell layout code safe to break
positions in a whole paragraph or run?

+
+  HarfBuzzShaper shaper(string.characters16(), 12, TextDirection::kLtr);
+  RefPtr<ShapeResult> combined = shaper.shape(&font);
+  RefPtr<ShapeResult> first = shaper.shape(&font, 0, 6);
+  RefPtr<ShapeResult> second = shaper.shape(&font, 6, 11);
+  RefPtr<ShapeResult> third = shaper.shape(&font, 11, 12);
+
+  HarfBuzzShaper shaper2(string.characters16(), 6, TextDirection::kLtr);
+  RefPtr<ShapeResult> firstReference = shaper2.shape(&font);
+
+  HarfBuzzShaper shaper3(string.characters16() + 6, 5, TextDirection::kLtr);
+  RefPtr<ShapeResult> secondReference = shaper3.shape(&font);
+
+  HarfBuzzShaper shaper4(string.characters16() + 11, 1, TextDirection::kLtr);
+  RefPtr<ShapeResult> thirdReference = shaper4.shape(&font);
+
+  // Width of each segment should be the same when shaped using start and end
+  // offset as it is when shaping the three segments using separate shaper
+  // instances.
+  ASSERT_NEAR(firstReference->width(), first->width(), 0.1);

[drott, 2017/02/08 15:52:59]

I think the previous patchset had 0.01 here, was that not enough to make this
pass?

+  ASSERT_NEAR(secondReference->width(), second->width(), 0.1);
+  ASSERT_NEAR(thirdReference->width(), third->width(), 0.1);
+
+  // Width of shape results for the entire string should match the combined
+  // shape results from the three segments.
+  float totalWidth = first->width() + second->width() + third->width();
+  ASSERT_NEAR(combined->width(), totalWidth, 0.1);
+}
+
+// This test requires context-aware shaping which hasn't been implemented yet.
+// See crbug.com/689155
+TEST_F(HarfBuzzShaperTest, DISABLED_ShapeArabicWithContext) {
+  UChar arabicString[] = {0x647, 0x64A};
+  HarfBuzzShaper shaper(arabicString, 2, TextDirection::kRtl);
+
+  RefPtr<ShapeResult> combined = shaper.shape(&font);
+
+  RefPtr<ShapeResult> first = shaper.shape(&font, 0, 1);
+  RefPtr<ShapeResult> second = shaper.shape(&font, 1, 2);
+
+  // Combined width should be the same when shaping the two characters
+  // separately as when shaping them combined.
+  ASSERT_NEAR(combined->width(), first->width() + second->width(), 0.1);
+}
+
 }  // namespace blink