Changeset 163655 in webkit

Timestamp:

Feb 7, 2014 3:07:29 PM (10 years ago)

Author:

rniwa@webkit.org

Message:

Revert r154384 and r154674 as they broke selection rect computations for text with ligatures.

Source/WebCore:

• 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:
• platform/graphics/WidthIterator.cpp:

(WebCore::WidthIterator::WidthIterator):
(WebCore::WidthIterator::advanceInternal):
(WebCore::WidthIterator::advanceOneCharacter):

• platform/graphics/WidthIterator.h:

LayoutTests:

• fast/text/partial-textruns-expected.html: Removed.
• fast/text/partial-textruns.html: Removed.
• fast/text/resources/PTS55F-webfont.ttf: Removed.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/partial-textruns-expected.html (deleted)
• LayoutTests/fast/text/partial-textruns.html (deleted)
• LayoutTests/fast/text/resources/PTS55F-webfont.ttf (deleted)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/Font.cpp (modified) (4 diffs)
• Source/WebCore/platform/graphics/FontFastPath.cpp (modified) (3 diffs)
• Source/WebCore/platform/graphics/GlyphBuffer.h (modified) (1 diff)
• Source/WebCore/platform/graphics/WidthIterator.cpp (modified) (5 diffs)
• Source/WebCore/platform/graphics/WidthIterator.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-02-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Revert r154384 and r154674 as they broke selection rect computations for text with ligatures.
+
+        * fast/text/partial-textruns-expected.html: Removed.
+        * fast/text/partial-textruns.html: Removed.
+        * fast/text/resources/PTS55F-webfont.ttf: Removed.
+
 2014-02-07  Brendan Long  <b.long@cablelabs.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-02-07  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Revert r154384 and r154674 as they broke selection rect computations for text with ligatures.
+
+        * 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:
+        * platform/graphics/WidthIterator.cpp:
+        (WebCore::WidthIterator::WidthIterator):
+        (WebCore::WidthIterator::advanceInternal):
+        (WebCore::WidthIterator::advanceOneCharacter):
+        * platform/graphics/WidthIterator.h:
+
 2014-02-07  Benjamin Poulain  <bpoulain@apple.com>
 

trunk/Source/WebCore/platform/graphics/Font.cpp

@@ -335 +335 @@
     to = (to == -1 ? run.length() : to);
 
-    if (codePath(run) != Complex)
+    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()) && !run.renderingContext())
+        codePathToUse = Complex;
+
+    if (codePathToUse != Complex)
         return drawSimpleText(context, run, point, from, to);
 
@@ -349 +354 @@
         to = run.length();
 
-    if (codePath(run) != Complex)
+    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()) && !run.renderingContext())
+        codePathToUse = Complex;
+
+    if (codePathToUse != Complex)
         drawEmphasisMarksForSimpleText(context, run, mark, point, from, to);
     else
@@ -421 +431 @@
     to = (to == -1 ? run.length() : to);
 
-    if (codePath(run) != Complex)
+    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()) && !run.renderingContext())
+        codePathToUse = Complex;
+
+    if (codePathToUse != Complex)
         return selectionRectForSimpleText(run, point, h, from, to);
 
