Enable subpixel positioning for UI

ui/gfx/render_text_harfbuzz.cc

Index: ui/gfx/render_text_harfbuzz.cc
diff --git a/ui/gfx/render_text_harfbuzz.cc b/ui/gfx/render_text_harfbuzz.cc
index cc7d16955ac1041cdc7e6cd77371223f2fc01ebc..1fb8f80bbed842a79d59c5fe3141bbb3ece0ad4e 100644
--- a/ui/gfx/render_text_harfbuzz.cc
+++ b/ui/gfx/render_text_harfbuzz.cc
@@ -24,6 +24,9 @@
 #include "ui/gfx/font_fallback_win.h"
 #endif
 
+using gfx::internal::RangeF;
+using gfx::internal::RoundRangeF;
+
 namespace gfx {
 
 namespace {
@@ -460,6 +463,10 @@ void GetClusterAtImpl(size_t pos,
 
 namespace internal {
 
+Range RoundRangeF(const RangeF& range_f) {
+  return Range(std::round(range_f.first), std::round(range_f.second));
+}
+
 TextRunHarfBuzz::TextRunHarfBuzz()
     : width(0.0f),
       preceding_run_widths(0.0f),
@@ -521,21 +528,19 @@ size_t TextRunHarfBuzz::CountMissingGlyphs() const {
   return missing;
 }
 
-Range TextRunHarfBuzz::GetGraphemeBounds(
+RangeF TextRunHarfBuzz::GetGraphemeBounds(
     base::i18n::BreakIterator* grapheme_iterator,
     size_t text_index) {
   DCHECK_LT(text_index, range.end());
-  // TODO(msw): Support floating point grapheme bounds.
-  const int preceding_run_widths_int = SkScalarRoundToInt(preceding_run_widths);
   if (glyph_count == 0)
-    return Range(preceding_run_widths_int, preceding_run_widths_int + width);
+    return RangeF(preceding_run_widths, preceding_run_widths + width);
 
   Range chars;
   Range glyphs;
   GetClusterAt(text_index, &chars, &glyphs);
-  const int cluster_begin_x = SkScalarRoundToInt(positions[glyphs.start()].x());
-  const int cluster_end_x = glyphs.end() < glyph_count ?
-      SkScalarRoundToInt(positions[glyphs.end()].x()) : width;
+  const float cluster_begin_x = positions[glyphs.start()].x();
+  const float cluster_end_x = glyphs.end() < glyph_count ?
+      positions[glyphs.end()].x() : SkFloatToScalar(width);
 
   // A cluster consists of a number of code points and corresponds to a number
   // of glyphs that should be drawn together. A cluster can contain multiple
@@ -562,13 +567,13 @@ Range TextRunHarfBuzz::GetGraphemeBounds(
           cluster_width * before / static_cast<float>(total));
       const int grapheme_end_x = cluster_begin_x + static_cast<int>(0.5f +
           cluster_width * (before + 1) / static_cast<float>(total));
-      return Range(preceding_run_widths_int + grapheme_begin_x,
-                   preceding_run_widths_int + grapheme_end_x);
+      return RangeF(preceding_run_widths + grapheme_begin_x,
+                    preceding_run_widths + grapheme_end_x);
     }
   }
 
-  return Range(preceding_run_widths_int + cluster_begin_x,
-               preceding_run_widths_int + cluster_end_x);
+  return RangeF(preceding_run_widths + cluster_begin_x,
+                preceding_run_widths + cluster_end_x);
 }
 
 }  // namespace internal
@@ -595,7 +600,7 @@ SelectionModel RenderTextHarfBuzz::FindCursorPosition(const Point& point) {
   EnsureLayout();
 
   int x = ToTextPoint(point).x();
-  int offset = 0;
+  float offset = 0;
   size_t run_index = GetRunContainingXCoord(x, &offset);
   if (run_index >= runs_.size())
     return EdgeSelectionModel((x < 0) ? CURSOR_LEFT : CURSOR_RIGHT);
@@ -645,8 +650,13 @@ Range RenderTextHarfBuzz::GetGlyphBounds(size_t index) {
     return Range(GetStringSize().width());
   const size_t layout_index = TextIndexToLayoutIndex(index);
   internal::TextRunHarfBuzz* run = runs_[run_index];
-  Range bounds = run->GetGraphemeBounds(grapheme_iterator_.get(), layout_index);
-  return run->is_rtl ? Range(bounds.end(), bounds.start()) : bounds;
+  RangeF bounds =
+      run->GetGraphemeBounds(grapheme_iterator_.get(), layout_index);
+  if (cursor_enabled() && run_index == runs_.size() - 1 &&
+      index == (run->is_rtl ? run->range.start() : run->range.end() - 1))
+    bounds.second = std::ceil(bounds.second);

[msw, 2014/11/24 21:51:00]

Add a comment here explaining why this is necessary. Should this also std::floor
the bounds.first for the leftmost char in RTL when the cursor is enabled?

[ckocagil, 2014/11/25 01:39:47]

Done.
Do you mean visually leftmost or logically? We handle visually rightmost for
both RTL and LTR. Visually leftmost isn't a problem since the first glyph always
starts at 0.

[msw, 2014/11/25 04:52:05]

Acknowledged.

+  return RoundRangeF(run->is_rtl ?
+      RangeF(bounds.second, bounds.first) : bounds);
 }
 
 int RenderTextHarfBuzz::GetLayoutTextBaseline() {
@@ -777,12 +787,12 @@ std::vector<Rect> RenderTextHarfBuzz::GetSubstringBounds(const Range& range) {
     if (!intersection.IsValid())
       continue;
     DCHECK(!intersection.is_reversed());
-    const Range leftmost_character_x = run->GetGraphemeBounds(
+    const Range leftmost_character_x = RoundRangeF(run->GetGraphemeBounds(
         grapheme_iterator_.get(),
-        run->is_rtl ? intersection.end() - 1 : intersection.start());
-    const Range rightmost_character_x = run->GetGraphemeBounds(
+        run->is_rtl ? intersection.end() - 1 : intersection.start()));
+    const Range rightmost_character_x = RoundRangeF(run->GetGraphemeBounds(
         grapheme_iterator_.get(),
-        run->is_rtl ? intersection.start() : intersection.end() - 1);
+        run->is_rtl ? intersection.start() : intersection.end() - 1));
     Range range_x(leftmost_character_x.start(), rightmost_character_x.end());
     DCHECK(!range_x.is_reversed());
     if (range_x.is_empty())
@@ -905,7 +915,6 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
   ApplyTextShadows(&renderer);
   ApplyCompositionAndSelectionStyles();
 
-  int current_x = 0;
   const Vector2d line_offset = GetLineOffset(0);
   for (size_t i = 0; i < runs_.size(); ++i) {
     const internal::TextRunHarfBuzz& run = *runs_[visual_to_logical_[i]];
@@ -914,11 +923,12 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
     renderer.SetFontRenderParams(run.render_params,
                                  background_is_transparent());
 
-    Vector2d origin = line_offset + Vector2d(current_x, lines()[0].baseline);
+    Vector2d origin = line_offset + Vector2d(0, lines()[0].baseline);
     scoped_ptr<SkPoint[]> positions(new SkPoint[run.glyph_count]);
     for (size_t j = 0; j < run.glyph_count; ++j) {
       positions[j] = run.positions[j];
-      positions[j].offset(SkIntToScalar(origin.x()), SkIntToScalar(origin.y()));
+      positions[j].offset(SkIntToScalar(origin.x()) + run.preceding_run_widths,
+                          SkIntToScalar(origin.y()));
     }
 
     for (BreakList<SkColor>::const_iterator it =
@@ -940,13 +950,11 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
                            colored_glyphs.length());
       int start_x = SkScalarRoundToInt(positions[colored_glyphs.start()].x());
       int end_x = SkScalarRoundToInt((colored_glyphs.end() == run.glyph_count) ?
-          (run.width + SkIntToScalar(origin.x())) :
+          (run.width + run.preceding_run_widths + SkIntToScalar(origin.x())) :
           positions[colored_glyphs.end()].x());
       renderer.DrawDecorations(start_x, origin.y(), end_x - start_x,
                                run.underline, run.strike, run.diagonal_strike);
     }
-
-    current_x += run.width;
   }
 
   renderer.EndDiagonalStrike();
@@ -966,12 +974,13 @@ size_t RenderTextHarfBuzz::GetRunContainingCaret(
   return runs_.size();
 }
 
-size_t RenderTextHarfBuzz::GetRunContainingXCoord(int x, int* offset) const {
+size_t RenderTextHarfBuzz::GetRunContainingXCoord(float x,
+                                                  float* offset) const {
   DCHECK(!needs_layout_);
   if (x < 0)
     return runs_.size();
   // Find the text run containing the argument point (assumed already offset).
-  int current_x = 0;
+  float current_x = 0;
   for (size_t i = 0; i < runs_.size(); ++i) {
     size_t run = visual_to_logical_[i];
     current_x += runs_[run]->width;
@@ -1133,7 +1142,7 @@ void RenderTextHarfBuzz::ShapeRun(internal::TextRunHarfBuzz* run) {
 #endif
 
   // Try shaping with the fallback fonts.
-  for (auto family : fallback_families) {
+  for (const auto& family : fallback_families) {
     if (family == primary_family)
       continue;
 #if defined(OS_WIN)