[Cocoa] Clean up m_isEmoji in FontPlatformData
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Jun 2015 18:18:23 +0000 (18:18 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Jun 2015 18:18:23 +0000 (18:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145644

Patch by Myles C. Maxfield <mmaxfield@apple.com> on 2015-06-04
Reviewed by Andreas Kling.

m_isEmoji used to be a public member variable only defined on iOS. This
means that, whenever it was consulted, the sites were surrounded by
PLATFORM(IOS) guards. A cleaner design is to use a getter and setter,
which on non-iOS platforms, always return false / do nothing. Then, the
use sites can just use these functions without having ugly guards.

No new tests because there is no behavior change.

* platform/graphics/FontPlatformData.h:
(WebCore::FontPlatformData::hash): Simplify isEmoji use site.
(WebCore::FontPlatformData::isEmoji): Getter. Returns false on Mac.
(WebCore::FontPlatformData::setIsEmoji): Setter. Does nothing on Mac.
* platform/graphics/cocoa/FontCascadeCocoa.mm:
(WebCore::pointAdjustedForEmoji): Simplify isEmoji use site.
(WebCore::FontCascade::drawGlyphs): Ditto.
* platform/graphics/cocoa/FontCocoa.mm:
(WebCore::Font::platformInit): Ditto.
(WebCore::canUseFastGlyphAdvanceGetter): Ditto.
(WebCore::isEmoji): Deleted.
* platform/graphics/cocoa/FontPlatformDataCocoa.mm:
(WebCore::FontPlatformData::platformDataInit): Simplify isEmoji use
site.
(WebCore::FontPlatformData::platformDataAssign): Ditto.
(WebCore::FontPlatformData::platformIsEqual): Ditto.
(WebCore::FontPlatformData::ctFontSize): Ditto.
* platform/graphics/ios/FontCacheIOS.mm:
(WebCore::FontCache::getSystemFontFallbackForCharacters): Ditto.
(WebCore::FontCache::createFontPlatformData): Ditto.
* platform/graphics/mac/ComplexTextController.cpp:
(WebCore::ComplexTextController::adjustGlyphsAndAdvances): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185205 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/FontPlatformData.h
Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm
Source/WebCore/platform/graphics/cocoa/FontCocoa.mm
Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm
Source/WebCore/platform/graphics/ios/FontCacheIOS.mm
Source/WebCore/platform/graphics/mac/ComplexTextController.cpp

index 8308b58..deeb9df 100644 (file)
@@ -1,3 +1,41 @@
+2015-06-04  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        [Cocoa] Clean up m_isEmoji in FontPlatformData
+        https://bugs.webkit.org/show_bug.cgi?id=145644
+
+        Reviewed by Andreas Kling.
+
+        m_isEmoji used to be a public member variable only defined on iOS. This
+        means that, whenever it was consulted, the sites were surrounded by
+        PLATFORM(IOS) guards. A cleaner design is to use a getter and setter,
+        which on non-iOS platforms, always return false / do nothing. Then, the
+        use sites can just use these functions without having ugly guards.
+
+        No new tests because there is no behavior change.
+
+        * platform/graphics/FontPlatformData.h:
+        (WebCore::FontPlatformData::hash): Simplify isEmoji use site.
+        (WebCore::FontPlatformData::isEmoji): Getter. Returns false on Mac.
+        (WebCore::FontPlatformData::setIsEmoji): Setter. Does nothing on Mac.
+        * platform/graphics/cocoa/FontCascadeCocoa.mm:
+        (WebCore::pointAdjustedForEmoji): Simplify isEmoji use site.
+        (WebCore::FontCascade::drawGlyphs): Ditto.
+        * platform/graphics/cocoa/FontCocoa.mm:
+        (WebCore::Font::platformInit): Ditto.
+        (WebCore::canUseFastGlyphAdvanceGetter): Ditto.
+        (WebCore::isEmoji): Deleted.
+        * platform/graphics/cocoa/FontPlatformDataCocoa.mm:
+        (WebCore::FontPlatformData::platformDataInit): Simplify isEmoji use
+        site.
+        (WebCore::FontPlatformData::platformDataAssign): Ditto.
+        (WebCore::FontPlatformData::platformIsEqual): Ditto.
+        (WebCore::FontPlatformData::ctFontSize): Ditto.
+        * platform/graphics/ios/FontCacheIOS.mm:
+        (WebCore::FontCache::getSystemFontFallbackForCharacters): Ditto.
+        (WebCore::FontCache::createFontPlatformData): Ditto.
+        * platform/graphics/mac/ComplexTextController.cpp:
+        (WebCore::ComplexTextController::adjustGlyphsAndAdvances): Ditto.
+
 2015-06-03  Brent Fulgham  <bfulgham@apple.com>
 
         REGRESSION (r181879): Scrolling order on pages with focused iframe is broken.
index 9dbb4da..ce1a9fd 100644 (file)
@@ -149,13 +149,14 @@ public:
 #if PLATFORM(WIN) && !USE(CAIRO)
         return m_font ? m_font->hash() : 0;
 #elif OS(DARWIN)
+        ASSERT(m_font || !m_cgFont || isEmoji());
+        uintptr_t flags = static_cast<uintptr_t>(m_isHashTableDeletedValue << 4 | isEmoji() << 3 | m_orientation << 2 | m_syntheticBold << 1 | m_syntheticOblique);
 #if USE(APPKIT)
-        ASSERT(m_font || !m_cgFont);
-        uintptr_t hashCodes[3] = { reinterpret_cast<uintptr_t>(const_cast<__CTFont*>(m_font.get())), m_widthVariant, static_cast<uintptr_t>(m_isHashTableDeletedValue << 3 | m_orientation << 2 | m_syntheticBold << 1 | m_syntheticOblique) };
+        uintptr_t fontHash = reinterpret_cast<uintptr_t>(const_cast<__CTFont*>(m_font.get()));
 #else
-        ASSERT(m_font || !m_cgFont || m_isEmoji);
-        uintptr_t hashCodes[3] = { static_cast<uintptr_t>(CFHash(m_font.get())), m_widthVariant, static_cast<uintptr_t>(m_isHashTableDeletedValue << 4 | m_isEmoji << 3 | m_orientation << 2 | m_syntheticBold << 1 | m_syntheticOblique) };
+        uintptr_t fontHash = reinterpret_cast<uintptr_t>(CFHash(m_font.get()));
 #endif
+        uintptr_t hashCodes[3] = { fontHash, m_widthVariant, flags };
         return StringHasher::hashMemory<sizeof(hashCodes)>(hashCodes);
 #elif USE(CAIRO)
         return PtrHash<cairo_scaled_font_t*>::hash(m_scaledFont);
@@ -190,6 +191,14 @@ public:
     String description() const;
 #endif
 
+#if PLATFORM(IOS)
+    bool isEmoji() const { return m_isEmoji; }
+    void setIsEmoji(bool isEmoji) { m_isEmoji = isEmoji; }
+#elif PLATFORM(MAC)
+    bool isEmoji() const { return false; }
+    void setIsEmoji(bool) { }
+#endif
+
 private:
     bool platformIsEqual(const FontPlatformData&) const;
     void platformDataInit(const FontPlatformData&);
@@ -205,9 +214,6 @@ public:
     bool m_syntheticBold { false };
     bool m_syntheticOblique { false };
     FontOrientation m_orientation { Horizontal };
-#if PLATFORM(IOS)
-    bool m_isEmoji { false };
-#endif
     float m_size { 0 };
     FontWidthVariant m_widthVariant { RegularWidth };
 
@@ -230,6 +236,9 @@ private:
     bool m_isColorBitmapFont { false };
     bool m_isCompositeFontReference { false };
     bool m_isHashTableDeletedValue { false };
+#if PLATFORM(IOS)
+    bool m_isEmoji { false };
+#endif
 
 #if PLATFORM(WIN)
     bool m_useGDI { false };
index 55ad798..b602d20 100644 (file)
@@ -246,10 +246,19 @@ static CGSize dilationSizeForTextColor(const Color& color)
 }
 #endif
 
-static FloatPoint pointAdjustedForEmoji(const FontPlatformData& platformData, FloatPoint point)
+static inline bool isOnOrAfterIOS6()
 {
 #if PLATFORM(IOS)
-    if (!platformData.m_isEmoji)
+    return iosExecutableWasLinkedOnOrAfterVersion(wkIOSSystemVersion_6_0);
+#else
+    ASSERT_NOT_REACHED();
+    return false;
+#endif
+}
+
+static FloatPoint pointAdjustedForEmoji(const FontPlatformData& platformData, FloatPoint point)
+{
+    if (!platformData.isEmoji())
         return point;
 
     // Mimic the positioining of non-bitmap glyphs, which are not subpixel-positioned.
@@ -266,19 +275,16 @@ static FloatPoint pointAdjustedForEmoji(const FontPlatformData& platformData, Fl
     float y = point.y();
     if (fontSize <= 15) {
         // Undo Core Text's y adjustment.
-        static float yAdjustmentFactor = iosExecutableWasLinkedOnOrAfterVersion(wkIOSSystemVersion_6_0) ? .19 : .1;
+        static float yAdjustmentFactor = isOnOrAfterIOS6() ? .19 : .1;
         point.setY(floorf(y - yAdjustmentFactor * (fontSize + 2) + 2));
     } else {
         if (fontSize < 26)
             y -= .35f * fontSize - 10;
 
         // Undo Core Text's y adjustment.
-        static float yAdjustment = iosExecutableWasLinkedOnOrAfterVersion(wkIOSSystemVersion_6_0) ? 3.8 : 2;
+        static float yAdjustment = isOnOrAfterIOS6() ? 3.8 : 2;
         point.setY(floorf(y - yAdjustment));
     }
-#else
-    UNUSED_PARAM(platformData);
-#endif
     return point;
 }
 
@@ -416,11 +422,7 @@ void FontCascade::drawGlyphs(GraphicsContext* context, const Font* font, const G
         // If shadows are ignoring transforms, then we haven't applied the Y coordinate flip yet, so down is negative.
         float shadowTextY = point.y() + shadowOffset.height() * (context->shadowsIgnoreTransforms() ? -1 : 1);
         showGlyphsWithAdvances(FloatPoint(shadowTextX, shadowTextY), font, cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
-#if !PLATFORM(IOS)
-        if (syntheticBoldOffset)
-#else
-        if (syntheticBoldOffset && !platformData.m_isEmoji)
-#endif
+        if (syntheticBoldOffset && !platformData.isEmoji())
             showGlyphsWithAdvances(FloatPoint(shadowTextX + syntheticBoldOffset, shadowTextY), font, cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
         context->setFillColor(fillColor, fillColorSpace);
     }
@@ -429,11 +431,7 @@ void FontCascade::drawGlyphs(GraphicsContext* context, const Font* font, const G
         showLetterpressedGlyphsWithAdvances(point, font, cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
     else
         showGlyphsWithAdvances(point, font, cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
-#if !PLATFORM(IOS)
-    if (syntheticBoldOffset)
-#else
-    if (syntheticBoldOffset && !platformData.m_isEmoji)
-#endif
+    if (syntheticBoldOffset && !platformData.isEmoji())
         showGlyphsWithAdvances(FloatPoint(point.x() + syntheticBoldOffset, point.y()), font, cgContext, glyphBuffer.glyphs(from), static_cast<const CGSize*>(glyphBuffer.advances(from)), numGlyphs);
 
     if (hasSimpleShadow)
index 810d937..88f9acf 100644 (file)
@@ -267,7 +267,7 @@ void Font::platformInit()
     if (platformData().orientation() == Vertical && !isTextOrientationFallback())
         m_hasVerticalGlyphs = fontHasVerticalGlyphs(m_platformData.ctFont());
 
-    if (m_platformData.m_isEmoji) {
+    if (m_platformData.isEmoji()) {
         int thirdOfSize = m_platformData.size() / 3;
         m_fontMetrics.setAscent(thirdOfSize);
         m_fontMetrics.setDescent(thirdOfSize);
@@ -455,16 +455,6 @@ static inline bool advanceForColorBitmapFont(const FontPlatformData& platformDat
 #endif
 }
 
-static inline bool isEmoji(const FontPlatformData& platformData)
-{
-#if PLATFORM(IOS)
-    return platformData.m_isEmoji;
-#else
-    UNUSED_PARAM(platformData);
-    return false;
-#endif
-}
-
 static inline bool canUseFastGlyphAdvanceGetter(const Font& font, Glyph glyph, CGSize& advance, bool& populatedAdvance)
 {
     const FontPlatformData& platformData = font.platformData();
@@ -472,7 +462,7 @@ static inline bool canUseFastGlyphAdvanceGetter(const Font& font, Glyph glyph, C
     if (font.hasCustomTracking())
         return false;
     // Fast getter doesn't work for emoji
-    if (isEmoji(platformData))
+    if (platformData.isEmoji())
         return false;
     // ... or for any bitmap fonts in general
     if (advanceForColorBitmapFont(platformData, glyph, advance)) {
index 5682f44..9e2a126 100644 (file)
@@ -58,9 +58,7 @@ void FontPlatformData::platformDataInit(const FontPlatformData& f)
 {
     m_font = f.m_font;
 
-#if PLATFORM(IOS)
-    m_isEmoji = f.m_isEmoji;
-#endif
+    setIsEmoji(f.isEmoji());
     m_cgFont = f.m_cgFont;
     m_ctFont = f.m_ctFont;
 }
@@ -68,9 +66,7 @@ void FontPlatformData::platformDataInit(const FontPlatformData& f)
 const FontPlatformData& FontPlatformData::platformDataAssign(const FontPlatformData& f)
 {
     m_cgFont = f.m_cgFont;
-#if PLATFORM(IOS)
-    m_isEmoji = f.m_isEmoji;
-#endif
+    setIsEmoji(f.isEmoji());
     if (m_font && f.m_font && CFEqual(m_font.get(), f.m_font.get()))
         return *this;
     m_font = f.m_font;
@@ -85,19 +81,13 @@ bool FontPlatformData::platformIsEqual(const FontPlatformData& other) const
     if (m_font || other.m_font) {
 #if PLATFORM(IOS)
         result = m_font && other.m_font && CFEqual(m_font.get(), other.m_font.get());
-#if !ASSERT_DISABLED
-        if (result)
-            ASSERT(m_isEmoji == other.m_isEmoji);
-#endif
 #else
         result = m_font == other.m_font;
 #endif
+        ASSERT(!result || isEmoji() == other.isEmoji());
         return result;
     }
-#if PLATFORM(IOS) && !ASSERT_DISABLED
-    if (m_cgFont == other.m_cgFont)
-        ASSERT(m_isEmoji == other.m_isEmoji);
-#endif
+    ASSERT(m_cgFont != other.m_cgFont || isEmoji() == other.isEmoji());
     return m_cgFont == other.m_cgFont;
 }
 
@@ -202,12 +192,8 @@ static CTFontDescriptorRef cascadeToLastResortAndDisableSwashesFontDescriptor()
 
 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
+    // On iOS, Apple Color Emoji size is adjusted (and then re-adjusted by Core Text) and capped.
+    return !isEmoji() ? m_size : m_size <= 15 ? 4 * (m_size + 2) / static_cast<CGFloat>(5) : 16;
 }
 
 CTFontRef FontPlatformData::ctFont() const
index 9671bcf..49a4796 100644 (file)
@@ -94,7 +94,7 @@ PassRefPtr<Font> FontCache::getSystemFontFallbackForCharacters(const FontDescrip
     bool syntheticOblique = (originalTraits & kCTFontTraitItalic) && !(actualTraits & kCTFontTraitItalic);
 
     FontPlatformData alternateFont(substituteFont.get(), platformData.size(), syntheticBold, syntheticOblique, platformData.m_orientation);
-    alternateFont.m_isEmoji = CTFontIsAppleColorEmoji(substituteFont.get());
+    alternateFont.setIsEmoji(CTFontIsAppleColorEmoji(substituteFont.get()));
 
     return fontForPlatformData(alternateFont);
 }
@@ -698,7 +698,7 @@ std::unique_ptr<FontPlatformData> FontCache::createFontPlatformData(const FontDe
 
     auto result = std::make_unique<FontPlatformData>(ctFont.get(), size, syntheticBold, syntheticOblique, fontDescription.orientation(), fontDescription.widthVariant());
     if (isAppleColorEmoji)
-        result->m_isEmoji = true;
+        result->setIsEmoji(true);
     return result;
 }
 
index aac1994..4a9e7a3 100644 (file)
@@ -603,9 +603,7 @@ void ComplexTextController::adjustGlyphsAndAdvances()
         ComplexTextRun& complexTextRun = *m_complexTextRuns[r];
         unsigned glyphCount = complexTextRun.glyphCount();
         const Font& font = complexTextRun.font();
-#if PLATFORM(IOS)
-        bool isEmoji = font.platformData().m_isEmoji;
-#endif
+        bool isEmoji = font.platformData().isEmoji();
 
         // Represent the initial advance for a text run by adjusting the advance
         // of the last glyph of the previous text run in the glyph buffer.
@@ -653,10 +651,8 @@ void ComplexTextController::adjustGlyphsAndAdvances()
             bool treatAsSpace = FontCascade::treatAsSpace(ch);
             CGGlyph glyph = treatAsSpace ? font.spaceGlyph() : glyphs[i];
             CGSize advance = treatAsSpace ? CGSizeMake(spaceWidth, advances[i].height) : advances[i];
-#if PLATFORM(IOS)
             if (isEmoji && advance.width)
                 advance.width = font.widthForGlyph(glyph);
-#endif
 
             if (ch == '\t' && m_run.allowTabs())
                 advance.width = m_font.tabWidth(font, m_run.tabSize(), m_run.xPos() + m_totalWidth + widthSinceLastCommit);