Move buildFilterOperations to FilterEffectBuilder

third_party/WebKit/Source/core/paint/PaintLayer.cpp

Index: third_party/WebKit/Source/core/paint/PaintLayer.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintLayer.cpp b/third_party/WebKit/Source/core/paint/PaintLayer.cpp
index 52ad1408b3d2c41d93722417ea5935d6f6635614..42a9862d0da81f73018766fbc2fb3c7402c7e41d 100644
--- a/third_party/WebKit/Source/core/paint/PaintLayer.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintLayer.cpp
@@ -2689,24 +2689,6 @@ bool PaintLayer::scrollsOverflow() const
     return false;
 }
 
-namespace {
-
-FilterOperations resolveReferenceFilters(const FilterEffectBuilder& builder, const FilterOperations& filters)
-{
-    DCHECK(filters.hasReferenceFilter());
-
-    for (FilterOperation* filterOperation : filters.operations()) {
-        if (filterOperation->type() != FilterOperation::REFERENCE)
-            continue;
-        ReferenceFilterOperation& referenceOperation = toReferenceFilterOperation(*filterOperation);
-        // TODO(fs): Cache the Filter if it didn't change.
-        referenceOperation.setFilter(builder.buildReferenceFilter(referenceOperation));
-    }
-    return filters;
-}
-
-} // unnamed namespace
-
 FilterOperations PaintLayer::addReflectionToFilterOperations(const ComputedStyle& style) const

[Stephen White, 2016/09/21 14:21:24]

I'm wondering if it would make sense to make the builder responsible for this
step. As in, we would construct the builder with just the style, and it would
extract the filters and box-reflect (if any) internally. It would avoid a bit of
duplicated code below. OTOH, we'd probably have to have a
"buildFilterOperations()" and "buildBackdropFilterOperations()" on the builder.
WDYT?

[fs, 2016/09/21 14:45:36]

Yes, I've been pondering this a little bit... the downside to putting in the
builder is that it requires even more state to be tracked. Handling it outside,
as a separate step, has other issues (might need to instantiate a Filter if
'filter' is 'none' and things like that.) I guess I could experiment a bit with
it and see what it will look like.

 {
     FilterOperations filterOperations = style.filter();
@@ -2719,22 +2701,20 @@ FilterOperations PaintLayer::addReflectionToFilterOperations(const ComputedStyle
 
 CompositorFilterOperations PaintLayer::createCompositorFilterOperationsForFilter(const ComputedStyle& style)
 {
-    FilterOperations filterOperations = addReflectionToFilterOperations(style);
-    if (filterOperations.hasReferenceFilter()) {
-        FilterEffectBuilder builder(toElement(enclosingNode()), boxForFilter(), style.effectiveZoom());
-        filterOperations = resolveReferenceFilters(builder, filterOperations);
-    }
-    return SkiaImageFilterBuilder::buildFilterOperations(filterOperations);
+    FloatRect zoomedReferenceBox;
+    if (style.filter().hasReferenceFilter())
+        zoomedReferenceBox = boxForFilter();
+    FilterEffectBuilder builder(enclosingNode(), zoomedReferenceBox, style.effectiveZoom());
+    return builder.buildFilterOperations(addReflectionToFilterOperations(style));
 }
 
 CompositorFilterOperations PaintLayer::createCompositorFilterOperationsForBackdropFilter(const ComputedStyle& style)
 {
-    FilterOperations operations = style.backdropFilter();
-    if (operations.hasReferenceFilter()) {
-        FilterEffectBuilder builder(toElement(enclosingNode()), boxForFilter(), style.effectiveZoom());
-        operations = resolveReferenceFilters(builder, operations);
-    }
-    return SkiaImageFilterBuilder::buildFilterOperations(operations);
+    FloatRect zoomedReferenceBox;
+    if (style.backdropFilter().hasReferenceFilter())
+        zoomedReferenceBox = boxForFilter();
+    FilterEffectBuilder builder(enclosingNode(), zoomedReferenceBox, style.effectiveZoom());
+    return builder.buildFilterOperations(style.backdropFilter());
 }
 
 PaintLayerFilterInfo& PaintLayer::ensureFilterInfo()
@@ -2795,17 +2775,11 @@ FilterEffect* PaintLayer::updateFilterEffect() const
         return filterInfo->lastEffect();
 
     const ComputedStyle& style = layoutObject()->styleRef();
-    const bool hasReferenceFilter = style.filter().hasReferenceFilter();
     FloatRect zoomedReferenceBox;
-    if (hasReferenceFilter)
+    if (style.filter().hasReferenceFilter())
         zoomedReferenceBox = boxForFilter();
-
-    FilterOperations operations = addReflectionToFilterOperations(style);
-    FilterEffectBuilder builder(toElement(enclosingNode()), zoomedReferenceBox, style.effectiveZoom());
-    if (hasReferenceFilter)
-        operations = resolveReferenceFilters(builder, operations);
-
-    filterInfo->setLastEffect(builder.buildFilterEffect(operations));
+    FilterEffectBuilder builder(enclosingNode(), zoomedReferenceBox, style.effectiveZoom());
+    filterInfo->setLastEffect(builder.buildFilterEffect(addReflectionToFilterOperations(style)));
     return filterInfo->lastEffect();
 }