[Webkit-unassigned] [Bug 41605] Fixes to the gaussian blur algorithm

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=41605


Dirk Schulze <krit at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #60652|review?                     |review-
               Flag|                            |




--- Comment #5 from Dirk Schulze <krit at webkit.org>  2010-07-06 22:45:56 PST ---
(From update of attachment 60652)
WebCore/platform/graphics/filters/FEGaussianBlur.cpp:38
 +  static const float gGaussianKernelFactor = (3 * sqrt(2 * piDouble) / 4);
Again, was my mistake, see comment above. Can you delete #include <math.h> as well please? Shouldn't be needed.

WebCore/platform/graphics/filters/FEGaussianBlur.cpp:150
 +      unsigned kernelSizeX = static_cast<unsigned>(floor(max(m_stdX, static_cast<float>(2.0)) * filter->filterResolution().width() * gGaussianKernelFactor + 0.5));
Isn't it enough to write 2.f instead of static_cast<float>(2.0)? Please use 0.5f. Only if you have a whole-number, you can omit .0 or .f (not sure about the denominator on a division).

WebCore/platform/graphics/filters/FEGaussianBlur.cpp:151
 +      unsigned kernelSizeY = static_cast<unsigned>(floor(max(m_stdY, static_cast<float>(2.0)) * filter->filterResolution().height() * gGaussianKernelFactor + 0.5));
dito

To the LayoutTests.. You changes behavior, so you need some more layoutTests to cover the changes. I suggest a test, with a stdDerivation of "0", "0 3" and "3 0". According to the new spec, we should see results in all three cases (not the case at the moment).
You changed the result of one of the tests, so that other platforms don't need new test results? That's not the regular case to do so :-) If you don't have a Mac, or no access to a Mac, I can create the results for you.
The patch misses a ChangeLog entry for LayoutTests.

Great patch! Hope to see an update soon. r- for the few style issues and the missing LayoutTests/Changelog-entry.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.