Title: [191386] trunk/Source/WebCore
Revision
191386
Author
commit-qu...@webkit.org
Date
2015-10-21 05:52:17 -0700 (Wed, 21 Oct 2015)

Log Message

ASSERTION FAILED: markFontData in FontCascade::emphasisMarkHeight
https://bugs.webkit.org/show_bug.cgi?id=150171

Patch by Carlos Garcia Campos <cgar...@igalia.com> on 2015-10-21
Reviewed by Myles C. Maxfield.

It happens with several tests like fast/ruby/text-emphasis.html in
the GTK Debug bot. The tests seem to pass in Release and the rendering
looks correct as well removing the assert. The thing is that
for some reason we can get an empty GlyphData from
FontCascade::getEmphasisMarkGlyphData() when it ends up falling
back to system (FontCascadeFonts::glyphDataForSystemFallback).

* platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::getEmphasisMarkGlyphData): Return
Optional<GlyphData> instead of returning a boolean and an out
parameter. If we get an invalid GlyphData, Nullopt is
returned. Also use a SurrogatePairAwareTextIterator to handle
surrogate pairs.
(WebCore::FontCascade::emphasisMarkAscent):
(WebCore::FontCascade::emphasisMarkDescent):
(WebCore::FontCascade::emphasisMarkHeight):
(WebCore::FontCascade::drawEmphasisMarks):
* platform/graphics/FontCascade.h:
* platform/graphics/GlyphPage.h:
(WebCore::GlyphData::isValid): Return whether the GlyphData is valid.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (191385 => 191386)


--- trunk/Source/WebCore/ChangeLog	2015-10-21 11:47:16 UTC (rev 191385)
+++ trunk/Source/WebCore/ChangeLog	2015-10-21 12:52:17 UTC (rev 191386)
@@ -1,3 +1,31 @@
+2015-10-21  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        ASSERTION FAILED: markFontData in FontCascade::emphasisMarkHeight
+        https://bugs.webkit.org/show_bug.cgi?id=150171
+
+        Reviewed by Myles C. Maxfield.
+
+        It happens with several tests like fast/ruby/text-emphasis.html in
+        the GTK Debug bot. The tests seem to pass in Release and the rendering
+        looks correct as well removing the assert. The thing is that
+        for some reason we can get an empty GlyphData from
+        FontCascade::getEmphasisMarkGlyphData() when it ends up falling
+        back to system (FontCascadeFonts::glyphDataForSystemFallback).
+
+        * platform/graphics/FontCascade.cpp:
+        (WebCore::FontCascade::getEmphasisMarkGlyphData): Return
+        Optional<GlyphData> instead of returning a boolean and an out
+        parameter. If we get an invalid GlyphData, Nullopt is
+        returned. Also use a SurrogatePairAwareTextIterator to handle
+        surrogate pairs.
+        (WebCore::FontCascade::emphasisMarkAscent):
+        (WebCore::FontCascade::emphasisMarkDescent):
+        (WebCore::FontCascade::emphasisMarkHeight):
+        (WebCore::FontCascade::drawEmphasisMarks):
+        * platform/graphics/FontCascade.h:
+        * platform/graphics/GlyphPage.h:
+        (WebCore::GlyphData::isValid): Return whether the GlyphData is valid.
+
 2015-10-20  Sergio Villar Senin  <svil...@igalia.com>
 
         [css-grid] Fix availableLogicalSpace computation with non-zero baseSize flex tracks

Modified: trunk/Source/WebCore/platform/graphics/FontCascade.cpp (191385 => 191386)


--- trunk/Source/WebCore/platform/graphics/FontCascade.cpp	2015-10-21 11:47:16 UTC (rev 191385)
+++ trunk/Source/WebCore/platform/graphics/FontCascade.cpp	2015-10-21 12:52:17 UTC (rev 191386)
@@ -29,6 +29,7 @@
 #include "FontCache.h"
 #include "GlyphBuffer.h"
 #include "LayoutRect.h"
+#include "SurrogatePairAwareTextIterator.h"
 #include "TextRun.h"
 #include "WidthIterator.h"
 #include <wtf/MainThread.h>
@@ -1197,38 +1198,31 @@
 
 // FIXME: This function may not work if the emphasis mark uses a complex script, but none of the
 // standard emphasis marks do so.
