Merge "Make some thread-unsafe functions thread-safe in Layout.cpp"

2 files changed, 101 insertions, 102 deletions

diff --git a/include/minikin/HbUtils.h b/include/minikin/HbUtils.h
index 1cf375d..68e4ab8 100644
--- a/include/minikin/HbUtils.h
+++ b/include/minikin/HbUtils.h
@@ -33,9 +33,15 @@ struct HbFaceDeleter {
 struct HbFontDeleter {
     void operator()(hb_font_t* v) { hb_font_destroy(v); }
 };
+
+struct HbBufferDeleter {
+    void operator()(hb_buffer_t* v) { hb_buffer_destroy(v); }
+};
+
 using HbBlobUniquePtr = std::unique_ptr<hb_blob_t, HbBlobDeleter>;
 using HbFaceUniquePtr = std::unique_ptr<hb_face_t, HbFaceDeleter>;
 using HbFontUniquePtr = std::unique_ptr<hb_font_t, HbFontDeleter>;
+using HbBufferUniquePtr = std::unique_ptr<hb_buffer_t, HbBufferDeleter>;
 
 }  // namespace minikin
 
diff --git a/libs/minikin/Layout.cpp b/libs/minikin/Layout.cpp
index e4e6010..f5fe89f 100644
--- a/libs/minikin/Layout.cpp
+++ b/libs/minikin/Layout.cpp
@@ -30,7 +30,6 @@
 #include <unicode/utf16.h>
 #include <utils/JenkinsHash.h>
 #include <utils/LruCache.h>
-#include <utils/Singleton.h>
 
 #include "minikin/Emoji.h"
 #include "minikin/HbUtils.h"
@@ -49,6 +48,70 @@ struct SkiaArguments {
     FontFakery fakery;
 };
 
+static hb_position_t harfbuzzGetGlyphHorizontalAdvance(hb_font_t* /* hbFont */, void* fontData,
+                                                       hb_codepoint_t glyph, void* /* userData */) {
+    SkiaArguments* args = reinterpret_cast<SkiaArguments*>(fontData);
+    float advance = args->font->GetHorizontalAdvance(glyph, *args->paint, args->fakery);
+    return 256 * advance + 0.5;
+}
+
+static hb_bool_t harfbuzzGetGlyphHorizontalOrigin(hb_font_t* /* hbFont */, void* /* fontData */,
+                                                  hb_codepoint_t /* glyph */,
+                                                  hb_position_t* /* x */, hb_position_t* /* y */,
+                                                  void* /* userData */) {
+    // Just return true, following the way that Harfbuzz-FreeType implementation does.
+    return true;
+}
+
+// TODO: Initialize in Zygote if it helps.
+hb_unicode_funcs_t* getUnicodeFunctions() {
+    static hb_unicode_funcs_t* unicodeFunctions = nullptr;
+    static std::once_flag once;
+    std::call_once(once, [&]() {
+        unicodeFunctions = hb_unicode_funcs_create(hb_icu_get_unicode_funcs());
+        /* Disable the function used for compatibility decomposition */
+        hb_unicode_funcs_set_decompose_compatibility_func(
+                unicodeFunctions,
+                [](hb_unicode_funcs_t*, hb_codepoint_t, hb_codepoint_t*, void*) -> unsigned int {
+                    return 0;
+                },
+                nullptr, nullptr);
+    });
+    return unicodeFunctions;
+}
+
+// TODO: Initialize in Zygote if it helps.
+hb_font_funcs_t* getFontFuncs() {
+    static hb_font_funcs_t* fontFuncs = nullptr;
+    static std::once_flag once;
+    std::call_once(once, [&]() {
+        fontFuncs = hb_font_funcs_create();
+        // Override the h_advance function since we can't use HarfBuzz's implemenation. It may
+        // return the wrong value if the font uses hinting aggressively.
+        hb_font_funcs_set_glyph_h_advance_func(fontFuncs, harfbuzzGetGlyphHorizontalAdvance, 0, 0);
+        hb_font_funcs_set_glyph_h_origin_func(fontFuncs, harfbuzzGetGlyphHorizontalOrigin, 0, 0);
+        hb_font_funcs_make_immutable(fontFuncs);
+    });
+    return fontFuncs;
+}
+
+// TODO: Initialize in Zygote if it helps.
+hb_font_funcs_t* getFontFuncsForEmoji() {
+    static hb_font_funcs_t* fontFuncs = nullptr;
+    static std::once_flag once;
+    std::call_once(once, [&]() {
+        fontFuncs = hb_font_funcs_create();
+        // Don't override the h_advance function since we use HarfBuzz's implementation for emoji
+        // for performance reasons.
+        // Note that it is technically possible for a TrueType font to have outline and embedded
+        // bitmap at the same time. We ignore modified advances of hinted outline glyphs in that
+        // case.
+        hb_font_funcs_set_glyph_h_origin_func(fontFuncs, harfbuzzGetGlyphHorizontalOrigin, 0, 0);
+        hb_font_funcs_make_immutable(fontFuncs);
+    });
+    return fontFuncs;
+}
+
 }  // namespace
 
 static inline UBiDiLevel bidiToUBidiLevel(Bidi bidi) {
@@ -174,6 +237,11 @@ public:
         dprintf(fd, "  Hit ratio: %d/%d (%f)\n", mCacheHitCount, mRequestCount, ratio);
     }
 
