From 41e02e96131c1ec66d013e4615348be013518dc4 Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Thu, 29 Dec 2016 00:36:30 +0900 Subject: [PATCH] Remove FontFamily.addFont and make FontFamily immutable. This lays the groundwork for making SparseBitSet serializable. FontFamily.addFont is only used when the FontFamily is constructed. Thus, instead of calling FontFamily.addFont multiple time, passes Font list to the constructor. By this change, FontFamily can be immutable now. By making FontFamily immutable, We can create FontFamily with pre-calculated SparseBitSet. Bug: 34042446 Test: minikin_tests has passed Change-Id: I2576789fba6cb27687e920e2488e8bedbcf7d36f --- include/minikin/FontFamily.h | 57 +++++---- libs/minikin/FontCollection.cpp | 18 ++- libs/minikin/FontFamily.cpp | 115 ++++++++----------- sample/example.cpp | 8 +- sample/example_skia.cpp | 8 +- tests/unittest/FontCollectionItemizeTest.cpp | 46 ++++---- tests/unittest/FontCollectionTest.cpp | 4 +- tests/unittest/FontFamilyTest.cpp | 22 ++-- tests/util/FontTestUtils.cpp | 25 ++-- 9 files changed, 139 insertions(+), 164 deletions(-) diff --git a/include/minikin/FontFamily.h b/include/minikin/FontFamily.h index 10362c24de1..bdf00e9f7c4 100644 --- a/include/minikin/FontFamily.h +++ b/include/minikin/FontFamily.h @@ -98,64 +98,61 @@ struct FakedFont { FontFakery fakery; }; +struct Font { + Font(MinikinFont* typeface, FontStyle style); + Font(Font&& o); + Font(const Font& o); + ~Font(); + + MinikinFont* typeface; + FontStyle style; +}; + class FontFamily : public MinikinRefCounted { public: - FontFamily(); - - explicit FontFamily(int variant); - - FontFamily(uint32_t langId, int variant) - : mLangId(langId), - mVariant(variant), - mHasVSTable(false), - mCoverageValid(false) { - } + explicit FontFamily(std::vector&& fonts); + FontFamily(int variant, std::vector&& fonts); + FontFamily(uint32_t langId, int variant, std::vector&& fonts); ~FontFamily(); - // Add font to family, extracting style information from the font - bool addFont(MinikinFont* typeface); + // TODO: Good to expose FontUtil.h. + static bool analyzeStyle(MinikinFont* typeface, int* weight, bool* italic); - void addFont(MinikinFont* typeface, FontStyle style); FakedFont getClosestMatch(FontStyle style) const; uint32_t langId() const { return mLangId; } int variant() const { return mVariant; } // API's for enumerating the fonts in a family. These don't guarantee any particular order - size_t getNumFonts() const; - MinikinFont* getFont(size_t index) const; - FontStyle getStyle(size_t index) const; + size_t getNumFonts() const { return mFonts.size(); } + MinikinFont* getFont(size_t index) const { return mFonts[index].typeface; } + FontStyle getStyle(size_t index) const { return mFonts[index].style; } bool isColorEmojiFamily() const; - // Get Unicode coverage. Lifetime of returned bitset is same as receiver. May return nullptr on - // error. - const SparseBitSet* getCoverage(); + // Get Unicode coverage. + const SparseBitSet& getCoverage() const { return mCoverage; } // Returns true if the font has a glyph for the code point and variation selector pair. // Caller should acquire a lock before calling the method. - bool hasGlyph(uint32_t codepoint, uint32_t variationSelector); + bool hasGlyph(uint32_t codepoint, uint32_t variationSelector) const; // Returns true if this font family has a variaion sequence table (cmap format 14 subtable). - bool hasVSTable() const; + bool hasVSTable() const { return mHasVSTable; } private: - void addFontLocked(MinikinFont* typeface, FontStyle style); + void computeCoverage(); - class Font { - public: - Font(MinikinFont* typeface, FontStyle style) : - typeface(typeface), style(style) { } - MinikinFont* typeface; - FontStyle style; - }; uint32_t mLangId; int mVariant; std::vector mFonts; SparseBitSet mCoverage; bool mHasVSTable; - bool mCoverageValid; + + // Forbid copying and assignment. + FontFamily(const FontFamily&) = delete; + void operator=(const FontFamily&) = delete; }; } // namespace minikin diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp index 9d26377e6d1..4688520699d 100644 --- a/libs/minikin/FontCollection.cpp +++ b/libs/minikin/FontCollection.cpp @@ -96,17 +96,13 @@ FontCollection::FontCollection(const vector& typefaces) : continue; } family->RefLocked(); - const SparseBitSet* coverage = family->getCoverage(); - if (coverage == nullptr) { - family->UnrefLocked(); - continue; - } + const SparseBitSet& coverage = family->getCoverage(); mFamilies.push_back(family); // emplace_back would be better if (family->hasVSTable()) { mVSFamilyVec.push_back(family); } - mMaxChar = max(mMaxChar, coverage->length()); - lastChar.push_back(coverage->nextSetBit(0)); + mMaxChar = max(mMaxChar, coverage.length()); + lastChar.push_back(coverage.nextSetBit(0)); } nTypefaces = mFamilies.size(); LOG_ALWAYS_FATAL_IF(nTypefaces == 0, @@ -130,7 +126,7 @@ FontCollection::FontCollection(const vector& typefaces) : FontFamily* family = mFamilies[j]; mFamilyVec.push_back(family); offset++; - uint32_t nextChar = family->getCoverage()->nextSetBit((i + 1) << kLogCharsPerPage); + uint32_t nextChar = family->getCoverage().nextSetBit((i + 1) << kLogCharsPerPage); #ifdef VERBOSE_DEBUG ALOGD("nextChar = %d (j = %zd)\n", nextChar, j); #endif @@ -197,7 +193,7 @@ uint32_t FontCollection::calcFamilyScore(uint32_t ch, uint32_t vs, int variant, // variation sequence's base character. uint32_t FontCollection::calcCoverageScore(uint32_t ch, uint32_t vs, FontFamily* fontFamily) const { const bool hasVSGlyph = (vs != 0) && fontFamily->hasGlyph(ch, vs); - if (!hasVSGlyph && !fontFamily->getCoverage()->get(ch)) { + if (!hasVSGlyph && !fontFamily->getCoverage().get(ch)) { // The font doesn't support either variation sequence or even the base character. return kUnsupportedFontScore; } @@ -416,7 +412,7 @@ void FontCollection::itemize(const uint16_t *string, size_t string_size, FontSty if (lastFamily != nullptr) { if (isStickyWhitelisted(ch)) { // Continue using existing font as long as it has coverage and is whitelisted - shouldContinueRun = lastFamily->getCoverage()->get(ch); + shouldContinueRun = lastFamily->getCoverage().get(ch); } else if (isVariationSelector(ch)) { // Always continue if the character is a variation selector. shouldContinueRun = true; @@ -436,7 +432,7 @@ void FontCollection::itemize(const uint16_t *string, size_t string_size, FontSty if (utf16Pos != 0 && ((U_GET_GC_MASK(ch) & U_GC_M_MASK) != 0 || (isEmojiModifier(ch) && isEmojiBase(prevCh))) && - family && family->getCoverage()->get(prevCh)) { + family && family->getCoverage().get(prevCh)) { const size_t prevChLength = U16_LENGTH(prevCh); run->end -= prevChLength; if (run->start == run->end) { diff --git a/libs/minikin/FontFamily.cpp b/libs/minikin/FontFamily.cpp index 8efa32a937e..164cc7d8ad8 100644 --- a/libs/minikin/FontFamily.cpp +++ b/libs/minikin/FontFamily.cpp @@ -64,45 +64,51 @@ uint32_t FontStyle::pack(int variant, int weight, bool italic) { return (weight & kWeightMask) | (italic ? kItalicMask : 0) | (variant << kVariantShift); } -FontFamily::FontFamily() : FontFamily(0 /* variant */) { +Font::Font(MinikinFont* typeface, FontStyle style) + : typeface(typeface), style(style) { + typeface->Ref(); } -FontFamily::FontFamily(int variant) : FontFamily(FontLanguageListCache::kEmptyListId, variant) { +Font::Font(Font&& o) { + typeface = o.typeface; + style = o.style; + o.typeface = nullptr; +} + +Font::Font(const Font& o) { + typeface = o.typeface; + typeface->Ref(); + style = o.style; +} + +Font::~Font() { + if (typeface == nullptr) { + return; + } + typeface->UnrefLocked(); +} + +FontFamily::FontFamily(std::vector&& fonts) : FontFamily(0 /* variant */, std::move(fonts)) { +} + +FontFamily::FontFamily(int variant, std::vector&& fonts) + : FontFamily(FontLanguageListCache::kEmptyListId, variant, std::move(fonts)) { +} + +FontFamily::FontFamily(uint32_t langId, int variant, std::vector&& fonts) + : mLangId(langId), mVariant(variant), mFonts(std::move(fonts)), mHasVSTable(false) { + computeCoverage(); } FontFamily::~FontFamily() { - for (size_t i = 0; i < mFonts.size(); i++) { - mFonts[i].typeface->UnrefLocked(); - } } -bool FontFamily::addFont(MinikinFont* typeface) { +bool FontFamily::analyzeStyle(MinikinFont* typeface, int* weight, bool* italic) { android::AutoMutex _l(gMinikinLock); const uint32_t os2Tag = MinikinFont::MakeTag('O', 'S', '/', '2'); HbBlob os2Table(getFontTable(typeface, os2Tag)); if (os2Table.get() == nullptr) return false; - int weight; - bool italic; - if (analyzeStyle(os2Table.get(), os2Table.size(), &weight, &italic)) { - //ALOGD("analyzed weight = %d, italic = %s", weight, italic ? "true" : "false"); - FontStyle style(weight, italic); - addFontLocked(typeface, style); - return true; - } else { - ALOGD("failed to analyze style"); - } - return false; -} - -void FontFamily::addFont(MinikinFont* typeface, FontStyle style) { - android::AutoMutex _l(gMinikinLock); - addFontLocked(typeface, style); -} - -void FontFamily::addFontLocked(MinikinFont* typeface, FontStyle style) { - typeface->RefLocked(); - mFonts.push_back(Font(typeface, style)); - mCoverageValid = false; + return ::minikin::analyzeStyle(os2Table.get(), os2Table.size(), weight, italic); } // Compute a matching metric between two styles - 0 is an exact match @@ -146,18 +152,6 @@ FakedFont FontFamily::getClosestMatch(FontStyle style) const { return result; } -size_t FontFamily::getNumFonts() const { - return mFonts.size(); -} - -MinikinFont* FontFamily::getFont(size_t index) const { - return mFonts[index].typeface; -} - -FontStyle FontFamily::getStyle(size_t index) const { - return mFonts[index].style; -} - bool FontFamily::isColorEmojiFamily() const { const FontLanguages& languageList = FontLanguageListCache::getById(mLangId); for (size_t i = 0; i < languageList.size(); ++i) { @@ -168,30 +162,24 @@ bool FontFamily::isColorEmojiFamily() const { return false; } -const SparseBitSet* FontFamily::getCoverage() { - if (!mCoverageValid) { - const FontStyle defaultStyle; - MinikinFont* typeface = getClosestMatch(defaultStyle).font; - const uint32_t cmapTag = MinikinFont::MakeTag('c', 'm', 'a', 'p'); - HbBlob cmapTable(getFontTable(typeface, cmapTag)); - if (cmapTable.get() == nullptr) { - ALOGE("Could not get cmap table size!\n"); - // Note: This means we will retry on the next call to getCoverage, as we can't store - // the failure. This is fine, as we assume this doesn't really happen in practice. - return nullptr; - } - // TODO: Error check? - CmapCoverage::getCoverage(mCoverage, cmapTable.get(), cmapTable.size(), &mHasVSTable); -#ifdef VERBOSE_DEBUG - ALOGD("font coverage length=%d, first ch=%x\n", mCoverage.length(), - mCoverage.nextSetBit(0)); -#endif - mCoverageValid = true; +void FontFamily::computeCoverage() { + android::AutoMutex _l(gMinikinLock); + const FontStyle defaultStyle; + MinikinFont* typeface = getClosestMatch(defaultStyle).font; + const uint32_t cmapTag = MinikinFont::MakeTag('c', 'm', 'a', 'p'); + HbBlob cmapTable(getFontTable(typeface, cmapTag)); + if (cmapTable.get() == nullptr) { + ALOGE("Could not get cmap table size!\n"); + return; } - return &mCoverage; + // TODO: Error check? + CmapCoverage::getCoverage(mCoverage, cmapTable.get(), cmapTable.size(), &mHasVSTable); +#ifdef VERBOSE_DEBUG + ALOGD("font coverage length=%d, first ch=%x\n", mCoverage.length(), mCoverage.nextSetBit(0)); +#endif } -bool FontFamily::hasGlyph(uint32_t codepoint, uint32_t variationSelector) { +bool FontFamily::hasGlyph(uint32_t codepoint, uint32_t variationSelector) const { assertMinikinLocked(); if (variationSelector != 0 && !mHasVSTable) { // Early exit if the variation selector is specified but the font doesn't have a cmap format @@ -208,9 +196,4 @@ bool FontFamily::hasGlyph(uint32_t codepoint, uint32_t variationSelector) { return result; } -bool FontFamily::hasVSTable() const { - LOG_ALWAYS_FATAL_IF(!mCoverageValid, "Do not call this method before getCoverage() call"); - return mHasVSTable; -} - } // namespace minikin diff --git a/sample/example.cpp b/sample/example.cpp index a27918ea758..1c9c322d232 100644 --- a/sample/example.cpp +++ b/sample/example.cpp @@ -45,9 +45,9 @@ FontCollection *makeFontCollection() { "/system/fonts/Roboto-LightItalic.ttf" }; - FontFamily *family = new FontFamily(); FT_Face face; FT_Error error; + std::vector fonts; for (size_t i = 0; i < sizeof(fns)/sizeof(fns[0]); i++) { const char *fn = fns[i]; printf("adding %s\n", fn); @@ -56,16 +56,16 @@ FontCollection *makeFontCollection() { printf("error loading %s, %d\n", fn, error); } MinikinFont *font = new MinikinFontFreeType(face); - family->addFont(font); + fonts.push_back(Font(font, FontStyle())); } + FontFamily *family = new FontFamily(std::move(fonts)); typefaces.push_back(family); #if 1 - family = new FontFamily(); const char *fn = "/system/fonts/DroidSansDevanagari-Regular.ttf"; error = FT_New_Face(library, fn, 0, &face); MinikinFont *font = new MinikinFontFreeType(face); - family->addFont(font); + family = new FontFamily(std::vector({ Font(font, FontStyle()) })); typefaces.push_back(family); #endif diff --git a/sample/example_skia.cpp b/sample/example_skia.cpp index b04c8abcbcc..6e6f868051c 100644 --- a/sample/example_skia.cpp +++ b/sample/example_skia.cpp @@ -54,21 +54,21 @@ FontCollection *makeFontCollection() { "/system/fonts/Roboto-LightItalic.ttf" }; - FontFamily *family = new FontFamily(); + std::vector fonts; for (size_t i = 0; i < sizeof(fns)/sizeof(fns[0]); i++) { const char *fn = fns[i]; sk_sp skFace = SkTypeface::MakeFromFile(fn); MinikinFont *font = new MinikinFontSkia(std::move(skFace)); - family->addFont(font); + fonts.push_back(Font(font, FontStyle())); } + FontFamily *family = new FontFamily(std::move(fonts)); typefaces.push_back(family); #if 1 - family = new FontFamily(); const char *fn = "/system/fonts/DroidSansDevanagari-Regular.ttf"; sk_sp skFace = SkTypeface::MakeFromFile(fn); MinikinFont *font = new MinikinFontSkia(std::move(skFace)); - family->addFont(font); + family = new FontFamily(std::vector({ Font(font, FontStyle()) })); typefaces.push_back(family); #endif diff --git a/tests/unittest/FontCollectionItemizeTest.cpp b/tests/unittest/FontCollectionItemizeTest.cpp index 52786d0c7e0..5c5a5d343cb 100644 --- a/tests/unittest/FontCollectionItemizeTest.cpp +++ b/tests/unittest/FontCollectionItemizeTest.cpp @@ -668,14 +668,14 @@ TEST_F(FontCollectionItemizeTest, itemize_vs_sequence_but_no_base_char) { const std::string kVSTestFont = kTestFontDir "VarioationSelectorTest-Regular.ttf"; std::vector families; - FontFamily* family1 = new FontFamily(VARIANT_DEFAULT); MinikinAutoUnref font(new MinikinFontForTest(kLatinFont)); - family1->addFont(font.get()); + FontFamily* family1 = new FontFamily(VARIANT_DEFAULT, + std::vector{ Font(font.get(), FontStyle()) }); families.push_back(family1); - FontFamily* family2 = new FontFamily(VARIANT_DEFAULT); MinikinAutoUnref font2(new MinikinFontForTest(kVSTestFont)); - family2->addFont(font2.get()); + FontFamily* family2 = new FontFamily(VARIANT_DEFAULT, + std::vector{ Font(font2.get(), FontStyle()) }); families.push_back(family2); FontCollection collection(families); @@ -811,11 +811,11 @@ TEST_F(FontCollectionItemizeTest, itemize_LanguageScore) { std::vector families; // Prepare first font which doesn't supports U+9AA8 - FontFamily* firstFamily = new FontFamily( - FontStyle::registerLanguageList("und"), 0 /* variant */); MinikinAutoUnref firstFamilyMinikinFont( new MinikinFontForTest(kNoGlyphFont)); - firstFamily->addFont(firstFamilyMinikinFont.get()); + FontFamily* firstFamily = new FontFamily( + FontStyle::registerLanguageList("und"), 0 /* variant */, + std::vector({ Font(firstFamilyMinikinFont.get(), FontStyle()) })); families.push_back(firstFamily); // Prepare font families @@ -824,10 +824,10 @@ TEST_F(FontCollectionItemizeTest, itemize_LanguageScore) { std::unordered_map fontLangIdxMap; for (size_t i = 0; i < testCase.fontLanguages.size(); ++i) { - FontFamily* family = new FontFamily( - FontStyle::registerLanguageList(testCase.fontLanguages[i]), 0 /* variant */); MinikinAutoUnref minikin_font(new MinikinFontForTest(kJAFont)); - family->addFont(minikin_font.get()); + FontFamily* family = new FontFamily( + FontStyle::registerLanguageList(testCase.fontLanguages[i]), 0 /* variant */, + std::vector({ Font(minikin_font.get(), FontStyle()) })); families.push_back(family); fontLangIdxMap.insert(std::make_pair(minikin_font.get(), i)); } @@ -1417,13 +1417,12 @@ TEST_F(FontCollectionItemizeTest, itemizeShouldKeepOrderForVS) { MinikinAutoUnref fontA(new MinikinFontForTest(kZH_HansFont)); MinikinAutoUnref fontB(new MinikinFontForTest(kZH_HansFont)); - MinikinAutoUnref dummyFamily(new FontFamily()); - MinikinAutoUnref familyA(new FontFamily()); - MinikinAutoUnref familyB(new FontFamily()); - - dummyFamily->addFont(dummyFont.get()); - familyA->addFont(fontA.get()); - familyB->addFont(fontB.get()); + MinikinAutoUnref dummyFamily(new FontFamily( + std::vector({ Font(dummyFont.get(), FontStyle()) }))); + MinikinAutoUnref familyA(new FontFamily( + std::vector({ Font(fontA.get(), FontStyle()) }))); + MinikinAutoUnref familyB(new FontFamily( + std::vector({ Font(fontB.get(), FontStyle()) }))); std::vector families = { dummyFamily.get(), familyA.get(), familyB.get() }; @@ -1453,13 +1452,12 @@ TEST_F(FontCollectionItemizeTest, itemizeShouldKeepOrderForVS2) { MinikinAutoUnref noCmapFormat14Font( new MinikinFontForTest(kNoCmapFormat14Font)); - MinikinAutoUnref dummyFamily(new FontFamily()); - MinikinAutoUnref hasCmapFormat14Family(new FontFamily()); - MinikinAutoUnref noCmapFormat14Family(new FontFamily()); - - dummyFamily->addFont(dummyFont.get()); - hasCmapFormat14Family->addFont(hasCmapFormat14Font.get()); - noCmapFormat14Family->addFont(noCmapFormat14Font.get()); + MinikinAutoUnref dummyFamily(new FontFamily( + std::vector({ Font(dummyFont.get(), FontStyle()) }))); + MinikinAutoUnref hasCmapFormat14Family(new FontFamily( + std::vector({ Font(hasCmapFormat14Font.get(), FontStyle()) }))); + MinikinAutoUnref noCmapFormat14Family(new FontFamily( + std::vector({ Font(noCmapFormat14Font.get(), FontStyle()) }))); std::vector families = { dummyFamily.get(), hasCmapFormat14Family.get(), noCmapFormat14Family.get() }; diff --git a/tests/unittest/FontCollectionTest.cpp b/tests/unittest/FontCollectionTest.cpp index 62d2f022fa3..c0b6b526b78 100644 --- a/tests/unittest/FontCollectionTest.cpp +++ b/tests/unittest/FontCollectionTest.cpp @@ -57,9 +57,9 @@ void expectVSGlyphs(const FontCollection* fc, uint32_t codepoint, const std::set } TEST(FontCollectionTest, hasVariationSelectorTest) { - MinikinAutoUnref family(new FontFamily()); MinikinAutoUnref font(new MinikinFontForTest(kVsTestFont)); - family->addFont(font.get()); + MinikinAutoUnref family(new FontFamily( + std::vector({ Font(font.get(), FontStyle()) }))); std::vector families({family.get()}); MinikinAutoUnref fc(new FontCollection(families)); diff --git a/tests/unittest/FontFamilyTest.cpp b/tests/unittest/FontFamilyTest.cpp index 44098018929..ddc36e45704 100644 --- a/tests/unittest/FontFamilyTest.cpp +++ b/tests/unittest/FontFamilyTest.cpp @@ -464,8 +464,10 @@ void expectVSGlyphs(FontFamily* family, uint32_t codepoint, const std::set minikinFont(new MinikinFontForTest(kVsTestFont)); - MinikinAutoUnref family(new FontFamily); - family->addFont(minikinFont.get()); + MinikinAutoUnref family( + new FontFamily(std::vector{ + Font(minikinFont.get(), FontStyle()) + })); android::AutoMutex _l(gMinikinLock); @@ -478,23 +480,23 @@ TEST_F(FontFamilyTest, hasVariationSelectorTest) { const uint32_t kVS20 = 0xE0103; const uint32_t kSupportedChar1 = 0x82A6; - EXPECT_TRUE(family->getCoverage()->get(kSupportedChar1)); + EXPECT_TRUE(family->getCoverage().get(kSupportedChar1)); expectVSGlyphs(family.get(), kSupportedChar1, std::set({kVS1, kVS17, kVS18, kVS19})); const uint32_t kSupportedChar2 = 0x845B; - EXPECT_TRUE(family->getCoverage()->get(kSupportedChar2)); + EXPECT_TRUE(family->getCoverage().get(kSupportedChar2)); expectVSGlyphs(family.get(), kSupportedChar2, std::set({kVS2, kVS18, kVS19, kVS20})); const uint32_t kNoVsSupportedChar = 0x537F; - EXPECT_TRUE(family->getCoverage()->get(kNoVsSupportedChar)); + EXPECT_TRUE(family->getCoverage().get(kNoVsSupportedChar)); expectVSGlyphs(family.get(), kNoVsSupportedChar, std::set()); const uint32_t kVsOnlySupportedChar = 0x717D; - EXPECT_FALSE(family->getCoverage()->get(kVsOnlySupportedChar)); + EXPECT_FALSE(family->getCoverage().get(kVsOnlySupportedChar)); expectVSGlyphs(family.get(), kVsOnlySupportedChar, std::set({kVS3, kVS19, kVS20})); const uint32_t kNotSupportedChar = 0x845C; - EXPECT_FALSE(family->getCoverage()->get(kNotSupportedChar)); + EXPECT_FALSE(family->getCoverage().get(kNotSupportedChar)); expectVSGlyphs(family.get(), kNotSupportedChar, std::set()); } @@ -518,11 +520,9 @@ TEST_F(FontFamilyTest, hasVSTableTest) { MinikinAutoUnref minikinFont( new MinikinFontForTest(testCase.fontPath)); - MinikinAutoUnref family(new FontFamily); - family->addFont(minikinFont.get()); + MinikinAutoUnref family(new FontFamily( + std::vector{ Font(minikinFont.get(), FontStyle()) })); android::AutoMutex _l(gMinikinLock); - family->getCoverage(); - EXPECT_EQ(testCase.hasVSTable, family->hasVSTable()); } } diff --git a/tests/util/FontTestUtils.cpp b/tests/util/FontTestUtils.cpp index 0c7b2ba032c..e29a2fe5344 100644 --- a/tests/util/FontTestUtils.cpp +++ b/tests/util/FontTestUtils.cpp @@ -48,16 +48,7 @@ FontCollection* getFontCollection(const char* fontDir, const char* fontXml) { } } - xmlChar* lang = xmlGetProp(familyNode, (const xmlChar*)"lang"); - FontFamily* family; - if (lang == nullptr) { - family = new FontFamily(variant); - } else { - uint32_t langId = FontStyle::registerLanguageList( - std::string((const char*)lang, xmlStrlen(lang))); - family = new FontFamily(langId, variant); - } - + std::vector fonts; for (xmlNode* fontNode = familyNode->children; fontNode; fontNode = fontNode->next) { if (xmlStrcmp(fontNode->name, (const xmlChar*)"font") != 0) { continue; @@ -80,13 +71,23 @@ FontCollection* getFontCollection(const char* fontDir, const char* fontXml) { if (index == nullptr) { MinikinAutoUnref minikinFont(new MinikinFontForTest(fontPath)); - family->addFont(minikinFont.get(), FontStyle(weight, italic)); + fonts.push_back(Font(minikinFont.get(), FontStyle(weight, italic))); } else { MinikinAutoUnref minikinFont(new MinikinFontForTest(fontPath, atoi((const char*)index))); - family->addFont(minikinFont.get(), FontStyle(weight, italic)); + fonts.push_back(Font(minikinFont.get(), FontStyle(weight, italic))); } } + + xmlChar* lang = xmlGetProp(familyNode, (const xmlChar*)"lang"); + FontFamily* family; + if (lang == nullptr) { + family = new FontFamily(variant, std::move(fonts)); + } else { + uint32_t langId = FontStyle::registerLanguageList( + std::string((const char*)lang, xmlStrlen(lang))); + family = new FontFamily(langId, variant, std::move(fonts)); + } families.push_back(family); } xmlFreeDoc(doc);