Merge "Improve Paint.measureText and Paint.hasGlyph for variation sequences." into nyc-dev

7 files changed, 91 insertions, 18 deletions

diff --git a/include/minikin/CmapCoverage.h b/include/minikin/CmapCoverage.h
index 7054e31..56abac7 100644
--- a/include/minikin/CmapCoverage.h
+++ b/include/minikin/CmapCoverage.h
@@ -23,7 +23,8 @@ namespace android {
 
 class CmapCoverage {
 public:
-    static bool getCoverage(SparseBitSet &coverage, const uint8_t* cmap_data, size_t cmap_size);
+    static bool getCoverage(SparseBitSet &coverage, const uint8_t* cmap_data, size_t cmap_size,
+            bool* has_cmap_format14_subtable);
 };
 
 }  // namespace android
diff --git a/include/minikin/FontCollection.h b/include/minikin/FontCollection.h
index 3a63c07..5b9424c 100644
--- a/include/minikin/FontCollection.h
+++ b/include/minikin/FontCollection.h
@@ -89,6 +89,9 @@ private:
     // This vector contains pointers into mInstances
     std::vector<FontFamily*> mFamilyVec;
 
+    // This vector has pointers to the font family instance which has cmap 14 subtable.
+    std::vector<FontFamily*> mVSFamilyVec;
+
     // These are offsets into mInstanceVec, one range per page
     std::vector<Range> mRanges;
 };
diff --git a/include/minikin/FontFamily.h b/include/minikin/FontFamily.h
index 2b59160..149dc7b 100644
--- a/include/minikin/FontFamily.h
+++ b/include/minikin/FontFamily.h
@@ -104,7 +104,11 @@ public:
 
     FontFamily(int variant);
 
-    FontFamily(uint32_t langId, int variant) : mLangId(langId), mVariant(variant) {
+    FontFamily(uint32_t langId, int variant)
+        : mLangId(langId),
+        mVariant(variant),
+        mHasVSTable(false),
+        mCoverageValid(false) {
     }
 
     ~FontFamily();
@@ -131,6 +135,9 @@ public:
     // Caller should acquire a lock before calling the method.
     bool hasVariationSelector(uint32_t codepoint, uint32_t variationSelector);
 
+    // Returns true if this font family has a variaion sequence table (cmap format 14 subtable).
+    bool hasVSTable() const;
+
 private:
     void addFontLocked(MinikinFont* typeface, FontStyle style);
 
@@ -146,6 +153,7 @@ private:
     std::vector<Font> mFonts;
 
     SparseBitSet mCoverage;
+    bool mHasVSTable;
     bool mCoverageValid;
 };
 
diff --git a/libs/minikin/CmapCoverage.cpp b/libs/minikin/CmapCoverage.cpp
index a4503af..2961d2f 100644
--- a/libs/minikin/CmapCoverage.cpp
+++ b/libs/minikin/CmapCoverage.cpp
@@ -131,7 +131,8 @@ static bool getCoverageFormat12(vector<uint32_t>& coverage, const uint8_t* data,
     return true;
 }
 
-bool CmapCoverage::getCoverage(SparseBitSet& coverage, const uint8_t* cmap_data, size_t cmap_size) {
+bool CmapCoverage::getCoverage(SparseBitSet& coverage, const uint8_t* cmap_data, size_t cmap_size,
+        bool* has_cmap_format14_subtable) {
     vector<uint32_t> coverageVec;
     const size_t kHeaderSize = 4;
     const size_t kNumTablesOffset = 2;
@@ -139,8 +140,10 @@ bool CmapCoverage::getCoverage(SparseBitSet& coverage, const uint8_t* cmap_data,
     const size_t kPlatformIdOffset = 0;
     const size_t kEncodingIdOffset = 2;
     const size_t kOffsetOffset = 4;
+    const uint16_t kUnicodePlatformId = 0;
     const uint16_t kMicrosoftPlatformId = 3;
     const uint16_t kUnicodeBmpEncodingId = 1;
+    const uint16_t kVariationSequencesEncodingId = 5;
     const uint16_t kUnicodeUcs4EncodingId = 10;
     const uint32_t kNoTable = UINT32_MAX;
     if (kHeaderSize > cmap_size) {
@@ -151,6 +154,7 @@ bool CmapCoverage::getCoverage(SparseBitSet& coverage, const uint8_t* cmap_data,
         return false;
     }
     uint32_t bestTable = kNoTable;
+    bool hasCmapFormat14Subtable = false;
     for (uint32_t i = 0; i < numTables; i++) {
         uint16_t platformId = readU16(cmap_data, kHeaderSize + i * kTableSize + kPlatformIdOffset);
         uint16_t encodingId = readU16(cmap_data, kHeaderSize + i * kTableSize + kEncodingIdOffset);
@@ -159,8 +163,15 @@ bool CmapCoverage::getCoverage(SparseBitSet& coverage, const uint8_t* cmap_data,
             break;
         } else if (platformId == kMicrosoftPlatformId && encodingId == kUnicodeBmpEncodingId) {
             bestTable = i;
+        } else if (platformId == kUnicodePlatformId &&
+                encodingId == kVariationSequencesEncodingId) {
+            uint32_t offset = readU32(cmap_data, kHeaderSize + i * kTableSize + kOffsetOffset);
+            if (offset <= cmap_size - 2 && readU16(cmap_data, offset) == 14) {
+                hasCmapFormat14Subtable = true;
+            }
         }
     }
+    *has_cmap_format14_subtable = hasCmapFormat14Subtable;
 #ifdef VERBOSE_DEBUG
     ALOGD("best table = %d\n", bestTable);
 #endif
diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp
index 26aefe4..dd905a3 100644
--- a/libs/minikin/FontCollection.cpp
+++ b/libs/minikin/FontCollection.cpp
@@ -62,6 +62,9 @@ FontCollection::FontCollection(const vector<FontFamily*>& typefaces) :
             continue;
         }
         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));
     }
