Attachment 357679 Details for Bug 191976 – Patch

[patch] Patch

wk-colored-emojis.diff (text/plain), 18.88 KB, created by Carlos Garcia Campos on 2018-12-19 07:30:15 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Carlos Garcia Campos

Created: 2018-12-19 07:30:15 PST

Size: 18.88 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 28d445a7e29..959da17fe76 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-12-19 Carlos Garcia Campos <cgarcia@igalia.com>
>+
>+ [FreeType] Color emoji not properly supported
>+ https://bugs.webkit.org/show_bug.cgi?id=191976
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ Add new test to ensure we fallback to noto color emoji for emojis.
>+
>+ * platform/gtk/fonts/font-emoji-system-fallback-expected.html: Added.
>+ * platform/gtk/fonts/font-emoji-system-fallback.html: Added.
>+
> 2018-12-15 Youenn Fablet <youenn@apple.com>
> 
> Make RTCRtpSender.setParameters to activate specific encodings
>diff --git a/LayoutTests/platform/gtk/fonts/font-emoji-system-fallback-expected.html b/LayoutTests/platform/gtk/fonts/font-emoji-system-fallback-expected.html
>new file mode 100644
>index 00000000000..cb0c96ab2ce
>--- /dev/null
>+++ b/LayoutTests/platform/gtk/fonts/font-emoji-system-fallback-expected.html
>@@ -0,0 +1,2 @@
>+&#x1F601
>+
>diff --git a/LayoutTests/platform/gtk/fonts/font-emoji-system-fallback.html b/LayoutTests/platform/gtk/fonts/font-emoji-system-fallback.html
>new file mode 100644
>index 00000000000..b9ed0d9115f
>--- /dev/null
>+++ b/LayoutTests/platform/gtk/fonts/font-emoji-system-fallback.html
>@@ -0,0 +1 @@
>+&#x1F601
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index ca0e856cd23..3d12ed2abde 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-12-19 Carlos Garcia Campos <cgarcia@igalia.com>
>+
>+ [FreeType] Color emoji not properly supported
>+ https://bugs.webkit.org/show_bug.cgi?id=191976
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ Add a name for combining enclosing keycap character.
>+
>+ * wtf/unicode/CharacterNames.h:
>+
> 2018-12-15 Darin Adler <darin@apple.com>
> 
> Use warning-ignoring macros more consistently and simply
>diff --git a/Source/WTF/wtf/unicode/CharacterNames.h b/Source/WTF/wtf/unicode/CharacterNames.h
>index 696660370e4..967211c8126 100644
>--- a/Source/WTF/wtf/unicode/CharacterNames.h
>+++ b/Source/WTF/wtf/unicode/CharacterNames.h
>@@ -100,6 +100,7 @@ const UChar zeroWidthJoiner = 0x200D;
> const UChar zeroWidthNoBreakSpace = 0xFEFF;
> const UChar zeroWidthNonJoiner = 0x200C;
> const UChar zeroWidthSpace = 0x200B;
>+const UChar combiningEnclosingKeycap = 0x20E3;
> 
> } // namespace Unicode
> } // namespace WTF
>@@ -166,3 +167,4 @@ using WTF::Unicode::zeroWidthJoiner;
> using WTF::Unicode::zeroWidthNoBreakSpace;
> using WTF::Unicode::zeroWidthNonJoiner;
> using WTF::Unicode::zeroWidthSpace;
>+using WTF::Unicode::combiningEnclosingKeycap;
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 05949be24b6..bccab261d37 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2018-12-19 Carlos Garcia Campos <cgarcia@igalia.com>
>+
>+ [FreeType] Color emoji not properly supported
>+ https://bugs.webkit.org/show_bug.cgi?id=191976
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ Always try to fallback to a colored font for emojis.
>+
>+ Test: platform/gtk/fonts/font-emoji-system-fallback.html
>+
>+ * platform/graphics/ComplexTextController.cpp:
>+ (WebCore::advanceByCombiningCharacterSequence): Group regional indicators in pairs.
>+ * platform/graphics/Font.cpp:
>+ (WebCore::Font::systemFallbackFontForCharacter const): Pass false to coloredFont parameter.
>+ * platform/graphics/FontCache.h:
>+ * platform/graphics/cairo/FontCairoHarfbuzzNG.cpp:
>+ (WebCore::characterSequenceIsEmoji): Checke whether the character sequence is an emoji.
>+ (WebCore::FontCascade::fontForCombiningCharacterSequence const): In case of emojis try to fallback to a colored
>+ font even if base font can render the emoji in black and white.
>+ * platform/graphics/cocoa/FontCacheCoreText.cpp:
>+ (WebCore::FontCache::systemFallbackForCharacters): Add coloredFont parameter that is ignored.
>+ * platform/graphics/freetype/FontCacheFreeType.cpp:
>+ (WebCore::FontCache::systemFallbackForCharacters): Add coloredFont parameter used to prefer colored fonts when
>+ it's true.
>+ * platform/graphics/freetype/FontPlatformDataFreeType.cpp:
>+ (WebCore::FontPlatformData::FontPlatformData): Initialize m_isColorBitmapFont.
>+ * platform/graphics/freetype/SimpleFontDataFreeType.cpp:
>+ (WebCore::Font::variantCapsSupportsCharacterForSynthesis const): Moved from cross-platform file.
>+ (WebCore::Font::platformSupportsCodePoint const): Add freetype implementation.
>+ * platform/graphics/win/FontCacheWin.cpp:
>+ (WebCore::FontCache::systemFallbackForCharacters): Add coloredFont parameter that is ignored.
>+ * platform/text/CharacterProperties.h:
>+ (WebCore::isEmojiKeycapBase):
>+ (WebCore::isEmojiRegionalIndicator):
>+ (WebCore::isEmojiWithPresentationByDefault):
>+ (WebCore::isEmojiModifierBase):
>+
> 2018-12-15 Yusuke Suzuki <yusukesuzuki@slowstart.org>
> 
> Null pointer dereference in JSC::WriteBarrierBase()
>diff --git a/Source/WebCore/platform/graphics/ComplexTextController.cpp b/Source/WebCore/platform/graphics/ComplexTextController.cpp
>index e07a72d8648..f938b736247 100644
>--- a/Source/WebCore/platform/graphics/ComplexTextController.cpp
>+++ b/Source/WebCore/platform/graphics/ComplexTextController.cpp
>@@ -286,6 +286,7 @@ static bool advanceByCombiningCharacterSequence(const UChar*& iterator, const UC
> // Consume marks.
> bool sawEmojiGroupCandidate = isEmojiGroupCandidate(baseCharacter);
> bool sawJoiner = false;
>+ bool sawRegionalIndicator = isEmojiRegionalIndicator(baseCharacter);
> while (iterator < end) {
> UChar32 nextCharacter;
> unsigned markLength = 0;
>@@ -296,6 +297,11 @@ static bool advanceByCombiningCharacterSequence(const UChar*& iterator, const UC
> if (isVariationSelector(nextCharacter) || isEmojiFitzpatrickModifier(nextCharacter))
> shouldContinue = true;
> 
>+ if (sawRegionalIndicator && isEmojiRegionalIndicator(nextCharacter)) {
>+ shouldContinue = true;
>+ sawRegionalIndicator = false;
>+ }
>+
> if (sawJoiner && isEmojiGroupCandidate(nextCharacter))
> shouldContinue = true;
> 
>diff --git a/Source/WebCore/platform/graphics/Font.cpp b/Source/WebCore/platform/graphics/Font.cpp
>index 2ddc79c6d6d..20b2398d280 100644
>--- a/Source/WebCore/platform/graphics/Font.cpp
>+++ b/Source/WebCore/platform/graphics/Font.cpp
>@@ -566,7 +566,7 @@ RefPtr Font::systemFallbackFontForCharacter(UChar32 character, const FontD
> 
> if (!character) {
> UChar codeUnit = 0;
>- return FontCache::singleton().systemFallbackForCharacters(description, this, isForPlatformFont, &codeUnit, 1);
>+ return FontCache::singleton().systemFallbackForCharacters(description, this, isForPlatformFont, false, &codeUnit, 1);
> }
> 
> auto key = CharacterFallbackMapKey(description.locale(), character, isForPlatformFont);
>@@ -586,7 +586,7 @@ RefPtr Font::systemFallbackFontForCharacter(UChar32 character, const FontD
> codeUnitsLength = 2;
> }
> 
>- fallbackFont = FontCache::singleton().systemFallbackForCharacters(description, this, isForPlatformFont, codeUnits, codeUnitsLength).get();
>+ fallbackFont = FontCache::singleton().systemFallbackForCharacters(description, this, isForPlatformFont, false, codeUnits, codeUnitsLength).get();
> if (fallbackFont)
> fallbackFont->m_isUsedInSystemFallbackCache = true;
> }
>@@ -612,7 +612,7 @@ void Font::removeFromSystemFallbackCache()
> }
> }
> 
>-#if !PLATFORM(COCOA)
>+#if !PLATFORM(COCOA) && !USE(FREETYPE)
> bool Font::variantCapsSupportsCharacterForSynthesis(FontVariantCaps fontVariantCaps, UChar32) const
> {
> switch (fontVariantCaps) {
>diff --git a/Source/WebCore/platform/graphics/FontCache.h b/Source/WebCore/platform/graphics/FontCache.h
>index 396036a1fee..523231b1990 100644
>--- a/Source/WebCore/platform/graphics/FontCache.h
>+++ b/Source/WebCore/platform/graphics/FontCache.h
>@@ -190,7 +190,7 @@ public:
> WEBCORE_EXPORT static FontCache& singleton();
> 
> // These methods are implemented by the platform.
>- RefPtr systemFallbackForCharacters(const FontDescription&, const Font* originalFontData, bool isPlatformFont, const UChar* characters, unsigned length);
>+ RefPtr systemFallbackForCharacters(const FontDescription&, const Font* originalFontData, bool isPlatformFont, bool coloredFont, const UChar* characters, unsigned length);
> Vector<String> systemFontFamilies();
> void platformInit();
> 
>diff --git a/Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp b/Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp
>index 9efe6f587c7..b9736358301 100644
>--- a/Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp
>+++ b/Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp
>@@ -29,6 +29,7 @@
> 
> #if USE(CAIRO)
> 
>+#include "CharacterProperties.h"
> #include "FontCache.h"
> #include "SurrogatePairAwareTextIterator.h"
> #include <unicode/normlzr.h>
>@@ -45,6 +46,60 @@ bool FontCascade::canExpandAroundIdeographsInComplexText()
> return false;
> }
> 
>+static bool characterSequenceIsEmoji(const Vector<UChar, 4>& normalizedCharacters, int32_t normalizedLength)
>+{
>+ UChar32 character;
>+ unsigned clusterLength = 0;
>+ SurrogatePairAwareTextIterator iterator(normalizedCharacters.data(), 0, normalizedLength, normalizedLength);
>+ if (!iterator.consume(character, clusterLength))
>+ return false;
>+
>+ if (isEmojiKeycapBase(character)) {
>+ iterator.advance(clusterLength);
>+ UChar32 nextCharacter;
>+ if (!iterator.consume(nextCharacter, clusterLength))
>+ return false;
>+
>+ if (nextCharacter == combiningEnclosingKeycap)
>+ return true;
>+
>+ // Variation selector 16.
>+ if (nextCharacter == 0xFE0F) {
>+ iterator.advance(clusterLength);
>+ if (!iterator.consume(nextCharacter, clusterLength))
>+ return false;
>+
>+ if (nextCharacter == combiningEnclosingKeycap)
>+ return true;
>+ }
>+
>+ return false;
>+ }
>+
>+ // Regional indicator.
>+ if (isEmojiRegionalIndicator(character)) {
>+ iterator.advance(clusterLength);
>+ UChar32 nextCharacter;
>+ if (!iterator.consume(nextCharacter, clusterLength))
>+ return false;
>+
>+ if (isEmojiRegionalIndicator(nextCharacter))
>+ return true;
>+
>+ return false;
>+ }
>+
>+ if (character == combiningEnclosingKeycap)
>+ return true;
>+
>+ if (isEmojiWithPresentationByDefault(character)
>+ || isEmojiModifierBase(character)
>+ || isEmojiFitzpatrickModifier(character))
>+ return true;
>+
>+ return false;
>+}
>+
> const Font* FontCascade::fontForCombiningCharacterSequence(const UChar* characters, size_t length) const
> {
> UErrorCode error = U_ZERO_ERROR;
>@@ -61,8 +116,12 @@ const Font* FontCascade::fontForCombiningCharacterSequence(const UChar* characte
> if (!iterator.consume(character, clusterLength))
> return nullptr;
> 
>+ bool isEmoji = characterSequenceIsEmoji(normalizedCharacters, normalizedLength);
>+
> const Font* baseFont = glyphDataForCharacter(character, false, NormalVariant).font;
>- if (baseFont && (static_cast<int32_t>(clusterLength) == normalizedLength || baseFont->canRenderCombiningCharacterSequence(characters, length)))
>+ if (baseFont
>+ && (static_cast<int32_t>(clusterLength) == normalizedLength || baseFont->canRenderCombiningCharacterSequence(characters, length))
>+ && (!isEmoji || baseFont->platformData().isColorBitmapFont()))
> return baseFont;
> 
> for (unsigned i = 0; !fallbackRangesAt(i).isNull(); ++i) {
>@@ -70,12 +129,12 @@ const Font* FontCascade::fontForCombiningCharacterSequence(const UChar* characte
> if (!fallbackFont || fallbackFont == baseFont)
> continue;
> 
>- if (fallbackFont->canRenderCombiningCharacterSequence(characters, length))
>+ if (fallbackFont->canRenderCombiningCharacterSequence(characters, length) && (!isEmoji || fallbackFont->platformData().isColorBitmapFont()))
> return fallbackFont;
> }
> 
>- if (auto systemFallback = FontCache::singleton().systemFallbackForCharacters(m_fontDescription, baseFont, false, characters, length)) {
>- if (systemFallback->canRenderCombiningCharacterSequence(characters, length))
>+ if (auto systemFallback = FontCache::singleton().systemFallbackForCharacters(m_fontDescription, baseFont, false, isEmoji, characters, length)) {
>+ if (systemFallback->canRenderCombiningCharacterSequence(characters, length) && (!isEmoji || systemFallback->platformData().isColorBitmapFont()))
> return systemFallback.get();
> }
> 
>diff --git a/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp b/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp
>index 30244cf6582..ade8b6145a4 100644
>--- a/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp
>+++ b/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp
>@@ -1354,7 +1354,7 @@ static RetainPtr<CTFontRef> lookupFallbackFont(CTFontRef font, FontSelectionValu
> return result;
> }
> 
>-RefPtr FontCache::systemFallbackForCharacters(const FontDescription& description, const Font* originalFontData, bool isPlatformFont, const UChar* characters, unsigned length)
>+RefPtr FontCache::systemFallbackForCharacters(const FontDescription& description, const Font* originalFontData, bool isPlatformFont, bool, const UChar* characters, unsigned length)
> {
> #if PLATFORM(IOS_FAMILY)
> if (length && requiresCustomFallbackFont(*characters)) {
>diff --git a/Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp b/Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp
>index c446edb5107..722ae1f05eb 100644
>--- a/Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp
>+++ b/Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp
>@@ -118,7 +118,7 @@ static void getFontPropertiesFromPattern(FcPattern* pattern, const FontDescripti
> }
> }
> 
>-RefPtr FontCache::systemFallbackForCharacters(const FontDescription& description, const Font*, bool, const UChar* characters, unsigned length)
>+RefPtr FontCache::systemFallbackForCharacters(const FontDescription& description, const Font*, bool, bool coloredFont, const UChar* characters, unsigned length)
> {
> FcUniquePtr<FcCharSet> fontConfigCharSet(FcCharSetCreate());
> UTF16UChar32Iterator iterator(characters, length);
>@@ -132,6 +132,8 @@ RefPtr FontCache::systemFallbackForCharacters(const FontDescription& descr
> FcPatternAddCharSet(pattern.get(), FC_CHARSET, fontConfigCharSet.get());
> 
> FcPatternAddBool(pattern.get(), FC_SCALABLE, FcTrue);
>+ if (coloredFont)
>+ FcPatternAddBool(pattern.get(), FC_COLOR, FcTrue);
> 
> if (!configurePatternForFontDescription(pattern.get(), description))
> return nullptr;
>diff --git a/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp b/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp
>index 645cd173528..e7ff7d99f10 100644
>--- a/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp
>+++ b/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp
>@@ -32,6 +32,7 @@
> #include <cairo-ft.h>
> #include <fontconfig/fcfreetype.h>
> #include <ft2build.h>
>+#include FT_FREETYPE_H
> #include FT_TRUETYPE_TABLES_H
> #include <hb-ft.h>
> #include <hb-ot.h>
>@@ -117,6 +118,12 @@ FontPlatformData::FontPlatformData(cairo_font_face_t* fontFace, FcPattern* patte
> m_fixedWidth = fixedWidth;
> 
> buildScaledFont(fontFace);
>+
>+#ifdef FT_HAS_COLOR
>+ CairoFtFaceLocker cairoFtFaceLocker(m_scaledFont.get());
>+ if (FT_Face ftFace = cairoFtFaceLocker.ftFace())
>+ m_isColorBitmapFont = FT_HAS_COLOR(ftFace);
>+#endif
> }
> 
> FontPlatformData::FontPlatformData(const FontPlatformData& other)
>diff --git a/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp b/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp
>index 4bff7cc83d0..d137845c788 100644
>--- a/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp
>+++ b/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp
>@@ -188,4 +188,27 @@ float Font::platformWidthForGlyph(Glyph glyph) const
> return width ? width : m_spaceWidth;
> }
> 
>+bool Font::variantCapsSupportsCharacterForSynthesis(FontVariantCaps fontVariantCaps, UChar32) const
>+{
>+ switch (fontVariantCaps) {
>+ case FontVariantCaps::Small:
>+ case FontVariantCaps::Petite:
>+ case FontVariantCaps::AllSmall:
>+ case FontVariantCaps::AllPetite:
>+ return false;
>+ default:
>+ // Synthesis only supports the variant-caps values listed above.
>+ return true;
>+ }
>+}
>+
>+bool Font::platformSupportsCodePoint(UChar32 character) const
>+{
>+ CairoFtFaceLocker cairoFtFaceLocker(m_platformData.scaledFont());
>+ if (FT_Face face = cairoFtFaceLocker.ftFace())
>+ return !!FcFreeTypeCharIndex(face, character);
>+
>+ return false;
>+}
>+
> }
>diff --git a/Source/WebCore/platform/graphics/win/FontCacheWin.cpp b/Source/WebCore/platform/graphics/win/FontCacheWin.cpp
>index f2d660da9fb..7f757143683 100644
>--- a/Source/WebCore/platform/graphics/win/FontCacheWin.cpp
>+++ b/Source/WebCore/platform/graphics/win/FontCacheWin.cpp
>@@ -211,7 +211,7 @@ static HFONT createMLangFont(IMLangFontLinkType* langFontLink, HDC hdc, DWORD co
> return hfont;
> }
> 
>-RefPtr FontCache::systemFallbackForCharacters(const FontDescription& description, const Font* originalFontData, bool, const UChar* characters, unsigned length)
>+RefPtr FontCache::systemFallbackForCharacters(const FontDescription& description, const Font* originalFontData, bool, bool, const UChar* characters, unsigned length)
> {
> UChar character = characters[0];
> RefPtr fontData;
>diff --git a/Source/WebCore/platform/text/CharacterProperties.h b/Source/WebCore/platform/text/CharacterProperties.h
>index c6530a48aca..e20d680137c 100644
>--- a/Source/WebCore/platform/text/CharacterProperties.h
>+++ b/Source/WebCore/platform/text/CharacterProperties.h
>@@ -76,4 +76,34 @@ inline bool isVariationSelector(UChar32 character)
> return character >= 0xFE00 && character <= 0xFE0F;
> }
> 
>+inline bool isEmojiKeycapBase(UChar32 character)
>+{
>+ return (character >= '0' && character <= '9') || character == '#' || character == '*';
>+}
>+
>+inline bool isEmojiRegionalIndicator(UChar32 character)
>+{
>+ return character >= 0x1F1E6 && character <= 0x1F1FF;
>+}
>+
>+inline bool isEmojiWithPresentationByDefault(UChar32 character)
>+{
>+#if U_ICU_VERSION_MAJOR_NUM >= 57
>+ return u_hasBinaryProperty(character, UCHAR_EMOJI_PRESENTATION);
>+#else
>+ UNUSED_PARAM(character);
>+ return false;
>+#endif
>+}
>+
>+inline bool isEmojiModifierBase(UChar32 character)
>+{
>+#if U_ICU_VERSION_MAJOR_NUM >= 57
>+ return u_hasBinaryProperty(character, UCHAR_EMOJI_MODIFIER_BASE);
>+#else
>+ UNUSED_PARAM(character);
>+ return false;
>+#endif
>+}
>+
> }

Flags:

mcatanzaro: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 191976: 355977 | 357567 | 357679 | 358583