Optimize Complex Text Shaping and Caching

Source/platform/fonts/Font.cpp

Index: Source/platform/fonts/Font.cpp
diff --git a/Source/platform/fonts/Font.cpp b/Source/platform/fonts/Font.cpp
index 0e37b836daa12fb56811319037cd8680a5ea3f53..99bb792a3c9728b4eed4312a4bb87f6b7145daa7 100644
--- a/Source/platform/fonts/Font.cpp
+++ b/Source/platform/fonts/Font.cpp
@@ -27,6 +27,8 @@
 
 #include "SkPaint.h"
 #include "SkTemplates.h"
+#include "hb-ot.h"
+#include "hb.h"
 #include "platform/LayoutUnit.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #include "platform/fonts/Character.h"
@@ -35,6 +37,7 @@
 #include "platform/fonts/GlyphBuffer.h"
 #include "platform/fonts/GlyphPageTreeNode.h"
 #include "platform/fonts/SimpleFontData.h"
+#include "platform/fonts/shaping/HarfBuzzFace.h"
 #include "platform/fonts/shaping/HarfBuzzShaper.h"
 #include "platform/fonts/shaping/SimpleShaper.h"
 #include "platform/geometry/FloatRect.h"
@@ -60,12 +63,16 @@ Font::Font()
 
 Font::Font(const FontDescription& fd)
     : m_fontDescription(fd)
+    , m_canShapeWordByWord(0)
+    , m_shapeWordByWordComputed(0)
 {
 }
 
 Font::Font(const Font& other)
     : m_fontDescription(other.m_fontDescription)
     , m_fontFallbackList(other.m_fontFallbackList)
+    , m_canShapeWordByWord(0)
+    , m_shapeWordByWordComputed(0)
 {
 }
 
@@ -73,6 +80,8 @@ Font& Font::operator=(const Font& other)
 {
     m_fontDescription = other.m_fontDescription;
     m_fontFallbackList = other.m_fontFallbackList;
+    m_canShapeWordByWord = other.m_canShapeWordByWord;
+    m_shapeWordByWordComputed = other.m_shapeWordByWordComputed;
     return *this;
 }
 
@@ -103,10 +112,21 @@ float Font::buildGlyphBuffer(const TextRunPaintInfo& runInfo, GlyphBuffer& glyph
     const GlyphData* emphasisData) const
 {
     if (codePath(runInfo) == ComplexPath) {
-        HarfBuzzShaper shaper(this, runInfo.run, emphasisData);
-        shaper.setDrawRange(runInfo.from, runInfo.to);
-        shaper.shape(&glyphBuffer);
-        return shaper.totalWidth();
+        float width;
+        if (emphasisData) {
+            // FIXME: Populate using CachingWordShaper.

[drott, 2015/06/23 12:45:51]

IIRC, in a couple of reviews dpranke asked me to never leave FIXME's dangling
without corresponding bugs, which I find good practice. Could you file an issue
for this issue of emphasis glyphbuffer filling and mark it as blocking 462643?

On the issue itself, I've been thinking about what it would take to solve this:
I think the problem is that we lose the original word/string, right? For
emphasis glyphBuffer filling we need the shapeResult, especially the
glyphToCharacterIndex() method, which is based on the
m_glyphData.characterIndex, but also the original string to do
countGraphemesInCluster. So, the only way to solve this would be to somehow do 
countGraphemesInCluster over the required intervals - when we need emphasis
drawing - and then keep this information with our ShapeCacheEntry / ShapeResult.

+            HarfBuzzShaper shaper(this, runInfo.run);
+            RefPtr<ShapeResult> shapeResult = shaper.shapeResult();
+            shapeResult->fillGlyphBufferForTextEmphasis(&glyphBuffer, 0,
+                runInfo.run, emphasisData, runInfo.from, runInfo.to, 0);
+            width = shapeResult->width();
+        } else {
+            CachingWordShaper& shaper = m_fontFallbackList->cachingWordShaper();
+            width = shaper.fillGlyphBuffer(this, runInfo.run, nullptr,
+                &glyphBuffer, runInfo.from, runInfo.to);
+        }
+
+        return width;
     }
 
     SimpleShaper shaper(this, runInfo.run, emphasisData, nullptr /* fallbackFonts */, nullptr);
@@ -334,6 +354,70 @@ CodePath Font::codePath(const TextRunPaintInfo& runInfo) const
     return Character::characterRangeCodePath(run.characters16(), run.length());
 }
 
