Title: [183271] trunk/Source/WebCore
Revision
183271
Author
mmaxfi...@apple.com
Date
2015-04-24 11:45:10 -0700 (Fri, 24 Apr 2015)

Log Message

[iOS] Reimplement r182512 and r183153 in a cleaner way
https://bugs.webkit.org/show_bug.cgi?id=144151

Reviewed by Enrica Casucci.

On iOS, we create FontPlatformData's ctFont() by round tripping it through a CGFontRef.
This causes the resultant ctFont() to lose system-font-ness. Patches r182512 and r183153
react to this fact by making users of a FontPlatformData use the original font passed in
to the FontPlatformData instead of the FontPlatformData's ctFont(), but only if we
detect that the FontPlatformData represents a system font (the underlying APIs have
different behavior for system fonts and non-system-fonts).

However, on OS X, we create a FontPlatformData's ctFont() directly from the original
font passed in to the constructor. This preserves system-font-ness (because it no
longer has the CGFontRef in the middle of the transformation). Therefore, OS X has the
correct behavior regarding system fonts.

The difference between the two ctFont() creation codepaths seems to be historical
rather than intentional. Rather than change all the call sites of ctFont() to use a
different platform font object if a system font is detected, a cleaner solution is to
simply unify the two ctFont() creation codepaths to the version which preserves system-
font-ness. Doing this will make all users of FontPlatformData automatically have the
correct behavior with no updates.

This patch reverts the relevant parts of r182512 and r183153 in favor of this new
solution.

No new tests because there is no behavior change.

* platform/graphics/FontPlatformData.h:
(WebCore::FontPlatformData::hashTableDeletedFontValue): Deleted.
* platform/graphics/cocoa/FontCocoa.mm:
(WebCore::Font::platformWidthForGlyph):
* platform/graphics/cocoa/FontPlatformDataCocoa.mm:
(WebCore::FontPlatformData::ctFontSize):
(WebCore::FontPlatformData::ctFont):
* platform/graphics/mac/SimpleFontDataCoreText.cpp:
(WebCore::Font::getCFStringAttributes):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (183270 => 183271)


--- trunk/Source/WebCore/ChangeLog	2015-04-24 18:26:14 UTC (rev 183270)
+++ trunk/Source/WebCore/ChangeLog	2015-04-24 18:45:10 UTC (rev 183271)
@@ -1,5 +1,46 @@
 2015-04-24  Myles C. Maxfield  <mmaxfi...@apple.com>
 
+        [iOS] Reimplement r182512 and r183153 in a cleaner way
+        https://bugs.webkit.org/show_bug.cgi?id=144151
+
+        Reviewed by Enrica Casucci.
+
+        On iOS, we create FontPlatformData's ctFont() by round tripping it through a CGFontRef.
+        This causes the resultant ctFont() to lose system-font-ness. Patches r182512 and r183153
+        react to this fact by making users of a FontPlatformData use the original font passed in
+        to the FontPlatformData instead of the FontPlatformData's ctFont(), but only if we
+        detect that the FontPlatformData represents a system font (the underlying APIs have
+        different behavior for system fonts and non-system-fonts).
+
+        However, on OS X, we create a FontPlatformData's ctFont() directly from the original
+        font passed in to the constructor. This preserves system-font-ness (because it no
+        longer has the CGFontRef in the middle of the transformation). Therefore, OS X has the
+        correct behavior regarding system fonts.
+
+        The difference between the two ctFont() creation codepaths seems to be historical
+        rather than intentional. Rather than change all the call sites of ctFont() to use a
+        different platform font object if a system font is detected, a cleaner solution is to
+        simply unify the two ctFont() creation codepaths to the version which preserves system-
+        font-ness. Doing this will make all users of FontPlatformData automatically have the
+        correct behavior with no updates.
+
+        This patch reverts the relevant parts of r182512 and r183153 in favor of this new
+        solution.
+
+        No new tests because there is no behavior change.
+
+        * platform/graphics/FontPlatformData.h:
+        (WebCore::FontPlatformData::hashTableDeletedFontValue): Deleted.
+        * platform/graphics/cocoa/FontCocoa.mm:
+        (WebCore::Font::platformWidthForGlyph):
+        * platform/graphics/cocoa/FontPlatformDataCocoa.mm:
+        (WebCore::FontPlatformData::ctFontSize):
+        (WebCore::FontPlatformData::ctFont):
+        * platform/graphics/mac/SimpleFontDataCoreText.cpp:
+        (WebCore::Font::getCFStringAttributes):
+
+2015-04-24  Myles C. Maxfield  <mmaxfi...@apple.com>
+
         Unreviewed build fix.
 
         * platform/graphics/cocoa/FontPlatformDataCocoa.mm:

Modified: trunk/Source/WebCore/platform/graphics/FontPlatformData.h (183270 => 183271)


--- trunk/Source/WebCore/platform/graphics/FontPlatformData.h	2015-04-24 18:26:14 UTC (rev 183270)
+++ trunk/Source/WebCore/platform/graphics/FontPlatformData.h	2015-04-24 18:45:10 UTC (rev 183271)
@@ -203,6 +203,7 @@
 #if PLATFORM(COCOA)
     static CTFontRef hashTableDeletedFontValue() { return reinterpret_cast<CTFontRef>(-1); }
     static bool isValidCTFontRef(CTFontRef font) { return font && font != hashTableDeletedFontValue(); }
