Windows implementation of multiline RenderText

ui/gfx/render_text_unittest.cc

Index: ui/gfx/render_text_unittest.cc
diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc
index 263ef92d8159c7088650ddd538457e49061d7ea0..27b1427d2f6c1a04fe8550b2b09e6c76733470bb 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "base/format_macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -1143,10 +1144,6 @@ TEST_F(RenderTextTest, StringSizeEmptyString) {
   EXPECT_EQ(font_list.GetHeight(), render_text->GetStringSize().height());
   EXPECT_EQ(0, render_text->GetStringSize().width());
   EXPECT_EQ(font_list.GetBaseline(), render_text->GetBaseline());
-
-  render_text->SetText(UTF8ToUTF16(" "));

[msw, 2013/09/11 17:32:37]

Why did you remove this?

[ckocagil, 2013/09/11 20:31:16]

Added back. See my comment above LineBreaker in render_text_win.cc.

-  EXPECT_EQ(font_list.GetHeight(), render_text->GetStringSize().height());
-  EXPECT_EQ(font_list.GetBaseline(), render_text->GetBaseline());
 }
 #endif  // !defined(OS_MACOSX)
 
@@ -1182,17 +1179,15 @@ TEST_F(RenderTextTest, StringSizeRespectsFontListMetrics) {
   EXPECT_EQ(smaller_font.GetBaseline(), render_text->GetBaseline());
 
   // Layout the same text with mixed fonts.  The text should be rendered with
-  // the smaller font, but the height and baseline are determined with the
-  // metrics of the font list, which is equal to the larger font.
+  // the smaller font, and the height and baseline are determined with the
+  // metrics of the smaller font.

[msw, 2013/09/11 17:32:37]

Why has this changed? I think it was correct before, so RenderText
heights/baselines won't change if text changes wind up using other fonts in the
list. Perhaps that's not needed if RenderText users adjust the placement to
account for font list metrics, but that's not the case right now, afaik, and
this change will probably break some omnibox/textfield alignment. Can you
restore the previous behavior?

[ckocagil, 2013/09/11 20:31:16]

Reverted the changes to existing tests. See my comment above LineBreaker in
render_text_win.cc.

   std::vector<Font> fonts;
   fonts.push_back(smaller_font);  // The primary font is the smaller font.
   fonts.push_back(larger_font);
   const FontList font_list(fonts);
   render_text->SetFontList(font_list);
-  EXPECT_LT(smaller_font.GetHeight(), render_text->GetStringSize().height());
-  EXPECT_LT(smaller_font.GetBaseline(), render_text->GetBaseline());
-  EXPECT_EQ(font_list.GetHeight(), render_text->GetStringSize().height());
-  EXPECT_EQ(font_list.GetBaseline(), render_text->GetBaseline());
+  EXPECT_EQ(smaller_font.GetHeight(), render_text->GetStringSize().height());
+  EXPECT_EQ(smaller_font.GetBaseline(), render_text->GetBaseline());
 }
 
 TEST_F(RenderTextTest, SetFont) {
@@ -1294,7 +1289,7 @@ TEST_F(RenderTextTest, GetTextOffset) {
   Rect display_rect(font_size);
   render_text->SetDisplayRect(display_rect);
 
-  Vector2d offset = render_text->GetTextOffset();
+  Vector2d offset = render_text->GetLineOffset(0);
   EXPECT_TRUE(offset.IsZero());
 
   // Set display area's size greater than font size.
@@ -1303,30 +1298,30 @@ TEST_F(RenderTextTest, GetTextOffset) {
   render_text->SetDisplayRect(display_rect);
 
   // Check the default horizontal and vertical alignment.
-  offset = render_text->GetTextOffset();
+  offset = render_text->GetLineOffset(0);
   EXPECT_EQ(kEnlargement / 2, offset.y());
   EXPECT_EQ(0, offset.x());
 
   // Check explicitly setting the horizontal alignment.
   render_text->SetHorizontalAlignment(ALIGN_LEFT);
-  offset = render_text->GetTextOffset();
+  offset = render_text->GetLineOffset(0);
   EXPECT_EQ(0, offset.x());
   render_text->SetHorizontalAlignment(ALIGN_CENTER);
-  offset = render_text->GetTextOffset();
+  offset = render_text->GetLineOffset(0);
   EXPECT_EQ(kEnlargement / 2, offset.x());
   render_text->SetHorizontalAlignment(ALIGN_RIGHT);
-  offset = render_text->GetTextOffset();
+  offset = render_text->GetLineOffset(0);
   EXPECT_EQ(kEnlargement, offset.x());
 
   // Check explicitly setting the vertical alignment.
   render_text->SetVerticalAlignment(ALIGN_TOP);
-  offset = render_text->GetTextOffset();
+  offset = render_text->GetLineOffset(0);
   EXPECT_EQ(0, offset.y());
   render_text->SetVerticalAlignment(ALIGN_VCENTER);
-  offset = render_text->GetTextOffset();
+  offset = render_text->GetLineOffset(0);
   EXPECT_EQ(kEnlargement / 2, offset.y());
   render_text->SetVerticalAlignment(ALIGN_BOTTOM);
-  offset = render_text->GetTextOffset();
+  offset = render_text->GetLineOffset(0);
   EXPECT_EQ(kEnlargement, offset.y());
 
   SetRTL(was_rtl);
@@ -1334,7 +1329,7 @@ TEST_F(RenderTextTest, GetTextOffset) {
 
 TEST_F(RenderTextTest, GetTextOffsetHorizontalDefaultInRTL) {
   // This only checks the default horizontal alignment in RTL mode; all other
-  // GetTextOffset() attributes are checked by the test above.
+  // GetLineOffset(0) attributes are checked by the test above.
   const bool was_rtl = base::i18n::IsRTL();
   SetRTL(true);
   scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
@@ -1345,7 +1340,7 @@ TEST_F(RenderTextTest, GetTextOffsetHorizontalDefaultInRTL) {
                        render_text->GetStringSize().height());
   Rect display_rect(font_size);
   render_text->SetDisplayRect(display_rect);
-  Vector2d offset = render_text->GetTextOffset();
+  Vector2d offset = render_text->GetLineOffset(0);
   EXPECT_EQ(kEnlargement, offset.x());
   SetRTL(was_rtl);
 }
@@ -1650,5 +1645,78 @@ TEST_F(RenderTextTest, SelectionKeepsLigatures) {
     render_text->MoveCursorTo(SelectionModel(0, CURSOR_FORWARD));
   }
 }
+
+#if defined(OS_WIN)
+TEST_F(RenderTextTest, Multiline_MinWidth) {

[msw, 2013/09/11 17:32:37]

nit: Add high-level comments explaining what each test does, like:
// Ensure strings wrap onto multiple lines for a small available width.

[ckocagil, 2013/09/11 20:31:16]

Done.

+  const wchar_t* kTestStrings[] = { kWeak, kLtr, kLtrRtl, kLtrRtlLtr, kRtl,
+                                    kRtlLtr, kRtlLtrRtl };
+
+  scoped_ptr<RenderTextWin> render_text(
+      static_cast<RenderTextWin*>(RenderText::CreateInstance()));
+  render_text->SetDisplayRect(Rect(Point(), Size(1, 1000)));
+  render_text->SetMultiline(true);
+  Canvas canvas;
+
+  for (size_t i = 0; i < arraysize(kTestStrings); ++i) {
+    SCOPED_TRACE(base::StringPrintf("kTestStrings[%" PRIuS "]", i));
+    render_text->SetText(WideToUTF16(kTestStrings[i]));
+    render_text->Draw(&canvas);
+    EXPECT_GT(render_text->lines_.size(), 1U);
+  }
+}
+
+TEST_F(RenderTextTest, Multiline_NormalWidth) {
+  const struct {
+    const wchar_t* const text;
+    const ui::Range first_line_char_range;
+    const ui::Range second_line_char_range;
+  } kTestStrings[] = {
+    { L"abc defg hijkl", ui::Range(0, 9), ui::Range(9, 14) },
+    { L"qwertyuiop",     ui::Range(0, 8), ui::Range(8, 10) },
+    { L"\x0647\x0630\x0627 \x0647\x0648 \x0627\x0644\x0628\x0627\x0628",
+                         ui::Range(0, 7), ui::Range(7, 12) }
+  };
+
+  scoped_ptr<RenderTextWin> render_text(
+      static_cast<RenderTextWin*>(RenderText::CreateInstance()));
+  render_text->SetDisplayRect(Rect(Point(), Size(50, 1000)));
+  render_text->SetMultiline(true);
+  Canvas canvas;
+
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kTestStrings); ++i) {
+    SCOPED_TRACE(base::StringPrintf("kTestStrings[%" PRIuS "]", i));
+    render_text->SetText(WideToUTF16(kTestStrings[i].text));
+    render_text->Draw(&canvas);
+    CheckLineIntegrity(render_text->lines_, render_text->runs_);

[msw, 2013/09/11 17:32:37]

You shouldn't need to call this explicitly here, it happens all the time.

[ckocagil, 2013/09/11 20:31:16]

Done.

+    ASSERT_EQ(2U, render_text->lines_.size());
+    ASSERT_EQ(1U, render_text->lines_[0].segments.size());
+    EXPECT_EQ(kTestStrings[i].first_line_char_range,
+              render_text->lines_[0].segments[0].char_range);
+    ASSERT_EQ(1U, render_text->lines_[1].segments.size());
+    EXPECT_EQ(kTestStrings[i].second_line_char_range,
+              render_text->lines_[1].segments[0].char_range);
+  }
+}
+
+TEST_F(RenderTextTest, Multiline_SufficientWidth) {
+  const wchar_t* kTestStrings[] = { L"", L" ",
+                                    L".", L" . ",
+                                    L"abc", L"a b c",
+                                    L"\x62E\x628\x632", L"\x62E \x628 \x632" };
+
+  scoped_ptr<RenderTextWin> render_text(
+      static_cast<RenderTextWin*>(RenderText::CreateInstance()));
+  render_text->SetDisplayRect(Rect(Point(), Size(30, 1000)));
+  render_text->SetMultiline(true);
+  Canvas canvas;
+
+  for (size_t i = 0; i < arraysize(kTestStrings); ++i) {
+    SCOPED_TRACE(base::StringPrintf("kTestStrings[%" PRIuS "]", i));
+    render_text->SetText(WideToUTF16(kTestStrings[i]));
+    render_text->Draw(&canvas);
+    EXPECT_EQ(1U, render_text->lines_.size());
+  }
+}
+#endif  // defined(OS_WIN)
 
 }  // namespace gfx