<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[188263] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/188263">188263</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2015-08-11 11:05:40 -0700 (Tue, 11 Aug 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>[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</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsplatformeflTestExpectations">trunk/LayoutTests/platform/efl/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsplatformgtkTestExpectations">trunk/LayoutTests/platform/gtk/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsplatformmacTestExpectations">trunk/LayoutTests/platform/mac/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsplatformwinTestExpectations">trunk/LayoutTests/platform/win/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsmacGlyphPageMaccpp">trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasttextarabicglyphcachefillcombineexpectedhtml">trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine-expected.html</a></li>
<li><a href="#trunkLayoutTestsfasttextarabicglyphcachefillcombinehtml">trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (188262 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-08-11 17:57:08 UTC (rev 188262)
+++ trunk/LayoutTests/ChangeLog 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -1,3 +1,18 @@
</span><ins>+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
+
</ins><span class="cx"> 2015-08-11 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> The 'length' property on interface objects should be configurable
</span></span></pre></div>
<a id="trunkLayoutTestsfasttextarabicglyphcachefillcombineexpectedhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine-expected.html (0 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine-expected.html 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+<!DOCTYPE>
+<html>
+<body>
+This test hits a codepath where codepoints were being combined before they were being input into our glyph cache.
+This character occurs just before combining marks in Unicode, and therefore was being erroneously combined. The test
+passes if the glyph is drawn in a font other than LastResort (if the test fails, the character looks like a box or like
+three horizontal lines).
+<div style="font: 50px 'Geeza Pro';">&#x64a;</div>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsfasttextarabicglyphcachefillcombinehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine.html (0 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine.html (rev 0)
+++ trunk/LayoutTests/fast/text/arabic-glyph-cache-fill-combine.html 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+<!DOCTYPE>
+<html>
+<body>
+This test hits a codepath where codepoints were being combined before they were being input into our glyph cache.
+This character occurs just before combining marks in Unicode, and therefore was being erroneously combined. The test
+passes if the glyph is drawn in a font other than LastResort (if the test fails, the character looks like a box or like
+three horizontal lines).
+<div style="font: 50px UICTFontTextStyleBody;">&#x64a;</div>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsplatformeflTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/efl/TestExpectations (188262 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/efl/TestExpectations 2015-08-11 17:57:08 UTC (rev 188262)
+++ trunk/LayoutTests/platform/efl/TestExpectations 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -2253,3 +2253,6 @@
</span><span class="cx">
</span><span class="cx"> # This test hardcodes the result of a platform-dependent font lookup algorithm.
</span><span class="cx"> fast/text/fallback-language-han.html [ Skip ]
</span><ins>+
+# This test relies on iOS-specific font fallback.
+fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]
</ins></span></pre></div>
<a id="trunkLayoutTestsplatformgtkTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/gtk/TestExpectations (188262 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/gtk/TestExpectations 2015-08-11 17:57:08 UTC (rev 188262)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -2420,3 +2420,6 @@
</span><span class="cx">
</span><span class="cx"> # This test hardcodes the result of a platform-dependent font lookup algorithm.
</span><span class="cx"> fast/text/fallback-language-han.html [ Skip ]
</span><ins>+
+# This test relies on iOS-specific font fallback.
+fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]
</ins></span></pre></div>
<a id="trunkLayoutTestsplatformmacTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/mac/TestExpectations (188262 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/mac/TestExpectations 2015-08-11 17:57:08 UTC (rev 188262)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -1313,3 +1313,6 @@
</span><span class="cx">
</span><span class="cx"> # Language-specific font fallback is disabled on certain versions of OS X
</span><span class="cx"> webkit.org/b/147390 [ Mavericks Yosemite ElCapitan ] fast/text/fallback-language-han.html [ ImageOnlyFailure ]
</span><ins>+
+# This test relies on iOS-specific font fallback.
+[ Mavericks Yosemite ElCapitan ] fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]
</ins></span></pre></div>
<a id="trunkLayoutTestsplatformwinTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/win/TestExpectations (188262 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/win/TestExpectations 2015-08-11 17:57:08 UTC (rev 188262)
+++ trunk/LayoutTests/platform/win/TestExpectations 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -3149,3 +3149,6 @@
</span><span class="cx">
</span><span class="cx"> # This test hardcodes the result of a platform-dependent font lookup algorithm.
</span><span class="cx"> fast/text/fallback-language-han.html [ Skip ]
</span><ins>+
+# This test relies on iOS-specific font fallback.
+fast/text/arabic-glyph-cache-fill-combine.html [ ImageOnlyFailure ]
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (188262 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-08-11 17:57:08 UTC (rev 188262)
+++ trunk/Source/WebCore/ChangeLog 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -1,3 +1,40 @@
</span><ins>+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):
+
</ins><span class="cx"> 2015-08-11 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> The 'length' property on interface objects should be configurable
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsmacGlyphPageMaccpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp (188262 => 188263)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp 2015-08-11 17:57:08 UTC (rev 188262)
+++ trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp 2015-08-11 18:05:40 UTC (rev 188263)
</span><span class="lines">@@ -87,9 +87,11 @@
</span><span class="cx"> haveGlyphs = true;
</span><span class="cx"> }
</span><span class="cx"> }
</span><del>- } 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))) {
</del><ins>+ } 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);
</ins><span class="cx"> // When buffer consists of surrogate pairs, CTFontGetVerticalGlyphsForCharacters and CTFontGetGlyphsForCharacters
</span><span class="cx"> // place the glyphs at indices corresponding to the first character of each pair.
</span><span class="cx"> ASSERT(!(bufferLength % length) && (bufferLength / length == 1 || bufferLength / length == 2));
</span><span class="lines">@@ -103,6 +105,9 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> } else {
</span><ins>+ // 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.
+
</ins><span class="cx"> // We ask CoreText for possible vertical variant glyphs
</span><span class="cx"> RetainPtr<CFStringRef> string = adoptCF(CFStringCreateWithCharactersNoCopy(kCFAllocatorDefault, buffer, bufferLength, kCFAllocatorNull));
</span><span class="cx"> RetainPtr<CFAttributedStringRef> attributedString = adoptCF(CFAttributedStringCreate(kCFAllocatorDefault, string.get(), fontData->getCFStringAttributes(0, fontData->hasVerticalGlyphs() ? Vertical : Horizontal)));
</span></span></pre>
</div>
</div>
</body>
</html>