+    static LayoutCache& getInstance() {
+        static LayoutCache cache;
+        return cache;
+    }
+
 private:
     // callback for OnEntryRemoved
     void operator()(LayoutCacheKey& key, Layout*& value) {
@@ -193,27 +261,6 @@ private:
     static const size_t kMaxEntries = 5000;
 };
 
-static unsigned int disabledDecomposeCompatibility(hb_unicode_funcs_t*, hb_codepoint_t,
-                                                   hb_codepoint_t*, void*) {
-    return 0;
-}
-
-class LayoutEngine : public ::android::Singleton<LayoutEngine> {
-public:
-    LayoutEngine() {
-        unicodeFunctions = hb_unicode_funcs_create(hb_icu_get_unicode_funcs());
-        /* Disable the function used for compatibility decomposition */
-        hb_unicode_funcs_set_decompose_compatibility_func(
-                unicodeFunctions, disabledDecomposeCompatibility, NULL, NULL);
-        hbBuffer = hb_buffer_create();
-        hb_buffer_set_unicode_funcs(hbBuffer, unicodeFunctions);
-    }
-
-    hb_buffer_t* hbBuffer;
-    hb_unicode_funcs_t* unicodeFunctions;
-    LayoutCache layoutCache;
-};
-
 bool LayoutCacheKey::operator==(const LayoutCacheKey& other) const {
     return mId == other.mId && mStart == other.mStart && mCount == other.mCount &&
            mStyle == other.mStyle && mSize == other.mSize && mScaleX == other.mScaleX &&
@@ -268,48 +315,6 @@ void Layout::reset() {
     mAdvance = 0;
 }
 
-static hb_position_t harfbuzzGetGlyphHorizontalAdvance(hb_font_t* /* hbFont */, void* fontData,
-                                                       hb_codepoint_t glyph, void* /* userData */) {
-    SkiaArguments* args = reinterpret_cast<SkiaArguments*>(fontData);
-    float advance = args->font->GetHorizontalAdvance(glyph, *args->paint, args->fakery);
-    return 256 * advance + 0.5;
-}
-
-static hb_bool_t harfbuzzGetGlyphHorizontalOrigin(hb_font_t* /* hbFont */, void* /* fontData */,
-                                                  hb_codepoint_t /* glyph */,
-                                                  hb_position_t* /* x */, hb_position_t* /* y */,
-                                                  void* /* userData */) {
-    // Just return true, following the way that Harfbuzz-FreeType
-    // implementation does.
-    return true;
-}
-
-hb_font_funcs_t* getHbFontFuncs(bool forColorBitmapFont) {
-    assertMinikinLocked();
-
-    static hb_font_funcs_t* hbFuncs = nullptr;
-    static hb_font_funcs_t* hbFuncsForColorBitmap = nullptr;
-
-    hb_font_funcs_t** funcs = forColorBitmapFont ? &hbFuncs : &hbFuncsForColorBitmap;
-    if (*funcs == nullptr) {
-        *funcs = hb_font_funcs_create();
-        if (forColorBitmapFont) {
-            // Don't override the h_advance function since we use HarfBuzz's implementation for
-            // emoji for performance reasons.
-            // Note that it is technically possible for a TrueType font to have outline and embedded
-            // bitmap at the same time. We ignore modified advances of hinted outline glyphs in that
-            // case.
-        } else {
-            // Override the h_advance function since we can't use HarfBuzz's implemenation. It may
-            // return the wrong value if the font uses hinting aggressively.
-            hb_font_funcs_set_glyph_h_advance_func(*funcs, harfbuzzGetGlyphHorizontalAdvance, 0, 0);
-        }
-        hb_font_funcs_set_glyph_h_origin_func(*funcs, harfbuzzGetGlyphHorizontalOrigin, 0, 0);
-        hb_font_funcs_make_immutable(*funcs);
-    }
-    return *funcs;
-}
-
 static bool isColorBitmapFont(const HbFontUniquePtr& font) {
     HbBlob cbdt(font, HB_TAG('C', 'B', 'D', 'T'));
     return cbdt;
@@ -346,7 +351,7 @@ int Layout::findOrPushBackFace(const FakedFont& face, LayoutContext* ctx) {
         // Create new hb_font_t from base font.
         HbFontUniquePtr font(hb_font_create_sub_font(face.font->baseFont().get()));
         hb_font_set_funcs(
-                font.get(), getHbFontFuncs(isColorBitmapFont(font)),
+                font.get(), isColorBitmapFont(font) ? getFontFuncsForEmoji() : getFontFuncs(),
                 new SkiaArguments({face.font->typeface().get(), &ctx->paint, face.fakery}),
                 [](void* data) { delete reinterpret_cast<SkiaArguments*>(data); });
         ctx->hbFonts.push_back(std::move(font));
@@ -354,14 +359,6 @@ int Layout::findOrPushBackFace(const FakedFont& face, LayoutContext* ctx) {
     return ix;
 }
 
-static hb_script_t codePointToScript(hb_codepoint_t codepoint) {
-    static hb_unicode_funcs_t* u = 0;
-    if (!u) {
-        u = LayoutEngine::getInstance().unicodeFunctions;
-    }
-    return hb_unicode_script(u, codepoint);
-}
-
 static hb_codepoint_t decodeUtf16(const uint16_t* chars, size_t len, ssize_t* iter) {
     UChar32 result;
     U16_NEXT(chars, *iter, (ssize_t)len, result);
@@ -376,12 +373,12 @@ static hb_script_t getScriptRun(const uint16_t* chars, size_t len, ssize_t* iter
         return HB_SCRIPT_UNKNOWN;
     }
     uint32_t cp = decodeUtf16(chars, len, iter);
-    hb_script_t current_script = codePointToScript(cp);
+    hb_script_t current_script = hb_unicode_script(getUnicodeFunctions(), cp);
     for (;;) {
         if (size_t(*iter) == len) break;
         const ssize_t prev_iter = *iter;
         cp = decodeUtf16(chars, len, iter);
-        const hb_script_t script = codePointToScript(cp);
+        const hb_script_t script = hb_unicode_script(getUnicodeFunctions(), cp);
         if (script != current_script) {
             if (current_script == HB_SCRIPT_INHERITED || current_script == HB_SCRIPT_COMMON) {
                 current_script = script;
@@ -682,7 +679,7 @@ float Layout::doLayoutWord(const uint16_t* buf, size_t start, size_t count, size
                            bool isRtl, LayoutContext* ctx, size_t bufStart,
                            StartHyphenEdit startHyphen, EndHyphenEdit endHyphen, Layout* layout,
                            float* advances, MinikinExtent* extents, LayoutPieces* lpOut) {
-    LayoutCache& cache = LayoutEngine::getInstance().layoutCache;
+    LayoutCache& cache = LayoutCache::getInstance();
     LayoutCacheKey key(ctx->paint, buf, start, count, bufSize, isRtl, startHyphen, endHyphen);
 
     float wordSpacing = count == 1 && isWordSpace(buf[start]) ? ctx->paint.wordSpacing : 0;
@@ -769,22 +766,23 @@ static inline hb_codepoint_t determineHyphenChar(hb_codepoint_t preferredHyphen,
 }
 
 template <typename HyphenEdit>
-static inline void addHyphenToHbBuffer(hb_buffer_t* buffer, const HbFontUniquePtr& font,
+static inline void addHyphenToHbBuffer(const HbBufferUniquePtr& buffer, const HbFontUniquePtr& font,
                                        HyphenEdit hyphen, uint32_t cluster) {
     const uint32_t* chars;
     size_t size;
     std::tie(chars, size) = getHyphenString(hyphen);
     for (size_t i = 0; i < size; i++) {
-        hb_buffer_add(buffer, determineHyphenChar(chars[i], font.get()), cluster);
+        hb_buffer_add(buffer.get(), determineHyphenChar(chars[i], font.get()), cluster);
     }
 }
 
 // Returns the cluster value assigned to the first codepoint added to the buffer, which can be used
 // to translate cluster values returned by HarfBuzz to input indices.
-static inline uint32_t addToHbBuffer(hb_buffer_t* buffer, const uint16_t* buf, size_t start,
-                                     size_t count, size_t bufSize, ssize_t scriptRunStart,
-                                     ssize_t scriptRunEnd, StartHyphenEdit inStartHyphen,
-                                     EndHyphenEdit inEndHyphen, const HbFontUniquePtr& hbFont) {
+static inline uint32_t addToHbBuffer(const HbBufferUniquePtr& buffer, const uint16_t* buf,
+                                     size_t start, size_t count, size_t bufSize,
+                                     ssize_t scriptRunStart, ssize_t scriptRunEnd,
+                                     StartHyphenEdit inStartHyphen, EndHyphenEdit inEndHyphen,
+                                     const HbFontUniquePtr& hbFont) {
     // Only hyphenate the very first script run for starting hyphens.
     const StartHyphenEdit startHyphen =
             (scriptRunStart == 0) ? inStartHyphen : StartHyphenEdit::NO_EDIT;
@@ -845,10 +843,10 @@ static inline uint32_t addToHbBuffer(hb_buffer_t* buffer, const uint16_t* buf, s
         hbTextLength = hbItemOffset + hbItemLength;
     }
 
-    hb_buffer_add_utf16(buffer, hbText, hbTextLength, hbItemOffset, hbItemLength);
+    hb_buffer_add_utf16(buffer.get(), hbText, hbTextLength, hbItemOffset, hbItemLength);
 
     unsigned int numCodepoints;
-    hb_glyph_info_t* cpInfo = hb_buffer_get_glyph_infos(buffer, &numCodepoints);
+    hb_glyph_info_t* cpInfo = hb_buffer_get_glyph_infos(buffer.get(), &numCodepoints);
 
     // Add the hyphen at the end, if there's any.
     if (hasEndInsertion || hasEndReplacement) {
@@ -872,7 +870,7 @@ static inline uint32_t addToHbBuffer(hb_buffer_t* buffer, const uint16_t* buf, s
         addHyphenToHbBuffer(buffer, hbFont, endHyphen, hyphenCluster);
         // Since we have just added to the buffer, cpInfo no longer necessarily points to
         // the right place. Refresh it.
-        cpInfo = hb_buffer_get_glyph_infos(buffer, nullptr /* we don't need the size */);
+        cpInfo = hb_buffer_get_glyph_infos(buffer.get(), nullptr /* we don't need the size */);
     }
     return cpInfo[0].cluster;
 }
@@ -880,7 +878,8 @@ static inline uint32_t addToHbBuffer(hb_buffer_t* buffer, const uint16_t* buf, s
 void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t bufSize,
                          bool isRtl, LayoutContext* ctx, StartHyphenEdit startHyphen,
                          EndHyphenEdit endHyphen) {
-    hb_buffer_t* buffer = LayoutEngine::getInstance().hbBuffer;
+    HbBufferUniquePtr buffer(hb_buffer_create());
+    hb_buffer_set_unicode_funcs(buffer.get(), getUnicodeFunctions());
     std::vector<FontCollection::Run> items;
     ctx->paint.font->itemize(buf + start, count, ctx->paint, &items);
 
@@ -953,9 +952,9 @@ void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t
                 letterSpaceHalfRight = letterSpace - letterSpaceHalfLeft;
             }
 
-            hb_buffer_clear_contents(buffer);
-            hb_buffer_set_script(buffer, script);
-            hb_buffer_set_direction(buffer, isRtl ? HB_DIRECTION_RTL : HB_DIRECTION_LTR);
+            hb_buffer_clear_contents(buffer.get());
+            hb_buffer_set_script(buffer.get(), script);
+            hb_buffer_set_direction(buffer.get(), isRtl ? HB_DIRECTION_RTL : HB_DIRECTION_LTR);
             const LocaleList& localeList = LocaleListCache::getById(ctx->paint.localeListId);
             if (localeList.size() != 0) {
                 hb_language_t hbLanguage = localeList.getHbLanguage(0);
@@ -965,17 +964,18 @@ void Layout::doLayoutRun(const uint16_t* buf, size_t start, size_t count, size_t
                         break;
                     }
                 }
-                hb_buffer_set_language(buffer, hbLanguage);
+                hb_buffer_set_language(buffer.get(), hbLanguage);
             }
 
             const uint32_t clusterStart =
                     addToHbBuffer(buffer, buf, start, count, bufSize, scriptRunStart, scriptRunEnd,
                                   startHyphen, endHyphen, hbFont);
 
-            hb_shape(hbFont.get(), buffer, features.empty() ? NULL : &features[0], features.size());
+            hb_shape(hbFont.get(), buffer.get(), features.empty() ? NULL : &features[0],
+                     features.size());
             unsigned int numGlyphs;
-            hb_glyph_info_t* info = hb_buffer_get_glyph_infos(buffer, &numGlyphs);
-            hb_glyph_position_t* positions = hb_buffer_get_glyph_positions(buffer, NULL);
+            hb_glyph_info_t* info = hb_buffer_get_glyph_infos(buffer.get(), &numGlyphs);
+            hb_glyph_position_t* positions = hb_buffer_get_glyph_positions(buffer.get(), NULL);
 
             // At this point in the code, the cluster values in the info buffer correspond to the
             // input characters with some shift. The cluster value clusterStart corresponds to the
@@ -1130,19 +1130,12 @@ void Layout::getBounds(MinikinRect* bounds) const {
 
 void Layout::purgeCaches() {
     android::AutoMutex _l(gMinikinLock);
-    LayoutCache& layoutCache = LayoutEngine::getInstance().layoutCache;
-    layoutCache.clear();
+    LayoutCache::getInstance().clear();
 }
 
 void Layout::dumpMinikinStats(int fd) {
     android::AutoMutex _l(gMinikinLock);
-    LayoutEngine::getInstance().layoutCache.dumpStats(fd);
+    LayoutCache::getInstance().dumpStats(fd);
 }
 
 }  // namespace minikin
-
-// Unable to define the static data member outside of android.
-// TODO: introduce our own Singleton to drop android namespace.
-namespace android {
-ANDROID_SINGLETON_STATIC_INSTANCE(minikin::LayoutEngine);
-}  // namespace android