SVGGlyphToPathTranslator ASSERTs when encountering a missing glyph in an SVG font
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Jun 2014 02:24:25 +0000 (02:24 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Jun 2014 02:24:25 +0000 (02:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=133528

Reviewed by Simon Fraser.

Source/WebCore:
Turns out this assertion is benign. We can take an early out of advance() (which
is then handled properly by Font::dashesForIntersectionsWithRect()

Test: svg/custom/skip-underline-missing-glyph.html

* platform/graphics/mac/FontMac.mm:
(WebCore::Font::dashesForIntersectionsWithRect): Rather than skip partial results,
don't skip anything at all to be consistent.
* rendering/svg/SVGTextRunRenderingContext.cpp:
(WebCore::SVGGlyphToPathTranslator::advance): Take an early out to avoid an ASSERT.

LayoutTests:
Make sure that no ASSERT occurs in this situation. In addition, make sure that the
whole element doesn't get skip:ink gaps. This will need to be updated when we
support SVG + non-SVG mixed runs.

* svg/custom/skip-underline-missing-glyph-expected.html: Added
* svg/custom/skip-underline-missing-glyph.html: Added

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/skip-underline-missing-glyph-expected.html [new file with mode: 0644]
LayoutTests/svg/custom/skip-underline-missing-glyph.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/mac/FontMac.mm
Source/WebCore/rendering/svg/SVGTextRunRenderingContext.cpp

index 198d7f26e63bb4d0cb42c53d902b0c6d60ec62b9..d082f0e7f9467b951b261555ac8439678e574de2 100644 (file)
@@ -1,3 +1,17 @@
+2014-06-11  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        SVGGlyphToPathTranslator ASSERTs when encountering a missing glyph in an SVG font
+        https://bugs.webkit.org/show_bug.cgi?id=133528
+
+        Reviewed by Simon Fraser.
+
+        Make sure that no ASSERT occurs in this situation. In addition, make sure that the
+        whole element doesn't get skip:ink gaps. This will need to be updated when we
+        support SVG + non-SVG mixed runs.
+
+        * svg/custom/skip-underline-missing-glyph-expected.html: Added
+        * svg/custom/skip-underline-missing-glyph.html: Added
+
 2014-06-11  Alexey Proskuryakov  <ap@apple.com>
 
         editing/selection/selection-in-iframe-removed-crash.html or selection-invalid-offset.html crashes intermittently
diff --git a/LayoutTests/svg/custom/skip-underline-missing-glyph-expected.html b/LayoutTests/svg/custom/skip-underline-missing-glyph-expected.html
new file mode 100644 (file)
index 0000000..6425cf9
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<style>
+@font-face {
+    font-family: freesans;
+    src: url(../../svg/custom/resources/SVGFreeSans.svg) format("svg");
+}
+</style>
+</head>
+<body>
+This test makes sure that there is no ASSERT hit when we are attempting to draw
+a skip:ink text decoration on SVG text where the SVG font does not have all the
+glyphs that we need.
+
+This test also tests that, in this situation, no gaps are drawn at all. This
+test will need to be altered when we finally support mixed SVG and non-SVG
+fonts along with skip:ink decorations.
+<div style="text-decoration: underline; -webkit-text-decoration-skip: none; font: 48px freesans;">abycとefygh</div>
+</body>
+</html>
diff --git a/LayoutTests/svg/custom/skip-underline-missing-glyph.html b/LayoutTests/svg/custom/skip-underline-missing-glyph.html
new file mode 100644 (file)
index 0000000..2359a25
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<style>
+@font-face {
+    font-family: freesans;
+    src: url(../../svg/custom/resources/SVGFreeSans.svg) format("svg");
+}
+</style>
+</head>
+<body>
+This test makes sure that there is no ASSERT hit when we are attempting to draw
+a skip:ink text decoration on SVG text where the SVG font does not have all the
+glyphs that we need.
+
+This test also tests that, in this situation, no gaps are drawn at all. This
+test will need to be altered when we finally support mixed SVG and non-SVG
+fonts along with skip:ink decorations.
+<div style="text-decoration: underline; -webkit-text-decoration-skip: ink; font: 48px freesans;">abycとefygh</div>
+</body>
+</html>
index 15bfbb1012a07ad411bd0e0e037e9c668a258b1e..fb9c9c737d83db1b16aaf5a48e6709cfef20f74b 100644 (file)
@@ -1,3 +1,21 @@
+2014-06-11  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        SVGGlyphToPathTranslator ASSERTs when encountering a missing glyph in an SVG font
+        https://bugs.webkit.org/show_bug.cgi?id=133528
+
+        Reviewed by Simon Fraser.
+
+        Turns out this assertion is benign. We can take an early out of advance() (which
+        is then handled properly by Font::dashesForIntersectionsWithRect()
+
+        Test: svg/custom/skip-underline-missing-glyph.html
+
+        * platform/graphics/mac/FontMac.mm:
+        (WebCore::Font::dashesForIntersectionsWithRect): Rather than skip partial results,
+        don't skip anything at all to be consistent.
+        * rendering/svg/SVGTextRunRenderingContext.cpp:
+        (WebCore::SVGGlyphToPathTranslator::advance): Take an early out to avoid an ASSERT.
+
 2014-06-11  Simon Fraser  <simon.fraser@apple.com>
 
         [iOS WK2] Give WebKitTestRunner a viewport configuration with initial scale=1 for testing
index 595750848188e3b35929caee4c0992a805d07491..c771ee9e9b5c90a4a3e9e0b78f92be70e28e730f 100644 (file)
@@ -528,7 +528,7 @@ DashArray Font::dashesForIntersectionsWithRect(const TextRun& run, const FloatPo
     if (!glyphBuffer.size())
         return DashArray();
     
-    // FIXME: Handle SVG + non-SVG interleaved runs
+    // FIXME: Handle SVG + non-SVG interleaved runs. https://bugs.webkit.org/show_bug.cgi?id=133778
     const SimpleFontData* fontData = glyphBuffer.fontDataAt(0);
     std::unique_ptr<GlyphToPathTranslator> translator;
     bool isSVG = false;
@@ -543,8 +543,11 @@ DashArray Font::dashesForIntersectionsWithRect(const TextRun& run, const FloatPo
     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.
+        if (!localFontData || (!isSVG && localFontData->isSVGFont()) || (isSVG && localFontData != fontData)) {
+            // The advances will get all messed up if we do anything other than bail here.
+            result.clear();
+            break;
+        }
         switch (translator->underlineType()) {
         case GlyphToPathTranslator::GlyphUnderlineType::SkipDescenders: {
             Path path = translator->path();
index 61b021711aa245975a94318a366377f69941182c..61773863c7cf4332e2d439c11dfa9e77fdb7d874 100644 (file)
@@ -202,7 +202,7 @@ void SVGGlyphToPathTranslator::advance()
         }
 
         ++m_index;
-        if (m_index >= m_stoppingPoint)
+        if (m_index >= m_stoppingPoint || !m_glyphBuffer.fontDataAt(m_index)->isSVGFont())
             break;
         m_glyph = m_glyphBuffer.glyphAt(m_index);
         if (!m_glyph)