[webkit-reviews] review granted: [Bug 31370] SVG filter effects need smarter size calculation : [Attachment 69902] Patch

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has granted Dirk Schulze
<krit at webkit.org>'s request for review:
Bug 31370: SVG filter effects need smarter size calculation
https://bugs.webkit.org/show_bug.cgi?id=31370

Attachment 69902: Patch
https://bugs.webkit.org/attachment.cgi?id=69902&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=69902&action=review

> WebCore/WebCore.xcodeproj/project.pbxproj:21023
> +			developmentRegion = English;

Can you revert this line? Newer Xcodes fault :-)

> WebCore/platform/graphics/filters/FEGaussianBlur.cpp:38
>  static const float gGaussianKernelFactor = (3 * sqrtf(2 * piFloat) / 4.f);

While you're at it removes those braces, and say "3 / 4.f * sqrtf(2 * piFloat)"
here :-)

> WebCore/platform/graphics/filters/FEGaussianBlur.cpp:152
> +    IntRect absolutePaintRect = inputEffect(0)->absolutePaintRect();

Stil a problem, use FloatRect absolutePaintRect here.

> WebCore/platform/graphics/filters/FEGaussianBlur.cpp:160
> +    absolutePaintRect.inflateX(3 * kernelSizeX * 0.5f);

Otherwhise this inflateX will produce artefacts.
How about using 1.5f * kernelSizeX here?

> WebCore/platform/graphics/filters/FEGaussianBlur.cpp:162
> +    setAbsolutePaintRect(absolutePaintRect);

Then use enclosingIntRect here.

> WebCore/platform/graphics/filters/FETile.cpp:76
> +    if (!SVGImageBufferTools::createImageBuffer(tileRect, tileRect,
tileImage, DeviceRGB))

Won't DeviceRGB be problematic for non-cg platforms here? Still unsure...
RenderSVGResourceMasker uses LinearRGB for instance.

> WebCore/rendering/RenderSVGResourceFilter.cpp:167
> +    // Eliminate shear of the absolute transformation matrix, to be able to
produce unsheared tile images in feTile.

s/in feTile/for feTile/

Other than these issues, it looks great! Looking forward to have non-pixelated
filters :-)