Revert "Cache hb_face."
This reverts commit d19bb17782d09e7fe2f7ea85eabf7e649e6becc1.
Reason for revert: windows build failure?
lld-link: error: undefined symbol: private: void __cdecl SkSemaphore::osSignal(int)
Original change's description:
> Cache hb_face.
>
> Creating an hb_face can be quite expensive, cache them.
>
> This implementation is similar to the super simple caching strategy used
> by libtxt. It uses a simple global LRU cache from SkFontID to hb_hbface
> of size 100.
>
> Change-Id: I364a4548699cece50073e829a065c0a303245873
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/289442
> Commit-Queue: Ben Wagner <bungeman@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
TBR=bungeman@google.com,reed@google.com
Change-Id: I31967a638fb497f28ca3d3f26ef3692dddff004d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/290718
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
diff --git a/modules/skshaper/src/SkShaper_harfbuzz.cpp b/modules/skshaper/src/SkShaper_harfbuzz.cpp
index d2e3bd4..29d9fe9 100644
--- a/modules/skshaper/src/SkShaper_harfbuzz.cpp
+++ b/modules/skshaper/src/SkShaper_harfbuzz.cpp
@@ -20,13 +20,11 @@
#include "include/core/SkTypes.h"
#include "include/private/SkBitmaskEnum.h"
#include "include/private/SkMalloc.h"
-#include "include/private/SkMutex.h"
#include "include/private/SkTArray.h"
#include "include/private/SkTFitsIn.h"
#include "include/private/SkTemplates.h"
#include "include/private/SkTo.h"
#include "modules/skshaper/include/SkShaper.h"
-#include "src/core/SkLRUCache.h"
#include "src/core/SkSpan.h"
#include "src/core/SkTDPQueue.h"
#include "src/utils/SkUTF.h"
@@ -75,6 +73,25 @@
using ICUBrk = resource<UBreakIterator, decltype(ubrk_close) , ubrk_close >;
using ICUUText = resource<UText , decltype(utext_close) , utext_close >;
+HBBlob stream_to_blob(std::unique_ptr<SkStreamAsset> asset) {
+ size_t size = asset->getLength();
+ HBBlob blob;
+ if (const void* base = asset->getMemoryBase()) {
+ blob.reset(hb_blob_create((char*)base, SkToUInt(size),
+ HB_MEMORY_MODE_READONLY, asset.release(),
+ [](void* p) { delete (SkStreamAsset*)p; }));
+ } else {
+ // SkDebugf("Extra SkStreamAsset copy\n");
+ void* ptr = size ? sk_malloc_throw(size) : nullptr;
+ asset->read(ptr, size);
+ blob.reset(hb_blob_create((char*)ptr, SkToUInt(size),
+ HB_MEMORY_MODE_READONLY, ptr, sk_free));
+ }
+ SkASSERT(blob);
+ hb_blob_make_immutable(blob.get());
+ return blob;
+}
+
hb_position_t skhb_position(SkScalar value) {
// Treat HarfBuzz hb_position_t as 16.16 fixed-point.
constexpr int kHbPosition1 = 1 << 16;
@@ -249,65 +266,31 @@
}
SkData* rawData = data.release();
return hb_blob_create(reinterpret_cast<char*>(rawData->writable_data()), rawData->size(),
- HB_MEMORY_MODE_READONLY, rawData, [](void* ctx) {
+ HB_MEMORY_MODE_WRITABLE, rawData, [](void* ctx) {
SkSafeUnref(((SkData*)ctx));
});
}
-HBBlob stream_to_blob(std::unique_ptr<SkStreamAsset> asset) {
- size_t size = asset->getLength();
- HBBlob blob;
- if (const void* base = asset->getMemoryBase()) {
- blob.reset(hb_blob_create((char*)base, SkToUInt(size),
- HB_MEMORY_MODE_READONLY, asset.release(),
- [](void* p) { delete (SkStreamAsset*)p; }));
- } else {
- // SkDebugf("Extra SkStreamAsset copy\n");
- void* ptr = size ? sk_malloc_throw(size) : nullptr;
- asset->read(ptr, size);
- blob.reset(hb_blob_create((char*)ptr, SkToUInt(size),
- HB_MEMORY_MODE_READONLY, ptr, sk_free));
- }
- SkASSERT(blob);
- hb_blob_make_immutable(blob.get());
- return blob;
-}
-
-SkDEBUGCODE(static hb_user_data_key_t gDataIdKey;)
-
-HBFace create_hb_face(const SkTypeface& typeface) {
+HBFont create_hb_font(const SkFont& font) {
+ SkASSERT(font.getTypeface());
int index;
- std::unique_ptr<SkStreamAsset> typefaceAsset = typeface.openStream(&index);
+ std::unique_ptr<SkStreamAsset> typefaceAsset = font.getTypeface()->openStream(&index);
HBFace face;
- if (typefaceAsset && typefaceAsset->getMemoryBase()) {
- HBBlob blob(stream_to_blob(std::move(typefaceAsset)));
- face.reset(hb_face_create(blob.get(), (unsigned)index));
- } else {
+ if (!typefaceAsset) {
face.reset(hb_face_create_for_tables(
skhb_get_table,
- const_cast<SkTypeface*>(SkRef(&typeface)),
+ reinterpret_cast<void *>(font.refTypeface().release()),
[](void* user_data){ SkSafeUnref(reinterpret_cast<SkTypeface*>(user_data)); }));
+ } else {
+ HBBlob blob(stream_to_blob(std::move(typefaceAsset)));
+ face.reset(hb_face_create(blob.get(), (unsigned)index));
}
SkASSERT(face);
if (!face) {
return nullptr;
}
hb_face_set_index(face.get(), (unsigned)index);
- hb_face_set_upem(face.get(), typeface.getUnitsPerEm());
-
- SkDEBUGCODE(
- hb_face_set_user_data(face.get(), &gDataIdKey, const_cast<SkTypeface*>(&typeface),
- nullptr, false);
- )
-
- return face;
-}
-
-HBFont create_hb_font(const SkFont& font, const HBFace& face) {
- SkDEBUGCODE(
- void* dataId = hb_face_get_user_data(face.get(), &gDataIdKey);
- SkASSERT(dataId == font.getTypeface());
- )
+ hb_face_set_upem(face.get(), font.getTypeface()->getUnitsPerEm());
HBFont otFont(hb_font_create(face.get()));
SkASSERT(otFont);
@@ -1335,24 +1318,8 @@
hb_buffer_set_language(buffer, hb_language_from_string(language.currentLanguage(), -1));
hb_buffer_guess_segment_properties(buffer);
- // TODO: better cache HBFace (data) / hbfont (typeface)
- // An HBFace is expensive (it sanitizes the bits).
- // An HBFont is fairly inexpensive.
- // An HBFace is actually tied to the data, not the typeface.
- // The size of 100 here is completely arbitrary and used to match libtxt.
- static SkLRUCache<SkFontID, HBFace> gHBFaceCache(100);
- static SkMutex gHBFaceCacheMutex;
- HBFont hbFont;
- {
- SkAutoMutexExclusive lock(gHBFaceCacheMutex);
- SkFontID dataId = font.currentFont().getTypeface()->uniqueID();
- HBFace* hbFaceCached = gHBFaceCache.find(dataId);
- if (!hbFaceCached) {
- HBFace hbFace(create_hb_face(*font.currentFont().getTypeface()));
- hbFaceCached = gHBFaceCache.insert(dataId, std::move(hbFace));
- }
- hbFont = create_hb_font(font.currentFont(), *hbFaceCached);
- }
+ // TODO: how to cache hbface (typeface) / hbfont (font)
+ HBFont hbFont(create_hb_font(font.currentFont()));
if (!hbFont) {
return run;
}