Changeset 228525 in webkit


Ignore:
Timestamp:
Feb 15, 2018, 11:06:05 AM (8 years ago)
Author:
zandobersek@gmail.com
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):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r228524 r228525  
     12018-02-15  Zan Dobersek  <zdobersek@igalia.com>
     2
     3        HarfBuzzFace: rework cache entry reference holding
     4        https://bugs.webkit.org/show_bug.cgi?id=182828
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        Move the FaceCacheEntry and HarfBuzzFaceCache types into the
     9        HarfBuzzFace class as CacheEntry and Cache, respectively. The Cache
     10        singleton is also moved there.
     11
     12        In the HarfBuzzFace constructor, we now don't increase the CacheEntry
     13        reference, but instead just keep a reference to that object through
     14        a RefPtr<CacheEntry> object. We don't need to retrieve the hb_face_t
     15        object and the glyph cache HashMap in the constructor anymore, we just
     16        retrieve them when necessary through that CacheEntry reference.
     17
     18        In the destructor, that RefPtr<CacheEntry> object is nulled out before
     19        the object in Cache is removed if that's where the final reference is
     20        kept.
     21
     22        * platform/graphics/harfbuzz/HarfBuzzFace.cpp:
     23        (WebCore::HarfBuzzFace::CacheEntry::CacheEntry):
     24        (WebCore::HarfBuzzFace::CacheEntry::~CacheEntry):
     25        (WebCore::HarfBuzzFace::cache):
     26        (WebCore::HarfBuzzFace::HarfBuzzFace):
     27        (WebCore::HarfBuzzFace::~HarfBuzzFace):
     28        (WebCore::HarfBuzzFace::setScriptForVerticalGlyphSubstitution):
     29        (WebCore::FaceCacheEntry::create): Deleted.
     30        (WebCore::FaceCacheEntry::~FaceCacheEntry): Deleted.
     31        (WebCore::FaceCacheEntry::face): Deleted.
     32        (WebCore::FaceCacheEntry::glyphCache): Deleted.
     33        (WebCore::FaceCacheEntry::FaceCacheEntry): Deleted.
     34        (WebCore::harfBuzzFaceCache): Deleted.
     35        * platform/graphics/harfbuzz/HarfBuzzFace.h:
     36        (WebCore::HarfBuzzFace::CacheEntry::create):
     37        (WebCore::HarfBuzzFace::CacheEntry::face):
     38        (WebCore::HarfBuzzFace::CacheEntry::glyphCache):
     39        * platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp:
     40        (WebCore::harfBuzzGetGlyph):
     41        (WebCore::HarfBuzzFace::createFont):
     42
    1432018-02-15  Zan Dobersek  <zdobersek@igalia.com>
    244

  • trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.cpp

    r219858 r228525  
    4848// underling font data (e.g. CTFontRef, FTFace).
    4949
    50 class FaceCacheEntry : public RefCounted<FaceCacheEntry> {
    51 public:
    52     static Ref<FaceCacheEntry> create(hb_face_t* face)
    53     {
    54         ASSERT(face);
    55         return adoptRef(*new FaceCacheEntry(face));
    56     }
    57     ~FaceCacheEntry()
    58     {
    59         hb_face_destroy(m_face);
    60     }
     50HarfBuzzFace::CacheEntry::CacheEntry(hb_face_t* face)
     51    : m_face(face)
     52{
     53    ASSERT(m_face);
     54}
    6155
    62     hb_face_t* face() { return m_face; }
    63     HashMap<uint32_t, uint16_t>* glyphCache() { return &m_glyphCache; }
     56HarfBuzzFace::CacheEntry::~CacheEntry()
     57{
     58    hb_face_destroy(m_face);
     59}
    6460
    65 private:
    66     explicit FaceCacheEntry(hb_face_t* face)
    67         : m_face(face)
    68     { }
    69 
    70     hb_face_t* m_face;
    71     HashMap<uint32_t, uint16_t> m_glyphCache;
    72 };
    73 
    74 typedef HashMap<uint64_t, RefPtr<FaceCacheEntry>, WTF::IntHash<uint64_t>, WTF::UnsignedWithZeroKeyHashTraits<uint64_t> > HarfBuzzFaceCache;
    75 
    76 static HarfBuzzFaceCache* harfBuzzFaceCache()
     61HarfBuzzFace::Cache& HarfBuzzFace::cache()
    7762{
    78     static NeverDestroyed<HarfBuzzFaceCache> s_harfBuzzFaceCache;
    79     return &s_harfBuzzFaceCache.get();
     63    static NeverDestroyed<Cache> s_cache;
     64    return s_cache;
    8065}
    8166
    … …  
    8570    , m_scriptForVerticalText(HB_SCRIPT_INVALID)
    8671{
    87     HarfBuzzFaceCache::AddResult result = harfBuzzFaceCache()->add(m_uniqueID, nullptr);
     72    auto result = cache().add(m_uniqueID, nullptr);
    8873    if (result.isNewEntry)
    89         result.iterator->value = FaceCacheEntry::create(createFace());
    90     result.iterator->value->ref();
    91     m_face = result.iterator->value->face();
    92     m_glyphCacheForFaceCacheEntry = result.iterator->value->glyphCache();
     74        result.iterator->value = CacheEntry::create(createFace());
     75    m_cacheEntry = result.iterator->value;
    9376}
    9477
    9578HarfBuzzFace::~HarfBuzzFace()
    9679{
    97     HarfBuzzFaceCache::iterator result = harfBuzzFaceCache()->find(m_uniqueID);
    98     ASSERT(result != harfBuzzFaceCache()->end());
    99     ASSERT(result.get()->value->refCount() > 1);
    100     result.get()->value->deref();
    101     if (result.get()->value->refCount() == 1)
    102         harfBuzzFaceCache()->remove(m_uniqueID);
     80    auto it = cache().find(m_uniqueID);
     81    ASSERT(it != cache().end());
     82    ASSERT(it->value == m_cacheEntry);
     83    ASSERT(it->value->refCount() > 1);
     84
     85    m_cacheEntry = nullptr;
     86    if (it->value->refCount() == 1)
     87        cache().remove(it);
    10388}
    10489
    … …  
    127112{
    128113    if (m_scriptForVerticalText == HB_SCRIPT_INVALID)
    129         m_scriptForVerticalText = findScriptForVerticalGlyphSubstitution(m_face);
     114        m_scriptForVerticalText = findScriptForVerticalGlyphSubstitution(m_cacheEntry->face());
    130115    hb_buffer_set_script(buffer, m_scriptForVerticalText);
    131116}

  • trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFace.h

    r228507 r228525  
    3737#include <wtf/FastMalloc.h>
    3838#include <wtf/HashMap.h>
     39#include <wtf/Ref.h>
     40#include <wtf/RefCounted.h>
    3941
    4042namespace WebCore {
    … …  
    5759
    5860private:
     61    class CacheEntry : public RefCounted<CacheEntry> {
     62    public:
     63        using GlyphCache = HashMap<uint32_t, uint16_t>;
     64
     65        static Ref<CacheEntry> create(hb_face_t* face)
     66        {
     67            return adoptRef(*new CacheEntry(face));
     68        }
     69        ~CacheEntry();
     70
     71        hb_face_t* face() { return m_face; }
     72        GlyphCache& glyphCache() { return m_glyphCache; }
     73
     74    private:
     75        CacheEntry(hb_face_t*);
     76
     77        hb_face_t* m_face;
     78        GlyphCache m_glyphCache;
     79    };
     80
     81    using Cache = HashMap<uint64_t, RefPtr<CacheEntry>, WTF::IntHash<uint64_t>, WTF::UnsignedWithZeroKeyHashTraits<uint64_t>>;
     82    static Cache& cache();
     83
    5984    hb_face_t* createFace();
    6085
    6186    FontPlatformData* m_platformData;
    6287    uint64_t m_uniqueID;
    63     hb_face_t* m_face;
    64     WTF::HashMap<uint32_t, uint16_t>* m_glyphCacheForFaceCacheEntry;
     88    RefPtr<CacheEntry> m_cacheEntry;
    6589
    6690    hb_script_t m_scriptForVerticalText;

  • trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp

    r228509 r228525  
    5151
    5252struct HarfBuzzFontData {
    53     WTF::HashMap<uint32_t, uint16_t>* glyphCacheForFaceCacheEntry;
     53    WTF::HashMap<uint32_t, uint16_t>& glyphCacheForFaceCacheEntry;
    5454    RefPtr<cairo_scaled_font_t> cairoScaledFont;
    5555};
    … …  
    9292    ASSERT(scaledFont);
    9393
    94     WTF::HashMap<uint32_t, uint16_t>::AddResult result = hbFontData.glyphCacheForFaceCacheEntry->add(unicode, 0);
     94    auto result = hbFontData.glyphCacheForFaceCacheEntry.add(unicode, 0);
    9595    if (result.isNewEntry) {
    9696        cairo_glyph_t* glyphs = 0;
    … …  
    202202hb_font_t* HarfBuzzFace::createFont()
    203203{
    204     hb_font_t* font = hb_font_create(m_face);
    205     hb_font_set_funcs(font, harfBuzzCairoTextGetFontFuncs(), new HarfBuzzFontData { m_glyphCacheForFaceCacheEntry, m_platformData->scaledFont() },
     204    hb_font_t* font = hb_font_create(m_cacheEntry->face());
     205    hb_font_set_funcs(font, harfBuzzCairoTextGetFontFuncs(), new HarfBuzzFontData { m_cacheEntry->glyphCache(), m_platformData->scaledFont() },
    206206        [](void* data)
    207207        {
Note: See TracChangeset for help on using the changeset viewer.