REGRESSION: missing underline under CJK text
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jun 2014 21:18:07 +0000 (21:18 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 9 Jun 2014 21:18:07 +0000 (21:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=128145

Reviewed by Darin Adler.

Source/WebCore:
This patch refactors the GlyphToPathTranslator which is used to find intersections of
glyphs and underlines. It was modified to allow for querying these pieces of
information:
1) The extents of the glyph. This can be used to make the underlines skip an entire
glyph, wholesale
2) What kind of skipping behavior should be used
3) The Path which represents the glyph
There are three skipping behaviors:
1) (SkipDescenders) The previous behavior
2) (SkipGlyph) Make the underline skip over the entire glyph, using the extents() function
3) (DrawOverGlyph) Make the underline plow through the glyph, ignoring any descenders

Calculating which underlining behavior to use depends on what the base codepoint that
originated that glyph is. This means that we have to map from glyphs to characters,
something which is nontrivial to do. In order to solve this problem, this patch adds
an optional vector to GlyphBuffer which represents the location in the original string
from which a particular glyph originated. Then, when our WidthIterator code adds
glyphs to the GlyphBuffer, we can include the extra information about where we are
in the input string. Once this data is available, the GlyphPathTranslator can look up
the base codepoint from which this glyph originates, and can run ICU functions on that
codepoint.

We can use the ICU ublock_getCode() function to find which Unicode block a particular
codepoint comes from. If the codepoint comes from a CJK block, we will use
DrawOverGlyph; otherwise, we will use SkipDescenders.

Test: fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk.html

* platform/graphics/Font.cpp:
(WebCore::sharedUnderlineType): Look up the base codepoint from which this glyph
originates, call ublock_getCode to get its Unicode block, then return
a GlyphUnderlineType accordingly. This code is shared between SVG and non-SVG.
* platform/graphics/Font.h: New virtual functions in GlyphToPathTranslator, as well as
function signatures for the above two functions.
* platform/graphics/GlyphBuffer.h: Add an optional instance member for the location
from within the original string from which a particular glyph originates.
(WebCore::GlyphBuffer::clear): Updated for new member.
(WebCore::GlyphBuffer::add): Ditto.
(WebCore::GlyphBuffer::saveOffsetsInString): Opt-in to using the new variable
(WebCore::GlyphBuffer::offsetInString): New variable accessor.
* platform/graphics/TextRun.h: SVG needs the TextRun to use sharedUnderlineType.
* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::advanceInternal): Use GlyphBuffer's new variable (if present).
* platform/graphics/mac/ComplexTextController.cpp:
(WebCore::ComplexTextController::advance): Use GlyphBuffer's new variable (if present).
* platform/graphics/mac/FontMac.mm: Implement new GlyphToPathTranslator functions.
(WebCore::MacGlyphToPathTranslator::path):
(WebCore::MacGlyphToPathTranslator::extents):
(WebCore::MacGlyphToPathTranslator::underlineType): Calls sharedUnderlineType().
(WebCore::MacGlyphToPathTranslator::moveToNextValidGlyph):
(WebCore::MacGlyphToPathTranslator::increment):
(WebCore::Font::dashesForIntersectionsWithRect): Ask the translator what kind of underline
behavior should be used. React accordingly.
(WebCore::MacGlyphToPathTranslator::nextPath): Deleted.
(WebCore::MacGlyphToPathTranslator::incrementIndex): Deleted.
* platform/graphics/win/UniscribeController.cpp:
(WebCore::UniscribeController::shapeAndPlaceItem): Update to new signature of GlyphBuffer::add()
* rendering/svg/SVGTextRunRenderingContext.cpp: Implement new GlyphToPathTranslator functions.
(WebCore::SVGGlyphToPathTranslator::SVGGlyphToPathTranslator):
(WebCore::SVGGlyphToPathTranslator::getCurrentTransform):
(WebCore::SVGGlyphToPathTranslator::path):
(WebCore::SVGGlyphToPathTranslator::extents):
(WebCore::MacGlyphToPathTranslator::underlineType): Calls sharedUnderlineType().
(WebCore::SVGGlyphToPathTranslator::moveToNextValidGlyph):
(WebCore::SVGGlyphToPathTranslator::increment):
(WebCore::SVGTextRunRenderingContext::createGlyphToPathTranslator):
(WebCore::SVGTextRunRenderingContext::drawSVGGlyphs):
(WebCore::SVGGlyphToPathTranslator::nextPath): Deleted.
(WebCore::SVGGlyphToPathTranslator::incrementIndex): Deleted.
* rendering/svg/SVGTextRunRenderingContext.h: SVG needs the TextRun to use sharedUnderlineType.

LayoutTests:
This test makes sure that underlines under CJK text don't skip over descenders.

* fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk-expected.html: Added.
* fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@169715 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip/resources/Litherum.svg
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk-expected.html [new file with mode: 0644]
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Font.cpp
Source/WebCore/platform/graphics/Font.h
Source/WebCore/platform/graphics/GlyphBuffer.h
Source/WebCore/platform/graphics/TextRun.h
Source/WebCore/platform/graphics/WidthIterator.cpp
Source/WebCore/platform/graphics/mac/ComplexTextController.cpp
Source/WebCore/platform/graphics/mac/FontMac.mm
Source/WebCore/platform/graphics/win/UniscribeController.cpp
Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp
Source/WebCore/rendering/svg/SVGTextRunRenderingContext.h

