Changeset 189557 in webkit

Timestamp:

Sep 9, 2015 3:42:50 PM (9 years ago)

Author:

mmaxfield@apple.com

Message:

ASSERTION FAILED: typesettingFeatures & (Kerning | Ligatures) in WebCore::applyFontTransforms
​https://bugs.webkit.org/show_bug.cgi?id=146194

Reviewed by Dean Jackson.

Source/WebCore:

We might trigger shaping even if the author hasn't specified kerning or ligatures.

Test: fast/text/softbank-emoji-no-ligatures-nor-kerning.html

• platform/graphics/WidthIterator.cpp:

(WebCore::isSoftBankEmoji):
(WebCore::WidthIterator::applyFontTransforms):
(WebCore::WidthIterator::advanceInternal):
(WebCore::applyFontTransforms): Deleted.

• platform/graphics/WidthIterator.h:

LayoutTests:

• fast/text/softbank-emoji-no-ligatures-nor-kerning-expected.html: Added
• fast/text/softbank-emoji-no-ligatures-nor-kerning.html: Added

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/softbank-emoji-no-ligatures-nor-kerning-expected.txt (added)
• LayoutTests/fast/text/softbank-emoji-no-ligatures-nor-kerning.html (added)
• Source/WebCore/ChangeLog (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 @@
+2015-09-09  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        ASSERTION FAILED: typesettingFeatures & (Kerning | Ligatures) in WebCore::applyFontTransforms
+        https://bugs.webkit.org/show_bug.cgi?id=146194
+
+        Reviewed by Dean Jackson.
+
+        * fast/text/softbank-emoji-no-ligatures-nor-kerning-expected.html: Added
+        * fast/text/softbank-emoji-no-ligatures-nor-kerning.html: Added
+
 2015-09-09  Manuel Rego Casasnovas  <rego@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-09-09  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        ASSERTION FAILED: typesettingFeatures & (Kerning | Ligatures) in WebCore::applyFontTransforms
+        https://bugs.webkit.org/show_bug.cgi?id=146194
+
+        Reviewed by Dean Jackson.
+
+        We might trigger shaping even if the author hasn't specified kerning or ligatures.
+
+        Test: fast/text/softbank-emoji-no-ligatures-nor-kerning.html
+
+        * platform/graphics/WidthIterator.cpp:
+        (WebCore::isSoftBankEmoji):
+        (WebCore::WidthIterator::applyFontTransforms):
+        (WebCore::WidthIterator::advanceInternal):
+        (WebCore::applyFontTransforms): Deleted.
+        * platform/graphics/WidthIterator.h:
+
 2015-09-09  Chris Dumez  <cdumez@apple.com>
 

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

@@ -96 +96 @@
 };
 
-typedef Vector<std::pair<int, OriginalAdvancesForCharacterTreatedAsSpace>, 64> CharactersTreatedAsSpace;
-
-static inline float applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int& lastGlyphCount, const Font* font, WidthIterator& iterator, TypesettingFeatures typesettingFeatures, bool force, CharactersTreatedAsSpace& charactersTreatedAsSpace)
-{
-    ASSERT(typesettingFeatures & (Kerning | Ligatures));
+static inline bool isSoftBankEmoji(UChar32 codepoint)
+{
+    return codepoint >= 0xE001 && codepoint <= 0xE537;
+}
+
+inline auto WidthIterator::shouldApplyFontTransforms(const GlyphBuffer* glyphBuffer, int lastGlyphCount, UChar32 previousCharacter) const -> TransformsType
+{
+    if (glyphBuffer && glyphBuffer->size() == (lastGlyphCount + 1) && isSoftBankEmoji(previousCharacter))
+        return TransformsType::Forced;
+    if (m_run.length() <= 1 || !(m_typesettingFeatures & (Kerning | Ligatures)))
+        return TransformsType::None;
+    return TransformsType::NotForced;
+}
+
+inline float WidthIterator::applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int& lastGlyphCount, const Font* font, TypesettingFeatures typesettingFeatures, UChar32 previousCharacter, bool force, CharactersTreatedAsSpace& charactersTreatedAsSpace)
+{
+    ASSERT_UNUSED(previousCharacter, shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter) != WidthIterator::TransformsType::None);
 
     if (!glyphBuffer)
