[Cairo] Remove the cairo_glyph_t complexity from GlyphBuffer

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.


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

Source/WebCore/ChangeLog

@@ -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.

Source/WebCore/platform/graphics/FontCascade.cpp

@@ -40,10 +40,6 @@
 #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) {

Source/WebCore/platform/graphics/GlyphBuffer.h

@@ -39,18 +39,11 @@
 #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 @@ class GlyphBuffer {
 
     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 @@ class GlyphBuffer {
     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 @@ class GlyphBuffer {
     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);
 

Source/WebCore/platform/graphics/cairo/FontCairo.cpp

@@ -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);
     }
 

Source/WebCore/platform/graphics/displaylists/DisplayListItems.cpp

@@ -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;
 }

Source/WebKit/ChangeLog

@@ -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

Source/WebKit/Shared/API/c/cairo/WKImageCairo.cpp

@@ -32,6 +32,7 @@
 #include "WebImage.h"
 #include <WebCore/GraphicsContext.h>
 #include <WebCore/PlatformContextCairo.h>
+#include <cairo.h>
 
 using namespace WebKit;
 using namespace WebCore;

Source/WebKit/WebProcess/WebCoreSupport/gtk/WebDragClientGtk.cpp

@@ -38,6 +38,7 @@
 #include <WebCore/GraphicsContext.h>
 #include <WebCore/Pasteboard.h>
 #include <WebCore/PlatformContextCairo.h>
+#include <cairo.h>
 
 using namespace WebCore;