<!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>[200364] 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/200364">200364</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-05-03 01:18:13 -0700 (Tue, 03 May 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[OpenType] OpenTypeVerticalData object should not be created if the font is not OpenType
https://bugs.webkit.org/show_bug.cgi?id=157172
Reviewed by Michael Catanzaro.
It's a bit weird that the object is always created and has an isOpenType() method to check whether it's an
OpenType or not. The caller is always deleting the object when it's not an OpenType, so it would be better if
the create method returned nullptr instead of creating the object when the font is not OpenType.
* platform/graphics/FontCache.cpp:
(WebCore::FontCache::verticalData): Do not use isOpenType(), we can now simply use the return value of OpenTypeVerticalData::create().
* platform/graphics/opentype/OpenTypeVerticalData.cpp:
(WebCore::loadHmtxTable): Moved to a helper funtion that returns false if the font is not OpenType.
(WebCore::OpenTypeVerticalData::create): Try to load the Hmtx table, and create the object if succeeded or
return nullptr otherwise.
(WebCore::OpenTypeVerticalData::OpenTypeVerticalData): Receive the advanceWidths as constructor parameter.
(WebCore::OpenTypeVerticalData::loadMetrics): Load all other tables.
* platform/graphics/opentype/OpenTypeVerticalData.h:
(WebCore::OpenTypeVerticalData::isOpenType): Deleted.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsFontCachecpp">trunk/Source/WebCore/platform/graphics/FontCache.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsopentypeOpenTypeVerticalDatacpp">trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsopentypeOpenTypeVerticalDatah">trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (200363 => 200364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-05-03 07:19:07 UTC (rev 200363)
+++ trunk/Source/WebCore/ChangeLog 2016-05-03 08:18:13 UTC (rev 200364)
</span><span class="lines">@@ -1,3 +1,25 @@
</span><ins>+2016-05-03 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ [OpenType] OpenTypeVerticalData object should not be created if the font is not OpenType
+ https://bugs.webkit.org/show_bug.cgi?id=157172
+
+ Reviewed by Michael Catanzaro.
+
+ It's a bit weird that the object is always created and has an isOpenType() method to check whether it's an
+ OpenType or not. The caller is always deleting the object when it's not an OpenType, so it would be better if
+ the create method returned nullptr instead of creating the object when the font is not OpenType.
+
+ * platform/graphics/FontCache.cpp:
+ (WebCore::FontCache::verticalData): Do not use isOpenType(), we can now simply use the return value of OpenTypeVerticalData::create().
+ * platform/graphics/opentype/OpenTypeVerticalData.cpp:
+ (WebCore::loadHmtxTable): Moved to a helper funtion that returns false if the font is not OpenType.
+ (WebCore::OpenTypeVerticalData::create): Try to load the Hmtx table, and create the object if succeeded or
+ return nullptr otherwise.
+ (WebCore::OpenTypeVerticalData::OpenTypeVerticalData): Receive the advanceWidths as constructor parameter.
+ (WebCore::OpenTypeVerticalData::loadMetrics): Load all other tables.
+ * platform/graphics/opentype/OpenTypeVerticalData.h:
+ (WebCore::OpenTypeVerticalData::isOpenType): Deleted.
+
</ins><span class="cx"> 2016-05-02 Darin Adler <darin@apple.com>
</span><span class="cx">
</span><span class="cx"> Change IDL enumerations to be nested in their C++ class instead of at WebCore namespace level
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsFontCachecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/FontCache.cpp (200363 => 200364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/FontCache.cpp 2016-05-03 07:19:07 UTC (rev 200363)
+++ trunk/Source/WebCore/platform/graphics/FontCache.cpp 2016-05-03 08:18:13 UTC (rev 200364)
</span><span class="lines">@@ -317,11 +317,9 @@
</span><span class="cx">
</span><span class="cx"> RefPtr<OpenTypeVerticalData> FontCache::verticalData(const FontPlatformData& platformData)
</span><span class="cx"> {
</span><del>- auto addResult = fontVerticalDataCache().add(platformData, nullptr);
- if (addResult.isNewEntry) {
- RefPtr<OpenTypeVerticalData> data = OpenTypeVerticalData::create(platformData);
- addResult.iterator->value = data->isOpenType() ? WTFMove(data) : nullptr;
- }
</del><ins>+ auto addResult = fontVerticalDataCache().ensure(platformData, [&platformData] {
+ return OpenTypeVerticalData::create(platformData);
+ });
</ins><span class="cx"> return addResult.iterator->value;
</span><span class="cx"> }
</span><span class="cx"> #endif
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsopentypeOpenTypeVerticalDatacpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp (200363 => 200364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp 2016-05-03 07:19:07 UTC (rev 200363)
+++ trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.cpp 2016-05-03 08:18:13 UTC (rev 200364)
</span><span class="lines">@@ -379,39 +379,52 @@
</span><span class="cx">
</span><span class="cx"> } // namespace OpenType
</span><span class="cx">
</span><del>-OpenTypeVerticalData::OpenTypeVerticalData(const FontPlatformData& platformData)
- : m_defaultVertOriginY(0)
</del><ins>+static bool loadHmtxTable(const FontPlatformData& platformData, Vector<uint16_t>& advanceWidths)
</ins><span class="cx"> {
</span><del>- loadMetrics(platformData);
- loadVerticalGlyphSubstitutions(platformData);
-}
-
-void OpenTypeVerticalData::loadMetrics(const FontPlatformData& platformData)
-{
- // Load hhea and hmtx to get x-component of vertical origins.
- // If these tables are missing, it's not an OpenType font.
</del><span class="cx"> RefPtr<SharedBuffer> buffer = platformData.openTypeTable(OpenType::HheaTag);
</span><span class="cx"> const OpenType::HheaTable* hhea = OpenType::validateTable<OpenType::HheaTable>(buffer);
</span><span class="cx"> if (!hhea)
</span><del>- return;
</del><ins>+ return false;
</ins><span class="cx"> uint16_t countHmtxEntries = hhea->numberOfHMetrics;
</span><span class="cx"> if (!countHmtxEntries) {
</span><span class="cx"> LOG_ERROR("Invalid numberOfHMetrics");
</span><del>- return;
</del><ins>+ return false;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> buffer = platformData.openTypeTable(OpenType::HmtxTag);
</span><span class="cx"> const OpenType::HmtxTable* hmtx = OpenType::validateTable<OpenType::HmtxTable>(buffer, countHmtxEntries);
</span><span class="cx"> if (!hmtx) {
</span><span class="cx"> LOG_ERROR("hhea exists but hmtx does not (or broken)");
</span><del>- return;
</del><ins>+ return false;
</ins><span class="cx"> }
</span><del>- m_advanceWidths.resize(countHmtxEntries);
</del><ins>+
+ advanceWidths.resize(countHmtxEntries);
</ins><span class="cx"> for (uint16_t i = 0; i < countHmtxEntries; ++i)
</span><del>- m_advanceWidths[i] = hmtx->entries[i].advanceWidth;
</del><ins>+ advanceWidths[i] = hmtx->entries[i].advanceWidth;
+ return true;
+}
</ins><span class="cx">
</span><ins>+RefPtr<OpenTypeVerticalData> OpenTypeVerticalData::create(const FontPlatformData& platformData)
+{
+ // Load hhea and hmtx to get x-component of vertical origins. If these tables are missing, it's not an OpenType font.
+ Vector<uint16_t> advanceWidths;
+ if (!loadHmtxTable(platformData, advanceWidths))
+ return nullptr;
+
+ return adoptRef(new OpenTypeVerticalData(platformData, WTFMove(advanceWidths)));
+}
+
+OpenTypeVerticalData::OpenTypeVerticalData(const FontPlatformData& platformData, Vector<uint16_t>&& advanceWidths)
+ : m_advanceWidths(WTFMove(advanceWidths))
+{
+ loadMetrics(platformData);
+ loadVerticalGlyphSubstitutions(platformData);
+}
+
+void OpenTypeVerticalData::loadMetrics(const FontPlatformData& platformData)
+{
</ins><span class="cx"> // Load vhea first. This table is required for fonts that support vertical flow.
</span><del>- buffer = platformData.openTypeTable(OpenType::VheaTag);
</del><ins>+ RefPtr<SharedBuffer> buffer = platformData.openTypeTable(OpenType::VheaTag);
</ins><span class="cx"> const OpenType::VheaTable* vhea = OpenType::validateTable<OpenType::VheaTable>(buffer);
</span><span class="cx"> if (!vhea)
</span><span class="cx"> return;
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsopentypeOpenTypeVerticalDatah"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.h (200363 => 200364)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.h 2016-05-03 07:19:07 UTC (rev 200363)
+++ trunk/Source/WebCore/platform/graphics/opentype/OpenTypeVerticalData.h 2016-05-03 08:18:13 UTC (rev 200364)
</span><span class="lines">@@ -41,19 +41,15 @@
</span><span class="cx">
</span><span class="cx"> class OpenTypeVerticalData : public RefCounted<OpenTypeVerticalData> {
</span><span class="cx"> public:
</span><del>- static PassRefPtr<OpenTypeVerticalData> create(const FontPlatformData& platformData)
- {
- return adoptRef(new OpenTypeVerticalData(platformData));
- }
</del><ins>+ static RefPtr<OpenTypeVerticalData> create(const FontPlatformData&);
</ins><span class="cx">
</span><del>- bool isOpenType() const { return !m_advanceWidths.isEmpty(); }
</del><span class="cx"> bool hasVerticalMetrics() const { return !m_advanceHeights.isEmpty(); }
</span><span class="cx"> float advanceHeight(const Font*, Glyph) const;
</span><span class="cx"> void getVerticalTranslationsForGlyphs(const Font*, const Glyph*, size_t, float* outXYArray) const;
</span><span class="cx"> void substituteWithVerticalGlyphs(const Font*, GlyphPage*) const;
</span><span class="cx">
</span><span class="cx"> private:
</span><del>- explicit OpenTypeVerticalData(const FontPlatformData&);
</del><ins>+ explicit OpenTypeVerticalData(const FontPlatformData&, Vector<uint16_t>&& advanceWidths);
</ins><span class="cx">
</span><span class="cx"> void loadMetrics(const FontPlatformData&);
</span><span class="cx"> void loadVerticalGlyphSubstitutions(const FontPlatformData&);
</span><span class="lines">@@ -63,7 +59,7 @@
</span><span class="cx"> Vector<uint16_t> m_advanceWidths;
</span><span class="cx"> Vector<uint16_t> m_advanceHeights;
</span><span class="cx"> Vector<int16_t> m_topSideBearings;
</span><del>- int16_t m_defaultVertOriginY;
</del><ins>+ int16_t m_defaultVertOriginY { 0 };
</ins><span class="cx"> HashMap<Glyph, int16_t> m_vertOriginY;
</span><span class="cx"> };
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>