Always try metafile font fallback in the case of missing glyphs in RenderTextWin.

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/logging.h"
@@ skipping 594 matching lines @@
     SCRIPT_FONTPROPERTIES font_properties;
     memset(&font_properties, 0, sizeof(font_properties));
     font_properties.cBytes = sizeof(SCRIPT_FONTPROPERTIES);
     ScriptGetFontProperties(cached_hdc_, &run->script_cache, &font_properties);
 
     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) {
+      bool glyphs_missing = false;
       run->glyphs.reset(new WORD[max_glyphs]);
       run->visible_attributes.reset(new SCRIPT_VISATTR[max_glyphs]);
       hr = ScriptShape(cached_hdc_,
                        &run->script_cache,
                        run_text,
                        run_length,
                        max_glyphs,
                        &(run->script_analysis),
                        run->glyphs.get(),
                        run->logical_clusters.get(),
                        run->visible_attributes.get(),
                        &(run->glyph_count));
       if (hr == E_OUTOFMEMORY) {
         max_glyphs *= 2;
+        continue;
+      } else 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
-        //
-        // If there are missing glyphs, use font linking to try to find a
-        // matching font.
-        bool glyphs_missing = false;
-        for (int i = 0; i < run->glyph_count; i++) {
+        for (int i = 0; i < run->glyph_count; ++i) {
           if (run->glyphs[i] == font_properties.wgDefault) {
             glyphs_missing = true;
             break;
           }
         }
-        // No glyphs missing - good to go.
-        if (!glyphs_missing)
-          break;
+      }
 
-        // First time through, get the linked fonts list.
-        if (linked_fonts == NULL)
-          linked_fonts = GetLinkedFonts(run->font);
+      // Skip font substitution if there are no missing glyphs.
+      if (!glyphs_missing)
+        break;
 
-        // None of the linked fonts worked - break out of the loop.
-        if (linked_font_index == linked_fonts->size())
-          break;
-
-        // Try the next linked font.
-        run->font = linked_fonts->at(linked_font_index++);
-        DeriveFontIfNecessary(font_size, run->font_style, &run->font);
-        ScriptFreeCache(&run->script_cache);
-        SelectObject(cached_hdc_, run->font.GetNativeFont());
-      } else if (hr == USP_E_SCRIPT_NOT_IN_FONT) {
-        // Only try font fallback if it hasn't yet been attempted for this run.
-        if (tried_fallback) {
-          // 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.
-          hr = 0;
-          break;
-        }
-
-        // The run's font doesn't contain the required glyphs, use an alternate.
-        // TODO(msw): support RenderText's font_list().
+      // If there are missing glyphs, first try finding a fallback font using a
+      // meta file, if it hasn't yet been attempted for this run.
+      // TODO(msw|asvitkine): Support RenderText's font_list()?
+      if (!tried_fallback) {
+        tried_fallback = true;
         if (ChooseFallbackFont(cached_hdc_, run->font, run_text, run_length,
                                &run->font)) {
-          DeriveFontIfNecessary(font_size, run->font_style, &run->font);
-          ScriptFreeCache(&run->script_cache);
-          SelectObject(cached_hdc_, run->font.GetNativeFont());
+          UpdateRunFont(run, font_size);
+          continue;
         }
+      }
 
-        tried_fallback = true;
-      } else {
+      // Couldn't find a replacement font using the meta file, try to find one

[msw, 2012/04/12 21:32:14]

nit: consider "The meta file did not yield a replacement font,"

[Alexei Svitkine (slow), 2012/04/13 13:56:15]

Done.

+      // using font linking. First time through, get the linked fonts list.
+      if (linked_fonts == NULL)
+        linked_fonts = GetLinkedFonts(run->font);
+
+      // None of the linked fonts worked - break out of the loop.
+      if (linked_font_index == linked_fonts->size()) {
+        // 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.
+        hr = 0;
         break;
       }
+
+      // Try the next linked font.
+      run->font = linked_fonts->at(linked_font_index++);
+      UpdateRunFont(run, font_size);
     }
     DCHECK(SUCCEEDED(hr));
     string_size_.set_height(std::max(string_size_.height(),
                                      run->font.GetHeight()));
     common_baseline_ = std::max(common_baseline_, run->font.GetBaseline());
 
     if (run->glyph_count > 0) {
       run->advance_widths.reset(new int[run->glyph_count]);
       run->offsets.reset(new GOFFSET[run->glyph_count]);
       hr = ScriptPlace(cached_hdc_,
@@ skipping 28 matching lines @@
   for (size_t i = 0; i < runs_.size(); ++i) {
     internal::TextRun* run = runs_[visual_to_logical_[i]];
     run->preceding_run_widths = preceding_run_widths;
     const ABC& abc = run->abc_widths;
     run->width = abc.abcA + abc.abcB + abc.abcC;
     preceding_run_widths += run->width;
   }
   string_size_.set_width(preceding_run_widths);
 }
 
+void RenderTextWin::UpdateRunFont(internal::TextRun* run, int font_size) {

[msw, 2012/04/12 21:32:14]

I think it'd be a better to supply the substitute font as an argument here
(instead of |font_size|), and use the run's existing font size with
DeriveFontIfNecessary, as done with |run->font_style|.

[Alexei Svitkine (slow), 2012/04/12 22:11:43]

I agree it would be cleaner from an interface POV, but it would result in an
extra Font copy operation (since ChooseFallbackFont() would need to output to a
temp font), which (I just checked) is not cheap for PlatformFontWin. :(

(Although, I'm going to look at fixing this.)

Would you be okay with a TODO to clean this up, or do you prefer I block landing
this pending looking at making the PlatformFontWin copy constructor fast?

[Alexei Svitkine (slow), 2012/04/13 13:56:15]

Done.

+  DeriveFontIfNecessary(font_size, run->font_style, &run->font);
+  ScriptFreeCache(&run->script_cache);
+  SelectObject(cached_hdc_, run->font.GetNativeFont());
+}
+
 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>();
   std::vector<Font>* linked_fonts = &cached_linked_fonts_[font_name];
   QueryLinkedFontsFromRegistry(font, linked_fonts);
@@ skipping 32 matching lines @@
     internal::TextRun* run) {
   size_t caret = IndexOfAdjacentGrapheme(run->range.end(), CURSOR_BACKWARD);
   return SelectionModel(caret, CURSOR_FORWARD);
 }
 
 RenderText* RenderText::CreateRenderText() {
   return new RenderTextWin;
 }
 
 }  // namespace gfx