RenderTextHarfBuzz: Break runs at parentheses so they aren't affected by font fallbacks

ui/gfx/render_text_harfbuzz.cc

 // Copyright 2014 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_harfbuzz.h"
 
 #include <limits>
 #include <map>
 
 #include "base/i18n/bidi_line_iterator.h"
@@ skipping 285 matching lines @@
   hb_font_make_immutable(harfbuzz_font);
   return harfbuzz_font;
 }
 
 // Returns true if characters of |block_code| may trigger font fallback.
 bool IsUnusualBlockCode(UBlockCode block_code) {
   return block_code == UBLOCK_GEOMETRIC_SHAPES ||
          block_code == UBLOCK_MISCELLANEOUS_SYMBOLS;
 }
 
-// Returns the index of the first unusual character after a usual character or
-// vice versa. Unusual characters are defined by |IsUnusualBlockCode|.
-size_t FindUnusualCharacter(const base::string16& text,
-                            size_t run_start,
-                            size_t run_break) {
+// Returns the boundary between a special and a regular character. Special
+// characters are brackets or characters that satisfy |IsUnusualBlockCode|.
+size_t FindRunBreakingCharacter(const base::string16& text,
+                                size_t run_start,
+                                size_t run_break) {
+  static const char kBrackets[] = { '(', ')', '{', '}', '<', '>' };

[msw, 2014/09/02 18:20:25]

nit: add a comma after the last element.

[ckocagil, 2014/09/03 01:22:29]

Done.

+  static const char* kBrackets_end = kBrackets + arraysize(kBrackets);

[msw, 2014/09/02 18:20:25]

nit: kBracketsEnd.

[ckocagil, 2014/09/03 01:22:29]

Done.

   const int32 run_length = static_cast<int32>(run_break - run_start);
   base::i18n::UTF16CharIterator iter(text.c_str() + run_start,
                                      run_length);
-  const UBlockCode first_block_code = ublock_getCode(iter.get());
-  const bool first_block_unusual = IsUnusualBlockCode(first_block_code);
+  const UChar32 first_char = iter.get();
+  const UBlockCode first_block = ublock_getCode(first_char);
+  const bool first_parenthesis =

[msw, 2014/09/02 18:20:25]

nit: |first_bracket| to match |kBrackets|; ditto for |current_*|.

[ckocagil, 2014/09/03 01:22:29]

Done.

+      std::find(kBrackets, kBrackets_end, first_char) != kBrackets_end;

[msw, 2014/09/02 18:20:25]

How about adding a helper function IsBracket[Character] or similar?

[ckocagil, 2014/09/03 01:22:29]

Done.

+  const bool first_block_unusual = IsUnusualBlockCode(first_block);
+
   while (iter.Advance() && iter.array_pos() < run_length) {
-    const UBlockCode current_block_code = ublock_getCode(iter.get());
-    if (current_block_code != first_block_code &&
-        (first_block_unusual || IsUnusualBlockCode(current_block_code))) {
+    const UChar32 current_char = iter.get();
+    const UBlockCode current_block = ublock_getCode(current_char);
+    const bool current_parenthesis =
+        std::find(kBrackets, kBrackets_end, current_char) != kBrackets_end;
+    const bool block_break = current_block != first_block &&
+        (first_block_unusual || IsUnusualBlockCode(current_block));
+    if (block_break || first_parenthesis != current_parenthesis)
       return run_start + iter.array_pos();
-    }
   }
   return run_break;
 }
 
 // If the given scripts match, returns the one that isn't USCRIPT_COMMON or
 // USCRIPT_INHERITED, i.e. the more specific one. Otherwise returns
 // USCRIPT_INVALID_CODE.
 UScriptCode ScriptIntersect(UScriptCode first, UScriptCode second) {
   if (first == second ||
       (second > USCRIPT_INVALID_CODE && second <= USCRIPT_INHERITED)) {
@@ skipping 268 matching lines @@
     if (offset < end) {
       return SelectionModel(LayoutIndexToTextIndex(
           run.glyph_to_char[i] + (run.is_rtl ? 0 : 1)),
           (run.is_rtl ? CURSOR_FORWARD : CURSOR_BACKWARD));
     }
   }
   return EdgeSelectionModel(CURSOR_RIGHT);
 }
 
 std::vector<RenderText::FontSpan> RenderTextHarfBuzz::GetFontSpansForTesting() {
-  NOTIMPLEMENTED();
-  return std::vector<RenderText::FontSpan>();
+  EnsureLayout();
+
+  std::vector<RenderText::FontSpan> spans;
+  for (size_t i = 0; i < runs_.size(); ++i) {
+    SkString family_name;
+    runs_[i]->skia_face->getFamilyName(&family_name);
+    Font font(family_name.c_str(), runs_[i]->font_size);
+    spans.push_back(RenderText::FontSpan(font,
+        Range(LayoutIndexToTextIndex(runs_[i]->range.start()),
+              LayoutIndexToTextIndex(runs_[i]->range.end()))));
+  }
+
+  return spans;
 }
 
 Range RenderTextHarfBuzz::GetGlyphBounds(size_t index) {
   EnsureLayout();
   const size_t run_index =
       GetRunContainingCaret(SelectionModel(index, CURSOR_FORWARD));
   // Return edge bounds if the index is invalid or beyond the layout text size.
   if (run_index >= runs_.size())
     return Range(GetStringSize().width());
   const size_t layout_index = TextIndexToLayoutIndex(index);
@@ skipping 388 matching lines @@
     // Odd BiDi embedding levels correspond to RTL runs.
     run->is_rtl = (run->level % 2) == 1;
     // Find the length and script of this script run.
     script_item_break = ScriptInterval(text, run_break,
         script_item_break - run_break, &run->script) + run_break;
 
     // Find the next break and advance the iterators as needed.
     run_break = std::min(static_cast<size_t>(script_item_break),
                          TextIndexToLayoutIndex(style.GetRange().end()));
 
-    // Break runs adjacent to character substrings in certain code blocks.
-    // This avoids using their fallback fonts for more characters than needed,
-    // in cases like "\x25B6 Media Title", etc. http://crbug.com/278913
+    // This is a workaround until we can do partial font fallback in runs. Break

[msw, 2014/09/02 18:20:25]

I'm not sure we'll want to do partial font fallback in runs, perhaps remove that
first sentence.

[ckocagil, 2014/09/03 01:22:29]

I'd like to go ahead and implement partial font fallback after RTHB is enabled
and all good, but that's a different topic. Removed.

+    // runs at certain characters that need to be rendered separately to prevent
+    // either an unusual character from forcing a fallback on the entire run,

[msw, 2014/09/02 18:20:25]

nit: "fallback font" here and below.

[ckocagil, 2014/09/03 01:22:29]

Done.

+    // or brackets from being affected by a fallback.
+    // http://crbug.com/278913, http://crbug.com/396776
     if (run_break > run->range.start())
-      run_break = FindUnusualCharacter(text, run->range.start(), run_break);
+      run_break = FindRunBreakingCharacter(text, run->range.start(), run_break);
 
     DCHECK(IsValidCodePointIndex(text, run_break));
     style.UpdatePosition(LayoutIndexToTextIndex(run_break));
     run->range.set_end(run_break);
 
     runs_.push_back(run);
   }
 
   // Undo the temporarily applied composition underlines and selection colors.
   UndoCompositionAndSelectionStyles();
@@ skipping 125 matching lines @@
       run->width = std::floor(run->width + 0.5f);
 #endif
   }
 
   hb_buffer_destroy(buffer);
   hb_font_destroy(harfbuzz_font);
   return true;
 }
 
 }  // namespace gfx