ASSERTION FAILED: typesettingFeatures & (Kerning | Ligatures) in WebCore::applyFontTr...
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Sep 2015 22:42:50 +0000 (22:42 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Sep 2015 22:42:50 +0000 (22:42 +0000)
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

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

LayoutTests/ChangeLog
LayoutTests/fast/text/softbank-emoji-no-ligatures-nor-kerning-expected.txt [new file with mode: 0644]
LayoutTests/fast/text/softbank-emoji-no-ligatures-nor-kerning.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/WidthIterator.cpp
Source/WebCore/platform/graphics/WidthIterator.h

index c2c810d..2e0846a 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         [css-grid] Percentage columns shouldn't include border and padding
 2015-09-09  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Percentage columns shouldn't include border and padding
diff --git a/LayoutTests/fast/text/softbank-emoji-no-ligatures-nor-kerning-expected.txt b/LayoutTests/fast/text/softbank-emoji-no-ligatures-nor-kerning-expected.txt
new file mode 100644 (file)
index 0000000..ae983db
--- /dev/null
@@ -0,0 +1,2 @@
+This test makes sure there is no ASSERT when laying out SoftBank emoji symbols without kerning or ligatures. The test passes if there is no ASSERT.
+
diff --git a/LayoutTests/fast/text/softbank-emoji-no-ligatures-nor-kerning.html b/LayoutTests/fast/text/softbank-emoji-no-ligatures-nor-kerning.html
new file mode 100644 (file)
index 0000000..18cf17f
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+This test makes sure there is no ASSERT when laying out SoftBank emoji symbols without kerning or ligatures.
+The test passes if there is no ASSERT.
+<div style="text-rendering: optimizeSpeed;">&#xE001;</div>
+</body>
+</html>
index 15969d4..bc77bf5 100644 (file)
@@ -1,3 +1,21 @@
+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>
 
         Setting document.title when there is no title and no head element should no nothing
 2015-09-09  Chris Dumez  <cdumez@apple.com>
 
         Setting document.title when there is no title and no head element should no nothing
index ca488d9..08cef7c 100644 (file)
@@ -95,11 +95,23 @@ public:
     float advanceAtCharacter;
 };
 
     float advanceAtCharacter;
 };
 
-typedef Vector<std::pair<int, OriginalAdvancesForCharacterTreatedAsSpace>, 64> CharactersTreatedAsSpace;
+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;
+}
 
 
-static inline float applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int& lastGlyphCount, const Font* font, WidthIterator& iterator, TypesettingFeatures typesettingFeatures, bool force, CharactersTreatedAsSpace& charactersTreatedAsSpace)
+inline float WidthIterator::applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int& lastGlyphCount, const Font* font, TypesettingFeatures typesettingFeatures, UChar32 previousCharacter, bool force, CharactersTreatedAsSpace& charactersTreatedAsSpace)
 {
 {
-    ASSERT(typesettingFeatures & (Kerning | Ligatures));
+    ASSERT_UNUSED(previousCharacter, shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter) != WidthIterator::TransformsType::None);
 
     if (!glyphBuffer)
         return 0;
 
     if (!glyphBuffer)
         return 0;
@@ -118,16 +130,14 @@ static inline float applyFontTransforms(GlyphBuffer* glyphBuffer, bool ltr, int&
     if (!ltr)
         glyphBuffer->reverse(lastGlyphCount, glyphBufferSize - lastGlyphCount);
 
     if (!ltr)
         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.
     // 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.
             // 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
 #endif
         }
     } else
 #endif
@@ -194,20 +204,6 @@ static inline std::pair<bool, bool> expansionLocation(bool ideograph, bool treat
     return std::make_pair(expandLeft, expandRight);
 }
 
     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;
-}
-
 template <typename TextIterator>
 inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, GlyphBuffer* glyphBuffer)
 {
 template <typename TextIterator>
 inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, GlyphBuffer* glyphBuffer)
 {
@@ -269,7 +265,7 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
         if (font != lastFontData && width) {
             auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter);
             if (transformsType != TransformsType::None) {
         if (font != lastFontData && width) {
             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);
             }
                 if (glyphBuffer)
                     glyphBuffer->shrink(lastGlyphCount);
             }
@@ -427,7 +423,7 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
 
     auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, previousCharacter);
     if (transformsType != TransformsType::None) {
 
     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);
     }
         if (glyphBuffer)
             glyphBuffer->shrink(lastGlyphCount);
     }
index d5fe258..61d3047 100644 (file)
@@ -35,6 +35,9 @@ class GlyphBuffer;
 class Font;
 class TextRun;
 struct GlyphData;
 class Font;
 class TextRun;
 struct GlyphData;
+struct OriginalAdvancesForCharacterTreatedAsSpace;
+
+typedef Vector<std::pair<int, OriginalAdvancesForCharacterTreatedAsSpace>, 64> CharactersTreatedAsSpace;
 
 struct WidthIterator {
     WTF_MAKE_FAST_ALLOCATED;
 
 struct WidthIterator {
     WTF_MAKE_FAST_ALLOCATED;
@@ -90,6 +93,7 @@ private:
 
     enum class TransformsType { None, Forced, NotForced };
     TransformsType shouldApplyFontTransforms(const GlyphBuffer*, int lastGlyphCount, UChar32 previousCharacter) const;
 
     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;
     HashSet<const Font*>* m_fallbackFonts;
 
     TypesettingFeatures m_typesettingFeatures;
     HashSet<const Font*>* m_fallbackFonts;