Diff
Modified: trunk/LayoutTests/ChangeLog (87522 => 87523)
--- trunk/LayoutTests/ChangeLog 2011-05-27 17:39:35 UTC (rev 87522)
+++ trunk/LayoutTests/ChangeLog 2011-05-27 17:39:53 UTC (rev 87523)
@@ -1,3 +1,17 @@
+2011-05-25 Martin Robinson <mrobin...@igalia.com>
+
+ Reviewed by Dirk Schulze.
+
+ [GTK][Cairo] Twitter rendering breaks
+ https://bugs.webkit.org/show_bug.cgi?id=60917
+
+ Added a GTK+ specific test that verifies that zero pixel sized
+ fonts do not corrupt the Cairo context.
+
+ * platform/gtk/fonts/zero-pixel-sized-fonts-expected.png: Added.
+ * platform/gtk/fonts/zero-pixel-sized-fonts-expected.txt: Added.
+ * platform/gtk/fonts/zero-pixel-sized-fonts.html: Added.
+
2011-05-27 Sheriff Bot <webkit.review....@gmail.com>
Unreviewed, rolling out r87464.
Added: trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts-expected.png
(Binary files differ)
Added: svn:mime-type
Added: trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts-expected.txt (0 => 87523)
--- trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts-expected.txt 2011-05-27 17:39:53 UTC (rev 87523)
@@ -0,0 +1,36 @@
+layer at (0,0) size 800x600
+ RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+ RenderBlock {HTML} at (0,0) size 800x600
+ RenderBody {BODY} at (8,8) size 784x584
+ RenderBlock {DIV} at (0,0) size 784x38
+ RenderText {#text} at (0,0) size 582x19
+ text run at (0,0) width 428: "This test verifies that divs containing zero pixel fonts do not corrupt "
+ text run at (428,0) width 154: "the page rendering (bug "
+ RenderInline {A} at (0,0) size 40x19 [color=#0000EE]
+ RenderText {#text} at (582,0) size 40x19
+ text run at (582,0) width 40: "60917"
+ RenderText {#text} at (622,0) size 742x38
+ text run at (622,0) width 13: "). "
+ text run at (635,0) width 107: "There line below"
+ text run at (0,19) width 254: "should read \"GoldilocksBearBearBear\"."
+ RenderBlock {DIV} at (0,38) size 784x19
+ RenderBlock {DIV} at (0,0) size 69x19
+ RenderText {#text} at (0,0) size 69x19
+ text run at (0,0) width 69: "Goldilocks"
+ RenderText {#text} at (69,15) size 0x0
+ text run at (69,15) width 0: " "
+ RenderBlock {DIV} at (69,0) size 30x19
+ RenderText {#text} at (0,0) size 30x19
+ text run at (0,0) width 30: "Bear"
+ RenderText {#text} at (99,15) size 0x0
+ text run at (99,15) width 0: " "
+ RenderBlock {DIV} at (99,0) size 30x19
+ RenderText {#text} at (0,0) size 30x19
+ text run at (0,0) width 30: "Bear"
+ RenderText {#text} at (129,15) size 0x0
+ text run at (129,15) width 0: " "
+ RenderBlock {DIV} at (129,0) size 30x19
+ RenderText {#text} at (0,0) size 30x19
+ text run at (0,0) width 30: "Bear"
+ RenderText {#text} at (0,0) size 0x0
Property changes on: trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts.html (0 => 87523)
--- trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts.html (rev 0)
+++ trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts.html 2011-05-27 17:39:53 UTC (rev 87523)
@@ -0,0 +1,31 @@
+<html>
+
+<head>
+<style>
+ .buttons {
+ font-size: 0px;
+ }
+ .button {
+ display: inline-block;
+ float: none;
+ font-size: 16px;
+ }
+</style>
+</head>
+
+<body>
+ <div>
+ This test verifies that divs containing zero pixel fonts do not corrupt
+ the page rendering (bug <a href=""
+ There line below should read "GoldilocksBearBearBear".
+ </div>
+
+ <div class="buttons">
+ <div class="button">Goldilocks </div>
+ <div class="button">Bear </div>
+ <div class="button">Bear </div>
+ <div class="button">Bear </div>
+ </div>
+</body>
+
+</html>
Property changes on: trunk/LayoutTests/platform/gtk/fonts/zero-pixel-sized-fonts.html
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (87522 => 87523)
--- trunk/Source/WebCore/ChangeLog 2011-05-27 17:39:35 UTC (rev 87522)
+++ trunk/Source/WebCore/ChangeLog 2011-05-27 17:39:53 UTC (rev 87523)
@@ -1,3 +1,40 @@
+2011-05-25 Martin Robinson <mrobin...@igalia.com>
+
+ Reviewed by Dirk Schulze.
+
+ [GTK][Cairo] Twitter rendering breaks
+ https://bugs.webkit.org/show_bug.cgi?id=60917
+
+ When instantiating a cairo_scaled_font_t font would put the font in an error state,
+ leave the m_scaledFont member of platform data as null. Rendering with scaled fonts
+ in the error state can later lead to corrupted rendering.
+
+ Due to this change, we must always null check cairo_scaled_font_t and the platform
+ data now carries the cairo_font_face_t object with it, so that it can be accessed later
+ if there is no cairo_scaled_font_t.
+
+ Test: platform/gtk/fonts/zero-pixel-sized-fonts.html
+
+ * platform/graphics/cairo/FontCairo.cpp:
+ (WebCore::Font::drawGlyphs): If the scaled font is null, do not render.
+ * platform/graphics/freetype/FontPlatformData.h: Now include the cairo_font_face_t.
+ * platform/graphics/freetype/FontPlatformDataFreeType.cpp:
+ (WebCore::FontPlatformData::FontPlatformData): Initialize the new member.
+ (WebCore::FontPlatformData::operator=): Carry over the new member.
+ (WebCore::FontPlatformData::operator==): Check equality with the new member.
+ (WebCore::FontPlatformData::initializeWithFontFace): If the initialization of m_scaledFont
+ put the font into an error state, then just free it and return.
+ (WebCore::FontPlatformData::hasCompatibleCharmap): The font always has a compatible charmap
+ when the scaled font is null, because rendering is always a no-op and the font data never
+ needs to be read.
+ * platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp:
+ (WebCore::GlyphPage::fill): Don't read font data when the scaled font is null.
+ * platform/graphics/freetype/SimpleFontDataFreeType.cpp:
+ (WebCore::SimpleFontData::platformInit): Return early when there's no scaled font.
+ (WebCore::SimpleFontData::scaledFontData): Use the new m_font member.
+ (WebCore::SimpleFontData::containsCharacters): Return early when there's no scaled font.
+ (WebCore::SimpleFontData::platformWidthForGlyph): Ditto.
+
2011-05-27 David Levin <le...@chromium.org>
Reviewed by Darin Fisher.
Modified: trunk/Source/WebCore/platform/graphics/cairo/FontCairo.cpp (87522 => 87523)
--- trunk/Source/WebCore/platform/graphics/cairo/FontCairo.cpp 2011-05-27 17:39:35 UTC (rev 87522)
+++ trunk/Source/WebCore/platform/graphics/cairo/FontCairo.cpp 2011-05-27 17:39:53 UTC (rev 87523)
@@ -99,8 +99,11 @@
void Font::drawGlyphs(GraphicsContext* context, const SimpleFontData* font, const GlyphBuffer& glyphBuffer,
int from, int numGlyphs, const FloatPoint& point) const
{
- GlyphBufferGlyph* glyphs = (GlyphBufferGlyph*)glyphBuffer.glyphs(from);
+ if (!font->platformData().scaledFont())
+ return;
+ GlyphBufferGlyph* glyphs = const_cast<GlyphBufferGlyph*>(glyphBuffer.glyphs(from));
+
float offset = 0.0f;
for (int i = 0; i < numGlyphs; i++) {
glyphs[i].x = offset;
Modified: trunk/Source/WebCore/platform/graphics/freetype/FontPlatformData.h (87522 => 87523)
--- trunk/Source/WebCore/platform/graphics/freetype/FontPlatformData.h 2011-05-27 17:39:35 UTC (rev 87522)
+++ trunk/Source/WebCore/platform/graphics/freetype/FontPlatformData.h 2011-05-27 17:39:53 UTC (rev 87523)
@@ -97,6 +97,7 @@
bool m_syntheticBold;
bool m_syntheticOblique;
bool m_fixedWidth;
+ RefPtr<cairo_font_face_t> m_font;
cairo_scaled_font_t* m_scaledFont;
private:
Modified: trunk/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp (87522 => 87523)
--- trunk/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp 2011-05-27 17:39:35 UTC (rev 87522)
+++ trunk/Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp 2011-05-27 17:39:53 UTC (rev 87523)
@@ -121,8 +121,8 @@
, m_fixedWidth(false)
, m_scaledFont(0)
{
- RefPtr<cairo_font_face_t> fontFace = adoptRef(cairo_ft_font_face_create_for_pattern(m_pattern.get()));
- initializeWithFontFace(fontFace.get());
+ m_font = adoptRef(cairo_ft_font_face_create_for_pattern(m_pattern.get()));
+ initializeWithFontFace(m_font.get());
int spacing;
if (FcPatternGetInteger(pattern, FC_SPACING, 0, &spacing) == FcResultMatch && spacing == FC_MONO)
@@ -152,6 +152,7 @@
, m_size(size)
, m_syntheticBold(bold)
, m_syntheticOblique(italic)
+ , m_font(fontFace)
, m_scaledFont(0)
{
initializeWithFontFace(fontFace);
@@ -184,6 +185,7 @@
if (m_scaledFont && m_scaledFont != hashTableDeletedFontValue())
cairo_scaled_font_destroy(m_scaledFont);
m_scaledFont = cairo_scaled_font_reference(other.m_scaledFont);
+ m_font = other.m_font;
return *this;
}
@@ -228,7 +230,7 @@
if (!m_pattern || !other.m_pattern)
return false;
return FcPatternEqual(m_pattern.get(), other.m_pattern.get())
- && m_scaledFont == other.m_scaledFont && m_size == other.m_size
+ && m_scaledFont == other.m_scaledFont && m_font == other.m_font && m_size == other.m_size
&& m_syntheticOblique == other.m_syntheticOblique && m_syntheticBold == other.m_syntheticBold;
}
@@ -241,6 +243,12 @@
void FontPlatformData::initializeWithFontFace(cairo_font_face_t* fontFace)
{
+ // Fonts with zero size lead to failed cairo_scaled_font_t instantiations. Instead
+ // we just do not instantiate the scaled font at all. This will cause all renders
+ // to be no-ops and all metrics to be zero, which is the desired behavior anyway.
+ if (!m_size)
+ return;
+
cairo_font_options_t* options = getDefaultFontOptions();
cairo_matrix_t ctm;
@@ -273,8 +281,10 @@
bool FontPlatformData::hasCompatibleCharmap()
{
+ // If m_scaledFont is null, it means that this is a size zero font, which always
+ // has a compatible charmap since we never really read any font data from the font.
if (!m_scaledFont)
- return false;
+ return true;
FT_Face freeTypeFace = cairo_ft_scaled_font_lock_face(m_scaledFont);
bool hasCompatibleCharmap = !(FT_Select_Charmap(freeTypeFace, ft_encoding_unicode)
Modified: trunk/Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp (87522 => 87523)
--- trunk/Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp 2011-05-27 17:39:35 UTC (rev 87522)
+++ trunk/Source/WebCore/platform/graphics/freetype/GlyphPageTreeNodeFreeType.cpp 2011-05-27 17:39:53 UTC (rev 87523)
@@ -45,7 +45,11 @@
if (bufferLength > GlyphPage::size)
return false;
- FT_Face face = cairo_ft_scaled_font_lock_face(fontData->platformData().scaledFont());
+ cairo_scaled_font_t* scaledFont = fontData->platformData().scaledFont();
+ if (!scaledFont)
+ return false;
+
+ FT_Face face = cairo_ft_scaled_font_lock_face(scaledFont);
if (!face)
return false;
@@ -60,8 +64,7 @@
}
}
- cairo_ft_scaled_font_unlock_face(fontData->platformData().scaledFont());
-
+ cairo_ft_scaled_font_unlock_face(scaledFont);
return haveGlyphs;
}
Modified: trunk/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp (87522 => 87523)
--- trunk/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp 2011-05-27 17:39:35 UTC (rev 87522)
+++ trunk/Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp 2011-05-27 17:39:53 UTC (rev 87523)
@@ -47,6 +47,9 @@
void SimpleFontData::platformInit()
{
+ if (!m_platformData.scaledFont())
+ return;
+
cairo_font_extents_t font_extents;
cairo_text_extents_t text_extents;
cairo_scaled_font_extents(m_platformData.scaledFont(), &font_extents);
@@ -88,9 +91,11 @@
PassOwnPtr<SimpleFontData> SimpleFontData::createScaledFontData(const FontDescription& fontDescription, float scaleFactor) const
{
- return adoptPtr(new SimpleFontData(FontPlatformData(cairo_scaled_font_get_font_face(m_platformData.scaledFont()),
- scaleFactor * fontDescription.computedSize(), m_platformData.syntheticBold(), m_platformData.syntheticOblique()),
- isCustomFont(), false));
+ return adoptPtr(new SimpleFontData(FontPlatformData(m_platformData.m_font.get(),
+ scaleFactor * fontDescription.computedSize(),
+ m_platformData.syntheticBold(),
+ m_platformData.syntheticOblique()),
+ isCustomFont(), false));
}
SimpleFontData* SimpleFontData::smallCapsFontData(const FontDescription& fontDescription) const
@@ -116,8 +121,12 @@
bool SimpleFontData::containsCharacters(const UChar* characters, int length) const
{
+ // If this is a no-op font (zero size), then it should always contain the characters,
+ // since we never read from or render from this font.
+ if (!m_platformData.scaledFont())
+ return true;
+
FT_Face face = cairo_ft_scaled_font_lock_face(m_platformData.scaledFont());
-
if (!face)
return false;
@@ -129,7 +138,6 @@
}
cairo_ft_scaled_font_unlock_face(m_platformData.scaledFont());
-
return true;
}
@@ -145,7 +153,8 @@
float SimpleFontData::platformWidthForGlyph(Glyph glyph) const
{
- ASSERT(m_platformData.scaledFont());
+ if (!m_platformData.scaledFont())
+ return 0;
cairo_glyph_t cglyph = { glyph, 0, 0 };
cairo_text_extents_t extents;
