Move glyph lookup to hb-ot-font and remove glyph cache in HarfBuzzFace

third_party/WebKit/Source/platform/fonts/shaping/HarfBuzzFace.cpp

 /*
  * Copyright (c) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 30 matching lines @@
 #include "SkRect.h"
 #include "SkTypeface.h"
 #include "platform/fonts/FontCache.h"
 #include "platform/fonts/FontPlatformData.h"
 #include "platform/fonts/SimpleFontData.h"
 #include "platform/fonts/shaping/HarfBuzzShaper.h"
 #include "wtf/HashMap.h"
 
 namespace blink {
 
-const hb_tag_t HarfBuzzFace::vertTag = HB_TAG('v', 'e', 'r', 't');
-
 // Though we have FontCache class, which provides the cache mechanism for
 // WebKit's font objects, we also need additional caching layer for HarfBuzz
 // 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 PassRefPtr<FaceCacheEntry> create(hb_face_t* face)
     {
         ASSERT(face);
         return adoptRef(new FaceCacheEntry(face));
     }
     ~FaceCacheEntry()
     {
         hb_face_destroy(m_face);
     }
 
     hb_face_t* face() { return m_face; }
-    HashMap<uint32_t, uint16_t>* glyphCache() { return &m_glyphCache; }
 
 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()
 {
     DEFINE_STATIC_LOCAL(HarfBuzzFaceCache, s_harfBuzzFaceCache, ());
     return &s_harfBuzzFaceCache;
 }
 
+
 HarfBuzzFace::HarfBuzzFace(FontPlatformData* platformData, uint64_t uniqueID)
     : m_platformData(platformData)
     , m_uniqueID(uniqueID)
 {
     HarfBuzzFaceCache::AddResult result = harfBuzzFaceCache()->add(m_uniqueID, nullptr);
     if (result.isNewEntry)
         result.storedValue->value = FaceCacheEntry::create(createFace());
     result.storedValue->value->ref();
     m_face = result.storedValue->value->face();
-    m_glyphCacheForFaceCacheEntry = result.storedValue->value->glyphCache();
+    prepareHarfBuzzFontData();
 }
 
 HarfBuzzFace::~HarfBuzzFace()
 {
     HarfBuzzFaceCache::iterator result = harfBuzzFaceCache()->find(m_uniqueID);
     ASSERT_WITH_SECURITY_IMPLICATION(result != harfBuzzFaceCache()->end());
     ASSERT(result.get()->value->refCount() > 1);
     result.get()->value->deref();
     if (result.get()->value->refCount() == 1)
         harfBuzzFaceCache()->remove(m_uniqueID);
 }
 
+// struct to carry user-pointer data for hb_font_t callback functions.
 struct HarfBuzzFontData {
     USING_FAST_MALLOC(HarfBuzzFontData);
     WTF_MAKE_NONCOPYABLE(HarfBuzzFontData);
 public:
-    HarfBuzzFontData(WTF::HashMap<uint32_t, uint16_t>* glyphCacheForFaceCacheEntry, hb_face_t* face, unsigned rangeFrom, unsigned rangeTo)
-        : m_glyphCacheForFaceCacheEntry(glyphCacheForFaceCacheEntry)
-        , m_face(face)
-        , m_hbOpenTypeFont(nullptr)
-        , m_rangeFrom(rangeFrom)
-        , m_rangeTo(rangeTo)
+    HarfBuzzFontData()
+        : m_rangeFrom(0)
+        , m_rangeTo(kMaxCodepoint)
     { }
 
-    ~HarfBuzzFontData()
-    {
-        if (m_hbOpenTypeFont)
-            hb_font_destroy(m_hbOpenTypeFont);
-    }
-
     SkPaint m_paint;
     RefPtr<SimpleFontData> m_simpleFontData;
-    WTF::HashMap<uint32_t, uint16_t>* m_glyphCacheForFaceCacheEntry;
-    hb_face_t* m_face;
-    hb_font_t* m_hbOpenTypeFont;
     unsigned m_rangeFrom;
     unsigned m_rangeTo;
 };
 
 static hb_position_t SkiaScalarToHarfBuzzPosition(SkScalar value)
 {
     return SkScalarToFixed(value);
 }
 
 static void SkiaGetGlyphWidthAndExtents(SkPaint* paint, hb_codepoint_t codepoint, hb_position_t* width, hb_glyph_extents_t* extents)
@@ skipping 20 matching lines @@
             skBounds.set(ir);
         }
         // Invert y-axis because Skia is y-grows-down but we set up HarfBuzz to be y-grows-up.
         extents->x_bearing = SkiaScalarToHarfBuzzPosition(skBounds.fLeft);
         extents->y_bearing = SkiaScalarToHarfBuzzPosition(-skBounds.fTop);
         extents->width = SkiaScalarToHarfBuzzPosition(skBounds.width());
         extents->height = SkiaScalarToHarfBuzzPosition(-skBounds.height());
     }
 }
 
-#if !defined(HB_VERSION_ATLEAST)
-#define HB_VERSION_ATLEAST(major, minor, micro) 0
-#endif
-
 static hb_bool_t harfBuzzGetGlyph(hb_font_t* hbFont, void* fontData, hb_codepoint_t unicode, hb_codepoint_t variationSelector, hb_codepoint_t* glyph, void* userData)
 {
     HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
 
     RELEASE_ASSERT(hbFontData);
     if (unicode < hbFontData->m_rangeFrom || unicode > hbFontData->m_rangeTo)
         return false;
 
-    if (variationSelector) {
-#if !HB_VERSION_ATLEAST(0, 9, 28)
-        return false;
-#else
-        // Skia does not support variation selectors, but hb does.
-        // We're not fully ready to switch to hb-ot-font yet,
-        // but are good enough to get glyph IDs for OpenType fonts.
-        if (!hbFontData->m_hbOpenTypeFont) {
-            hbFontData->m_hbOpenTypeFont = hb_font_create(hbFontData->m_face);
-            hb_ot_font_set_funcs(hbFontData->m_hbOpenTypeFont);
-        }
-        return hb_font_get_glyph(hbFontData->m_hbOpenTypeFont, unicode, variationSelector, glyph);
-        // When not found, glyph_func should return false rather than fallback to the base.
-        // http://lists.freedesktop.org/archives/harfbuzz/2015-May/004888.html
-#endif
-    }
-
-    WTF::HashMap<uint32_t, uint16_t>::AddResult result = hbFontData->m_glyphCacheForFaceCacheEntry->add(unicode, 0);
-    if (result.isNewEntry) {
-        SkPaint* paint = &hbFontData->m_paint;
-        paint->setTextEncoding(SkPaint::kUTF32_TextEncoding);
-        uint16_t glyph16;
-        paint->textToGlyphs(&unicode, sizeof(hb_codepoint_t), &glyph16);
-        result.storedValue->value = glyph16;
-        *glyph = glyph16;
-    }
-    *glyph = result.storedValue->value;
-    return !!*glyph;
+    return hb_font_get_glyph(hb_font_get_parent(hbFont), unicode, variationSelector, glyph);
 }
 
 static hb_position_t harfBuzzGetGlyphHorizontalAdvance(hb_font_t* hbFont, void* fontData, hb_codepoint_t glyph, void* userData)
 {
     HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
     hb_position_t advance = 0;
 
     SkiaGetGlyphWidthAndExtents(&hbFontData->m_paint, glyph, &advance, 0);
     return advance;
 }
 
-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;
-}
-
 static hb_bool_t harfBuzzGetGlyphVerticalOrigin(hb_font_t* hbFont, void* fontData, hb_codepoint_t glyph, hb_position_t* x, hb_position_t* y, void* userData)
 {
     HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
     const OpenTypeVerticalData* verticalData = hbFontData->m_simpleFontData->verticalData();
     if (!verticalData)
         return false;
 
     float result[] = { 0, 0 };
     Glyph theGlyph = glyph;
     verticalData->getVerticalTranslationsForGlyphs(hbFontData->m_simpleFontData.get(), &theGlyph, 1, result);
@@ skipping 48 matching lines @@
 {
     static hb_font_funcs_t* harfBuzzSkiaFontFuncs = 0;
 
     // We don't set callback functions which we can't support.
     // HarfBuzz will use the fallback implementation if they aren't set.
     if (!harfBuzzSkiaFontFuncs) {
         harfBuzzSkiaFontFuncs = hb_font_funcs_create();
         hb_font_funcs_set_glyph_func(harfBuzzSkiaFontFuncs, harfBuzzGetGlyph, 0, 0);
         hb_font_funcs_set_glyph_h_advance_func(harfBuzzSkiaFontFuncs, harfBuzzGetGlyphHorizontalAdvance, 0, 0);
         hb_font_funcs_set_glyph_h_kerning_func(harfBuzzSkiaFontFuncs, harfBuzzGetGlyphHorizontalKerning, 0, 0);
-        hb_font_funcs_set_glyph_h_origin_func(harfBuzzSkiaFontFuncs, harfBuzzGetGlyphHorizontalOrigin, 0, 0);
         hb_font_funcs_set_glyph_v_advance_func(harfBuzzSkiaFontFuncs, harfBuzzGetGlyphVerticalAdvance, 0, 0);
         hb_font_funcs_set_glyph_v_origin_func(harfBuzzSkiaFontFuncs, harfBuzzGetGlyphVerticalOrigin, 0, 0);
         hb_font_funcs_set_glyph_extents_func(harfBuzzSkiaFontFuncs, harfBuzzGetGlyphExtents, 0, 0);
         hb_font_funcs_make_immutable(harfBuzzSkiaFontFuncs);
     }
     return harfBuzzSkiaFontFuncs;
 }
 
 #if !OS(MACOSX)
 static hb_blob_t* harfBuzzSkiaGetTable(hb_face_t* face, hb_tag_t tag, void* userData)
@@ skipping 11 matching lines @@
     size_t actualSize = typeface->getTableData(tag, 0, tableSize, buffer);
     if (tableSize != actualSize) {
         WTF::Partitions::fastFree(buffer);
         return nullptr;
     }
 
     return hb_blob_create(const_cast<char*>(buffer), tableSize, HB_MEMORY_MODE_WRITABLE, buffer, WTF::Partitions::fastFree);
 }
 #endif
 
-static void destroyHarfBuzzFontData(void* userData)
-{
-    HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(userData);
-    delete hbFontData;
-}
-
 hb_face_t* HarfBuzzFace::createFace()
 {
 #if OS(MACOSX)
     hb_face_t* face = hb_coretext_face_create(m_platformData->cgFont());
 #else
     hb_face_t* face = hb_face_create_for_tables(harfBuzzSkiaGetTable, m_platformData->typeface(), 0);
 #endif
     ASSERT(face);
     return face;
 }
 
+void HarfBuzzFace::prepareHarfBuzzFontData()
+{
+    m_harfBuzzFontData = adoptPtr(new HarfBuzzFontData());
+    m_harfBuzzFontData->m_simpleFontData = FontCache::fontCache()->fontDataFromFontPlatformData(m_platformData);
+    ASSERT(m_harfBuzzFontData->m_simpleFontData);
+    HarfBuzzScopedPtr<hb_font_t> otFont(hb_font_create(m_face), hb_font_destroy);

[kojii, 2016/02/25 07:55:17]

HarfBuzzScopedPtr is not needed for hb_font_t as I have added
OwnedPtrDeleter<hb_font_t> before in:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

so just |OwnPtr<hb_font_t>| should work, like this:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[drott, 2016/02/25 09:05:59]

Thanks, good point, updated accordingly.

+    hb_ot_font_set_funcs(otFont.get());
+    // Creating a sub font means that non-available functions
+    // are found from the parent.
+    m_unscaledFont = hb_font_create_sub_font(otFont.get());
+    hb_font_set_funcs(m_unscaledFont, harfBuzzSkiaGetFontFuncs(), m_harfBuzzFontData.get(), nullptr);
+}
+
 hb_font_t* HarfBuzzFace::createFont(unsigned rangeFrom, unsigned rangeTo) const
 {
-    HarfBuzzFontData* hbFontData = new HarfBuzzFontData(m_glyphCacheForFaceCacheEntry, m_face, rangeFrom, rangeTo);
-    m_platformData->setupPaint(&hbFontData->m_paint);
-    hbFontData->m_simpleFontData = FontCache::fontCache()->fontDataFromFontPlatformData(m_platformData);
-    ASSERT(hbFontData->m_simpleFontData);
-    hb_font_t* font = hb_font_create(m_face);
-    hb_font_set_funcs(font, harfBuzzSkiaGetFontFuncs(), hbFontData, destroyHarfBuzzFontData);
-    float size = m_platformData->size();
-    int scale = SkiaScalarToHarfBuzzPosition(size);
-    hb_font_set_scale(font, scale, scale);
-    hb_font_make_immutable(font);
-    return font;
+    m_platformData->setupPaint(&m_harfBuzzFontData->m_paint);
+    m_harfBuzzFontData->m_rangeFrom = rangeFrom;
+    m_harfBuzzFontData->m_rangeTo = rangeTo;
+    int scale = SkiaScalarToHarfBuzzPosition(m_platformData->size());
+    hb_font_set_scale(m_unscaledFont, scale, scale);
+    return hb_font_reference(m_unscaledFont);
 }
 
 } // namespace blink