RenderTextWin: Break runs between any two characters that are not in the same code block

ui/gfx/render_text_win.cc

Index: ui/gfx/render_text_win.cc
diff --git a/ui/gfx/render_text_win.cc b/ui/gfx/render_text_win.cc
index ac018de3f446226f8e39276ee65f24ebf1e76478..0b5694e94031af49e21a61fce26050a70a402d41 100644
--- a/ui/gfx/render_text_win.cc
+++ b/ui/gfx/render_text_win.cc
@@ -12,6 +12,8 @@
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/win/windows_version.h"
+#include "third_party/icu/source/common/unicode/uchar.h"
+#include "third_party/icu/source/common/unicode/utf16.h"
 #include "ui/base/text/utf16_indexing.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font_fallback_win.h"
@@ -592,9 +594,28 @@ void RenderTextWin::ItemizeLogicalText() {
     const size_t script_item_break = (script_item + 1)->iCharPos;
     run_break = std::min(script_item_break,
                          TextIndexToLayoutIndex(style.GetRange().end()));
+
     // Clamp run lengths to avoid exceeding the maximum supported glyph count.
     if ((run_break - run->range.start()) > max_run_length)
       run_break = run->range.start() + max_run_length;
+
+    // Break runs between any two characters that are not in the same code

[msw, 2013/09/03 16:15:31]

Can you explain why you chose to break runs between characters in different code
blocks, versus between characters in different scripts? As long as fonts can
usually shape all the characters in a given script, it seems like splitting by
script change would be more correct. UBlockCode has 221 entries, while
UScriptCode only has 159, so it would likely offer less unnecessary run breaks
too. Does the ▶ (black right-pointing triangle, ▶) share the same script
as english characters, but have a different code block? Resource links below.

http://www.icu-project.org/apiref/icu4c/uchar_8h.html#a19f1662c9c33c31593612d...
http://www.icu-project.org/apiref/icu4c/uchar_8h_source.html
http://www.icu-project.org/apiref/icu4c/uscript_8h.html#a5036ff9ca2a7f1d2bde0...
http://www.icu-project.org/apiref/icu4c/uscript_8h_source.html

[ckocagil, 2013/09/04 13:02:50]

I simply didn't know Unicode had classification by script in its standard!

I tried to achieve this with UScriptCode. There is a problem. Common characters
such as spaces are in USCRIPT_COMMON. I tried always accepting USCRIPT_COMMON in
runs, but it turns out the YouTube play character '▶' is in USCRIPT_COMMON too!
UBlockCode doesn't have either of these problems.

+    // block. http://crbug.com/278913

[msw, 2013/09/03 16:15:31]

nit: consider using a comment like:
// Break runs between characters in different code blocks. This avoids using
// fallback fonts for more characters than needed. http://crbug.com/278913

[ckocagil, 2013/09/04 13:02:50]

Done.

+    size_t i = run->range.start();
+    UChar32 current_char = 0;
+    U16_NEXT(text().c_str(), i, text().length(), current_char);

[msw, 2013/09/03 16:15:31]

Use base::i18n::UTF16CharIterator instead.

[ckocagil, 2013/09/04 13:02:50]

Done.

+    const UBlockCode first_block = ublock_getCode(current_char);
+    while (i < run_break) {
+      const size_t current_index = i;
+      U16_NEXT(text().c_str(), i, text().length(), current_char);
+      if (ublock_getCode(current_char) != first_block) {
+        DCHECK_LE(current_index, run_break);
+        run_break = current_index;
+        break;
+      }
+    }
+    DCHECK_LT(run->range.start(), run_break);
+
     style.UpdatePosition(LayoutIndexToTextIndex(run_break));
     if (script_item_break == run_break)
       script_item++;