Changeset 195402 in webkit

Timestamp:

Jan 20, 2016, 11:57:07 PM (11 years ago)

Author:

bshafiei@apple.com

Message:

Merged r188263. rdar://problem/24208102

Location:

branches/safari-601-branch

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/arabic-glyph-cache-fill-combine-expected.html (copied) (copied from trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine-expected.html )
• LayoutTests/fast/text/arabic-glyph-cache-fill-combine.html (copied) (copied from trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine.html )
• LayoutTests/platform/efl/TestExpectations (modified) (1 diff)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp (modified) (2 diffs)

branches/safari-601-branch/LayoutTests/ChangeLog

@@ +1 @@
+2016-01-20  Babak Shafiei  <bshafiei@apple.com>
+
+        Merge r188263.
+
+    2015-08-11  Myles C. Maxfield  <mmaxfield@apple.com>
+
+            [iOS] Arabic letter Yeh is drawn in LastResort
+            https://bugs.webkit.org/show_bug.cgi?id=147862
+            <rdar://problem/22202935>
+
+            Reviewed by Darin Adler.
+
+            * 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
+
 2016-01-20  Matthew Hanson  <matthew_hanson@apple.com>
 

branches/safari-601-branch/LayoutTests/platform/efl/TestExpectations

@@ -2302 +2302 @@
 # This test uses an MPEG-4 video
 media/video-seek-to-current-time.html [ Skip ]
+
+# This test relies on iOS-specific font fallback.
+fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]

branches/safari-601-branch/LayoutTests/platform/gtk/TestExpectations

@@ -2404 +2404 @@
 # This test uses an MPEG-4 video
 media/video-seek-to-current-time.html [ Skip ]
+
+# This test relies on iOS-specific font fallback.
+fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]

branches/safari-601-branch/LayoutTests/platform/mac/TestExpectations

@@ -1377 +1377 @@
 [ Yosemite ElCapitan ] css3/font-variant-small-caps-synthesis-coverage.html [ ImageOnlyFailure ]
 [ Yosemite ElCapitan ] css3/font-variant-petite-caps-synthesis-coverage.html [ ImageOnlyFailure ]
+
+# This test relies on iOS-specific font fallback.
+[ Mavericks Yosemite ElCapitan ] fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]

branches/safari-601-branch/LayoutTests/platform/win/TestExpectations

@@ -3141 +3141 @@
 # This test uses an MPEG-4 video
 media/video-seek-to-current-time.html [ Skip ]
+
+# This test relies on iOS-specific font fallback.
+fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]

branches/safari-601-branch/Source/WebCore/ChangeLog

@@ +1 @@
+2016-01-20  Babak Shafiei  <bshafiei@apple.com>
+
+        Merge r188263.
+
+    2015-08-11  Myles C. Maxfield  <mmaxfield@apple.com>
+
+            [iOS] Arabic letter Yeh is drawn in LastResort
+            https://bugs.webkit.org/show_bug.cgi?id=147862
+            <rdar://problem/22202935>
+
+            Reviewed by Darin Adler.
+
+            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):
+
 2016-01-20  Timothy Hatcher  <timothy@apple.com>
 

branches/safari-601-branch/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp

@@ -88 +88 @@
             }
         }
-    } else if (!fontData->platformData().isCompositeFontReference() && ((fontData->platformData().widthVariant() == RegularWidth)
-        ? CTFontGetVerticalGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength)
-        : CTFontGetGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength))) {
+    } else if (!fontData->platformData().isCompositeFontReference()) {
+        if (fontData->platformData().widthVariant() == RegularWidth)
+            CTFontGetVerticalGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
+        else
+            CTFontGetGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
         // When buffer consists of surrogate pairs, CTFontGetVerticalGlyphsForCharacters and CTFontGetGlyphsForCharacters
         // place the glyphs at indices corresponding to the first character of each pair.
@@ -104 +106 @@
         }
     } else {
+        // 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
+        // combine adjacent codepoints into grapheme clusters. We should delete this entire else {} block.
+
         // We ask CoreText for possible vertical variant glyphs
         RetainPtr<CFStringRef> string = adoptCF(CFStringCreateWithCharactersNoCopy(kCFAllocatorDefault, buffer, bufferLength, kCFAllocatorNull));