Changeset 245393 in webkit


Ignore:
Timestamp:
May 16, 2019 2:31:16 AM (5 years ago)
Author:
Carlos Garcia Campos
Message:

[FreeType] Some character sequences with a variation selector are not rendered
https://bugs.webkit.org/show_bug.cgi?id=197838

Reviewed by Michael Catanzaro.

We get the invalid glyph instead. See http://mts.io/2015/04/21/unicode-symbol-render-text-emoji/. In the table at
the end the Emoji and Text columns are not correctly rendered. It happens also when copying an emoji from
GtkEmojiChooser and pasting in WebKit text field, because GTK appends U+FE0F to all emojis to force the emoji
style. We need to take into account the variation selector when checking if a font can render a combining
sequence, using FT_Face_GetCharVariantIndex to get the right glyph in case of variation character present.

  • platform/graphics/Font.cpp:

(WebCore::Font::platformSupportsCodePoint const): Add optional variation parameter.
(WebCore::Font::canRenderCombiningCharacterSequence const): Take into account variation selector characters

  • platform/graphics/Font.h:
  • platform/graphics/cairo/FontCairoHarfbuzzNG.cpp:

(WebCore::FontCascade::fontForCombiningCharacterSequence const): Check variation selectors 0xFE0E and 0xFE0F to
decide whether to use the emoji or text style.

  • platform/graphics/cocoa/FontCocoa.mm:

(WebCore::Font::platformSupportsCodePoint const): Return false when a variation character is passed so that
characters are checked individually.

  • platform/graphics/freetype/SimpleFontDataFreeType.cpp:

(WebCore::Font::platformSupportsCodePoint const): Use FT_Face_GetCharVariantIndex when a variation character is
passed.

  • platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:

(WebCore::harfBuzzFontFunctions): Do not return true when FT_Face_GetCharVariantIndex returns 0.

