Returning IntSize to store calculated kernel size values

Source/platform/graphics/filters/FEGaussianBlur.cpp

Index: Source/platform/graphics/filters/FEGaussianBlur.cpp
diff --git a/Source/platform/graphics/filters/FEGaussianBlur.cpp b/Source/platform/graphics/filters/FEGaussianBlur.cpp
index 631eac48165ff4e7b57d86d0067fbdda7db7d160..a5d8213d7d8a9fac73871133e766883d07d064a6 100644
--- a/Source/platform/graphics/filters/FEGaussianBlur.cpp
+++ b/Source/platform/graphics/filters/FEGaussianBlur.cpp
@@ -44,7 +44,7 @@ static inline float gaussianKernelFactor()
     return 3 / 4.f * sqrtf(twoPiFloat);
 }
 
-static const unsigned gMaxKernelSize = 1000;
+static const int gMaxKernelSize = 1000;

[pdr., 2014/03/26 00:00:13]

Curious: Why int? Can this be changed to unsigned and "int size" below changed
to "unsigned size"?

[Savago, 2014/03/26 00:12:24]

IntSize.setWidth/setHeight() operates on integers. It would require casting, so
the path of less resistance is to change the const gMaxKernelSize to become an
int.

 
 namespace WebCore {
 
@@ -228,43 +228,43 @@ inline void FEGaussianBlur::platformApply(Uint8ClampedArray* srcPixelArray, Uint
     platformApplyGeneric(srcPixelArray, tmpPixelArray, kernelSizeX, kernelSizeY, paintSize);
 }
 
-void FEGaussianBlur::calculateUnscaledKernelSize(unsigned& kernelSizeX, unsigned& kernelSizeY, float stdX, float stdY)
+IntSize FEGaussianBlur::calculateUnscaledKernelSize(FloatPoint std)
 {
-    ASSERT(stdX >= 0 && stdY >= 0);
-
-    kernelSizeX = 0;
-    if (stdX)
-        kernelSizeX = max<unsigned>(2, static_cast<unsigned>(floorf(stdX * gaussianKernelFactor() + 0.5f)));
-    kernelSizeY = 0;
-    if (stdY)
-        kernelSizeY = max<unsigned>(2, static_cast<unsigned>(floorf(stdY * gaussianKernelFactor() + 0.5f)));
+    ASSERT(std.x() >= 0 && std.y() >= 0);
 
+    IntSize kernelSize(0, 0);
     // Limit the kernel size to 1000. A bigger radius won't make a big difference for the result image but
     // inflates the absolute paint rect to much. This is compatible with Firefox' behavior.
-    if (kernelSizeX > gMaxKernelSize)
-        kernelSizeX = gMaxKernelSize;
-    if (kernelSizeY > gMaxKernelSize)
-        kernelSizeY = gMaxKernelSize;
+    if (std.x()) {
+        int size = max<unsigned>(2, static_cast<unsigned>(floorf(std.x() * gaussianKernelFactor() + 0.5f)));
+        kernelSize.setWidth(min(size, gMaxKernelSize));
+    }
+
+    if (std.y()) {
+        int size = max<unsigned>(2, static_cast<unsigned>(floorf(std.y() * gaussianKernelFactor() + 0.5f)));
+        kernelSize.setHeight(min(size, gMaxKernelSize));
+    }
+
+    return kernelSize;
 }
 
-void FEGaussianBlur::calculateKernelSize(Filter* filter, unsigned& kernelSizeX, unsigned& kernelSizeY, float stdX, float stdY)
+IntSize FEGaussianBlur::calculateKernelSize(Filter* filter, float stdX, float stdY)
 {
     stdX = filter->applyHorizontalScale(stdX);
     stdY = filter->applyVerticalScale(stdY);
 
-    calculateUnscaledKernelSize(kernelSizeX, kernelSizeY, stdX, stdY);
+    return calculateUnscaledKernelSize(FloatPoint(stdX, stdY));
 }
 
 FloatRect FEGaussianBlur::mapRect(const FloatRect& rect, bool)
 {
     FloatRect result = rect;
-    unsigned kernelSizeX = 0;
-    unsigned kernelSizeY = 0;
-    calculateKernelSize(filter(), kernelSizeX, kernelSizeY, m_stdX, m_stdY);
+    IntSize kernelSize(0, 0);

[Stephen White, 2014/03/26 00:22:21]

No point in explicitly initializing this if you're going to assign it on the
next line. Please combine the declaration with the next line.

+    kernelSize = calculateKernelSize(filter(), m_stdX, m_stdY);
 
     // We take the half kernel size and multiply it with three, because we run box blur three times.
-    result.inflateX(3 * kernelSizeX * 0.5f);
-    result.inflateY(3 * kernelSizeY * 0.5f);
+    result.inflateX(3 * kernelSize.width() * 0.5f);
+    result.inflateY(3 * kernelSize.height() * 0.5f);
     return result;
 }
 
@@ -304,15 +304,14 @@ void FEGaussianBlur::applySoftware()
     if (!m_stdX && !m_stdY)
         return;
 
-    unsigned kernelSizeX = 0;
-    unsigned kernelSizeY = 0;
-    calculateKernelSize(filter(), kernelSizeX, kernelSizeY, m_stdX, m_stdY);
+    IntSize kernelSize(0, 0);

[pdr., 2014/03/26 00:00:13]

Is this needed?

[Stephen White, 2014/03/26 00:22:21]

Same here.

+    kernelSize = calculateKernelSize(filter(), m_stdX, m_stdY);
 
     IntSize paintSize = absolutePaintRect().size();
     RefPtr<Uint8ClampedArray> tmpImageData = Uint8ClampedArray::createUninitialized(paintSize.width() * paintSize.height() * 4);
     Uint8ClampedArray* tmpPixelArray = tmpImageData.get();
 
-    platformApply(srcPixelArray, tmpPixelArray, kernelSizeX, kernelSizeY, paintSize);
+    platformApply(srcPixelArray, tmpPixelArray, kernelSize.width(), kernelSize.height(), paintSize);
 }
 
 bool FEGaussianBlur::applySkia()