Bug2790521 - complex script overlap issue.

Applied the fixes for complex script overlap for webkit/Chrome to
webkit/Android.

Change-Id: I4e34e36230b832524d7b8e78406de1e4c8438cf1

1 files changed, 45 insertions, 48 deletions

diff --git a/WebCore/platform/graphics/android/FontAndroid.cpp b/WebCore/platform/graphics/android/FontAndroid.cpp
index 98a571b..725793b 100644
--- a/WebCore/platform/graphics/android/FontAndroid.cpp
+++ b/WebCore/platform/graphics/android/FontAndroid.cpp
@@ -314,10 +314,9 @@ public:
         memset(&m_item, 0, sizeof(m_item));
         // We cannot know, ahead of time, how many glyphs a given script run
         // will produce. We take a guess that script runs will not produce more
-        // than twice as many glyphs as there are code points and fallback if
-        // we find that we are wrong.
-        m_maxGlyphs = m_run.length() * 2;
-        createGlyphArrays();
+        // than twice as many glyphs as there are code points plus a bit of
+        // padding and fallback if we find that we are wrong.
+        createGlyphArrays((m_run.length() + 2) * 2);
 
         m_item.log_clusters = new unsigned short[m_run.length()];
 
@@ -383,12 +382,29 @@ public:
             if (!hb_utf16_script_run_next(&m_numCodePoints, &m_item.item,
                 m_run.characters(), m_run.length(), &m_indexOfNextScriptRun))
                 return false;
+
+            // It is actually wrong to consider script runs at all in this code.
+            // Other WebKit code (e.g. Mac) segments complex text just by finding
+            // the longest span of text covered by a single font.
+            // But we currently need to call hb_utf16_script_run_next anyway to fill
+            // in the harfbuzz data structures to e.g. pick the correct script's shaper.
+            // So we allow that to run first, then do a second pass over the range it
+            // found and take the largest subregion that stays within a single font.
+            const FontData* glyphData = m_font->glyphDataForCharacter(
+                m_item.string[m_item.item.pos], false, false).fontData;
+            int endOfRun;
+            for (endOfRun = 1; endOfRun < static_cast<int>(m_item.item.length); ++endOfRun) {
+                const FontData* nextGlyphData = m_font->glyphDataForCharacter(
+                    m_item.string[m_item.item.pos + endOfRun], false, false).fontData;
+                if (nextGlyphData != glyphData)
+                    break;
+            }
+            m_item.item.length = endOfRun;
+            m_indexOfNextScriptRun = m_item.item.pos + endOfRun;
         }
 
         setupFontForScriptRun();
-
-        if (!shapeGlyphs())
-            return false;
+        shapeGlyphs();
         setGlyphXPositions(rtl());
         return true;
     }
@@ -508,11 +524,8 @@ private:
 
     void setupFontForScriptRun()
     {
-        const FontData* fontData = m_font->fontDataAt(0);
-        if (!fontData->containsCharacters(m_item.string + m_item.item.pos,
-                                          m_item.item.length))
-            fontData = m_font->fontDataForCharacters(
-                       m_item.string + m_item.item.pos, m_item.item.length);
+        const FontData* fontData = m_font->glyphDataForCharacter(
+            m_item.string[m_item.item.pos], false, false).fontData;
         const FontPlatformData& platformData =
             fontData->fontDataForCharacter(' ')->platformData();
         m_item.face = platformData.harfbuzzFace();
@@ -548,46 +561,31 @@ private:
         delete[] m_xPositions;
     }
 
-    bool createGlyphArrays()
+    void createGlyphArrays(int size)
     {
-        m_item.glyphs = new HB_Glyph[m_maxGlyphs];
-        m_item.attributes = new HB_GlyphAttributes[m_maxGlyphs];
-        m_item.advances = new HB_Fixed[m_maxGlyphs];
-        m_item.offsets = new HB_FixedPoint[m_maxGlyphs];
-        // HB_FixedPoint is a struct, so we must use memset to clear it.
-        memset(m_item.offsets, 0, m_maxGlyphs * sizeof(HB_FixedPoint));
-        m_glyphs16 = new uint16_t[m_maxGlyphs];
-        m_xPositions = new SkScalar[m_maxGlyphs];
-
-        return m_item.glyphs
-            && m_item.attributes
-            && m_item.advances
-            && m_item.offsets
-            && m_glyphs16
-            && m_xPositions;
-    }
-
-    bool expandGlyphArrays()
-    {
-        deleteGlyphArrays();
-        m_maxGlyphs <<= 1;
-        return createGlyphArrays();
+        m_item.glyphs = new HB_Glyph[size];
+        memset(m_item.glyphs, 0, size * sizeof(HB_Glyph));
+        m_item.attributes = new HB_GlyphAttributes[size];
+        memset(m_item.attributes, 0, size * sizeof(HB_GlyphAttributes));
+        m_item.advances = new HB_Fixed[size];
+        memset(m_item.advances, 0, size * sizeof(HB_Fixed));
+        m_item.offsets = new HB_FixedPoint[size];
+        memset(m_item.offsets, 0, size * sizeof(HB_FixedPoint));
+
+        m_glyphs16 = new uint16_t[size];
+        m_xPositions = new SkScalar[size];
+
+        m_item.num_glyphs = size;
     }
 
-    bool shapeGlyphs()
+    void shapeGlyphs()
     {
-        for (;;) {
-            m_item.num_glyphs = m_maxGlyphs;
-            HB_ShapeItem(&m_item);
-            if (m_item.num_glyphs < m_maxGlyphs)
-                break;
-
+        while (!HB_ShapeItem(&m_item)) {
             // We overflowed our arrays. Resize and retry.
-            if (!expandGlyphArrays())
-                return false;
+            // HB_ShapeItem fills in m_item.num_glyphs with the needed size.
+            deleteGlyphArrays();
+            createGlyphArrays(m_item.num_glyphs);
         }
-
-        return true;
     }
 
     void setGlyphXPositions(bool isRTL)
@@ -618,7 +616,6 @@ private:
     unsigned m_offsetX; // Offset in pixels to the start of the next script run.
     unsigned m_pixelWidth; // Width (in px) of the current script run.
     unsigned m_numCodePoints; // Code points in current script run.
-    unsigned m_maxGlyphs; // Current size of all the Harfbuzz arrays.
 
     OwnPtr<TextRun> m_normalizedRun;
     OwnArrayPtr<UChar> m_normalizedBuffer; // A buffer for normalized run.
@@ -799,7 +796,7 @@ int Font::offsetForPositionForComplexText(const TextRun& run, int x,
         if (walker.rtl())
             basePosition -= walker.numCodePoints();
 
-        if (x < static_cast<int>(walker.width())) {
+        if (x >= 0 && x < static_cast<int>(walker.width())) {
             // The x value in question is within this script run. We consider
             // each glyph in presentation order and stop when we find the one
             // covering this position.