+    CGFloat ctFontSize() const;
 #endif
 #if PLATFORM(WIN)
     void platformDataInit(HFONT, float size, HDC, WCHAR* faceName);

Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm (183270 => 183271)


--- trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm	2015-04-24 18:26:14 UTC (rev 183270)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm	2015-04-24 18:45:10 UTC (rev 183271)
@@ -495,12 +495,8 @@
             LOG_ERROR("Unable to cache glyph widths for %@ %f", fullName.get(), pointSize);
             advance.width = 0;
         }
-    } else if (!populatedAdvance) {
-        // m_platformData.font() returns the original font that was passed into the FontPlatformData constructor. In the case of fonts that have custom tracking,
-        // the custom tracking does not survive the transformation to either m_platformData.cgFont() nor m_platformData.ctFont(), so we must use the original
-        // font() that was passed in. However, for web fonts, m_platformData.font() is null, so we must use m_platformData.ctFont() for those cases.
-        CTFontGetAdvancesForGlyphs(hasCustomTracking() ? m_platformData.font() : m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, &glyph, &advance, 1);
-    }
+    } else if (!populatedAdvance)
+        CTFontGetAdvancesForGlyphs(m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, &glyph, &advance, 1);
 
     return advance.width + m_syntheticBoldOffset;
 }

Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm (183270 => 183271)


--- trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm	2015-04-24 18:26:14 UTC (rev 183270)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm	2015-04-24 18:45:10 UTC (rev 183271)
@@ -221,13 +221,22 @@
     return descriptor;
 }
 
+CGFloat FontPlatformData::ctFontSize() const
+{
+#if PLATFORM(IOS)
+    // Apple Color Emoji size is adjusted (and then re-adjusted by Core Text) and capped.
+    return !m_isEmoji ? m_size : m_size <= 15 ? 4 * (m_size + 2) / static_cast<CGFloat>(5) : 16;
+#else
+    return m_size;
+#endif
+}
+
 CTFontRef FontPlatformData::ctFont() const
 {
     if (m_ctFont)
         return m_ctFont.get();
 
     ASSERT(m_cgFont.get());
-#if !PLATFORM(IOS)
     m_ctFont = m_font;
     if (m_ctFont) {
         CTFontDescriptorRef fontDescriptor;
@@ -237,20 +246,9 @@
             fontDescriptor = cascadeToLastResortAndDisableSwashesFontDescriptor();
         else
             fontDescriptor = cascadeToLastResortFontDescriptor();
-        m_ctFont = adoptCF(CTFontCreateCopyWithAttributes(m_ctFont.get(), m_size, 0, fontDescriptor));
+        m_ctFont = adoptCF(CTFontCreateCopyWithAttributes(m_ctFont.get(), ctFontSize(), 0, fontDescriptor));
     } else
-        m_ctFont = adoptCF(CTFontCreateWithGraphicsFont(m_cgFont.get(), m_size, 0, cascadeToLastResortFontDescriptor()));
-#else
-    // Apple Color Emoji size is adjusted (and then re-adjusted by Core Text) and capped.
-    CGFloat size = !m_isEmoji ? m_size : m_size <= 15 ? 4 * (m_size + 2) / static_cast<CGFloat>(5) : 16;
-    CTFontDescriptorRef fontDescriptor;
-    const char* postScriptName = CGFontGetPostScriptName(m_cgFont.get());
-    if (postScriptName && (!strcmp(postScriptName, "HoeflerText-Italic") || !strcmp(postScriptName, "HoeflerText-BlackItalic")))
-        fontDescriptor = cascadeToLastResortAndDisableSwashesFontDescriptor();
-    else
-        fontDescriptor = cascadeToLastResortFontDescriptor();
-    m_ctFont = adoptCF(CTFontCreateWithGraphicsFont(m_cgFont.get(), size, 0, fontDescriptor));
-#endif // !PLATFORM(IOS)
+        m_ctFont = adoptCF(CTFontCreateWithGraphicsFont(m_cgFont.get(), ctFontSize(), 0, cascadeToLastResortFontDescriptor()));
 
     if (m_widthVariant != RegularWidth) {
         int featureTypeValue = kTextSpacingType;

Modified: trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp (183270 => 183271)


--- trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp	2015-04-24 18:26:14 UTC (rev 183270)
+++ trunk/Source/WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp	2015-04-24 18:45:10 UTC (rev 183271)
@@ -46,10 +46,7 @@
     attributesDictionary = adoptCF(CFDictionaryCreateMutable(kCFAllocatorDefault, 4, &kCFCopyStringDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
     CFMutableDictionaryRef mutableAttributes = (CFMutableDictionaryRef)attributesDictionary.get();
 
-    // m_platformData.font() returns the original font that was passed into the FontPlatformData constructor. In the case of fonts that have custom tracking,
-    // the custom tracking does not survive the transformation to either m_platformData.cgFont() nor m_platformData.ctFont(), so we must use the original
-    // font() that was passed in. However, for web fonts, m_platformData.font() is null, so we must use m_platformData.ctFont() for those cases.
-    CFDictionarySetValue(mutableAttributes, kCTFontAttributeName, hasCustomTracking() ? platformData().font() : platformData().ctFont());
+    CFDictionarySetValue(mutableAttributes, kCTFontAttributeName, platformData().ctFont());
 
     if (!(typesettingFeatures & Kerning)) {
         const float zero = 0;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to