Bug 65035 – [Cairo] Text extent of shadowed text is calculated incorrectly

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Martin Robinson]

This became apparently during conversion of Cairo shadows to using ShadowBlur. The failure is covered by LayoutTests/fast/canvas/fillText-shadow.html.

[Onne Gorter]

I confirm and have a patch:

--- a/Source/WebCore/platform/graphics/cairo/FontCairo.cpp
+++ b/Source/WebCore/platform/graphics/cairo/FontCairo.cpp
@@ -85,7 +85,7 @@ static void drawGlyphsShadow(GraphicsContext* graphicsContext, const FloatPoint&
 
     cairo_text_extents_t extents;
     cairo_scaled_font_glyph_extents(font->platformData().scaledFont(), glyphs, numGlyphs, &extents);
-    FloatRect fontExtentsRect(point.x(), point.y() - extents.height, extents.width, extents.height);
+    FloatRect fontExtentsRect(point.x(), point.y() + extents.y_bearing, extents.width, extents.height);
     if (GraphicsContext* shadowContext = shadow.beginShadowLayer(graphicsContext, fontExtentsRect)) {
         drawGlyphsToContext(shadowContext->platformContext()->cr(), font, glyphs, numGlyphs);
         shadow.endShadowLayer(graphicsContext);

[Onne Gorter]

Created attachment 123923 [details]
use y_bearing instead of height to correctly get rect covering the shadow

[Martin Robinson]

Thanks for your contribution. Do you mind attaching a changelog as explained here? http://www.webkit.org/coding/contributing.html? If you do that I can commit your patch straight from this bug. Please just remove the line about missing tests and replace it with a statement that it's covered by the test I listed above.

[Martin Robinson]

(In reply to comment #1)
> I confirm and have a patch:
> 
> --- a/Source/WebCore/platform/graphics/cairo/FontCairo.cpp
> +++ b/Source/WebCore/platform/graphics/cairo/FontCairo.cpp
> @@ -85,7 +85,7 @@ static void drawGlyphsShadow(GraphicsContext* graphicsContext, const FloatPoint&
> 
>      cairo_text_extents_t extents;
>      cairo_scaled_font_glyph_extents(font->platformData().scaledFont(), glyphs, numGlyphs, &extents);
> -    FloatRect fontExtentsRect(point.x(), point.y() - extents.height, extents.width, extents.height);
> +    FloatRect fontExtentsRect(point.x(), point.y() + extents.y_bearing, extents.width, extents.height);

It seems that we also need to look at the x_bearing here as well. Will make this change and land this patch. Thanks for the contribution!

[Martin Robinson]

Committed r111110: <http://trac.webkit.org/changeset/111110>