index 2ffc1c1..031759f 100644 (file)
@@ -1,3 +1,15 @@
+2014-05-21  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION: missing underline under CJK text
+        https://bugs.webkit.org/show_bug.cgi?id=128145
+
+        Reviewed by Darin Adler.
+
+        This test makes sure that underlines under CJK text don't skip over descenders.
+
+        * fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk-expected.html: Added.
+        * fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk.html: Added.
+
 2014-06-09  Daniel Bates  <dabates@apple.com>
 
         [iOS] Amazon app: Cannot interact with product page after tapping on product image
index 956b4e6..f17792d 100644 (file)
@@ -7,6 +7,15 @@
 <font-face units-per-em="56" ascent="56" descent="-28"/>
 <glyph unicode="|" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
 <glyph unicode="_" horiz-adv-x="56" d="M1 -5v1h54v-1z"/>
+<glyph unicode="體" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
+<glyph unicode="現" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
+<glyph unicode="浪" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
+<glyph unicode="外" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
+<glyph unicode="装" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
+<glyph unicode="に" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
+<glyph unicode="일" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
+<glyph unicode="서" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
+<glyph unicode="공" horiz-adv-x="56" d="M20 -28v84h16v-84z"/>
 </font>
 </defs>
 </svg>
diff --git a/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk-expected.html b/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk-expected.html
new file mode 100644 (file)
index 0000000..ef91622
--- /dev/null
@@ -0,0 +1,18 @@
+<style>
+@font-face {
+    font-family: 'Litherum';
+    src: url("./resources/Litherum.svg") format(svg)
+}
+</style>
+<meta charset="utf-8">
+This test makes sure that underlines under CJK text do not skip over low-hanging curves in the glyphs.
+<div style="text-decoration: underline; -webkit-text-decoration-skip: none">
+<p>體現浪漫愛情見證永恆承諾踢走包包面面收毛孔認識世界著名瑞士腕錶品牌美白抗老最強組合即看推介</p>
+<p>外装に目立つ傷は感じられませんがあくまで中古品ですので新品と比べれば使用感はあると思います</p>
+<p>일서울올림픽공원올림픽홀에서펼쳐지는을통해신곡그런너를첫공개할예정이어서새로운음악으로관객</p>
+<div style="font-family: Litherum">
+<p>體現浪</p>
+<p>外装に</p>
+<p>일서공</p>
+</div>
+</div>
diff --git a/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk.html b/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk.html
new file mode 100644 (file)
index 0000000..4fc9149
--- /dev/null
@@ -0,0 +1,18 @@
+<style>
+@font-face {
+    font-family: 'Litherum';
+    src: url("./resources/Litherum.svg") format(svg)
+}
+</style>
+<meta charset="utf-8">
+This test makes sure that underlines under CJK text do not skip over low-hanging curves in the glyphs.
+<div style="text-decoration: underline;">
+<p>體現浪漫愛情見證永恆承諾踢走包包面面收毛孔認識世界著名瑞士腕錶品牌美白抗老最強組合即看推介</p>
+<p>外装に目立つ傷は感じられませんがあくまで中古品ですので新品と比べれば使用感はあると思います</p>
+<p>일서울올림픽공원올림픽홀에서펼쳐지는을통해신곡그런너를첫공개할예정이어서새로운음악으로관객</p>
+<div style="font-family: Litherum">
+<p>體現浪</p>
+<p>外装に</p>
+<p>일서공</p>
+</div>
+</div>
index b0815c1..2dc8488 100644 (file)
@@ -1,3 +1,81 @@
+2014-05-21  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        REGRESSION: missing underline under CJK text
+        https://bugs.webkit.org/show_bug.cgi?id=128145
+
+        Reviewed by Darin Adler.
+
+        This patch refactors the GlyphToPathTranslator which is used to find intersections of
+        glyphs and underlines. It was modified to allow for querying these pieces of
+        information:
+        1) The extents of the glyph. This can be used to make the underlines skip an entire
+        glyph, wholesale
+        2) What kind of skipping behavior should be used
+        3) The Path which represents the glyph
+        There are three skipping behaviors:
+        1) (SkipDescenders) The previous behavior
+        2) (SkipGlyph) Make the underline skip over the entire glyph, using the extents() function
+        3) (DrawOverGlyph) Make the underline plow through the glyph, ignoring any descenders
+
+        Calculating which underlining behavior to use depends on what the base codepoint that
+        originated that glyph is. This means that we have to map from glyphs to characters,
+        something which is nontrivial to do. In order to solve this problem, this patch adds
+        an optional vector to GlyphBuffer which represents the location in the original string
+        from which a particular glyph originated. Then, when our WidthIterator code adds
+        glyphs to the GlyphBuffer, we can include the extra information about where we are
+        in the input string. Once this data is available, the GlyphPathTranslator can look up
+        the base codepoint from which this glyph originates, and can run ICU functions on that
+        codepoint.
+
+        We can use the ICU ublock_getCode() function to find which Unicode block a particular
+        codepoint comes from. If the codepoint comes from a CJK block, we will use
+        DrawOverGlyph; otherwise, we will use SkipDescenders.
+
+        Test: fast/css3-text/css3-text-decoration/text-decoration-skip/text-decoration-skip-ink-cjk.html
+
+        * platform/graphics/Font.cpp:
+        (WebCore::sharedUnderlineType): Look up the base codepoint from which this glyph
+        originates, call ublock_getCode to get its Unicode block, then return
+        a GlyphUnderlineType accordingly. This code is shared between SVG and non-SVG.
+        * platform/graphics/Font.h: New virtual functions in GlyphToPathTranslator, as well as
+        function signatures for the above two functions.
+        * platform/graphics/GlyphBuffer.h: Add an optional instance member for the location
+        from within the original string from which a particular glyph originates.
+        (WebCore::GlyphBuffer::clear): Updated for new member.
+        (WebCore::GlyphBuffer::add): Ditto.
+        (WebCore::GlyphBuffer::saveOffsetsInString): Opt-in to using the new variable
+        (WebCore::GlyphBuffer::offsetInString): New variable accessor.
+        * platform/graphics/TextRun.h: SVG needs the TextRun to use sharedUnderlineType.
+        * platform/graphics/WidthIterator.cpp:
+        (WebCore::WidthIterator::advanceInternal): Use GlyphBuffer's new variable (if present).
+        * platform/graphics/mac/ComplexTextController.cpp:
+        (WebCore::ComplexTextController::advance): Use GlyphBuffer's new variable (if present).
+        * platform/graphics/mac/FontMac.mm: Implement new GlyphToPathTranslator functions.
+        (WebCore::MacGlyphToPathTranslator::path):
+        (WebCore::MacGlyphToPathTranslator::extents):
+        (WebCore::MacGlyphToPathTranslator::underlineType): Calls sharedUnderlineType().
+        (WebCore::MacGlyphToPathTranslator::moveToNextValidGlyph):
+        (WebCore::MacGlyphToPathTranslator::increment):
+        (WebCore::Font::dashesForIntersectionsWithRect): Ask the translator what kind of underline
+        behavior should be used. React accordingly.
+        (WebCore::MacGlyphToPathTranslator::nextPath): Deleted.
+        (WebCore::MacGlyphToPathTranslator::incrementIndex): Deleted.
+        * platform/graphics/win/UniscribeController.cpp:
+        (WebCore::UniscribeController::shapeAndPlaceItem): Update to new signature of GlyphBuffer::add()
+        * rendering/svg/SVGTextRunRenderingContext.cpp: Implement new GlyphToPathTranslator functions.
+        (WebCore::SVGGlyphToPathTranslator::SVGGlyphToPathTranslator):
+        (WebCore::SVGGlyphToPathTranslator::getCurrentTransform):
+        (WebCore::SVGGlyphToPathTranslator::path):
+        (WebCore::SVGGlyphToPathTranslator::extents):
+        (WebCore::MacGlyphToPathTranslator::underlineType): Calls sharedUnderlineType().
+        (WebCore::SVGGlyphToPathTranslator::moveToNextValidGlyph):
+        (WebCore::SVGGlyphToPathTranslator::increment):
+        (WebCore::SVGTextRunRenderingContext::createGlyphToPathTranslator):
+        (WebCore::SVGTextRunRenderingContext::drawSVGGlyphs):
+        (WebCore::SVGGlyphToPathTranslator::nextPath): Deleted.
+        (WebCore::SVGGlyphToPathTranslator::incrementIndex): Deleted.
+        * rendering/svg/SVGTextRunRenderingContext.h: SVG needs the TextRun to use sharedUnderlineType.
+
 2014-06-09  Alex Christensen  <achristensen@webkit.org>
 
         [iOS WebGL] Implement OES_vertex_array_object for iOS.
