[RenderText] Adding vertical alignment options

ui/gfx/render_text_unittest.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 "base/format_macros.h"
 #include "base/i18n/break_iterator.h"
@@ skipping 184 matching lines @@
                                      << ", width " << width << ", stride_ "
                                      << stride_;
     for (int y = top; y < top + height; ++y) {
       for (int x = left; x < left + width; ++x) {
         SkColor buffer_color = buffer_[x + y * stride_];
         EXPECT_EQ(color, buffer_color) << string_ << " at " << x << ", " << y;
       }
     }
   }
 
+  void EnsureSolidBorder(SkColor color, const Rect& rect) const {
+    {
+      SCOPED_TRACE("EnsureSolidBorder Top Side");
+      EnsureSolidRect(SK_ColorWHITE, 0, 0, stride_, rect.y());
+    }
+    {
+      SCOPED_TRACE("EnsureSolidBorder Bottom Side");
+      EnsureSolidRect(SK_ColorWHITE, 0, rect.y() + rect.height(), stride_,

[msw, 2015/03/26 22:55:45]

rect.y() + rect.height() == rect.bottom()

[dschuyler, 2015/03/27 21:20:35]

Done.

+                      row_count_ - rect.y() - rect.height());

[msw, 2015/03/26 22:55:45]

- rect.y() - rect.height() == - rect.bottom()

[dschuyler, 2015/03/27 21:20:35]

Done.

+    }
+    {
+      SCOPED_TRACE("EnsureSolidBorder Left Side");
+      EnsureSolidRect(SK_ColorWHITE, 0, rect.y(), rect.x(), rect.height());
+    }
+    {
+      SCOPED_TRACE("EnsureSolidBorder Right Side");
+      EnsureSolidRect(SK_ColorWHITE, rect.x() + rect.width(), rect.y(),

[msw, 2015/03/26 22:55:45]

rect.x() + rect.width() == rect.right()

[dschuyler, 2015/03/27 21:20:35]

Done.

+                      stride_ - rect.x() - rect.width(), rect.height());

[msw, 2015/03/26 22:55:45]

- rect.x() - rect.width() == - rect.right()

[dschuyler, 2015/03/27 21:20:34]

Done.

+    }
+  }
+
  private:
   const wchar_t* string_;
   const SkColor* buffer_;
   int stride_;
   int row_count_;
 
   DISALLOW_COPY_AND_ASSIGN(TestRectangleBuffer);
 };
 
 }  // namespace
@@ skipping 2370 matching lines @@
   render_text->SetText(kString);
   render_text->ApplyStyle(BOLD, true, Range(0, 3));
   render_text->SetElideBehavior(ELIDE_TAIL);
 
   render_text->SetDisplayRect(Rect(0, 0, 500, 100));
   EXPECT_EQ(kString, render_text->GetDisplayText());
   render_text->SetDisplayRect(Rect(0, 0, render_text->GetContentWidth(), 100));
   EXPECT_EQ(kString, render_text->GetDisplayText());
 }
 
