Bug 19388

Summary: Filter Effects being cleared and reloaded multiple times
Product: WebKit Reporter: Alex Mathews <possessedpenguinbob>
Component: SVGAssignee: Alex Mathews <possessedpenguinbob>
Severity: Minor CC: eric, jeffschiller, krit, oliver
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
URL: http://www.w3.org/TR/SVG11/images/filters/filters01.svg
Bug Depends on: 33091    
Bug Blocks: 68469, 26389    
Description Flags
Possible patch for eseidel/MacDome??
Calculate the filters once, not on every call af applyFilter none

Description Alex Mathews 2008-06-03 21:28:24 PDT
On close inspection the filter effects for each filter were being cleared and added to the  Vector<SVGFilterEffect*> in SVGResourceFilter every time a styled svg element is found, I believe.

To find this I added a printf telling me what the values of the effect's "in" attribute are as they go through SVGResourceFilter::addFilterEffect() and looked at the output from an example svg filter image:


Though it is probably easier to just set a breakpoint in the addFilterEffect function and debug it.

The output I got when running through that example svg with my printf was:

(null impl)       <-- that is just because the feMerge doesn't have an "in" attribute

7 times total.
Comment 1 Alex Mathews 2008-06-03 21:34:49 PDT
Created attachment 21494 [details]
Possible patch for eseidel/MacDome??

If I am indeed correct and this is not the correct behavior, checking for whether or not the vector is populated before clearing it and repopulating it provides a fix.
Comment 2 Eric Seidel (no email) 2008-06-05 07:02:48 PDT
I'll look at this again later today.  I'll need to stare at the exiting code a bit.
Comment 3 Alex Mathews 2008-06-06 14:00:29 PDT
Had a talk with Eric about this and while it is not really correct behavior, for the moment it will stand. My patch would break many things because it doesn't take into account the fact that filter effects can be added and removed as well as individual attributes changed. 

So while the current behavior isn't very good performance wise, as it reloads the filter before each call to the filter whether or not it needs to, it allows for the filter to change dynamically. 

Possible fixes include adding the ability for filter effects to force a reload of the filter when their attributes change or when they are added or removed. Though in the grand scheme of things I would like to make <filter>s as smart as possible to reduce the amount of reloading needs to be done on changes.
Comment 4 Dirk Schulze 2010-01-01 01:25:20 PST
Created attachment 45729 [details]
Calculate the filters once, not on every call af applyFilter

This patch does not only create the filter effects once, they are also just calculated onece (simuliar to SVG Masker). This is a massive speed up of filters. Not on the first calculation, but on the scrolling or animations. SVGResourceFilter would depend on bug 33091 like SVGResourceMasker with this bug.
Comment 5 Dirk Schulze 2010-07-01 02:27:04 PDT
This was fixed with moving the filter code to a renderer. Closing the bug.