Bug 28141 – SVG Filter feGaussianBlur implementation is missing

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Dirk Schulze]

SVG Filter feGaussianBlur implementation is missing.

Details for implementing:

The Specification http://www.w3.org/TR/SVG11/filters.html#feGaussianBlur give an alternative for a real gaussian blur, if stdDeviation is >= 2.0. Many other SVG Implementations take this alternative for every stdDeviation >= 0. It looks like even FireFox just uses the alternative. We might follow this becaues most SVG's with gaussianBlur take a stdDeviation > 2 and it reduces the complexity of calculation.

[Dirk Schulze]

Created attachment 34824 [details]
naive feGaussian-blur

A very simlpe feGaussian-blur with box blur.

[Dirk Schulze]

calculateDrawingIntRect will be introduced by bug 28133 , the last feBlend patch.

[Dirk Schulze]

Created attachment 40597 [details]
feGaussianBlur implementation

Implementation of feGaussianBlur.

[Eric Seidel (no email)]

This looks like a patch a graphics person like Oliver should comment on.

[Dirk Schulze]

Nikolas asked why I call boxBlur twice in the for loop. I would like to explain it for the reviewer. The first call is for the horizontal bluring, the second call is for the vertical bluring. This is the same like a bluring with a complete kernel, but faster.
The reason for bluring the image 3 times is, that the spec want us to.

[Oliver Hunt]

Comment on attachment 40597 [details]
feGaussianBlur implementation

There are a few places where blurring is misspelled as bluring.

double(sum / dx) seems bogus -- both sum and dx are integer types, so you're just doing an integer divide and casting to double, which is either superfluous, or not what you intended to do.

What happens if i apply this filter to a 3x3 image?  it looks to me like there's potential for overflow...

[Dirk Schulze]

Created attachment 40713 [details]
feGaussianBlur implementation

I changed the double conversion to unsigned char. And fixed the writing of blur(r)ing :-). I also missed an early return, if one dimension of the kernel has the size of 0.

You were right with the overflow. There is a overflow on filling the kernel if the kernel size is bigger than the width/height of the effect. I double checked the rest of the code and didn't see further problems. I calculated some examples manualy (e.g. A kernel with 3x3) and it works as expected. An overflow on the blurring itself seems to me impossible, since I check if the next or previous value is not catched if it isn't in the current row/column (between 0 and width/height of a column/row).

I compared many other examples with FF and Opera and did not see any differences.

[Dirk Schulze]

Created attachment 40762 [details]
feGaussianBlur implementation

> +                if (x + dxRight < effectHeight)

mixed up effectHeight with effectWidth here. So it just works if effectHeight == effectWidth. This patch fixes this issue.

[Oliver Hunt]

Comment on attachment 40762 [details]
feGaussianBlur implementation

Code looks good, but i want additional tests for filtering 0x0, x*y with x!=y, and x*y with x<3, y > 3 and vice versa.

[Dirk Schulze]

Created attachment 40777 [details]
feGaussianBlur implementation

added test with different kernel sizes. No change to the source.

[Dirk Schulze]

landed in r49402.