Harmonize FilterEffect::mapRect and mapPaintRect

third_party/WebKit/Source/platform/graphics/filters/FEComposite.cpp

Index: third_party/WebKit/Source/platform/graphics/filters/FEComposite.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/filters/FEComposite.cpp b/third_party/WebKit/Source/platform/graphics/filters/FEComposite.cpp
index 2b663728d28bab97effb40bbd6d24233957db0ea..c8711cf8d8004b960c492b67e16e5ee4ff741b18 100644
--- a/third_party/WebKit/Source/platform/graphics/filters/FEComposite.cpp
+++ b/third_party/WebKit/Source/platform/graphics/filters/FEComposite.cpp
@@ -113,55 +113,39 @@ bool FEComposite::setK4(float k4)
     return true;
 }
 
-FloatRect FEComposite::determineAbsolutePaintRect(const FloatRect& originalRequestedRect) const
+bool FEComposite::affectsTransparentPixels() const
 {
-    FloatRect requestedRect = originalRequestedRect;
-    if (clipsToBounds())
-        requestedRect.intersect(absoluteBounds());
+    return k4() > 0;

[pdr., 2016/09/16 18:29:18]

Can you add a comment explaining this line?

There's this comment in mapInputs but it doesn't really explain why. The spec's
equation is "result = k1*i1*i2 + k2*i1 + k3*i2 + k4" and it's not clear how k4
different from k1-k3 in terms of affecting transparent pixels.

[fs, 2016/09/16 18:53:28]

Sure.
i1=i2=0, k1=k2=k3=x, k4>0 => result > 0 (not a transparent pixel)

More generally "(transparent <op> transparent) != transparent".  

[fs, 2016/09/16 20:16:21]

Noticed that I should probably have checked m_type here as well to not pessimize
unnecessarily, so thank for prodding! =)

+}
 
-    // No mapPaintRect required for FEComposite.
-    FloatRect input1Rect = inputEffect(1)->determineAbsolutePaintRect(requestedRect);
-    FloatRect affectedRect;
+FloatRect FEComposite::mapInputs(const FloatRect& rect) const
+{
+    FloatRect input1Rect = inputEffect(1)->mapRect(rect);
     switch (m_type) {
     case FECOMPOSITE_OPERATOR_IN:
         // 'in' has output only in the intersection of both inputs.
-        affectedRect = intersection(input1Rect, inputEffect(0)->determineAbsolutePaintRect(input1Rect));
-        break;
+        return intersection(input1Rect, inputEffect(0)->mapRect(input1Rect));
     case FECOMPOSITE_OPERATOR_ATOP:
         // 'atop' has output only in the extents of the second input.
-        // Make sure first input knows where it needs to produce output.
-        inputEffect(0)->determineAbsolutePaintRect(input1Rect);
-        affectedRect = input1Rect;
-        break;
+        return input1Rect;
     case FECOMPOSITE_OPERATOR_ARITHMETIC:
-        if (k4() > 0) {
-            // Make sure first input knows where it needs to produce output.
-            inputEffect(0)->determineAbsolutePaintRect(requestedRect);
-            // Arithmetic with non-zero k4 may influnce the complete filter primitive
-            // region. So we can't optimize the paint region here.
-            affectedRect = requestedRect;
-            break;
-        }
+        // Arithmetic with non-zero k4 may influnce the complete filter primitive

[pdr., 2016/09/16 18:29:18]

Nit: "influence"

If possible, can you add comments above each of these conditionals explaining
why?

[fs, 2016/09/16 18:53:28]

I'll see if can express this in a not too verbose way.

[fs, 2016/09/16 20:16:21]

Comments added.

+        // region.
+        if (k4() > 0)
+            return rect;
         if (k2() <= 0) {
             // Input 0 does not appear where input 1 is not present.
-            FloatRect input0Rect = inputEffect(0)->determineAbsolutePaintRect(input1Rect);
-            if (k3() > 0) {
-                affectedRect = input1Rect;
-            } else {
-                // Just k1 is positive. Use intersection.
-                affectedRect = intersection(input1Rect, input0Rect);
-            }
-            break;
+            if (k3() > 0)
+                return input1Rect;
+            // Just k1 is positive. Use intersection.
+            return intersection(input1Rect, inputEffect(0)->mapRect(input1Rect));
         }
         // else fall through to use union
     default:
-        // Take the union of both input effects.
-        affectedRect = unionRect(input1Rect, inputEffect(0)->determineAbsolutePaintRect(requestedRect));
         break;
     }
-
-    affectedRect.intersect(requestedRect);
-    return affectedRect;
+    // Take the union of both input effects.
+    return unionRect(input1Rect, inputEffect(0)->mapRect(rect));
 }
 
 SkXfermode::Mode toXfermode(CompositeOperationType mode)