RenderTextHarfBuzz: Allow mid-glyph cursors in multi-grapheme clusters

ui/gfx/render_text_unittest.cc

 // Copyright (c) 2012 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 "ui/gfx/render_text.h"
 
 #include <algorithm>
 
 #include "base/format_macros.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 1906 matching lines @@
 
   render_text->SetText(WideToUTF16(L"x \x25B6 y"));
   render_text->EnsureLayout();
   ASSERT_EQ(3U, render_text->runs_.size());
   EXPECT_EQ(Range(0, 2), render_text->runs_[0]->range);
   EXPECT_EQ(Range(2, 3), render_text->runs_[1]->range);
   EXPECT_EQ(Range(3, 5), render_text->runs_[2]->range);
 }
 #endif  // defined(OS_WIN)
 
-TEST_F(RenderTextTest, HarfBuzz_CharToGlyph) {
+TEST_F(RenderTextTest, HarfBuzz_Clusters) {

[msw, 2014/06/29 22:29:10]

nit: can you add high-level comments for each of these tests?

[ckocagil, 2014/07/24 21:46:56]

Done.

   struct {
     uint32 glyph_to_char[4];
-    size_t char_to_glyph_expected[4];
-    Range char_range_to_glyph_range_expected[4];
+    Range chars[4];
+    Range glyphs[4];
     bool is_rtl;
   } cases[] = {
     { // From string "A B C D" to glyphs "a b c d".
       { 0, 1, 2, 3 },
-      { 0, 1, 2, 3 },
+      { Range(0, 1), Range(1, 2), Range(2, 3), Range(3, 4) },
       { Range(0, 1), Range(1, 2), Range(2, 3), Range(3, 4) },
       false
     },
-    { // From string "A B C D" to glyphs "d b c a".
+    { // From string "A B C D" to glyphs "d c b a".
       { 3, 2, 1, 0 },
-      { 3, 2, 1, 0 },
+      { Range(0, 1), Range(1, 2), Range(2, 3), Range(3, 4) },
       { Range(3, 4), Range(2, 3), Range(1, 2), Range(0, 1) },
       true
     },
     { // From string "A B C D" to glyphs "ab c c d".
       { 0, 2, 2, 3 },
-      { 0, 0, 1, 3 },
+      { Range(0, 2), Range(0, 2), Range(2, 3), Range(3, 4) },
       { Range(0, 1), Range(0, 1), Range(1, 3), Range(3, 4) },
       false
     },
     { // From string "A B C D" to glyphs "d c c ba".
       { 3, 2, 2, 0 },
-      { 3, 3, 1, 0 },
+      { Range(0, 2), Range(0, 2), Range(2, 3), Range(3, 4) },
       { Range(3, 4), Range(3, 4), Range(1, 3), Range(0, 1) },
       true
     },
   };
 
   internal::TextRunHarfBuzz run;
   run.range = Range(0, 4);
   run.glyph_count = 4;
-  run.glyph_to_char.reset(new uint32[4]);
+  run.glyph_to_char.resize(4);
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
     std::copy(cases[i].glyph_to_char, cases[i].glyph_to_char + 4,
-              run.glyph_to_char.get());
+              run.glyph_to_char.begin());
     run.is_rtl = cases[i].is_rtl;
+
     for (size_t j = 0; j < 4; ++j) {
       SCOPED_TRACE(base::StringPrintf("Case %" PRIuS ", char %" PRIuS, i, j));
-      EXPECT_EQ(cases[i].char_to_glyph_expected[j], run.CharToGlyph(j));
-      EXPECT_EQ(cases[i].char_range_to_glyph_range_expected[j],
-                run.CharRangeToGlyphRange(Range(j, j + 1)));
+      Range chars;
+      Range glyphs;
+      run.GetClusterAt(j, &chars, &glyphs);
+      EXPECT_EQ(cases[i].chars[j], chars);
+      EXPECT_EQ(cases[i].glyphs[j], glyphs);
+      EXPECT_EQ(cases[i].glyphs[j], run.CharRangeToGlyphRange(chars));
     }
   }
 }
+
+TEST_F(RenderTextTest, HarfBuzz_SubglyphGrapheme) {
+  struct {
+    uint32 glyph_to_char[2];
+    Range bounds[4];
+    bool is_rtl;
+  } cases[] = {
+    { // From string "A B C D" to glyphs "a bcd".

[msw, 2014/06/29 22:29:10]

Are there still cases where we'd expect a cursor not to be placed between
characters of a multi-char grapheme, like in the old comment "\x0915\x093f -
(ki) - one of many Devanagari biconsonantal conjuncts."? If so, can we test
something like that?

[ckocagil, 2014/07/24 21:46:56]

Done, see HarfBuzz_SubglyphGraphemeCases.

+      { 0, 1 },
+      { Range(0, 10), Range(10, 13), Range(13, 17), Range(17, 20) },
+      false
+    },
+    { // From string "A B C D" to glyphs "ab cd".
+      { 0, 2 },
+      { Range(0, 5), Range(5, 10), Range(10, 15), Range(15, 20) },
+      false
+    },
+    { // From string "A B C D" to glyphs "dcb a".
+      { 1, 0 },
+      { Range(10, 20), Range(7, 10), Range(3, 7), Range(0, 3) },
+      true
+    },
+  };
+
+  internal::TextRunHarfBuzz run;
+  run.range = Range(0, 4);
+  run.glyph_count = 2;
+  run.glyph_to_char.resize(2);
+  run.positions.reset(new SkPoint[4]);
+  run.width = 20;
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i) {
+    std::copy(cases[i].glyph_to_char, cases[i].glyph_to_char + 2,
+              run.glyph_to_char.begin());
+    run.is_rtl = cases[i].is_rtl;
+    for (int j = 0; j < 2; ++j)
+      run.positions[j].set(j * 10, 0);
+
+    for (size_t j = 0; j < 4; ++j) {
+      SCOPED_TRACE(base::StringPrintf("Case %" PRIuS ", char %" PRIuS, i, j));
+      EXPECT_EQ(cases[i].bounds[j], run.GetGraphemeBounds(L"abcd", j));
+    }
+  }
+}
 
 TEST_F(RenderTextTest, HarfBuzz_RunDirection) {
   RenderTextHarfBuzz render_text;
   const base::string16 mixed =
       WideToUTF16(L"\x05D0\x05D1" L"1234" L"\x05D2\x05D3");
   render_text.SetText(mixed);
   render_text.EnsureLayout();
   ASSERT_EQ(3U, render_text.runs_.size());
   EXPECT_TRUE(render_text.runs_[0]->is_rtl);
   EXPECT_FALSE(render_text.runs_[1]->is_rtl);
@@ skipping 47 matching lines @@
   for (size_t i = 0; i < arraysize(kTestStrings); ++i) {
     render_text->SetText(WideToUTF16(kTestStrings[i]));
     render_text->EnsureLayout();
 
     for (size_t j = 0; j < render_text->text().length(); ++j)
       EXPECT_FALSE(render_text->GetGlyphBounds(j).is_empty());
   }
 }
 
 }  // namespace gfx