30017 2009-10-02 07:27:51 -0700 [CHROMIUM] Implement two point radial gradients in Chromium/Skia 2010-07-04 11:13:59 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 senorblanco senorblanco eric oldest_to_newest 151838 0 senorblanco 2009-10-02 07:27:51 -0700 Two point radial gradients (as used by SVG, Canvas and -webkit-gradient) need to be enabled in Chromium/Skia. See http://crbug.com/8696. 151842 1 40514 senorblanco 2009-10-02 07:57:56 -0700 Created attachment 40514 Enable the new gradients 151858 2 40514 dglazkov 2009-10-02 09:00:32 -0700 Comment on attachment 40514 Enable the new gradients r=me, except for the two-space indents. Can you fix before landing? 151882 3 senorblanco 2009-10-02 10:13:20 -0700 (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. 152013 4 eric 2009-10-02 16:06:29 -0700 Done? Did this land? 153509 5 senorblanco 2009-10-08 13:31:50 -0700 Yes, I landed this. Sorry, forgot to close. 153510 6 senorblanco 2009-10-08 13:33:45 -0700 (In reply to comment #5) > Yes, I landed this. Sorry, forgot to close. Landed as http://trac.webkit.org/changeset/49025. 246431 7 40514 thakis 2010-07-04 11:13:59 -0700 Comment on attachment 40514 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.) 40514 2009-10-02 07:57:56 -0700 2010-07-04 11:13:59 -0700 Enable the new gradients two_point_radial_webkit.patch text/plain 3182 senorblanco SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0OTAyMSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjEgQEAKKzIwMDktMTAtMDIgIFN0ZXBoZW4gV2hpdGUgIDxzZW5vcmJsYW5jb0Bj aHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgRGltaXRyaSBHbGF6a292LgorCisg ICAgICAgIEVuYWJsZSB0d28gcG9pbnQgcmFkaWFsIGdyYWRpZW50cyBpbiBDaHJvbWl1bS9Ta2lh LgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0zMDAx NworCisgICAgICAgIENvdmVyZWQgYnkgdGhlIGZvbGxvd2luZyB0ZXN0czoKKworICAgICAgICBM YXlvdXRUZXN0cy9zdmcvVzNDLVNWRy0xLjEvcHNlcnZlcnMtZ3JhZC0xMy1iLnN2ZworICAgICAg ICBMYXlvdXRUZXN0cy9mYXN0L2JhY2tncm91bmRzL3N2Zy1hcy1iYWNrZ3JvdW5kLTMuaHRtbAor ICAgICAgICBMYXlvdXRUZXN0cy9mYXN0L2dyYWRpZW50cy9nZW5lcmF0ZWQtZ3JhZGllbnRzLmh0 bWwKKyAgICAgICAgTGF5b3V0VGVzdHMvZmFzdC9ncmFkaWVudHMvc2ltcGxlLWdyYWRpZW50cy5o dG1sCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9za2lhL0dyYWRpZW50U2tpYS5jcHA6 CisgICAgICAgIChXZWJDb3JlOjpHcmFkaWVudDo6cGxhdGZvcm1HcmFkaWVudCk6CisKIDIwMDkt MTAtMDIgIFBoaWxpcHBlIE5vcm1hbmQgIDxwbm9ybWFuZEBpZ2FsaWEuY29tPgogCiAgICAgICAg IFJldmlld2VkIGJ5IEd1c3Rhdm8gTm9yb25oYS4KSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3Mvc2tpYS9HcmFkaWVudFNraWEuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvcGxhdGZv cm0vZ3JhcGhpY3Mvc2tpYS9HcmFkaWVudFNraWEuY3BwCShyZXZpc2lvbiA0OTAyMSkKKysrIFdl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mvc2tpYS9HcmFkaWVudFNraWEuY3BwCSh3b3JraW5nIGNv cHkpCkBAIC0xNTIsMTkgKzE1MiwyMSBAQCBTa1NoYWRlciogR3JhZGllbnQ6OnBsYXRmb3JtR3Jh ZGllbnQoKQogICAgIH0KIAogICAgIGlmIChtX3JhZGlhbCkgewotICAgICAgICAvLyBGSVhNRTog Q1NTIHJhZGlhbCBHcmFkaWVudHMgYWxsb3cgYW4gb2Zmc2V0IGZvY2FsIHBvaW50ICh0aGUKLSAg ICAgICAgLy8gInN0YXJ0IGNpcmNsZSIpLCBidXQgc2tpYSBkb2Vzbid0IHNlZW0gdG8gc3VwcG9y dCB0aGF0LCBzbyB0aGlzIGp1c3QKLSAgICAgICAgLy8gaWdub3JlcyBtX3AwL21fcjAgYW5kIGRy YXdzIHRoZSBncmFkaWVudCBjZW50ZXJlZCBpbiB0aGUgImVuZAotICAgICAgICAvLyBjaXJjbGUi IChtX3AxL21fcjEpLgotICAgICAgICAvLyBTZWUgaHR0cDovL3dlYmtpdC5vcmcvYmxvZy8xNzUv aW50cm9kdWNpbmctY3NzLWdyYWRpZW50cy8gZm9yIGEKLSAgICAgICAgLy8gZGVzY3JpcHRpb24g b2YgdGhlIGV4cGVjdGVkIGJlaGF2aW9yLgotICAgICAgICAKLSAgICAgICAgLy8gVGhlIHJhZGl1 cyB3ZSBnaXZlIHRvIFNraWEgbXVzdCBiZSBwb3NpdGl2ZSAoYW5kIG5vbi16ZXJvKS4gIElmCi0g ICAgICAgIC8vIHdlJ3JlIGdpdmVuIGEgemVybyByYWRpdXMsIGp1c3QgYXNrIGZvciBhIHZlcnkg c21hbGwgcmFkaXVzIHNvCi0gICAgICAgIC8vIFNraWEgd2lsbCBzdGlsbCByZXR1cm4gYW4gb2Jq ZWN0LgotICAgICAgICBTa1NjYWxhciByYWRpdXMgPSBtX3IxID4gMCA/IFdlYkNvcmVGbG9hdFRv U2tTY2FsYXIobV9yMSkgOiBTS19TY2FsYXJNaW47Ci0gICAgICAgIG1fZ3JhZGllbnQgPSBTa0dy YWRpZW50U2hhZGVyOjpDcmVhdGVSYWRpYWwobV9wMSwKLSAgICAgICAgICAgIHJhZGl1cywgY29s b3JzLCBwb3MsIHN0YXRpY19jYXN0PGludD4oY291bnRVc2VkKSwgdGlsZSk7CisgICAgICAgIC8v IFNpbmNlIHRoZSB0d28tcG9pbnQgcmFkaWFsIGdyYWRpZW50IGlzIHNsb3dlciB0aGFuIHRoZSBw bGFpbiByYWRpYWwsCisgICAgICAgIC8vIG9ubHkgdXNlIGl0IGlmIHdlIGhhdmUgdG8uCisgICAg ICAgIGlmIChtX3AwICE9IG1fcDEpIHsKKyAgICAgICAgICAvLyBUaGUgcmFkaWkgd2UgZ2l2ZSB0 byBTa2lhIG11c3QgYmUgcG9zaXRpdmUuICBJZiB3ZSdyZSBnaXZlbiBhIAorICAgICAgICAgIC8v IG5lZ2F0aXZlIHJhZGl1cywgYXNrIGZvciB6ZXJvIGluc3RlYWQuCisgICAgICAgICAgU2tTY2Fs YXIgcmFkaXVzMCA9IG1fcjAgPj0gMC4wZiA/IFdlYkNvcmVGbG9hdFRvU2tTY2FsYXIobV9yMCkg OiAwOworICAgICAgICAgIFNrU2NhbGFyIHJhZGl1czEgPSBtX3IxID49IDAuMGYgPyBXZWJDb3Jl RmxvYXRUb1NrU2NhbGFyKG1fcjEpIDogMDsKKyAgICAgICAgICBtX2dyYWRpZW50ID0gU2tHcmFk aWVudFNoYWRlcjo6Q3JlYXRlVHdvUG9pbnRSYWRpYWwobV9wMCwgcmFkaXVzMCwgbV9wMSwgcmFk aXVzMSwgY29sb3JzLCBwb3MsIHN0YXRpY19jYXN0PGludD4oY291bnRVc2VkKSwgdGlsZSk7Cisg ICAgICAgIH0gZWxzZSB7CisgICAgICAgICAgLy8gVGhlIHJhZGl1cyB3ZSBnaXZlIHRvIFNraWEg bXVzdCBiZSBwb3NpdGl2ZSAoYW5kIG5vbi16ZXJvKS4gIElmCisgICAgICAgICAgLy8gd2UncmUg Z2l2ZW4gYSB6ZXJvIHJhZGl1cywganVzdCBhc2sgZm9yIGEgdmVyeSBzbWFsbCByYWRpdXMgc28K KyAgICAgICAgICAvLyBTa2lhIHdpbGwgc3RpbGwgcmV0dXJuIGFuIG9iamVjdC4KKyAgICAgICAg ICBTa1NjYWxhciByYWRpdXMgPSBtX3IxID4gMCA/IFdlYkNvcmVGbG9hdFRvU2tTY2FsYXIobV9y MSkgOiBTS19TY2FsYXJNaW47CisgICAgICAgICAgbV9ncmFkaWVudCA9IFNrR3JhZGllbnRTaGFk ZXI6OkNyZWF0ZVJhZGlhbChtX3AxLCByYWRpdXMsIGNvbG9ycywgcG9zLCBzdGF0aWNfY2FzdDxp bnQ+KGNvdW50VXNlZCksIHRpbGUpOworICAgICAgICB9CiAgICAgfSBlbHNlIHsKICAgICAgICAg U2tQb2ludCBwdHNbMl0gPSB7IG1fcDAsIG1fcDEgfTsKICAgICAgICAgbV9ncmFkaWVudCA9IFNr R3JhZGllbnRTaGFkZXI6OkNyZWF0ZUxpbmVhcihwdHMsIGNvbG9ycywgcG9zLAo=