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 1323 matching lines @@
     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.
     const int negate_rtl = horizontal_alignment_ == ALIGN_RIGHT ? -1 : 1;

[msw, 2014/07/11 22:33:04]

nit: consider combining the two scale factors.

-    const int offset = negate_rtl * display_offset_.x();
+    const int double_center = horizontal_alignment_ == ALIGN_CENTER ? 2 : 1;

[msw, 2014/07/11 22:33:04]

nit: consider |scale_center|

+    const int offset = negate_rtl * double_center * display_offset_.x();
     if (display_width > (content_width + offset)) {

[msw, 2014/07/11 22:33:04]

I don't think this condition will capture all the necessary cases. Consider a
10px display width, with a 50px text width, offset by -10px. Now, increase the
display width to 21px. This condition will not fire, because 21 < 50 + 2 *
(-10),  but that'll leave some extra empty space on the right...

I don't understand the |double_center| scale factor for |offset| and wonder if
we need to consider the value of |GetAlignmentOffset|. Can you take a closer
look and more clearly explain what's going on (especially if I'm incorrect)?

[mohsen, 2014/07/14 20:57:01]

In your example, no extra empty space will be shown on the right. Suppose center
of textfield is fixed at point 0. If |display_width| is 21 and |content_width|
is 50, textfield will cover [-10.5, 10.5] and text will cover [-25, 25], when
there is no offset. Now adding offset of -10 shifts the text 10 pixels to left,
so it will cover [-35, 15] which is still covering the entire textfield.

As for |double_center| scale factor, maybe it's best to think of it as follows:
in case of center-aligned text, if we divide both text and textfield in half and
only consider their right halves, it's like they're left-aligned and all
calculations for left-aligned texts would work.

The above explanation works when offset is less than zero. If offset is greater
than zero, then we should use calculations for right-aligned texts. I haven't
handled this case here (my mistake) and adding it will make this code more
complicated. Instead, I'm going to switch to using SetDisplayOffset() function
which handles all these cases.

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

I think this is wrong, the alignment offset 
Ah, I didn't account for GetAlignmentOffset.

-      delta_x = negate_rtl * (display_width - (content_width + offset));
+      delta_x = negate_rtl * (display_width - (content_width + offset)) /
+          double_center;
     }
   }
 
   Vector2d delta_offset(delta_x, 0);
   display_offset_ += delta_offset;
   cursor_bounds_ += delta_offset;
 }
 
 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