[Webkit-unassigned] [Bug 24687] Gradient text in canvas not working on chromium/skia.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=24687


senorblanco at chromium.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |senorblanco at chromium.org




------- Comment #3 from senorblanco at chromium.org  2009-03-19 08:33 PDT -------
(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().


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.