Revamp filter primitive region calculations for Filter Effects

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

Index: third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp b/third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp
index 4cbb72d65edbb79b619a8703b3f0a4017f68ecdc..b18971f8d2d0865728b5acf27af0e69d2f1ae37b 100644
--- a/third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp
+++ b/third_party/WebKit/Source/platform/graphics/filters/FilterEffect.cpp
@@ -33,10 +33,6 @@ namespace blink {
 
 FilterEffect::FilterEffect(Filter* filter)
     : m_filter(filter)
-    , m_hasX(false)
-    , m_hasY(false)
-    , m_hasWidth(false)
-    , m_hasHeight(false)
     , m_clipsToBounds(true)
     , m_originTainted(false)
     , m_operatingColorSpace(ColorSpaceLinearRGB)
@@ -133,58 +129,44 @@ TextStream& FilterEffect::externalRepresentation(TextStream& ts, int) const
     return ts;
 }
 
-FloatRect FilterEffect::applyEffectBoundaries(const FloatRect& rect) const
+FloatRect FilterEffect::determineMaximumEffectRect(DetermineMaxEffectRectFlags flags)
 {
-    FloatRect clippedRect = rect;
-    if (hasX())
-        clippedRect.setX(effectBoundaries().x());
-    if (hasY())
-        clippedRect.setY(effectBoundaries().y());
-    if (hasWidth())
-        clippedRect.setWidth(effectBoundaries().width());
-    if (hasHeight())
-        clippedRect.setHeight(effectBoundaries().height());
-    return clippedRect;
-}
-
-FloatRect FilterEffect::determineFilterPrimitiveSubregion(DetermineSubregionFlags flags)
-{
-    Filter* filter = this->getFilter();
-    ASSERT(filter);
-
-    // FETile, FETurbulence, FEFlood don't have input effects, take the filter region as unite rect.
-    FloatRect subregion;
-    if (unsigned numberOfInputEffects = inputEffects().size()) {
-        subregion = inputEffect(0)->determineFilterPrimitiveSubregion(flags);
-        for (unsigned i = 1; i < numberOfInputEffects; ++i)
-            subregion.unite(inputEffect(i)->determineFilterPrimitiveSubregion(flags));
-    } else {
-        subregion = filter->filterRegion();
-    }
+    DCHECK(getFilter());
+    Filter* filter = getFilter();
 
-    // After calling determineFilterPrimitiveSubregion on the target effect, reset the subregion again for <feTile>.
-    if (getFilterEffectType() == FilterEffectTypeTile)
-        subregion = filter->filterRegion();
-
-    if (flags & MapRectForward) {
-        // mapRect works on absolute rectangles.
-        subregion = filter->mapAbsoluteRectToLocalRect(mapRect(
-            filter->mapLocalRectToAbsoluteRect(subregion)));
+    // Compute the union of the inputs. Always do this because it has
+    // side-effects. (It computes the maximum effect rect of the input.)
+    FloatRect inputUnion;

[pdr., 2016/09/03 04:50:03]

Nit: absoluteInputUnion?

[fs, 2016/09/04 09:34:24]

Done.

+    for (auto& effect : m_inputEffects)
+        inputUnion.unite(effect->determineMaximumEffectRect(flags));
+
+    FloatRect absoluteSubregion;
+    switch (getFilterEffectType()) {
+    default:
+        if (m_inputEffects.size()) {
+            absoluteSubregion = inputUnion;
+            if (clipsToBounds())
+                absoluteSubregion.intersect(filter->mapLocalRectToAbsoluteRect(filterPrimitiveSubregion()));
+            break;
+        }
+        // Else fall-through and use the primitive region. (FETurbulence/FEFlood/FEImage)
+    case FilterEffectTypeTile:
+        absoluteSubregion = filter->mapLocalRectToAbsoluteRect(filterPrimitiveSubregion());
+        break;
+    case FilterEffectTypeSourceInput:
+        absoluteSubregion = filter->absoluteFilterRegion();
+        break;
     }
 
-    subregion = applyEffectBoundaries(subregion);
-
-    setFilterPrimitiveSubregion(subregion);
-
-    FloatRect absoluteSubregion = filter->mapLocalRectToAbsoluteRect(subregion);
+    if (flags & MapRectForward)
+        absoluteSubregion = mapRect(absoluteSubregion);
 
     // Clip every filter effect to the filter region.
-    if (flags & ClipToFilterRegion) {
+    if (flags & ClipToFilterRegion)
         absoluteSubregion.intersect(filter->absoluteFilterRegion());
-    }
 
-    setMaxEffectRect(absoluteSubregion);
-    return subregion;
+    m_maxEffectRect = absoluteSubregion;

[pdr., 2016/09/03 04:50:03]

(Not for this patch, possibly one of the pessimizations :)
It seems brittle that we rely on determineMaximumEffectRect being called before
maxEffectRect is used, and that the determined max effect rect type (forward,
clip, etc) happens to be what the maxEffectRect caller wants. And... yeah.

[fs, 2016/09/04 09:34:24]

Yeah...

+    return absoluteSubregion;
 }
 
 sk_sp<SkImageFilter> FilterEffect::createImageFilter()
@@ -216,8 +198,7 @@ sk_sp<SkImageFilter> FilterEffect::createTransparentBlack() const
 SkImageFilter::CropRect FilterEffect::getCropRect() const
 {
     if (!filterPrimitiveSubregion().isEmpty()) {
-        FloatRect rect = filterPrimitiveSubregion();
-        rect.scale(getFilter()->scale());
+        FloatRect rect = getFilter()->mapLocalRectToAbsoluteRect(filterPrimitiveSubregion());
         return SkImageFilter::CropRect(rect);
     } else {
         return SkImageFilter::CropRect(SkRect::MakeEmpty(), 0);