@@ -429 +444 @@
 int Font::offsetForPosition(const TextRun& run, float x, bool includePartialGlyphs) const
 {
-    if (codePath(run) != Complex)
+    // 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())
         return offsetForPositionForSimpleText(run, x, includePartialGlyphs);
 

trunk/Source/WebCore/platform/graphics/FontFastPath.cpp

@@ -128 +128 @@
 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(run.length(), &localGlyphBuffer);
-
-    if (localGlyphBuffer.isEmpty())
-        return 0;
-
-    float totalWidth = it.m_runWidthSoFar;
-    float beforeWidth = 0;
-    int glyphPos = 0;
-    for (; glyphPos < localGlyphBuffer.size() && it.m_characterIndexOfGlyph[glyphPos] < from; ++glyphPos)
-        beforeWidth += localGlyphBuffer.advanceAt(glyphPos).width();
-    int glyphFrom = glyphPos;
-
-    float afterWidth = totalWidth;
-    glyphPos = localGlyphBuffer.size() - 1;
-    for (; glyphPos >= glyphFrom && it.m_characterIndexOfGlyph[glyphPos] >= to; --glyphPos)
-        afterWidth -= localGlyphBuffer.advanceAt(glyphPos).width();
-    int glyphTo = glyphPos + 1;
-
-    glyphBuffer.add(&localGlyphBuffer, glyphFrom, glyphTo - glyphFrom);
+    it.advance(from, &localGlyphBuffer);
+    float beforeWidth = it.m_runWidthSoFar;
+    it.advance(to, &glyphBuffer);
+
+    if (glyphBuffer.isEmpty())
+        return 0;
+
+    float afterWidth = it.m_runWidthSoFar;
 
     if (run.rtl()) {
+        float finalRoundingWidth = it.m_finalRoundingWidth;
+        it.advance(run.length(), &localGlyphBuffer);
+        initialAdvance = finalRoundingWidth + it.m_runWidthSoFar - afterWidth;
+    } else
+        initialAdvance = beforeWidth;
+
+    if (run.rtl())
         glyphBuffer.reverse(0, glyphBuffer.size());
-        return totalWidth - afterWidth;
-    }
-
-    return beforeWidth;
+
+    return initialAdvance;
 }
 
@@ -302 +300 @@
     GlyphBuffer glyphBuffer;
     WidthIterator it(this, run);
-    it.advance(run.length(), &glyphBuffer);
-
-    float totalWidth = it.m_runWidthSoFar;
-    float beforeWidth = 0;
-    int glyphPos = 0;
-    for (; glyphPos < glyphBuffer.size() && it.m_characterIndexOfGlyph[glyphPos] < from; ++glyphPos)
-        beforeWidth += glyphBuffer.advanceAt(glyphPos).width();
-    int glyphFrom = glyphPos;
-
-    float afterWidth = totalWidth;
-    glyphPos = glyphBuffer.size() - 1;
-    for (; glyphPos >= glyphFrom && it.m_characterIndexOfGlyph[glyphPos] >= to; --glyphPos)
-        afterWidth -= glyphBuffer.advanceAt(glyphPos).width();
+    it.advance(from, &glyphBuffer);
+    float beforeWidth = it.m_runWidthSoFar;
+    it.advance(to, &glyphBuffer);
+    float afterWidth = it.m_runWidthSoFar;
 
     // 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);
     }
