Clean up HarfBuzzFaceCairo
authorzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Feb 2018 10:12:38 +0000 (10:12 +0000)
committerzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Feb 2018 10:12:38 +0000 (10:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182824

Reviewed by Carlos Garcia Campos.

Clean up Cairo-specific bits of HarfBuzzFace implementation.

HarfBuzzFontData is simplified, removing the constructor and turning the
cairo_scaled_font_t member into a RefPtr<>, tying the Cairo object's
lifetime to the lifetime of the HarfBuzzFontData instance.

HarfBuzz font callbacks have the HarfBuzzFontData casting cleaned up,
casting the user data pointer straight into a HarfBuzzFontData reference
that's then used in the functions. HarfBuzzFontData member access is
also adjusted.

HarfBuzzFace::createFace() now references the cairo_scaled_font_t object
that is then set as the user data pointer, with the destroy callback
that dereferences that object also specified. With hb_face_t being a
reference-counted object itself, this ensures the cairo_scaled_font_t
object doesn't get destroyed while hb_face_t is still alive.

In HarfBuzzFace::createFont(), the hb_font_t creation is cleaned up,
with a C++ lambda used as the destroy callback.

* platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp:
(WebCore::harfBuzzGetGlyph):
(WebCore::harfBuzzGetGlyphHorizontalAdvance):
(WebCore::harfBuzzGetGlyphExtents):
(WebCore::harfBuzzCairoGetTable):
(WebCore::HarfBuzzFace::createFace):
(WebCore::HarfBuzzFace::createFont):
(WebCore::HarfBuzzFontData::HarfBuzzFontData): Deleted.
(WebCore::destroyHarfBuzzFontData): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@228509 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp

index 5a674a7..cdc00ae 100644 (file)
@@ -1,3 +1,40 @@
+2018-02-15  Zan Dobersek  <zdobersek@igalia.com>
+
+        Clean up HarfBuzzFaceCairo
+        https://bugs.webkit.org/show_bug.cgi?id=182824
+
+        Reviewed by Carlos Garcia Campos.
+
+        Clean up Cairo-specific bits of HarfBuzzFace implementation.
+
+        HarfBuzzFontData is simplified, removing the constructor and turning the
+        cairo_scaled_font_t member into a RefPtr<>, tying the Cairo object's
+        lifetime to the lifetime of the HarfBuzzFontData instance.
+
+        HarfBuzz font callbacks have the HarfBuzzFontData casting cleaned up,
+        casting the user data pointer straight into a HarfBuzzFontData reference
+        that's then used in the functions. HarfBuzzFontData member access is
+        also adjusted.
+
+        HarfBuzzFace::createFace() now references the cairo_scaled_font_t object
+        that is then set as the user data pointer, with the destroy callback
+        that dereferences that object also specified. With hb_face_t being a
+        reference-counted object itself, this ensures the cairo_scaled_font_t
+        object doesn't get destroyed while hb_face_t is still alive.
+
+        In HarfBuzzFace::createFont(), the hb_font_t creation is cleaned up,
+        with a C++ lambda used as the destroy callback.
+
+        * platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp:
+        (WebCore::harfBuzzGetGlyph):
+        (WebCore::harfBuzzGetGlyphHorizontalAdvance):
+        (WebCore::harfBuzzGetGlyphExtents):
+        (WebCore::harfBuzzCairoGetTable):
+        (WebCore::HarfBuzzFace::createFace):
+        (WebCore::HarfBuzzFace::createFont):
+        (WebCore::HarfBuzzFontData::HarfBuzzFontData): Deleted.
+        (WebCore::destroyHarfBuzzFontData): Deleted.
+
 2018-02-15  Philippe Normand  <pnormand@igalia.com>
 
         [GStreamer] WebVTT caps changed in GStreamer 1.14
index cd254ef..1e1de4a 100644 (file)
 namespace WebCore {
 
 struct HarfBuzzFontData {
-    HarfBuzzFontData(WTF::HashMap<uint32_t, uint16_t>* glyphCacheForFaceCacheEntry, cairo_scaled_font_t* cairoScaledFont)
-        : m_glyphCacheForFaceCacheEntry(glyphCacheForFaceCacheEntry)
-        , m_cairoScaledFont(cairoScaledFont)
-    { }
-    WTF::HashMap<uint32_t, uint16_t>* m_glyphCacheForFaceCacheEntry;
-    cairo_scaled_font_t* m_cairoScaledFont;
+    WTF::HashMap<uint32_t, uint16_t>* glyphCacheForFaceCacheEntry;
+    RefPtr<cairo_scaled_font_t> cairoScaledFont;
 };
 
 static hb_position_t floatToHarfBuzzPosition(float value)
@@ -91,11 +87,11 @@ static void CairoGetGlyphWidthAndExtents(cairo_scaled_font_t* scaledFont, hb_cod
 
 static hb_bool_t harfBuzzGetGlyph(hb_font_t*, void* fontData, hb_codepoint_t unicode, hb_codepoint_t, hb_codepoint_t* glyph, void*)
 {
-    HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
-    cairo_scaled_font_t* scaledFont = hbFontData->m_cairoScaledFont;
+    auto& hbFontData = *static_cast<HarfBuzzFontData*>(fontData);
+    auto* scaledFont = hbFontData.cairoScaledFont.get();
     ASSERT(scaledFont);
 
-    WTF::HashMap<uint32_t, uint16_t>::AddResult result = hbFontData->m_glyphCacheForFaceCacheEntry->add(unicode, 0);
+    WTF::HashMap<uint32_t, uint16_t>::AddResult result = hbFontData.glyphCacheForFaceCacheEntry->add(unicode, 0);
     if (result.isNewEntry) {
         cairo_glyph_t* glyphs = 0;
         int numGlyphs = 0;
@@ -117,8 +113,8 @@ static hb_bool_t harfBuzzGetGlyph(hb_font_t*, void* fontData, hb_codepoint_t uni
 
 static hb_position_t harfBuzzGetGlyphHorizontalAdvance(hb_font_t*, void* fontData, hb_codepoint_t glyph, void*)
 {
-    HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
-    cairo_scaled_font_t* scaledFont = hbFontData->m_cairoScaledFont;
+    auto& hbFontData = *static_cast<HarfBuzzFontData*>(fontData);
+    auto* scaledFont = hbFontData.cairoScaledFont.get();
     ASSERT(scaledFont);
 
     hb_position_t advance = 0;
@@ -135,8 +131,8 @@ static hb_bool_t harfBuzzGetGlyphHorizontalOrigin(hb_font_t*, void*, hb_codepoin
 
 static hb_bool_t harfBuzzGetGlyphExtents(hb_font_t*, void* fontData, hb_codepoint_t glyph, hb_glyph_extents_t* extents, void*)
 {
-    HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
-    cairo_scaled_font_t* scaledFont = hbFontData->m_cairoScaledFont;
+    auto& hbFontData = *static_cast<HarfBuzzFontData*>(fontData);
+    auto* scaledFont = hbFontData.cairoScaledFont.get();
     ASSERT(scaledFont);
 
     CairoGetGlyphWidthAndExtents(scaledFont, glyph, 0, extents);
@@ -162,7 +158,7 @@ static hb_font_funcs_t* harfBuzzCairoTextGetFontFuncs()
 
 static hb_blob_t* harfBuzzCairoGetTable(hb_face_t*, hb_tag_t tag, void* userData)
 {
-    cairo_scaled_font_t* scaledFont = reinterpret_cast<cairo_scaled_font_t*>(userData);
+    auto* scaledFont = static_cast<cairo_scaled_font_t*>(userData);
     if (!scaledFont)
         return 0;
 
@@ -189,15 +185,16 @@ static hb_blob_t* harfBuzzCairoGetTable(hb_face_t*, hb_tag_t tag, void* userData
     return hb_blob_create(reinterpret_cast<const char*>(buffer), tableSize, HB_MEMORY_MODE_WRITABLE, buffer, fastFree);
 }
 
-static void destroyHarfBuzzFontData(void* userData)
-{
-    HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(userData);
-    delete hbFontData;
-}
-
 hb_face_t* HarfBuzzFace::createFace()
 {
-    hb_face_t* face = hb_face_create_for_tables(harfBuzzCairoGetTable, m_platformData->scaledFont(), 0);
+    auto* scaledFont = m_platformData->scaledFont();
+    cairo_scaled_font_reference(scaledFont);
+
+    hb_face_t* face = hb_face_create_for_tables(harfBuzzCairoGetTable, scaledFont,
+        [](void* data)
+        {
+            cairo_scaled_font_destroy(static_cast<cairo_scaled_font_t*>(data));
+        });
     ASSERT(face);
     return face;
 }
@@ -205,8 +202,12 @@ hb_face_t* HarfBuzzFace::createFace()
 hb_font_t* HarfBuzzFace::createFont()
 {
     hb_font_t* font = hb_font_create(m_face);
-    HarfBuzzFontData* hbFontData = new HarfBuzzFontData(m_glyphCacheForFaceCacheEntry, m_platformData->scaledFont());
-    hb_font_set_funcs(font, harfBuzzCairoTextGetFontFuncs(), hbFontData, destroyHarfBuzzFontData);
+    hb_font_set_funcs(font, harfBuzzCairoTextGetFontFuncs(), new HarfBuzzFontData { m_glyphCacheForFaceCacheEntry, m_platformData->scaledFont() },
+        [](void* data)
+        {
+            delete static_cast<HarfBuzzFontData*>(data);
+        });
+
     const float size = m_platformData->size();
     if (floorf(size) == size)
         hb_font_set_ppem(font, size, size);