Changeset 191386 in webkit


Ignore:
Timestamp:
Oct 21, 2015 5:52:17 AM (8 years ago)
Author:
commit-queue@webkit.org
Message:

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

Patch by Carlos Garcia Campos <cgarcia@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.

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r191385 r191386  
     12015-10-21  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        ASSERTION FAILED: markFontData in FontCascade::emphasisMarkHeight
     4        https://bugs.webkit.org/show_bug.cgi?id=150171
     5
     6        Reviewed by Myles C. Maxfield.
     7
     8        It happens with several tests like fast/ruby/text-emphasis.html in
     9        the GTK Debug bot. The tests seem to pass in Release and the rendering
     10        looks correct as well removing the assert. The thing is that
     11        for some reason we can get an empty GlyphData from
     12        FontCascade::getEmphasisMarkGlyphData() when it ends up falling
     13        back to system (FontCascadeFonts::glyphDataForSystemFallback).
     14
     15        * platform/graphics/FontCascade.cpp:
     16        (WebCore::FontCascade::getEmphasisMarkGlyphData): Return
     17        Optional<GlyphData> instead of returning a boolean and an out
     18        parameter. If we get an invalid GlyphData, Nullopt is
     19        returned. Also use a SurrogatePairAwareTextIterator to handle
     20        surrogate pairs.
     21        (WebCore::FontCascade::emphasisMarkAscent):
     22        (WebCore::FontCascade::emphasisMarkDescent):
     23        (WebCore::FontCascade::emphasisMarkHeight):
     24        (WebCore::FontCascade::drawEmphasisMarks):
     25        * platform/graphics/FontCascade.h:
     26        * platform/graphics/GlyphPage.h:
     27        (WebCore::GlyphData::isValid): Return whether the GlyphData is valid.
     28
    1292015-10-20  Sergio Villar Senin  <svillar@igalia.com>
    230
  • trunk/Source/WebCore/platform/graphics/FontCascade.cpp

    r191331 r191386  
    3030#include "GlyphBuffer.h"
    3131#include "LayoutRect.h"
     32#include "SurrogatePairAwareTextIterator.h"
    3233#include "TextRun.h"
    3334#include "WidthIterator.h"
     
    11981199// FIXME: This function may not work if the emphasis mark uses a complex script, but none of the
    11991200// standard emphasis marks do so.
    1200 bool FontCascade::getEmphasisMarkGlyphData(const AtomicString& mark, GlyphData& glyphData) const
     1201Optional<GlyphData> FontCascade::getEmphasisMarkGlyphData(const AtomicString& mark) const
    12011202{
    12021203    if (mark.isEmpty())
    1203         return false;
    1204 
    1205     UChar32 character = mark[0];
    1206 
    1207     if (U16_IS_SURROGATE(character)) {
    1208         if (!U16_IS_SURROGATE_LEAD(character))
    1209             return false;
    1210 
    1211         if (mark.length() < 2)
    1212             return false;
    1213 
    1214         UChar low = mark[1];
    1215         if (!U16_IS_TRAIL(low))
    1216             return false;
    1217 
    1218         character = U16_GET_SUPPLEMENTARY(character, low);
    1219     }
    1220 
    1221     glyphData = glyphDataForCharacter(character, false, EmphasisMarkVariant);
    1222     return true;
     1204        return Nullopt;
     1205
     1206    UChar32 character;
     1207    if (!mark.is8Bit()) {
     1208        SurrogatePairAwareTextIterator iterator(mark.characters16(), 0, mark.length(), mark.length());
     1209        unsigned clusterLength;
     1210        if (!iterator.consume(character, clusterLength))
     1211            return Nullopt;
     1212    } else
     1213        character = mark[0];
     1214
     1215    Optional<GlyphData> glyphData(glyphDataForCharacter(character, false, EmphasisMarkVariant));
     1216    return glyphData.value().isValid() ? glyphData : Nullopt;
    12231217}
    12241218
    12251219int FontCascade::emphasisMarkAscent(const AtomicString& mark) const
    12261220{
    1227     GlyphData markGlyphData;
    1228     if (!getEmphasisMarkGlyphData(mark, markGlyphData))
     1221    Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
     1222    if (!markGlyphData)
    12291223        return 0;
    12301224
    1231     const Font* markFontData = markGlyphData.font;
     1225    const Font* markFontData = markGlyphData.value().font;
    12321226    ASSERT(markFontData);
    12331227    if (!markFontData)
     
    12391233int FontCascade::emphasisMarkDescent(const AtomicString& mark) const
    12401234{
    1241     GlyphData markGlyphData;
    1242     if (!getEmphasisMarkGlyphData(mark, markGlyphData))
     1235    Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
     1236    if (!markGlyphData)
    12431237        return 0;
    12441238
    1245     const Font* markFontData = markGlyphData.font;
     1239    const Font* markFontData = markGlyphData.value().font;
    12461240    ASSERT(markFontData);
    12471241    if (!markFontData)
     
    12531247int FontCascade::emphasisMarkHeight(const AtomicString& mark) const
    12541248{
    1255     GlyphData markGlyphData;
    1256     if (!getEmphasisMarkGlyphData(mark, markGlyphData))
     1249    Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
     1250    if (!markGlyphData)
    12571251        return 0;
    12581252
    1259     const Font* markFontData = markGlyphData.font;
     1253    const Font* markFontData = markGlyphData.value().font;
    12601254    ASSERT(markFontData);
    12611255    if (!markFontData)
     
    13901384void FontCascade::drawEmphasisMarks(GraphicsContext& context, const TextRun& run, const GlyphBuffer& glyphBuffer, const AtomicString& mark, const FloatPoint& point) const
    13911385{
    1392     GlyphData markGlyphData;
    1393     if (!getEmphasisMarkGlyphData(mark, markGlyphData))
     1386    Optional<GlyphData> markGlyphData = getEmphasisMarkGlyphData(mark);
     1387    if (!markGlyphData)
    13941388        return;
    13951389
    1396     const Font* markFontData = markGlyphData.font;
     1390    const Font* markFontData = markGlyphData.value().font;
    13971391    ASSERT(markFontData);
    13981392    if (!markFontData)
    13991393        return;
    14001394
    1401     Glyph markGlyph = markGlyphData.glyph;
     1395    Glyph markGlyph = markGlyphData.value().glyph;
    14021396    Glyph spaceGlyph = markFontData->spaceGlyph();
    14031397
  • trunk/Source/WebCore/platform/graphics/FontCascade.h

    r191331 r191386  
    3434#include <wtf/HashMap.h>
    3535#include <wtf/HashSet.h>
     36#include <wtf/Optional.h>
    3637#include <wtf/WeakPtr.h>
    3738#include <wtf/unicode/CharacterNames.h>
     
    232233    void adjustSelectionRectForSimpleText(const TextRun&, LayoutRect& selectionRect, int from, int to) const;
    233234
    234     bool getEmphasisMarkGlyphData(const AtomicString&, GlyphData&) const;
     235    Optional<GlyphData> getEmphasisMarkGlyphData(const AtomicString&) const;
    235236
    236237    static bool canReturnFallbackFontsForComplexText();
  • trunk/Source/WebCore/platform/graphics/GlyphPage.h

    r189539 r191386  
    5050    {
    5151    }
     52
     53    bool isValid() const { return glyph || font; }
     54
    5255    Glyph glyph;
    5356    const Font* font;
Note: See TracChangeset for help on using the changeset viewer.