⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 188263 in webkit


Ignore:
Timestamp:
Aug 11, 2015, 11:05:40 AM (11 years ago)
Author:
mmaxfield@apple.com
Message:

[iOS] Arabic letter Yeh is drawn in LastResort
​https://bugs.webkit.org/show_bug.cgi?id=147862
<rdar://problem/22202935>

Reviewed by Darin Adler.

Source/WebCore:

In order to perform font fallback, we must know which fonts support which characters. We
perform this check by asking each font to map a sequence of codepoints to glyphs, and
any glyphs which end up with a 0 value are unsupported by the font.

One of the mechanisms that we use to do this is to combine the code points into a string,
and tell Core Text to lay out the string. However, this is fundamentally a different
operation than the one we are trying to perform. Strings combine adjacent codepoints into
grapheme clusters, and CoreText operates on these. However, we are trying to gain
information regarding codepoints, not grapheme clusters.

Instead of taking this string-based approach, we should try harder to use Core Text
functions which operate on ordered collections of characters, rather than strings. In
particular, CTFontGetGlyphsForCharacters() and CTFontGetVerticalGlyphsForCharacters()
have the behavior we want where any unmapped characters end up with a 0 value glyph.

Previously, we were only using the result of those functions if they were successfully
able to map their entire input. However, given the fact that we can degrade gracefully
in the case of a partial mapping, we shouldn't need to bail completely to the
string-based approach should a partial mapping occur.

At some point we should delete the string-based approach entirely. However, this path
is still explicitly used for composite fonts. Fixing that use case is out of scope
for this patch.

Test: fast/text/arabic-glyph-cache-fill-combine.html

  • platform/graphics/mac/GlyphPageMac.cpp:

(WebCore::GlyphPage::fill):

LayoutTests:

  • fast/text/arabic-glyph-cache-fill-combine-expected.html: Added.
  • fast/text/arabic-glyph-cache-fill-combine.html: Added.
  • platform/mac/TestExpectations: Mark test as iOS-specific
  • platform/gtk/TestExpectations: Mark test as iOS-specific
  • platform/efl/TestExpectations: Mark test as iOS-specific
  • platform/efl/TestExpectations: Mark test as iOS-specific
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r188261 r188263  
     12015-08-11  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        [iOS] Arabic letter Yeh is drawn in LastResort
     4        https://bugs.webkit.org/show_bug.cgi?id=147862
     5        <rdar://problem/22202935>
     6
     7        Reviewed by Darin Adler.
     8
     9        * fast/text/arabic-glyph-cache-fill-combine-expected.html: Added.
     10        * fast/text/arabic-glyph-cache-fill-combine.html: Added.
     11        * platform/mac/TestExpectations: Mark test as iOS-specific
     12        * platform/gtk/TestExpectations: Mark test as iOS-specific
     13        * platform/efl/TestExpectations: Mark test as iOS-specific
     14        * platform/efl/TestExpectations: Mark test as iOS-specific
     15
    1162015-08-11  Chris Dumez  <cdumez@apple.com>
    217

  • trunk/LayoutTests/platform/efl/TestExpectations

    r188167 r188263  
    22542254# This test hardcodes the result of a platform-dependent font lookup algorithm.
    22552255fast/text/fallback-language-han.html [ Skip ]
     2256
     2257# This test relies on iOS-specific font fallback.
     2258fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r188167 r188263  
    24212421# This test hardcodes the result of a platform-dependent font lookup algorithm.
    24222422fast/text/fallback-language-han.html [ Skip ]
     2423
     2424# This test relies on iOS-specific font fallback.
     2425fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/platform/mac/TestExpectations

    r188174 r188263  
    13141314# Language-specific font fallback is disabled on certain versions of OS X
    13151315webkit.org/b/147390 [ Mavericks Yosemite ElCapitan ] fast/text/fallback-language-han.html [ ImageOnlyFailure ]
     1316
     1317# This test relies on iOS-specific font fallback.
     1318[ Mavericks Yosemite ElCapitan ] fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/platform/win/TestExpectations

    r188167 r188263  
    31503150# This test hardcodes the result of a platform-dependent font lookup algorithm.
    31513151fast/text/fallback-language-han.html [ Skip ]
     3152
     3153# This test relies on iOS-specific font fallback.
     3154fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]

  • trunk/Source/WebCore/ChangeLog

    r188261 r188263  
     12015-08-11  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        [iOS] Arabic letter Yeh is drawn in LastResort
     4        https://bugs.webkit.org/show_bug.cgi?id=147862
     5        <rdar://problem/22202935>
     6
     7        Reviewed by Darin Adler.
     8
     9        In order to perform font fallback, we must know which fonts support which characters. We
     10        perform this check by asking each font to map a sequence of codepoints to glyphs, and
     11        any glyphs which end up with a 0 value are unsupported by the font.
     12
     13        One of the mechanisms that we use to do this is to combine the code points into a string,
     14        and tell Core Text to lay out the string. However, this is fundamentally a different
     15        operation than the one we are trying to perform. Strings combine adjacent codepoints into
     16        grapheme clusters, and CoreText operates on these. However, we are trying to gain
     17        information regarding codepoints, not grapheme clusters.
     18
     19        Instead of taking this string-based approach, we should try harder to use Core Text
     20        functions which operate on ordered collections of characters, rather than strings. In
     21        particular, CTFontGetGlyphsForCharacters() and CTFontGetVerticalGlyphsForCharacters()
     22        have the behavior we want where any unmapped characters end up with a 0 value glyph.
     23
     24        Previously, we were only using the result of those functions if they were successfully
     25        able to map their entire input. However, given the fact that we can degrade gracefully
     26        in the case of a partial mapping, we shouldn't need to bail completely to the
     27        string-based approach should a partial mapping occur.
     28
     29        At some point we should delete the string-based approach entirely. However, this path
     30        is still explicitly used for composite fonts. Fixing that use case is out of scope
     31        for this patch.
     32
     33        Test: fast/text/arabic-glyph-cache-fill-combine.html
     34
     35        * platform/graphics/mac/GlyphPageMac.cpp:
     36        (WebCore::GlyphPage::fill):
     37
    1382015-08-11  Chris Dumez  <cdumez@apple.com>
    239

  • trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp

    r187693 r188263  
    8888            }
    8989        }
    90     } else if (!fontData->platformData().isCompositeFontReference() && ((fontData->platformData().widthVariant() == RegularWidth)
    91         ? CTFontGetVerticalGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength)
    92         : CTFontGetGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength))) {
     90    } else if (!fontData->platformData().isCompositeFontReference()) {
     91        if (fontData->platformData().widthVariant() == RegularWidth)
     92            CTFontGetVerticalGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
     93        else
     94            CTFontGetGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
    9395        // When buffer consists of surrogate pairs, CTFontGetVerticalGlyphsForCharacters and CTFontGetGlyphsForCharacters
    9496        // place the glyphs at indices corresponding to the first character of each pair.
    … …  
    104106        }
    105107    } else {
     108        // FIXME: webkit.org/b/147859 This code is fundamentally broken. A string is not the same as an ordered sequence of codepoints. In particular, strings
     109        // combine adjacent codepoints into grapheme clusters. We should delete this entire else {} block.
     110
    106111        // We ask CoreText for possible vertical variant glyphs
    107112        RetainPtr<CFStringRef> string = adoptCF(CFStringCreateWithCharactersNoCopy(kCFAllocatorDefault, buffer, bufferLength, kCFAllocatorNull));
Note: See TracChangeset for help on using the changeset viewer.