@@ -233,15 +236,22 @@ FontFamily* FontCollection::getFamilyForChar(uint32_t ch, uint32_t vs,
         return NULL;
     }
 
-    // Even if the font supports variation sequence, mRanges isn't aware of the base character of
-    // the sequence. Search all FontFamilies if variation sequence is specified.
-    // TODO: Always use mRanges for font search.
-    const std::vector<FontFamily*>& familyVec = (vs == 0) ? mFamilyVec : mFamilies;
-    Range range;
-    if (vs == 0) {
-        range = mRanges[ch >> kLogCharsPerPage];
-    } else {
-        range = { 0, mFamilies.size() };
+    const std::vector<FontFamily*>* familyVec = &mFamilyVec;
+    Range range = mRanges[ch >> kLogCharsPerPage];
+
+    std::vector<FontFamily*> 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() };
     }
 
 #ifdef VERBOSE_DEBUG
@@ -250,7 +260,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
@@ -310,12 +320,15 @@ bool FontCollection::hasVariationSelector(uint32_t baseCodepoint,
     if (baseCodepoint >= mMaxChar) {
         return false;
     }
+    if (variationSelector == 0) {
+        return false;
+    }
+
     // Currently mRanges can not be used here since it isn't aware of the variation sequence.
-    // TODO: Use mRanges for narrowing down the search range.
-    for (size_t i = 0; i < mFamilies.size(); i++) {
+    for (size_t i = 0; i < mVSFamilyVec.size(); i++) {
         AutoMutex _l(gMinikinLock);
-        if (mFamilies[i]->hasVariationSelector(baseCodepoint, variationSelector)) {
-          return true;
+        if (mVSFamilyVec[i]->hasVariationSelector(baseCodepoint, variationSelector)) {
+            return true;
         }
     }
     return false;
diff --git a/libs/minikin/FontFamily.cpp b/libs/minikin/FontFamily.cpp
index 9eda3c2..88448a1 100644
--- a/libs/minikin/FontFamily.cpp
+++ b/libs/minikin/FontFamily.cpp
@@ -177,7 +177,8 @@ const SparseBitSet* FontFamily::getCoverage() {
             ALOGE("Unexpected failure to read cmap table!\n");
             return nullptr;
         }
-        CmapCoverage::getCoverage(mCoverage, cmapData.get(), cmapSize);  // TODO: Error check?
+        // TODO: Error check?
+        CmapCoverage::getCoverage(mCoverage, cmapData.get(), cmapSize, &mHasVSTable);
 #ifdef VERBOSE_DEBUG
         ALOGD("font coverage length=%d, first ch=%x\n", mCoverage.length(),
                 mCoverage.nextSetBit(0));
@@ -189,6 +190,10 @@ const SparseBitSet* FontFamily::getCoverage() {
 
 bool FontFamily::hasVariationSelector(uint32_t codepoint, uint32_t variationSelector) {
     assertMinikinLocked();
+    if (!mHasVSTable) {
+        return false;
+    }
+
     const FontStyle defaultStyle;
     MinikinFont* minikinFont = getClosestMatch(defaultStyle).font;
     hb_font_t* font = getHbFontLocked(minikinFont);
@@ -196,4 +201,9 @@ bool FontFamily::hasVariationSelector(uint32_t codepoint, uint32_t variationSele
     return hb_font_get_glyph(font, codepoint, variationSelector, &unusedGlyph);
 }
 
+bool FontFamily::hasVSTable() const {
+    LOG_ALWAYS_FATAL_IF(!mCoverageValid, "Do not call this method before getCoverage() call");
+    return mHasVSTable;
+}
+
 }  // namespace android
diff --git a/tests/FontFamilyTest.cpp b/tests/FontFamilyTest.cpp
index fef464f..11407a3 100644
--- a/tests/FontFamilyTest.cpp
+++ b/tests/FontFamilyTest.cpp
@@ -378,4 +378,31 @@ TEST_F(FontFamilyTest, hasVariationSelectorTest) {
     expectVSGlyphs(&family, kNotSupportedChar, std::set<uint32_t>());
 }
 
+TEST_F(FontFamilyTest, hasVSTableTest) {
+    struct TestCase {
+        const std::string fontPath;
+        bool hasVSTable;
+    } testCases[] = {
+        { kTestFontDir "Ja.ttf", true },
+        { kTestFontDir "ZhHant.ttf", true },
+        { kTestFontDir "ZhHans.ttf", true },
+        { kTestFontDir "Italic.ttf", false },
+        { kTestFontDir "Bold.ttf", false },
+        { kTestFontDir "BoldItalic.ttf", false },
+    };
+
+    for (auto testCase : testCases) {
+        SCOPED_TRACE(testCase.hasVSTable ?
+                "Font " + testCase.fontPath + " should have a variation sequence table." :
+                "Font " + testCase.fontPath + " shouldn't have a variation sequence table.");
+
+        MinikinFontForTest minikinFont(testCase.fontPath);
+        FontFamily family;
+        family.addFont(&minikinFont);
+        family.getCoverage();
+
+        EXPECT_EQ(testCase.hasVSTable, family.hasVSTable());
+    }
+}
+
 }  // namespace android