[webkit-reviews] review denied: [Bug 55361] SVG 1.1: ineffectual transform attribute for ClipPath : [Attachment 94542] Work in progress patch

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied  review:
Bug 55361: SVG 1.1: ineffectual transform attribute for ClipPath
https://bugs.webkit.org/show_bug.cgi?id=55361

Attachment 94542: Work in progress patch
https://bugs.webkit.org/attachment.cgi?id=94542&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=94542&action=review

> LayoutTests/svg/custom/transformed-clip-path.svg:6
> +<clipPath id="clip" clipPathUnits="objectBoundingBox" transform="scale(2)">
> +    <rect width="50%" height="50%"/>
> +    <!-- second rect causes masking -->
> +</clipPath>

If this is for testing the fastClip option, I'm fine but please remove the
comment. Nevertheless we need a test where we switch to masking, thats why I
added a second rect (which you removed). Please also take the comment below
into account. This test is in the wrong folder, we have a own folder for
clipping tests.

> Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp:144
> +   
clipPath.transform(static_cast<SVGClipPathElement*>(node())->animatedLocalTrans
form());

This looks ok.

> Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp:190
> +	  
maskContext->concatCTM(static_cast<SVGClipPathElement*>(node())->animatedLocalT
ransform());

Shouldn't that happen before we create the imageBuffer to get the correct size?
Imagine a scale of 10 or 1000. Wouldn't we get pixelation here? Can you add a
test case with such a scale level?