Respect paragraphstyle defaults, slight font name discovery improvements, better italic tests.

Change-Id: I4f032b315a68e2279087f701ab7bd920846cd723

src/font_collection.cc

@@ -103,23 +103,27 @@ void FontCollection::AddFontMgr(std::string dir, bool rediscover_family_names) {
 #endif
 
   if (rediscover_family_names)
-    DiscoverFamilyNames();
+    DiscoverFamilyNames(skia_font_managers_.back());
 }
 
 void FontCollection::AddFontMgr(sk_sp<SkFontMgr> font_mgr,
                                 bool rediscover_family_names) {
   skia_font_managers_.push_back(font_mgr);
   if (rediscover_family_names)
-    DiscoverFamilyNames();
+    DiscoverFamilyNames(font_mgr);
 }
 
 void FontCollection::DiscoverFamilyNames() {
-  SkString str;
   for (sk_sp<SkFontMgr> mgr : skia_font_managers_) {
-    for (int i = 0; i < mgr->countFamilies(); i++) {
-      mgr->getFamilyName(i, &str);
-      family_names_.insert(std::string{str.writable_str()});
-    }
+    DiscoverFamilyNames(mgr);
+  }
+}
+
+void FontCollection::DiscoverFamilyNames(sk_sp<SkFontMgr> mgr) {
+  SkString str;
+  for (int i = 0; i < mgr->countFamilies(); i++) {
+    mgr->getFamilyName(i, &str);
+    family_names_.insert(std::string{str.writable_str()});
   }
 }
 

src/font_collection.h

