Fix the Char to Glyph mapping logic in RenderTextHarfBuzz

BUG=321868
R=msw@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/300623003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275754 0039d316-1c4b-4281-b951-d872f2087c98

ui/gfx/render_text_harfbuzz.cc

@@ -393,11 +393,11 @@ size_t TextRunHarfBuzz::CharToGlyph(size_t pos) const {
   DCHECK(range.start() <= pos && pos < range.end());
 
   if (!is_rtl) {
-    for (size_t i = 0; i < glyph_count - 1; ++i) {
-      if (pos < glyph_to_char[i + 1])
-        return i;
-    }
-    return glyph_count - 1;
+    size_t cluster_start = 0;
+    for (size_t i = 1; i < glyph_count && pos >= glyph_to_char[i]; ++i)
+      if (glyph_to_char[i] != glyph_to_char[i - 1])
+        cluster_start = i;
+    return cluster_start;
   }
 
   for (size_t i = 0; i < glyph_count; ++i) {
@@ -408,16 +408,34 @@ size_t TextRunHarfBuzz::CharToGlyph(size_t pos) const {
   return 0;
 }
 
-Range TextRunHarfBuzz::CharRangeToGlyphRange(const Range& range) const {
-  DCHECK(range.Contains(range));
-  DCHECK(!range.is_reversed());
-  DCHECK(!range.is_empty());
+Range TextRunHarfBuzz::CharRangeToGlyphRange(const Range& char_range) const {
+  DCHECK(range.Contains(char_range));
+  DCHECK(!char_range.is_reversed());
+  DCHECK(!char_range.is_empty());
+
+  size_t first = 0;
+  size_t last = 0;
+
+  if (is_rtl) {
+    // For RTL runs, we subtract 1 from |char_range| to get the leading edges.
+    last = CharToGlyph(char_range.end() - 1);
+    // Loop until we find a non-empty glyph range. For multi-character clusters,
+    // the loop is needed to find the cluster end. Do the same for LTR below.
+    for (size_t i = char_range.start(); i > range.start(); --i) {
+      first = CharToGlyph(i - 1);
+      if (first != last)
+        return Range(last, first);
+    }
+    return Range(last, glyph_count);
+  }
 
-  const size_t first = CharToGlyph(range.start());
-  const size_t last = CharToGlyph(range.end() - 1);
-  // TODO(ckocagil): What happens when the character has zero or multiple
-  // glyphs? Is the "+ 1" below correct then?
-  return Range(std::min(first, last), std::max(first, last) + 1);
+  first = CharToGlyph(char_range.start());
+  for (size_t i = char_range.end(); i < range.end(); ++i) {
+    last = CharToGlyph(i);
+    if (first != last)
+      return Range(first, last);
+  }
+  return Range(first, glyph_count);
 }
 
 // Returns whether the given shaped run contains any missing glyphs.

ui/gfx/render_text_harfbuzz.h

@@ -16,7 +16,7 @@ namespace gfx {
 
 namespace internal {
 
-struct TextRunHarfBuzz {
+struct GFX_EXPORT TextRunHarfBuzz {
   TextRunHarfBuzz();
   ~TextRunHarfBuzz();
 

ui/gfx/render_text_unittest.cc

@@ -14,6 +14,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/break_list.h"
 #include "ui/gfx/canvas.h"
+#include "ui/gfx/render_text_harfbuzz.h"
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
@@ -1924,4 +1925,56 @@ TEST_F(RenderTextTest, Win_BreakRunsByUnicodeBlocks) {
 }
 #endif  // defined(OS_WIN)
 
+TEST_F(RenderTextTest, HarfBuzz_CharToGlyph) {
+  struct {
+    uint32 glyph_to_char[4];
+    size_t char_to_glyph_expected[4];
+    Range char_range_to_glyph_range_expected[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) },
+      false
+    },
+    { // From string "A B C D" to glyphs "d b c a".
+      { 3, 2, 1, 0 },
+      { 3, 2, 1, 0 },
+      { 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, 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(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]);
+
+  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.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)));
+    }
+  }
+
+}
+
 }  // namespace gfx