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 882 matching lines @@
                         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();
+  const int cursor_width = cursor_enabled_ ? 1 : 0;
 
   int min_offset = 0;
   int max_offset = 0;
   if (extra_content > 0) {
-    switch (horizontal_alignment_) {
+    switch (GetCurrentHorizontalAlignment()) {
       case ALIGN_LEFT:
         min_offset = -extra_content;
         break;
       case ALIGN_RIGHT:
         max_offset = extra_content;
         break;
       case ALIGN_CENTER:
-        min_offset = -extra_content / 2;
-        max_offset = extra_content / 2;
+        min_offset = -(extra_content - cursor_width + 1) / 2 - cursor_width;

[msw, 2014/07/22 22:00:27]

nit: It's a bit odd that ALIGN_[LEFT|RIGHT] consider the extra cursor width part
of the text width to be offset, but ALIGN_CENTER doesn't. I suppose that may be
reasonable (at least when the content width is smaller than the display width),
but I think this deserves a comment at least.

[mohsen, 2014/07/22 23:38:35]

I'm not getting exactly what you mean here. The extra pixel for cursor is
considered in all cases (it is considered in GetcontentWidth() if cursor is
enabled). But, for ALIGN_CENTER we need to subtract it, do the calculations, and
then add it back.

[msw, 2014/07/23 03:08:16]

That's exactly what I'm saying is confusing, and warrants an explanatory
comment, that you "need to subtract it, do the calculations, and then add it
back." Add a comment explaining why you need to do that.

+        max_offset = (extra_content - cursor_width) / 2;
         break;
       default:
         break;
     }
   }
   if (horizontal_offset < min_offset)
     horizontal_offset = min_offset;
   else if (horizontal_offset > max_offset)
     horizontal_offset = max_offset;
 
@@ skipping 466 matching lines @@
   username = Elide(username, available_width, ELIDE_TAIL);
   return username + kAtSignUTF16 + domain;
 }
 
 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 move 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
-    // width increases.
-    HorizontalAlignment horizontal_alignment = GetCurrentHorizontalAlignment();
-    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));
-    }
+    // TODO(bidi): Show RTL glyphs at the cursor position for ALIGN_LEFT, etc.
+    if (cursor_bounds_.right() > display_rect_.right())
+      delta_x = display_rect_.right() - cursor_bounds_.right();
+    else if (cursor_bounds_.x() < display_rect_.x())
+      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