@@ -129,9 +129,13 @@ class FontCollection {
   // when not found and reverting to the default name when no fallback is found.
   const std::string ProcessFamilyName(const std::string& family);
 
-  // Polls the SkFontMgrs to obtain a set of all available font family names.
+  // Polls all of the SkFontMgrs to obtain a set of all available font family
+  // names.
   void DiscoverFamilyNames();
 
+  // Add the family names of mgr to set of available font family names.
+  void DiscoverFamilyNames(sk_sp<SkFontMgr> mgr);
+
   void TrimCache();
 
   static const std::string GetDefaultFamilyName() {

src/paragraph.cc

@@ -478,6 +478,8 @@ void Paragraph::Layout(double width, bool force) {
   postprocess_line();
   if (line_width != 0)
     line_widths_.push_back(line_width);
+
+  // Finalize measurements
   line_heights_.push_back((line_heights_.empty() ? 0 : line_heights_.back()) +
                           roundf(max_line_spacing + max_descent));
   glyph_single_line_position_x.push_back(glyph_single_line_position_x.back() +
@@ -489,11 +491,6 @@ void Paragraph::Layout(double width, bool force) {
   if (paragraph_style_.text_align == TextAlign::justify &&
       buffer_sizes.size() > 0) {
     JustifyLine(buffers, buffer_sizes, word_count, justify_spacing, -1);
-    // Remove decoration extra width if the last line.
-    size_t i = records_.size() - 1;
-    while (records_[i].line() == lines_ - 1) {
-      --i;
-    }
   }
   line_widths_ =
       std::vector<double>(breaker_.getWidths(), breaker_.getWidths() + lines_);
@@ -543,24 +540,22 @@ const ParagraphStyle& Paragraph::GetParagraphStyle() const {
 }
 
 double Paragraph::GetAlphabeticBaseline() const {
+  // Currently -fAscent
   return alphabetic_baseline_;
 }
 
 double Paragraph::GetIdeographicBaseline() const {
-  // TODO(garyq): Currently fCapHeight + fUnderlinePosition. Verify this.
+  // TODO(garyq): Currently -fAscent + fUnderlinePosition. Verify this.
   return ideographic_baseline_;
 }
 
 void Paragraph::CalculateIntrinsicWidths() {
-  // TODO(garyq): Investigate correctness of the following implementation of max
-  // intrinsic width. This is currently the sum of all the widths of each line
-  // after layout.
   max_intrinsic_width_ = 0;
   for (size_t i = 0; i < line_widths_.size(); ++i) {
     max_intrinsic_width_ += line_widths_[i];
   }
 
-  // TODO(garyq): Investigate correctness of the following implementation of max
+  // TODO(garyq): Investigate correctness of the following implementation of min
   // intrinsic width. This is currently the longest line in the text after
   // layout.
   min_intrinsic_width_ = 0;

src/paragraph_builder.cc

@@ -25,15 +25,25 @@ namespace txt {
 
 ParagraphBuilder::ParagraphBuilder(ParagraphStyle style,
                                    FontCollection* font_collection)
-    : paragraph_style_(style), font_collection_(font_collection) {}
+    : font_collection_(font_collection) {
+  SetParagraphStyle(style);
+}
 
-ParagraphBuilder::ParagraphBuilder(ParagraphStyle style)
-    : paragraph_style_(style) {}
+ParagraphBuilder::ParagraphBuilder(ParagraphStyle style) {
+  SetParagraphStyle(style);
+}
 
 ParagraphBuilder::ParagraphBuilder() {}
 
 void ParagraphBuilder::SetParagraphStyle(const ParagraphStyle& style) {
   paragraph_style_ = style;
+  // Keep a default style to fall back to.
+  TextStyle text_style;
+  text_style.font_weight = paragraph_style_.font_weight;
+  text_style.font_style = paragraph_style_.font_style;
+  text_style.font_family = paragraph_style_.font_family;
+  text_style.font_size = paragraph_style_.font_size;
+  PushStyle(text_style);
 }
 
 void ParagraphBuilder::SetFontCollection(FontCollection* font_collection) {

src/paragraph_style.h

@@ -29,11 +29,12 @@ namespace txt {
 
 class ParagraphStyle {
  public:
-  TextAlign text_align = TextAlign::left;
   FontWeight font_weight = FontWeight::w400;
   FontStyle font_style = FontStyle::normal;
   std::string font_family = "";
   double font_size = 14;
+
+  TextAlign text_align = TextAlign::left;
   size_t max_lines = UINT_MAX;
   double line_height = 1.0;
   std::string ellipsis = "...";

tests/txt/paragraph_unittests.cc

@@ -54,8 +54,8 @@ TEST_F(RenderTest, SimpleParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_TRUE(Snapshot());
 }
@@ -88,8 +88,8 @@ TEST_F(RenderTest, SimpleRedParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_TRUE(Snapshot());
 }
@@ -176,11 +176,11 @@ TEST_F(RenderTest, RainbowParagraph) {
   }
   ASSERT_TRUE(Snapshot());
   ASSERT_EQ(paragraph->runs_.runs_.size(), 4ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 4ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style1));
-  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style2));
-  ASSERT_TRUE(paragraph->runs_.styles_[2].equals(text_style3));
-  ASSERT_TRUE(paragraph->runs_.styles_[3].equals(text_style4));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 5ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style1));
+  ASSERT_TRUE(paragraph->runs_.styles_[2].equals(text_style2));
+  ASSERT_TRUE(paragraph->runs_.styles_[3].equals(text_style3));
+  ASSERT_TRUE(paragraph->runs_.styles_[4].equals(text_style4));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style1.color);
   ASSERT_EQ(paragraph->records_[1].style().color, text_style2.color);
   ASSERT_EQ(paragraph->records_[2].style().color, text_style3.color);
@@ -214,8 +214,8 @@ TEST_F(RenderTest, DefaultStyleParagraph) {
   for (size_t i = 0; i < u16_text.length(); i++) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
-  ASSERT_EQ(paragraph->runs_.runs_.size(), 0ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 0ull);
+  ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
   ASSERT_TRUE(Snapshot());
 }
 
@@ -251,8 +251,8 @@ TEST_F(RenderTest, BoldParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_TRUE(Snapshot());
 }
@@ -312,8 +312,8 @@ TEST_F(RenderTest, LeftAlignParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_.size(), paragraph_style.max_lines);
   double expected_y = 24;
 
@@ -408,8 +408,8 @@ TEST_F(RenderTest, RightAlignParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_.size(), paragraph_style.max_lines);
   double expected_y = 24;
 
@@ -512,8 +512,8 @@ TEST_F(RenderTest, CenterAlignParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_.size(), paragraph_style.max_lines);
   double expected_y = 24;
 
@@ -618,8 +618,8 @@ TEST_F(RenderTest, JustifyAlignParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_.size(), paragraph_style.max_lines);
   double expected_y = 24;
 
@@ -731,38 +731,34 @@ TEST_F(RenderTest, DecorationsParagraph) {
 }
 
 TEST_F(RenderTest, ItalicsParagraph) {
-  const char* text = "I am Italicized!                           ";
-  auto icu_text = icu::UnicodeString::fromUTF8(text);
-  std::u16string u16_text(icu_text.getBuffer(),
-                          icu_text.getBuffer() + icu_text.length());
-
   txt::ParagraphStyle paragraph_style;
   auto font_collection = FontCollection::GetFontCollection(txt::GetFontDir());
   txt::ParagraphBuilder builder(paragraph_style, &font_collection);
 
   txt::TextStyle text_style;
   text_style.color = SK_ColorRED;
-  text_style.font_style = txt::FontStyle::italic;
-  text_style.font_size = 35;
+  text_style.font_size = 10;
   builder.PushStyle(text_style);
+  builder.AddText("No italic ");
 
-  builder.AddText(u16_text);
+  text_style.font_style = txt::FontStyle::italic;
+  builder.PushStyle(text_style);
+  builder.AddText("Yes Italic ");
 
   builder.Pop();
+  builder.AddText("No Italic again.");
 
   auto paragraph = builder.Build();
   paragraph->Layout(GetTestCanvasWidth());
 
-  paragraph->Paint(GetCanvas(), 10.0, 35.0);
+  paragraph->Paint(GetCanvas(), 0, 0);
 
-  ASSERT_EQ(paragraph->text_.size(), std::string{text}.length());
-  for (size_t i = 0; i < u16_text.length(); i++) {
-    ASSERT_EQ(paragraph->text_[i], u16_text[i]);
-  }
-  ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
-  ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
+  ASSERT_EQ(paragraph->runs_.runs_.size(), 3ull);
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 3ull);
+  ASSERT_EQ(paragraph->records_[1].style().color, text_style.color);
+  ASSERT_EQ(paragraph->records_[1].style().font_style, txt::FontStyle::italic);
+  ASSERT_EQ(paragraph->records_[2].style().font_style, txt::FontStyle::normal);
+  ASSERT_EQ(paragraph->records_[0].style().font_style, txt::FontStyle::normal);
   ASSERT_TRUE(Snapshot());
 }
 
@@ -803,8 +799,8 @@ TEST_F(RenderTest, ChineseParagraph) {
   paragraph->Paint(GetCanvas(), 0, 0);
 
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_EQ(paragraph->records_.size(), 7ull);
 
@@ -849,8 +845,8 @@ TEST_F(RenderTest, DISABLED_ArabicParagraph) {
   ASSERT_EQ(paragraph->text_.size(), std::string{text}.length());
 
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_EQ(paragraph->records_.size(), 2ull);
   ASSERT_EQ(paragraph->paragraph_style_.rtl, true);
@@ -1289,8 +1285,8 @@ TEST_F(RenderTest, LongWordParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_EQ(paragraph->GetLineCount(), 4);
   ASSERT_TRUE(Snapshot());
@@ -1477,8 +1473,8 @@ TEST_F(RenderTest, HyphenBreakParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_EQ(paragraph->GetLineCount(), 5);
   ASSERT_TRUE(Snapshot());
@@ -1522,8 +1518,8 @@ TEST_F(RenderTest, RepeatLayoutParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_EQ(paragraph->GetLineCount(), 12);
 
@@ -1538,8 +1534,8 @@ TEST_F(RenderTest, RepeatLayoutParagraph) {
     ASSERT_EQ(paragraph->text_[i], u16_text[i]);
   }
   ASSERT_EQ(paragraph->runs_.runs_.size(), 1ull);
-  ASSERT_EQ(paragraph->runs_.styles_.size(), 1ull);
-  ASSERT_TRUE(paragraph->runs_.styles_[0].equals(text_style));
+  ASSERT_EQ(paragraph->runs_.styles_.size(), 2ull);
+  ASSERT_TRUE(paragraph->runs_.styles_[1].equals(text_style));
   ASSERT_EQ(paragraph->records_[0].style().color, text_style.color);
   ASSERT_EQ(paragraph->GetLineCount(), 6);
   ASSERT_TRUE(Snapshot());