Gradient text in canvas is not working on chromium/skia: the gradient is there, but is not scaled correctly (is solid colour, should be gradient). See LayoutTest/fast/canvas/canvas-text-alignment.html.
Created attachment 28739 [details] Fix for canvas text gradient problem.
> +#if PLATFORM(CG) > if (boundingBoxMode() && !isPaintingText) { > +#else > + if (boundingBoxMode()) { > +#endif It's not just CG, that uses this code. Please just use #if !PLATFORM(SKIA) if (boundingBoxMode() && !isPaintingText) { #else if (boundingBoxMode()) { #endif if it is necessary. I think you will break some tests of batik. Have you checked this? And the special skia code, that we have in SVGPaintServer, is just needed for fillAndStrokePath. (btw. this can be fixed by closing the path after filling or stroking inf gc:fillPath() and gc::strokePath()). I'm still not sure if I get the problem. Gradients don't work for Skia in Canvas, right? And you fix SVG, because the transformation matrix is applied twice for Canvas. Wehre (beside SkiaFontWin.cpp) and why? To add more platform code to SVG is maybe not the best way and it can break some tests. Can we fix in internally?
(In reply to comment #2) > > +#if PLATFORM(CG) > > if (boundingBoxMode() && !isPaintingText) { > > +#else > > + if (boundingBoxMode()) { > > +#endif > > It's not just CG, that uses this code. Please just use > > #if !PLATFORM(SKIA) > if (boundingBoxMode() && !isPaintingText) { > #else > if (boundingBoxMode()) { > #endif > > if it is necessary. I think you will break some tests of batik. Have you > checked this? There were no regressions in the batik tests on Chromium on Win32 or Webkit on Mac. I'm pretty sure that those other platforms will need this matrix in order to do gradient text correctly. It's only CG that won't need it, since it uses the gradient later in teardown() to fill a rect. I can change it to be skia-only, if you still want, but I'm pretty sure you're going to need this matrix for Cairo as well (I'm assuming that's what you're worried about). > And the special skia code, that we have in SVGPaintServer, is just needed for > fillAndStrokePath. (btw. this can be fixed by closing the path after filling or > stroking inf gc:fillPath() and gc::strokePath()). You're saying that we could remove the skia-specific code in SVGPaintServer? That sounds good, but is orthogonal to this patch. > I'm still not sure if I get the problem. Gradients don't work for Skia in > Canvas, right? And you fix SVG, because the transformation matrix is applied > twice for Canvas. Wehre (beside SkiaFontWin.cpp) and why? Basically, the correct way to do gradient text is to scale up the gradient to the appropriate size (the bounding box, in this case) before rendering the text. Canvas was already doing that (m_p0 was 0,0 and m_p1 was 600,600 in this test), so when I added code in a previous CL to scale up the gradient in SkiaFontWin, it was getting scaled twice. So I removed my previous code to scale the gradient (in SkiaFontWin), and allowed the SVG path to set the matrix from the bounding box instead. > To add more platform code to SVG is maybe not the best way and it can break > some tests. Can we fix in internally? I don't think there's any other way to fix it, since the lower-level rendering code needs that bounding box in order to correctly scale the gradient. And the lower level doesn't know if it has been already set (as in the canvas case) or unset (as in the SVG case). The only reason CG doesn't need the matrix is that it doesn't use the gradient during text rendering; it applies it in teardown().
I got it. Yes the current code in SVGPaintServerGradient does only work for CG correctly. Your patch is ok. :-). eseidel? r+?
Comment on attachment 28739 [details] Fix for canvas text gradient problem. Looks fine.
I expect this will break qt/gtk pixel results (if those exist), but I have no easy way to update those, and from what I can tell from Dirk and Stephen this will be a progression for both sets of results. Also, Stephen, be sure to add the bug URL to the ChangeLog in the future. ;)
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/platform/graphics/skia/SkiaFontWin.cpp M WebCore/svg/graphics/SVGPaintServerGradient.cpp Committed r41860 Thanks again for the patch!