Do not skip ink for ideographic scripts

third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBuffer.cpp

Index: third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBuffer.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBuffer.cpp b/third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBuffer.cpp
index c75d18caeddd36d9e9f498ab54c945c3764b35e3..23b55ef9ff608b4d92cc8c732adb875c052953d7 100644
--- a/third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBuffer.cpp
+++ b/third_party/WebKit/Source/platform/fonts/shaping/ShapeResultBuffer.cpp
@@ -17,15 +17,63 @@ namespace blink {
 
 namespace {
 
+inline void addIsSkipInkException(GlyphBuffer* glyphBuffer,
+                                  const TextRun& run,
+                                  unsigned characterIndex) {
+  DCHECK(!run.is8Bit()) << "8Bit() is always false, better to avoid to call";
+  UChar32 baseCharacter = run.codepointAt(characterIndex);
+  UBlockCode blockCode = ublock_getCode(baseCharacter);

[drott, 2017/01/02 15:07:46]

I'd very much prefer if we could reuse the isCJKIdeographOrSymbol function
instead of defining another class. Is that possible? If not, at least the
test/code block list should move to Character.h

+  switch (blockCode) {
+    case UBLOCK_BOPOMOFO:
+    case UBLOCK_BOPOMOFO_EXTENDED:
+    case UBLOCK_CJK_COMPATIBILITY:
+    case UBLOCK_CJK_COMPATIBILITY_FORMS:
+    case UBLOCK_CJK_COMPATIBILITY_IDEOGRAPHS:
+    case UBLOCK_CJK_COMPATIBILITY_IDEOGRAPHS_SUPPLEMENT:
+    case UBLOCK_CJK_RADICALS_SUPPLEMENT:
+    case UBLOCK_CJK_STROKES:
+    case UBLOCK_CJK_SYMBOLS_AND_PUNCTUATION:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_A:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_B:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_C:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_D:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_E:
+    case UBLOCK_ENCLOSED_CJK_LETTERS_AND_MONTHS:
+    case UBLOCK_ENCLOSED_IDEOGRAPHIC_SUPPLEMENT:
+    case UBLOCK_HANGUL_COMPATIBILITY_JAMO:
+    case UBLOCK_HANGUL_JAMO:
+    case UBLOCK_HANGUL_JAMO_EXTENDED_A:
+    case UBLOCK_HANGUL_JAMO_EXTENDED_B:
+    case UBLOCK_HANGUL_SYLLABLES:
+    case UBLOCK_HIRAGANA:
+    case UBLOCK_IDEOGRAPHIC_DESCRIPTION_CHARACTERS:
+    case UBLOCK_IDEOGRAPHIC_SYMBOLS_AND_PUNCTUATION:
+    case UBLOCK_KATAKANA:
+    case UBLOCK_LINEAR_B_IDEOGRAMS:
+    case UBLOCK_TANGUT:
+    case UBLOCK_TANGUT_COMPONENTS:
+      glyphBuffer->addIsSkipInkException(true);
+      break;
+    default:
+      glyphBuffer->addIsSkipInkException(false);
+      break;
+  }
+}
+
 inline void addGlyphToBuffer(GlyphBuffer* glyphBuffer,
                              float advance,
                              hb_direction_t direction,
                              const SimpleFontData* fontData,
-                             const HarfBuzzRunGlyphData& glyphData) {
+                             const HarfBuzzRunGlyphData& glyphData,
+                             const TextRun& run,
+                             unsigned characterIndex) {
   FloatPoint startOffset = HB_DIRECTION_IS_HORIZONTAL(direction)
                                ? FloatPoint(advance, 0)
                                : FloatPoint(0, advance);
   glyphBuffer->add(glyphData.glyph, fontData, startOffset + glyphData.offset);
+  if (glyphBuffer->shouldSaveSkipInkExceptions() && !run.is8Bit())
+    addIsSkipInkException(glyphBuffer, run, characterIndex);
 }
 
 inline void addEmphasisMark(GlyphBuffer* buffer,
@@ -78,6 +126,7 @@ inline unsigned countGraphemesInCluster(const UChar* str,
 template <TextDirection direction>
 float ShapeResultBuffer::fillGlyphBufferForRun(GlyphBuffer* glyphBuffer,
                                                const ShapeResult::RunInfo* run,
+                                               const TextRun& textRun,
                                                float initialAdvance,
                                                unsigned from,
                                                unsigned to,
@@ -98,7 +147,8 @@ float ShapeResultBuffer::fillGlyphBufferForRun(GlyphBuffer* glyphBuffer,
                (direction == TextDirection::Ltr &&
                 currentCharacterIndex < to)) {
       addGlyphToBuffer(glyphBuffer, advanceSoFar, run->m_direction,
-                       run->m_fontData.get(), glyphData);
+                       run->m_fontData.get(), glyphData, textRun,
+                       currentCharacterIndex);
       advanceSoFar += glyphData.advance;
     }
   }
@@ -197,13 +247,14 @@ float ShapeResultBuffer::fillGlyphBufferForTextEmphasisRun(
 
 float ShapeResultBuffer::fillFastHorizontalGlyphBuffer(
     GlyphBuffer* glyphBuffer,
-    TextDirection dir) const {
+    const TextRun& textRun) const {
   ASSERT(!hasVerticalOffsets());
 
   float advance = 0;
 
+  unsigned characterIndex = 0;
   for (unsigned i = 0; i < m_results.size(); ++i) {
-    const auto& wordResult = isLeftToRightDirection(dir)
+    const auto& wordResult = isLeftToRightDirection(textRun.direction())
                                  ? m_results[i]
                                  : m_results[m_results.size() - 1 - i];
     ASSERT(!wordResult->hasVerticalOffsets());
@@ -217,9 +268,15 @@ float ShapeResultBuffer::fillFastHorizontalGlyphBuffer(
 
         glyphBuffer->add(glyphData.glyph, run->m_fontData.get(),
                          advance + glyphData.offset.width());
+        if (glyphBuffer->shouldSaveSkipInkExceptions() && !textRun.is8Bit()) {
+          addIsSkipInkException(glyphBuffer, textRun,
+                                characterIndex + glyphData.characterIndex);
+        }
+
         advance += glyphData.advance;
       }
     }
+    characterIndex += wordResult->m_numCharacters;
   }
 
   ASSERT(!glyphBuffer->hasVerticalOffsets());
@@ -233,7 +290,7 @@ float ShapeResultBuffer::fillGlyphBuffer(GlyphBuffer* glyphBuffer,
                                          unsigned to) const {
   // Fast path: full run with no vertical offsets
   if (!from && to == textRun.length() && !hasVerticalOffsets())
-    return fillFastHorizontalGlyphBuffer(glyphBuffer, textRun.direction());
+    return fillFastHorizontalGlyphBuffer(glyphBuffer, textRun);
 
   float advance = 0;
 
@@ -244,8 +301,8 @@ float ShapeResultBuffer::fillGlyphBuffer(GlyphBuffer* glyphBuffer,
       const RefPtr<const ShapeResult>& wordResult = m_results[resolvedIndex];
       for (unsigned i = 0; i < wordResult->m_runs.size(); i++) {
         advance += fillGlyphBufferForRun<TextDirection::Rtl>(
-            glyphBuffer, wordResult->m_runs[i].get(), advance, from, to,
-            wordOffset - wordResult->numCharacters());
+            glyphBuffer, wordResult->m_runs[i].get(), textRun, advance, from,
+            to, wordOffset - wordResult->numCharacters());
       }
       wordOffset -= wordResult->numCharacters();
     }
@@ -255,8 +312,8 @@ float ShapeResultBuffer::fillGlyphBuffer(GlyphBuffer* glyphBuffer,
       const RefPtr<const ShapeResult>& wordResult = m_results[j];
       for (unsigned i = 0; i < wordResult->m_runs.size(); i++) {
         advance += fillGlyphBufferForRun<TextDirection::Ltr>(
-            glyphBuffer, wordResult->m_runs[i].get(), advance, from, to,
-            wordOffset);
+            glyphBuffer, wordResult->m_runs[i].get(), textRun, advance, from,
+            to, wordOffset);
       }
       wordOffset += wordResult->numCharacters();
     }