index 7980c60..234d3c0 100644 (file)
@@ -1053,5 +1053,58 @@ bool Font::canReceiveTextEmphasis(UChar32 c)
 
     return true;
 }
+    
+GlyphToPathTranslator::GlyphUnderlineType computeUnderlineType(const TextRun& textRun, const GlyphBuffer& glyphBuffer, int index)
+{
+    // In general, we want to skip descenders. However, skipping descenders on CJK characters leads to undesirable renderings,
+    // so we want to draw through CJK characters (on a character-by-character basis).
+    UChar32 baseCharacter;
+    int offsetInString = glyphBuffer.offsetInString(index);
+
+    if (offsetInString == GlyphBuffer::kNoOffset) {
+        // We have no idea which character spawned this glyph. Bail.
+        return GlyphToPathTranslator::GlyphUnderlineType::DrawOverGlyph;
+    }
+    
+    if (textRun.is8Bit())
+        baseCharacter = textRun.characters8()[offsetInString];
+    else {
+        U16_NEXT(textRun.characters16(), offsetInString, textRun.length(), baseCharacter);
+    }
+    
+    // u_getIntPropertyValue with UCHAR_IDEOGRAPHIC doesn't return true for Japanese or Korean codepoints.
+    // Instead, we can use the "Unicode allocation block" for the character.
+    UBlockCode blockCode = ublock_getCode(baseCharacter);
+    switch (blockCode) {
+    case UBLOCK_CJK_RADICALS_SUPPLEMENT:
+    case UBLOCK_CJK_SYMBOLS_AND_PUNCTUATION:
+    case UBLOCK_ENCLOSED_CJK_LETTERS_AND_MONTHS:
+    case UBLOCK_CJK_COMPATIBILITY:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_A:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS:
+    case UBLOCK_CJK_COMPATIBILITY_IDEOGRAPHS:
+    case UBLOCK_CJK_COMPATIBILITY_FORMS:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_B:
+    case UBLOCK_CJK_COMPATIBILITY_IDEOGRAPHS_SUPPLEMENT:
+    case UBLOCK_CJK_STROKES:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_C:
+    case UBLOCK_CJK_UNIFIED_IDEOGRAPHS_EXTENSION_D:
+    case UBLOCK_IDEOGRAPHIC_DESCRIPTION_CHARACTERS:
+    case UBLOCK_LINEAR_B_IDEOGRAMS:
+    case UBLOCK_ENCLOSED_IDEOGRAPHIC_SUPPLEMENT:
+    case UBLOCK_HIRAGANA:
+    case UBLOCK_KATAKANA:
+    case UBLOCK_BOPOMOFO:
+    case UBLOCK_BOPOMOFO_EXTENDED:
+    case UBLOCK_HANGUL_JAMO:
+    case UBLOCK_HANGUL_COMPATIBILITY_JAMO:
+    case UBLOCK_HANGUL_SYLLABLES:
+    case UBLOCK_HANGUL_JAMO_EXTENDED_A:
+    case UBLOCK_HANGUL_JAMO_EXTENDED_B:
+        return GlyphToPathTranslator::GlyphUnderlineType::DrawOverGlyph;
+    default:
+        return GlyphToPathTranslator::GlyphUnderlineType::SkipDescenders;
+    }
+}
 
 }
