Bug 30017 - [CHROMIUM] Implement two point radial gradients in Chromium/Skia
Summary: [CHROMIUM] Implement two point radial gradients in Chromium/Skia
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Stephen White
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-02 07:27 PDT by Stephen White
Modified: 2010-07-04 11:13 PDT (History)
1 user (show)

See Also:


Attachments
Enable the new gradients (3.11 KB, patch)
2009-10-02 07:57 PDT, Stephen White
dglazkov: review+
dglazkov: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen White 2009-10-02 07:27:51 PDT
Two point radial gradients (as used by SVG, Canvas and -webkit-gradient) need to be enabled in Chromium/Skia.

See http://crbug.com/8696.
Comment 1 Stephen White 2009-10-02 07:57:56 PDT
Created attachment 40514 [details]
Enable the new gradients
Comment 2 Dimitri Glazkov (Google) 2009-10-02 09:00:32 PDT
Comment on attachment 40514 [details]
Enable the new gradients

r=me, except for the two-space indents. Can you fix before landing?
Comment 3 Stephen White 2009-10-02 10:13:20 PDT
(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.
Comment 4 Eric Seidel (no email) 2009-10-02 16:06:29 PDT
Done?  Did this land?
Comment 5 Stephen White 2009-10-08 13:31:50 PDT
Yes, I landed this.  Sorry, forgot to close.
Comment 6 Stephen White 2009-10-08 13:33:45 PDT
(In reply to comment #5)
> Yes, I landed this.  Sorry, forgot to close.

Landed as http://trac.webkit.org/changeset/49025.
Comment 7 Nico Weber 2010-07-04 11:13:59 PDT
Comment on attachment 40514 [details]
Enable the new gradients

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 49021)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,21 @@
> +2009-10-02  Stephen White  <senorblanco@chromium.org>
> +
> +        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  <pnormand@igalia.com>
>  
>          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.)