[Cairo] Remove the cairo_glyph_t complexity from GlyphBuffer
authorzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Sep 2017 14:54:26 +0000 (14:54 +0000)
committerzandobersek@gmail.com <zandobersek@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Sep 2017 14:54:26 +0000 (14:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177598

Reviewed by Carlos Garcia Campos.

Source/WebCore:

Remove the decade-old use of cairo_glyph_t as the underlying type for
Glyph. The former spans 24 bytes in size, while the latte is just 2
bytes. The x and y coordinate attributes of cairo_glyph_t were only
used in FontCascade::drawGlyphs() implementation, where they were
overridden even if the same GlyphBuffer object was used here repeatedly.

FontCascade::drawGlyphs() now creates a new Vector<cairo_glyph_t> object
and fills it only with the glyph index and coordinates data for glyphs
that will actually be drawn. This will likely in the future be leveraged
to pack the necessary data and perform the drawing operations in
parallelized tasks. GlyphBuffer usages that before required USE(CAIRO)
special-casing can now be simplified.

This also removes the need for <cairo.h> header inclusion in the
GlyphBuffer.h header. This further removes Cairo header inclusion in
roughly 600 build targets.

No new tests -- no change in behavior.

* platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::widthForSimpleText const):
* platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBuffer::glyphAt const):
(WebCore::GlyphBuffer::add):
* platform/graphics/cairo/FontCairo.cpp:
(WebCore::drawGlyphsToContext):
(WebCore::drawGlyphsShadow):
(WebCore::FontCascade::drawGlyphs):
* platform/graphics/displaylists/DisplayListItems.cpp:
(WebCore::DisplayList::DrawGlyphs::generateGlyphBuffer const):

Source/WebKit:

* Shared/API/c/cairo/WKImageCairo.cpp: Explicitly include the <cairo.h>
header here now that it's not included in GlyphBuffer.h.
* WebProcess/WebCoreSupport/gtk/WebDragClientGtk.cpp: Ditto.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/FontCascade.cpp
Source/WebCore/platform/graphics/GlyphBuffer.h
Source/WebCore/platform/graphics/cairo/FontCairo.cpp
Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp
Source/WebKit/ChangeLog
Source/WebKit/Shared/API/c/cairo/WKImageCairo.cpp
Source/WebKit/WebProcess/WebCoreSupport/gtk/WebDragClientGtk.cpp

index 92ba465..1930278 100644 (file)
@@ -1,3 +1,41 @@
+2017-09-28  Zan Dobersek  <zdobersek@igalia.com>
+
+        [Cairo] Remove the cairo_glyph_t complexity from GlyphBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=177598
+
+        Reviewed by Carlos Garcia Campos.
+
+        Remove the decade-old use of cairo_glyph_t as the underlying type for
+        Glyph. The former spans 24 bytes in size, while the latte is just 2
+        bytes. The x and y coordinate attributes of cairo_glyph_t were only
+        used in FontCascade::drawGlyphs() implementation, where they were
+        overridden even if the same GlyphBuffer object was used here repeatedly.
+
+        FontCascade::drawGlyphs() now creates a new Vector<cairo_glyph_t> object
+        and fills it only with the glyph index and coordinates data for glyphs
+        that will actually be drawn. This will likely in the future be leveraged
+        to pack the necessary data and perform the drawing operations in
+        parallelized tasks. GlyphBuffer usages that before required USE(CAIRO)
+        special-casing can now be simplified.
+
+        This also removes the need for <cairo.h> header inclusion in the
+        GlyphBuffer.h header. This further removes Cairo header inclusion in
+        roughly 600 build targets.
+
+        No new tests -- no change in behavior.
+
+        * platform/graphics/FontCascade.cpp:
+        (WebCore::FontCascade::widthForSimpleText const):
+        * platform/graphics/GlyphBuffer.h:
+        (WebCore::GlyphBuffer::glyphAt const):
+        (WebCore::GlyphBuffer::add):
+        * platform/graphics/cairo/FontCairo.cpp:
+        (WebCore::drawGlyphsToContext):
+        (WebCore::drawGlyphsShadow):
+        (WebCore::FontCascade::drawGlyphs):
+        * platform/graphics/displaylists/DisplayListItems.cpp:
+        (WebCore::DisplayList::DrawGlyphs::generateGlyphBuffer const):
+
 2017-09-28  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Unreviewed, rolling out r222606.
