Font’s fast code path doesn’t handle partial runs correctly when kerning or ligatures are enabled

https://bugs.webkit.org/show_bug.cgi?id=100050

Not reviewed for WebKit.

Always let WidthIterator iterate over an entire TextRun to avoid problems
with pixel rounding or shaping on partial runs.

This fix is necessary for Qt because the complex font-path can not disable
shaping, leading to the complex path painting slighly different from the
fast path, which messes up selection painting.

No change in functionality, no new tests.

* platform/graphics/Font.cpp:
(WebCore::Font::drawText):
(WebCore::Font::drawEmphasisMarks):
(WebCore::Font::selectionRectForText):
(WebCore::Font::offsetForPosition):
* platform/graphics/FontFastPath.cpp:
(WebCore::Font::getGlyphsAndAdvancesForSimpleText):
(WebCore::Font::selectionRectForSimpleText):
(WebCore::Font::offsetForPositionForSimpleText):
* platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBuffer::add):
(GlyphBuffer):
* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::advanceInternal):
* platform/graphics/WidthIterator.h:
(WidthIterator): Removed now unused advanceOneCharacter method.

Change-Id: I3647de93774bf2502ee37c7eb6d623790a691b42
Reviewed-by: Jocelyn Turcotte <jocelyn.turcotte@digia.com>

5 files changed, 81 insertions, 80 deletions

