Add Vector2d classes that represent offsets, instead of using Point.

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 "base/i18n/break_iterator.h"
 #include "base/logging.h"
@@ skipping 388 matching lines @@
   if (directionality_mode_ == DIRECTIONALITY_FROM_TEXT)
     text_direction_ = base::i18n::UNKNOWN_DIRECTION;
 
   UpdateObscuredText();
   ResetLayout();
 }
 
 void RenderText::SetHorizontalAlignment(HorizontalAlignment alignment) {
   if (horizontal_alignment_ != alignment) {
     horizontal_alignment_ = alignment;
-    display_offset_ = Point();
+    display_offset_ = Vector2d();
     cached_bounds_and_offset_valid_ = false;
   }
 }
 
 void RenderText::SetFontList(const FontList& font_list) {
   font_list_ = font_list;
   cached_bounds_and_offset_valid_ = false;
   ResetLayout();
 }
 
@@ skipping 338 matching lines @@
       focused_(false),
       composition_range_(ui::Range::InvalidRange()),
       obscured_(false),
       fade_head_(false),
       fade_tail_(false),
       background_is_transparent_(false),
       clip_to_display_rect_(true),
       cached_bounds_and_offset_valid_(false) {
 }
 
-const Point& RenderText::GetUpdatedDisplayOffset() {
+const Vector2d& RenderText::GetUpdatedDisplayOffset() {
   UpdateCachedBoundsAndOffset();
   return display_offset_;
 }
 
 SelectionModel RenderText::GetAdjacentSelectionModel(
     const SelectionModel& current,
     BreakType break_type,
     VisualCursorDirection direction) {
   EnsureLayout();
 
@@ skipping 53 matching lines @@
     StyleRange replacement_mode_style(default_style_);
     replacement_mode_style.foreground = selection_color_;
     size_t cursor = cursor_position();
     replacement_mode_style.range.set_start(cursor);
     replacement_mode_style.range.set_end(
         IndexOfAdjacentGrapheme(cursor, CURSOR_FORWARD));
     ApplyStyleRangeImpl(style_ranges, replacement_mode_style);
   }
 }
 
