- Revision
- 228525
- Author
- [email protected]
- Date
- 2018-02-15 11:06:05 -0800 (Thu, 15 Feb 2018)
Log Message
HarfBuzzFace: rework cache entry reference holding
https://bugs.webkit.org/show_bug.cgi?id=182828
Reviewed by Michael Catanzaro.
Move the FaceCacheEntry and HarfBuzzFaceCache types into the
HarfBuzzFace class as CacheEntry and Cache, respectively. The Cache
singleton is also moved there.
In the HarfBuzzFace constructor, we now don't increase the CacheEntry
reference, but instead just keep a reference to that object through
a RefPtr<CacheEntry> object. We don't need to retrieve the hb_face_t
object and the glyph cache HashMap in the constructor anymore, we just
retrieve them when necessary through that CacheEntry reference.
In the destructor, that RefPtr<CacheEntry> object is nulled out before
the object in Cache is removed if that's where the final reference is
kept.
* platform/graphics/harfbuzz/HarfBuzzFace.cpp:
(WebCore::HarfBuzzFace::CacheEntry::CacheEntry):
(WebCore::HarfBuzzFace::CacheEntry::~CacheEntry):
(WebCore::HarfBuzzFace::cache):
(WebCore::HarfBuzzFace::HarfBuzzFace):
(WebCore::HarfBuzzFace::~HarfBuzzFace):
(WebCore::HarfBuzzFace::setScriptForVerticalGlyphSubstitution):
(WebCore::FaceCacheEntry::create): Deleted.
(WebCore::FaceCacheEntry::~FaceCacheEntry): Deleted.
(WebCore::FaceCacheEntry::face): Deleted.
(WebCore::FaceCacheEntry::glyphCache): Deleted.
(WebCore::FaceCacheEntry::FaceCacheEntry): Deleted.
(WebCore::harfBuzzFaceCache): Deleted.
* platform/graphics/harfbuzz/HarfBuzzFace.h:
(WebCore::HarfBuzzFace::CacheEntry::create):
(WebCore::HarfBuzzFace::CacheEntry::face):
(WebCore::HarfBuzzFace::CacheEntry::glyphCache):
* platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp:
(WebCore::harfBuzzGetGlyph):
(WebCore::HarfBuzzFace::createFont):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (228524 => 228525)
--- trunk/Source/WebCore/ChangeLog 2018-02-15 19:04:01 UTC (rev 228524)
+++ trunk/Source/WebCore/ChangeLog 2018-02-15 19:06:05 UTC (rev 228525)
@@ -1,5 +1,47 @@
2018-02-15 Zan Dobersek <[email protected]>
+ HarfBuzzFace: rework cache entry reference holding
+ https://bugs.webkit.org/show_bug.cgi?id=182828
+
+ Reviewed by Michael Catanzaro.
+
+ Move the FaceCacheEntry and HarfBuzzFaceCache types into the
+ HarfBuzzFace class as CacheEntry and Cache, respectively. The Cache
+ singleton is also moved there.
+
+ In the HarfBuzzFace constructor, we now don't increase the CacheEntry
+ reference, but instead just keep a reference to that object through
+ a RefPtr<CacheEntry> object. We don't need to retrieve the hb_face_t
+ object and the glyph cache HashMap in the constructor anymore, we just
+ retrieve them when necessary through that CacheEntry reference.
+
+ In the destructor, that RefPtr<CacheEntry> object is nulled out before
+ the object in Cache is removed if that's where the final reference is
+ kept.
+
+ * platform/graphics/harfbuzz/HarfBuzzFace.cpp:
+ (WebCore::HarfBuzzFace::CacheEntry::CacheEntry):
+ (WebCore::HarfBuzzFace::CacheEntry::~CacheEntry):
+ (WebCore::HarfBuzzFace::cache):
+ (WebCore::HarfBuzzFace::HarfBuzzFace):
+ (WebCore::HarfBuzzFace::~HarfBuzzFace):
+ (WebCore::HarfBuzzFace::setScriptForVerticalGlyphSubstitution):
+ (WebCore::FaceCacheEntry::create): Deleted.
+ (WebCore::FaceCacheEntry::~FaceCacheEntry): Deleted.
+ (WebCore::FaceCacheEntry::face): Deleted.
+ (WebCore::FaceCacheEntry::glyphCache): Deleted.
+ (WebCore::FaceCacheEntry::FaceCacheEntry): Deleted.
+ (WebCore::harfBuzzFaceCache): Deleted.
+ * platform/graphics/harfbuzz/HarfBuzzFace.h:
+ (WebCore::HarfBuzzFace::CacheEntry::create):
+ (WebCore::HarfBuzzFace::CacheEntry::face):
+ (WebCore::HarfBuzzFace::CacheEntry::glyphCache):
+ * platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp:
+ (WebCore::harfBuzzGetGlyph):
+ (WebCore::HarfBuzzFace::createFont):
+
+2018-02-15 Zan Dobersek <[email protected]>
+
FontPlatformData::harfBuzzFace() should return a reference
https://bugs.webkit.org/show_bug.cgi?id=182825
Modified: trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.cpp (228524 => 228525)
--- trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.cpp 2018-02-15 19:04:01 UTC (rev 228524)
+++ trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.cpp 2018-02-15 19:06:05 UTC (rev 228525)
@@ -47,36 +47,21 @@
// to reduce the memory consumption because hb_face_t should be associated with
// underling font data (e.g. CTFontRef, FTFace).
-class FaceCacheEntry : public RefCounted<FaceCacheEntry> {
-public:
- static Ref<FaceCacheEntry> create(hb_face_t* face)
- {
- ASSERT(face);
- return adoptRef(*new FaceCacheEntry(face));
- }
- ~FaceCacheEntry()
- {
- hb_face_destroy(m_face);
- }
+HarfBuzzFace::CacheEntry::CacheEntry(hb_face_t* face)
+ : m_face(face)
+{
+ ASSERT(m_face);
+}
- hb_face_t* face() { return m_face; }
- HashMap<uint32_t, uint16_t>* glyphCache() { return &m_glyphCache; }
+HarfBuzzFace::CacheEntry::~CacheEntry()
+{
+ hb_face_destroy(m_face);
+}
-private:
- explicit FaceCacheEntry(hb_face_t* face)
- : m_face(face)
- { }
-
- hb_face_t* m_face;
- HashMap<uint32_t, uint16_t> m_glyphCache;
-};
-
-typedef HashMap<uint64_t, RefPtr<FaceCacheEntry>, WTF::IntHash<uint64_t>, WTF::UnsignedWithZeroKeyHashTraits<uint64_t> > HarfBuzzFaceCache;
-
-static HarfBuzzFaceCache* harfBuzzFaceCache()
+HarfBuzzFace::Cache& HarfBuzzFace::cache()
{
- static NeverDestroyed<HarfBuzzFaceCache> s_harfBuzzFaceCache;
- return &s_harfBuzzFaceCache.get();
+ static NeverDestroyed<Cache> s_cache;
+ return s_cache;
}
HarfBuzzFace::HarfBuzzFace(FontPlatformData* platformData, uint64_t uniqueID)
@@ -84,22 +69,22 @@
, m_uniqueID(uniqueID)
, m_scriptForVerticalText(HB_SCRIPT_INVALID)
{
- HarfBuzzFaceCache::AddResult result = harfBuzzFaceCache()->add(m_uniqueID, nullptr);
+ auto result = cache().add(m_uniqueID, nullptr);
if (result.isNewEntry)
- result.iterator->value = FaceCacheEntry::create(createFace());
- result.iterator->value->ref();
- m_face = result.iterator->value->face();
- m_glyphCacheForFaceCacheEntry = result.iterator->value->glyphCache();
+ result.iterator->value = CacheEntry::create(createFace());
+ m_cacheEntry = result.iterator->value;
}
HarfBuzzFace::~HarfBuzzFace()
{
- HarfBuzzFaceCache::iterator result = harfBuzzFaceCache()->find(m_uniqueID);
- ASSERT(result != harfBuzzFaceCache()->end());
- ASSERT(result.get()->value->refCount() > 1);
- result.get()->value->deref();
- if (result.get()->value->refCount() == 1)
- harfBuzzFaceCache()->remove(m_uniqueID);
+ auto it = cache().find(m_uniqueID);
+ ASSERT(it != cache().end());
+ ASSERT(it->value == m_cacheEntry);
+ ASSERT(it->value->refCount() > 1);
+
+ m_cacheEntry = nullptr;
+ if (it->value->refCount() == 1)
+ cache().remove(it);
}
static hb_script_t findScriptForVerticalGlyphSubstitution(hb_face_t* face)
@@ -126,7 +111,7 @@
void HarfBuzzFace::setScriptForVerticalGlyphSubstitution(hb_buffer_t* buffer)
{
if (m_scriptForVerticalText == HB_SCRIPT_INVALID)
- m_scriptForVerticalText = findScriptForVerticalGlyphSubstitution(m_face);
+ m_scriptForVerticalText = findScriptForVerticalGlyphSubstitution(m_cacheEntry->face());
hb_buffer_set_script(buffer, m_scriptForVerticalText);
}
Modified: trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.h (228524 => 228525)
--- trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.h 2018-02-15 19:04:01 UTC (rev 228524)
+++ trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.h 2018-02-15 19:06:05 UTC (rev 228525)
@@ -36,6 +36,8 @@
#include <memory>
#include <wtf/FastMalloc.h>
#include <wtf/HashMap.h>
+#include <wtf/Ref.h>
+#include <wtf/RefCounted.h>
namespace WebCore {
@@ -56,12 +58,34 @@
void setScriptForVerticalGlyphSubstitution(hb_buffer_t*);
private:
+ class CacheEntry : public RefCounted<CacheEntry> {
+ public:
+ using GlyphCache = HashMap<uint32_t, uint16_t>;
+
+ static Ref<CacheEntry> create(hb_face_t* face)
+ {
+ return adoptRef(*new CacheEntry(face));
+ }
+ ~CacheEntry();
+
+ hb_face_t* face() { return m_face; }
+ GlyphCache& glyphCache() { return m_glyphCache; }
+
+ private:
+ CacheEntry(hb_face_t*);
+
+ hb_face_t* m_face;
+ GlyphCache m_glyphCache;
+ };
+
+ using Cache = HashMap<uint64_t, RefPtr<CacheEntry>, WTF::IntHash<uint64_t>, WTF::UnsignedWithZeroKeyHashTraits<uint64_t>>;
+ static Cache& cache();
+
hb_face_t* createFace();
FontPlatformData* m_platformData;
uint64_t m_uniqueID;
- hb_face_t* m_face;
- WTF::HashMap<uint32_t, uint16_t>* m_glyphCacheForFaceCacheEntry;
+ RefPtr<CacheEntry> m_cacheEntry;
hb_script_t m_scriptForVerticalText;
};
Modified: trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp (228524 => 228525)
--- trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp 2018-02-15 19:04:01 UTC (rev 228524)
+++ trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp 2018-02-15 19:06:05 UTC (rev 228525)
@@ -50,7 +50,7 @@
namespace WebCore {
struct HarfBuzzFontData {
- WTF::HashMap<uint32_t, uint16_t>* glyphCacheForFaceCacheEntry;
+ WTF::HashMap<uint32_t, uint16_t>& glyphCacheForFaceCacheEntry;
RefPtr<cairo_scaled_font_t> cairoScaledFont;
};
@@ -91,7 +91,7 @@
auto* scaledFont = hbFontData.cairoScaledFont.get();
ASSERT(scaledFont);
- WTF::HashMap<uint32_t, uint16_t>::AddResult result = hbFontData.glyphCacheForFaceCacheEntry->add(unicode, 0);
+ auto result = hbFontData.glyphCacheForFaceCacheEntry.add(unicode, 0);
if (result.isNewEntry) {
cairo_glyph_t* glyphs = 0;
int numGlyphs = 0;
@@ -201,8 +201,8 @@
hb_font_t* HarfBuzzFace::createFont()
{
- hb_font_t* font = hb_font_create(m_face);
- hb_font_set_funcs(font, harfBuzzCairoTextGetFontFuncs(), new HarfBuzzFontData { m_glyphCacheForFaceCacheEntry, m_platformData->scaledFont() },
+ hb_font_t* font = hb_font_create(m_cacheEntry->face());
+ hb_font_set_funcs(font, harfBuzzCairoTextGetFontFuncs(), new HarfBuzzFontData { m_cacheEntry->glyphCache(), m_platformData->scaledFont() },
[](void* data)
{
delete static_cast<HarfBuzzFontData*>(data);
