<!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>[188377] 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/188377">188377</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2015-08-12 22:36:21 -0700 (Wed, 12 Aug 2015)</dd>
</dl>
<h3>Log Message</h3>
<pre>[Cocoa] [CJK-configured device] System font has vertical punctuation
https://bugs.webkit.org/show_bug.cgi?id=147964
<rdar://problem/22256660>
Reviewed by Dean Jackson.
Source/WebCore:
GlyphPage::fill() has multiple code paths to accomplish its goal. It uses the shouldUseCoreText() helper
function to determine which one of the paths should be taken. However, not all of the code paths in
GlyphPage::fill() are able of handling all situations. Indeed, the CoreText code paths in GlyphPage::fill()
are only able to handle the situations which shouldUseCoreText() returns true for. This happens in the
following cases:
1. If the font is a composite font
2. If the font is used for text-combine
3. If the font has vertical glyphs
In <a href="http://trac.webkit.org/projects/webkit/changeset/187693">r187693</a>, I added one more case to this list: If the font is the system font. However, I failed to add
the necessary support to GlyphPage::fill() for this case. Becasue of this, we just happened to fall into
the case of vertical fonts (just by coincidence), which causes us to use
CTFontGetVerticalGlyphsForCharacters() instead of CTFontGetGlyphsForCharacters().
The solution is to adopt the same behavior we were using before <a href="http://trac.webkit.org/projects/webkit/changeset/187693">r187693</a>. Back then, we were using
CGFontGetGlyphsForUnichars(), which always returned horizontal glyphs. We should simply adopt this same
behavior, except in the Core Text case. Therefore, this patch is just a simple check to see if we are
using the system font when determining which Core Text function to use.
Test: fast/text/system-font-punctuation.html
* platform/graphics/FontDescription.h:
(WebCore::FontDescription::setWidthVariant):
* platform/graphics/FontPlatformData.h:
(WebCore::FontPlatformData::isForTextCombine):
* platform/graphics/mac/GlyphPageMac.cpp:
(WebCore::shouldUseCoreText):
(WebCore::GlyphPage::fill):
* rendering/RenderCombineText.cpp:
(WebCore::RenderCombineText::combineText):
LayoutTests:
Make sure punctuation isn't vertical.
* fast/text/system-font-punctuation.html: Added.
* platform/ios-simulator/fast/text/system-font-punctuation-expected.txt: Added
* platform/mac/fast/text/system-font-punctuation-expected.txt: Added</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsFontDescriptionh">trunk/Source/WebCore/platform/graphics/FontDescription.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsFontPlatformDatah">trunk/Source/WebCore/platform/graphics/FontPlatformData.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsmacGlyphPageMaccpp">trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderCombineTextcpp">trunk/Source/WebCore/rendering/RenderCombineText.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfasttextsystemfontpunctuationhtml">trunk/LayoutTests/fast/text/system-font-punctuation.html</a></li>
<li><a href="#trunkLayoutTestsplatformiossimulatorfasttextsystemfontpunctuationexpectedtxt">trunk/LayoutTests/platform/ios-simulator/fast/text/system-font-punctuation-expected.txt</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (188376 => 188377)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2015-08-13 05:30:21 UTC (rev 188376)
+++ trunk/LayoutTests/ChangeLog 2015-08-13 05:36:21 UTC (rev 188377)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2015-08-12 Myles C. Maxfield <mmaxfield@apple.com>
+
+ [Cocoa] [CJK-configured device] System font has vertical punctuation
+ https://bugs.webkit.org/show_bug.cgi?id=147964
+ <rdar://problem/22256660>
+
+ Reviewed by Dean Jackson.
+
+ Make sure punctuation isn't vertical.
+
+ * fast/text/system-font-punctuation.html: Added.
+ * platform/ios-simulator/fast/text/system-font-punctuation-expected.txt: Added
+ * platform/mac/fast/text/system-font-punctuation-expected.txt: Added
+
</ins><span class="cx"> 2015-08-12 Alexey Proskuryakov <ap@apple.com>
</span><span class="cx">
</span><span class="cx"> Removing an expectation for a long fixed bug.
</span></span></pre></div>
<a id="trunkLayoutTestsfasttextsystemfontpunctuationhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/text/system-font-punctuation.html (0 => 188377)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/text/system-font-punctuation.html (rev 0)
+++ trunk/LayoutTests/fast/text/system-font-punctuation.html 2015-08-13 05:36:21 UTC (rev 188377)
</span><span class="lines">@@ -0,0 +1,7 @@
</span><ins>+<!DOCTYPE>
+<html>
+<body>
+This test makes sure punctuation laid out with the system font does not use vertical glyphs. The test passes if the semicolon below looks like a regular horizontal semicolon (;) and is not sideways.
+<div style="font: 50px UICTFontTextStyleBody;">;</div>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsplatformiossimulatorfasttextsystemfontpunctuationexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/platform/ios-simulator/fast/text/system-font-punctuation-expected.txt (0 => 188377)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/ios-simulator/fast/text/system-font-punctuation-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/ios-simulator/fast/text/system-font-punctuation-expected.txt 2015-08-13 05:36:21 UTC (rev 188377)
</span><span class="lines">@@ -0,0 +1,12 @@
</span><ins>+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+ RenderBlock {HTML} at (0,0) size 800x600
+ RenderBody {BODY} at (8,8) size 784x584
+ RenderBlock (anonymous) at (0,0) size 784x40
+ RenderText {#text} at (0,0) size 720x39
+ text run at (0,0) width 720: "This test makes sure punctuation laid out with the system font does not use vertical glyphs. The test passes if the"
+ text run at (0,20) width 527: "semicolon below looks like a regular horizontal semicolon (;) and is not sideways."
+ RenderBlock {DIV} at (0,40) size 784x62
+ RenderText {#text} at (0,0) size 14x61
+ text run at (0,0) width 14: ";"
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (188376 => 188377)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2015-08-13 05:30:21 UTC (rev 188376)
+++ trunk/Source/WebCore/ChangeLog 2015-08-13 05:36:21 UTC (rev 188377)
</span><span class="lines">@@ -1,3 +1,43 @@
</span><ins>+2015-08-12 Myles C. Maxfield <mmaxfield@apple.com>
+
+ [Cocoa] [CJK-configured device] System font has vertical punctuation
+ https://bugs.webkit.org/show_bug.cgi?id=147964
+ <rdar://problem/22256660>
+
+ Reviewed by Dean Jackson.
+
+ GlyphPage::fill() has multiple code paths to accomplish its goal. It uses the shouldUseCoreText() helper
+ function to determine which one of the paths should be taken. However, not all of the code paths in
+ GlyphPage::fill() are able of handling all situations. Indeed, the CoreText code paths in GlyphPage::fill()
+ are only able to handle the situations which shouldUseCoreText() returns true for. This happens in the
+ following cases:
+
+ 1. If the font is a composite font
+ 2. If the font is used for text-combine
+ 3. If the font has vertical glyphs
+
+ In r187693, I added one more case to this list: If the font is the system font. However, I failed to add
+ the necessary support to GlyphPage::fill() for this case. Becasue of this, we just happened to fall into
+ the case of vertical fonts (just by coincidence), which causes us to use
+ CTFontGetVerticalGlyphsForCharacters() instead of CTFontGetGlyphsForCharacters().
+
+ The solution is to adopt the same behavior we were using before r187693. Back then, we were using
+ CGFontGetGlyphsForUnichars(), which always returned horizontal glyphs. We should simply adopt this same
+ behavior, except in the Core Text case. Therefore, this patch is just a simple check to see if we are
+ using the system font when determining which Core Text function to use.
+
+ Test: fast/text/system-font-punctuation.html
+
+ * platform/graphics/FontDescription.h:
+ (WebCore::FontDescription::setWidthVariant):
+ * platform/graphics/FontPlatformData.h:
+ (WebCore::FontPlatformData::isForTextCombine):
+ * platform/graphics/mac/GlyphPageMac.cpp:
+ (WebCore::shouldUseCoreText):
+ (WebCore::GlyphPage::fill):
+ * rendering/RenderCombineText.cpp:
+ (WebCore::RenderCombineText::combineText):
+
</ins><span class="cx"> 2015-08-12 Jinyoung Hur <hur.ims@navercorp.com>
</span><span class="cx">
</span><span class="cx"> [WinCairo] Turn on WOFF font
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsFontDescriptionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/FontDescription.h (188376 => 188377)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/FontDescription.h 2015-08-13 05:30:21 UTC (rev 188376)
+++ trunk/Source/WebCore/platform/graphics/FontDescription.h 2015-08-13 05:36:21 UTC (rev 188377)
</span><span class="lines">@@ -124,7 +124,7 @@
</span><span class="cx"> void setIsSpecifiedFont(bool isSpecifiedFont) { m_isSpecifiedFont = isSpecifiedFont; }
</span><span class="cx"> void setOrientation(FontOrientation orientation) { m_orientation = orientation; }
</span><span class="cx"> void setNonCJKGlyphOrientation(NonCJKGlyphOrientation orientation) { m_nonCJKGlyphOrientation = orientation; }
</span><del>- void setWidthVariant(FontWidthVariant widthVariant) { m_widthVariant = widthVariant; }
</del><ins>+ void setWidthVariant(FontWidthVariant widthVariant) { m_widthVariant = widthVariant; } // Make sure new callers of this sync with FontPlatformData::isForTextCombine()!
</ins><span class="cx"> void setLocale(const AtomicString&);
</span><span class="cx"> void setFeatureSettings(PassRefPtr<FontFeatureSettings> settings) { m_featureSettings = settings; }
</span><span class="cx"> void setFontSynthesis(FontSynthesis fontSynthesis) { m_fontSynthesis = fontSynthesis; }
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsFontPlatformDatah"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/FontPlatformData.h (188376 => 188377)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/FontPlatformData.h 2015-08-13 05:30:21 UTC (rev 188376)
+++ trunk/Source/WebCore/platform/graphics/FontPlatformData.h 2015-08-13 05:36:21 UTC (rev 188377)
</span><span class="lines">@@ -137,6 +137,7 @@
</span><span class="cx"> bool isCompositeFontReference() const { return m_isCompositeFontReference; }
</span><span class="cx"> FontOrientation orientation() const { return m_orientation; }
</span><span class="cx"> FontWidthVariant widthVariant() const { return m_widthVariant; }
</span><ins>+ bool isForTextCombine() const { return widthVariant() != RegularWidth; } // Keep in sync with callers of FontDescription::setWidthVariant().
</ins><span class="cx">
</span><span class="cx"> void setOrientation(FontOrientation orientation) { m_orientation = orientation; }
</span><span class="cx"> void setSyntheticOblique(bool syntheticOblique) { m_syntheticOblique = syntheticOblique; }
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsmacGlyphPageMaccpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp (188376 => 188377)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp 2015-08-13 05:30:21 UTC (rev 188376)
+++ trunk/Source/WebCore/platform/graphics/mac/GlyphPageMac.cpp 2015-08-13 05:36:21 UTC (rev 188377)
</span><span class="lines">@@ -42,9 +42,12 @@
</span><span class="cx">
</span><span class="cx"> static bool shouldUseCoreText(const UChar* buffer, unsigned bufferLength, const Font* fontData)
</span><span class="cx"> {
</span><ins>+ // This needs to be kept in sync with GlyphPage::fill(). Currently, the CoreText paths are not able to handle
+ // every situtation. Returning true from this function in a new situation will require you to explicitly add
+ // handling for that situation in the CoreText paths of GlyphPage::fill().
</ins><span class="cx"> if (fontData->platformData().isCompositeFontReference() || fontData->isSystemFont())
</span><span class="cx"> return true;
</span><del>- if (fontData->platformData().widthVariant() != RegularWidth || fontData->hasVerticalGlyphs()) {
</del><ins>+ if (fontData->platformData().isForTextCombine() || fontData->hasVerticalGlyphs()) {
</ins><span class="cx"> // Ideographs don't have a vertical variant or width variants.
</span><span class="cx"> for (unsigned i = 0; i < bufferLength; ++i) {
</span><span class="cx"> if (!FontCascade::isCJKIdeograph(buffer[i]))
</span><span class="lines">@@ -88,10 +91,12 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx"> } else if (!fontData->platformData().isCompositeFontReference()) {
</span><del>- if (fontData->platformData().widthVariant() == RegularWidth)
</del><ins>+ // Because we know the implementation of shouldUseCoreText(), if the font isn't for text combine and it isn't a system font,
+ // we know it must have vertical glyphs.
+ if (fontData->platformData().isForTextCombine() || fontData->isSystemFont())
+ CTFontGetGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
+ else
</ins><span class="cx"> CTFontGetVerticalGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
</span><del>- else
- CTFontGetGlyphsForCharacters(fontData->platformData().ctFont(), buffer, glyphs.data(), bufferLength);
</del><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></pre></div>
<a id="trunkSourceWebCorerenderingRenderCombineTextcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderCombineText.cpp (188376 => 188377)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderCombineText.cpp 2015-08-13 05:30:21 UTC (rev 188376)
+++ trunk/Source/WebCore/rendering/RenderCombineText.cpp 2015-08-13 05:36:21 UTC (rev 188377)
</span><span class="lines">@@ -117,7 +117,7 @@
</span><span class="cx"> // Need to try compressed glyphs.
</span><span class="cx"> static const FontWidthVariant widthVariants[] = { HalfWidth, ThirdWidth, QuarterWidth };
</span><span class="cx"> for (size_t i = 0 ; i < WTF_ARRAY_LENGTH(widthVariants) ; ++i) {
</span><del>- description.setWidthVariant(widthVariants[i]);
</del><ins>+ description.setWidthVariant(widthVariants[i]); // When modifying this, make sure to keep it in sync with FontPlatformData::isForTextCombine()!
</ins><span class="cx">
</span><span class="cx"> FontCascade compressedFont(description, style().fontCascade().letterSpacing(), style().fontCascade().wordSpacing());
</span><span class="cx"> compressedFont.update(fontSelector);
</span></span></pre>
</div>
</div>
</body>
</html>