REGRESSION(r239915): about 130 test failures on WPE
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 21 Jan 2019 10:14:00 +0000 (10:14 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 21 Jan 2019 10:14:00 +0000 (10:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193395

Reviewed by Žan Doberšek.

Since r239915 we are only overriding the characters with Default_Ignorable unicode property when the font
doesn't support the code point. If the font happens to provide a glyph for the character, it's later ignored by
harfbuzz when shaping, but the simple text code path doesn't ignore them unless there isn't a glyph.

* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::advanceInternal): Always ignore characters with Default_Ignorable unicode property.
(WebCore::characterMustDrawSomething): Moved to CharacterProperties.h and renamed as isDefaultIgnorableCodePoint().
* platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:
(WebCore::GlyphPage::fill): Use isDefaultIgnorableCodePoint().
* platform/text/CharacterProperties.h:
(WebCore::isDefaultIgnorableCodePoint): Return whether the character has Default_Ignorable unicode property.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/WidthIterator.cpp
Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp
Source/WebCore/platform/text/CharacterProperties.h

index 9cf7f91..e9f0c95 100644 (file)
@@ -1,3 +1,22 @@
+2019-01-21  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        REGRESSION(r239915): about 130 test failures on WPE
+        https://bugs.webkit.org/show_bug.cgi?id=193395
+
+        Reviewed by Žan Doberšek.
+
+        Since r239915 we are only overriding the characters with Default_Ignorable unicode property when the font
+        doesn't support the code point. If the font happens to provide a glyph for the character, it's later ignored by
+        harfbuzz when shaping, but the simple text code path doesn't ignore them unless there isn't a glyph.
+
+        * platform/graphics/WidthIterator.cpp:
+        (WebCore::WidthIterator::advanceInternal): Always ignore characters with Default_Ignorable unicode property.
+        (WebCore::characterMustDrawSomething): Moved to CharacterProperties.h and renamed as isDefaultIgnorableCodePoint().
+        * platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:
+        (WebCore::GlyphPage::fill): Use isDefaultIgnorableCodePoint().
+        * platform/text/CharacterProperties.h:
+        (WebCore::isDefaultIgnorableCodePoint): Return whether the character has Default_Ignorable unicode property.
+
 2019-01-20  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [WHLSL] Implement Metal code generation
index 9bde879..2c6d951 100644 (file)
@@ -22,6 +22,7 @@
 #include "config.h"
 #include "WidthIterator.h"
 
+#include "CharacterProperties.h"
 #include "Font.h"
 #include "FontCascade.h"
 #include "GlyphBuffer.h"
@@ -165,11 +166,6 @@ static inline std::pair<bool, bool> expansionLocation(bool ideograph, bool treat
     return std::make_pair(expandLeft, expandRight);
 }
 
-static inline bool characterMustDrawSomething(UChar32 character)
-{
-    return !u_hasBinaryProperty(character, UCHAR_DEFAULT_IGNORABLE_CODE_POINT);
-}
-
 template <typename TextIterator>
 inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, GlyphBuffer* glyphBuffer)
 {
@@ -201,10 +197,19 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
     // We are iterating in string order, not glyph order. Compare this to ComplexTextController::adjustGlyphsAndAdvances()
     while (textIterator.consume(character, clusterLength)) {
         unsigned advanceLength = clusterLength;
+        bool characterMustDrawSomething = !isDefaultIgnorableCodePoint(character);
+#if USE(FREETYPE)
+        // Freetype based ports only override the characters with Default_Ignorable unicode property when the font
+        // doesn't support the code point. We should ignore them at this point to ensure they are not displayed.
+        if (!characterMustDrawSomething) {
+            textIterator.advance(advanceLength);
+            continue;
+        }
+#endif
         int currentCharacter = textIterator.currentIndex();
         const GlyphData& glyphData = m_font->glyphDataForCharacter(character, rtl);
         Glyph glyph = glyphData.glyph;
-        if (!glyph && !characterMustDrawSomething(character)) {
+        if (!glyph && !characterMustDrawSomething) {
             textIterator.advance(advanceLength);
             continue;
         }
index 7c01975..1e85866 100644 (file)
@@ -32,6 +32,7 @@
 #include "GlyphPage.h"
 
 #include "CairoUtilities.h"
+#include "CharacterProperties.h"
 #include "Font.h"
 #include "FontCascade.h"
 #include "UTF16UChar32Iterator.h"
@@ -70,7 +71,7 @@ bool GlyphPage::fill(UChar* buffer, unsigned bufferLength)
 
         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))
+        if (!glyph && isDefaultIgnorableCodePoint(character))
             glyph = zeroWidthSpaceGlyph();
 
         if (!glyph)
index e20d680..c3a10d8 100644 (file)
@@ -106,4 +106,9 @@ inline bool isEmojiModifierBase(UChar32 character)
 #endif
 }
 
+inline bool isDefaultIgnorableCodePoint(UChar32 character)
+{
+    return u_hasBinaryProperty(character, UCHAR_DEFAULT_IGNORABLE_CODE_POINT);
+}
+
 }