-Point RenderText::GetTextOrigin() {
-  Point origin = display_rect().origin();
-  origin = origin.Add(GetUpdatedDisplayOffset());
-  origin = origin.Add(GetAlignmentOffset());
-  return origin;
+Vector2d RenderText::GetTextOffset() {
+  Vector2d offset = display_rect().OffsetFromOrigin();
+  offset.Add(GetUpdatedDisplayOffset());
+  offset.Add(GetAlignmentOffset());
+  return offset;
 }
 
 Point RenderText::ToTextPoint(const Point& point) {
-  return point.Subtract(GetTextOrigin());
+  return point.Subtract(GetTextOffset());
 }
 
 Point RenderText::ToViewPoint(const Point& point) {
-  return point.Add(GetTextOrigin());
+  return point.Add(GetTextOffset());
 }
 
 int RenderText::GetContentWidth() {
   return GetStringSize().width() + (cursor_enabled_ ? 1 : 0);
 }
 
-Point RenderText::GetAlignmentOffset() {
-  if (horizontal_alignment() != ALIGN_LEFT) {
-    int x_offset = display_rect().width() - GetContentWidth();
-    if (horizontal_alignment() == ALIGN_CENTER)
-      x_offset /= 2;
-    return Point(x_offset, 0);
-  }
-  return Point();
+Vector2d RenderText::GetAlignmentOffset() {
+  if (horizontal_alignment() == ALIGN_LEFT)
+    return Vector2d();
+
+  int x_offset = display_rect().width() - GetContentWidth();
+  if (horizontal_alignment() == ALIGN_CENTER)
+    x_offset /= 2;
+  return Vector2d(x_offset, 0);
 }
 
 Point RenderText::GetOriginForDrawing() {
-  Point origin(GetTextOrigin());
-  const int height = GetStringSize().height();
   // Center the text vertically in the display area.
-  origin.Offset(0, (display_rect().height() - height) / 2);
-  return origin;
+  return gfx::PointAtOffsetFromOrigin(
+      GetTextOffset() +
+      Vector2d(0, (display_rect().height() - GetStringSize().height()) / 2));

[Peter Kasting, 2012/10/31 01:04:41]

Nit: Technically this line would need to be indented by 4 more spaces.

I'd fix this by instead putting "GetTextOffset() +" on the end of the previous
line and leaving this line alone.

[danakj, 2012/10/31 16:55:46]

Then the Vector2d() line needs to line up below the open brace for
PointAtOffsetFromOrigin, right? And ends up spanning like 3 lines. I'll use a
temp var here.

[Peter Kasting, 2012/10/31 17:13:41]

No, it doesn't need to (and in fact shouldn't) line up.  Subsequent lines get
indented 4, only function args get indented even.  (And for loops, for some
reason.)

Or at least that's how I've understood our typical style...

[danakj, 2012/10/31 17:15:00]

Oh! I was adding the two offsets, then making a point. You're thinking to make a
point then add the offset. I'll do that :)

[Peter Kasting, 2012/10/31 17:18:41]

Uh... I was?

I thought I was advocating for

  return gfx::PointAtOffsetFromOrigin(GetTextOffset() +
      Vector2d(0, (display_rect().height() - GetStringSize().height()) / 2));

But maybe you're saying we could do:

  gfx::Point p(0, (display_rect().height() - GetStringSize().height()) / 2);
  return p + GetTextOffset();

...which I hadn't thought of, but is also fine.

[Peter Kasting, 2012/10/31 17:19:53]

Incidentally, I suspect that this code's callers might really want a vector
offset rather than a point.  But I haven't checked.

 }
 
 void RenderText::ApplyFadeEffects(internal::SkiaTextRenderer* renderer) {
   if (!fade_head() && !fade_tail())
     return;
 
   const int text_width = GetStringSize().width();
   const int display_width = display_rect().width();
 
   // If the text fits as-is, no need to fade.
@@ skipping 80 matching lines @@
   // First, set the valid flag true to calculate the current cursor bounds using
   // the stale |display_offset_|. Applying |delta_offset| at the end of this
   // function will set |cursor_bounds_| and |display_offset_| to correct values.
   cached_bounds_and_offset_valid_ = true;
   cursor_bounds_ = GetCursorBounds(selection_model_, insert_mode_);
 
   // Update |display_offset_| to ensure the current cursor is visible.
   const int display_width = display_rect_.width();
   const int content_width = GetContentWidth();
 
-  int delta_offset = 0;
+  int delta_x = 0;
   if (content_width <= display_width || !cursor_enabled()) {
     // Don't pan if the text fits in the display width or when the cursor is
     // disabled.
-    delta_offset = -display_offset_.x();
+    delta_x = -display_offset_.x();
   } else if (cursor_bounds_.right() >= display_rect_.right()) {
     // TODO(xji): when the character overflow is a RTL character, currently, if
     // we pan cursor at the rightmost position, the entered RTL character is not
     // displayed. Should pan cursor to show the last logical characters.
     //
     // Pan to show the cursor when it overflows to the right,
-    delta_offset = display_rect_.right() - cursor_bounds_.right() - 1;
+    delta_x = display_rect_.right() - cursor_bounds_.right() - 1;
   } else if (cursor_bounds_.x() < display_rect_.x()) {
     // TODO(xji): have similar problem as above when overflow character is a
     // LTR character.
     //
     // Pan to show the cursor when it overflows to the left.
-    delta_offset = display_rect_.x() - cursor_bounds_.x();
+    delta_x = display_rect_.x() - cursor_bounds_.x();
   } else if (display_offset_.x() != 0) {
     // Reduce the pan offset to show additional overflow text when the display
     // width increases.
     const int negate_rtl = horizontal_alignment_ == ALIGN_RIGHT ? -1 : 1;
     const int offset = negate_rtl * display_offset_.x();
-    if (display_width > (content_width + offset))
-      delta_offset = negate_rtl * (display_width - (content_width + offset));
+    if (display_width > (content_width + offset)) {
+      delta_x = negate_rtl * (display_width - (content_width + offset));
+    }
   }
 
-  display_offset_.Offset(delta_offset, 0);
-  cursor_bounds_.Offset(delta_offset, 0);
+  gfx::Vector2d delta_offset(delta_x, 0);
+  display_offset_ += delta_offset;
+  cursor_bounds_.Offset(delta_offset);
 }
 
 void RenderText::DrawSelection(Canvas* canvas) {
   const SkColor color = focused() ? selection_background_focused_color_ :
                                     selection_background_unfocused_color_;
   const std::vector<Rect> sel = GetSubstringBounds(selection());
   for (std::vector<Rect>::const_iterator i = sel.begin(); i < sel.end(); ++i)
     canvas->FillRect(*i, color);
 }
 
 void RenderText::DrawCursor(Canvas* canvas) {
   // Paint cursor. Replace cursor is drawn as rectangle for now.
   // TODO(msw): Draw a better cursor with a better indication of association.
   if (cursor_enabled() && cursor_visible() && focused()) {
     const Rect& bounds = GetUpdatedCursorBounds();
     if (bounds.width() != 0)
       canvas->FillRect(bounds, cursor_color_);
     else
       canvas->DrawRect(bounds, cursor_color_);
   }
 }
 
 }  // namespace gfx