Use RenderText directly in omnibox_result_view.cc.

chrome/browser/ui/views/omnibox/omnibox_result_view.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.
 
 // For WinDDK ATL compatibility, these ATL headers must come first.
 #include "build/build_config.h"
 #if defined(OS_WIN)
 #include <atlbase.h>  // NOLINT
 #include <atlwin.h>  // NOLINT
 #endif
 
 #include "chrome/browser/ui/views/omnibox/omnibox_result_view.h"
 
 #include <algorithm>  // NOLINT
 
 #include "base/i18n/bidi_line_iterator.h"
+#include "base/memory/scoped_vector.h"
 #include "chrome/browser/ui/omnibox/omnibox_popup_model.h"
 #include "chrome/browser/ui/views/location_bar/location_bar_view.h"
 #include "chrome/browser/ui/views/omnibox/omnibox_result_view_model.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/native_theme/native_theme.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/text/text_elider.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
+#include "ui/gfx/render_text.h"
 
 namespace {
 
 const char16 kEllipsis[] = { 0x2026, 0x0 };
 
 // The minimum distance between the top and bottom of the {icon|text} and the
 // top or bottom of the row.
 const int kMinimumIconVerticalPadding = 2;
 const int kMinimumTextVerticalPadding = 3;
 
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 // OmniboxResultView, public:
 
 // Precalculated data used to draw the portion of a match classification that
 // fits entirely within one run.
 struct OmniboxResultView::ClassificationData {
   string16 text;
   const gfx::Font* font;
   SkColor color;
   gfx::Size pixel_size;
+  gfx::RenderText* render_text;  // Weak.
 };
 
 // Precalculated data used to draw a complete visual run within the match.
 // This will include all or part of at leasdt one, and possibly several,
 // classifications.
 struct OmniboxResultView::RunData {
   size_t run_start;  // Offset within the match text where this run begins.
   int visual_order;  // Where this run occurs in visual order.  The earliest
   // run drawn is run 0.
   bool is_rtl;
@@ skipping 264 matching lines @@
   // regardless of locale.
   bool is_url = true;
   for (ACMatchClassifications::const_iterator i(classifications.begin());
        i != classifications.end(); ++i) {
     if (!(i->style & ACMatchClassification::URL)) {
       is_url = false;
       break;
     }
   }
 
   // Split the text into visual runs.  We do this first so that we don't need to

[msw, 2012/07/23 15:48:12]

We should try to accomplish this styling and eliding/fading of text via
RenderText StyleRange and CanvasSkia instead of duplicating functionality here.

[Alexei Svitkine (slow), 2012/07/23 15:59:27]

I agree that it would be nice, but currently there's some support missing in
RenderText to do this.

This code basically splits the string into runs and then elides individual runs
as necessary according to omnibox suggestion-specific logic.

This isn't really possible currently with RenderText because it does not expose
its runs to the user of the API. Even if it did, it would also have to be
smarter about re-calculating partial layout - as we wouldn't want an eliding of
a single run to cause the other runs to invalidated.

So until we add support to RenderText for those, we can't really convert to use
a single instance of RenderText for this, but I agree that we should strive to
add such support and eventually do that.

[msw, 2012/07/23 18:56:08]

ok, sgtm.

   // worry about whether our eliding might change the visual display in
   // unintended ways, e.g. by removing directional markings or by adding an
   // ellipsis that's not enclosed in appropriate markings.
   base::i18n::BiDiLineIterator bidi_line;
   if (!bidi_line.Open(text, base::i18n::IsRTL(), is_url))
     return x;
   const int num_runs = bidi_line.CountRuns();
+  ScopedVector<gfx::RenderText> render_texts;
   Runs runs;
   for (int run = 0; run < num_runs; ++run) {
     int run_start_int = 0, run_length_int = 0;
     // The index we pass to GetVisualRun corresponds to the position of the run
     // in the displayed text. For example, the string "Google in HEBREW" (where
     // HEBREW is text in the Hebrew language) has two runs: "Google in " which
     // is an LTR run, and "HEBREW" which is an RTL run. In an LTR context, the
     // run "Google in " has the index 0 (since it is the leftmost run
     // displayed). In an RTL context, the same run has the index 1 because it
     // is the rightmost run. This is why the order in which we traverse the
@@ skipping 30 matching lines @@
       const int style = classifications[i].style;
       const bool use_bold_font = !!(style & ACMatchClassification::MATCH);
       current_data->font = &(use_bold_font ? bold_font_ : normal_font_);
       const ResultViewState state = GetState();
       if (style & ACMatchClassification::URL)
         current_data->color = GetColor(state, URL);
       else if (style & ACMatchClassification::DIM)
         current_data->color = GetColor(state, DIMMED_TEXT);
       else
         current_data->color = GetColor(state, force_dim ? DIMMED_TEXT : TEXT);
-      int width = 0;
-      int height = 0;
-      gfx::Canvas::SizeStringInt(current_data->text, *current_data->font,
-                                 &width, &height, gfx::Canvas::NO_ELLIPSIS);
-      current_data->pixel_size = gfx::Size(width, height);
-      current_run->pixel_width += width;
+
+      render_texts.push_back(gfx::RenderText::CreateInstance());
+      current_data->render_text = render_texts.back();
+      current_data->render_text->SetFontList(
+          gfx::FontList(*current_data->font));
+      current_data->render_text->SetText(current_data->text);
+
+      gfx::StyleRange style_range;
+      style_range.foreground = current_data->color;
+      style_range.font_style = current_data->font->GetStyle();
+      current_data->render_text->set_default_style(style_range);
+      current_data->render_text->ApplyDefaultStyle();
+
+      current_data->pixel_size = current_data->render_text->GetStringSize();
+      current_run->pixel_width += current_data->pixel_size.width();
     }
     DCHECK(!current_run->classifications.empty());
   }
   DCHECK(!runs.empty());
 
   // Sort into logical order so we can elide logically.
   std::sort(runs.begin(), runs.end(), &SortRunsLogically);
 
   // Now determine what to elide, if anything.  Several subtle points:
   //   * Because we have the run data, we can get edge cases correct, like
@@ skipping 23 matching lines @@
       }
       break;
     }
     remaining_width -= i->pixel_width;
   }
 
   // Sort back into visual order so we can display the runs correctly.
   std::sort(runs.begin(), runs.end(), &SortRunsVisually);
 
   // Draw the runs.