Location:
trunk/Source/WebCore
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r245392 r245393  
     12019-05-16  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [FreeType] Some character sequences with a variation selector are not rendered
     4        https://bugs.webkit.org/show_bug.cgi?id=197838
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        We get the invalid glyph instead. See http://mts.io/2015/04/21/unicode-symbol-render-text-emoji/. In the table at
     9        the end the Emoji and Text columns are not correctly rendered. It happens also when copying an emoji from
     10        GtkEmojiChooser and pasting in WebKit text field, because GTK appends U+FE0F to all emojis to force the emoji
     11        style. We need to take into account the variation selector when checking if a font can render a combining
     12        sequence, using FT_Face_GetCharVariantIndex to get the right glyph in case of variation character present.
     13
     14        * platform/graphics/Font.cpp:
     15        (WebCore::Font::platformSupportsCodePoint const): Add optional variation parameter.
     16        (WebCore::Font::canRenderCombiningCharacterSequence const): Take into account variation selector characters
     17        * platform/graphics/Font.h:
     18        * platform/graphics/cairo/FontCairoHarfbuzzNG.cpp:
     19        (WebCore::FontCascade::fontForCombiningCharacterSequence const): Check variation selectors 0xFE0E and 0xFE0F to
     20        decide whether to use the emoji or text style.
     21        * platform/graphics/cocoa/FontCocoa.mm:
     22        (WebCore::Font::platformSupportsCodePoint const): Return false when a variation character is passed so that
     23        characters are checked individually.
     24        * platform/graphics/freetype/SimpleFontDataFreeType.cpp:
     25        (WebCore::Font::platformSupportsCodePoint const): Use FT_Face_GetCharVariantIndex when a variation character is
     26        passed.
     27        * platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:
     28        (WebCore::harfBuzzFontFunctions): Do not return true when FT_Face_GetCharVariantIndex returns 0.
     29
    1302019-05-16  Greg Hughes  <ghughes@apple.com>
    231
  • trunk/Source/WebCore/platform/graphics/Font.cpp

    r245094 r245393  
    3434#include <pal/spi/cocoa/CoreTextSPI.h>
    3535#endif
     36#include "CharacterProperties.h"
    3637#include "FontCache.h"
    3738#include "FontCascade.h"
     
    642643}
    643644
    644 bool Font::platformSupportsCodePoint(UChar32 character) const
    645 {
    646     return glyphForCharacter(character);
     645bool Font::platformSupportsCodePoint(UChar32 character, Optional<UChar32> variation) const
     646{
     647    return variation ? false : glyphForCharacter(character);
    647648}
    648649#endif
     
    672673    ASSERT(isMainThread());
    673674
    674     for (UChar32 codePoint : StringView(characters, length).codePoints()) {
     675    auto codePoints = StringView(characters, length).codePoints();
     676    auto it = codePoints.begin();
     677    auto end = codePoints.end();
     678    while (it != end) {
     679        auto codePoint = *it;
     680        ++it;
     681
     682        if (it != end && isVariationSelector(*it)) {
     683            if (!platformSupportsCodePoint(codePoint, *it)) {
     684                // Try the characters individually.
     685                if (!supportsCodePoint(codePoint) || !supportsCodePoint(*it))
     686                    return false;
     687            }
     688            ++it;
     689            continue;
     690        }
     691
    675692        if (!supportsCodePoint(codePoint))
    676693            return false;
  • trunk/Source/WebCore/platform/graphics/Font.h

    r239822 r245393  
    176176    Glyph glyphForCharacter(UChar32) const;
    177177    bool supportsCodePoint(UChar32) const;
    178     bool platformSupportsCodePoint(UChar32) const;
     178    bool platformSupportsCodePoint(UChar32, Optional<UChar32> variation = WTF::nullopt) const;
    179179
    180180    RefPtr<Font> systemFallbackFontForCharacter(UChar32, const FontDescription&, IsForPlatformFont) const;
  • trunk/Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp

    r243049 r245393  
    116116
    117117    bool isEmoji = characterSequenceIsEmoji(iterator, character, clusterLength);
     118    bool preferColoredFont = isEmoji;
     119    // U+FE0E forces text style.
     120    // U+FE0F forces emoji style.
     121    if (characters[length - 1] == 0xFE0E)
     122        preferColoredFont = false;
     123    else if (characters[length - 1] == 0xFE0F)
     124        preferColoredFont = true;
    118125
    119126    const Font* baseFont = glyphDataForCharacter(character, false, NormalVariant).font;
    120127    if (baseFont
    121128        && (clusterLength == length || baseFont->canRenderCombiningCharacterSequence(characters, length))
    122         && (!isEmoji || baseFont->platformData().isColorBitmapFont()))
     129        && (!preferColoredFont || baseFont->platformData().isColorBitmapFont()))
    123130        return baseFont;
    124131
     
    128135            continue;
    129136
    130         if (fallbackFont->canRenderCombiningCharacterSequence(characters, length) && (!isEmoji || fallbackFont->platformData().isColorBitmapFont()))
     137        if (fallbackFont->canRenderCombiningCharacterSequence(characters, length) && (!preferColoredFont || fallbackFont->platformData().isColorBitmapFont()))
    131138            return fallbackFont;
    132139    }
    133140
    134     if (auto systemFallback = FontCache::singleton().systemFallbackForCharacters(m_fontDescription, baseFont, IsForPlatformFont::No, isEmoji ? FontCache::PreferColoredFont::Yes : FontCache::PreferColoredFont::No, characters, length)) {
    135         if (systemFallback->canRenderCombiningCharacterSequence(characters, length) && (!isEmoji || systemFallback->platformData().isColorBitmapFont()))
     141    if (auto systemFallback = FontCache::singleton().systemFallbackForCharacters(m_fontDescription, baseFont, IsForPlatformFont::No, preferColoredFont ? FontCache::PreferColoredFont::Yes : FontCache::PreferColoredFont::No, characters, length)) {
     142        if (systemFallback->canRenderCombiningCharacterSequence(characters, length) && (!preferColoredFont || systemFallback->platformData().isColorBitmapFont()))
     143            return systemFallback.get();
     144
     145        // In case of emoji, if fallback font is colored try again without the variation selector character.
     146        if (isEmoji && characters[length - 1] == 0xFE0F && systemFallback->platformData().isColorBitmapFont() && systemFallback->canRenderCombiningCharacterSequence(characters, length - 1))
    136147            return systemFallback.get();
    137148    }
  • trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm

    r239427 r245393  
    614614}
    615615
    616 bool Font::platformSupportsCodePoint(UChar32 character) const
    617 {
     616bool Font::platformSupportsCodePoint(UChar32 character, Optional<UChar32> variation) const
     617{
     618    if (variation)
     619        return false;
     620
    618621    UniChar codeUnits[2];
    619622    CGGlyph glyphs[2];
  • trunk/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp

    r243049 r245393  
    202202}
    203203
    204 bool Font::platformSupportsCodePoint(UChar32 character) const
     204bool Font::platformSupportsCodePoint(UChar32 character, Optional<UChar32> variation) const
    205205{
    206206    CairoFtFaceLocker cairoFtFaceLocker(m_platformData.scaledFont());
    207207    if (FT_Face face = cairoFtFaceLocker.ftFace())
    208         return !!FcFreeTypeCharIndex(face, character);
     208        return variation ? !!FT_Face_GetCharVariantIndex(face, character, variation.value()) : !!FcFreeTypeCharIndex(face, character);
    209209
    210210    return false;
  • trunk/Source/WebCore/platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp

    r243602 r245393  
    7979            if (FT_Face ftFace = cairoFtFaceLocker.ftFace()) {
    8080                *glyph = FT_Face_GetCharVariantIndex(ftFace, unicode, variation);
    81                 return true;
     81                return !!*glyph;
    8282            }
    8383            return false;
Note: See TracChangeset for help on using the changeset viewer.