diff --git a/Source/WebCore/platform/graphics/Font.cpp b/Source/WebCore/platform/graphics/Font.cpp
index f55725e67..42dd14a6b 100644
--- a/Source/WebCore/platform/graphics/Font.cpp
+++ b/Source/WebCore/platform/graphics/Font.cpp
@@ -166,12 +166,7 @@ void Font::drawText(GraphicsContext* context, const TextRun& run, const FloatPoi
     
     to = (to == -1 ? run.length() : to);
 
-    CodePath codePathToUse = codePath(run);
-    // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
-    if (codePathToUse != Complex && typesettingFeatures() && (from || to != run.length()))
-        codePathToUse = Complex;
-
-    if (codePathToUse != Complex)
+    if (codePath(run) != Complex)
         return drawSimpleText(context, run, point, from, to);
 
     return drawComplexText(context, run, point, from, to);
@@ -185,12 +180,7 @@ void Font::drawEmphasisMarks(GraphicsContext* context, const TextRun& run, const
     if (to < 0)
         to = run.length();
 
-    CodePath codePathToUse = codePath(run);
-    // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
-    if (codePathToUse != Complex && typesettingFeatures() && (from || to != run.length()))
-        codePathToUse = Complex;
-
-    if (codePathToUse != Complex)
+    if (codePath(run) != Complex)
         drawEmphasisMarksForSimpleText(context, run, mark, point, from, to);
     else
         drawEmphasisMarksForComplexText(context, run, mark, point, from, to);
@@ -260,12 +250,7 @@ FloatRect Font::selectionRectForText(const TextRun& run, const FloatPoint& point
 {
     to = (to == -1 ? run.length() : to);
 
-    CodePath codePathToUse = codePath(run);
-    // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
-    if (codePathToUse != Complex && typesettingFeatures() && (from || to != run.length()))
-        codePathToUse = Complex;
-
-    if (codePathToUse != Complex)
+    if (codePath(run) != Complex)
         return selectionRectForSimpleText(run, point, h, from, to);
 
     return selectionRectForComplexText(run, point, h, from, to);
@@ -273,8 +258,7 @@ FloatRect Font::selectionRectForText(const TextRun& run, const FloatPoint& point
 
 int Font::offsetForPosition(const TextRun& run, float x, bool includePartialGlyphs) const
 {
-    // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
-    if (codePath(run) != Complex && !typesettingFeatures())
+    if (codePath(run) != Complex)
         return offsetForPositionForSimpleText(run, x, includePartialGlyphs);
 
     return offsetForPositionForComplexText(run, x, includePartialGlyphs);
diff --git a/Source/WebCore/platform/graphics/FontFastPath.cpp b/Source/WebCore/platform/graphics/FontFastPath.cpp
index 7ed7a9703..22ce7f57e 100644
--- a/Source/WebCore/platform/graphics/FontFastPath.cpp
+++ b/Source/WebCore/platform/graphics/FontFastPath.cpp
@@ -325,32 +325,34 @@ int Font::emphasisMarkHeight(const AtomicString& mark) const
 
 float Font::getGlyphsAndAdvancesForSimpleText(const TextRun& run, int from, int to, GlyphBuffer& glyphBuffer, ForTextEmphasisOrNot forTextEmphasis) const
 {
-    float initialAdvance;
-
     WidthIterator it(this, run, 0, false, forTextEmphasis);
-    // FIXME: Using separate glyph buffers for the prefix and the suffix is incorrect when kerning or
-    // ligatures are enabled.
     GlyphBuffer localGlyphBuffer;
-    it.advance(from, &localGlyphBuffer);
-    float beforeWidth = it.m_runWidthSoFar;
-    it.advance(to, &glyphBuffer);
+    it.advance(run.length(), &localGlyphBuffer);
 
-    if (glyphBuffer.isEmpty())
+    if (localGlyphBuffer.isEmpty())
         return 0;
 
-    float afterWidth = it.m_runWidthSoFar;
+    float totalWidth = it.m_runWidthSoFar;
+    float beforeWidth = 0;
+    int glyphPos = 0;
+    for (; glyphPos < localGlyphBuffer.size() && it.m_characterIndex[glyphPos] < from; ++glyphPos)
+        beforeWidth += localGlyphBuffer.advanceAt(glyphPos);
+    int glyphFrom = glyphPos;
 
-    if (run.rtl()) {
-        float finalRoundingWidth = it.m_finalRoundingWidth;
-        it.advance(run.length(), &localGlyphBuffer);
-        initialAdvance = finalRoundingWidth + it.m_runWidthSoFar - afterWidth;
-    } else
-        initialAdvance = beforeWidth;
+    float afterWidth = totalWidth;
+    glyphPos = localGlyphBuffer.size() - 1;
+    for (; glyphPos >= glyphFrom && it.m_characterIndex[glyphPos] >= to; --glyphPos)
+        afterWidth -= localGlyphBuffer.advanceAt(glyphPos);
+    int glyphTo = glyphPos + 1;
+
+    glyphBuffer.add(&localGlyphBuffer, glyphFrom, glyphTo - glyphFrom);
 
-    if (run.rtl())
+    if (run.rtl()) {
         glyphBuffer.reverse(0, glyphBuffer.size());
+        return totalWidth - afterWidth;
+    }
 
-    return initialAdvance;
+    return beforeWidth;
 }
 
 void Font::drawSimpleText(GraphicsContext* context, const TextRun& run, const FloatPoint& point, int from, int to) const
@@ -487,15 +489,22 @@ FloatRect Font::selectionRectForSimpleText(const TextRun& run, const FloatPoint&
 {
     GlyphBuffer glyphBuffer;
     WidthIterator it(this, run);
-    it.advance(from, &glyphBuffer);
-    float beforeWidth = it.m_runWidthSoFar;
-    it.advance(to, &glyphBuffer);
-    float afterWidth = it.m_runWidthSoFar;
+    it.advance(run.length(), &glyphBuffer);
+
+    float totalWidth = it.m_runWidthSoFar;
+    float beforeWidth = 0;
+    int glyphPos = 0;
+    for (; glyphPos < glyphBuffer.size() && it.m_characterIndex[glyphPos] < from; ++glyphPos)
+        beforeWidth += glyphBuffer.advanceAt(glyphPos);
+    int glyphFrom = glyphPos;
+
+    float afterWidth = totalWidth;
+    glyphPos = glyphBuffer.size() - 1;
+    for (; glyphPos >= glyphFrom && it.m_characterIndex[glyphPos] >= to; --glyphPos)
+        afterWidth -= glyphBuffer.advanceAt(glyphPos);
 
     // Using roundf() rather than ceilf() for the right edge as a compromise to ensure correct caret positioning.
     if (run.rtl()) {
-        it.advance(run.length(), &glyphBuffer);
-        float totalWidth = it.m_runWidthSoFar;
         return FloatRect(floorf(point.x() + totalWidth - afterWidth), point.y(), roundf(point.x() + totalWidth - beforeWidth) - floorf(point.x() + totalWidth - afterWidth), h);
     }
 
@@ -504,45 +513,50 @@ FloatRect Font::selectionRectForSimpleText(const TextRun& run, const FloatPoint&
 
 int Font::offsetForPositionForSimpleText(const TextRun& run, float x, bool includePartialGlyphs) const
 {
-    float delta = x;
-
+    GlyphBuffer glyphBuffer;
     WidthIterator it(this, run);
-    GlyphBuffer localGlyphBuffer;
-    unsigned offset;
+    it.advance(run.length(), &glyphBuffer);
+
+    int characterOffset = 0;
     if (run.rtl()) {
-        delta -= floatWidthForSimpleText(run);
-        while (1) {
-            offset = it.m_currentCharacter;
-            float w;
-            if (!it.advanceOneCharacter(w, localGlyphBuffer))
+        float currentX = it.m_runWidthSoFar;
+        for (int glyphPosition = 0; glyphPosition <= glyphBuffer.size(); ++glyphPosition) {
+            if (glyphPosition == glyphBuffer.size()) {
+                characterOffset = run.length();
                 break;
-            delta += w;
+            }
+            characterOffset = it.m_characterIndex[glyphPosition];
+            float glyphWidth = glyphBuffer.advanceAt(glyphPosition);
             if (includePartialGlyphs) {
-                if (delta - w / 2 >= 0)
+                if (currentX - glyphWidth / 2.0f <= x)
                     break;
             } else {
-                if (delta >= 0)
+                if (currentX - glyphWidth <= x)
                     break;
             }
+            currentX -= glyphWidth;
         }
     } else {
-        while (1) {
-            offset = it.m_currentCharacter;
-            float w;
-            if (!it.advanceOneCharacter(w, localGlyphBuffer))
+        float currentX = 0;
+        for (int glyphPosition = 0; glyphPosition <= glyphBuffer.size(); ++glyphPosition) {
+            if (glyphPosition == glyphBuffer.size()) {
+                characterOffset = run.length();
                 break;
-            delta -= w;
+            }
+            characterOffset = it.m_characterIndex[glyphPosition];
+            float glyphWidth = glyphBuffer.advanceAt(glyphPosition);
             if (includePartialGlyphs) {
-                if (delta + w / 2 <= 0)
+                if (currentX + glyphWidth / 2.0f >= x)
                     break;
             } else {
-                if (delta <= 0)
+                if (currentX + glyphWidth >= x)
                     break;
             }
+            currentX += glyphWidth;
         }
     }
 
-    return offset;
+    return characterOffset;
 }
 
-}
+} // namespace WebCore
diff --git a/Source/WebCore/platform/graphics/GlyphBuffer.h b/Source/WebCore/platform/graphics/GlyphBuffer.h
index 3aec08e65..9d757bc20 100644
--- a/Source/WebCore/platform/graphics/GlyphBuffer.h
+++ b/Source/WebCore/platform/graphics/GlyphBuffer.h
@@ -153,6 +153,16 @@ public:
 #endif
     }
 
+    void add(const GlyphBuffer* glyphBuffer, int from, int len)
+    {
+        m_glyphs.append(glyphBuffer->glyphs(from), len);
+        m_advances.append(glyphBuffer->advances(from), len);
+        m_fontData.append(glyphBuffer->m_fontData.data() + from, len);
+#if PLATFORM(WIN)
+        m_offsets.append(glyphBuffer->m_offsets.data() + from, len);
+#endif
+    }
+
     void add(Glyph glyph, const SimpleFontData* font, float width, const FloatSize* offset = 0)
     {
         m_fontData.append(font);
diff --git a/Source/WebCore/platform/graphics/WidthIterator.cpp b/Source/WebCore/platform/graphics/WidthIterator.cpp
index 96600c732..1fb1d9b40 100644
--- a/Source/WebCore/platform/graphics/WidthIterator.cpp
+++ b/Source/WebCore/platform/graphics/WidthIterator.cpp
@@ -162,7 +162,8 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
     CharactersTreatedAsSpace charactersTreatedAsSpace;
     while (textIterator.consume(character, clusterLength)) {
         unsigned advanceLength = clusterLength;
-        const GlyphData& glyphData = glyphDataForCharacter(character, rtl, textIterator.currentCharacter(), advanceLength);
+        int currentCharacterIndex = textIterator.currentCharacter();
+        const GlyphData& glyphData = glyphDataForCharacter(character, rtl, currentCharacterIndex, advanceLength);
         Glyph glyph = glyphData.glyph;
         const SimpleFontData* fontData = glyphData.fontData;
 
@@ -223,9 +224,10 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
                         float expansionAtThisOpportunity = !m_run.applyWordRounding() ? m_expansionPerOpportunity : roundf(previousExpansion) - roundf(m_expansion);
                         m_runWidthSoFar += expansionAtThisOpportunity;
                         if (glyphBuffer) {
-                            if (glyphBuffer->isEmpty())
+                            if (glyphBuffer->isEmpty()) {
                                 glyphBuffer->add(fontData->spaceGlyph(), fontData, expansionAtThisOpportunity);
-                            else
+                                m_characterIndex.append(currentCharacterIndex);
+                            } else
                                 glyphBuffer->expandLastAdvance(expansionAtThisOpportunity);
                         }
                         previousExpansion = m_expansion;
@@ -291,8 +293,10 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
                 widthSinceLastRounding += width;
         }
 
-        if (glyphBuffer)
+        if (glyphBuffer) {
             glyphBuffer->add(glyph, fontData, (rtl ? oldWidth + lastRoundingWidth : width));
+            m_characterIndex.append(currentCharacterIndex);
+        }
 
         lastRoundingWidth = width - oldWidth;
 
@@ -332,15 +336,4 @@ unsigned WidthIterator::advance(int offset, GlyphBuffer* glyphBuffer)
     return advanceInternal(textIterator, glyphBuffer);
 }
 
-bool WidthIterator::advanceOneCharacter(float& width, GlyphBuffer& glyphBuffer)
-{
-    int oldSize = glyphBuffer.size();
-    advance(m_currentCharacter + 1, &glyphBuffer);
-    float w = 0;
-    for (int i = oldSize; i < glyphBuffer.size(); ++i)
-        w += glyphBuffer.advanceAt(i);
-    width = w;
-    return glyphBuffer.size() > oldSize;
-}
-
 }
diff --git a/Source/WebCore/platform/graphics/WidthIterator.h b/Source/WebCore/platform/graphics/WidthIterator.h
index 8e4e06997..6dc1c206c 100644
--- a/Source/WebCore/platform/graphics/WidthIterator.h
+++ b/Source/WebCore/platform/graphics/WidthIterator.h
@@ -43,7 +43,6 @@ public:
     WidthIterator(const Font*, const TextRun&, HashSet<const SimpleFontData*>* fallbackFonts = 0, bool accountForGlyphBounds = false, bool forTextEmphasis = false);
 
     unsigned advance(int to, GlyphBuffer*);
-    bool advanceOneCharacter(float& width, GlyphBuffer&);
 
     float maxGlyphBoundingBoxY() const { ASSERT(m_accountForGlyphBounds); return m_maxGlyphBoundingBoxY; }
     float minGlyphBoundingBoxY() const { ASSERT(m_accountForGlyphBounds); return m_minGlyphBoundingBoxY; }
@@ -83,6 +82,7 @@ public:
     float m_expansionPerOpportunity;
     bool m_isAfterExpansion;
     float m_finalRoundingWidth;
+    Vector<int> m_characterIndex;
 
 #if ENABLE(SVG_FONTS)
     String m_lastGlyphName;