SVG feDropShadow implementation of SVG Filters 1.2

• Component: SVG
• Status: RESOLVED FIXED

[Dirk Schulze]

SVG feDropShadow implementation of SVG Filters 1.2.

[Dirk Schulze]

Created attachment 79074 [details]
Patch

[Dirk Schulze]

I added this patch for discussion.

[Early Warning System Bot]

Attachment 79074 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7584083

[Dirk Schulze]

Created attachment 79078 [details]
Patch

[Dirk Schulze]

Fix for Gtk and Qt. Patch applies nicely to Mac, the bots should get updated more regularly.

[WebKit Review Bot]

Attachment 79078 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/7569176

[Eric Seidel (no email)]

SVG 1.2 scares me. :)

[Dirk Schulze]

(In reply to comment #7)
> SVG 1.2 scares me. :)

Actually it's SVG Filter 1.2, and will be a submodule of SVG 2 ;-)

[Dirk Schulze]

Comment on attachment 79078 [details]
Patch

Clearing review flag after talking with Simon. ContextShadow should be land in a different patch.

[Dirk Schulze]

Created attachment 90363 [details]
Patch

[Dirk Schulze]

Comment on attachment 90363 [details]
Patch

Clearing flags on attachment: 90363

Committed r84410: <http://trac.webkit.org/changeset/84410>

[Dirk Schulze]

All reviewed patches have been landed.  Closing bug.

[Dirk Schulze]

Landed preparation patch. Going on with feDropShadow now.

[Dirk Schulze]

Created attachment 90515 [details]
Patch

[Dirk Schulze]

(In reply to comment #14)
> Created an attachment (id=90515) [details]
> Patch

The patch itself is not that big. But I attached the images of the pixel tests. Don't be afraid.

[Nikolas Zimmermann]

Comment on attachment 90515 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90515&action=review

Patch looks great, one problem though leading to r-:

> Source/WebCore/svg/SVGFEDropShadowElement.cpp:130
> +    RefPtr<RenderStyle> filterStyle = styleForRenderer();

styleForRenderer()? SVGFEDropShadowElement has it's own renderer through SVGFilterPrimitiveStandardAttributes. You have to ask your renderer for the style, don't do manual style resolution!
(I guess you copied that from another wrong SVGFE* Element?)

[Dirk Schulze]

Created attachment 90524 [details]
Patch

[Dirk Schulze]

(In reply to comment #16)
> (I guess you copied that from another wrong SVGFE* Element?)

It has historical reasons why other SVGFE*Elements don't call the renderer. They didn't have renderers at the beginning. I'll fix the other elements in another bug report.

[Nikolas Zimmermann]

Comment on attachment 90524 [details]
Patch

Looks great, r=me. Note ASSERT(renderer->style()->svgStyle()); this is always true, as the svgStyle gets created.
Make sure it builds before landing though, as discussed on IRC :-)

[Dirk Schulze]

Committed r84522: <http://trac.webkit.org/changeset/84522>

[WebKit Review Bot]

http://trac.webkit.org/changeset/84522 might have broken Qt Linux ARMv7 Release, Qt Windows 32-bit Release, and Qt Windows 32-bit Debug

[Dirk Schulze]

Committed r84530: <http://trac.webkit.org/changeset/84530>

[Dirk Schulze]

Committed r84537: <http://trac.webkit.org/changeset/84537>

[Dirk Schulze]

Committed r84541: <http://trac.webkit.org/changeset/84541>

[Simon Fraser (smfr)]

The new ShadowBlur code fails to handle vertical or horizontal radii of 0. I guess this wasn't tested? Bug 59710.