+static inline bool tableHasSpace(hb_face_t* face, hb_set_t* glyphs,
+    hb_tag_t tag, hb_codepoint_t space)
+{
+    unsigned count = hb_ot_layout_table_get_lookup_count(face, tag);
+    for (unsigned i = 0; i < count; i++) {
+        hb_ot_layout_lookup_collect_glyphs(face, tag, i, glyphs, glyphs, glyphs,
+            0);
+        if (hb_set_has(glyphs, space))
+            return true;
+    }
+    return false;
+}
+
+bool Font::canShapeWordByWord() const
+{
+    if (!m_shapeWordByWordComputed) {
+        m_canShapeWordByWord = computeCanShapeWordByWord();
+        m_shapeWordByWordComputed = true;
+    }
+    return m_canShapeWordByWord;
+};
+
+bool Font::computeCanShapeWordByWord() const
+{
+    if (!fontDescription().typesettingFeatures())
+        return true;
+
+    const FontPlatformData& platformData = primaryFont()->platformData();

[drott, 2015/06/23 12:45:51]

I'd suggest to move the code from below this line to the end of the method to
FontPlatformData and then call something like hasSpaceInLigaturesOrKerning() to
combine this with typeSettingFeatures() to have your local answer for
computeCanShapeWordByWord(). My reasoning would be that the code is more low
level, actually looks at the TTF blob and that it's actually operating on
FontPlatformData.

+    const HarfBuzzFace* hbFace = platformData.harfBuzzFace();
+    if (!hbFace)
+        return true;
+
+    hb_face_t* face = hbFace->face();
+    ASSERT(face);
+    hb_font_t* font = hbFace->createFont();
+    ASSERT(font);
+
+    hb_codepoint_t space;
+    // If the space glyph isn't present in the font then each space character
+    // will be rendering using a fallback font, which grantees that it cannot
+    // affect the shape of the preceding word.
+    if (!hb_font_get_glyph(font, spaceCharacter, 0, &space))
+        return true;
+
+    if (!hb_ot_layout_has_substitution(face)
+        && !hb_ot_layout_has_positioning(face)) {
+        return true;
+    }
+
+    hb_set_t* glyphs = hb_set_create();
+    TypesettingFeatures features = fontDescription().typesettingFeatures();
+
+    bool foundSpaceInTable = false;
+    if (features & Kerning)
+        foundSpaceInTable = tableHasSpace(face, glyphs, HB_OT_TAG_GPOS, space);
+    if (!foundSpaceInTable && (features & Ligatures))
+        foundSpaceInTable = tableHasSpace(face, glyphs, HB_OT_TAG_GSUB, space);
+
+    hb_set_destroy(glyphs);
+    hb_font_destroy(font);
+
+    return !foundSpaceInTable;
+};
+
 void Font::willUseFontData(UChar32 character) const
 {
     const FontFamily& family = fontDescription().family();
@@ -670,39 +754,11 @@ void Font::drawTextBlob(SkCanvas* canvas, const SkPaint& paint, const SkTextBlob
     canvas->drawTextBlob(blob, origin.x(), origin.y(), paint);
 }
 
-float Font::floatWidthForComplexText(const TextRun& run, HashSet<const SimpleFontData*>* fallbackFonts, FloatRect* outGlyphBounds) const
+float Font::floatWidthForComplexText(const TextRun& run, HashSet<const SimpleFontData*>* fallbackFonts, FloatRect* glyphBounds) const
 {
-    bool hasWordSpacingOrLetterSpacing = fontDescription().wordSpacing()
-        || fontDescription().letterSpacing();
-    // Word spacing and letter spacing can change the width of a word.
-    // If a tab occurs inside a word, the width of the word varies based on its
-    // position on the line.
-    bool isCacheable = !hasWordSpacingOrLetterSpacing && !run.allowTabs();
-
-    WidthCacheEntry* cacheEntry = isCacheable
-        ? m_fontFallbackList->widthCache().add(run, WidthCacheEntry())
-        : 0;
-    if (cacheEntry && cacheEntry->isValid()) {
-        if (outGlyphBounds)
-            *outGlyphBounds = cacheEntry->glyphBounds;
-        return cacheEntry->width;
-    }
-
-    FloatRect glyphBounds;
-    HarfBuzzShaper shaper(this, run, nullptr, fallbackFonts, &glyphBounds);
-    if (!shaper.shape())
-        return 0;
-
-    float result = shaper.totalWidth();
-
-    if (cacheEntry && (!fallbackFonts || fallbackFonts->isEmpty())) {
-        cacheEntry->glyphBounds = glyphBounds;
-        cacheEntry->width = result;
-    }
-
-    if (outGlyphBounds)
-        *outGlyphBounds = glyphBounds;
-    return result;
+    CachingWordShaper& shaper = m_fontFallbackList->cachingWordShaper();
+    float width = shaper.width(this, run, fallbackFonts, glyphBounds);
+    return width;
 }
 
 // Return the code point index for the given |x| offset into the text run.
@@ -710,9 +766,10 @@ int Font::offsetForPositionForComplexText(const TextRun& run, float xFloat,
     bool includePartialGlyphs) const
 {
     HarfBuzzShaper shaper(this, run);
-    if (!shaper.shape())
+    RefPtr<ShapeResult> shapeResult = shaper.shapeResult();
+    if (!shapeResult)
         return 0;
-    return shaper.offsetForPosition(xFloat);
+    return shapeResult->offsetForPosition(xFloat);
 }
 
 // Return the rectangle for selecting the given range of code-points in the TextRun.
@@ -720,9 +777,10 @@ FloatRect Font::selectionRectForComplexText(const TextRun& run,
     const FloatPoint& point, int height, int from, int to) const
 {
     HarfBuzzShaper shaper(this, run);
-    if (!shaper.shape())
+    RefPtr<ShapeResult> shapeResult = shaper.shapeResult();
+    if (!shapeResult)
         return FloatRect();
-    return shaper.selectionRect(point, height, from, to);
+    return shapeResult->selectionRect(point, height, from, to);
 }
 
 void Font::drawGlyphBuffer(SkCanvas* canvas, const SkPaint& paint, const TextRunPaintInfo& runInfo, const GlyphBuffer& glyphBuffer, const FloatPoint& point, float deviceScaleFactor) const