<!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>[213204] trunk/Source/WebCore</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/213204">213204</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2017-02-28 19:55:41 -0800 (Tue, 28 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Addressing post-review comments on <a href="http://trac.webkit.org/projects/webkit/changeset/213163">r213163</a>
https://bugs.webkit.org/show_bug.cgi?id=168678
Unreviewed.
No new tests because there is no behavior change.
* platform/graphics/cocoa/FontCacheCoreText.cpp:
(WebCore::platformFontLookupWithFamily):</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicscocoaFontCacheCoreTextcpp">trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (213203 => 213204)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-03-01 03:04:24 UTC (rev 213203)
+++ trunk/Source/WebCore/ChangeLog 2017-03-01 03:55:41 UTC (rev 213204)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2017-02-28 Myles C. Maxfield <mmaxfield@apple.com>
+
+ Addressing post-review comments on r213163
+ https://bugs.webkit.org/show_bug.cgi?id=168678
+
+ Unreviewed.
+
+ No new tests because there is no behavior change.
+
+ * platform/graphics/cocoa/FontCacheCoreText.cpp:
+ (WebCore::platformFontLookupWithFamily):
+
</ins><span class="cx"> 2017-02-28 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Don't use the LinearGlyphMask format if accelerated drawing is disabled
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicscocoaFontCacheCoreTextcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp (213203 => 213204)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp 2017-03-01 03:04:24 UTC (rev 213203)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp 2017-03-01 03:55:41 UTC (rev 213204)
</span><span class="lines">@@ -923,7 +923,7 @@
</span><span class="cx"> Range weightBounds;
</span><span class="cx"> };
</span><span class="cx">
</span><del>- const InstalledFontFamily& lookUpFamilyName(const String& familyName)
</del><ins>+ const InstalledFontFamily& collectionForFamily(const String& familyName)
</ins><span class="cx"> {
</span><span class="cx"> auto folded = familyName.foldCase();
</span><span class="cx"> return m_familyNameToFontDescriptors.ensure(folded, [&] {
</span><span class="lines">@@ -946,7 +946,7 @@
</span><span class="cx"> }).iterator->value;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- const InstalledFont& lookupPostScriptName(const AtomicString& postScriptName)
</del><ins>+ const InstalledFont& fontForPostScriptName(const AtomicString& postScriptName)
</ins><span class="cx"> {
</span><span class="cx"> auto folded = postScriptName.string().foldCase();
</span><span class="cx"> return m_postScriptNameToFontDescriptors.ensure(folded, [&] {
</span><span class="lines">@@ -979,9 +979,8 @@
</span><span class="cx"> using IterateActiveFontsWithReturnCallback = std::function<std::optional<T>(const FontDatabase::InstalledFont&, size_t)>;
</span><span class="cx">
</span><span class="cx"> template <typename T>
</span><del>-inline std::optional<T> iterateActiveFontsWithReturn(const FontDatabase::InstalledFontFamily& installedFonts, const Vector<bool>& filter, IterateActiveFontsWithReturnCallback<T> callback)
</del><ins>+inline std::optional<T> iterateActiveFontsWithReturn(const FontDatabase::InstalledFontFamily& installedFonts, const std::unique_ptr<bool[]>& filter, IterateActiveFontsWithReturnCallback<T> callback)
</ins><span class="cx"> {
</span><del>- ASSERT(installedFonts.size() == filter.size());
</del><span class="cx"> for (size_t i = 0; i < installedFonts.size(); ++i) {
</span><span class="cx"> if (!filter[i])
</span><span class="cx"> continue;
</span><span class="lines">@@ -992,7 +991,7 @@
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> template <typename T>
</span><del>-inline void iterateActiveFonts(const FontDatabase::InstalledFontFamily& installedFonts, const Vector<bool>& filter, T callback)
</del><ins>+inline void iterateActiveFonts(const FontDatabase::InstalledFontFamily& installedFonts, const std::unique_ptr<bool[]>& filter, T callback)
</ins><span class="cx"> {
</span><span class="cx"> iterateActiveFontsWithReturn<int>(installedFonts, filter, [&](const FontDatabase::InstalledFont& font, size_t i) -> std::optional<int> {
</span><span class="cx"> callback(font, i);
</span><span class="lines">@@ -1000,18 +999,18 @@
</span><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline std::optional<float> findClosestStretch(float, const FontDatabase::InstalledFontFamily&, const Vector<bool>&)
</del><ins>+static inline std::optional<float> findClosestStretch(float, const FontDatabase::InstalledFontFamily&, const std::unique_ptr<bool[]>&)
</ins><span class="cx"> {
</span><span class="cx"> // FIXME: Implement this.
</span><span class="cx"> return 0;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline void filterStretch(float, const FontDatabase::InstalledFontFamily&, Vector<bool>&)
</del><ins>+static inline void filterStretch(float, const FontDatabase::InstalledFontFamily&, std::unique_ptr<bool[]>&)
</ins><span class="cx"> {
</span><span class="cx"> // FIXME: Implement this.
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline std::optional<float> findClosestStyle(float targetStyle, const FontDatabase::InstalledFontFamily& installedFonts, const Vector<bool>& filter)
</del><ins>+static inline std::optional<float> findClosestStyle(float targetStyle, const FontDatabase::InstalledFontFamily& installedFonts, const std::unique_ptr<bool[]>& filter)
</ins><span class="cx"> {
</span><span class="cx"> std::function<float(const FontDatabase::InstalledFont&)> computeScore;
</span><span class="cx">
</span><span class="lines">@@ -1081,7 +1080,7 @@
</span><span class="cx"> });
</span><span class="cx">
</span><span class="cx"> if (!minimumScore)
</span><del>- return { };
</del><ins>+ return std::nullopt;
</ins><span class="cx"> auto& winner = installedFonts.installedFonts[closestIndex];
</span><span class="cx"> if (winner.style.includes(targetStyle))
</span><span class="cx"> return targetStyle;
</span><span class="lines">@@ -1091,7 +1090,7 @@
</span><span class="cx"> return winner.style.maximum;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline void filterStyle(float target, const FontDatabase::InstalledFontFamily& installedFonts, Vector<bool>& filter)
</del><ins>+static inline void filterStyle(float target, const FontDatabase::InstalledFontFamily& installedFonts, std::unique_ptr<bool[]>& filter)
</ins><span class="cx"> {
</span><span class="cx"> iterateActiveFonts(installedFonts, filter, [&](auto& installedFont, size_t i) {
</span><span class="cx"> if (!installedFont.style.includes(target))
</span><span class="lines">@@ -1099,29 +1098,30 @@
</span><span class="cx"> });
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline std::optional<float> findClosestWeight(float targetWeight, const FontDatabase::InstalledFontFamily& installedFonts, const Vector<bool>& filter)
</del><ins>+static inline std::optional<float> findClosestWeight(float targetWeight, const FontDatabase::InstalledFontFamily& installedFonts, const std::unique_ptr<bool[]>& filter)
</ins><span class="cx"> {
</span><span class="cx"> {
</span><ins>+ // The spec states: "If the desired weight is 400, 500 is checked first ... If the desired weight is 500, 400 is checked first"
</ins><span class="cx"> IterateActiveFontsWithReturnCallback<float> searchFor400 = [&](const FontDatabase::InstalledFont& font, size_t) -> std::optional<float> {
</span><span class="cx"> if (font.weight.includes(400))
</span><span class="cx"> return 400;
</span><del>- return { };
</del><ins>+ return std::nullopt;
</ins><span class="cx"> };
</span><span class="cx"> IterateActiveFontsWithReturnCallback<float> searchFor500 = [&](const FontDatabase::InstalledFont& font, size_t) -> std::optional<float> {
</span><span class="cx"> if (font.weight.includes(500))
</span><span class="cx"> return 500;
</span><del>- return { };
</del><ins>+ return std::nullopt;
</ins><span class="cx"> };
</span><span class="cx"> if (targetWeight == 400) {
</span><span class="cx"> if (auto result = iterateActiveFontsWithReturn(installedFonts, filter, searchFor400))
</span><del>- return result.value();
</del><ins>+ return result;
</ins><span class="cx"> if (auto result = iterateActiveFontsWithReturn(installedFonts, filter, searchFor500))
</span><del>- return result.value();
</del><ins>+ return result;
</ins><span class="cx"> } else if (targetWeight == 500) {
</span><span class="cx"> if (auto result = iterateActiveFontsWithReturn(installedFonts, filter, searchFor500))
</span><del>- return result.value();
</del><ins>+ return result;
</ins><span class="cx"> if (auto result = iterateActiveFontsWithReturn(installedFonts, filter, searchFor400))
</span><del>- return result.value();
</del><ins>+ return result;
</ins><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1162,7 +1162,7 @@
</span><span class="cx"> });
</span><span class="cx">
</span><span class="cx"> if (!minimumScore)
</span><del>- return { };
</del><ins>+ return std::nullopt;
</ins><span class="cx"> auto& winner = installedFonts.installedFonts[closestIndex];
</span><span class="cx"> if (winner.weight.includes(targetWeight))
</span><span class="cx"> return targetWeight;
</span><span class="lines">@@ -1172,7 +1172,7 @@
</span><span class="cx"> return winner.weight.maximum;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-static inline void filterWeight(float target, const FontDatabase::InstalledFontFamily& installedFonts, Vector<bool>& filter)
</del><ins>+static inline void filterWeight(float target, const FontDatabase::InstalledFontFamily& installedFonts, std::unique_ptr<bool[]>& filter)
</ins><span class="cx"> {
</span><span class="cx"> iterateActiveFonts(installedFonts, filter, [&](auto& installedFont, size_t i) {
</span><span class="cx"> if (!installedFont.weight.includes(target))
</span><span class="lines">@@ -1212,8 +1212,11 @@
</span><span class="cx"> ASSERT(!familyFonts.isEmpty());
</span><span class="cx">
</span><span class="cx"> // Parallel to familyFonts.
</span><del>- Vector<bool> filter(familyFonts.size(), true);
</del><ins>+ std::unique_ptr<bool[]> filter { new bool[familyFonts.size()] };
+ for (size_t i = 0; i < familyFonts.size(); ++i)
+ filter[i] = true;
</ins><span class="cx">
</span><ins>+ // FIXME: Implement this.
</ins><span class="cx"> float targetStretch = 0;
</span><span class="cx"> if (auto closestStretch = findClosestStretch(targetStretch, familyFonts, filter))
</span><span class="cx"> filterStretch(closestStretch.value(), familyFonts, filter);
</span><span class="lines">@@ -1232,12 +1235,9 @@
</span><span class="cx"> else
</span><span class="cx"> return nullptr;
</span><span class="cx">
</span><del>- auto findFont = [&](const FontDatabase::InstalledFont& font, size_t) -> std::optional<std::reference_wrapper<const FontDatabase::InstalledFont>> {
- return std::reference_wrapper<const FontDatabase::InstalledFont>(font);
- };
- if (const auto& result = iterateActiveFontsWithReturn<std::reference_wrapper<const FontDatabase::InstalledFont>>(familyFonts, filter, findFont))
- return &result.value().get();
- return nullptr;
</del><ins>+ return iterateActiveFontsWithReturn<const FontDatabase::InstalledFont*>(familyFonts, filter, [](const FontDatabase::InstalledFont& font, size_t) {
+ return &font;
+ }).value_or(nullptr);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> #endif // !SHOULD_USE_CORE_TEXT_FONT_LOOKUP
</span><span class="lines">@@ -1252,9 +1252,16 @@
</span><span class="cx"> #if SHOULD_USE_CORE_TEXT_FONT_LOOKUP
</span><span class="cx"> return adoptCF(CTFontCreateForCSS(family.string().createCFString().get(), toCoreTextFontWeight(weight), requestedTraits, size));
</span><span class="cx"> #else
</span><del>- const auto& familyFonts = FontDatabase::singleton().lookUpFamilyName(family.string());
</del><ins>+ const auto& familyFonts = FontDatabase::singleton().collectionForFamily(family.string());
</ins><span class="cx"> if (familyFonts.isEmpty()) {
</span><del>- const auto& postScriptFont = FontDatabase::singleton().lookupPostScriptName(family);
</del><ins>+ // The CSS spec states that font-family only accepts a name of an actual font family. However, in WebKit, we claim to also
+ // support supplying a PostScript name instead. However, this creates problems when the other properties (font-weight,
+ // font-style) disagree with the traits of the PostScript-named font. The solution we have come up with is, when the default
+ // values for font-weight and font-style are supplied, honor the PostScript name, but if font-weight specifies bold or
+ // font-style specifies italic, then we run the regular matching algorithm on the family of the PostScript font. This way,
+ // if content simply states "font-family: PostScriptName;" without specifying the other font properties, it will be honored,
+ // but if a <b> appears as a descendent element, it will be honored too.
+ const auto& postScriptFont = FontDatabase::singleton().fontForPostScriptName(family);
</ins><span class="cx"> if (!postScriptFont.fontDescriptor)
</span><span class="cx"> return nullptr;
</span><span class="cx"> if (((requestedTraits & kCTFontTraitItalic) && postScriptFont.style.maximum < italicThreshold) || (weight >= FontWeight600 && postScriptFont.weight.maximum < 600)) {
</span><span class="lines">@@ -1261,7 +1268,7 @@
</span><span class="cx"> auto postScriptFamilyName = adoptCF(static_cast<CFStringRef>(CTFontDescriptorCopyAttribute(postScriptFont.fontDescriptor.get(), kCTFontFamilyNameAttribute)));
</span><span class="cx"> if (!postScriptFamilyName)
</span><span class="cx"> return nullptr;
</span><del>- const auto& familyFonts = FontDatabase::singleton().lookUpFamilyName(String(postScriptFamilyName.get()));
</del><ins>+ const auto& familyFonts = FontDatabase::singleton().collectionForFamily(String(postScriptFamilyName.get()));
</ins><span class="cx"> if (familyFonts.isEmpty())
</span><span class="cx"> return nullptr;
</span><span class="cx"> if (const auto* installedFont = findClosestFont(familyFonts, requestedTraits, weight)) {
</span></span></pre>
</div>
</div>
</body>
</html>