Improve Paint.hasGlyph performance by caching hb_font_t

It turned out that hb_font_t creation is not a lightweight operation.
Especially, Paint.hasGlyph creates hb_font_t for all existing fonts
every time. To improve the performance, cache hb_font_t instead
of hb_face_t.

Note that to calculate horizontal advance, MinikinPaint needs to be
associated with hb_font_t by calling hb_font_set_funcs. With this patch,
hb_font_set_funcs may be called multiple times for the same hb_font_t
object. However this is not an issue since MinikinPaint is unique
during layout.

Bug: 26784699

Change-Id: I516498ae9f0127d700fc9829327e9789845a1416

6 files changed, 54 insertions, 75 deletions

diff --git a/libs/minikin/Android.mk b/libs/minikin/Android.mk
index 769b5fc..1fd0e6e 100644
--- a/libs/minikin/Android.mk
+++ b/libs/minikin/Android.mk
@@ -24,7 +24,7 @@ minikin_src_files := \
     FontLanguage.cpp \
     FontLanguageListCache.cpp \
     GraphemeBreak.cpp \
-    HbFaceCache.cpp \
+    HbFontCache.cpp \
     Hyphenator.cpp \
     Layout.cpp \
     LayoutUtils.cpp \
diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp
index da58fa3..26aefe4 100644
--- a/libs/minikin/FontCollection.cpp
+++ b/libs/minikin/FontCollection.cpp
@@ -410,11 +410,4 @@ uint32_t FontCollection::getId() const {
     return mId;
 }
 
-void FontCollection::purgeFontFamilyHbFontCache() const {
-    assertMinikinLocked();
-    for (size_t i = 0; i < mFamilies.size(); ++i) {
-        mFamilies[i]->purgeHbFontCache();
-    }
-}
-
 }  // namespace android
diff --git a/libs/minikin/FontFamily.cpp b/libs/minikin/FontFamily.cpp
index 1405789..9eda3c2 100644
--- a/libs/minikin/FontFamily.cpp
+++ b/libs/minikin/FontFamily.cpp
@@ -28,7 +28,7 @@
 
 #include "FontLanguage.h"
 #include "FontLanguageListCache.h"
-#include "HbFaceCache.h"
+#include "HbFontCache.h"
 #include "MinikinInternal.h"
 #include <minikin/MinikinFont.h>
 #include <minikin/AnalyzeStyle.h>
@@ -189,23 +189,11 @@ const SparseBitSet* FontFamily::getCoverage() {
 
 bool FontFamily::hasVariationSelector(uint32_t codepoint, uint32_t variationSelector) {
     assertMinikinLocked();
-    if (!mHbFont) {
-        const FontStyle defaultStyle;
-        MinikinFont* minikinFont = getClosestMatch(defaultStyle).font;
-        hb_face_t* face = getHbFaceLocked(minikinFont);
-        mHbFont = hb_font_create(face);
-        hb_ot_font_set_funcs(mHbFont);
-    }
+    const FontStyle defaultStyle;
+    MinikinFont* minikinFont = getClosestMatch(defaultStyle).font;
+    hb_font_t* font = getHbFontLocked(minikinFont);
     uint32_t unusedGlyph;
-    return hb_font_get_glyph(mHbFont, codepoint, variationSelector, &unusedGlyph);
-}
-
-void FontFamily::purgeHbFontCache() {
-    assertMinikinLocked();
-    if (mHbFont) {
-        hb_font_destroy(mHbFont);
-        mHbFont = nullptr;
-    }
+    return hb_font_get_glyph(font, codepoint, variationSelector, &unusedGlyph);
 }
 
 }  // namespace android
diff --git a/libs/minikin/HbFaceCache.cpp b/libs/minikin/HbFontCache.cpp
index 235f7f1..73308ef 100644
--- a/libs/minikin/HbFaceCache.cpp
+++ b/libs/minikin/HbFontCache.cpp
@@ -16,10 +16,11 @@
 
 #define LOG_TAG "Minikin"
 
-#include "HbFaceCache.h"
+#include "HbFontCache.h"
 
 #include <cutils/log.h>
 #include <hb.h>
