Merge r182067 - FEMorphology::platformApplyGeneric() should bail out …

…if the radius is less than or equal to zero.

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

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2015-03-27
Reviewed by Dean Jackson.

Source/WebCore:

FEMorphology class implementation code clean up.

Tests: svg/filters/feMorphology-radius-cases.svg

* platform/graphics/filters/FEMorphology.cpp:
(WebCore::shouldSupersedeExtremum): Reuse code instead of repeating it and
use < and > instead of =< and >=.

(WebCore::pixelArrayIndex): Returns the array index of a pixel in an image
buffer, given: position(x, y), image width and the color channel.

(WebCore::columnExtremum): Returns the extremum of a column of pixels.

(WebCore::kernelExtremum): Returns the extremum of a filter kernel.

(WebCore::FEMorphology::platformApplyGeneric): Apply some code clean-up.
The kernel size should be equal to radius of the filter. The extra pixel
was causing the resulted image to be asymmetric in some cases.

(WebCore::FEMorphology::platformApplyDegenerate):
(WebCore::FEMorphology::platformApplySoftware): After applying scaling, we
still need to check the resulted radius is negative (overflow case) or less
than one (zero radius case) and treat these cases differently.

(WebCore::FEMorphology::morphologyOperator): Deleted.
(WebCore::FEMorphology::radiusX): Deleted.
(WebCore::FEMorphology::radiusY): Deleted.
* platform/graphics/filters/FEMorphology.h:
(WebCore::FEMorphology::morphologyOperator):
(WebCore::FEMorphology::radiusX):
(WebCore::FEMorphology::radiusY):
Move a single line functions from the source file to the header file.

LayoutTests:

* svg/filters/feMorphology-radius-cases-expected.svg: Added.
* svg/filters/feMorphology-radius-cases.svg: Added.
Test different cases for radius of the feMorphology filter. There are three
cases for the radius:
    1. radius < 0: This is an error case, the source image should not be rendered.
    2. radius = 0: This case is treated as if the filter never exists.
    3. radius > 0: If the scaled radius is > 0, the filter is applied.

LayoutTests/ChangeLog

@@ -1,3 +1,18 @@
+2015-03-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        FEMorphology::platformApplyGeneric() should bail out if the radius is less than or equal to zero.
+        https://bugs.webkit.org/show_bug.cgi?id=142885.
+
+        Reviewed by Dean Jackson.
+
+        * svg/filters/feMorphology-radius-cases-expected.svg: Added.
+        * svg/filters/feMorphology-radius-cases.svg: Added.
+        Test different cases for radius of the feMorphology filter. There are three 
+        cases for the radius:
+            1. radius < 0: This is an error case, the source image should not be rendered.
+            2. radius = 0: This case is treated as if the filter never exists.
+            3. radius > 0: If the scaled radius is > 0, the filter is applied.
+
 2015-04-13  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Fix LayoutTests/http/tests/canvas/canvas-tainted-after-draw-image.html on all bots

Source/WebCore/ChangeLog

