Diff
Modified: trunk/Source/WebCore/ChangeLog (182511 => 182512)
--- trunk/Source/WebCore/ChangeLog 2015-04-08 00:38:11 UTC (rev 182511)
+++ trunk/Source/WebCore/ChangeLog 2015-04-08 01:08:15 UTC (rev 182512)
@@ -1,3 +1,36 @@
+2015-04-07 Myles C. Maxfield <mmaxfi...@apple.com>
+
+ [Cocoa] System fonts do not get correct tracking
+ https://bugs.webkit.org/show_bug.cgi?id=143395
+
+ Reviewed by Ryosuke Niwa.
+
+ Inside FontPlatformData, we have two CTFonts. If the user has specified
+ -webkit-system-font, we will pass in a CTFont, and the FontPlatformData
+ will wrap it. However, we will then roundtrip through CGFont in order
+ to create a second CTFont. We were basing our tracking and system
+ font knowledge off of this round-tripped font instead of the original font.
+
+ Note that this design is terrible and needs to be overhauled.
+ FontPlatformData should only have a single platform font inside it.
+
+ This patch also caches whether or not a font is a system font.
+
+ No new tests because it is impossible to test the tracking of the
+ system font in a robust way.
+
+ * platform/graphics/Font.cpp:
+ (WebCore::Font::Font): Rearrange member variables.
+ * platform/graphics/Font.h: Move member variables around for better
+ packing, and cache whether or not the font is a system font.
+ * platform/graphics/FontData.h: Add comment
+ * platform/graphics/cocoa/FontCocoa.mm:
+ (WebCore::Font::platformInit): Cache whether or not the font is a system
+ font.
+ (WebCore::hasCustomTracking): Use cached value.
+ (WebCore::canUseFastGlyphAdvanceGetter):
+ (WebCore::Font::platformWidthForGlyph):
+
2015-04-07 Alex Christensen <achristen...@webkit.org>
Block popups from content extensions.
Modified: trunk/Source/WebCore/platform/graphics/Font.cpp (182511 => 182512)
--- trunk/Source/WebCore/platform/graphics/Font.cpp 2015-04-08 00:38:11 UTC (rev 182511)
+++ trunk/Source/WebCore/platform/graphics/Font.cpp 2015-04-08 01:08:15 UTC (rev 182512)
@@ -50,21 +50,31 @@
const float smallCapsFontSizeMultiplier = 0.7f;
const float emphasisMarkFontSizeMultiplier = 0.5f;
-Font::Font(const FontPlatformData& platformData, bool isCustomFont, bool isLoading, bool isTextOrientationFallback)
+Font::Font(const FontPlatformData& platformData, std::unique_ptr<SVGData>&& svgData, bool isCustomFont, bool isLoading, bool isTextOrientationFallback)
: m_maxCharWidth(-1)
, m_avgCharWidth(-1)
, m_platformData(platformData)
+ , m_svgData(WTF::move(svgData))
+ , m_mathData(nullptr)
, m_treatAsFixedPitch(false)
, m_isCustomFont(isCustomFont)
, m_isLoading(isLoading)
, m_isTextOrientationFallback(isTextOrientationFallback)
, m_isBrokenIdeographFallback(false)
- , m_mathData(nullptr)
-#if ENABLE(OPENTYPE_VERTICAL)
- , m_verticalData(0)
-#endif
, m_hasVerticalGlyphs(false)
+ , m_isUsedInSystemFallbackCache(false)
+#if PLATFORM(COCOA) || PLATFORM(WIN)
+ , m_isSystemFont(false)
+#endif
+#if PLATFORM(IOS)
+ , m_shouldNotBeUsedForArabic(false)
+#endif
{
+}
+
+Font::Font(const FontPlatformData& platformData, bool isCustomFont, bool isLoading, bool isTextOrientationFallback)
+ : Font(platformData, std::unique_ptr<SVGData>(), isCustomFont, isLoading, isTextOrientationFallback)
+{
platformInit();
platformGlyphInit();
platformCharWidthInit();
@@ -77,21 +87,7 @@
}
Font::Font(std::unique_ptr<SVGData> svgData, float fontSize, bool syntheticBold, bool syntheticItalic)
- : m_platformData(FontPlatformData(fontSize, syntheticBold, syntheticItalic))
- , m_svgData(WTF::move(svgData))
- , m_treatAsFixedPitch(false)
- , m_isCustomFont(true)
- , m_isLoading(false)
- , m_isTextOrientationFallback(false)
- , m_isBrokenIdeographFallback(false)
- , m_mathData(nullptr)
-#if ENABLE(OPENTYPE_VERTICAL)
- , m_verticalData(0)
-#endif
- , m_hasVerticalGlyphs(false)
-#if PLATFORM(IOS)
- , m_shouldNotBeUsedForArabic(false)
-#endif
+ : Font(FontPlatformData(fontSize, syntheticBold, syntheticItalic), WTF::move(svgData), true, false, false)
{
m_svgData->initializeFont(this, fontSize);
}
Modified: trunk/Source/WebCore/platform/graphics/Font.h (182511 => 182512)
--- trunk/Source/WebCore/platform/graphics/Font.h 2015-04-08 00:38:11 UTC (rev 182511)
+++ trunk/Source/WebCore/platform/graphics/Font.h 2015-04-08 01:08:15 UTC (rev 182512)
@@ -203,8 +203,11 @@
bool applyTransforms(GlyphBufferGlyph*, GlyphBufferAdvance*, size_t glyphCount, TypesettingFeatures) const;
+#if PLATFORM(COCOA) || PLATFORM(WIN)
+ bool isSystemFont() const { return m_isSystemFont; }
+#endif
+
#if PLATFORM(WIN)
- bool isSystemFont() const { return m_isSystemFont; }
SCRIPT_FONTPROPERTIES* scriptFontProperties() const;
SCRIPT_CACHE* scriptCache() const { return &m_scriptCache; }
static void setShouldApplyMacAscentHack(bool);
@@ -217,6 +220,8 @@
Font(std::unique_ptr<SVGData>, float fontSize, bool syntheticBold, bool syntheticItalic);
+ Font(const FontPlatformData&, std::unique_ptr<SVGData>&&, bool isCustomFont = false, bool isLoading = false, bool isTextOrientationFallback = false);
+
void platformInit();
void platformGlyphInit();
void platformCharWidthInit();
@@ -248,20 +253,10 @@
mutable std::unique_ptr<GlyphMetricsMap<FloatRect>> m_glyphToBoundsMap;
mutable GlyphMetricsMap<float> m_glyphToWidthMap;
- bool m_treatAsFixedPitch;
- bool m_isCustomFont; // Whether or not we are custom font loaded via @font-face
- bool m_isLoading; // Whether or not this custom font is still in the act of loading.
-
- bool m_isTextOrientationFallback;
- bool m_isBrokenIdeographFallback;
-
- bool m_isUsedInSystemFallbackCache { false };
-
mutable RefPtr<OpenTypeMathData> m_mathData;
#if ENABLE(OPENTYPE_VERTICAL)
RefPtr<OpenTypeVerticalData> m_verticalData;
#endif
- bool m_hasVerticalGlyphs;
Glyph m_spaceGlyph;
float m_spaceWidth;
@@ -304,12 +299,25 @@
#endif
#if PLATFORM(WIN)
- bool m_isSystemFont;
mutable SCRIPT_CACHE m_scriptCache;
mutable SCRIPT_FONTPROPERTIES* m_scriptFontProperties;
#endif
+
+ unsigned m_treatAsFixedPitch : 1;
+ unsigned m_isCustomFont : 1; // Whether or not we are custom font loaded via @font-face
+ unsigned m_isLoading : 1; // Whether or not this custom font is still in the act of loading.
+
+ unsigned m_isTextOrientationFallback : 1;
+ unsigned m_isBrokenIdeographFallback : 1;
+ unsigned m_hasVerticalGlyphs : 1;
+
+ unsigned m_isUsedInSystemFallbackCache : 1;
+
+#if PLATFORM(COCOA) || PLATFORM(WIN)
+ unsigned m_isSystemFont : 1;
+#endif
#if PLATFORM(IOS)
- bool m_shouldNotBeUsedForArabic;
+ unsigned m_shouldNotBeUsedForArabic : 1;
#endif
};
Modified: trunk/Source/WebCore/platform/graphics/FontPlatformData.h (182511 => 182512)
--- trunk/Source/WebCore/platform/graphics/FontPlatformData.h 2015-04-08 00:38:11 UTC (rev 182511)
+++ trunk/Source/WebCore/platform/graphics/FontPlatformData.h 2015-04-08 01:08:15 UTC (rev 182512)
@@ -226,6 +226,7 @@
private:
#if PLATFORM(COCOA)
+ // FIXME: Get rid of one of these. These two fonts are subtly different, and it is not obvious which one to use where.
CTFontRef m_font { nullptr };
mutable RetainPtr<CTFontRef> m_ctFont;
#elif PLATFORM(WIN)
Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm (182511 => 182512)
--- trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm 2015-04-08 00:38:11 UTC (rev 182511)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm 2015-04-08 01:08:15 UTC (rev 182512)
@@ -267,15 +267,16 @@
if (platformData().orientation() == Vertical && !isTextOrientationFallback())
m_hasVerticalGlyphs = fontHasVerticalGlyphs(m_platformData.ctFont());
- if (!m_platformData.m_isEmoji)
- return;
+ if (m_platformData.m_isEmoji) {
+ int thirdOfSize = m_platformData.size() / 3;
+ m_fontMetrics.setAscent(thirdOfSize);
+ m_fontMetrics.setDescent(thirdOfSize);
+ m_fontMetrics.setLineGap(thirdOfSize);
+ m_fontMetrics.setLineSpacing(0);
+ }
+#endif
- int thirdOfSize = m_platformData.size() / 3;
- m_fontMetrics.setAscent(thirdOfSize);
- m_fontMetrics.setDescent(thirdOfSize);
- m_fontMetrics.setLineGap(thirdOfSize);
- m_fontMetrics.setLineSpacing(0);
-#endif
+ m_isSystemFont = CTFontDescriptorIsSystemUIFont(adoptCF(CTFontCopyFontDescriptor(m_platformData.font())).get());
}
void Font::platformCharWidthInit()
@@ -454,9 +455,9 @@
#endif
}
-static bool hasCustomTracking(CTFontRef font)
+static inline bool hasCustomTracking(const Font& font)
{
- return CTFontDescriptorIsSystemUIFont(adoptCF(CTFontCopyFontDescriptor(font)).get());
+ return font.isSystemFont();
}
static inline bool isEmoji(const FontPlatformData& platformData)
@@ -469,10 +470,11 @@
#endif
}
-static inline bool canUseFastGlyphAdvanceGetter(const FontPlatformData& platformData, Glyph glyph, CGSize& advance, bool& populatedAdvance)
+static inline bool canUseFastGlyphAdvanceGetter(const Font& font, Glyph glyph, CGSize& advance, bool& populatedAdvance)
{
+ const FontPlatformData& platformData = font.platformData();
// Fast getter doesn't take custom tracking into account
- if (hasCustomTracking(platformData.ctFont()))
+ if (hasCustomTracking(font))
return false;
// Fast getter doesn't work for emoji
if (isEmoji(platformData))
@@ -490,7 +492,7 @@
CGSize advance = CGSizeZero;
bool horizontal = platformData().orientation() == Horizontal;
bool populatedAdvance = false;
- if ((horizontal || m_isBrokenIdeographFallback) && canUseFastGlyphAdvanceGetter(platformData(), glyph, advance, populatedAdvance)) {
+ if ((horizontal || m_isBrokenIdeographFallback) && canUseFastGlyphAdvanceGetter(*this, glyph, advance, populatedAdvance)) {
float pointSize = platformData().m_size;
CGAffineTransform m = CGAffineTransformMakeScale(pointSize, pointSize);
if (!CGFontGetGlyphAdvancesForStyle(platformData().cgFont(), &m, renderingStyle(platformData()), &glyph, 1, &advance)) {
@@ -498,8 +500,15 @@
LOG_ERROR("Unable to cache glyph widths for %@ %f", fullName.get(), pointSize);
advance.width = 0;
}
- } else if (!populatedAdvance)
- CTFontGetAdvancesForGlyphs(m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, &glyph, &advance, 1);
+ } 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.
+ if (hasCustomTracking(*this))
+ CTFontGetAdvancesForGlyphs(m_platformData.font(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, &glyph, &advance, 1);
+ else
+ CTFontGetAdvancesForGlyphs(m_platformData.ctFont(), horizontal ? kCTFontOrientationHorizontal : kCTFontOrientationVertical, &glyph, &advance, 1);
+ }
return advance.width + m_syntheticBoldOffset;
}
