Clean Up: Removing unused interface GetTable from MinikinFont.

After Id766ab16a8d342bf7322a90e076e801271d527d4, GetTable is no longer
used in production due to poor performance and it is now only used in
tests. This CL removes GetTable interface from MinikinFont and update
tests code to use new interfaces, GetFontData, GetFontSize and
GetFontIndex.

Bug: 27860101
Test: Manually done

Change-Id: Ifcd7a348d7fb5af081192899dbcdfc7fb4eebbf9

include/minikin/MinikinFont.h

@@ -109,8 +109,6 @@ public:
     virtual void GetBounds(MinikinRect* bounds, uint32_t glyph_id,
         const MinikinPaint &paint) const = 0;
 
-    virtual const void* GetTable(uint32_t tag, size_t* size, MinikinDestroyFunc* destroy) = 0;
-
     // Override if font can provide access to raw data
     virtual const void* GetFontData() const {
         return nullptr;

libs/minikin/HbFontCache.cpp

@@ -28,22 +28,6 @@
 
 namespace minikin {
 
-static hb_blob_t* referenceTable(hb_face_t* /* face */, hb_tag_t tag, void* userData) {
-    MinikinFont* font = reinterpret_cast<MinikinFont*>(userData);
-    MinikinDestroyFunc destroy = 0;
-    size_t size = 0;
-    const void* buffer = font->GetTable(tag, &size, &destroy);
-    if (buffer == nullptr) {
-        return nullptr;
-    }
-#ifdef VERBOSE_DEBUG
-    ALOGD("referenceTable %c%c%c%c length=%zd",
-        (tag >>24)&0xff, (tag>>16)&0xff, (tag>>8)&0xff, tag&0xff, size);
-#endif
-    return hb_blob_create(reinterpret_cast<const char*>(buffer), size,
-            HB_MEMORY_MODE_READONLY, const_cast<void*>(buffer), destroy);
-}
-
 class HbFontCache : private android::OnEntryRemoved<int32_t, hb_font_t*> {
 public:
     HbFontCache() : mCache(kMaxEntries) {
@@ -119,15 +103,12 @@ hb_font_t* getHbFontLocked(MinikinFont* minikinFont) {
 
     hb_face_t* face;
     const void* buf = minikinFont->GetFontData();
-    if (buf == nullptr) {
-        face = hb_face_create_for_tables(referenceTable, minikinFont, nullptr);
-    } else {
-        size_t size = minikinFont->GetFontSize();
-        hb_blob_t* blob = hb_blob_create(reinterpret_cast<const char*>(buf), size,
-            HB_MEMORY_MODE_READONLY, nullptr, nullptr);
-        face = hb_face_create(blob, minikinFont->GetFontIndex());
-        hb_blob_destroy(blob);
-    }
+    size_t size = minikinFont->GetFontSize();
+    hb_blob_t* blob = hb_blob_create(reinterpret_cast<const char*>(buf), size,
+        HB_MEMORY_MODE_READONLY, nullptr, nullptr);
+    face = hb_face_create(blob, minikinFont->GetFontIndex());
+    hb_blob_destroy(blob);
+
     hb_font_t* parent_font = hb_font_create(face);
     hb_ot_font_set_funcs(parent_font);
 

tests/unittest/FontCollectionItemizeTest.cpp

@@ -666,12 +666,12 @@ TEST_F(FontCollectionItemizeTest, itemize_vs_sequence_but_no_base_char) {
 
     std::vector<FontFamily*> families;
     FontFamily* family1 = new FontFamily(VARIANT_DEFAULT);
-    MinikinAutoUnref<MinikinFont> font(MinikinFontForTest::createFromFile(kLatinFont));
+    MinikinAutoUnref<MinikinFont> font(new MinikinFontForTest(kLatinFont));
     family1->addFont(font.get());
     families.push_back(family1);
 
     FontFamily* family2 = new FontFamily(VARIANT_DEFAULT);
-    MinikinAutoUnref<MinikinFont> font2(MinikinFontForTest::createFromFile(kVSTestFont));
+    MinikinAutoUnref<MinikinFont> font2(new MinikinFontForTest(kVSTestFont));
     family2->addFont(font2.get());
     families.push_back(family2);
 
@@ -797,7 +797,7 @@ TEST_F(FontCollectionItemizeTest, itemize_LanguageScore) {
         FontFamily* firstFamily = new FontFamily(
                 FontStyle::registerLanguageList("und"), 0 /* variant */);
         MinikinAutoUnref<MinikinFont> firstFamilyMinikinFont(
-                MinikinFontForTest::createFromFile(kNoGlyphFont));
+                new MinikinFontForTest(kNoGlyphFont));
         firstFamily->addFont(firstFamilyMinikinFont.get());
         families.push_back(firstFamily);
 
@@ -809,7 +809,7 @@ TEST_F(FontCollectionItemizeTest, itemize_LanguageScore) {
         for (size_t i = 0; i < testCase.fontLanguages.size(); ++i) {
             FontFamily* family = new FontFamily(
                     FontStyle::registerLanguageList(testCase.fontLanguages[i]), 0 /* variant */);
-            MinikinAutoUnref<MinikinFont> minikin_font(MinikinFontForTest::createFromFile(kJAFont));
+            MinikinAutoUnref<MinikinFont> minikin_font(new MinikinFontForTest(kJAFont));
             family->addFont(minikin_font.get());
             families.push_back(family);
             fontLangIdxMap.insert(std::make_pair(minikin_font.get(), i));

tests/unittest/FontCollectionTest.cpp

@@ -58,7 +58,7 @@ void expectVSGlyphs(const FontCollection* fc, uint32_t codepoint, const std::set
 
 TEST(FontCollectionTest, hasVariationSelectorTest) {
   MinikinAutoUnref<FontFamily> family(new FontFamily());
-  MinikinAutoUnref<MinikinFont> font(MinikinFontForTest::createFromFile(kVsTestFont));
+  MinikinAutoUnref<MinikinFont> font(new MinikinFontForTest(kVsTestFont));
   family->addFont(font.get());
   std::vector<FontFamily*> families({family.get()});
   MinikinAutoUnref<FontCollection> fc(new FontCollection(families));

tests/unittest/FontFamilyTest.cpp

@@ -351,7 +351,7 @@ void expectVSGlyphs(FontFamily* family, uint32_t codepoint, const std::set<uint3
 
 TEST_F(FontFamilyTest, hasVariationSelectorTest) {
     MinikinAutoUnref<MinikinFontForTest>
-            minikinFont(MinikinFontForTest::createFromFile(kVsTestFont));
+            minikinFont(new MinikinFontForTest(kVsTestFont));
     MinikinAutoUnref<FontFamily> family(new FontFamily);
     family->addFont(minikinFont.get());
 
@@ -405,7 +405,7 @@ TEST_F(FontFamilyTest, hasVSTableTest) {
                 "Font " + testCase.fontPath + " shouldn't have a variation sequence table.");
 
         MinikinAutoUnref<MinikinFontForTest> minikinFont(
-                MinikinFontForTest::createFromFile(testCase.fontPath));
+                new MinikinFontForTest(testCase.fontPath));
         MinikinAutoUnref<FontFamily> family(new FontFamily);
         family->addFont(minikinFont.get());
         android::AutoMutex _l(gMinikinLock);

tests/unittest/HbFontCacheTest.cpp

@@ -38,13 +38,13 @@ public:
 
 TEST_F(HbFontCacheTest, getHbFontLockedTest) {
     MinikinAutoUnref<MinikinFontForTest> fontA(
-            MinikinFontForTest::createFromFile(kTestFontDir "Regular.ttf"));
+            new MinikinFontForTest(kTestFontDir "Regular.ttf"));
 
     MinikinAutoUnref<MinikinFontForTest> fontB(
-            MinikinFontForTest::createFromFile(kTestFontDir "Bold.ttf"));
+            new MinikinFontForTest(kTestFontDir "Bold.ttf"));
 
     MinikinAutoUnref<MinikinFontForTest> fontC(
-            MinikinFontForTest::createFromFile(kTestFontDir "BoldItalic.ttf"));
+            new MinikinFontForTest(kTestFontDir "BoldItalic.ttf"));
 
     android::AutoMutex _l(gMinikinLock);
     // Never return NULL.
@@ -66,7 +66,7 @@ TEST_F(HbFontCacheTest, getHbFontLockedTest) {
 
 TEST_F(HbFontCacheTest, purgeCacheTest) {
     MinikinAutoUnref<MinikinFontForTest> minikinFont(
-            MinikinFontForTest::createFromFile(kTestFontDir "Regular.ttf"));
+            new MinikinFontForTest(kTestFontDir "Regular.ttf"));
 
     android::AutoMutex _l(gMinikinLock);
     hb_font_t* font = getHbFontLocked(minikinFont.get());

tests/util/FontTestUtils.cpp

@@ -77,12 +77,11 @@ FontCollection* getFontCollection(const char* fontDir, const char* fontXml) {
 
             if (index == nullptr) {
                 MinikinAutoUnref<MinikinFontForTest>
-                        minikinFont(MinikinFontForTest::createFromFile(fontPath));
+                        minikinFont(new MinikinFontForTest(fontPath));
                 family->addFont(minikinFont.get(), FontStyle(weight, italic));
             } else {
                 MinikinAutoUnref<MinikinFontForTest>
-                        minikinFont(MinikinFontForTest::createFromFileWithIndex(fontPath,
-                                atoi((const char*)index)));
+                        minikinFont(new MinikinFontForTest(fontPath, atoi((const char*)index)));
                 family->addFont(minikinFont.get(), FontStyle(weight, italic));
             }
         }

tests/util/MinikinFontForTest.cpp

@@ -18,31 +18,31 @@
 
 #include <minikin/MinikinFont.h>
 
-#include <SkTypeface.h>
-
 #include <cutils/log.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
 
 namespace minikin {
 
-// static
-MinikinFontForTest* MinikinFontForTest::createFromFile(const std::string& font_path) {
-    sk_sp<SkTypeface> typeface = SkTypeface::MakeFromFile(font_path.c_str());
-    MinikinFontForTest* font = new MinikinFontForTest(font_path, std::move(typeface));
-    return font;
-}
-
-// static
-MinikinFontForTest* MinikinFontForTest::createFromFileWithIndex(const std::string& font_path,
-        int index) {
-    sk_sp<SkTypeface> typeface = SkTypeface::MakeFromFile(font_path.c_str(), index);
-    MinikinFontForTest* font = new MinikinFontForTest(font_path, std::move(typeface));
-    return font;
+static int uniqueId = 0;  // TODO: make thread safe if necessary.
+
+MinikinFontForTest::MinikinFontForTest(const std::string& font_path, int index) :
+        MinikinFont(uniqueId++),
+        mFontPath(font_path),
+        mFontIndex(index) {
+    int fd = open(font_path.c_str(), O_RDONLY);
+    LOG_ALWAYS_FATAL_IF(fd == -1);
+    struct stat st = {};
+    LOG_ALWAYS_FATAL_IF(fstat(fd, &st) != 0);
+    mFontSize = st.st_size;
+    mFontData = mmap(NULL, mFontSize, PROT_READ, MAP_SHARED, fd, 0);
+    LOG_ALWAYS_FATAL_IF(mFontData == nullptr);
+    close(fd);
 }
 
-MinikinFontForTest::MinikinFontForTest(const std::string& font_path, sk_sp<SkTypeface> typeface) :
-        MinikinFont(typeface->uniqueID()),
-        mTypeface(std::move(typeface)),
-        mFontPath(font_path) {
+MinikinFontForTest::~MinikinFontForTest() {
+    munmap(mFontData, mFontSize);
 }
 
 float MinikinFontForTest::GetHorizontalAdvance(uint32_t /* glyph_id */,
@@ -56,20 +56,4 @@ void MinikinFontForTest::GetBounds(MinikinRect* /* bounds */, uint32_t /* glyph_
     LOG_ALWAYS_FATAL("MinikinFontForTest::GetBounds is not yet implemented");
 }
 
-const void* MinikinFontForTest::GetTable(uint32_t tag, size_t* size,
-        MinikinDestroyFunc* destroy) {
-    const size_t tableSize = mTypeface->getTableSize(tag);
-    *size = tableSize;
-    if (tableSize == 0) {
-        return nullptr;
-    }
-    void* buf = malloc(tableSize);
-    if (buf == nullptr) {
-        return nullptr;
-    }
-    mTypeface->getTableData(tag, 0, tableSize, buf);
-    *destroy = free;
-    return buf;
-}
-
 }  // namespace minikin

tests/util/MinikinFontForTest.h

@@ -18,7 +18,6 @@
 #define MINIKIN_TEST_MINIKIN_FONT_FOR_TEST_H
 
 #include <minikin/MinikinFont.h>
-#include <SkRefCnt.h>
 
 class SkTypeface;
 
@@ -26,27 +25,28 @@ namespace minikin {
 
 class MinikinFontForTest : public MinikinFont {
 public:
-    MinikinFontForTest(const std::string& font_path, sk_sp<SkTypeface> typeface);
-
-    // Helper function for creating MinikinFontForTest instance from font file.
-    // Calller need to unref returned object.
-    static MinikinFontForTest* createFromFile(const std::string& font_path);
-    static MinikinFontForTest* createFromFileWithIndex(const std::string& font_path, int index);
+    MinikinFontForTest(const std::string& font_path, int index);
+    MinikinFontForTest(const std::string& font_path) : MinikinFontForTest(font_path, 0) {}
+    virtual ~MinikinFontForTest();
 
     // MinikinFont overrides.
     float GetHorizontalAdvance(uint32_t glyph_id, const MinikinPaint &paint) const;
     void GetBounds(MinikinRect* bounds, uint32_t glyph_id,
             const MinikinPaint& paint) const;
-    const void* GetTable(uint32_t tag, size_t* size, MinikinDestroyFunc* destroy);
 
     const std::string& fontPath() const { return mFontPath; }
+    const void* GetFontData() const { return mFontData; }
+    size_t GetFontSize() const { return mFontSize; }
+    int GetFontIndex() const { return mFontIndex; }
 private:
     MinikinFontForTest() = delete;
     MinikinFontForTest(const MinikinFontForTest&) = delete;
     MinikinFontForTest& operator=(MinikinFontForTest&) = delete;
 
-    sk_sp<SkTypeface> mTypeface;
     const std::string mFontPath;
+    const int mFontIndex;
+    void* mFontData;
+    size_t mFontSize;
 };
 
 }  // namespace minikin