Bug 30017 – [CHROMIUM] Implement two point radial gradients in Chromium/Skia

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

[Stephen White]

Two point radial gradients (as used by SVG, Canvas and -webkit-gradient) need to be enabled in Chromium/Skia.

See http://crbug.com/8696.

[Stephen White]

Created attachment 40514 [details]
Enable the new gradients

[Dimitri Glazkov (Google)]

Comment on attachment 40514 [details]
Enable the new gradients

r=me, except for the two-space indents. Can you fix before landing?

[Stephen White]

(In reply to comment #2)
> (From update of attachment 40514 [details])
> r=me, except for the two-space indents. Can you fix before landing?

Whoops, done.

[Eric Seidel (no email)]

Done?  Did this land?

[Stephen White]

Yes, I landed this.  Sorry, forgot to close.

[Stephen White]

(In reply to comment #5)
> Yes, I landed this.  Sorry, forgot to close.

Landed as http://trac.webkit.org/changeset/49025.

[Nico Weber]

Comment on attachment 40514 [details]
Enable the new gradients

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 49021)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,21 @@
> [phone]  Stephen White  <[e-mail]>
> +
> +        Reviewed by Dimitri Glazkov.
> +
> +        Enable two point radial gradients in Chromium/Skia.
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=30017
> +
> +        Covered by the following tests:
> +
> +        LayoutTests/svg/W3C-SVG-1.1/pservers-grad-13-b.svg
> +        LayoutTests/fast/backgrounds/svg-as-background-3.html
> +        LayoutTests/fast/gradients/generated-gradients.html
> +        LayoutTests/fast/gradients/simple-gradients.html
> +
> +        * platform/graphics/skia/GradientSkia.cpp:
> +        (WebCore::Gradient::platformGradient):
> +
>  2009-10-02  Philippe Normand  <[e-mail]>
>  
>          Reviewed by Gustavo Noronha.
> Index: WebCore/platform/graphics/skia/GradientSkia.cpp
> ===================================================================
> --- WebCore/platform/graphics/skia/GradientSkia.cpp	(revision 49021)
> +++ WebCore/platform/graphics/skia/GradientSkia.cpp	(working copy)
> @@ -152,19 +152,21 @@ SkShader* Gradient::platformGradient()
>      }
>  
>      if (m_radial) {
> -        // FIXME: CSS radial Gradients allow an offset focal point (the
> -        // "start circle"), but skia doesn't seem to support that, so this just
> -        // ignores m_p0/m_r0 and draws the gradient centered in the "end
> -        // circle" (m_p1/m_r1).
> -        // See http://webkit.org/blog/175/introducing-css-gradients/ for a
> -        // description of the expected behavior.
> -        
> -        // The radius we give to Skia must be positive (and non-zero).  If
> -        // we're given a zero radius, just ask for a very small radius so
> -        // Skia will still return an object.
> -        SkScalar radius = m_r1 > 0 ? WebCoreFloatToSkScalar(m_r1) : SK_ScalarMin;
> -        m_gradient = SkGradientShader::CreateRadial(m_p1,
> -            radius, colors, pos, static_cast<int>(countUsed), tile);
> +        // Since the two-point radial gradient is slower than the plain radial,
> +        // only use it if we have to.
> +        if (m_p0 != m_p1) {
> +          // The radii we give to Skia must be positive.  If we're given a 
> +          // negative radius, ask for zero instead.
> +          SkScalar radius0 = m_r0 >= 0.0f ? WebCoreFloatToSkScalar(m_r0) : 0;
> +          SkScalar radius1 = m_r1 >= 0.0f ? WebCoreFloatToSkScalar(m_r1) : 0;
> +          m_gradient = SkGradientShader::CreateTwoPointRadial(m_p0, radius0, m_p1, radius1, colors, pos, static_cast<int>(countUsed), tile);
> +        } else {
> +          // The radius we give to Skia must be positive (and non-zero).  If
> +          // we're given a zero radius, just ask for a very small radius so
> +          // Skia will still return an object.
> +          SkScalar radius = m_r1 > 0 ? WebCoreFloatToSkScalar(m_r1) : SK_ScalarMin;
> +          m_gradient = SkGradientShader::CreateRadial(m_p1, radius, colors, pos, static_cast<int>(countUsed), tile);
> +        }
>      } else {
>          SkPoint pts[2] = { m_p0, m_p1 };
>          m_gradient = SkGradientShader::CreateLinear(pts, colors, pos,

WebCore/platform/graphics/skia/GradientSkia.cpp:157
 +          if (m_p0 != m_p1) {
This is wrong in the following two cases:
-webkit-gradient(radial, 50 50, 40, 50 50, 80, from(red), to(black))
-webkit-gradient(radial, 50 50, 40, 50 50, 0, from(red), to(black))

I posted https://bugs.webkit.org/show_bug.cgi?id=41580 to fix this.

(Also, it feels like this kind of optimization should be in skia's SkGradientShader::CreateTwoPointRadial() and not here.)