Returning IntSize to store calculated kernel size values

Source/platform/graphics/filters/FEGaussianBlur.cpp

 /*
  * Copyright (C) 2004, 2005, 2006, 2007 Nikolas Zimmermann <zimmermann@kde.org>
  * Copyright (C) 2004, 2005 Rob Buis <buis@kde.org>
  * Copyright (C) 2005 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2009 Dirk Schulze <krit@webkit.org>
  * Copyright (C) 2010 Igalia, S.L.
  * Copyright (C) Research In Motion Limited 2010. All rights reserved.
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 26 matching lines @@
 
 #include "SkBlurImageFilter.h"
 
 using namespace std;
 
 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 {
 
 FEGaussianBlur::FEGaussianBlur(Filter* filter, float x, float y)
     : FilterEffect(filter)
     , m_stdX(x)
     , m_stdY(y)
 {
 }
 
@@ skipping 163 matching lines @@
             }
             return;
         }
         // Fallback to single threaded mode.
     }
 
     // The selection here eventually should happen dynamically on some platforms.
     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);
+    ASSERT(std.x() >= 0 && std.y() >= 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)));
-
+    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;
 }
 
 FloatRect FEGaussianBlur::determineAbsolutePaintRect(const FloatRect& originalRequestedRect)
 {
     FloatRect requestedRect = originalRequestedRect;
     if (clipsToBounds())
         requestedRect.intersect(maxEffectRect());
 
     FilterEffect* input = inputEffect(0);
@@ skipping 19 matching lines @@
         return;
 
     setIsAlphaImage(in->isAlphaImage());
 
     IntRect effectDrawingRect = requestedRegionOfInputImageData(in->absolutePaintRect());
     in->copyPremultipliedImage(srcPixelArray, effectDrawingRect);
 
     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()
 {
     ImageBuffer* resultImage = createImageBufferResult();
     if (!resultImage)
         return false;
 
     FilterEffect* in = inputEffect(0);
 
@@ skipping 30 matching lines @@
 {
     writeIndent(ts, indent);
     ts << "[feGaussianBlur";
     FilterEffect::externalRepresentation(ts);
     ts << " stdDeviation=\"" << m_stdX << ", " << m_stdY << "\"]\n";
     inputEffect(0)->externalRepresentation(ts, indent + 1);
     return ts;
 }
 
 } // namespace WebCore