index fcdd68f..1adff0a 100644 (file)
@@ -96,10 +96,15 @@ struct GlyphOverflow {
 
 class GlyphToPathTranslator {
 public:
+    enum class GlyphUnderlineType {SkipDescenders, SkipGlyph, DrawOverGlyph};
     virtual bool containsMorePaths() = 0;
-    virtual Path nextPath() = 0;
+    virtual Path path() = 0;
+    virtual std::pair<float, float> extents() = 0;
+    virtual GlyphUnderlineType underlineType() = 0;
+    virtual void advance() = 0;
     virtual ~GlyphToPathTranslator() { }
 };
+GlyphToPathTranslator::GlyphUnderlineType computeUnderlineType(const TextRun&, const GlyphBuffer&, int index);
 
 class Font {
 public:
index 333c34d..3c5935f 100644 (file)
@@ -83,6 +83,8 @@ public:
         m_fontData.clear();
         m_glyphs.clear();
         m_advances.clear();
+        if (m_offsetsInString)
+            m_offsetsInString->clear();
 #if PLATFORM(WIN)
         m_offsets.clear();
 #endif
@@ -121,8 +123,9 @@ public:
         return FloatSize();
 #endif
     }
-
-    void add(Glyph glyph, const SimpleFontData* font, float width, const FloatSize* offset = 0)
+    
+    static const int kNoOffset = -1;
+    void add(Glyph glyph, const SimpleFontData* font, float width, int offsetInString = kNoOffset, const FloatSize* offset = 0)
     {
         m_fontData.append(font);
 
@@ -149,10 +152,13 @@ public:
 #else
         UNUSED_PARAM(offset);
 #endif
+        
+        if (offsetInString != kNoOffset && m_offsetsInString)
+            m_offsetsInString->append(offsetInString);
     }
     
 #if !USE(WINGDI)
-    void add(Glyph glyph, const SimpleFontData* font, GlyphBufferAdvance advance)
+    void add(Glyph glyph, const SimpleFontData* font, GlyphBufferAdvance advance, int offsetInString = kNoOffset)
     {
         m_fontData.append(font);
 #if USE(CAIRO)
@@ -164,6 +170,9 @@ public:
 #endif
 
         m_advances.append(advance);
+        
+        if (offsetInString != kNoOffset && m_offsetsInString)
+            m_offsetsInString->append(offsetInString);
     }
 #endif
 
@@ -179,6 +188,17 @@ public:
         GlyphBufferAdvance& lastAdvance = m_advances.last();
         lastAdvance.setWidth(lastAdvance.width() + width);
     }
+    
+    void saveOffsetsInString()
+    {
+        m_offsetsInString.reset(new Vector<int, 2048>());
+    }
+    
+    int offsetInString(int index) const
+    {
+        ASSERT(m_offsetsInString);
+        return (*m_offsetsInString)[index];
+    }
 
 private:
     void swap(int index1, int index2)
@@ -206,6 +226,7 @@ private:
     Vector<GlyphBufferGlyph, 2048> m_glyphs;
     Vector<GlyphBufferAdvance, 2048> m_advances;
     GlyphBufferAdvance m_initialAdvance;
