 Chromium Code Reviews
 Chromium Code Reviews Issue 1013543006:
  [RenderText] Adding vertical alignment options  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master
    
  
    Issue 1013543006:
  [RenderText] Adding vertical alignment options  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/src.git@master| Index: ui/gfx/render_text_unittest.cc | 
| diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc | 
| index 75d2b1dc4949fa7a6baeddc88952a5f26beeee22..26aa35bcb0ea57318229a29ae848bdfbebc0bb13 100644 | 
| --- a/ui/gfx/render_text_unittest.cc | 
| +++ b/ui/gfx/render_text_unittest.cc | 
| @@ -194,14 +194,41 @@ class TestRectangleBuffer { | 
| ASSERT_LE(left + width, stride_) << string_ << ", left " << left | 
| << ", width " << width << ", stride_ " | 
| << stride_; | 
| + // Only report the first N failures. | 
| + uint32_t fail_limit = 5; | 
| for (int y = top; y < top + height; ++y) { | 
| for (int x = left; x < left + width; ++x) { | 
| SkColor buffer_color = buffer_[x + y * stride_]; | 
| + if (color != buffer_color && !(--fail_limit)) { | 
| + EXPECT_TRUE(fail_limit) << "fail_limit reached"; | 
| + return; | 
| + } | 
| 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.bottom(), stride_, | 
| + row_count_ - rect.bottom()); | 
| + } | 
| + { | 
| + SCOPED_TRACE("EnsureSolidBorder Left Side"); | 
| + EnsureSolidRect(SK_ColorWHITE, 0, rect.y(), rect.x(), rect.height()); | 
| + } | 
| + { | 
| + SCOPED_TRACE("EnsureSolidBorder Right Side"); | 
| + EnsureSolidRect(SK_ColorWHITE, rect.right(), rect.y(), | 
| + stride_ - rect.right(), rect.height()); | 
| + } | 
| + } | 
| + | 
| private: | 
| const wchar_t* string_; | 
| const SkColor* buffer_; | 
| @@ -2698,6 +2725,68 @@ TEST_F(RenderTextTest, StringFitsOwnWidth) { | 
| EXPECT_EQ(kString, render_text->GetDisplayText()); | 
| } | 
| +// TODO(dschuyler): Alignment tests disabled on Mac because RenderTextMac | 
| +// does not implement this yet. | 
| +#if !defined(OS_MACOSX) | 
| +TEST_F(RenderTextTest, VerticalAlignment) { | 
| 
msw
2015/04/01 15:07:17
Can you add a separate test for multiline vertical
 | 
| + scoped_ptr<RenderText> render_text(RenderText::CreateInstance()); | 
| + const wchar_t* string = L"g Hello g"; | 
| 
msw
2015/04/01 15:07:18
nit: inline this below
 | 
| + render_text->SetText(WideToUTF16(string)); | 
| + render_text->ApplyStyle(BOLD, true, Range(0, 3)); | 
| 
msw
2015/04/01 15:07:18
Why apply bold? Do you check this somehow?
 | 
| + const Size kCanvasSize(171, 50); | 
| 
msw
2015/04/01 15:07:18
nit: where did 171 come from? That seems awfully s
 | 
| + const int kTestSize = 10; | 
| + const Size string_size(render_text->GetContentWidth(), | 
| + render_text->GetStringSize().height()); | 
| + // The + 1 accounts for legacy behavior in GetAlignmentOffset(). | 
| + const int kCenterBegin = (kCanvasSize.width() + 1 - string_size.width()) / 2; | 
| + const int kRightBegin = kCanvasSize.width() - string_size.width() - kTestSize; | 
| + // TODO(dschuyler): Determine where the need for the -1 below originates from. | 
| + const int kMiddleBegin = | 
| + ((kCanvasSize.height()) / 2) - 1 + | 
| 
msw
2015/04/01 15:07:17
nit: remove the "kCanvasSize.height()" parens, opt
 | 
| + (render_text->GetBaseline() - render_text->GetDisplayTextBaseline()); | 
| + const int kBottomBegin = | 
| + kCanvasSize.height() - string_size.height() - kTestSize; | 
| + | 
| + struct Tests { | 
| + HorizontalAlignment horizontal; | 
| 
msw
2015/04/01 15:07:17
Is testing vertical alignment functionality for ea
 | 
| + VerticalAlignment vertical; | 
| + int x; | 
| + int y; | 
| + } const kTests[] = { | 
| + {ALIGN_LEFT, VALIGN_TOP, kTestSize, kTestSize}, | 
| + {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/04/01 15:07:18
Remove this, it's no longer necessary.
 | 
| + SkSurface::NewRasterN32Premul(kCanvasSize.width(), kCanvasSize.height())); | 
| + render_text->SetColor(SK_ColorBLACK); | 
| 
msw
2015/04/01 15:07:18
ditto: Remove this, it's no longer necessary.
 | 
| + 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); | 
| 
msw
2015/04/01 15:07:18
ditto: Remove this, it's no longer necessary.
 | 
| + ASSERT_LE(string_size.width() + kTestSize * 2, kCanvasSize.width()); | 
| + ASSERT_LE(string_size.height() + kTestSize * 2, kCanvasSize.height()); | 
| + for (const auto& test : kTests) { | 
| + SCOPED_TRACE(base::StringPrintf("VerticalAlignement H %i, V %i", | 
| 
msw
2015/04/01 15:07:18
nit: "Alignment", but again, the test name isn't n
 | 
| + test.horizontal, test.vertical)); | 
| + | 
| + surface->getCanvas()->clear(SK_ColorWHITE); | 
| 
msw
2015/04/01 15:07:18
ditto: Remove this, it's no longer necessary.
 | 
| + render_text->SetHorizontalAlignment(test.horizontal); | 
| + render_text->SetVerticalAlignment(test.vertical); | 
| + EXPECT_EQ(test.x - kTestSize, render_text->GetAlignmentOffset(0).x()); | 
| + EXPECT_EQ(test.y - kTestSize, render_text->GetAlignmentOffset(0).y()); | 
| + } | 
| +} | 
| +#endif // !defined(OS_MACOSX) | 
| + | 
| // 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 | 
| @@ -2792,7 +2881,8 @@ TEST_F(RenderTextTest, TextDoesntClip) { | 
| for (auto string : kTestStrings) { | 
| surface->getCanvas()->clear(SK_ColorWHITE); | 
| render_text->SetText(WideToUTF16(string)); | 
| - const Size string_size = render_text->GetStringSize(); | 
| + const Size string_size(render_text->GetContentWidth(), | 
| + render_text->GetStringSize().height()); | 
| render_text->ApplyBaselineStyle(SUPERSCRIPT, Range(1, 2)); | 
| render_text->ApplyBaselineStyle(SUPERIOR, Range(3, 4)); | 
| render_text->ApplyBaselineStyle(INFERIOR, Range(5, 6)); | 
| @@ -2803,14 +2893,16 @@ TEST_F(RenderTextTest, TextDoesntClip) { | 
| // 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()); | 
| + ASSERT_LE(string_size.height() + kTestSize * 2, kCanvasSize.height()); | 
| 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 issues are resolved below, change to | 
| + // using EnsureSolidBorder() rather than EnsureSolidRect(). | 
| { | 
| #if !defined(OS_CHROMEOS) | 
| // TODO(dschuyler): On ChromeOS text draws above the GetStringSize rect. | 
| @@ -2875,40 +2967,22 @@ TEST_F(RenderTextTest, TextDoesClip) { | 
| for (auto string : kTestStrings) { | 
| surface->getCanvas()->clear(SK_ColorWHITE); | 
| render_text->SetText(WideToUTF16(string)); | 
| - const Size string_size = render_text->GetStringSize(); | 
| + const Size string_size(render_text->GetContentWidth(), | 
| + render_text->GetStringSize().height()); | 
| 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()); | 
| + ASSERT_LE(string_size.height() + kTestSize * 2, kCanvasSize.height()); | 
| 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); | 
| } | 
| } |