@@ -1,3 +1,43 @@
+2015-03-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        FEMorphology::platformApplyGeneric() should bail out if the radius is less than or equal to zero.
+        https://bugs.webkit.org/show_bug.cgi?id=142885.
+
+        Reviewed by Dean Jackson.
+
+        FEMorphology class implementation code clean up.
+
+        Tests: svg/filters/feMorphology-radius-cases.svg
+
+        * platform/graphics/filters/FEMorphology.cpp:
+        (WebCore::shouldSupersedeExtremum): Reuse code instead of repeating it and
+        use < and > instead of =< and >=.
+
+        (WebCore::pixelArrayIndex): Returns the array index of a pixel in an image
+        buffer, given: position(x, y), image width and the color channel.
+
+        (WebCore::columnExtremum): Returns the extremum of a column of pixels.
+
+        (WebCore::kernelExtremum): Returns the extremum of a filter kernel.
+
+        (WebCore::FEMorphology::platformApplyGeneric): Apply some code clean-up.
+        The kernel size should be equal to radius of the filter. The extra pixel
+        was causing the resulted image to be asymmetric in some cases.
+
+        (WebCore::FEMorphology::platformApplyDegenerate):
+        (WebCore::FEMorphology::platformApplySoftware): After applying scaling, we
+        still need to check the resulted radius is negative (overflow case) or less
+        than one (zero radius case) and treat these cases differently.
+
+        (WebCore::FEMorphology::morphologyOperator): Deleted.
+        (WebCore::FEMorphology::radiusX): Deleted.
+        (WebCore::FEMorphology::radiusY): Deleted.
+        * platform/graphics/filters/FEMorphology.h:
+        (WebCore::FEMorphology::morphologyOperator):
+        (WebCore::FEMorphology::radiusX):
+        (WebCore::FEMorphology::radiusY):
+        Move a single line functions from the source file to the header file.
+
 2015-04-13  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Canvas drawImage() has a security hole when the image isn't yet fully loaded.

Source/WebCore/platform/graphics/filters/FEMorphology.cpp

@@ -48,11 +48,6 @@ PassRefPtr<FEMorphology> FEMorphology::create(Filter* filter, MorphologyOperator
     return adoptRef(new FEMorphology(filter, type, radiusX, radiusY));
 }
 
-MorphologyOperatorType FEMorphology::morphologyOperator() const
-{
-    return m_type;
-}
-
 bool FEMorphology::setMorphologyOperator(MorphologyOperatorType type)
 {
     if (m_type == type)
@@ -61,11 +56,6 @@ bool FEMorphology::setMorphologyOperator(MorphologyOperatorType type)
     return true;
 }
 
-float FEMorphology::radiusX() const
-{
-    return m_radiusX;
-}
-
 bool FEMorphology::setRadiusX(float radiusX)
 {
     if (m_radiusX == radiusX)
@@ -74,9 +64,12 @@ bool FEMorphology::setRadiusX(float radiusX)
     return true;
 }
 
-float FEMorphology::radiusY() const
+bool FEMorphology::setRadiusY(float radiusY)
 {
-    return m_radiusY;
+    if (m_radiusY == radiusY)
+        return false;
+    m_radiusY = radiusY;
+    return true;
 }
 
 void FEMorphology::determineAbsolutePaintRect()
@@ -92,66 +85,77 @@ void FEMorphology::determineAbsolutePaintRect()
     setAbsolutePaintRect(enclosingIntRect(paintRect));
 }
 
