<!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>[177689] 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/177689">177689</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2014-12-23 11:23:48 -0800 (Tue, 23 Dec 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>platform/mac/editing/input/devanagari-ligature.html is flaky on Yosemite, ligature fails to form
https://bugs.webkit.org/show_bug.cgi?id=138683
Reviewed by Darin Adler.
This patch changes how we check fonts for equality. In particular, this patch adds a
objectForEqualityCheck() to Cocoa's FontPlatformData, and callers should pass this object
to CFEqual() to determine if two platform fonts are equal. This patch also migrates all
call sites to using this function.
I don't want to implement operator==() because there are many cases where the same font
is compared against many others, and this solution is cleaner than caching a comparison
object inside the font object itself.
No new tests because this is covered by platform/mac/editing/input/devanagari-ligature.html.
* platform/graphics/FontPlatformData.h:
* platform/graphics/cocoa/FontPlatformDataCocoa.mm:
(WebCore::FontPlatformData::objectForEqualityCheck):
* platform/graphics/mac/ComplexTextControllerCoreText.mm:
(WebCore::ComplexTextController::collectComplexTextRunsForCharacters):
* platform/graphics/mac/GlyphPageTreeNodeMac.cpp:
(WebCore::GlyphPage::fill):
* platform/graphics/mac/SimpleFontDataMac.mm:
(WebCore::SimpleFontData::canRenderCombiningCharacterSequence):
* platform/spi/cocoa/CoreTextSPI.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsFontPlatformDatah">trunk/Source/WebCore/platform/graphics/FontPlatformData.h</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicscocoaFontPlatformDataCocoamm">trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsmacComplexTextControllerCoreTextmm">trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsmacGlyphPageTreeNodeMaccpp">trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsmacSimpleFontDataMacmm">trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm</a></li>
<li><a href="#trunkSourceWebCoreplatformspicocoaCoreTextSPIh">trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/ChangeLog        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -1,5 +1,34 @@
</span><span class="cx"> 2014-12-23 Myles C. Maxfield <mmaxfield@apple.com>
</span><span class="cx">
</span><ins>+ platform/mac/editing/input/devanagari-ligature.html is flaky on Yosemite, ligature fails to form
+ https://bugs.webkit.org/show_bug.cgi?id=138683
+
+ Reviewed by Darin Adler.
+
+ This patch changes how we check fonts for equality. In particular, this patch adds a
+ objectForEqualityCheck() to Cocoa's FontPlatformData, and callers should pass this object
+ to CFEqual() to determine if two platform fonts are equal. This patch also migrates all
+ call sites to using this function.
+
+ I don't want to implement operator==() because there are many cases where the same font
+ is compared against many others, and this solution is cleaner than caching a comparison
+ object inside the font object itself.
+
+ No new tests because this is covered by platform/mac/editing/input/devanagari-ligature.html.
+
+ * platform/graphics/FontPlatformData.h:
+ * platform/graphics/cocoa/FontPlatformDataCocoa.mm:
+ (WebCore::FontPlatformData::objectForEqualityCheck):
+ * platform/graphics/mac/ComplexTextControllerCoreText.mm:
+ (WebCore::ComplexTextController::collectComplexTextRunsForCharacters):
+ * platform/graphics/mac/GlyphPageTreeNodeMac.cpp:
+ (WebCore::GlyphPage::fill):
+ * platform/graphics/mac/SimpleFontDataMac.mm:
+ (WebCore::SimpleFontData::canRenderCombiningCharacterSequence):
+ * platform/spi/cocoa/CoreTextSPI.h:
+
+2014-12-23 Myles C. Maxfield <mmaxfield@apple.com>
+
</ins><span class="cx"> [SVG -> OTF Converter] Make Placeholder a move-only type
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=139870
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsFontPlatformDatah"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/FontPlatformData.h (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/FontPlatformData.h        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/FontPlatformData.h        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -120,6 +120,8 @@
</span><span class="cx"> void setFont(CTFontRef);
</span><span class="cx">
</span><span class="cx"> CTFontRef ctFont() const;
</span><ins>+ static RetainPtr<CFTypeRef> objectForEqualityCheck(CTFontRef);
+ RetainPtr<CFTypeRef> objectForEqualityCheck() const;
</ins><span class="cx">
</span><span class="cx"> bool allowsLigatures() const;
</span><span class="cx"> bool roundsGlyphAdvances() const;
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicscocoaFontPlatformDataCocoamm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -24,6 +24,7 @@
</span><span class="cx"> #import "config.h"
</span><span class="cx"> #import "FontPlatformData.h"
</span><span class="cx">
</span><ins>+#import "CoreTextSPI.h"
</ins><span class="cx"> #import "SharedBuffer.h"
</span><span class="cx"> #import "WebCoreSystemInterface.h"
</span><span class="cx"> #import <wtf/text/WTFString.h>
</span><span class="lines">@@ -289,6 +290,25 @@
</span><span class="cx"> return m_ctFont.get();
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+RetainPtr<CFTypeRef> FontPlatformData::objectForEqualityCheck(CTFontRef ctFont)
+{
+#if (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED <= 80000) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED <= 101000)
+ auto fontDescriptor = adoptCF(CTFontCopyFontDescriptor(ctFont));
+ // FIXME: https://bugs.webkit.org/show_bug.cgi?id=138683 This is a shallow pointer compare for web fonts
+ // because the URL contains the address of the font. This means we might erroneously get false negatives.
+ RetainPtr<CFURLRef> url = adoptCF(static_cast<CFURLRef>(CTFontDescriptorCopyAttribute(fontDescriptor.get(), kCTFontReferenceURLAttribute)));
+ ASSERT(CFGetTypeID(url.get()) == CFURLGetTypeID());
+ return url;
+#else
+ return adoptCF(CTFontCopyGraphicsFont(ctFont, 0));
+#endif
+}
+
+RetainPtr<CFTypeRef> FontPlatformData::objectForEqualityCheck() const
+{
+ return objectForEqualityCheck(ctFont());
+}
+
</ins><span class="cx"> PassRefPtr<SharedBuffer> FontPlatformData::openTypeTable(uint32_t table) const
</span><span class="cx"> {
</span><span class="cx"> if (RetainPtr<CFDataRef> data = adoptCF(CGFontCopyTableForTag(cgFont(), table)))
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsmacComplexTextControllerCoreTextmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/mac/ComplexTextControllerCoreText.mm        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -249,14 +249,14 @@
</span><span class="cx"> CFDictionaryRef runAttributes = CTRunGetAttributes(ctRun);
</span><span class="cx"> CTFontRef runFont = static_cast<CTFontRef>(CFDictionaryGetValue(runAttributes, kCTFontAttributeName));
</span><span class="cx"> ASSERT(CFGetTypeID(runFont) == CTFontGetTypeID());
</span><del>- if (!CFEqual(runFont, fontData->platformData().ctFont())) {
</del><ins>+ RetainPtr<CFTypeRef> runFontEqualityObject = FontPlatformData::objectForEqualityCheck(runFont);
+ if (!CFEqual(runFontEqualityObject.get(), fontData->platformData().objectForEqualityCheck().get())) {
</ins><span class="cx"> // Begin trying to see if runFont matches any of the fonts in the fallback list.
</span><del>- RetainPtr<CGFontRef> runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
</del><span class="cx"> unsigned i = 0;
</span><span class="cx"> for (const FontData* candidateFontData = m_font.fontDataAt(i); candidateFontData; candidateFontData = m_font.fontDataAt(++i)) {
</span><span class="cx"> runFontData = candidateFontData->fontDataForCharacter(baseCharacter);
</span><del>- RetainPtr<CGFontRef> cgFont = adoptCF(CTFontCopyGraphicsFont(runFontData->platformData().ctFont(), 0));
- if (CFEqual(cgFont.get(), runCGFont.get()))
</del><ins>+ RetainPtr<CFTypeRef> runFontEqualityObject = runFontData->platformData().objectForEqualityCheck();
+ if (CFEqual(runFontEqualityObject.get(), runFontEqualityObject.get()))
</ins><span class="cx"> break;
</span><span class="cx"> runFontData = 0;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsmacGlyphPageTreeNodeMaccpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -111,9 +111,7 @@
</span><span class="cx"> Vector<CFIndex, 512> indexVector;
</span><span class="cx"> bool done = false;
</span><span class="cx">
</span><del>- // For the CGFont comparison in the loop, use the CGFont that Core Text assigns to the CTFont. This may
- // be non-CFEqual to fontData->platformData().cgFont().
- RetainPtr<CGFontRef> cgFont = adoptCF(CTFontCopyGraphicsFont(fontData->platformData().ctFont(), 0));
</del><ins>+ RetainPtr<CFTypeRef> fontEqualityObject = fontData->platformData().objectForEqualityCheck();
</ins><span class="cx">
</span><span class="cx"> for (CFIndex r = 0; r < runCount && !done ; ++r) {
</span><span class="cx"> // CTLine could map characters over multiple fonts using its own font fallback list.
</span><span class="lines">@@ -123,9 +121,7 @@
</span><span class="cx">
</span><span class="cx"> CFDictionaryRef attributes = CTRunGetAttributes(ctRun);
</span><span class="cx"> CTFontRef runFont = static_cast<CTFontRef>(CFDictionaryGetValue(attributes, kCTFontAttributeName));
</span><del>- RetainPtr<CGFontRef> runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
- // Use CGFont here as CFEqual for CTFont counts all attributes for font.
- bool gotBaseFont = CFEqual(cgFont.get(), runCGFont.get());
</del><ins>+ bool gotBaseFont = CFEqual(fontEqualityObject.get(), FontPlatformData::objectForEqualityCheck(runFont).get());
</ins><span class="cx"> if (gotBaseFont || fontData->platformData().isCompositeFontReference()) {
</span><span class="cx"> // This run uses the font we want. Extract glyphs.
</span><span class="cx"> CFIndex glyphCount = CTRunGetGlyphCount(ctRun);
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsmacSimpleFontDataMacmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -480,7 +480,7 @@
</span><span class="cx"> if (!addResult.isNewEntry)
</span><span class="cx"> return addResult.iterator->value;
</span><span class="cx">
</span><del>- RetainPtr<CGFontRef> cgFont = adoptCF(CTFontCopyGraphicsFont(platformData().ctFont(), 0));
</del><ins>+ RetainPtr<CFTypeRef> fontEqualityObject = platformData().objectForEqualityCheck();
</ins><span class="cx">
</span><span class="cx"> ProviderInfo info = { characters, length, getCFStringAttributes(0, platformData().orientation()) };
</span><span class="cx"> RetainPtr<CTLineRef> line = adoptCF(CTLineCreateWithUniCharProvider(&provideStringAndAttributes, 0, &info));
</span><span class="lines">@@ -493,8 +493,7 @@
</span><span class="cx"> ASSERT(CFGetTypeID(ctRun) == CTRunGetTypeID());
</span><span class="cx"> CFDictionaryRef runAttributes = CTRunGetAttributes(ctRun);
</span><span class="cx"> CTFontRef runFont = static_cast<CTFontRef>(CFDictionaryGetValue(runAttributes, kCTFontAttributeName));
</span><del>- RetainPtr<CGFontRef> runCGFont = adoptCF(CTFontCopyGraphicsFont(runFont, 0));
- if (!CFEqual(runCGFont.get(), cgFont.get()))
</del><ins>+ if (!CFEqual(fontEqualityObject.get(), FontPlatformData::objectForEqualityCheck(runFont).get()))
</ins><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformspicocoaCoreTextSPIh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h (177688 => 177689)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h        2014-12-23 18:45:05 UTC (rev 177688)
+++ trunk/Source/WebCore/platform/spi/cocoa/CoreTextSPI.h        2014-12-23 19:23:48 UTC (rev 177689)
</span><span class="lines">@@ -48,6 +48,8 @@
</span><span class="cx"> typedef const UniChar* (*CTUniCharProviderCallback)(CFIndex stringIndex, CFIndex* charCount, CFDictionaryRef* attributes, void* refCon);
</span><span class="cx"> typedef void (*CTUniCharDisposeCallback)(const UniChar* chars, void* refCon);
</span><span class="cx">
</span><ins>+extern const CFStringRef kCTFontReferenceURLAttribute;
+
</ins><span class="cx"> #if PLATFORM(IOS) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED > 1080)
</span><span class="cx"> #if !USE(APPLE_INTERNAL_SDK)
</span><span class="cx"> typedef CF_OPTIONS(uint32_t, CTFontTransformOptions)
</span></span></pre>
</div>
</div>
</body>
</html>