+TEST_F(RenderTextTest, VerticalAlignement) {

[msw, 2015/03/26 22:55:45]

nit: "Alignment"

[dschuyler, 2015/03/27 21:20:35]

Done.

+  SCOPED_TRACE("VerticalAlignement");

[msw, 2015/03/26 22:55:45]

What's the use of this SCOPED_TRACE? The failing test should be readily
identifiable without this. (otherwise, ditto nit: "Alignment")

[dschuyler, 2015/03/27 21:20:34]

Done.

+  scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
+  const wchar_t* string = L"g Hello g";
+  render_text->SetText(WideToUTF16(string));
+  render_text->ApplyStyle(BOLD, true, Range(0, 3));
+  const Size kCanvasSize(300, 50);
+  const int kTestSize = 10;
+  const Size string_size = render_text->GetStringSize();
+  const int kCenterBegin = (kCanvasSize.width() - string_size.width()) / 2;
+  const int kRightBegin = kCanvasSize.width() - string_size.width() - kTestSize;
+  const int kMiddleBegin = (kCanvasSize.height() - string_size.height()) / 2;
+  const int kBottomBegin =
+      kCanvasSize.height() - string_size.height() - kTestSize;
+
+  struct Tests {
+    HorizontalAlignment horizontal;
+    VerticalAlignment vertical;
+    int x;
+    int y;
+  } const kTests[] = {
+      {ALIGN_LEFT, VALIGN_TOP, kTestSize, kTestSize},

[msw, 2015/03/26 22:55:45]

nit: outdent all the these lines by 2 spaces

[dschuyler, 2015/03/27 21:20:35]

Afaik, it's an outstanding bug in cl format.  I.e. this is the git cl format of
this code :(

+      {ALIGN_LEFT, VALIGN_MIDDLE, kTestSize, kMiddleBegin},
+      {ALIGN_LEFT, VALIGN_BOTTOM, kTestSize, kBottomBegin},
+      {ALIGN_CENTER, VALIGN_TOP, kCenterBegin, kTestSize},
+      {ALIGN_CENTER, VALIGN_MIDDLE, kCenterBegin, kMiddleBegin},
+      {ALIGN_CENTER, VALIGN_BOTTOM, kCenterBegin, kBottomBegin},
+      {ALIGN_RIGHT, VALIGN_TOP, kRightBegin, kTestSize},
+      {ALIGN_RIGHT, VALIGN_MIDDLE, kRightBegin, kMiddleBegin},
+      {ALIGN_RIGHT, VALIGN_BOTTOM, kRightBegin, kBottomBegin},
+  };
+
+  skia::RefPtr<SkSurface> surface = skia::AdoptRef(

[msw, 2015/03/26 22:55:45]

I think painting the text and checking the painted pixels is an overly complex
and expensive way of checking alignment calculations. This test should check
GetAlignmentOffset() (or GetLineOffset()) values for each of the settings,
similar to the GetTextOffset or Multiline_HorizontalAlignment tests.

You can probably just extend the GetTextOffset test with vertical alignments,
and add a Multiline_VerticalAlignment test (or check the same info for multiline
RenderText in the same GetTextOffset test)?

[dschuyler, 2015/03/27 21:20:34]

I put in some tests for the get alignment offset.  They don't seem to catch
errors on Mac where the text still draws in the wrong place.  Though maybe the
pixel testing should be an entirely separate test from a set/get alignment test?
 I.e. This kind of test does seem useful to do - are you suggesting that it be
done as a separate test?  (That separate test would also need to set alignment
values to see the issues).

[msw, 2015/03/30 19:19:12]

How many ms do the pixel tests take on the bots? I suspect their value
diminishes with the cost of each new test. I don't think we ultimately want to
do this expensive testing on every variation of settings that affect text
rendering. Just checking the alignment/line offset here should suffice, as long
as some other test(s) perform a pixel check with non-zero line offsets
(TextDoesntClip setting the display rect should suffice). ie. We should have
separate tests for (1) checking alignment offsets given the alignment setting
and (2) for checking that the text paints respecting some alignment offset(s).

[dschuyler, 2015/03/31 18:38:14]

Done.

+      SkSurface::NewRasterN32Premul(kCanvasSize.width(), kCanvasSize.height()));
+  scoped_ptr<Canvas> canvas(
+      Canvas::CreateCanvasWithoutScaling(surface->getCanvas(), 1.0f));
+  render_text->SetColor(SK_ColorBLACK);
+  Rect bounds = Rect(kTestSize, kTestSize, kCanvasSize.width() - kTestSize * 2,
+                     kCanvasSize.height() - kTestSize * 2);
+  render_text->SetDisplayRect(bounds);
+  // Allow the RenderText to paint outside of its display rect.
+  render_text->set_clip_to_display_rect(false);
+  const uint32* buffer =
+      static_cast<const uint32*>(surface->peekPixels(nullptr, nullptr));
+  ASSERT_NE(nullptr, buffer);
+  TestRectangleBuffer rect_buffer(string, buffer, kCanvasSize.width(),
+                                  kCanvasSize.height());
+  ASSERT_LE(string_size.width() + kTestSize * 2, kCanvasSize.width());
+  for (const auto& test : kTests) {
+    surface->getCanvas()->clear(SK_ColorWHITE);
+    render_text->SetHorizontalAlignment(test.horizontal);
+    render_text->SetVerticalAlignment(test.vertical);
+    Rect expected_rect(test.x, test.y, string_size.width(),
+                       string_size.height());
+    expected_rect.Inset(-1, -1);
+    render_text->Draw(canvas.get());
+    rect_buffer.EnsureSolidBorder(SK_ColorWHITE, expected_rect);
+  }
+}
+
 // TODO(derat): Figure out why this fails on Windows: http://crbug.com/427184
 #if !defined(OS_WIN)
 // Ensure that RenderText examines all of the fonts in its FontList before
 // falling back to other fonts.
 TEST_F(RenderTextTest, HarfBuzz_FontListFallback) {
   // Double-check that the requested fonts are present.
   FontList font_list("Arial, Symbol, 12px");
   const std::vector<Font>& fonts = font_list.GetFonts();
   ASSERT_EQ(2u, fonts.size());
   ASSERT_EQ("arial",
@@ skipping 87 matching lines @@
     render_text->ApplyBaselineStyle(INFERIOR, Range(5, 6));
     render_text->ApplyBaselineStyle(SUBSCRIPT, Range(7, 8));
     render_text->SetStyle(BOLD, true);
     render_text->SetDisplayRect(
         Rect(kTestSize, kTestSize, string_size.width(), string_size.height()));
     // Allow the RenderText to paint outside of its display rect.
     render_text->set_clip_to_display_rect(false);
     ASSERT_LE(string_size.width() + kTestSize * 2, kCanvasSize.width());
 
     render_text->Draw(canvas.get());
-    ASSERT_LT(string_size.width() + kTestSize, kCanvasSize.width());
     const uint32* buffer =
         static_cast<const uint32*>(surface->peekPixels(nullptr, nullptr));
     ASSERT_NE(nullptr, buffer);
     TestRectangleBuffer rect_buffer(string, buffer, kCanvasSize.width(),
                                     kCanvasSize.height());
+    // TODO(dschuyler): After platform issued are resolved below, change to

[msw, 2015/03/26 22:55:45]

nit: "issues"

[dschuyler, 2015/03/27 21:20:34]

Done.

+    // using EnsureSolidBorder() rather than EnsureSolidRect().
     {
 #if !defined(OS_CHROMEOS)
       // TODO(dschuyler): On ChromeOS text draws above the GetStringSize rect.
       SCOPED_TRACE("TextDoesntClip Top Side");
       rect_buffer.EnsureSolidRect(SK_ColorWHITE, 0, 0, kCanvasSize.width(),
                                   kTestSize);
 #endif
     }
     {
       SCOPED_TRACE("TextDoesntClip Bottom Side");
@@ skipping 29 matching lines @@
                                   kTestSize + string_size.width(), kTestSize,
                                   kTestSize, string_size.height());
 #endif
     }
   }
 }
 
 // Ensure that the text will clip to the display rect. Draws to a canvas and
 // checks whether any pixel beyond the bounding rectangle is colored.
 TEST_F(RenderTextTest, TextDoesClip) {
+  SCOPED_TRACE("TextDoesClip");

[msw, 2015/03/26 22:55:45]

Ditto, what's the use of this SCOPED_TRACE? The failing test should be readily
identifiable without this.

   const wchar_t* kTestStrings[] = {L"TEST", L"W", L"WWWW", L"gAXAXWWWW"};
   const Size kCanvasSize(300, 50);
   const int kTestSize = 10;
 
   skia::RefPtr<SkSurface> surface = skia::AdoptRef(
       SkSurface::NewRasterN32Premul(kCanvasSize.width(), kCanvasSize.height()));
   scoped_ptr<Canvas> canvas(
       Canvas::CreateCanvasWithoutScaling(surface->getCanvas(), 1.0f));
   scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
   render_text->SetHorizontalAlignment(ALIGN_LEFT);
   render_text->SetColor(SK_ColorBLACK);
 
   for (auto string : kTestStrings) {
     surface->getCanvas()->clear(SK_ColorWHITE);
     render_text->SetText(WideToUTF16(string));
     const Size string_size = render_text->GetStringSize();
     int fake_width = string_size.width() / 2;
     int fake_height = string_size.height() / 2;
-    render_text->SetDisplayRect(
-        Rect(kTestSize, kTestSize, fake_width, fake_height));
+    const Rect bounds(kTestSize, kTestSize, fake_width, fake_height);
+    render_text->SetDisplayRect(bounds);
     render_text->set_clip_to_display_rect(true);
+    ASSERT_LE(string_size.width() + kTestSize * 2, kCanvasSize.width());
     render_text->Draw(canvas.get());
-    ASSERT_LT(string_size.width() + kTestSize, kCanvasSize.width());
     const uint32* buffer =
         static_cast<const uint32*>(surface->peekPixels(nullptr, nullptr));
     ASSERT_NE(nullptr, buffer);
     TestRectangleBuffer rect_buffer(string, buffer, kCanvasSize.width(),
                                     kCanvasSize.height());
-    {
-      SCOPED_TRACE("TextDoesClip Top Side");
-      rect_buffer.EnsureSolidRect(SK_ColorWHITE, 0, 0, kCanvasSize.width(),
-                                  kTestSize);
-    }
-
-    {
-      SCOPED_TRACE("TextDoesClip Bottom Side");
-      rect_buffer.EnsureSolidRect(SK_ColorWHITE, 0, kTestSize + fake_height,
-                                  kCanvasSize.width(), kTestSize);
-    }
-    {
-      SCOPED_TRACE("TextDoesClip Left Side");
-      rect_buffer.EnsureSolidRect(SK_ColorWHITE, 0, kTestSize, kTestSize,
-                                  fake_height);
-    }
-    {
-      SCOPED_TRACE("TextDoesClip Right Side");
-      rect_buffer.EnsureSolidRect(SK_ColorWHITE, kTestSize + fake_width,
-                                  kTestSize, kTestSize, fake_height);
-    }
+    rect_buffer.EnsureSolidBorder(SK_ColorWHITE, bounds);
   }
 }
 
 #if defined(OS_MACOSX)
 TEST_F(RenderTextTest, Mac_ElidedText) {
   RenderTextMac render_text;
   base::string16 text(ASCIIToUTF16("This is an example."));
   render_text.SetText(text);
   gfx::Size string_size = render_text.GetStringSize();
   render_text.SetDisplayRect(gfx::Rect(string_size));
   render_text.EnsureLayout();
   // NOTE: Character and glyph counts are only comparable for simple text.
   EXPECT_EQ(text.size(),
             static_cast<size_t>(CTLineGetGlyphCount(render_text.line_)));
 
   render_text.SetElideBehavior(ELIDE_TAIL);
   string_size.set_width(string_size.width() / 2);
   render_text.SetDisplayRect(gfx::Rect(string_size));
   render_text.EnsureLayout();
   CFIndex glyph_count = CTLineGetGlyphCount(render_text.line_);
   EXPECT_GT(text.size(), static_cast<size_t>(glyph_count));
   EXPECT_NE(0, glyph_count);
 }
 #endif
 
 }  // namespace gfx