+#include <hb-ot.h>
 #include <utils/LruCache.h>
 
 #include <minikin/MinikinFont.h>
@@ -51,23 +52,23 @@ static hb_blob_t* referenceTable(hb_face_t* /* face */, hb_tag_t tag, void* user
             HB_MEMORY_MODE_WRITABLE, buffer, free);
 }
 
-class HbFaceCache : private OnEntryRemoved<int32_t, hb_face_t*> {
+class HbFontCache : private OnEntryRemoved<int32_t, hb_font_t*> {
 public:
-    HbFaceCache() : mCache(kMaxEntries) {
+    HbFontCache() : mCache(kMaxEntries) {
         mCache.setOnEntryRemovedListener(this);
     }
 
     // callback for OnEntryRemoved
-    void operator()(int32_t& /* key */, hb_face_t*& value) {
-        hb_face_destroy(value);
+    void operator()(int32_t& /* key */, hb_font_t*& value) {
+        hb_font_destroy(value);
     }
 
-    hb_face_t* get(int32_t fontId) {
+    hb_font_t* get(int32_t fontId) {
         return mCache.get(fontId);
     }
 
-    void put(int32_t fontId, hb_face_t* face) {
-        mCache.put(fontId, face);
+    void put(int32_t fontId, hb_font_t* font) {
+        mCache.put(fontId, font);
     }
 
     void clear() {
@@ -77,39 +78,52 @@ public:
 private:
     static const size_t kMaxEntries = 100;
 
-    LruCache<int32_t, hb_face_t*> mCache;
+    LruCache<int32_t, hb_font_t*> mCache;
 };
 
-HbFaceCache* getFaceCacheLocked() {
+HbFontCache* getFontCacheLocked() {
     assertMinikinLocked();
-    static HbFaceCache* cache = nullptr;
+    static HbFontCache* cache = nullptr;
     if (cache == nullptr) {
-        cache = new HbFaceCache();
+        cache = new HbFontCache();
     }
     return cache;
 }
 
-void purgeHbFaceCacheLocked() {
+void purgeHbFontCacheLocked() {
     assertMinikinLocked();
-    getFaceCacheLocked()->clear();
+    getFontCacheLocked()->clear();
 }
 
-hb_face_t* getHbFaceLocked(MinikinFont* minikinFont) {
+hb_font_t* getHbFontLocked(MinikinFont* minikinFont) {
     assertMinikinLocked();
+    static hb_font_t* nullFaceFont = nullptr;
     if (minikinFont == nullptr) {
-        return nullptr;
+        if (nullFaceFont == nullptr) {
+            nullFaceFont = hb_font_create(nullptr);
+        }
+        return nullFaceFont;
     }
 
-    HbFaceCache* faceCache = getFaceCacheLocked();
+    HbFontCache* fontCache = getFontCacheLocked();
     const int32_t fontId = minikinFont->GetUniqueId();
-    hb_face_t* face = faceCache->get(fontId);
-    if (face != nullptr) {
-        return face;
+    hb_font_t* font = fontCache->get(fontId);
+    if (font != nullptr) {
+        return font;
     }
 
-    face = hb_face_create_for_tables(referenceTable, minikinFont, nullptr);
-    faceCache->put(fontId, face);
-    return face;
+    hb_face_t* face = hb_face_create_for_tables(referenceTable, minikinFont, nullptr);
+    hb_font_t* parent_font = hb_font_create(face);
+    hb_ot_font_set_funcs(parent_font);
+
+    unsigned int upem = hb_face_get_upem(face);
+    hb_font_set_scale(parent_font, upem, upem);
+
+    font = hb_font_create_sub_font(parent_font);
+    hb_font_destroy(parent_font);
+    hb_face_destroy(face);
+    fontCache->put(fontId, font);
+    return font;
 }
 
 }  // namespace android
diff --git a/libs/minikin/HbFaceCache.h b/libs/minikin/HbFontCache.h
index 8ee38c3..62564d3 100644
--- a/libs/minikin/HbFaceCache.h
+++ b/libs/minikin/HbFontCache.h
@@ -14,16 +14,16 @@
  * limitations under the License.
  */
 
-#ifndef MINIKIN_HBFACE_CACHE_H
-#define MINIKIN_HBFACE_CACHE_H
+#ifndef MINIKIN_HBFONT_CACHE_H
+#define MINIKIN_HBFONT_CACHE_H
 
-struct hb_face_t;
+struct hb_font_t;
 
 namespace android {
 class MinikinFont;
 
-void purgeHbFaceCacheLocked();
-hb_face_t* getHbFaceLocked(MinikinFont* minikinFont);
+void purgeHbFontCacheLocked();
+hb_font_t* getHbFontLocked(MinikinFont* minikinFont);
 
 }  // namespace android
-#endif  // MINIKIN_HBFACE_CACHE_H
+#endif  // MINIKIN_HBFONT_CACHE_H
diff --git a/libs/minikin/Layout.cpp b/libs/minikin/Layout.cpp
index 2839e5b..71e0d89 100644
--- a/libs/minikin/Layout.cpp
+++ b/libs/minikin/Layout.cpp
@@ -37,7 +37,7 @@
 #include "FontLanguage.h"
 #include "FontLanguageListCache.h"
 #include "LayoutUtils.h"
-#include "HbFaceCache.h"
+#include "HbFontCache.h"
 #include "MinikinInternal.h"
 #include <minikin/MinikinFontFreeType.h>
 #include <minikin/Layout.h>
@@ -98,7 +98,7 @@ struct LayoutContext {
 
     void clearHbFonts() {
         for (size_t i = 0; i < hbFonts.size(); i++) {
-            hb_font_destroy(hbFonts[i]);
+            hb_font_set_funcs(hbFonts[i], nullptr, nullptr, nullptr);
         }
         hbFonts.clear();
     }
@@ -133,7 +133,6 @@ public:
         layout->setFontCollection(collection);
         layout->mAdvances.resize(mCount, 0);
         ctx->clearHbFonts();
-        collection->purgeFontFamilyHbFontCache();
         layout->doLayoutRun(mChars, mStart, mCount, mNchars, mIsRtl, ctx);
     }
 
@@ -306,21 +305,6 @@ hb_font_funcs_t* getHbFontFuncs() {
     return hbFontFuncs;
 }
 
-static hb_font_t* create_hb_font(MinikinFont* minikinFont, MinikinPaint* minikinPaint) {
-    hb_face_t* face = getHbFaceLocked(minikinFont);
-    hb_font_t* parent_font = hb_font_create(face);
-    hb_ot_font_set_funcs(parent_font);
-
-    unsigned int upem = hb_face_get_upem(face);
-    hb_font_set_scale(parent_font, upem, upem);
-
-    hb_font_t* font = hb_font_create_sub_font(parent_font);
-    hb_font_destroy(parent_font);
-
-    hb_font_set_funcs(font, getHbFontFuncs(), minikinPaint, 0);
-    return font;
-}
-
 static float HBFixedToFloat(hb_position_t v)
 {
     return scalbnf (v, -8);
@@ -349,7 +333,8 @@ int Layout::findFace(FakedFont face, LayoutContext* ctx) {
     // Note: ctx == NULL means we're copying from the cache, no need to create
     // corresponding hb_font object.
     if (ctx != NULL) {
-        hb_font_t* font = create_hb_font(face.font, &ctx->paint);
+        hb_font_t* font = getHbFontLocked(face.font);
+        hb_font_set_funcs(font, getHbFontFuncs(), &ctx->paint, 0);
         ctx->hbFonts.push_back(font);
     }
     return ix;
@@ -599,7 +584,6 @@ void Layout::doLayout(const uint16_t* buf, size_t start, size_t count, size_t bu
                 start, mCollection, this, NULL);
     }
     ctx.clearHbFonts();
-    mCollection->purgeFontFamilyHbFontCache();
 }
 
 float Layout::measureText(const uint16_t* buf, size_t start, size_t count, size_t bufSize,
@@ -976,7 +960,7 @@ void Layout::purgeCaches() {
     AutoMutex _l(gMinikinLock);
     LayoutCache& layoutCache = LayoutEngine::getInstance().layoutCache;
     layoutCache.clear();
-    purgeHbFaceCacheLocked();
+    purgeHbFontCacheLocked();
 }
 
 }  // namespace android