Skip to content

Commit

Permalink
Revert "Cache hb_face."
Browse files Browse the repository at this point in the history
This reverts commit d19bb17.

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>
  • Loading branch information
reed-at-google authored and Skia Commit-Bot committed May 19, 2020
1 parent 98bc22c commit 7a4ea2b
Showing 1 changed file with 31 additions and 64 deletions.
95 changes: 31 additions & 64 deletions modules/skshaper/src/SkShaper_harfbuzz.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -75,6 +73,25 @@ using ICUBiDi = resource<UBiDi , decltype(ubidi_close) , ubidi_clo
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;
Expand Down Expand Up @@ -249,65 +266,31 @@ hb_blob_t* skhb_get_table(hb_face_t* face, hb_tag_t tag, void* user_data) {
}
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);
Expand Down Expand Up @@ -1335,24 +1318,8 @@ ShapedRun ShaperHarfBuzz::shape(char const * const utf8,
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;
}
Expand Down

0 comments on commit 7a4ea2b

Please sign in to comment.