Extracting the SVG Filter Reference implementation out of the FilterEffectRenderer into its own typ…

Source/WebCore/rendering/RenderLayer.cpp

Index: Source/WebCore/rendering/RenderLayer.cpp
diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
index ae74bfb086d4eed85146c0f0a4da023a14ff006d..91c2a6933555dad8348cccfe0cd4b4a4b47e7585 100644
--- a/Source/WebCore/rendering/RenderLayer.cpp
+++ b/Source/WebCore/rendering/RenderLayer.cpp
@@ -54,6 +54,7 @@
 #include "FEColorMatrix.h"
 #include "FEMerge.h"
 #include "FilterEffectRenderer.h"
+#include "ReferenceFilterRenderer.h"
 #endif
 #include "FeatureObserver.h"
 #include "FloatConversion.h"
@@ -5989,13 +5990,6 @@ void RenderLayer::updateFilters(const RenderStyle* oldStyle, const RenderStyle*
     if (shouldUpdateFilters)
         backing()->updateFilters(renderer()->style());
     updateOrRemoveFilterEffectRenderer();
-    // FIXME: Accelerated SVG reference filters still rely on FilterEffectRenderer to build the filter graph.
-    // Thus, we have to call updateFilters again, after we have a FilterEffectRenderer.
-    // FilterEffectRenderer is intended to render software filters and shouldn't be needed for accelerated filters.
-    // We should extract the SVG graph building functionality out of FilterEffectRenderer, and it should happen in RenderLayer::computeFilterOperations.
-    // https://bugs.webkit.org/show_bug.cgi?id=114051
-    if (shouldUpdateFilters && newStyle->filter().hasReferenceFilter())
-        backing()->updateFilters(renderer()->style());
 }
 #endif
 
@@ -6192,24 +6186,21 @@ bool RenderLayer::isCSSCustomFilterEnabled() const
 #if ENABLE(CSS_FILTERS)
 FilterOperations RenderLayer::computeFilterOperations(const RenderStyle* style)
 {
-#if !ENABLE(CSS_SHADERS)
-    return style->filter();
-#else
     const FilterOperations& filters = style->filter();
-    if (!filters.hasCustomFilter())
+    if (!filters.hasCustomFilter() && !filters.hasReferenceFilter())
         return filters;
 
-    if (!isCSSCustomFilterEnabled()) {
-        // CSS Custom filters should not parse at all in this case, but there might be
-        // remaining styles that were parsed when the flag was enabled. Reproduces in DumpRenderTree
-        // because it resets the flag while the previous test is still loaded.
-        return FilterOperations();
-    }
-
     FilterOperations outputFilters;
     for (size_t i = 0; i < filters.size(); ++i) {
         RefPtr<FilterOperation> filterOperation = filters.operations().at(i);
-        if (filterOperation->getOperationType() == FilterOperation::CUSTOM) {
+        switch (filterOperation->getOperationType()) {
+        case FilterOperation::CUSTOM: {
+            if (!isCSSCustomFilterEnabled()) {
+                // CSS Custom filters should not parse at all in this case, but there might be
+                // remaining styles that were parsed when the flag was enabled. Reproduces in DumpRenderTree
+                // because it resets the flag while the previous test is still loaded.
+                continue;
+            }
             // We have to wait until the program of CSS Shaders is loaded before setting it on the layer.
             // Note that we will handle the loading of the shaders and repainting of the layer in updateOrRemoveFilterClients.
             const CustomFilterOperation* customOperation = static_cast<const CustomFilterOperation*>(filterOperation.get());
@@ -6227,10 +6218,17 @@ FilterOperations RenderLayer::computeFilterOperations(const RenderStyle* style)
             outputFilters.operations().append(validatedOperation.release());
             continue;
         }
+        case FilterOperation::REFERENCE: {
+            ReferenceFilterOperation* referenceOperation = static_cast<ReferenceFilterOperation*>(filterOperation.get());
+            ReferenceFilterRenderer::computeFilter(renderer(), referenceOperation);

[Stephen White, 2013/04/10 22:19:38]

OK, bear with me, I'm trying to understand the callgraph here.

FilterEffectRenderer::build() calls ReferenceFilterBuilder::build().  That's ok,
this was occuring before, just now refactored to its own file in rendering/svg.

ReferenceFilterRenderer::build() also calls ReferenceFilterBuilder::build(), as
does ReferenceFilterRenderer::computeFilter().

ReferenceFilterRenderer::computeFilter() is only (AFAICT) called here, from
RenderLayer::computeFilterOperations().

But the latter is called by
RenderLayerBacking::updateOrRemoveFilterEffectRenderer(), just prior to calling
FilterEffectRenderer::build(), which looks like it'll do the build again, and
also by RenderLayerBacking::updateFilters(), which looks like (although I'm not
certain), it'll do the build again before handing it off to the graphics layer.

It seems like we just have too many callsites for this builder code.  It may
work, but the ordering is not clear from the code, and doesn't look performant.

[achicu, 2013/04/16 22:11:16]

FilterEffectRenderer::build will only be called when used in software mode. So
the call-path is like this:
1. Resolving the filter operations would update the Filter tree in the
ReferenceFilterOperations
2. GraphicsLayer will try to apply the filter
3. If that fails, it will recompute the filter in the
FilterEffectRenderer::build. This is when the filter is recomputed.

For now we always have to rebuild the SVG filter reference because there's no
way to know if the referenced filter changed or not. If I don't rebuild the
filter every time, then cases when one attribute changes are not covered. I
suppose in a different patch, I could set a flag from the filter tree, saying
that the filters need a rebuild.
I guess the current patch is not changing how many times the filter get rebuild,
it's just extracting functionality out FilterEffectRenderer::build.

+        }
+        default:
+            // All the other filters don't need to be computed.
+            break;
+        }
         outputFilters.operations().append(filterOperation.release());
     }
     return outputFilters;
-#endif
 }
 
 void RenderLayer::updateOrRemoveFilterClients()
@@ -6266,14 +6264,7 @@ void RenderLayer::updateOrRemoveFilterEffectRenderer()
         if (RenderLayerFilterInfo* filterInfo = this->filterInfo())
             filterInfo->setRenderer(0);
 
-        // FIXME: Accelerated SVG reference filters shouldn't rely on FilterEffectRenderer to build the filter graph.
-        // https://bugs.webkit.org/show_bug.cgi?id=114051
-
-        // Early-return only if we *don't* have reference filters.
-        // For reference filters, we still want the FilterEffect graph built
-        // for us, even if we're composited.
-        if (!renderer()->style()->filter().hasReferenceFilter())
-            return;
+        return;
     }
     
     RenderLayerFilterInfo* filterInfo = ensureFilterInfo();