[webkit-changes] [141908] trunk/Source/WebCore

Tue, 05 Feb 2013 11:36:35 -0800

Title: [141908] trunk/Source/WebCore
Revision
141908
Author
[email protected]
Date
2013-02-05 11:38:19 -0800 (Tue, 05 Feb 2013)

Log Message

[HarfBuzz][Cairo] harfBuzzGetGlyph is slow and hot
https://bugs.webkit.org/show_bug.cgi?id=108941

Reviewed by Kenneth Rohde Christiansen.

The text to glyph conversion using Cairo is slow
due to expensive text codec conversion to UTF-8.
Additionally, the glyph lookup itself is expensive.

Inspired by the approach taken in HarfBuzzFaceSkia.cpp
I suggest to implement a similar caching mechanism to
accelerate this conversion.

Arabic line breaking test, under review in
bug 108948 shows about 58% improvement on my system
with this patch.

* platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp:
(WebCore::HarfBuzzFontData::HarfBuzzFontData):
    New container structure that keeps pointers
    to the cairo scaled font as well as the glyph cache.
(HarfBuzzFontData):
(WebCore):
(WebCore::harfBuzzGetGlyph): Using the new container structure for accessing the cache.
(WebCore::harfBuzzGetGlyphHorizontalAdvance): Using the new container structure for accessing the scaled font.
(WebCore::harfBuzzGetGlyphExtents): Ditto.
(WebCore::destroyHarfBuzzFontData): Destroying the container that held the pointers.
(WebCore::HarfBuzzFace::createFont):
    Initializing the container structure with the pointers
    to the cache that is held in HarfBuzzFace and the cairo scaled font.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (141907 => 141908)


--- trunk/Source/WebCore/ChangeLog	2013-02-05 19:26:52 UTC (rev 141907)
+++ trunk/Source/WebCore/ChangeLog	2013-02-05 19:38:19 UTC (rev 141908)
@@ -1,3 +1,36 @@
+2013-02-05  Dominik Röttsches  <[email protected]>
+
+        [HarfBuzz][Cairo] harfBuzzGetGlyph is slow and hot
+        https://bugs.webkit.org/show_bug.cgi?id=108941
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        The text to glyph conversion using Cairo is slow
+        due to expensive text codec conversion to UTF-8.
+        Additionally, the glyph lookup itself is expensive.
+
+        Inspired by the approach taken in HarfBuzzFaceSkia.cpp
+        I suggest to implement a similar caching mechanism to
+        accelerate this conversion.
+
+        Arabic line breaking test, under review in
+        bug 108948 shows about 58% improvement on my system
+        with this patch.
+
+        * platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp:
+        (WebCore::HarfBuzzFontData::HarfBuzzFontData):
+            New container structure that keeps pointers
+            to the cairo scaled font as well as the glyph cache.
+        (HarfBuzzFontData):
+        (WebCore):
+        (WebCore::harfBuzzGetGlyph): Using the new container structure for accessing the cache.
+        (WebCore::harfBuzzGetGlyphHorizontalAdvance): Using the new container structure for accessing the scaled font.
+        (WebCore::harfBuzzGetGlyphExtents): Ditto.
+        (WebCore::destroyHarfBuzzFontData): Destroying the container that held the pointers.
+        (WebCore::HarfBuzzFace::createFont):
+            Initializing the container structure with the pointers
+            to the cache that is held in HarfBuzzFace and the cairo scaled font.
+
 2013-02-05  Tony Gentilcore  <[email protected]>
 
         Call XSSAuditor's didBlockScript() for the threaded HTML parser

Modified: trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp (141907 => 141908)


--- trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp	2013-02-05 19:26:52 UTC (rev 141907)
+++ trunk/Source/WebCore/platform/graphics/harfbuzz/HarfBuzzFaceCairo.cpp	2013-02-05 19:38:19 UTC (rev 141908)
@@ -46,6 +46,15 @@
 
 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;
