clipPath does not work on foreignObject

[Rob Crowther]

Created attachment 107670 [details]
A foreignObject containing a 300px square div is not clipped

No clipping occurs when applying a clipPath to a foreignObject.

[Rob Crowther]

Created attachment 107671 [details]
The same clipPath applied to an svg rect works correctly

[Florin Malita]

The foreignObject element currently doesn't support clip-path. I have a patch but it relies on the FO atomic paint fix to pass the tests (https://bugs.webkit.org/show_bug.cgi?id=58417).

[Florin Malita]

Created attachment 114155 [details]
Patch

[WebKit Review Bot]

Comment on attachment 114155 [details]
Patch

Attachment 114155 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10369241

New failing tests:
svg/foreignObject/clip-path.html

[Dirk Schulze]

Comment on attachment 114155 [details]
Patch

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

It looks good for me in  general, I r- it because not only clip-path is effected. I'd like to see masked and filtered FO examples as well. And one example for opacity.

> Source/WebCore/ChangeLog:6
> +        clipPath does not work on foreignObject
> +        https://bugs.webkit.org/show_bug.cgi?id=68251
> +
> +        Reviewed by NOBODY (OOPS!).

The change log needs to be more details. What have you done and why? This should be included in the change log.

> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:62
>      if (opacity < 1.0f)
>          childPaintInfo.context->beginTransparencyLayer(opacity);

I think we do the opacity in SVGRenderSupport as well. Can you remove the calls here and after your code please? Also, can you add a test to verify the behavior please?

> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:75
>      if (opacity < 1.0f)
>          childPaintInfo.context->endTransparencyLayer();

(needs to be removed as well)

[Florin Malita]

Created attachment 114593 [details]
Patch

[Florin Malita]

Comment on attachment 114155 [details]
Patch

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

Thanks for the review Dirk. I've added tests to exercise clipping, masking, filtering and opacity on FOs as suggested.

>> Source/WebCore/ChangeLog:6
>> +        Reviewed by NOBODY (OOPS!).
> 
> The change log needs to be more details. What have you done and why? This should be included in the change log.

Done.

>> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:62
>>          childPaintInfo.context->beginTransparencyLayer(opacity);
> 
> I think we do the opacity in SVGRenderSupport as well. Can you remove the calls here and after your code please? Also, can you add a test to verify the behavior please?

Done.

>> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:75
>>          childPaintInfo.context->endTransparencyLayer();
> 
> (needs to be removed as well)

Done.

[Dirk Schulze]

Comment on attachment 114593 [details]
Patch

LGTM. r=me.

[WebKit Review Bot]

Comment on attachment 114593 [details]
Patch

Clearing flags on attachment: 114593

Committed r100045: <http://trac.webkit.org/changeset/100045>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Erik Arvidsson]

*** Bug 44071 has been marked as a duplicate of this bug. ***