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

ui/gfx/render_text_win.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_win.h"
 
 #include <algorithm>
 
 #include "base/i18n/break_iterator.h"
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #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"
 #include "ui/gfx/font_smoothing_win.h"
 #include "ui/gfx/platform_font_win.h"
 
 namespace gfx {
 
 namespace {
 
@@ skipping 560 matching lines @@
                           run->font_style, &run->font);
     run->strike = style.style(STRIKE);
     run->diagonal_strike = style.style(DIAGONAL_STRIKE);
     run->underline = style.style(UNDERLINE);
     run->script_analysis = script_item->a;
 
     // Find the next break and advance the iterators as needed.
     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++;
     run->range.set_end(run_break);
     runs_.push_back(run);
   }
 
   // Undo the temporarily applied composition underlines and selection colors.
   UndoCompositionAndSelectionStyles();
 }
@@ skipping 289 matching lines @@
   size_t position = LayoutIndexToTextIndex(run->range.end());
   position = IndexOfAdjacentGrapheme(position, CURSOR_BACKWARD);
   return SelectionModel(position, CURSOR_FORWARD);
 }
 
 RenderText* RenderText::CreateInstance() {
   return new RenderTextWin;
 }
 
 }  // namespace gfx