-bool FEMorphology::setRadiusY(float radiusY)
+static inline bool shouldSupersedeExtremum(unsigned char newValue, unsigned char currentValue, MorphologyOperatorType type)
 {
-    if (m_radiusY == radiusY)
-        return false;
-    m_radiusY = radiusY;
-    return true;
+    return (type == FEMORPHOLOGY_OPERATOR_ERODE && newValue < currentValue)
+        || (type == FEMORPHOLOGY_OPERATOR_DILATE && newValue > currentValue);
+}
+
+static inline int pixelArrayIndex(int x, int y, int width, int colorChannel)
+{
+    return (y * width + x) * 4 + colorChannel;
+}
+
+static inline unsigned char columnExtremum(const Uint8ClampedArray* srcPixelArray, int x, int yStart, int yEnd, int width, unsigned colorChannel, MorphologyOperatorType type)
+{
+    unsigned char extremum = srcPixelArray->item(pixelArrayIndex(x, yStart, width, colorChannel));
+    for (int y = yStart + 1; y < yEnd; ++y) {
+        unsigned char pixel = srcPixelArray->item(pixelArrayIndex(x, y, width, colorChannel));
+        if (shouldSupersedeExtremum(pixel, extremum, type))
+            extremum = pixel;
+    }
+    return extremum;
+}
+
+static inline unsigned char kernelExtremum(const Vector<unsigned char>& kernel, MorphologyOperatorType type)
+{
+    Vector<unsigned char>::const_iterator iter = kernel.begin();
+    unsigned char extremum = *iter;
+    for (Vector<unsigned char>::const_iterator end = kernel.end(); ++iter != end; ) {
+        if (shouldSupersedeExtremum(*iter, extremum, type))
+            extremum = *iter;
+    }
+    return extremum;
 }
 
 void FEMorphology::platformApplyGeneric(PaintingData* paintingData, int yStart, int yEnd)
 {
     Uint8ClampedArray* srcPixelArray = paintingData->srcPixelArray;
     Uint8ClampedArray* dstPixelArray = paintingData->dstPixelArray;
-    const int width = paintingData->width;
-    const int height = paintingData->height;
-    const int effectWidth = width * 4;
     const int radiusX = paintingData->radiusX;
     const int radiusY = paintingData->radiusY;
+    const int width = paintingData->width;
+    const int height = paintingData->height;
+
+    ASSERT(radiusX <= width || radiusY <= height);
+    ASSERT(yStart >= 0 && yEnd <= height && yStart < yEnd);
 
     Vector<unsigned char> extrema;
     for (int y = yStart; y < yEnd; ++y) {
-        int extremaStartY = std::max(0, y - radiusY);
-        int extremaEndY = std::min(height - 1, y + radiusY);
-        for (unsigned int clrChannel = 0; clrChannel < 4; ++clrChannel) {
+        int yStartExtrema = std::max(0, y - radiusY);
+        int yEndExtrema = std::min(height - 1, y + radiusY);
+
+        for (unsigned colorChannel = 0; colorChannel < 4; ++colorChannel) {
             extrema.clear();
             // Compute extremas for each columns
-            for (int x = 0; x <= radiusX; ++x) {
-                unsigned char columnExtrema = srcPixelArray->item(extremaStartY * effectWidth + 4 * x + clrChannel);
-                for (int eY = extremaStartY + 1; eY < extremaEndY; ++eY) {
-                    unsigned char pixel = srcPixelArray->item(eY * effectWidth + 4 * x + clrChannel);
-                    if ((m_type == FEMORPHOLOGY_OPERATOR_ERODE && pixel <= columnExtrema)
-                        || (m_type == FEMORPHOLOGY_OPERATOR_DILATE && pixel >= columnExtrema)) {
-                        columnExtrema = pixel;
-                    }
-                }
-
-                extrema.append(columnExtrema);
-            }
+            for (int x = 0; x < radiusX; ++x)
+                extrema.append(columnExtremum(srcPixelArray, x, yStartExtrema, yEndExtrema, width, colorChannel, m_type));
 
             // Kernel is filled, get extrema of next column
             for (int x = 0; x < width; ++x) {
-                const int endX = std::min(x + radiusX, width - 1);
-                unsigned char columnExtrema = srcPixelArray->item(extremaStartY * effectWidth + endX * 4 + clrChannel);
-                for (int i = extremaStartY + 1; i <= extremaEndY; ++i) {
-                    unsigned char pixel = srcPixelArray->item(i * effectWidth + endX * 4 + clrChannel);
-                    if ((m_type == FEMORPHOLOGY_OPERATOR_ERODE && pixel <= columnExtrema)
-                        || (m_type == FEMORPHOLOGY_OPERATOR_DILATE && pixel >= columnExtrema))
-                        columnExtrema = pixel;
+                if (x < width - radiusX) {
+                    int xEnd = std::min(x + radiusX, width - 1);
+                    extrema.append(columnExtremum(srcPixelArray, xEnd, yStartExtrema, yEndExtrema + 1, width, colorChannel, m_type));
                 }
-                if (x - radiusX >= 0)
+
+                if (x > radiusX)
                     extrema.remove(0);
-                if (x + radiusX <= width)
-                    extrema.append(columnExtrema);
-
-                unsigned char entireExtrema = extrema[0];
-                for (unsigned kernelIndex = 1; kernelIndex < extrema.size(); ++kernelIndex) {
-                    if ((m_type == FEMORPHOLOGY_OPERATOR_ERODE && extrema[kernelIndex] <= entireExtrema)
-                        || (m_type == FEMORPHOLOGY_OPERATOR_DILATE && extrema[kernelIndex] >= entireExtrema))
-                        entireExtrema = extrema[kernelIndex];
-                }
-                dstPixelArray->set(y * effectWidth + 4 * x + clrChannel, entireExtrema);
+
+                // The extrema original size = radiusX.
+                // Number of new addition = width - radiusX.
+                // Number of removals = width - radiusX - 1.
+                ASSERT(extrema.size() >= static_cast<size_t>(radiusX + 1));
+                dstPixelArray->set(pixelArrayIndex(x, y, width, colorChannel), kernelExtremum(extrema, m_type));
             }
         }
     }
@@ -191,6 +195,23 @@ void FEMorphology::platformApply(PaintingData* paintingData)
     platformApplyGeneric(paintingData, 0, paintingData->height);
 }
 
+bool FEMorphology::platformApplyDegenerate(Uint8ClampedArray* dstPixelArray, const IntRect& imageRect, int radiusX, int radiusY)
+{
+    // Input radius is less than zero or an overflow happens when scaling it.
+    if (radiusX < 0 || radiusY < 0) {
+        dstPixelArray->zeroFill();
+        return true;
+    }
+
+    // Input radius is equal to zero or the scaled radius is less than one.
+    if (!m_radiusX || !m_radiusY) {
+        FilterEffect* in = inputEffect(0);
+        in->copyPremultipliedImage(dstPixelArray, imageRect);
+        return true;
+    }
+
+    return false;
+}
 
 void FEMorphology::platformApplySoftware()
 {
@@ -201,25 +222,28 @@ void FEMorphology::platformApplySoftware()
         return;
 
     setIsAlphaImage(in->isAlphaImage());
-    if (m_radiusX <= 0 || m_radiusY <= 0) {
-        dstPixelArray->zeroFill();
+
+    IntRect effectDrawingRect = requestedRegionOfInputImageData(in->absolutePaintRect());
+    if (platformApplyDegenerate(dstPixelArray, effectDrawingRect, m_radiusX, m_radiusY))
         return;
-    }
 
     Filter* filter = this->filter();
+    RefPtr<Uint8ClampedArray> srcPixelArray = in->asPremultipliedImage(effectDrawingRect);
     int radiusX = static_cast<int>(floorf(filter->applyHorizontalScale(m_radiusX)));
     int radiusY = static_cast<int>(floorf(filter->applyVerticalScale(m_radiusY)));
+    radiusX = std::min(effectDrawingRect.width() - 1, radiusX);
+    radiusY = std::min(effectDrawingRect.height() - 1, radiusY);
 
-    IntRect effectDrawingRect = requestedRegionOfInputImageData(in->absolutePaintRect());
-    RefPtr<Uint8ClampedArray> srcPixelArray = in->asPremultipliedImage(effectDrawingRect);
+    if (platformApplyDegenerate(dstPixelArray, effectDrawingRect, radiusX, radiusY))
+        return;
 
     PaintingData paintingData;
     paintingData.srcPixelArray = srcPixelArray.get();
     paintingData.dstPixelArray = dstPixelArray;
     paintingData.width = effectDrawingRect.width();
     paintingData.height = effectDrawingRect.height();
-    paintingData.radiusX = std::min(effectDrawingRect.width() - 1, radiusX);
-    paintingData.radiusY = std::min(effectDrawingRect.height() - 1, radiusY);
+    paintingData.radiusX = radiusX;
+    paintingData.radiusY = radiusY;
 
     platformApply(&paintingData);
 }