-bool FontCascade::getEmphasisMarkGlyphData(const AtomicString& mark, GlyphData& glyphData) const
+Optional<GlyphData> FontCascade::getEmphasisMarkGlyphData(const AtomicString& mark) const
 {
     if (mark.isEmpty())
-        return false;
+        return Nullopt;
 
-    UChar32 character = mark[0];
+    UChar32 character;
+    if (!mark.is8Bit()) {
+        SurrogatePairAwareTextIterator iterator(mark.characters16(), 0, mark.length(), mark.length());
+        unsigned clusterLength;
+        if (!iterator.consume(character, clusterLength))
+            return Nullopt;
+    } else
+        character = mark[0];
 
-    if (U16_IS_SURROGATE(character)) {
-        if (!U16_IS_SURROGATE_LEAD(character))
-            return false;
-
-        if (mark.length() < 2)
-            return false;
-
-        UChar low = mark[1];
-        if (!U16_IS_TRAIL(low))
-            return false;
-
-        character = U16_GET_SUPPLEMENTARY(character, low);
-    }
-
-    glyphData = glyphDataForCharacter(character, false, EmphasisMarkVariant);
-    return true;
+    Optional<GlyphData> glyphData(glyphDataForCharacter(character, false, EmphasisMarkVariant));
+    return glyphData.value().isValid() ? glyphData : Nullopt;
 }
 
 int FontCascade::emphasisMarkAscent(const AtomicString& mark) const
 {
-    GlyphData markGlyphData;
-    if (!getEmphasisMarkGlyphData(mark, markGlyphData))
+    Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
+    if (!markGlyphData)
         return 0;
 
-    const Font* markFontData = markGlyphData.font;
+    const Font* markFontData = markGlyphData.value().font;
     ASSERT(markFontData);
     if (!markFontData)
         return 0;
@@ -1238,11 +1232,11 @@
 
 int FontCascade::emphasisMarkDescent(const AtomicString& mark) const
 {
-    GlyphData markGlyphData;
-    if (!getEmphasisMarkGlyphData(mark, markGlyphData))
+    Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
+    if (!markGlyphData)
         return 0;
 
-    const Font* markFontData = markGlyphData.font;
+    const Font* markFontData = markGlyphData.value().font;
     ASSERT(markFontData);
     if (!markFontData)
         return 0;
@@ -1252,11 +1246,11 @@
 
 int FontCascade::emphasisMarkHeight(const AtomicString& mark) const
 {
-    GlyphData markGlyphData;
-    if (!getEmphasisMarkGlyphData(mark, markGlyphData))
+    Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
+    if (!markGlyphData)
         return 0;
 
-    const Font* markFontData = markGlyphData.font;
+    const Font* markFontData = markGlyphData.value().font;
     ASSERT(markFontData);
     if (!markFontData)
         return 0;
@@ -1389,16 +1383,16 @@
 
 void FontCascade::drawEmphasisMarks(GraphicsContext& context, const TextRun& run, const GlyphBuffer& glyphBuffer, const AtomicString& mark, const FloatPoint& point) const
 {
-    GlyphData markGlyphData;
-    if (!getEmphasisMarkGlyphData(mark, markGlyphData))
+    Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
+    if (!markGlyphData)
         return;
 
-    const Font* markFontData = markGlyphData.font;
+    const Font* markFontData = markGlyphData.value().font;
     ASSERT(markFontData);
     if (!markFontData)
         return;
 
-    Glyph markGlyph = markGlyphData.glyph;
+    Glyph markGlyph = markGlyphData.value().glyph;
     Glyph spaceGlyph = markFontData->spaceGlyph();
 
     float middleOfLastGlyph = offsetToMiddleOfGlyphAtIndex(glyphBuffer, 0);

Modified: trunk/Source/WebCore/platform/graphics/FontCascade.h (191385 => 191386)


--- trunk/Source/WebCore/platform/graphics/FontCascade.h	2015-10-21 11:47:16 UTC (rev 191385)
+++ trunk/Source/WebCore/platform/graphics/FontCascade.h	2015-10-21 12:52:17 UTC (rev 191386)
@@ -33,6 +33,7 @@
 #include "TextFlags.h"
 #include <wtf/HashMap.h>
 #include <wtf/HashSet.h>
+#include <wtf/Optional.h>
 #include <wtf/WeakPtr.h>
 #include <wtf/unicode/CharacterNames.h>
 
@@ -231,7 +232,7 @@
     int offsetForPositionForSimpleText(const TextRun&, float position, bool includePartialGlyphs) const;
     void adjustSelectionRectForSimpleText(const TextRun&, LayoutRect& selectionRect, int from, int to) const;
 
-    bool getEmphasisMarkGlyphData(const AtomicString&, GlyphData&) const;
+    Optional<GlyphData> getEmphasisMarkGlyphData(const AtomicString&) const;
 
     static bool canReturnFallbackFontsForComplexText();
     static bool canExpandAroundIdeographsInComplexText();

Modified: trunk/Source/WebCore/platform/graphics/GlyphPage.h (191385 => 191386)


--- trunk/Source/WebCore/platform/graphics/GlyphPage.h	2015-10-21 11:47:16 UTC (rev 191385)
+++ trunk/Source/WebCore/platform/graphics/GlyphPage.h	2015-10-21 12:52:17 UTC (rev 191386)
@@ -49,6 +49,9 @@
         , font(f)
     {
     }
+
+    bool isValid() const { return glyph || font; }
+
     Glyph glyph;
     const Font* font;
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to