RenderTextWin: Use a font that can at least partially display the run, if possible.

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"
@@ skipping 661 matching lines @@
   string_size_.set_height(0);
   for (size_t i = 0; i < runs_.size(); ++i) {
     internal::TextRun* run = runs_[i];
     const size_t run_length = run->range.length();
     const wchar_t* run_text = &(text()[run->range.start()]);
     bool tried_cached_font = false;
     bool tried_fallback = false;
     size_t linked_font_index = 0;
     const std::vector<Font>* linked_fonts = NULL;
     Font original_font = run->font;
+    // Keep track of the font producing the least missing glyphs for which
+    // ScriptShape() returned S_OK. This font will be used in the case where
+    // no font is able to display all the characters in this run.
+    int least_missing_count = INT_MAX;

[msw, 2012/07/10 20:52:15]

nit: optionally rename this for more context, like
|best_partial_font_missing_glyph_count| or |least_missing_glyph_count|.

[Alexei Svitkine (slow), 2012/07/10 21:16:35]

Done.

+    Font best_partial_font = run->font;
+    bool using_best_partial_font = false;
 
     // Select the font desired for glyph generation.
     SelectObject(cached_hdc_, run->font.GetNativeFont());
 
     run->logical_clusters.reset(new WORD[run_length]);
     run->glyph_count = 0;
     // Max glyph guess: http://msdn.microsoft.com/en-us/library/dd368564.aspx
     size_t max_glyphs = static_cast<size_t>(1.5 * run_length + 16);
     while (max_glyphs < kMaxGlyphs) {
       run->glyphs.reset(new WORD[max_glyphs]);
@@ skipping 12 matching lines @@
         max_glyphs *= 2;
         continue;
       }
 
       bool glyphs_missing = false;
       if (hr == USP_E_SCRIPT_NOT_IN_FONT) {
         glyphs_missing = true;
       } else if (hr == S_OK) {
         // If |hr| is S_OK, there could still be missing glyphs in the output,
         // see: http://msdn.microsoft.com/en-us/library/windows/desktop/dd368564.aspx
-        glyphs_missing = HasMissingGlyphs(run);
+        const int missing_count = CountCharsWithMissingGlyphs(run);
+        // Track the font that produced the least missing glyphs.
+        if (missing_count < least_missing_count) {
+          least_missing_count = missing_count;
+          best_partial_font = run->font;
+        }
+        glyphs_missing = (missing_count != 0);
       }
 
-      // Skip font substitution if there are no missing glyphs.
-      if (!glyphs_missing) {
+      // Skip font substitution if there are no missing glyphs or if the font
+      // with the least missing glyphs is being used as a last resort.
+      if (!glyphs_missing || using_best_partial_font) {
         // Save the successful fallback font that was chosen.
-        if (tried_fallback)
+        if (tried_fallback && !using_best_partial_font)
           successful_substitute_fonts_[original_font.GetFontName()] = run->font;
         break;
       }
 
       // First, try the cached font from previous runs, if any.
       if (!tried_cached_font) {
         tried_cached_font = true;
         std::map<std::string, Font>::const_iterator it =
             successful_substitute_fonts_.find(original_font.GetFontName());
         if (it != successful_substitute_fonts_.end()) {
@@ skipping 27 matching lines @@
         // ones for the Uniscribe fallback font. This may happen if the first
         // font is a custom font that has no linked fonts in the Registry.
         //
         // Note: One possibility would be to always merge both lists of fonts,
         //       but it is not clear whether there are any real world scenarios
         //       where this would actually help.
         if (linked_fonts->empty())
           linked_fonts = GetLinkedFonts(run->font);
       }
 
-      // None of the linked fonts worked, break out of the loop.
+      // None of the fallback fonts were able to display the entire run.
       if (linked_font_index == linked_fonts->size()) {
+        // If there was a font that was able to partially display the run, use

[msw, 2012/07/10 20:52:15]

nit: make one-liner as "If a font could partially display the run, use that
now." or similar.

[Alexei Svitkine (slow), 2012/07/10 21:16:35]

Done.

+        // that now.
+        if (least_missing_count != INT_MAX) {
+          ApplySubstituteFont(run, best_partial_font);
+          using_best_partial_font = true;
+          continue;
+        }
+
+        // If no font was able to partially display the run, replace all glyphs

[msw, 2012/07/10 20:52:15]

In the bug, you wrote (and I agree that) "In the failure case, we should also
display the missing glyph "box" character, using the original input font." So,
I'm not certain, but shouldn't this do something slightly different? Isn't the
script_cache that of the last attempted fallback font, and not the original
font?

[Alexei Svitkine (slow), 2012/07/10 21:16:35]

You're right that this is not solving that specific issue. I would like to leave
that (more minor) problem for another CL. I think it may be a bit easier to
handle that case once I refactor the fallback code into a separate file.

[msw, 2012/07/10 21:40:24]

sgtm

+        // with |wgDefault| to ensure they don't hold garbage values.
+        SCRIPT_FONTPROPERTIES properties;
+        memset(&properties, 0, sizeof(properties));
+        properties.cBytes = sizeof(properties);
+        ScriptGetFontProperties(cached_hdc_, &run->script_cache, &properties);
+        for (int i = 0; i < run->glyph_count; ++i)
+          run->glyphs[i] = properties.wgDefault;
+
         // TODO(msw): Don't use SCRIPT_UNDEFINED. Apparently Uniscribe can
         //            crash on certain surrogate pairs with SCRIPT_UNDEFINED.
         //            See https://bugzilla.mozilla.org/show_bug.cgi?id=341500
         //            And http://maxradi.us/documents/uniscribe/
         run->script_analysis.eScript = SCRIPT_UNDEFINED;
         // Reset |hr| to 0 to not trigger the DCHECK() below when a font is
         // not found that can display the text. This is expected behavior
         // under Windows XP without additional language packs installed and
         // may also happen on newer versions when trying to display text in
         // an obscure script that the system doesn't have the right font for.
@@ skipping 54 matching lines @@
 void RenderTextWin::ApplySubstituteFont(internal::TextRun* run,
                                         const Font& font) {
   const int font_size = run->font.GetFontSize();
   const int font_height = run->font.GetHeight();
   run->font = font;
   DeriveFontIfNecessary(font_size, font_height, run->font_style, &run->font);
   ScriptFreeCache(&run->script_cache);
   SelectObject(cached_hdc_, run->font.GetNativeFont());
 }
 
-bool RenderTextWin::HasMissingGlyphs(internal::TextRun* run) const {
+int RenderTextWin::CountCharsWithMissingGlyphs(internal::TextRun* run) const {
+  int chars_not_missing_glyphs = 0;

[msw, 2012/07/10 20:52:15]

Isn't this counting the number glyphs found? (not related to chars?)
If so, rename this |glyphs_found|, and rename the function too.
Otherwise, clarify exactly what's being counted.

[Alexei Svitkine (slow), 2012/07/10 21:16:35]

It is counting chars, in fact, due to how the loop is constructed. See the for
loop below; it's iterating over chars.

For each char, it finds the corresponding glyph. A glyph may cover multiple
chars (e.g. a decorated letter may be multiple chars, but a single glyph). So
this is indeed counting chars.

I believe both the function and variable are already named in a way to indicate
that. However, I have clarified the code elsewhere.

   SCRIPT_FONTPROPERTIES properties;
   memset(&properties, 0, sizeof(properties));
   properties.cBytes = sizeof(properties);
   ScriptGetFontProperties(cached_hdc_, &run->script_cache, &properties);
 
   const wchar_t* run_text = &(text()[run->range.start()]);
   for (size_t char_index = 0; char_index < run->range.length(); ++char_index) {
     const int glyph_index = run->logical_clusters[char_index];
     DCHECK_GE(glyph_index, 0);
     DCHECK_LT(glyph_index, run->glyph_count);
 
     if (run->glyphs[glyph_index] == properties.wgDefault)
-      return true;
+      continue;
 
     // Windows Vista sometimes returns glyphs equal to wgBlank (instead of
     // wgDefault), with fZeroWidth set. Treat such cases as having missing
     // glyphs if the corresponding character is not whitespace.
     // See: http://crbug.com/125629
     if (run->glyphs[glyph_index] == properties.wgBlank &&
         run->visible_attributes[glyph_index].fZeroWidth &&
         !IsWhitespace(run_text[char_index]) &&
         !IsUnicodeBidiControlCharacter(run_text[char_index])) {
-      return true;
+      continue;
     }
+
+    ++chars_not_missing_glyphs;
   }
 
-  return false;
+  return run->range.length() - chars_not_missing_glyphs;

[msw, 2012/07/10 20:52:15]

Shouldn't this be "return run->glyph_count - glyphs_found;"
Perhaps DCHECK that this value is not negative.

[Alexei Svitkine (slow), 2012/07/10 21:16:35]

Nope, it shouldn't. See above.

It can't be negative because the loop iterates over |run->range.length()|
characters and can only increments |chars_not_missing_glyphs| one or zero times
each iteration. I've added a DCHECK() though, in case the code changes.

 }
 
 const std::vector<Font>* RenderTextWin::GetLinkedFonts(const Font& font) const {
   const std::string& font_name = font.GetFontName();
   std::map<std::string, std::vector<Font> >::const_iterator it =
       cached_linked_fonts_.find(font_name);
   if (it != cached_linked_fonts_.end())
     return &it->second;
 
   cached_linked_fonts_[font_name] = std::vector<Font>();
@@ skipping 34 matching lines @@
     const internal::TextRun* run) {
   size_t caret = IndexOfAdjacentGrapheme(run->range.end(), CURSOR_BACKWARD);
   return SelectionModel(caret, CURSOR_FORWARD);
 }
 
 RenderText* RenderText::CreateInstance() {
   return new RenderTextWin;
 }
 
 }  // namespace gfx