RenderText: handle center-aligned text in UpdateCachedBoundsAndOffset()

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 874 matching lines @@
   int max_offset = 0;
   if (extra_content > 0) {
     switch (horizontal_alignment_) {
       case ALIGN_LEFT:
         min_offset = -extra_content;
         break;
       case ALIGN_RIGHT:
         max_offset = extra_content;
         break;
       case ALIGN_CENTER:
-        min_offset = -extra_content / 2;
+        min_offset = -(extra_content + 1) / 2;
         max_offset = extra_content / 2;
         break;
       default:
         break;
     }
   }
   if (horizontal_offset < min_offset)
     horizontal_offset = min_offset;
   else if (horizontal_offset > max_offset)
     horizontal_offset = max_offset;
@@ skipping 396 matching lines @@
 
   return render_text->text();
 }
 
 void RenderText::UpdateCachedBoundsAndOffset() {
   if (cached_bounds_and_offset_valid_)
     return;
 
   // TODO(ckocagil): Add support for scrolling multiline text.
 
-  // 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;
-  if (cursor_enabled())
+  int delta_x = 0;
+
+  if (cursor_enabled()) {
+    // When cursor is enabled, ensure it is visible. For this, set the valid
+    // flag true and calculate the current cursor bounds using the stale
+    // |display_offset_|. Then calculate the change in offset needed to to move

[msw, 2014/07/16 01:41:43]

nit: fix "to to"

[mohsen, 2014/07/16 20:48:10]

Done.

+    // the cursor into the visible area.
+    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_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_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_x = display_rect_.right() - cursor_bounds_.right();
-  } 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_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

[msw, 2014/07/16 01:41:43]

Does the new code still do this via min/max clamping in SetDisplayOffset?

[mohsen, 2014/07/16 20:48:10]

Yes.

-    // 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_x = negate_rtl * (display_width - (content_width + offset));
+    if (cursor_bounds_.right() > display_rect_.right()) {
+      // TODO(xji): when the character overflow is a RTL character, currently,

[msw, 2014/07/16 01:41:43]

nit: consolidate these two as TODO(bidi) above the if/else

[mohsen, 2014/07/16 20:48:10]

Done. Please check if the result is satisfactory.

+      // 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.

[msw, 2014/07/16 01:41:43]

nit: remove the "pan to show" comments, the top comment is sufficient.

[mohsen, 2014/07/16 20:48:10]

Done.

+      delta_x = display_rect_.right() - cursor_bounds_.right();
+    } 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_x = display_rect_.x() - cursor_bounds_.x();
     }
   }
 
-  Vector2d delta_offset(delta_x, 0);
-  display_offset_ += delta_offset;
-  cursor_bounds_ += delta_offset;
+  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