@@ -119 +131 @@
         glyphBuffer->reverse(lastGlyphCount, glyphBufferSize - lastGlyphCount);
 
-#if !ENABLE(SVG_FONTS)
-    UNUSED_PARAM(iterator);
-#else
+#if ENABLE(SVG_FONTS)
     // We need to handle transforms on SVG fonts internally, since they are rendered internally.
     if (font->isSVGFont()) {
         // SVG font ligatures are handled during glyph selection, only kerning remaining.
-        if (iterator.run().renderingContext() && (typesettingFeatures & Kerning)) {
+        if (run().renderingContext() && (typesettingFeatures & Kerning)) {
             // FIXME: We could pass the necessary context down to this level so we can lazily create rendering contexts at this point.
             // However, a larger refactoring of SVG fonts might necessary to sidestep this problem completely.
-            iterator.run().renderingContext()->applySVGKerning(font, iterator, glyphBuffer, lastGlyphCount);
+            run().renderingContext()->applySVGKerning(font, *this, glyphBuffer, lastGlyphCount);
         }
     } else
@@ -193 +203 @@
         expandRight = true;
     return std::make_pair(expandLeft, expandRight);
-}
-
-static inline bool isSoftBankEmoji(UChar32 codepoint)
-{
-    return codepoint >= 0xE001 && codepoint <= 0xE537;
-}
-
-inline auto WidthIterator::shouldApplyFontTransforms(const GlyphBuffer* glyphBuffer, int lastGlyphCount, UChar32 previousCharacter) const -> TransformsType
-{
-    if (glyphBuffer && glyphBuffer->size() == lastGlyphCount + 1 && isSoftBankEmoji(previousCharacter))
-        return TransformsType::Forced;
-    if (m_run.length() <= 1 || !(m_typesettingFeatures & (Kerning | Ligatures)))
-        return TransformsType::None;
-    return TransformsType::NotForced;
 }
 
@@ -270 +266 @@
             auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter);
             if (transformsType != TransformsType::None) {
-                m_runWidthSoFar += applyFontTransforms(glyphBuffer, m_run.ltr(), lastGlyphCount, lastFontData, *this, m_typesettingFeatures, transformsType == TransformsType::Forced, charactersTreatedAsSpace);
+                m_runWidthSoFar += applyFontTransforms(glyphBuffer, m_run.ltr(), lastGlyphCount, lastFontData, m_typesettingFeatures, previousCharacter, transformsType == TransformsType::Forced, charactersTreatedAsSpace);
                 if (glyphBuffer)
                     glyphBuffer->shrink(lastGlyphCount);
@@ -428 +424 @@
     auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter);
     if (transformsType != TransformsType::None) {
-        m_runWidthSoFar += applyFontTransforms(glyphBuffer, m_run.ltr(), lastGlyphCount, lastFontData, *this, m_typesettingFeatures, transformsType == TransformsType::Forced, charactersTreatedAsSpace);
+        m_runWidthSoFar += applyFontTransforms(glyphBuffer, m_run.ltr(), lastGlyphCount, lastFontData, m_typesettingFeatures, previousCharacter, transformsType == TransformsType::Forced, charactersTreatedAsSpace);
         if (glyphBuffer)
             glyphBuffer->shrink(lastGlyphCount);

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

@@ -36 +36 @@
 class TextRun;
 struct GlyphData;
+struct OriginalAdvancesForCharacterTreatedAsSpace;
+
+typedef Vector<std::pair<int, OriginalAdvancesForCharacterTreatedAsSpace>, 64> CharactersTreatedAsSpace;
 
 struct WidthIterator {
@@ -91 +94 @@
     enum class TransformsType { None, Forced, NotForced };
     TransformsType shouldApplyFontTransforms(const GlyphBuffer*, int lastGlyphCount, UChar32 previousCharacter) const;
+    float applyFontTransforms(GlyphBuffer*, bool ltr, int& lastGlyphCount, const Font*, TypesettingFeatures, UChar32 previousCharacter, bool force, CharactersTreatedAsSpace&);
 
     TypesettingFeatures m_typesettingFeatures;