@@ -326 +317 @@
 int Font::offsetForPositionForSimpleText(const TextRun& run, float x, bool includePartialGlyphs) const
 {
-    GlyphBuffer glyphBuffer;
+    float delta = x;
+
     WidthIterator it(this, run);
-    it.advance(run.length(), &glyphBuffer);
-
-    int characterOffset = 0;
+    GlyphBuffer localGlyphBuffer;
+    unsigned offset;
     if (run.rtl()) {
-        float currentX = it.m_runWidthSoFar;
-        for (int glyphPosition = 0; glyphPosition <= glyphBuffer.size(); ++glyphPosition) {
-            if (glyphPosition == glyphBuffer.size()) {
-                characterOffset = run.length();
+        delta -= floatWidthForSimpleText(run);
+        while (1) {
+            offset = it.m_currentCharacter;
+            float w;
+            if (!it.advanceOneCharacter(w, localGlyphBuffer))
                 break;
-            }
-            characterOffset = it.m_characterIndexOfGlyph[glyphPosition];
-            float glyphWidth = glyphBuffer.advanceAt(glyphPosition).width();
+            delta += w;
             if (includePartialGlyphs) {
-                if (currentX - glyphWidth / 2.0f <= x)
+                if (delta - w / 2 >= 0)
                     break;
             } else {
-                if (currentX - glyphWidth <= x)
+                if (delta >= 0)
                     break;
             }
-            currentX -= glyphWidth;
         }
     } else {
-        float currentX = 0;
-        for (int glyphPosition = 0; glyphPosition <= glyphBuffer.size(); ++glyphPosition) {
-            if (glyphPosition == glyphBuffer.size()) {
-                characterOffset = run.length();
+        while (1) {
+            offset = it.m_currentCharacter;
+            float w;
+            if (!it.advanceOneCharacter(w, localGlyphBuffer))
                 break;
-            }
-            characterOffset = it.m_characterIndexOfGlyph[glyphPosition];
-            float glyphWidth = glyphBuffer.advanceAt(glyphPosition).width();
+            delta -= w;
             if (includePartialGlyphs) {
-                if (currentX + glyphWidth / 2.0f >= x)
+                if (delta + w / 2 <= 0)
                     break;
             } else {
-                if (currentX + glyphWidth >= x)
+                if (delta <= 0)
                     break;
             }
-            currentX += glyphWidth;
         }
     }
 
-    return characterOffset;
-}
-
-} // namespace WebCore
+    return offset;
+}
+
+}

trunk/Source/WebCore/platform/graphics/GlyphBuffer.h

@@ -123 +123 @@
     }
 
-    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)
     {

trunk/Source/WebCore/platform/graphics/WidthIterator.cpp

@@ -67 +67 @@
             m_expansionPerOpportunity = m_expansion / expansionOpportunityCount;
     }
-    // Character-index will end up the same or slightly shorter than m_run, so if we reserve that much it will never need to resize.
-    m_characterIndexOfGlyph.reserveInitialCapacity(m_run.length());
 }
 
@@ -175 +173 @@
     while (textIterator.consume(character, clusterLength)) {
         unsigned advanceLength = clusterLength;
-        int currentCharacterIndex = textIterator.currentCharacter();
-        const GlyphData& glyphData = glyphDataForCharacter(character, rtl, currentCharacterIndex, advanceLength);
+        const GlyphData& glyphData = glyphDataForCharacter(character, rtl, textIterator.currentCharacter(), advanceLength);
         Glyph glyph = glyphData.glyph;
         const SimpleFontData* fontData = glyphData.fontData;
@@ -242 +239 @@
                                 else
                                     glyphBuffer->add(fontData->spaceGlyph(), fontData, expansionAtThisOpportunity);
-                                m_characterIndexOfGlyph.append(currentCharacterIndex);
                             } else
                                 glyphBuffer->expandLastAdvance(expansionAtThisOpportunity);
@@ -309 +305 @@
         }
 
-        if (glyphBuffer) {
+        if (glyphBuffer)
             glyphBuffer->add(glyph, fontData, (rtl ? oldWidth + lastRoundingWidth : width));
-            m_characterIndexOfGlyph.append(currentCharacterIndex);
-        }
 
         lastRoundingWidth = width - oldWidth;
@@ -352 +346 @@
 }
 
-}
+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();
+    width = w;
+    return glyphBuffer.size() > oldSize;
+}
+
+}

trunk/Source/WebCore/platform/graphics/WidthIterator.h

@@ -43 +43 @@
 
     unsigned advance(int to, GlyphBuffer*);
+    bool advanceOneCharacter(float& width, GlyphBuffer&);
 
     float maxGlyphBoundingBoxY() const { ASSERT(m_accountForGlyphBounds); return m_maxGlyphBoundingBoxY; }
@@ -80 +81 @@
     bool m_isAfterExpansion;
     float m_finalRoundingWidth;
-    // An inline capacity of 10 catches around 2/3 of the cases. To catch 90% we would need 32.
-    Vector<int, 10> m_characterIndexOfGlyph;
 
 #if ENABLE(SVG_FONTS)