diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp index 97c206881f1..19ad7523f7d 100644 --- a/libs/minikin/FontCollection.cpp +++ b/libs/minikin/FontCollection.cpp @@ -281,22 +281,11 @@ FontFamily* FontCollection::getFamilyForChar(uint32_t ch, uint32_t vs, return mFamilies[0]; } - const std::vector* familyVec = &mFamilyVec; + const std::vector& familyVec = (vs == 0) ? mFamilyVec : mFamilies; Range range = mRanges[ch >> kLogCharsPerPage]; - std::vector familyVecForVS; if (vs != 0) { - // If variation selector is specified, need to search for both the variation sequence and - // its base codepoint. Compute the union vector of them. - familyVecForVS = mVSFamilyVec; - familyVecForVS.insert(familyVecForVS.end(), - mFamilyVec.begin() + range.start, mFamilyVec.begin() + range.end); - std::sort(familyVecForVS.begin(), familyVecForVS.end()); - auto last = std::unique(familyVecForVS.begin(), familyVecForVS.end()); - familyVecForVS.erase(last, familyVecForVS.end()); - - familyVec = &familyVecForVS; - range = { 0, familyVecForVS.size() }; + range = { 0, mFamilies.size() }; } #ifdef VERBOSE_DEBUG @@ -305,7 +294,7 @@ FontFamily* FontCollection::getFamilyForChar(uint32_t ch, uint32_t vs, FontFamily* bestFamily = nullptr; uint32_t bestScore = kUnsupportedFontScore; for (size_t i = range.start; i < range.end; i++) { - FontFamily* family = (*familyVec)[i]; + FontFamily* family = familyVec[i]; const uint32_t score = calcFamilyScore(ch, vs, variant, langListId, family); if (score == kFirstFontScore) { // If the first font family supports the given character or variation sequence, always diff --git a/tests/data/NoCmapFormat14.ttf b/tests/data/NoCmapFormat14.ttf new file mode 100644 index 00000000000..2a0c46c7aca Binary files /dev/null and b/tests/data/NoCmapFormat14.ttf differ diff --git a/tests/data/NoCmapFormat14.ttx b/tests/data/NoCmapFormat14.ttx new file mode 100644 index 00000000000..3c7411b194e --- /dev/null +++ b/tests/data/NoCmapFormat14.ttx @@ -0,0 +1,207 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + No Cmap Format 14 Subtable Test + + + Regular + + + No Cmap Format 14 Subtable Test + + + No Cmap Format 14 SubtableTest-Regular + + + No Cmap Format 14 Subtable Test + + + Regular + + + No Cmap Format 14 Subtable Test + + + No Cmap Format 14 SubtableTest-Regular + + + + + + + + + + + + + + + + diff --git a/tests/data/VarioationSelectorTest-Regular.ttf b/tests/data/VarioationSelectorTest-Regular.ttf index dfb0b2d89ea..0504c67fb7a 100644 Binary files a/tests/data/VarioationSelectorTest-Regular.ttf and b/tests/data/VarioationSelectorTest-Regular.ttf differ diff --git a/tests/data/VarioationSelectorTest-Regular.ttx b/tests/data/VarioationSelectorTest-Regular.ttx index a063a5ee934..f86f008b081 100644 --- a/tests/data/VarioationSelectorTest-Regular.ttx +++ b/tests/data/VarioationSelectorTest-Regular.ttx @@ -18,18 +18,7 @@ - - - - - - - - - - - - + @@ -147,45 +136,36 @@ - - - - - - - - - - - - + - - - + + + + - - - + + + - - - + + + - - - + + + + @@ -202,40 +182,7 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + diff --git a/tests/unittest/Android.mk b/tests/unittest/Android.mk index b43e3c85703..a81d17cca7e 100644 --- a/tests/unittest/Android.mk +++ b/tests/unittest/Android.mk @@ -32,6 +32,7 @@ font_src_files := \ data/Italic.ttf \ data/Ja.ttf \ data/Ko.ttf \ + data/NoCmapFormat14.ttf \ data/NoGlyphFont.ttf \ data/Regular.ttf \ data/TextEmojiFont.ttf \ diff --git a/tests/unittest/FontCollectionItemizeTest.cpp b/tests/unittest/FontCollectionItemizeTest.cpp index 367739683d1..978ba9f499f 100644 --- a/tests/unittest/FontCollectionItemizeTest.cpp +++ b/tests/unittest/FontCollectionItemizeTest.cpp @@ -44,6 +44,9 @@ const char kColorEmojiFont[] = kTestFontDir "ColorEmojiFont.ttf"; const char kTextEmojiFont[] = kTestFontDir "TextEmojiFont.ttf"; const char kMixedEmojiFont[] = kTestFontDir "ColorTextMixedEmojiFont.ttf"; +const char kHasCmapFormat14Font[] = kTestFontDir "NoCmapFormat14.ttf"; +const char kNoCmapFormat14Font[] = kTestFontDir "VarioationSelectorTest-Regular.ttf"; + typedef ICUTestBase FontCollectionItemizeTest; // Utility function for calling itemize function. @@ -1392,4 +1395,74 @@ TEST_F(FontCollectionItemizeTest, itemize_genderBalancedEmoji) { EXPECT_EQ(kColorEmojiFont, getFontPath(runs[0])); } +// For b/29585939 +TEST_F(FontCollectionItemizeTest, itemizeShouldKeepOrderForVS) { + const FontStyle kDefaultFontStyle; + + MinikinAutoUnref dummyFont(MinikinFontForTest::createFromFile(kNoGlyphFont)); + MinikinAutoUnref fontA(MinikinFontForTest::createFromFile(kZH_HansFont)); + MinikinAutoUnref fontB(MinikinFontForTest::createFromFile(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()); + + std::vector families = + { dummyFamily.get(), familyA.get(), familyB.get() }; + std::vector reversedFamilies = + { dummyFamily.get(), familyB.get(), familyA.get() }; + + MinikinAutoUnref collection(new FontCollection(families)); + MinikinAutoUnref reversedCollection(new FontCollection(reversedFamilies)); + + // Both fontA/fontB support U+35A8 but don't support U+35A8 U+E0100. The first font should be + // selected. + std::vector runs; + itemize(collection.get(), "U+35A8 U+E0100", kDefaultFontStyle, &runs); + EXPECT_EQ(fontA.get(), runs[0].fakedFont.font); + + itemize(reversedCollection.get(), "U+35A8 U+E0100", kDefaultFontStyle, &runs); + EXPECT_EQ(fontB.get(), runs[0].fakedFont.font); +} + +// For b/29585939 +TEST_F(FontCollectionItemizeTest, itemizeShouldKeepOrderForVS2) { + const FontStyle kDefaultFontStyle; + + MinikinAutoUnref dummyFont(MinikinFontForTest::createFromFile(kNoGlyphFont)); + MinikinAutoUnref hasCmapFormat14Font( + MinikinFontForTest::createFromFile(kHasCmapFormat14Font)); + MinikinAutoUnref noCmapFormat14Font( + MinikinFontForTest::createFromFile(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()); + + std::vector families = + { dummyFamily.get(), hasCmapFormat14Family.get(), noCmapFormat14Family.get() }; + std::vector reversedFamilies = + { dummyFamily.get(), noCmapFormat14Family.get(), hasCmapFormat14Family.get() }; + + MinikinAutoUnref collection(new FontCollection(families)); + MinikinAutoUnref reversedCollection(new FontCollection(reversedFamilies)); + + // Both hasCmapFormat14Font/noCmapFormat14Font support U+5380 but don't support U+5380 U+E0100. + // The first font should be selected. + std::vector runs; + itemize(collection.get(), "U+5380 U+E0100", kDefaultFontStyle, &runs); + EXPECT_EQ(hasCmapFormat14Font.get(), runs[0].fakedFont.font); + + itemize(reversedCollection.get(), "U+5380 U+E0100", kDefaultFontStyle, &runs); + EXPECT_EQ(noCmapFormat14Font.get(), runs[0].fakedFont.font); +} + } // namespace minikin