+    std::unique_ptr<Vector<int, 2048>> m_offsetsInString;
 #if PLATFORM(WIN)
     Vector<FloatSize, 2048> m_offsets;
 #endif
index 5d0c87d..3720845 100644 (file)
@@ -221,7 +221,7 @@ public:
         virtual void drawSVGGlyphs(GraphicsContext*, const SimpleFontData*, const GlyphBuffer&, int from, int to, const FloatPoint&) const = 0;
         virtual float floatWidthUsingSVGFont(const Font&, const TextRun&, int& charsConsumed, String& glyphName) const = 0;
         virtual bool applySVGKerning(const SimpleFontData*, WidthIterator&, GlyphBuffer*, int from) const = 0;
-        virtual std::unique_ptr<GlyphToPathTranslator> createGlyphToPathTranslator(const SimpleFontData&, const GlyphBuffer&, int from, int numGlyphs, const FloatPoint&) const = 0;
+        virtual std::unique_ptr<GlyphToPathTranslator> createGlyphToPathTranslator(const SimpleFontData&, const TextRun*, const GlyphBuffer&, int from, int numGlyphs, const FloatPoint&) const = 0;
 #endif
     };
 
index d97cf7a..777dc29 100644 (file)
@@ -172,7 +172,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 currentCharacter = textIterator.currentCharacter();
+        const GlyphData& glyphData = glyphDataForCharacter(character, rtl, currentCharacter, advanceLength);
         Glyph glyph = glyphData.glyph;
         const SimpleFontData* fontData = glyphData.fontData;
 
@@ -235,16 +236,16 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
                         if (glyphBuffer) {
                             if (glyphBuffer->isEmpty()) {
                                 if (m_forTextEmphasis)
-                                    glyphBuffer->add(fontData->zeroWidthSpaceGlyph(), fontData, m_expansionPerOpportunity);
+                                    glyphBuffer->add(fontData->zeroWidthSpaceGlyph(), fontData, m_expansionPerOpportunity, currentCharacter);
                                 else
-                                    glyphBuffer->add(fontData->spaceGlyph(), fontData, expansionAtThisOpportunity);
+                                    glyphBuffer->add(fontData->spaceGlyph(), fontData, expansionAtThisOpportunity, currentCharacter);
                             } else
                                 glyphBuffer->expandLastAdvance(expansionAtThisOpportunity);
                         }
                         previousExpansion = m_expansion;
                     }
-                    if (m_run.allowsTrailingExpansion() || (m_run.ltr() && textIterator.currentCharacter() + advanceLength < static_cast<size_t>(m_run.length()))
-                        || (m_run.rtl() && textIterator.currentCharacter())) {
+                    if (m_run.allowsTrailingExpansion() || (m_run.ltr() && currentCharacter + advanceLength < static_cast<size_t>(m_run.length()))
+                        || (m_run.rtl() && currentCharacter)) {
                         m_expansion -= m_expansionPerOpportunity;
                         width += !m_run.applyWordRounding() ? m_expansionPerOpportunity : roundf(previousExpansion) - roundf(m_expansion);
                         m_isAfterExpansion = true;
@@ -254,7 +255,7 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
 
                 // Account for word spacing.
                 // We apply additional space between "words" by adding width to the space character.
-                if (treatAsSpace && (character != '\t' || !m_run.allowTabs()) && (textIterator.currentCharacter() || character == noBreakSpace) && m_font->wordSpacing())
+                if (treatAsSpace && (character != '\t' || !m_run.allowTabs()) && (currentCharacter || character == noBreakSpace) && m_font->wordSpacing())
                     width += m_font->wordSpacing();
             } else
                 m_isAfterExpansion = false;
@@ -266,7 +267,7 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
 
         if (m_accountForGlyphBounds) {
             bounds = fontData->boundsForGlyph(glyph);
-            if (!textIterator.currentCharacter())
+            if (!currentCharacter)
                 m_firstGlyphOverflow = std::max<float>(0, -bounds.x());
         }
 
@@ -305,7 +306,7 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
         }
 
         if (glyphBuffer)
-            glyphBuffer->add(glyph, fontData, (rtl ? oldWidth + lastRoundingWidth : width));
+            glyphBuffer->add(glyph, fontData, (rtl ? oldWidth + lastRoundingWidth : width), currentCharacter);
 
         lastRoundingWidth = width - oldWidth;
 
