Do not skip ink for ideographic scripts

third_party/WebKit/Source/platform/fonts/Font.cpp

Index: third_party/WebKit/Source/platform/fonts/Font.cpp
diff --git a/third_party/WebKit/Source/platform/fonts/Font.cpp b/third_party/WebKit/Source/platform/fonts/Font.cpp
index 838080911c8a562cb2cf2ca0061863304cbaf698..a35ca12ce40fdd56713e9b21a0b88e91fa860e54 100644
--- a/third_party/WebKit/Source/platform/fonts/Font.cpp
+++ b/third_party/WebKit/Source/platform/fonts/Font.cpp
@@ -257,18 +257,25 @@ class GlyphBufferBloberizer {
         m_deviceScaleFactor(deviceScaleFactor),
         m_hasVerticalOffsets(buffer.hasVerticalOffsets()),
         m_index(0),
+        m_endIndex(m_buffer.size()),
         m_blobCount(0),
         m_rotation(buffer.isEmpty() ? NoRotation : computeBlobRotation(
-                                                       buffer.fontDataAt(0))) {}
+                                                       buffer.fontDataAt(0))),
+        m_skipGlyphs(nullptr) {}
 
-  bool done() const { return m_index >= m_buffer.size(); }
+  bool done() const { return m_index >= m_endIndex; }
   unsigned blobCount() const { return m_blobCount; }
 
   std::pair<sk_sp<SkTextBlob>, BlobRotation> next() {
     ASSERT(!done());
     const BlobRotation currentRotation = m_rotation;
 
-    while (m_index < m_buffer.size()) {
+    while (m_index < m_endIndex) {
+      if (m_skipGlyphs) {
+        while (m_index < m_endIndex && (*m_skipGlyphs)[m_index])
+          m_index++;
+      }
+
       const SimpleFontData* fontData = m_buffer.fontDataAt(m_index);
       ASSERT(fontData);
 
@@ -282,8 +289,8 @@ class GlyphBufferBloberizer {
       }
 
       const unsigned start = m_index++;
-      while (m_index < m_buffer.size() &&
-             m_buffer.fontDataAt(m_index) == fontData)
+      while (m_index < m_endIndex && m_buffer.fontDataAt(m_index) == fontData &&
+             !(m_skipGlyphs && (*m_skipGlyphs)[m_index]))
         m_index++;
 
       appendRun(start, m_index - start, fontData);
@@ -293,6 +300,15 @@ class GlyphBufferBloberizer {
     return std::make_pair(m_builder.make(), currentRotation);
   }
 
+  void setSkipGlyphs(const Vector<bool>* skipGlyphs) {

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

As mentioned below, I'd prefer if we use the glyphBuffer's
shouldSaveSkipInkExceptions() check, remove this function and do the backwards
running skip checks in the constructor.

+    m_skipGlyphs = skipGlyphs;
+    m_endIndex = m_buffer.size();
+    if (m_skipGlyphs) {
+      while (m_endIndex > 0 && (*m_skipGlyphs)[m_endIndex - 1])
+        m_endIndex--;
+    }
+  }
+
  private:
   static BlobRotation computeBlobRotation(const SimpleFontData* font) {
     // For vertical upright text we need to compensate the inherited 90deg CW
@@ -343,8 +359,10 @@ class GlyphBufferBloberizer {
 
   SkTextBlobBuilder m_builder;
   unsigned m_index;
+  unsigned m_endIndex;
   unsigned m_blobCount;
   BlobRotation m_rotation;
+  const Vector<bool>* m_skipGlyphs;

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

I believe we can use this from m_buffer directly?

 };
 
 }  // anonymous namespace
@@ -391,9 +409,11 @@ static int getInterceptsFromBloberizer(const GlyphBuffer& glyphBuffer,
                                        const SkPaint& paint,
                                        float deviceScaleFactor,
                                        const std::tuple<float, float>& bounds,
+                                       const Vector<bool>* skipGlyphs,

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

See below, I think we can remove this argument...

                                        SkScalar* interceptsBuffer) {
   SkScalar boundsArray[2] = {std::get<0>(bounds), std::get<1>(bounds)};
   GlyphBufferBloberizer bloberizer(glyphBuffer, font, deviceScaleFactor);
+  bloberizer.setSkipGlyphs(skipGlyphs);

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

and this explicit transfer of the skipExceptions vector.

   std::pair<sk_sp<SkTextBlob>, BlobRotation> blob;
 
   int numIntervals = 0;
@@ -439,20 +459,28 @@ void Font::getTextIntercepts(const TextRunPaintInfo& runInfo,
   }
 
   GlyphBuffer glyphBuffer;
-  buildGlyphBuffer(runInfo, glyphBuffer);
+  const Vector<bool>* skipGlyphs = nullptr;
+  if (runInfo.run.is8Bit()) {
+    // All characters in Latin-1 should skip inks, so no skipInkExceptions.
+    buildGlyphBuffer(runInfo, glyphBuffer);
+  } else {
+    glyphBuffer.saveSkipInkExceptions();
+    buildGlyphBuffer(runInfo, glyphBuffer);
+    skipGlyphs = &glyphBuffer.skipInkExceptions();

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

Using state in the GlyphBuffer for shouldSaveSkipInkExceptions() is okay for me,
but then can we use it here as well? AFAICT, we don't use this exact same glyph
buffer for anything else except computing the intercepts, so in the bloberizer,
we can ask the GlyphBuffer for shouldSaveSkipInkExceptions() and handle it
accordingly there, instead of passing the skipGlyphs vector pointer around here.

+  }
 
   // Get the number of intervals, without copying the actual values by
   // specifying nullptr for the buffer, following the Skia allocation model for
   // retrieving text intercepts.
   int numIntervals = getInterceptsFromBloberizer(
-      glyphBuffer, this, paint, deviceScaleFactor, bounds, nullptr);
+      glyphBuffer, this, paint, deviceScaleFactor, bounds, skipGlyphs, nullptr);
   if (!numIntervals)
     return;
   DCHECK_EQ(numIntervals % 2, 0);
   intercepts.resize(numIntervals / 2);
 
   getInterceptsFromBloberizer(glyphBuffer, this, paint, deviceScaleFactor,
-                              bounds,
+                              bounds, skipGlyphs,
                               reinterpret_cast<SkScalar*>(intercepts.data()));
 }