REGRESSION(r239915): [FreeType] White space skipped when rendering plain text with...
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 May 2019 06:06:27 +0000 (06:06 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 May 2019 06:06:27 +0000 (06:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197658

Reviewed by Michael Catanzaro.

Since r239915 we no longer overwrite control characters with zero width space, they are handled later when
filling the glyph pages. In Font::platformGlyphInit() there's an optimization to get the glyph of zero with
space character that assumes that control characters are always overwritten. Since the glyph for character at 0
index is always overwritten with zero width space, we can avoid loading the page for the actual zero width space
character and use the first page instead. In the particular case of noto CJK font, character at 0 is mapped to
the same glyph as space character, so space and zero width space end up being the same glyph. That breaks the
space width calculation, that returns 0 when isZeroWidthSpaceGlyph() is true. That's why spaces are no
longer rendered, ComplexTextController::adjustGlyphsAndAdvances() is setting the x advance for the space glyphs
to 0.

* platform/graphics/Font.cpp:
(WebCore::Font::platformGlyphInit): Use the actual zero width space page to get the glyph instead of 0 when
using FreeType.

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Font.cpp

index d1ca81e..5bc103e 100644 (file)
@@ -1,3 +1,24 @@
+2019-05-08  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        REGRESSION(r239915): [FreeType] White space skipped when rendering plain text with noto CJK font
+        https://bugs.webkit.org/show_bug.cgi?id=197658
+
+        Reviewed by Michael Catanzaro.
+
+        Since r239915 we no longer overwrite control characters with zero width space, they are handled later when
+        filling the glyph pages. In Font::platformGlyphInit() there's an optimization to get the glyph of zero with
+        space character that assumes that control characters are always overwritten. Since the glyph for character at 0
+        index is always overwritten with zero width space, we can avoid loading the page for the actual zero width space
+        character and use the first page instead. In the particular case of noto CJK font, character at 0 is mapped to
+        the same glyph as space character, so space and zero width space end up being the same glyph. That breaks the
+        space width calculation, that returns 0 when isZeroWidthSpaceGlyph() is true. That's why spaces are no
+        longer rendered, ComplexTextController::adjustGlyphsAndAdvances() is setting the x advance for the space glyphs
+        to 0.
+
+        * platform/graphics/Font.cpp:
+        (WebCore::Font::platformGlyphInit): Use the actual zero width space page to get the glyph instead of 0 when
+        using FreeType.
+
 2019-05-08  Alex Christensen  <achristensen@webkit.org>
 
         Fix WPE build.
index eb5a17b..cc94d6c 100644 (file)
@@ -102,14 +102,20 @@ void Font::initCharWidths()
 
 void Font::platformGlyphInit()
 {
-    auto* glyphPageZero = glyphPage(0);
+#if USE(FREETYPE)
+    auto* glyphPageZeroWidthSpace = glyphPage(GlyphPage::pageNumberForCodePoint(zeroWidthSpace));
+    UChar32 zeroWidthSpaceCharacter = zeroWidthSpace;
+#else
+    // Ask for the glyph for 0 to avoid paging in ZERO WIDTH SPACE. Control characters, including 0,
+    // are mapped to the ZERO WIDTH SPACE glyph for non FreeType based ports.
+    auto* glyphPageZeroWidthSpace = glyphPage(0);
+    UChar32 zeroWidthSpaceCharacter = 0;
+#endif
     auto* glyphPageCharacterZero = glyphPage(GlyphPage::pageNumberForCodePoint('0'));
     auto* glyphPageSpace = glyphPage(GlyphPage::pageNumberForCodePoint(space));
 
-    // Ask for the glyph for 0 to avoid paging in ZERO WIDTH SPACE. Control characters, including 0,
-    // are mapped to the ZERO WIDTH SPACE glyph.
-    if (glyphPageZero)
-        m_zeroWidthSpaceGlyph = glyphPageZero->glyphDataForCharacter(0).glyph;
+    if (glyphPageZeroWidthSpace)
+        m_zeroWidthSpaceGlyph = glyphPageZeroWidthSpace->glyphDataForCharacter(zeroWidthSpaceCharacter).glyph;
 
     // Nasty hack to determine if we should round or ceil space widths.
     // If the font is monospace or fake monospace we ceil to ensure that