+};
+
 class CairoFtFaceLocker {
 public:
     CairoFtFaceLocker(cairo_scaled_font_t* cairoScaledFont) : m_scaledFont(cairoScaledFont) { };
@@ -92,26 +101,30 @@
 
 static hb_bool_t harfBuzzGetGlyph(hb_font_t*, void* fontData, hb_codepoint_t unicode, hb_codepoint_t, hb_codepoint_t* glyph, void*)
 {
-    FontPlatformData* platformData = reinterpret_cast<FontPlatformData*>(fontData);
-    cairo_scaled_font_t* scaledFont = platformData->scaledFont();
+    HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
+    cairo_scaled_font_t* scaledFont = hbFontData->m_cairoScaledFont;
     ASSERT(scaledFont);
 
-    cairo_glyph_t* glyphs = 0;
-    int numGlyphs = 0;
-    CString utf8Codepoint = UTF8Encoding().encode(reinterpret_cast<UChar*>(&unicode), 1, QuestionMarksForUnencodables);
-    if (CAIRO_STATUS_SUCCESS != cairo_scaled_font_text_to_glyphs(scaledFont, 0, 0, utf8Codepoint.data(), utf8Codepoint.length(), &glyphs, &numGlyphs, 0, 0, 0))
-        return false;
-    if (!numGlyphs)
-        return false;
-    *glyph = glyphs[0].index;
-    cairo_glyph_free(glyphs);
-    return true;
+    WTF::HashMap<uint32_t, uint16_t>::AddResult result = hbFontData->m_glyphCacheForFaceCacheEntry->add(unicode, 0);
+    if (result.isNewEntry) {
+        cairo_glyph_t* glyphs = 0;
+        int numGlyphs = 0;
+        CString utf8Codepoint = UTF8Encoding().encode(reinterpret_cast<UChar*>(&unicode), 1, QuestionMarksForUnencodables);
+        if (cairo_scaled_font_text_to_glyphs(scaledFont, 0, 0, utf8Codepoint.data(), utf8Codepoint.length(), &glyphs, &numGlyphs, 0, 0, 0) != CAIRO_STATUS_SUCCESS)
+            return false;
+        if (!numGlyphs)
+            return false;
+        result.iterator->value = glyphs[0].index;
+        cairo_glyph_free(glyphs);
+    }
+    *glyph = result.iterator->value;
+    return !!*glyph;
 }
 
 static hb_position_t harfBuzzGetGlyphHorizontalAdvance(hb_font_t*, void* fontData, hb_codepoint_t glyph, void*)
 {
-    FontPlatformData* platformData = reinterpret_cast<FontPlatformData*>(fontData);
-    cairo_scaled_font_t* scaledFont = platformData->scaledFont();
+    HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
+    cairo_scaled_font_t* scaledFont = hbFontData->m_cairoScaledFont;
     ASSERT(scaledFont);
 
     hb_position_t advance = 0;
@@ -128,8 +141,8 @@
 
 static hb_bool_t harfBuzzGetGlyphExtents(hb_font_t*, void* fontData, hb_codepoint_t glyph, hb_glyph_extents_t* extents, void*)
 {
-    FontPlatformData* platformData = reinterpret_cast<FontPlatformData*>(fontData);
-    cairo_scaled_font_t* scaledFont = platformData->scaledFont();
+    HarfBuzzFontData* hbFontData = reinterpret_cast<HarfBuzzFontData*>(fontData);
+    cairo_scaled_font_t* scaledFont = hbFontData->m_cairoScaledFont;
     ASSERT(scaledFont);
 
     CairoGetGlyphWidthAndExtents(scaledFont, glyph, 0, extents);
@@ -182,6 +195,12 @@
     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);
@@ -192,7 +211,8 @@
 hb_font_t* HarfBuzzFace::createFont()
 {
     hb_font_t* font = hb_font_create(m_face);
-    hb_font_set_funcs(font, harfBuzzCairoTextGetFontFuncs(), m_platformData, 0);
+    HarfBuzzFontData* hbFontData = new HarfBuzzFontData(m_glyphCacheForFaceCacheEntry, m_platformData->scaledFont());
+    hb_font_set_funcs(font, harfBuzzCairoTextGetFontFuncs(), hbFontData, destroyHarfBuzzFontData);
     const float size = m_platformData->size();
     if (floorf(size) == size)
         hb_font_set_ppem(font, size, size);

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to