index 21d6047..6cb7208 100644 (file)
 #include <wtf/text/AtomicStringHash.h>
 #include <wtf/text/StringBuilder.h>
 
-#if USE(CAIRO)
-#include <cairo.h>
-#endif
-
 using namespace WTF;
 using namespace Unicode;
 
@@ -411,13 +407,7 @@ float FontCascade::widthForSimpleText(StringView text) const
         runWidth += glyphWidth;
         if (!hasKerningOrLigatures)
             continue;
-#if USE(CAIRO)
-        cairo_glyph_t cairoGlyph;
-        cairoGlyph.index = glyph;
-        glyphs.append(cairoGlyph);
-#else
         glyphs.append(glyph);
-#endif
         advances.append(FloatSize(glyphWidth, 0));
     }
     if (hasKerningOrLigatures) {
index 61f3f05..1752dcb 100644 (file)
 #include <CoreGraphics/CGGeometry.h>
 #endif
 
-#if USE(CAIRO)
-#include <cairo.h>
-#endif
-
 namespace WebCore {
 
 class Font;
 
-#if USE(CAIRO)
-// FIXME: Why does Cairo use such a huge struct instead of just an offset into an array?
-typedef cairo_glyph_t GlyphBufferGlyph;
-#elif USE(WINGDI)
+#if USE(WINGDI)
 typedef wchar_t GlyphBufferGlyph;
 #else
 typedef Glyph GlyphBufferGlyph;
@@ -110,11 +103,7 @@ public:
     
     Glyph glyphAt(unsigned index) const
     {
-#if USE(CAIRO)
-        return m_glyphs[index].index;
-#else
         return m_glyphs[index];
-#endif
     }
 
     GlyphBufferAdvance advanceAt(unsigned index) const
@@ -136,14 +125,7 @@ public:
     void add(Glyph glyph, const Font* font, float width, unsigned offsetInString = noOffset, const FloatSize* offset = 0)
     {
         m_font.append(font);
-
-#if USE(CAIRO)
-        cairo_glyph_t cairoGlyph;
-        cairoGlyph.index = glyph;
-        m_glyphs.append(cairoGlyph);
-#else
         m_glyphs.append(glyph);
-#endif
 
 #if USE(CG)
         CGSize advance = { width, 0 };
@@ -169,13 +151,7 @@ public:
     void add(Glyph glyph, const Font* font, GlyphBufferAdvance advance, unsigned offsetInString = noOffset)
     {
         m_font.append(font);
-#if USE(CAIRO)
-        cairo_glyph_t cairoGlyph;
-        cairoGlyph.index = glyph;
-        m_glyphs.append(cairoGlyph);
-#else
         m_glyphs.append(glyph);
-#endif
 
         m_advances.append(advance);
         
index a712d00..87b8fb8 100644 (file)
@@ -47,7 +47,7 @@
 
 namespace WebCore {
 
-static void drawGlyphsToContext(cairo_t* context, const Font& font, GlyphBufferGlyph* glyphs, unsigned numGlyphs)
+static void drawGlyphsToContext(cairo_t* context, const Font& font, const cairo_glyph_t* glyphs, unsigned numGlyphs)
 {
     cairo_matrix_t originalTransform;
     float syntheticBoldOffset = font.syntheticBoldOffset();
@@ -66,7 +66,7 @@ static void drawGlyphsToContext(cairo_t* context, const Font& font, GlyphBufferG
         cairo_set_matrix(context, &originalTransform);
 }
 
-static void drawGlyphsShadow(GraphicsContext& graphicsContext, const FloatPoint& point, const Font& font, GlyphBufferGlyph* glyphs, unsigned numGlyphs)
+static void drawGlyphsShadow(GraphicsContext& graphicsContext, const FloatPoint& point, const Font& font, const cairo_glyph_t* glyphs, unsigned numGlyphs)
 {
     ShadowBlur& shadow = graphicsContext.platformContext()->shadowBlur();
 
@@ -103,37 +103,42 @@ void FontCascade::drawGlyphs(GraphicsContext& context, const Font& font, const G
     if (!font.platformData().size())
         return;
 
-    GlyphBufferGlyph* glyphs = const_cast<GlyphBufferGlyph*>(glyphBuffer.glyphs(from));
-
-    float offset = point.x();
-    for (unsigned i = 0; i < numGlyphs; i++) {
-        glyphs[i].x = offset;
-        glyphs[i].y = point.y();
-        offset += glyphBuffer.advanceAt(from + i).width();
+    auto xOffset = point.x();
+    Vector<cairo_glyph_t> cairoGlyphs(numGlyphs);
+    {
+        ASSERT(from + numGlyphs <= glyphBuffer.size());
+        auto* glyphs = glyphBuffer.glyphs(from);
+        auto* advances = glyphBuffer.advances(from);
+
+        auto yOffset = point.y();
+        for (size_t i = 0; i < numGlyphs; ++i) {
+            cairoGlyphs[i] = { glyphs[i], xOffset, yOffset };
+            xOffset += advances[i].width();
+        }
     }
 
     PlatformContextCairo* platformContext = context.platformContext();
-    drawGlyphsShadow(context, point, font, glyphs, numGlyphs);
+    drawGlyphsShadow(context, point, font, cairoGlyphs.data(), numGlyphs);
 
     cairo_t* cr = platformContext->cr();
     cairo_save(cr);
 
     if (context.textDrawingMode() & TextModeFill) {
         platformContext->prepareForFilling(context.state(), PlatformContextCairo::AdjustPatternForGlobalAlpha);
-        drawGlyphsToContext(cr, font, glyphs, numGlyphs);
+        drawGlyphsToContext(cr, font, cairoGlyphs.data(), numGlyphs);
     }
 
     // Prevent running into a long computation within cairo. If the stroke width is
     // twice the size of the width of the text we will not ask cairo to stroke
     // the text as even one single stroke would cover the full wdth of the text.
     //  See https://bugs.webkit.org/show_bug.cgi?id=33759.
-    if (context.textDrawingMode() & TextModeStroke && context.strokeThickness() < 2 * offset) {
+    if (context.textDrawingMode() & TextModeStroke && context.strokeThickness() < 2 * xOffset) {
         platformContext->prepareForStroking(context.state());
         cairo_set_line_width(cr, context.strokeThickness());
 
         // This may disturb the CTM, but we are going to call cairo_restore soon after.
         cairo_set_scaled_font(cr, font.platformData().scaledFont());
-        cairo_glyph_path(cr, glyphs, numGlyphs);
+        cairo_glyph_path(cr, cairoGlyphs.data(), numGlyphs);
         cairo_stroke(cr);
     }
 
index 0a7caba..e88c448 100644 (file)
@@ -364,11 +364,7 @@ inline GlyphBuffer DrawGlyphs::generateGlyphBuffer() const
 {
     GlyphBuffer result;
     for (size_t i = 0; i < m_glyphs.size(); ++i) {
-#if USE(CAIRO)
-        result.add(m_glyphs[i].index, &m_font.get(), m_advances[i]);
-#else
         result.add(m_glyphs[i], &m_font.get(), m_advances[i]);
-#endif
     }
     return result;
 }
index 24cbb91..89a5261 100644 (file)
@@ -1,3 +1,14 @@
+2017-09-28  Zan Dobersek  <zdobersek@igalia.com>
+
+        [Cairo] Remove the cairo_glyph_t complexity from GlyphBuffer
+        https://bugs.webkit.org/show_bug.cgi?id=177598
+
+        Reviewed by Carlos Garcia Campos.
+
+        * Shared/API/c/cairo/WKImageCairo.cpp: Explicitly include the <cairo.h>
+        header here now that it's not included in GlyphBuffer.h.
+        * WebProcess/WebCoreSupport/gtk/WebDragClientGtk.cpp: Ditto.
+
 2017-09-27  Alex Christensen  <achristensen@webkit.org>
 
         Add WKContentRuleList notify action type
index bb2485f..30af4df 100644 (file)
@@ -32,6 +32,7 @@
 #include "WebImage.h"
 #include <WebCore/GraphicsContext.h>
 #include <WebCore/PlatformContextCairo.h>
+#include <cairo.h>
 
 using namespace WebKit;
 using namespace WebCore;
index 4c3db08..02e6687 100644 (file)
@@ -38,6 +38,7 @@
 #include <WebCore/GraphicsContext.h>
 #include <WebCore/Pasteboard.h>
 #include <WebCore/PlatformContextCairo.h>
+#include <cairo.h>
 
 using namespace WebCore;