Do not skip ink for ideographic scripts

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2003, 2006, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (c) 2007, 2008, 2010 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 239 matching lines @@
   STACK_ALLOCATED()
  public:
   GlyphBufferBloberizer(const GlyphBuffer& buffer,
                         const Font* font,
                         float deviceScaleFactor)
       : m_buffer(buffer),
         m_font(font),
         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);
 
       const BlobRotation newRotation = computeBlobRotation(fontData);
       if (newRotation != m_rotation) {
         // We're switching to an orientation which requires a different rotation
         //   => emit the pending blob (and start a new one with the new
         //      rotation).
         m_rotation = newRotation;
         break;
       }
 
       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);
     }
 
     m_blobCount++;
     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
     // rotation (using a 90deg CCW rotation).
     return (font->platformData().isVerticalAnyUpright() && font->verticalData())
                ? CCWRotation
                : NoRotation;
   }
 
   void appendRun(unsigned start,
@@ skipping 30 matching lines @@
     }
   }
 
   const GlyphBuffer& m_buffer;
   const Font* m_font;
   const float m_deviceScaleFactor;
   const bool m_hasVerticalOffsets;
 
   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
 
 void Font::drawGlyphBuffer(SkCanvas* canvas,
                            const SkPaint& paint,
                            const TextRunPaintInfo& runInfo,
                            const GlyphBuffer& glyphBuffer,
                            const FloatPoint& point,
                            float deviceScaleFactor) const {
@@ skipping 26 matching lines @@
     *runInfo.cachedTextBlob = std::move(blob.first);
     ASSERT(*runInfo.cachedTextBlob);
   }
 }
 
 static int getInterceptsFromBloberizer(const GlyphBuffer& glyphBuffer,
                                        const Font* font,
                                        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;
   while (!bloberizer.done()) {
     blob = bloberizer.next();
     DCHECK(blob.first);
 
     // GlyphBufferBloberizer splits for a new blob rotation, but does not split
     // for a change in font. A TextBlob can contain runs with differing fonts
     // and the getTextBlobIntercepts method handles multiple fonts for us. For
@@ skipping 25 matching lines @@
     if (!numIntervals)
       return;
     DCHECK_EQ(numIntervals % 2, 0);
     intercepts.resize(numIntervals / 2);
     paint.getTextBlobIntercepts(runInfo.cachedTextBlob->get(), boundsArray,
                                 reinterpret_cast<SkScalar*>(intercepts.data()));
     return;
   }
 
   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()));
 }
 
 static inline FloatRect pixelSnappedSelectionRect(FloatRect rect) {
   // Using roundf() rather than ceilf() for the right edge as a compromise to
   // ensure correct caret positioning.
   float roundedX = roundf(rect.x());
   return FloatRect(roundedX, rect.y(), roundf(rect.maxX() - roundedX),
                    rect.height());
 }
@@ skipping 154 matching lines @@
 
 bool Font::loadingCustomFonts() const {
   return m_fontFallbackList && m_fontFallbackList->loadingCustomFonts();
 }
 
 bool Font::isFallbackValid() const {
   return !m_fontFallbackList || m_fontFallbackList->isValid();
 }
 
 }  // namespace blink