Type conversion fixes, text rendering edition.

ui/gfx/render_text.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.h"
 
 #include <algorithm>
 #include <climits>
 
 #include "base/command_line.h"
@@ skipping 251 matching lines @@
 }
 
 void SkiaTextRenderer::EndDiagonalStrike() {
   if (diagonal_) {
     diagonal_->Draw();
     diagonal_.reset();
   }
 }
 
 void SkiaTextRenderer::DrawUnderline(int x, int y, int width) {
-  SkRect r = SkRect::MakeLTRB(x, y + underline_position_, x + width,
-                              y + underline_position_ + underline_thickness_);
+  SkScalar x_scalar = SkIntToScalar(x);
+  SkRect r = SkRect::MakeLTRB(
+      x_scalar, y + underline_position_, x_scalar + width,
+      y + underline_position_ + underline_thickness_);
   if (underline_thickness_ == kUnderlineMetricsNotSet) {
     const SkScalar text_size = paint_.getTextSize();
     r.fTop = SkScalarMulAdd(text_size, kUnderlineOffset, y);
     r.fBottom = r.fTop + SkScalarMul(text_size, kLineThickness);
   }
   canvas_skia_->drawRect(r, paint_);
 }
 
 void SkiaTextRenderer::DrawStrike(int x, int y, int width) const {
   const SkScalar text_size = paint_.getTextSize();
   const SkScalar height = SkScalarMul(text_size, kLineThickness);
   const SkScalar offset = SkScalarMulAdd(text_size, kStrikeThroughOffset, y);
-  const SkRect r = SkRect::MakeLTRB(x, offset, x + width, offset + height);
+  SkScalar x_scalar = SkIntToScalar(x);
+  const SkRect r =
+      SkRect::MakeLTRB(x_scalar, offset, x_scalar + width, offset + height);
   canvas_skia_->drawRect(r, paint_);
 }
 
 SkiaTextRenderer::DiagonalStrike::DiagonalStrike(Canvas* canvas,
                                                  Point start,
                                                  const SkPaint& paint)
     : canvas_(canvas),
       start_(start),
       paint_(paint),
       total_length_(0) {
@@ skipping 10 matching lines @@
 void SkiaTextRenderer::DiagonalStrike::Draw() {
   const SkScalar text_size = paint_.getTextSize();
   const SkScalar offset = SkScalarMul(text_size, kDiagonalStrikeMarginOffset);
   const int thickness =
       SkScalarCeilToInt(SkScalarMul(text_size, kLineThickness) * 2);
   const int height = SkScalarCeilToInt(text_size - offset);
   const Point end = start_ + Vector2d(total_length_, -height);
   const int clip_height = height + 2 * thickness;
 
   paint_.setAntiAlias(true);
-  paint_.setStrokeWidth(thickness);
+  paint_.setStrokeWidth(SkIntToScalar(thickness));
 
   const bool clipped = pieces_.size() > 1;
   SkCanvas* sk_canvas = canvas_->sk_canvas();
   int x = start_.x();
 
   for (size_t i = 0; i < pieces_.size(); ++i) {
     paint_.setColor(pieces_[i].second);
 
     if (clipped) {
       canvas_->Save();
@@ skipping 385 matching lines @@
 
   return text_direction_;
 }
 
 VisualCursorDirection RenderText::GetVisualDirectionOfLogicalEnd() {
   return GetTextDirection() == base::i18n::LEFT_TO_RIGHT ?
       CURSOR_RIGHT : CURSOR_LEFT;
 }
 
 SizeF RenderText::GetStringSizeF() {
-  const Size size = GetStringSize();
-  return SizeF(size.width(), size.height());
+  return GetStringSize();

[msw, 2014/10/17 22:13:23]

Cool, I hadn't seen the gfx::Size operator SizeF() before.

 }
 
 float RenderText::GetContentWidth() {
   return GetStringSizeF().width() + (cursor_enabled_ ? 1 : 0);
 }
 
 int RenderText::GetBaseline() {
   if (baseline_ == kInvalidBaseline)
     baseline_ = DetermineBaselineCenteringText(display_rect(), font_list());
   DCHECK_NE(kInvalidBaseline, baseline_);
@@ skipping 115 matching lines @@
   return SelectionModel(sel.start(),
                         sel.is_reversed() ? CURSOR_BACKWARD : CURSOR_FORWARD);
 }
 
 const Vector2d& RenderText::GetUpdatedDisplayOffset() {
   UpdateCachedBoundsAndOffset();
   return display_offset_;
 }
 
 void RenderText::SetDisplayOffset(int horizontal_offset) {
-  const int extra_content = GetContentWidth() - display_rect_.width();
+  // !!! Should this round or ceil?

[msw, 2014/10/17 22:13:23]

Dan or Cem may have opinions here; we ultimately want float support everywhere.
For now, I'd naively suggest to ceil, expanding the content width to include
partially filled pixels, unless that breaks something, or conflicts with recent
eliding width treatment: https://codereview.chromium.org/631363002 

[Daniel Erat, 2014/10/17 23:20:05]

i have no opinion here.

[Peter Kasting, 2014/10/21 01:20:45]

It seems like ceiling here is a different behavior than the truncating the CL
you link does.  I can change this, but the inconsistency worries me.  I'm not
sure how to test whether this is really the right thing.

+  const int extra_content =
+      static_cast<int>(GetContentWidth()) - display_rect_.width();
   const int cursor_width = cursor_enabled_ ? 1 : 0;
 
   int min_offset = 0;
   int max_offset = 0;
   if (extra_content > 0) {
     switch (GetCurrentHorizontalAlignment()) {
       case ALIGN_LEFT:
         min_offset = -extra_content;
         break;
       case ALIGN_RIGHT:
@@ skipping 332 matching lines @@
       layout_text_.assign(text.substr(0, iter.getIndex()) + kEllipsisUTF16);
     }
   }
 
   if (elide_behavior_ != NO_ELIDE &&
       elide_behavior_ != FADE_TAIL &&
       !layout_text_.empty() &&
       static_cast<int>(GetContentWidth()) > display_rect_.width()) {
     // This doesn't trim styles so ellipsis may get rendered as a different
     // style than the preceding text. See crbug.com/327850.
-    layout_text_.assign(
-        Elide(layout_text_, display_rect_.width(), elide_behavior_));
+    layout_text_.assign(Elide(layout_text_,
+                              static_cast<float>(display_rect_.width()),
+                              elide_behavior_));
   }
 
   // Replace the newline character with a newline symbol in single line mode.
   static const base::char16 kNewline[] = { '\n', 0 };
   static const base::char16 kNewlineSymbol[] = { 0x2424, 0 };
   if (!multiline_ && replace_newline_chars_with_symbols_)
     base::ReplaceChars(layout_text_, kNewline, kNewlineSymbol, &layout_text_);
 
   ResetLayout();
 }
@@ skipping 179 matching lines @@
   SetDisplayOffset(display_offset_.x() + delta_x);
 }
 
 void RenderText::DrawSelection(Canvas* canvas) {
   const std::vector<Rect> sel = GetSubstringBounds(selection());
   for (std::vector<Rect>::const_iterator i = sel.begin(); i < sel.end(); ++i)
     canvas->FillRect(*i, selection_background_focused_color_);
 }
 
 }  // namespace gfx