Changeset 245094 in webkit

Timestamp:

May 8, 2019 11:06:27 PM (5 years ago)

Author:

Carlos Garcia Campos

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/Font.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

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

@@ -103 +103 @@
 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.