VR: Properly adjust texture size for non-multiline text

This change sees single-line text rendering properly update the vertical
dimension of the underlying texture.

In addition, the recently-added text unittest is reworked to allow more
thorough and easy testing, by separating text layout from rendering and
canvas, and also exposing the internal text layout to tests.

BUG=
R=bshe

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ib0ec5b678adf2727d58543e281685ce49ae691af
Reviewed-on: https://chromium-review.googlesource.com/801955Reviewed-by: Biao She <bshe@chromium.org>
Commit-Queue: Christopher Grant <cjgrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521095}

chrome/browser/vr/BUILD.gn

@@ -82,6 +82,8 @@ static_library("vr_common") {
     "elements/text.h",
     "elements/text_input.cc",
     "elements/text_input.h",
+    "elements/text_texture.cc",
+    "elements/text_texture.h",
     "elements/textured_element.cc",
     "elements/textured_element.h",
     "elements/throbber.cc",

chrome/browser/vr/elements/text.cc

@@ -6,7 +6,7 @@
 
 #include "base/memory/ptr_util.h"
 #include "cc/paint/skia_paint_canvas.h"
-#include "chrome/browser/vr/elements/ui_texture.h"
+#include "chrome/browser/vr/elements/text_texture.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/rect.h"
@@ -15,49 +15,6 @@
 
 namespace vr {
 
-class TextTexture : public UiTexture {
- public:
-  explicit TextTexture(float font_height) : font_height_(font_height) {}
-  ~TextTexture() override {}
-
-  void SetText(const base::string16& text) { SetAndDirty(&text_, text); }
-
-  void SetColor(SkColor color) { SetAndDirty(&color_, color); }
-
-  void SetAlignment(TextAlignment alignment) {
-    SetAndDirty(&alignment_, alignment);
-  }
-
-  void SetMultiLine(bool multiline) { SetAndDirty(&multiline_, multiline); }
-
-  void SetTextWidth(float width) { SetAndDirty(&text_width_, width); }
-
-  int rendered_lines() { return rendered_lines_; }
-
- private:
-  gfx::Size GetPreferredTextureSize(int width) const override {
-    return gfx::Size(width, width);
-  }
-
-  gfx::SizeF GetDrawnSize() const override { return size_; }
-
-  void Draw(SkCanvas* sk_canvas, const gfx::Size& texture_size) override;
-
-  gfx::SizeF size_;
-  base::string16 text_;
-  // These dimensions are in meters.
-  float font_height_ = 0;
-  float text_width_ = 0;
-  TextAlignment alignment_ = kTextAlignmentCenter;
-  bool multiline_ = true;
-  SkColor color_ = SK_ColorBLACK;
-
-  // The number of lines generated in the last draw operation.
-  int rendered_lines_ = 0;
-
-  DISALLOW_COPY_AND_ASSIGN(TextTexture);
-};
-
 Text::Text(int maximum_width_pixels, float font_height_meters)
     : TexturedElement(maximum_width_pixels),
       texture_(base::MakeUnique<TextTexture>(font_height_meters)) {}
@@ -83,44 +40,11 @@ void Text::OnSetSize(gfx::SizeF size) {
   texture_->SetTextWidth(size.width());
 }
 
-int Text::NumRenderedLinesForTest() const {
-  return texture_->rendered_lines();
-}
-
-UiTexture* Text::GetTexture() const {
+TextTexture* Text::GetTextureForTest() {
   return texture_.get();
 }
 
-void TextTexture::Draw(SkCanvas* sk_canvas, const gfx::Size& texture_size) {
-  cc::SkiaPaintCanvas paint_canvas(sk_canvas);
-  gfx::Canvas gfx_canvas(&paint_canvas, 1.0f);
-  gfx::Canvas* canvas = &gfx_canvas;
-
-  gfx::FontList fonts;
-  float pixels_per_meter = texture_size.width() / text_width_;
-  int pixel_font_height = static_cast<int>(font_height_ * pixels_per_meter);
-  GetDefaultFontList(pixel_font_height, text_, &fonts);
-  gfx::Rect text_bounds(texture_size.width(),
-                        multiline_ ? 0 : texture_size.height());
-
-  std::vector<std::unique_ptr<gfx::RenderText>> lines =
-      // TODO(vollick): if this subsumes all text, then we should probably move
-      // this function into this class.
-      PrepareDrawStringRect(
-          text_, fonts, color_, &text_bounds, alignment_,
-          multiline_ ? kWrappingBehaviorWrap : kWrappingBehaviorNoWrap);
-
-  // Draw the text.
-  for (auto& render_text : lines)
-    render_text->Draw(canvas);
-
-  // Note, there is no padding here whatsoever.
-  size_ = gfx::SizeF(text_bounds.size());
-
-  rendered_lines_ = lines.size();
-}
-
-UiTexture* Text::GetTextureForTest() const {
+UiTexture* Text::GetTexture() const {
   return texture_.get();
 }
 

chrome/browser/vr/elements/text.h

@@ -22,15 +22,12 @@ class Text : public TexturedElement {
 
   void SetText(const base::string16& text);
   void SetColor(SkColor color);
-
   void SetTextAlignment(UiTexture::TextAlignment alignment);
   void SetMultiLine(bool multiline);
 
   void OnSetSize(gfx::SizeF size) override;
 
-  int NumRenderedLinesForTest() const;
-
-  UiTexture* GetTextureForTest() const;
+  TextTexture* GetTextureForTest();
 
  private:
   UiTexture* GetTexture() const override;

chrome/browser/vr/elements/text_texture.cc

+// Copyright 2017 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 "chrome/browser/vr/elements/text_texture.h"
+
+#include "base/memory/ptr_util.h"
+#include "cc/paint/skia_paint_canvas.h"
+#include "chrome/browser/vr/elements/ui_texture.h"
+#include "ui/gfx/canvas.h"
+#include "ui/gfx/font_list.h"
+#include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/render_text.h"
+
+namespace vr {
+
+TextTexture::TextTexture(float font_height) : font_height_(font_height) {}
+
+TextTexture::~TextTexture() {}
+
+void TextTexture::SetText(const base::string16& text) {
+  SetAndDirty(&text_, text);
+}
+
+void TextTexture::SetColor(SkColor color) {
+  SetAndDirty(&color_, color);
+}
+
+void TextTexture::SetAlignment(TextAlignment alignment) {
+  SetAndDirty(&alignment_, alignment);
+}
+
+void TextTexture::SetMultiLine(bool multiline) {
+  SetAndDirty(&multiline_, multiline);
+}
+
+void TextTexture::SetTextWidth(float width) {
+  SetAndDirty(&text_width_, width);
+}
+
+gfx::SizeF TextTexture::GetDrawnSize() const {
+  return size_;
+}
+
+std::vector<std::unique_ptr<gfx::RenderText>> TextTexture::LayOutText(
+    const gfx::Size& texture_size) {
+  gfx::FontList fonts;
+  float pixels_per_meter = texture_size.width() / text_width_;
+  int pixel_font_height = static_cast<int>(font_height_ * pixels_per_meter);
+  GetDefaultFontList(pixel_font_height, text_, &fonts);
+  gfx::Rect text_bounds(texture_size.width(), 0);
+
+  std::vector<std::unique_ptr<gfx::RenderText>> lines =
+      // TODO(vollick): if this subsumes all text, then we should probably move
+      // this function into this class.
+      PrepareDrawStringRect(
+          text_, fonts, color_, &text_bounds, alignment_,
+          multiline_ ? kWrappingBehaviorWrap : kWrappingBehaviorNoWrap);
+
+  // Note, there is no padding here whatsoever.
+  size_ = gfx::SizeF(text_bounds.size());
+
+  return lines;
+}
+
+gfx::Size TextTexture::GetPreferredTextureSize(int width) const {
+  return gfx::Size(width, width);
+}
+
+void TextTexture::Draw(SkCanvas* sk_canvas, const gfx::Size& texture_size) {
+  cc::SkiaPaintCanvas paint_canvas(sk_canvas);
+  gfx::Canvas gfx_canvas(&paint_canvas, 1.0f);
+  gfx::Canvas* canvas = &gfx_canvas;
+
+  auto lines = LayOutText(texture_size);
+  for (auto& render_text : lines)
+    render_text->Draw(canvas);
+}
+
+}  // namespace vr

chrome/browser/vr/elements/text_texture.h

+// Copyright 2017 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.
+
+#ifndef CHROME_BROWSER_VR_ELEMENTS_TEXT_TEXTURE_H_
+#define CHROME_BROWSER_VR_ELEMENTS_TEXT_TEXTURE_H_
+
+#include <memory>
+
+#include "chrome/browser/vr/elements/textured_element.h"
+#include "chrome/browser/vr/elements/ui_texture.h"
+#include "third_party/skia/include/core/SkColor.h"
+
+namespace gfx {
+class RenderText;
+}
+
+namespace vr {
+
+class TextTexture : public UiTexture {
+ public:
+  explicit TextTexture(float font_height);
+  ~TextTexture() override;
+
+  void SetText(const base::string16& text);
+  void SetColor(SkColor color);
+  void SetAlignment(TextAlignment alignment);
+  void SetMultiLine(bool multiline);
+  void SetTextWidth(float width);
+
+  gfx::SizeF GetDrawnSize() const override;
+
+  // This method does all text preparation for the element other than drawing to
+  // the texture. This allows for deeper unit testing of the Text element
+  // without having to mock canvases and simulate frame rendering. The state of
+  // the texture is modified here.
+  std::vector<std::unique_ptr<gfx::RenderText>> LayOutText(
+      const gfx::Size& texture_size);
+
+ private:
+  gfx::Size GetPreferredTextureSize(int width) const override;
+  void Draw(SkCanvas* sk_canvas, const gfx::Size& texture_size) override;
+
+  gfx::SizeF size_;
+  base::string16 text_;
+  // These dimensions are in meters.
+  float font_height_ = 0;
+  float text_width_ = 0;
+  TextAlignment alignment_ = kTextAlignmentCenter;
+  bool multiline_ = true;
+  SkColor color_ = SK_ColorBLACK;
+
+  DISALLOW_COPY_AND_ASSIGN(TextTexture);
+};
+
+}  // namespace vr
+
+#endif  // CHROME_BROWSER_VR_ELEMENTS_TEXT_TEXTURE_H_

chrome/browser/vr/elements/text_unittest.cc

@@ -6,48 +6,42 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "cc/test/test_skcanvas.h"
-#include "chrome/browser/vr/ui_scene.h"
+#include "chrome/browser/vr/elements/text_texture.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/render_text.h"
 
 namespace vr {
 
 TEST(Text, MultiLine) {
-  UiScene scene;
-  testing::NiceMock<cc::MockCanvas> canvas;
-
   const float kInitialSize = 1.0f;
-  const int kPixelWidth = 512;
+  const gfx::Size texture_size({512, 512});
 
   // Create an initialize a text element with a long string.
-  auto text_instance = base::MakeUnique<Text>(kPixelWidth, 0.020);
-  Text* text = text_instance.get();
+  auto text = base::MakeUnique<Text>(texture_size.width(), 0.020);
   text->SetSize(kInitialSize, 0);
   text->SetText(base::UTF8ToUTF16(std::string(1000, 'x')));
-  text->SetInitializedForTesting();
-  scene.AddUiElement(kRoot, std::move(text_instance));
 
-  // Grab a pointer to the underlying texture to inspect it directly.
-  UiTexture* texture = text->GetTextureForTest();
-  auto texture_size = texture->GetPreferredTextureSize(kPixelWidth);
+  TextTexture* texture = text->GetTextureForTest();
 
   // Make sure we get multiple lines of rendered text from the string.
-  scene.OnBeginFrame(base::TimeTicks(), {0, 0, 0});
-  texture->DrawAndLayout(&canvas, texture_size);
-  int initial_num_lines = text->NumRenderedLinesForTest();
-  EXPECT_GT(initial_num_lines, 1);
-
-  // Reduce the field width, and ensure that the number of lines increases.
+  auto layout = texture->LayOutText(texture_size);
+  size_t initial_num_lines = layout.size();
+  auto initial_size = texture->GetDrawnSize();
+  EXPECT_GT(initial_num_lines, 1u);
+  EXPECT_GT(initial_size.height(), 0.f);
+
+  // Reduce the field width, and ensure that the number of lines increases along
+  // with the texture height.
   text->SetSize(kInitialSize / 2, 0);
-  scene.OnBeginFrame(base::TimeTicks(), {0, 0, 0});
-  texture->DrawAndLayout(&canvas, texture_size);
-  EXPECT_GT(text->NumRenderedLinesForTest(), initial_num_lines);
+  layout = texture->LayOutText(texture_size);
+  EXPECT_GT(layout.size(), initial_num_lines);
+  EXPECT_GT(texture->GetDrawnSize().height(), initial_size.height());
 
   // Enforce single-line rendering.
   text->SetMultiLine(false);
-  scene.OnBeginFrame(base::TimeTicks(), {0, 0, 0});
-  texture->DrawAndLayout(&canvas, texture_size);
-  EXPECT_EQ(1, text->NumRenderedLinesForTest());
+  layout = texture->LayOutText(texture_size);
+  EXPECT_EQ(layout.size(), 1u);
+  EXPECT_LT(texture->GetDrawnSize().height(), initial_size.height());
 }
 
 }  // namespace vr

chrome/browser/vr/elements/ui_texture.cc

@@ -69,9 +69,9 @@ std::vector<std::unique_ptr<gfx::RenderText>> UiTexture::PrepareDrawStringRect(
   DCHECK(bounds);
 
   std::vector<std::unique_ptr<gfx::RenderText>> lines;
-  gfx::Rect rect(*bounds);
 
   if (wrapping_behavior == kWrappingBehaviorWrap) {
+    gfx::Rect rect(*bounds);
     std::vector<base::string16> strings;
     gfx::ElideRectangleText(text, font_list, bounds->width(),
                             bounds->height() ? bounds->height() : INT_MAX,
@@ -108,18 +108,13 @@ std::vector<std::unique_ptr<gfx::RenderText>> UiTexture::PrepareDrawStringRect(
         CreateConfiguredRenderText(text, font_list, color, text_alignment);
     if (bounds->width() != 0)
       render_text->SetElideBehavior(gfx::TRUNCATE);
-    else
-      rect.set_width(INT_MAX);
 
-    render_text->SetDisplayRect(rect);
-
-    if (bounds->width() == 0) {
-      int text_width = render_text->GetStringSize().width();
-      bounds->set_width(text_width);
-      rect.set_width(text_width);
-      render_text->SetDisplayRect(rect);
-    }
+    if (bounds->width() == 0)
+      bounds->set_width(render_text->GetStringSize().width());
+    if (bounds->height() == 0)
+      bounds->set_height(render_text->GetStringSize().height());
 
+    render_text->SetDisplayRect(*bounds);
     lines.push_back(std::move(render_text));
   }
   return lines;