index c1ecd8d..42a971c 100644 (file)
@@ -529,7 +529,7 @@ void ComplexTextController::advance(unsigned offset, GlyphBuffer* glyphBuffer, G
                 return;
 
             if (glyphBuffer && !m_characterInCurrentGlyph)
-                glyphBuffer->add(m_adjustedGlyphs[k], complexTextRun.fontData(), adjustedAdvance);
+                glyphBuffer->add(m_adjustedGlyphs[k], complexTextRun.fontData(), adjustedAdvance, complexTextRun.indexAt(m_glyphInCurrentRun));
 
             unsigned oldCharacterInCurrentGlyph = m_characterInCurrentGlyph;
             m_characterInCurrentGlyph = std::min(m_currentCharacter - complexTextRun.stringLocation(), glyphEndOffset) - glyphStartOffset;
index c33a208..5957508 100644 (file)
@@ -448,8 +448,9 @@ static void findPathIntersections(void* stateAsVoidPointer, const CGPathElement*
 
 class MacGlyphToPathTranslator final : public GlyphToPathTranslator {
 public:
-    MacGlyphToPathTranslator(const GlyphBuffer& glyphBuffer, const FloatPoint& textOrigin)
+    MacGlyphToPathTranslator(const TextRun& textRun, const GlyphBuffer& glyphBuffer, const FloatPoint& textOrigin)
         : m_index(0)
+        , m_textRun(textRun)
         , m_glyphBuffer(glyphBuffer)
         , m_fontData(glyphBuffer.fontDataAt(m_index))
         , m_translation(CGAffineTransformScale(CGAffineTransformMakeTranslation(textOrigin.x(), textOrigin.y()), 1, -1))
@@ -461,31 +462,45 @@ private:
     {
         return m_index != m_glyphBuffer.size();
     }
-    virtual Path nextPath() override;
+    virtual Path path() override;
+    virtual std::pair<float, float> extents() override;
+    virtual GlyphUnderlineType underlineType() override;
+    virtual void advance() override;
     void moveToNextValidGlyph();
-    void incrementIndex();
 
     int m_index;
+    const TextRun& m_textRun;
     const GlyphBuffer& m_glyphBuffer;
     const SimpleFontData* m_fontData;
     CGAffineTransform m_translation;
 };
 
-Path MacGlyphToPathTranslator::nextPath()
+Path MacGlyphToPathTranslator::path()
 {
     RetainPtr<CGPathRef> result = adoptCF(CTFontCreatePathForGlyph(m_fontData->platformData().ctFont(), m_glyphBuffer.glyphAt(m_index), &m_translation));
-    incrementIndex();
     return adoptCF(CGPathCreateMutableCopy(result.get()));
 }
 
+std::pair<float, float> MacGlyphToPathTranslator::extents()
+{
+    CGPoint beginning = CGPointApplyAffineTransform(CGPointMake(0, 0), m_translation);
+    CGSize end = CGSizeApplyAffineTransform(m_glyphBuffer.advanceAt(m_index), m_translation);
+    return std::make_pair(static_cast<float>(beginning.x), static_cast<float>(beginning.x + end.width));
+}
+
+auto MacGlyphToPathTranslator::underlineType() -> GlyphUnderlineType
+{
+    return computeUnderlineType(m_textRun, m_glyphBuffer, m_index);
+}
+
 void MacGlyphToPathTranslator::moveToNextValidGlyph()
 {
     if (!m_fontData->isSVGFont())
         return;
-    incrementIndex();
+    advance();
 }
 
-void MacGlyphToPathTranslator::incrementIndex()
+void MacGlyphToPathTranslator::advance()
 {
     do {
         GlyphBufferAdvance advance = m_glyphBuffer.advanceAt(m_index);
@@ -503,6 +518,7 @@ DashArray Font::dashesForIntersectionsWithRect(const TextRun& run, const FloatPo
         return DashArray();
 
     GlyphBuffer glyphBuffer;
+    glyphBuffer.saveOffsetsInString();
     float deltaX;
     if (codePath(run) != Font::Complex)
         deltaX = getGlyphsAndAdvancesForSimpleText(run, 0, run.length(), glyphBuffer);
@@ -518,22 +534,36 @@ DashArray Font::dashesForIntersectionsWithRect(const TextRun& run, const FloatPo
     bool isSVG = false;
     FloatPoint origin = FloatPoint(textOrigin.x() + deltaX, textOrigin.y());
     if (!fontData->isSVGFont())
-        translator = std::move(std::make_unique<MacGlyphToPathTranslator>(glyphBuffer, origin));
+        translator = std::move(std::make_unique<MacGlyphToPathTranslator>(run, glyphBuffer, origin));
     else {
-        translator = std::move(run.renderingContext()->createGlyphToPathTranslator(*fontData, glyphBuffer, 0, glyphBuffer.size(), origin));
+        translator = std::move(run.renderingContext()->createGlyphToPathTranslator(*fontData, &run, glyphBuffer, 0, glyphBuffer.size(), origin));
         isSVG = true;
     }
     DashArray result;
-    for (int index = 0; translator->containsMorePaths(); ++index) {
+    for (int index = 0; translator->containsMorePaths(); ++index, translator->advance()) {
         GlyphIterationState info = GlyphIterationState(CGPointMake(0, 0), CGPointMake(0, 0), lineExtents.y(), lineExtents.y() + lineExtents.height(), lineExtents.x() + lineExtents.width(), lineExtents.x());
         const SimpleFontData* localFontData = glyphBuffer.fontDataAt(index);
         if (!localFontData || (!isSVG && localFontData->isSVGFont()) || (isSVG && localFontData != fontData))
             break; // The advances will get all messed up if we do anything other than bail here.
-        Path path = translator->nextPath();
-        CGPathApply(path.platformPath(), &info, &findPathIntersections);
-        if (info.minX < info.maxX) {
-            result.append(info.minX - lineExtents.x());
-            result.append(info.maxX - lineExtents.x());
+        switch (translator->underlineType()) {
+        case GlyphToPathTranslator::GlyphUnderlineType::SkipDescenders: {
+            Path path = translator->path();
+            CGPathApply(path.platformPath(), &info, &findPathIntersections);
+            if (info.minX < info.maxX) {
+                result.append(info.minX - lineExtents.x());
+                result.append(info.maxX - lineExtents.x());
+            }
+            break;
+        }
+        case GlyphToPathTranslator::GlyphUnderlineType::SkipGlyph: {
+            std::pair<float, float> extents = translator->extents();
+            result.append(extents.first - lineExtents.x());
+            result.append(extents.second - lineExtents.x());
+            break;
+        }
+        case GlyphToPathTranslator::GlyphUnderlineType::DrawOverGlyph:
+            // Nothing to do
+            break;
         }
     }
     return result;
index 7c6b0bc..d5d5339 100644 (file)
@@ -359,7 +359,7 @@ bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned i, const S
         // translation.
         if (glyphBuffer) {
             FloatSize size(offsetX, -offsetY);
-            glyphBuffer->add(glyph, fontData, advance, &size);
+            glyphBuffer->add(glyph, fontData, advance, GlyphBuffer::kNoOffset, &size);
         }
 
         FloatRect glyphBounds = fontData->boundsForGlyph(glyph);
index d73a2a4..61b0217 100644 (file)
@@ -103,16 +103,21 @@ bool SVGTextRunRenderingContext::applySVGKerning(const SimpleFontData* fontData,
 
 class SVGGlyphToPathTranslator final : public GlyphToPathTranslator {
 public:
-    SVGGlyphToPathTranslator(const GlyphBuffer& glyphBuffer, const FloatPoint& point, const SVGFontData& svgFontData, SVGFontElement& fontElement, const int from, const int numGlyphs, float scale, bool isVerticalText);
+    SVGGlyphToPathTranslator(const TextRun* const, const GlyphBuffer&, const FloatPoint&, const SVGFontData&, SVGFontElement&, const int from, const int numGlyphs, float scale, bool isVerticalText);
 private:
     virtual bool containsMorePaths() override
     {
         return m_index != m_stoppingPoint;
     }
-    virtual Path nextPath() override;
+
+    virtual Path path() override;
+    virtual std::pair<float, float> extents() override;
+    virtual GlyphUnderlineType underlineType() override;
+    virtual void advance() override;
     void moveToNextValidGlyph();
-    void incrementIndex();
+    AffineTransform transform();
 
+    const TextRun* const m_textRun;
     const GlyphBuffer& m_glyphBuffer;
     const SVGFontData& m_svgFontData;
     FloatPoint m_currentPoint;
@@ -126,8 +131,9 @@ private:
     const bool m_isVerticalText;
 };
 
-SVGGlyphToPathTranslator::SVGGlyphToPathTranslator(const GlyphBuffer& glyphBuffer, const FloatPoint& point, const SVGFontData& svgFontData, SVGFontElement& fontElement, const int from, const int numGlyphs, float scale, bool isVerticalText)
-    : m_glyphBuffer(glyphBuffer)
+SVGGlyphToPathTranslator::SVGGlyphToPathTranslator(const TextRun* const textRun, const GlyphBuffer& glyphBuffer, const FloatPoint& point, const SVGFontData& svgFontData, SVGFontElement& fontElement, const int from, const int numGlyphs, float scale, bool isVerticalText)
+    : m_textRun(textRun)
+    , m_glyphBuffer(glyphBuffer)
     , m_svgFontData(svgFontData)
     , m_currentPoint(point)
     , m_glyphOrigin(m_svgFontData.horizontalOriginX() * scale, m_svgFontData.horizontalOriginY() * scale)
@@ -148,31 +154,43 @@ SVGGlyphToPathTranslator::SVGGlyphToPathTranslator(const GlyphBuffer& glyphBuffe
     moveToNextValidGlyph();
 }
 
-Path SVGGlyphToPathTranslator::nextPath()
+AffineTransform SVGGlyphToPathTranslator::transform()
 {
-    if (m_isVerticalText) {
-        m_glyphOrigin.setX(m_svgGlyph.verticalOriginX * m_scale);
-        m_glyphOrigin.setY(m_svgGlyph.verticalOriginY * m_scale);
-    }
-
     AffineTransform glyphPathTransform;
     glyphPathTransform.translate(m_currentPoint.x() + m_glyphOrigin.x(), m_currentPoint.y() + m_glyphOrigin.y());
     glyphPathTransform.scale(m_scale, -m_scale);
+    return glyphPathTransform;
+}
 
+Path SVGGlyphToPathTranslator::path()
+{
     Path glyphPath = m_svgGlyph.pathData;
-    glyphPath.transform(glyphPathTransform);
-    incrementIndex();
+    glyphPath.transform(transform());
     return glyphPath;
 }
 
+std::pair<float, float> SVGGlyphToPathTranslator::extents()
+{
+    AffineTransform glyphPathTransform = transform();
+    FloatPoint beginning = glyphPathTransform.mapPoint(m_currentPoint);
+    FloatSize end = glyphPathTransform.mapSize(FloatSize(m_glyphBuffer.advanceAt(m_index)));
+    return std::make_pair(beginning.x(), beginning.x() + end.width());
+}
+
+auto SVGGlyphToPathTranslator::underlineType() -> GlyphUnderlineType
+{
+    ASSERT(m_textRun);
+    return computeUnderlineType(*m_textRun, m_glyphBuffer, m_index);
+}
+
 void SVGGlyphToPathTranslator::moveToNextValidGlyph()
 {
     if (m_glyph && !m_svgGlyph.pathData.isEmpty())
         return;
-    incrementIndex();
+    advance();
 }
 
-void SVGGlyphToPathTranslator::incrementIndex()
+void SVGGlyphToPathTranslator::advance()
 {
     do {
         if (m_glyph) {
@@ -194,6 +212,11 @@ void SVGGlyphToPathTranslator::incrementIndex()
         ASSERT(m_svgGlyph.tableEntry == m_glyph);
         SVGGlyphElement::inheritUnspecifiedAttributes(m_svgGlyph, &m_svgFontData);
     } while ((!m_glyph || m_svgGlyph.pathData.isEmpty()) && m_index < m_stoppingPoint);
+
+    if (containsMorePaths() && m_isVerticalText) {
+        m_glyphOrigin.setX(m_svgGlyph.verticalOriginX * m_scale);
+        m_glyphOrigin.setY(m_svgGlyph.verticalOriginY * m_scale);
+    }
 }
 
 class DummyGlyphToPathTranslator final : public GlyphToPathTranslator {
@@ -201,13 +224,24 @@ class DummyGlyphToPathTranslator final : public GlyphToPathTranslator {
     {
         return false;
     }
-    virtual Path nextPath() override
+    virtual Path path() override
     {
         return Path();
     }
+    virtual std::pair<float, float> extents() override
+    {
+        return std::make_pair(0.f, 0.f);
+    }
+    virtual GlyphUnderlineType underlineType() override
+    {
+        return GlyphUnderlineType::DrawOverGlyph;
+    }
+    virtual void advance() override
+    {
+    }
 };
 
-std::unique_ptr<GlyphToPathTranslator> SVGTextRunRenderingContext::createGlyphToPathTranslator(const SimpleFontData& fontData, const GlyphBuffer& glyphBuffer, int from, int numGlyphs, const FloatPoint& point) const
+std::unique_ptr<GlyphToPathTranslator> SVGTextRunRenderingContext::createGlyphToPathTranslator(const SimpleFontData& fontData, const TextRun* textRun, const GlyphBuffer& glyphBuffer, int from, int numGlyphs, const FloatPoint& point) const
 {
     SVGFontElement* fontElement = 0;
     SVGFontFaceElement* fontFaceElement = 0;
@@ -222,7 +256,7 @@ std::unique_ptr<GlyphToPathTranslator> SVGTextRunRenderingContext::createGlyphTo
 
     float scale = scaleEmToUnits(fontData.platformData().size(), fontFaceElement->unitsPerEm());
 
-    return std::make_unique<SVGGlyphToPathTranslator>(glyphBuffer, point, *svgFontData, *fontElement, from, numGlyphs, scale, isVerticalText);
+    return std::make_unique<SVGGlyphToPathTranslator>(textRun, glyphBuffer, point, *svgFontData, *fontElement, from, numGlyphs, scale, isVerticalText);
 }
 
 void SVGTextRunRenderingContext::drawSVGGlyphs(GraphicsContext* context, const SimpleFontData* fontData, const GlyphBuffer& glyphBuffer, int from, int numGlyphs, const FloatPoint& point) const
@@ -241,9 +275,8 @@ void SVGTextRunRenderingContext::drawSVGGlyphs(GraphicsContext* context, const S
     ASSERT(activePaintingResource);
 
     RenderSVGResourceMode resourceMode = context->textDrawingMode() == TextModeStroke ? ApplyToStrokeMode : ApplyToFillMode;
-    auto translator(createGlyphToPathTranslator(*fontData, glyphBuffer, from, numGlyphs, point));
-    while (translator->containsMorePaths()) {
-        Path glyphPath = translator->nextPath();
+    for (auto translator = createGlyphToPathTranslator(*fontData, nullptr, glyphBuffer, from, numGlyphs, point); translator->containsMorePaths(); translator->advance()) {
+        Path glyphPath = translator->path();
         if (activePaintingResource->applyResource(elementRenderer, style, context, resourceMode)) {
             float strokeThickness = context->strokeThickness();
             if (renderer().isSVGInlineText())
index 53681f9..d0b91f6 100644 (file)
@@ -60,7 +60,7 @@ private:
     virtual ~SVGTextRunRenderingContext() { }
 
 #if ENABLE(SVG_FONTS)
-    virtual std::unique_ptr<GlyphToPathTranslator> createGlyphToPathTranslator(const SimpleFontData&, const GlyphBuffer&, int from, int numGlyphs, const FloatPoint&) const override;
+    virtual std::unique_ptr<GlyphToPathTranslator> createGlyphToPathTranslator(const SimpleFontData&, const TextRun*, const GlyphBuffer&, int from, int numGlyphs, const FloatPoint&) const override;
 #endif
 
     RenderObject& m_renderer;