[FreeType] Support emoji modifiers
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Jan 2019 07:59:00 +0000 (07:59 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 Jan 2019 07:59:00 +0000 (07:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=177040

Reviewed by Myles C. Maxfield.

Source/WebCore:

The problem only happens with emojis having the zero with joiner (U+200D) in the sequence. The sequence is
broken because createAndFillGlyphPage() in Font.cpp overwrites zero with joiner with zero width space (U+200B),
but the emoji font actually supports zero with joiner. This patch moves the control characters override from
createAndFillGlyphPage() to GlyphPage::fill() only for FreeType based ports. This way we can do the override
only for the cases where the code point is not supported by the font.

* platform/graphics/Font.cpp:
(WebCore::overrideControlCharacters): Helper function to override the control characters.
(WebCore::createAndFillGlyphPage): Call overrideControlCharacters() only when not using FreeType.
* platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:
(WebCore::GlyphPage::fill): Use zero width space as fallback when the font doesn't support characters with
Default_Ignorable Unicode property.

LayoutTests:

Mark several emoji tests as passing now.

* platform/gtk/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/platform/gtk/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Font.cpp
Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp

index 0f9c05a..ea849cd 100644 (file)
@@ -1,3 +1,14 @@
+2019-01-13  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [FreeType] Support emoji modifiers
+        https://bugs.webkit.org/show_bug.cgi?id=177040
+
+        Reviewed by Myles C. Maxfield.
+
+        Mark several emoji tests as passing now.
+
+        * platform/gtk/TestExpectations:
+
 2019-01-13  Antti Koivisto  <antti@apple.com>
 
         Release assert with <img usemap> in shadow tree
index 9a85645..d43148c 100644 (file)
@@ -3780,6 +3780,12 @@ fast/text/emoji-gender-2-6.html [ Pass ]
 fast/text/emoji-gender-2-7.html [ Pass ]
 fast/text/emoji-gender-2-8.html [ Pass ]
 fast/text/emoji-gender-2-9.html [ Pass ]
+fast/text/emoji-gender-3.html [ Pass ]
+fast/text/emoji-gender-4.html [ Pass ]
+fast/text/emoji-gender-5.html [ Pass ]
+fast/text/emoji-gender-6.html [ Pass ]
+fast/text/emoji-gender-8.html [ Pass ]
+fast/text/emoji-gender-9.html [ Pass ]
 fast/text/emoji-gender.html [ Pass ]
 fast/text/emoji-gender-fe0f-3.html [ Pass ]
 fast/text/emoji-gender-fe0f-4.html [ Pass ]
index 1ba8143..28e3308 100644 (file)
@@ -1,3 +1,23 @@
+2019-01-13  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [FreeType] Support emoji modifiers
+        https://bugs.webkit.org/show_bug.cgi?id=177040
+
+        Reviewed by Myles C. Maxfield.
+
+        The problem only happens with emojis having the zero with joiner (U+200D) in the sequence. The sequence is
+        broken because createAndFillGlyphPage() in Font.cpp overwrites zero with joiner with zero width space (U+200B),
+        but the emoji font actually supports zero with joiner. This patch moves the control characters override from
+        createAndFillGlyphPage() to GlyphPage::fill() only for FreeType based ports. This way we can do the override
+        only for the cases where the code point is not supported by the font.
+
+        * platform/graphics/Font.cpp:
+        (WebCore::overrideControlCharacters): Helper function to override the control characters.
+        (WebCore::createAndFillGlyphPage): Call overrideControlCharacters() only when not using FreeType.
+        * platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:
+        (WebCore::GlyphPage::fill): Use zero width space as fallback when the font doesn't support characters with
+        Default_Ignorable Unicode property.
+
 2019-01-13  Dan Bernstein  <mitz@apple.com>
 
         More build fixing.
index d03c25f..b6c1164 100644 (file)
@@ -263,6 +263,50 @@ static Optional<size_t> codePointSupportIndex(UChar32 codePoint)
     return result;
 }
 
+#if !USE(FREETYPE)
+static void overrideControlCharacters(Vector<UChar>& buffer, unsigned start, unsigned end)
+{
+    auto overwriteCodePoints = [&](unsigned minimum, unsigned maximum, UChar newCodePoint) {
+        unsigned begin = std::max(start, minimum);
+        unsigned complete = std::min(end, maximum);
+        for (unsigned i = begin; i < complete; ++i) {
+            ASSERT(codePointSupportIndex(i));
+            buffer[i - start] = newCodePoint;
+        }
+    };
+
+    auto overwriteCodePoint = [&](UChar codePoint, UChar newCodePoint) {
+        ASSERT(codePointSupportIndex(codePoint));
+        if (codePoint >= start && codePoint < end)
+            buffer[codePoint - start] = newCodePoint;
+    };
+
+    // Code points 0x0 - 0x20 and 0x7F - 0xA0 are control character and shouldn't render. Map them to ZERO WIDTH SPACE.
+    overwriteCodePoints(0x0, 0x20, zeroWidthSpace);
+    overwriteCodePoints(0x7F, 0xA0, zeroWidthSpace);
+    overwriteCodePoint(softHyphen, zeroWidthSpace);
+    overwriteCodePoint('\n', space);
+    overwriteCodePoint('\t', space);
+    overwriteCodePoint(noBreakSpace, space);
+    overwriteCodePoint(narrowNoBreakSpace, zeroWidthSpace);
+    overwriteCodePoint(leftToRightMark, zeroWidthSpace);
+    overwriteCodePoint(rightToLeftMark, zeroWidthSpace);
+    overwriteCodePoint(leftToRightEmbed, zeroWidthSpace);
+    overwriteCodePoint(rightToLeftEmbed, zeroWidthSpace);
+    overwriteCodePoint(leftToRightOverride, zeroWidthSpace);
+    overwriteCodePoint(rightToLeftOverride, zeroWidthSpace);
+    overwriteCodePoint(leftToRightIsolate, zeroWidthSpace);
+    overwriteCodePoint(rightToLeftIsolate, zeroWidthSpace);
+    overwriteCodePoint(zeroWidthNonJoiner, zeroWidthSpace);
+    overwriteCodePoint(zeroWidthJoiner, zeroWidthSpace);
+    overwriteCodePoint(popDirectionalFormatting, zeroWidthSpace);
+    overwriteCodePoint(popDirectionalIsolate, zeroWidthSpace);
+    overwriteCodePoint(firstStrongIsolate, zeroWidthSpace);
+    overwriteCodePoint(objectReplacementCharacter, zeroWidthSpace);
+    overwriteCodePoint(zeroWidthNoBreakSpace, zeroWidthSpace);
+}
+#endif
+
 static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font& font)
 {
 #if PLATFORM(IOS_FAMILY)
@@ -276,7 +320,6 @@ static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font&
     unsigned glyphPageSize = GlyphPage::sizeForPageNumber(pageNumber);
 
     unsigned start = GlyphPage::startingCodePointInPageNumber(pageNumber);
-    unsigned end = start + glyphPageSize;
     Vector<UChar> buffer(glyphPageSize * 2 + 2);
     unsigned bufferLength;
     // Fill in a buffer with the entire "page" of characters that we want to look up glyphs for.
@@ -285,44 +328,9 @@ static RefPtr<GlyphPage> createAndFillGlyphPage(unsigned pageNumber, const Font&
         for (unsigned i = 0; i < bufferLength; i++)
             buffer[i] = start + i;
 
-        // Code points 0x0 - 0x20 and 0x7F - 0xA0 are control character and shouldn't render. Map them to ZERO WIDTH SPACE.
-        auto overwriteCodePoints = [&](unsigned minimum, unsigned maximum, UChar newCodePoint) {
-            unsigned begin = std::max(start, minimum);
-            unsigned complete = std::min(end, maximum);
-            for (unsigned i = begin; i < complete; ++i) {
-                ASSERT(codePointSupportIndex(i));
-                buffer[i - start] = newCodePoint;
-            }
-        };
-
-        auto overwriteCodePoint = [&](UChar codePoint, UChar newCodePoint) {
-            ASSERT(codePointSupportIndex(codePoint));
-            if (codePoint >= start && codePoint < end)
-                buffer[codePoint - start] = newCodePoint;
-        };
-
-        overwriteCodePoints(0x0, 0x20, zeroWidthSpace);
-        overwriteCodePoints(0x7F, 0xA0, zeroWidthSpace);
-        overwriteCodePoint(softHyphen, zeroWidthSpace);
-        overwriteCodePoint('\n', space);
-        overwriteCodePoint('\t', space);
-        overwriteCodePoint(noBreakSpace, space);
-        overwriteCodePoint(narrowNoBreakSpace, zeroWidthSpace);
-        overwriteCodePoint(leftToRightMark, zeroWidthSpace);
-        overwriteCodePoint(rightToLeftMark, zeroWidthSpace);
-        overwriteCodePoint(leftToRightEmbed, zeroWidthSpace);
-        overwriteCodePoint(rightToLeftEmbed, zeroWidthSpace);
-        overwriteCodePoint(leftToRightOverride, zeroWidthSpace);
-        overwriteCodePoint(rightToLeftOverride, zeroWidthSpace);
-        overwriteCodePoint(leftToRightIsolate, zeroWidthSpace);
-        overwriteCodePoint(rightToLeftIsolate, zeroWidthSpace);
-        overwriteCodePoint(zeroWidthNonJoiner, zeroWidthSpace);
-        overwriteCodePoint(zeroWidthJoiner, zeroWidthSpace);
-        overwriteCodePoint(popDirectionalFormatting, zeroWidthSpace);
-        overwriteCodePoint(popDirectionalIsolate, zeroWidthSpace);
-        overwriteCodePoint(firstStrongIsolate, zeroWidthSpace);
-        overwriteCodePoint(objectReplacementCharacter, zeroWidthSpace);
-        overwriteCodePoint(zeroWidthNoBreakSpace, zeroWidthSpace);
+#if !USE(FREETYPE)
+        overrideControlCharacters(buffer, start, start + glyphPageSize);
+#endif
     } else {
         bufferLength = glyphPageSize * 2;
         for (unsigned i = 0; i < glyphPageSize; i++) {
index caa4b8d..dcdd26d 100644 (file)
@@ -33,6 +33,7 @@
 
 #include "CairoUtilities.h"
 #include "Font.h"
+#include "FontCascade.h"
 #include "UTF16UChar32Iterator.h"
 #include <cairo-ft.h>
 #include <cairo.h>
@@ -58,7 +59,11 @@ bool GlyphPage::fill(UChar* buffer, unsigned bufferLength)
         if (character == iterator.end())
             break;
 
-        Glyph glyph = FcFreeTypeCharIndex(face, character);
+        Glyph glyph = FcFreeTypeCharIndex(face, FontCascade::treatAsSpace(character) ? space : character);
+        // If the font doesn't support a Default_Ignorable character, replace it with zero with space.
+        if (!glyph && u_hasBinaryProperty(character, UCHAR_DEFAULT_IGNORABLE_CODE_POINT))
+            glyph = FcFreeTypeCharIndex(face, zeroWidthSpace);
+
         if (!glyph)
             setGlyphForIndex(i, 0);
         else {