- Revision
- 219221
- Author
- [email protected]
- Date
- 2017-07-06 15:09:11 -0700 (Thu, 06 Jul 2017)
Log Message
REGRESSION(r216944): Font loads can cause Chinese characters to draw as .notdef
https://bugs.webkit.org/show_bug.cgi?id=173962
<rdar://problem/32925318>
Reviewed by Simon Fraser.
Source/WebCore:
Previously, there was no signalling between our font loading code
which determined whether or not a font should be invisible (because
its in the middle of loading) and our system fallback code which
created fonts when we fall off the end of the fallback list. Because
of this, we were doing two things wrong:
1. When we started downloading a font, we would try to use a fallback
font. However, if the fallback font didn't suppor the character we're
trying to render, we would just bail and draw .notdef
2. Even if we continued down the fallback list, and fell of the end,
we wouldn't realize that the system fallback font should also be drawn
as invisible.
This patch solves these two problems by:
1. Performing a search to find the best (local) fallback font with
which to fall systemFallbackFontForCharacter(). This way, if you say
"font-family: 'RemoteFont', 'Helvetica'" we will use Helvetica as
the lookup to ask the system to search for.
2. Give the Font class an accessor which can create a duplicate, but
invisible font. Give FontCascadeFonts::glyphDataForVariant() the
correct tracking to know when to use this invisible duplicate.
Tests: fast/text/font-loading-system-fallback.html
http/tests/webfont/font-loading-system-fallback-visibility.html
* platform/graphics/Font.cpp:
(WebCore::Font::invisibleFont):
* platform/graphics/Font.h:
* platform/graphics/FontCascadeFonts.cpp:
(WebCore::findBestFallbackFont):
(WebCore::FontCascadeFonts::glyphDataForSystemFallback):
(WebCore::FontCascadeFonts::glyphDataForVariant):
* platform/graphics/FontCascadeFonts.h:
LayoutTests:
* fast/text/font-loading-system-fallback-expected.html: Added.
* fast/text/font-loading-system-fallback.html: Added.
* http/tests/webfont/font-loading-system-fallback-visibility-expected.html: Added.
* http/tests/webfont/font-loading-system-fallback-visibility.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (219220 => 219221)
--- trunk/LayoutTests/ChangeLog 2017-07-06 21:59:33 UTC (rev 219220)
+++ trunk/LayoutTests/ChangeLog 2017-07-06 22:09:11 UTC (rev 219221)
@@ -1,3 +1,16 @@
+2017-07-06 Myles C. Maxfield <[email protected]>
+
+ REGRESSION(r216944): Font loads can cause Chinese characters to draw as .notdef
+ https://bugs.webkit.org/show_bug.cgi?id=173962
+ <rdar://problem/32925318>
+
+ Reviewed by Simon Fraser.
+
+ * fast/text/font-loading-system-fallback-expected.html: Added.
+ * fast/text/font-loading-system-fallback.html: Added.
+ * http/tests/webfont/font-loading-system-fallback-visibility-expected.html: Added.
+ * http/tests/webfont/font-loading-system-fallback-visibility.html: Added.
+
2017-07-06 Matt Lewis <[email protected]>
Unreviewed, rolling out r219193.
Added: trunk/LayoutTests/fast/text/font-loading-system-fallback-expected.html (0 => 219221)
--- trunk/LayoutTests/fast/text/font-loading-system-fallback-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/font-loading-system-fallback-expected.html 2017-07-06 22:09:11 UTC (rev 219221)
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+This test makes sure that system fallback fonts (occuring when falling off the end of the fallback list) during loading are chosen from the best local font possible. The test passes if the Japanese character below is drawn in a font which matches with Helvetica's design.
+<div style="font: 48px 'Helvetica';">の<div style="display: inline-block; width: 1px; height: 200px;"></div></div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/font-loading-system-fallback.html (0 => 219221)
--- trunk/LayoutTests/fast/text/font-loading-system-fallback.html (rev 0)
+++ trunk/LayoutTests/fast/text/font-loading-system-fallback.html 2017-07-06 22:09:11 UTC (rev 219221)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+ internals.settings.setWebFontsAlwaysFallBack(true);
+ internals.clearMemoryCache();
+ internals.invalidateFontCache();
+}
+</script>
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: url("../../resources/Ahem.ttf") format("truetype");
+}
+</style>
+</head>
+<body>
+This test makes sure that system fallback fonts (occuring when falling off the end of the fallback list) during loading are chosen from the best local font possible. The test passes if the Japanese character below is drawn in a font which matches with Helvetica's design.
+<div style="font: 48px 'WebFont', 'Helvetica';">の<div style="display: inline-block; width: 1px; height: 200px;"></div></div>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/webfont/font-loading-system-fallback-visibility-expected.html (0 => 219221)
--- trunk/LayoutTests/http/tests/webfont/font-loading-system-fallback-visibility-expected.html (rev 0)
+++ trunk/LayoutTests/http/tests/webfont/font-loading-system-fallback-visibility-expected.html 2017-07-06 22:09:11 UTC (rev 219221)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+ internals.clearMemoryCache();
+ internals.invalidateFontCache();
+}
+</script>
+</head>
+<body>
+This test makes sure that system fallback fonts (occuring when falling off the end of the fallback list) during loading are invisible when the loading font is invisible. The test passes if this text is the only text on the page.
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/webfont/font-loading-system-fallback-visibility.html (0 => 219221)
--- trunk/LayoutTests/http/tests/webfont/font-loading-system-fallback-visibility.html (rev 0)
+++ trunk/LayoutTests/http/tests/webfont/font-loading-system-fallback-visibility.html 2017-07-06 22:09:11 UTC (rev 219221)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals) {
+ internals.clearMemoryCache();
+ internals.invalidateFontCache();
+}
+</script>
+<style>
+@font-face {
+ font-family: "WebFont";
+ src: url("../resources/Ahem.woff");
+}
+
+@font-face {
+ font-family: "WebFont2";
+ src: url("slow-ahem-loading.cgi");
+}
+</style>
+</head>
+<body>
+This test makes sure that system fallback fonts (occuring when falling off the end of the fallback list) during loading are invisible when the loading font is invisible. The test passes if this text is the only text on the page.
+<div style="font: 48px 'WebFont', 'WebFont2', 'Helvetica';">の</div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (219220 => 219221)
--- trunk/Source/WebCore/ChangeLog 2017-07-06 21:59:33 UTC (rev 219220)
+++ trunk/Source/WebCore/ChangeLog 2017-07-06 22:09:11 UTC (rev 219221)
@@ -1,3 +1,45 @@
+2017-07-06 Myles C. Maxfield <[email protected]>
+
+ REGRESSION(r216944): Font loads can cause Chinese characters to draw as .notdef
+ https://bugs.webkit.org/show_bug.cgi?id=173962
+ <rdar://problem/32925318>
+
+ Reviewed by Simon Fraser.
+
+ Previously, there was no signalling between our font loading code
+ which determined whether or not a font should be invisible (because
+ its in the middle of loading) and our system fallback code which
+ created fonts when we fall off the end of the fallback list. Because
+ of this, we were doing two things wrong:
+
+ 1. When we started downloading a font, we would try to use a fallback
+ font. However, if the fallback font didn't suppor the character we're
+ trying to render, we would just bail and draw .notdef
+ 2. Even if we continued down the fallback list, and fell of the end,
+ we wouldn't realize that the system fallback font should also be drawn
+ as invisible.
+
+ This patch solves these two problems by:
+ 1. Performing a search to find the best (local) fallback font with
+ which to fall systemFallbackFontForCharacter(). This way, if you say
+ "font-family: 'RemoteFont', 'Helvetica'" we will use Helvetica as
+ the lookup to ask the system to search for.
+ 2. Give the Font class an accessor which can create a duplicate, but
+ invisible font. Give FontCascadeFonts::glyphDataForVariant() the
+ correct tracking to know when to use this invisible duplicate.
+
+ Tests: fast/text/font-loading-system-fallback.html
+ http/tests/webfont/font-loading-system-fallback-visibility.html
+
+ * platform/graphics/Font.cpp:
+ (WebCore::Font::invisibleFont):
+ * platform/graphics/Font.h:
+ * platform/graphics/FontCascadeFonts.cpp:
+ (WebCore::findBestFallbackFont):
+ (WebCore::FontCascadeFonts::glyphDataForSystemFallback):
+ (WebCore::FontCascadeFonts::glyphDataForVariant):
+ * platform/graphics/FontCascadeFonts.h:
+
2017-07-06 Chris Dumez <[email protected]>
FileMonitor should not be ref counted
Modified: trunk/Source/WebCore/platform/graphics/Font.cpp (219220 => 219221)
--- trunk/Source/WebCore/platform/graphics/Font.cpp 2017-07-06 21:59:33 UTC (rev 219220)
+++ trunk/Source/WebCore/platform/graphics/Font.cpp 2017-07-06 22:09:11 UTC (rev 219221)
@@ -261,47 +261,59 @@
return page->glyphDataForCharacter(character);
}
-const Font& Font::verticalRightOrientationFont() const
+auto Font::ensureDerivedFontData() const -> DerivedFonts&
{
if (!m_derivedFontData)
m_derivedFontData = std::make_unique<DerivedFonts>();
- if (!m_derivedFontData->verticalRightOrientation) {
+ return *m_derivedFontData;
+}
+
+const Font& Font::verticalRightOrientationFont() const
+{
+ DerivedFonts& derivedFontData = ensureDerivedFontData();
+ if (!derivedFontData.verticalRightOrientationFont) {
auto verticalRightPlatformData = FontPlatformData::cloneWithOrientation(m_platformData, Horizontal);
- m_derivedFontData->verticalRightOrientation = create(verticalRightPlatformData, origin(), Interstitial::No, Visibility::Visible, OrientationFallback::Yes);
+ derivedFontData.verticalRightOrientationFont = create(verticalRightPlatformData, origin(), Interstitial::No, Visibility::Visible, OrientationFallback::Yes);
}
- ASSERT(m_derivedFontData->verticalRightOrientation != this);
- return *m_derivedFontData->verticalRightOrientation;
+ ASSERT(derivedFontData.verticalRightOrientationFont != this);
+ return *derivedFontData.verticalRightOrientationFont;
}
const Font& Font::uprightOrientationFont() const
{
- if (!m_derivedFontData)
- m_derivedFontData = std::make_unique<DerivedFonts>();
- if (!m_derivedFontData->uprightOrientation)
- m_derivedFontData->uprightOrientation = create(m_platformData, origin(), Interstitial::No, Visibility::Visible, OrientationFallback::Yes);
- ASSERT(m_derivedFontData->uprightOrientation != this);
- return *m_derivedFontData->uprightOrientation;
+ DerivedFonts& derivedFontData = ensureDerivedFontData();
+ if (!derivedFontData.uprightOrientationFont)
+ derivedFontData.uprightOrientationFont = create(m_platformData, origin(), Interstitial::No, Visibility::Visible, OrientationFallback::Yes);
+ ASSERT(derivedFontData.uprightOrientationFont != this);
+ return *derivedFontData.uprightOrientationFont;
}
+const Font& Font::invisibleFont() const
+{
+ DerivedFonts& derivedFontData = ensureDerivedFontData();
+ if (!derivedFontData.invisibleFont)
+ derivedFontData.invisibleFont = create(m_platformData, origin(), Interstitial::Yes, Visibility::Invisible);
+ ASSERT(derivedFontData.invisibleFont != this);
+ return *derivedFontData.invisibleFont;
+}
+
const Font* Font::smallCapsFont(const FontDescription& fontDescription) const
{
- if (!m_derivedFontData)
- m_derivedFontData = std::make_unique<DerivedFonts>();
- if (!m_derivedFontData->smallCaps)
- m_derivedFontData->smallCaps = createScaledFont(fontDescription, smallCapsFontSizeMultiplier);
- ASSERT(m_derivedFontData->smallCaps != this);
- return m_derivedFontData->smallCaps.get();
+ DerivedFonts& derivedFontData = ensureDerivedFontData();
+ if (!derivedFontData.smallCapsFont)
+ derivedFontData.smallCapsFont = createScaledFont(fontDescription, smallCapsFontSizeMultiplier);
+ ASSERT(derivedFontData.smallCapsFont != this);
+ return derivedFontData.smallCapsFont.get();
}
const Font& Font::noSynthesizableFeaturesFont() const
{
#if PLATFORM(COCOA)
- if (!m_derivedFontData)
- m_derivedFontData = std::make_unique<DerivedFonts>();
- if (!m_derivedFontData->noSynthesizableFeatures)
- m_derivedFontData->noSynthesizableFeatures = createFontWithoutSynthesizableFeatures();
- ASSERT(m_derivedFontData->noSynthesizableFeatures != this);
- return *m_derivedFontData->noSynthesizableFeatures;
+ DerivedFonts& derivedFontData = ensureDerivedFontData();
+ if (!derivedFontData.noSynthesizableFeaturesFont)
+ derivedFontData.noSynthesizableFeaturesFont = createFontWithoutSynthesizableFeatures();
+ ASSERT(derivedFontData.noSynthesizableFeaturesFont != this);
+ return *derivedFontData.noSynthesizableFeaturesFont;
#else
return *this;
#endif
@@ -309,24 +321,22 @@
const Font* Font::emphasisMarkFont(const FontDescription& fontDescription) const
{
- if (!m_derivedFontData)
- m_derivedFontData = std::make_unique<DerivedFonts>();
- if (!m_derivedFontData->emphasisMark)
- m_derivedFontData->emphasisMark = createScaledFont(fontDescription, emphasisMarkFontSizeMultiplier);
- ASSERT(m_derivedFontData->emphasisMark != this);
- return m_derivedFontData->emphasisMark.get();
+ DerivedFonts& derivedFontData = ensureDerivedFontData();
+ if (!derivedFontData.emphasisMarkFont)
+ derivedFontData.emphasisMarkFont = createScaledFont(fontDescription, emphasisMarkFontSizeMultiplier);
+ ASSERT(derivedFontData.emphasisMarkFont != this);
+ return derivedFontData.emphasisMarkFont.get();
}
const Font& Font::brokenIdeographFont() const
{
- if (!m_derivedFontData)
- m_derivedFontData = std::make_unique<DerivedFonts>();
- if (!m_derivedFontData->brokenIdeograph) {
- m_derivedFontData->brokenIdeograph = create(m_platformData, origin(), Interstitial::No);
- m_derivedFontData->brokenIdeograph->m_isBrokenIdeographFallback = true;
+ DerivedFonts& derivedFontData = ensureDerivedFontData();
+ if (!derivedFontData.brokenIdeographFont) {
+ derivedFontData.brokenIdeographFont = create(m_platformData, origin(), Interstitial::No);
+ derivedFontData.brokenIdeographFont->m_isBrokenIdeographFallback = true;
}
- ASSERT(m_derivedFontData->brokenIdeograph != this);
- return *m_derivedFontData->brokenIdeograph;
+ ASSERT(derivedFontData.brokenIdeographFont != this);
+ return *derivedFontData.brokenIdeographFont;
}
#ifndef NDEBUG
Modified: trunk/Source/WebCore/platform/graphics/Font.h (219220 => 219221)
--- trunk/Source/WebCore/platform/graphics/Font.h 2017-07-06 21:59:33 UTC (rev 219220)
+++ trunk/Source/WebCore/platform/graphics/Font.h 2017-07-06 22:09:11 UTC (rev 219221)
@@ -134,6 +134,7 @@
const Font& verticalRightOrientationFont() const;
const Font& uprightOrientationFont() const;
+ const Font& invisibleFont() const;
bool hasVerticalGlyphs() const { return m_hasVerticalGlyphs; }
bool isTextOrientationFallback() const { return m_isTextOrientationFallback; }
@@ -237,6 +238,9 @@
RefPtr<Font> platformCreateScaledFont(const FontDescription&, float scaleFactor) const;
void removeFromSystemFallbackCache();
+
+ struct DerivedFonts;
+ DerivedFonts& ensureDerivedFontData() const;
#if PLATFORM(WIN)
void initGDIFont();
@@ -274,12 +278,13 @@
#endif
public:
- RefPtr<Font> smallCaps;
- RefPtr<Font> noSynthesizableFeatures;
- RefPtr<Font> emphasisMark;
- RefPtr<Font> brokenIdeograph;
- RefPtr<Font> verticalRightOrientation;
- RefPtr<Font> uprightOrientation;
+ RefPtr<Font> smallCapsFont;
+ RefPtr<Font> noSynthesizableFeaturesFont;
+ RefPtr<Font> emphasisMarkFont;
+ RefPtr<Font> brokenIdeographFont;
+ RefPtr<Font> verticalRightOrientationFont;
+ RefPtr<Font> uprightOrientationFont;
+ RefPtr<Font> invisibleFont;
};
mutable std::unique_ptr<DerivedFonts> m_derivedFontData;
Modified: trunk/Source/WebCore/platform/graphics/FontCascadeFonts.cpp (219220 => 219221)
--- trunk/Source/WebCore/platform/graphics/FontCascadeFonts.cpp 2017-07-06 21:59:33 UTC (rev 219220)
+++ trunk/Source/WebCore/platform/graphics/FontCascadeFonts.cpp 2017-07-06 22:09:11 UTC (rev 219221)
@@ -323,30 +323,49 @@
return data;
}
-GlyphData FontCascadeFonts::glyphDataForSystemFallback(UChar32 c, const FontCascadeDescription& description, FontVariant variant)
+static const Font* findBestFallbackFont(FontCascadeFonts& fontCascadeFonts, const FontCascadeDescription& description, UChar32 character)
{
- // System fallback is character-dependent.
- auto& primaryRanges = realizeFallbackRangesAt(description, 0);
- auto* originalFont = primaryRanges.fontForCharacter(c);
- if (!originalFont)
- originalFont = &primaryRanges.fontForFirstRange();
+ for (unsigned fallbackIndex = 0; ; ++fallbackIndex) {
+ auto& fontRanges = fontCascadeFonts.realizeFallbackRangesAt(description, fallbackIndex);
+ if (fontRanges.isNull())
+ break;
+ auto* currentFont = fontRanges.glyphDataForCharacter(character, ExternalResourceDownloadPolicy::Forbid).font;
+ if (!currentFont)
+ currentFont = &fontRanges.fontForFirstRange();
- auto systemFallbackFont = originalFont->systemFallbackFontForCharacter(c, description, m_isForPlatformFont);
+ if (!currentFont->isInterstitial())
+ return currentFont;
+ }
+
+ return nullptr;
+}
+
+GlyphData FontCascadeFonts::glyphDataForSystemFallback(UChar32 character, const FontCascadeDescription& description, FontVariant variant, bool systemFallbackShouldBeInvisible)
+{
+ const Font* font = findBestFallbackFont(*this, description, character);
+
+ if (!font)
+ font = &realizeFallbackRangesAt(description, 0).fontForFirstRange();
+
+ auto systemFallbackFont = font->systemFallbackFontForCharacter(character, description, m_isForPlatformFont);
if (!systemFallbackFont)
return GlyphData();
- if (systemFallbackFont->platformData().orientation() == Vertical && !systemFallbackFont->hasVerticalGlyphs() && FontCascade::isCJKIdeographOrSymbol(c))
+ if (systemFallbackShouldBeInvisible)
+ systemFallbackFont = const_cast<Font*>(&systemFallbackFont->invisibleFont());
+
+ if (systemFallbackFont->platformData().orientation() == Vertical && !systemFallbackFont->hasVerticalGlyphs() && FontCascade::isCJKIdeographOrSymbol(character))
variant = BrokenIdeographVariant;
GlyphData fallbackGlyphData;
if (variant == NormalVariant)
- fallbackGlyphData = systemFallbackFont->glyphDataForCharacter(c);
+ fallbackGlyphData = systemFallbackFont->glyphDataForCharacter(character);
else
- fallbackGlyphData = systemFallbackFont->variantFont(description, variant)->glyphDataForCharacter(c);
+ fallbackGlyphData = systemFallbackFont->variantFont(description, variant)->glyphDataForCharacter(character);
if (fallbackGlyphData.font && fallbackGlyphData.font->platformData().orientation() == Vertical && !fallbackGlyphData.font->isTextOrientationFallback()) {
- if (variant == NormalVariant && !FontCascade::isCJKIdeographOrSymbol(c))
- fallbackGlyphData = glyphDataForNonCJKCharacterWithGlyphOrientation(c, description.nonCJKGlyphOrientation(), fallbackGlyphData);
+ if (variant == NormalVariant && !FontCascade::isCJKIdeographOrSymbol(character))
+ fallbackGlyphData = glyphDataForNonCJKCharacterWithGlyphOrientation(character, description.nonCJKGlyphOrientation(), fallbackGlyphData);
}
// Keep the system fallback fonts we use alive.
@@ -356,8 +375,15 @@
return fallbackGlyphData;
}
+enum class SystemFallbackVisibility {
+ Immaterial,
+ Visible,
+ Invisible
+};
+
GlyphData FontCascadeFonts::glyphDataForVariant(UChar32 character, const FontCascadeDescription& description, FontVariant variant, unsigned fallbackIndex)
{
+ SystemFallbackVisibility systemFallbackVisibility = SystemFallbackVisibility::Immaterial;
ExternalResourceDownloadPolicy policy = ExternalResourceDownloadPolicy::Allow;
GlyphData loadingResult;
for (; ; ++fallbackIndex) {
@@ -371,7 +397,9 @@
if (data.font->isInterstitial()) {
policy = ExternalResourceDownloadPolicy::Forbid;
- if (!loadingResult.font)
+ if (systemFallbackVisibility == SystemFallbackVisibility::Immaterial)
+ systemFallbackVisibility = data.font->visibility() == Font::Visibility::Visible ? SystemFallbackVisibility::Visible : SystemFallbackVisibility::Invisible;
+ if (!loadingResult.font && data.glyph)
loadingResult = data;
continue;
}
@@ -399,7 +427,7 @@
if (loadingResult.font)
return loadingResult;
- return glyphDataForSystemFallback(character, description, variant);
+ return glyphDataForSystemFallback(character, description, variant, systemFallbackVisibility == SystemFallbackVisibility::Invisible);
}
static RefPtr<GlyphPage> glyphPageFromFontRanges(unsigned pageNumber, const FontRanges& fontRanges)
Modified: trunk/Source/WebCore/platform/graphics/FontCascadeFonts.h (219220 => 219221)
--- trunk/Source/WebCore/platform/graphics/FontCascadeFonts.h 2017-07-06 21:59:33 UTC (rev 219220)
+++ trunk/Source/WebCore/platform/graphics/FontCascadeFonts.h 2017-07-06 22:09:11 UTC (rev 219221)
@@ -76,7 +76,7 @@
FontCascadeFonts(RefPtr<FontSelector>&&);
FontCascadeFonts(const FontPlatformData&);
- GlyphData glyphDataForSystemFallback(UChar32, const FontCascadeDescription&, FontVariant);
+ GlyphData glyphDataForSystemFallback(UChar32, const FontCascadeDescription&, FontVariant, bool systemFallbackShouldBeInvisible);
GlyphData glyphDataForVariant(UChar32, const FontCascadeDescription&, FontVariant, unsigned fallbackIndex = 0);
Vector<FontRanges, 1> m_realizedFallbackRanges;
