Add RenderText DirectionalityMode enum and support; etc.

ui/views/controls/label.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/views/controls/label.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <vector>
 
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "ui/base/accessibility/accessible_view_state.h"
 #include "ui/base/native_theme/native_theme.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/text/text_elider.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/insets.h"
+#include "ui/gfx/text_constants.h"
 #include "ui/views/background.h"
 
 namespace views {
 
 // static
 const char Label::kViewClassName[] = "views/Label";
 
 const int Label::kFocusBorderPadding = 1;
 
 Label::Label() {
@@ skipping 60 matching lines @@
 void Label::SetShadowOffset(int x, int y) {
   shadow_offset_.SetPoint(x, y);
 }
 
 void Label::ClearEmbellishing() {
   has_shadow_ = false;
 }
 
 void Label::SetHorizontalAlignment(Alignment alignment) {
   // If the View's UI layout is right-to-left and directionality_mode_ is
-  // USE_UI_DIRECTIONALITY, we need to flip the alignment so that the alignment
-  // settings take into account the text directionality.
-  if (base::i18n::IsRTL() && (directionality_mode_ == USE_UI_DIRECTIONALITY) &&
-      (alignment != ALIGN_CENTER))
+  // gfx::DIRECTIONALITY_FROM_UI, we need to flip the alignment so that the
+  // alignment settings take into account the text directionality.
+  if (base::i18n::IsRTL() && (alignment != ALIGN_CENTER) &&
+      (directionality_mode_ == gfx::DIRECTIONALITY_FROM_UI))

[xji, 2012/07/30 18:32:16]

I can see that you are trying to re-use gfx::DirectionalityMode, but I feel the
name is pretty confusing here (at least I was confused at first).

The gfx::DIRECTIONALITY_FROM_UI/TEXT here is used to control whether the
alignment is going to be flipped or not (or whether the label's
directionality_mode is a forced one or not).
But in RenderText, in the current implementation, DIRECTIONALITY_FROM_TEXT is
the default, DIRECTIONALITY_FROM_UI means text directionality is based on UI.

the combinations are as following:
Label's DIRECTIONALITY_FROM_UI --> (falls to) RenderText's
DIRECTIONALTY_FROM_TEXT.
Label's DIRECTIONALITY_FROM_TEXT --> (falls to) RenderText's
DIRECTIONALITY_FORCE_xxx.

[msw, 2012/07/31 03:03:06]

You're right; Label doesn't currently support gfx::DIRECTIONALITY_FROM_UI.
I reverted this portion of the CL and will followup in a separate CL.

     alignment = (alignment == ALIGN_LEFT) ? ALIGN_RIGHT : ALIGN_LEFT;
   if (horiz_alignment_ != alignment) {
     horiz_alignment_ = alignment;
     SchedulePaint();
   }
 }
 
 void Label::SetMultiLine(bool multi_line) {
   DCHECK(!multi_line || !elide_in_middle_);
   if (multi_line != is_multi_line_) {
@@ skipping 236 matching lines @@
   background_color_ = ui::NativeTheme::instance()->GetSystemColor(
       ui::NativeTheme::kColorId_LabelBackgroundColor);
   auto_color_readability_ = true;
   RecalculateColors();
   horiz_alignment_ = ALIGN_CENTER;
   is_multi_line_ = false;
   allow_character_break_ = false;
   elide_in_middle_ = false;
   is_email_ = false;
   collapse_when_hidden_ = false;
-  directionality_mode_ = USE_UI_DIRECTIONALITY;
+  directionality_mode_ = gfx::DIRECTIONALITY_FROM_UI;
   paint_as_focused_ = false;
   has_focus_border_ = false;
   enabled_shadow_color_ = 0;
   disabled_shadow_color_ = 0;
   shadow_offset_.SetPoint(1, 1);
   has_shadow_ = false;
 
   SetText(text);
 }
 
@@ skipping 50 matching lines @@
   return gfx::Rect(text_origin, text_size);
 }
 
 int Label::ComputeDrawStringFlags() const {
   int flags = 0;
 
   // We can't use subpixel rendering if the background is non-opaque.
   if (SkColorGetA(background_color_) != 0xFF)
     flags |= gfx::Canvas::NO_SUBPIXEL_RENDERING;
 
-  if (directionality_mode_ == AUTO_DETECT_DIRECTIONALITY) {
-    base::i18n::TextDirection direction =
+  if (directionality_mode_ == gfx::DIRECTIONALITY_FROM_TEXT) {
+    const base::i18n::TextDirection direction =

[xji, 2012/07/30 19:00:04]

if we introduce gfx::Canvas::DIRECTIONALITY_FROM_TEXT, then, we do not need the
computation here. we can just do: 
flags |= gfx::Canvas::DIRECTIONALITY_FROM_TEXT

And in "bool AdjustStringDirection()" in canvas_skia.cc,
we need to add:

else if (flags & gfx::Canvas::DIRECTIONALITY_FROM_TEXT)
  render_text->SetDirectionalityMode(gfx::DIRECTIONALITY_FROM_TEXT);


So far, there is no real usage of FORCE directionality (they are actually the
current DIRECTIONALITY_FROM_TEXT. They were introduced before is because we
previously use DIRECTIONALITY_FROM_UI as default).
But I think the forced directionality might be useful later for the title page
case (http://code.google.com/p/chromium/issues/detail?id=27094). 

[msw, 2012/07/31 03:03:06]

I've reverted these changes in this CL and I'll follup in a separate CL.
(DIRECTIONALITY_FROM_TEXT is the default underlying behavior and this class
currently doesn't actually propagate DIRECTIONALITY_FROM_UI, so this block can
be removed altogether).

         base::i18n::GetFirstStrongCharacterDirection(text_);
     if (direction == base::i18n::RIGHT_TO_LEFT)
       flags |= gfx::Canvas::FORCE_RTL_DIRECTIONALITY;
     else
       flags |= gfx::Canvas::FORCE_LTR_DIRECTIONALITY;
   }
 
   if (!is_multi_line_)
     return flags;
 
@@ skipping 42 matching lines @@
                                 ui::ELIDE_IN_MIDDLE);
   } else {
     *paint_text = text_;
   }
 
   *text_bounds = GetTextBounds();
   *flags = ComputeDrawStringFlags();
 }
 
 }  // namespace views