Implement CSS Emphasis Marks for complex text

Source/platform/fonts/harfbuzz/HarfBuzzShaper.cpp

Index: Source/platform/fonts/harfbuzz/HarfBuzzShaper.cpp
diff --git a/Source/platform/fonts/harfbuzz/HarfBuzzShaper.cpp b/Source/platform/fonts/harfbuzz/HarfBuzzShaper.cpp
index 13ac6f80ffa7ef644d2849f29d95ca0854a8bf42..7dc2eda0b8162eafbf6d466cd08158344010d0af 100644
--- a/Source/platform/fonts/harfbuzz/HarfBuzzShaper.cpp
+++ b/Source/platform/fonts/harfbuzz/HarfBuzzShaper.cpp
@@ -348,7 +348,7 @@ static void normalizeCharacters(const TextRun& run, unsigned length, UChar* dest
     }
 }
 
-HarfBuzzShaper::HarfBuzzShaper(const Font* font, const TextRun& run)
+HarfBuzzShaper::HarfBuzzShaper(const Font* font, const TextRun& run, bool forTextEmphasis)
     : m_font(font)
     , m_normalizedBufferLength(0)
     , m_run(run)
@@ -359,6 +359,7 @@ HarfBuzzShaper::HarfBuzzShaper(const Font* font, const TextRun& run)
     , m_letterSpacing(font->letterSpacing())
     , m_fromIndex(0)
     , m_toIndex(m_run.length())
+    , m_forTextEmphasis(forTextEmphasis)
 {
     m_normalizedBuffer = adoptArrayPtr(new UChar[m_run.length() + 1]);
     normalizeCharacters(m_run, m_run.length(), m_normalizedBuffer.get(), &m_normalizedBufferLength);
@@ -674,7 +675,7 @@ bool HarfBuzzShaper::shapeHarfBuzzRuns()
         const UChar* src = m_normalizedBuffer.get() + currentRun->startIndex();
         std::wstring key(src, src + currentRun->numCharacters());
 
-        CachedShapingResults* cachedResults = runCache.find(key);
+        CachedShapingResults* cachedResults = m_forTextEmphasis ? 0 : runCache.find(key);
         if (cachedResults) {
             if (cachedResults->dir == props.direction && cachedResults->font == *m_font) {
                 currentRun->applyShapeResult(cachedResults->buffer);
@@ -713,7 +714,8 @@ bool HarfBuzzShaper::shapeHarfBuzzRuns()
         currentRun->applyShapeResult(harfBuzzBuffer.get());
         setGlyphPositionsForHarfBuzzRun(currentRun, harfBuzzBuffer.get());
 
-        runCache.insert(key, new CachedShapingResults(harfBuzzBuffer.get(), m_font, props.direction));
+        if (!m_forTextEmphasis)
+            runCache.insert(key, new CachedShapingResults(harfBuzzBuffer.get(), m_font, props.direction));
 
         harfBuzzBuffer.set(hb_buffer_create());
         hb_buffer_set_unicode_funcs(harfBuzzBuffer.get(), hb_icu_get_unicode_funcs());
@@ -787,16 +789,26 @@ void HarfBuzzShaper::fillGlyphBufferFromHarfBuzzRun(GlyphBuffer* glyphBuffer, Ha
         FloatPoint& nextOffset = (i == numGlyphs - 1) ? firstOffsetOfNextRun : offsets[i + 1];
         float glyphAdvanceX = advances[i] + nextOffset.x() - currentOffset.x();
         float glyphAdvanceY = nextOffset.y() - currentOffset.y();
+
+        Glyph glyphToAdd = glyphs[i];
+        UChar ch = m_run[currentCharacterIndex];
+        if (m_forTextEmphasis)
+            printf("Glyph: 0x%x, Character: 0x%x\n", glyphToAdd, ch);

[Dominik Röttsches, 2014/01/16 16:37:32]

When running this against:
http://roettsch.es/emphasis_simple.html
(just a simple กำ with CSS emphasis) I get currently get three emphasis marks,
but there should be only one for this cluster.

I get 
Glyph: 0xe3, Character: 0xe01
Glyph: 0x12c, Character: 0xe01
Glyph: 0x114, Character: 0xe01

The way this works on mac, for each run, it goes through the list of glyphs,
get's the character for each glyph and checks whether this matches the
U_GC_M_MASK - returns an empty glyph if it does. Which leads to no emphasis mark
being drawn for that glyph index.

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

The output on Mac is:
Glyph: 0x4ab, Character: 0xe01

In my patch, I get three emphasis marks, on the mac I get one.

The CSS Text Decoration Module says that there should be one emphasis mark per
character, which in this spec is a synonym for a grapheme cluster.

Behdad - would you have an idea how I can filter here? Would I need to scan
backwards and forwards through glyphToCharacterIndexes here to see whether the
glyph belongs to the same grapheme cluster, or are there better ways to figure
this out?

+
+        // FIXME: Combining marks should receive a text emphasis mark if they are combine with a space.
+        if (m_forTextEmphasis && (!Font::canReceiveTextEmphasis(ch) || (U_GET_GC_MASK(ch) & U_GC_M_MASK)))
+            glyphToAdd = 0;
+
         if (m_run.rtl()) {
             if (currentCharacterIndex >= m_toIndex)
                 m_startOffset.move(glyphAdvanceX, glyphAdvanceY);
             else if (currentCharacterIndex >= m_fromIndex)
-                glyphBuffer->add(glyphs[i], currentRun->fontData(), createGlyphBufferAdvance(glyphAdvanceX, glyphAdvanceY));
+                glyphBuffer->add(glyphToAdd, currentRun->fontData(), createGlyphBufferAdvance(glyphAdvanceX, glyphAdvanceY));
         } else {
             if (currentCharacterIndex < m_fromIndex)
                 m_startOffset.move(glyphAdvanceX, glyphAdvanceY);
             else if (currentCharacterIndex < m_toIndex)
-                glyphBuffer->add(glyphs[i], currentRun->fontData(), createGlyphBufferAdvance(glyphAdvanceX, glyphAdvanceY));
+                glyphBuffer->add(glyphToAdd, currentRun->fontData(), createGlyphBufferAdvance(glyphAdvanceX, glyphAdvanceY));
         }
     }
 }