+  const int text_height = GetTextHeight();

[msw, 2012/07/23 18:56:08]

nit: remove unused local variable.

[Alexei Svitkine (slow), 2012/07/23 19:38:12]

Done.

   for (Runs::iterator i(runs.begin()); i != runs.end(); ++i) {
     const bool reverse_visible_order = (i->is_rtl != base::i18n::IsRTL());
-    int flags = gfx::Canvas::NO_ELLIPSIS;  // We've already elided.
-    if (reverse_visible_order) {
+    if (reverse_visible_order)
       std::reverse(i->classifications.begin(), i->classifications.end());
-      if (i->is_rtl)
-        flags |= gfx::Canvas::FORCE_RTL_DIRECTIONALITY;

[Peter Kasting, 2012/07/12 17:51:48]

It worries me that you have no equivalent of this statement.  This seems like a
big red flag for RTL.

[Alexei Svitkine (slow), 2012/07/12 17:58:02]

RenderText will use RTL directionality in the presence of strong RTL characters
in the string. Is there a case where this will not give the results we want?

[Peter Kasting, 2012/07/12 18:05:14]

We may get bad results for URLs with Hebrew characters in them where we render
those sections in reversed order (note how when calculating is_rtl we exclude
the is_url cases).  I don't know how coherent the results of the GetVisualRun()
call on line 357 will be with RenderText's algorithm otherwise so I can't
speculate much.

[msw, 2012/07/23 15:48:12]

I'll write a patch to force base LTR/RTL with a flag (at least for
RenderTextWin), which should be done with Uniscribe settings instead of wrapping
the text with control characters like ui/gfx/canvas_skia.cc.

-    }
     for (Classifications::const_iterator j(i->classifications.begin());
          j != i->classifications.end(); ++j) {
       const int left =
           mirroring_context_->mirrored_left_coord(x, x + j->pixel_size.width());
-      // By passing the same y-coordinate for each run, we vertically align the
-      // tops of successive runs.  This isn't actually what we want; we want to
-      // align the baselines, but Canvas doesn't currently expose text
-      // measurement APIs sufficient to make that happen.  The problem here is
-      // font substitution: if no fonts are substituted, then all runs have the
-      // same font (in bold or normal styles), and thus the same height and same
-      // baseline.  If fonts are substituted within a run, the characters are
-      // baseline-aligned within the run, but using the same top coordinate as
-      // for other runs is only correct if the overall ascent for this run is
-      // the same as for other runs -- that is, if the tallest ascent of all
-      // fonts in the run is equal to the ascent of the normal font.  If this
-      // condition doesn't hold, the baseline for this run will be drawn too
-      // high or too low, depending on whether the run's tallest ascent is
-      // shorter or higher than the normal font's ascent, respectively.
-      //
-      // TODO(asvitkine): Fix this by replacing the SizeStringInt() calls
-      // elsewhere in this file with calls that can calculate actual baselines
-      // even in the face of font fallback. Tracked as: http://crbug.com/128027
-      canvas->DrawStringInt(j->text, *j->font, j->color, left, y,
-                            j->pixel_size.width(), j->pixel_size.height(),
-                            flags);
+      // Align the text runs to a common baseline.
+      const int top =
+          y + normal_font_.GetBaseline() - j->render_text->GetBaseline();
+      gfx::Rect rect(left, top, j->pixel_size.width(), j->pixel_size.height());
+      j->render_text->SetDisplayRect(rect);
+      j->render_text->Draw(canvas);
       x += j->pixel_size.width();
     }
   }
 
   return x;
 }
 
 void OmniboxResultView::Elide(Runs* runs, int remaining_width) const {
   // The complexity of this function is due to edge cases like the following:
   // We have 100 px of available space, an initial classification that takes 86
   // px, and a font that has a 15 px wide ellipsis character.  Now if the first
   // classification is followed by several very narrow classifications (e.g. 3
   // px wide each), we don't know whether we need to elide or not at the time we
   // see the first classification -- it depends on how many subsequent
   // classifications follow, and some of those may be in the next run (or
   // several runs!).  This is why instead we let our caller move forward until
   // we know we definitely need to elide, and then in this function we move
   // backward again until we find a string that we can successfully do the
   // eliding on.
   bool first_classification = true;
   for (Runs::reverse_iterator i(runs->rbegin()); i != runs->rend(); ++i) {
     for (Classifications::reverse_iterator j(i->classifications.rbegin());
          j != i->classifications.rend(); ++j) {
       if (!first_classification) {
         // For all but the first classification we consider, we need to append
         // an ellipsis, since there isn't enough room to draw it after this
         // classification.
         j->text += kEllipsis;
+        j->render_text->SetText(j->text);
 
         // We also add this classification's width (sans ellipsis) back to the
         // available width since we want to consider the available space we'll
         // have when we draw this classification.
         remaining_width += j->pixel_size.width();
       }
       first_classification = false;
 
       // Can we fit at least an ellipsis?
       string16 elided_text =
@@ skipping 14 matching lines @@
         // need to bother with this; see note below.
         elided_text = kEllipsis;
       }
       if (!elided_text.empty()) {
         // Success.  Elide this classification and stop.
         j->text = elided_text;
 
         // If we could only fit an ellipsis, then only make it bold if there was
         // an immediate prior classification in this run that was also bold, or
         // it will look orphaned.
-        if ((elided_text.length() == 1) &&
+        if ((j->font != &normal_font_) &&
+            (elided_text.length() == 1) &&

[msw, 2012/07/23 18:56:08]

optional nit: fit on line above.

[Alexei Svitkine (slow), 2012/07/23 19:38:12]

Done.

             (on_first_classification ||
-             (prior_classification->font == &normal_font_)))
+            (prior_classification->font == &normal_font_))) {

[msw, 2012/07/23 18:56:08]

nit: re-indent by one space.

[Alexei Svitkine (slow), 2012/07/23 19:38:12]

Done.

           j->font = &normal_font_;
+          j->render_text->SetFontList(gfx::FontList(*j->font));
+        }
 
-        int width = 0;
-        int height = 0;
-        gfx::Canvas::SizeStringInt(elided_text, *j->font, &width, &height,
-                                   gfx::Canvas::NO_ELLIPSIS);
-        j->pixel_size = gfx::Size(width, height);
+        j->render_text->SetText(elided_text);
+        j->pixel_size = j->render_text->GetStringSize();
 
         // Erase any other classifications that come after the elided one.
         i->classifications.erase(j.base(), i->classifications.end());
         runs->erase(i.base(), runs->end());
         return;
       }
 
       // We couldn't fit an ellipsis.  Move back one classification,
       // append an ellipsis, and try again.
       // NOTE: In the edge case that a bold ellipsis doesn't fit but a
@@ skipping 69 matching lines @@
     int x = GetMirroredXForRect(keyword_text_bounds_);
     mirroring_context_->Initialize(x, keyword_text_bounds_.width());
     PaintMatch(canvas, *match_.associated_keyword.get(), x);
   }
 }
 
 void OmniboxResultView::AnimationProgressed(const ui::Animation* animation) {
   Layout();
   SchedulePaint();
 }