Attachment 359029 Details for Bug 193395 – Patch

[patch] Patch

wc-simple-text-default-ignorable.diff (text/plain), 4.86 KB, created by Carlos Garcia Campos on 2019-01-14 04:21:10 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Carlos Garcia Campos

Created: 2019-01-14 04:21:10 PST

Size: 4.86 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 80277ebb9ea..1291214399b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2019-01-14  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 NOBODY (OOPS!).
>+
>+        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-14  Philippe Normand  <pnormand@igalia.com>
> 
>         [WPE] Add API to allow application to disable background rendering
>diff --git a/Source/WebCore/platform/graphics/WidthIterator.cpp b/Source/WebCore/platform/graphics/WidthIterator.cpp
>index 9bde879703f..108130222c4 100644
>--- a/Source/WebCore/platform/graphics/WidthIterator.cpp
>+++ b/Source/WebCore/platform/graphics/WidthIterator.cpp
>@@ -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,13 +197,13 @@ 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;
>-        int currentCharacter = textIterator.currentIndex();
>-        const GlyphData& glyphData = m_font->glyphDataForCharacter(character, rtl);
>-        Glyph glyph = glyphData.glyph;
>-        if (!glyph && !characterMustDrawSomething(character)) {
>+        if (isDefaultIgnorableCodePoint(character)) {
>             textIterator.advance(advanceLength);
>             continue;
>         }
>+        int currentCharacter = textIterator.currentIndex();
>+        const GlyphData& glyphData = m_font->glyphDataForCharacter(character, rtl);
>+        Glyph glyph = glyphData.glyph;
>         const Font* font = glyphData.font ? glyphData.font : &m_font->primaryFont();
>         ASSERT(font);
> 
>diff --git a/Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp b/Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp
>index dcdd26ded50..6fb4ea58663 100644
>--- a/Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp
>+++ b/Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp
>@@ -32,6 +32,7 @@
> #include "GlyphPage.h"
> 
> #include "CairoUtilities.h"
>+#include "CharacterProperties.h"
> #include "Font.h"
> #include "FontCascade.h"
> #include "UTF16UChar32Iterator.h"
>@@ -61,7 +62,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 = FcFreeTypeCharIndex(face, zeroWidthSpace);
> 
>         if (!glyph)
>diff --git a/Source/WebCore/platform/text/CharacterProperties.h b/Source/WebCore/platform/text/CharacterProperties.h
>index e20d680137c..c3a10d84179 100644
>--- a/Source/WebCore/platform/text/CharacterProperties.h
>+++ b/Source/WebCore/platform/text/CharacterProperties.h
>@@ -106,4 +106,9 @@ inline bool isEmojiModifierBase(UChar32 character)
> #endif
> }
> 
>+inline bool isDefaultIgnorableCodePoint(UChar32 character)
>+{
>+    return u_hasBinaryProperty(character, UCHAR_DEFAULT_IGNORABLE_CODE_POINT);
>+}
>+
> }

Flags:

mcatanzaro: review-
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 193395: 359029 | 359031 | 359032 | 359033 | 359677