[Webkit-unassigned] [Bug 52200] Small filter primitive renderer improvements

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52200





--- Comment #18 from Dirk Schulze <krit at webkit.org>  2011-01-19 05:15:06 PST ---
(In reply to comment #17)
> Krit, you asked me not to introduce unused functions, but you want to see the work in small steps in the same time. This is impossible because of the dependencies.
I did not ask you to delete more functions?

> 
> I try to explain you the concept.
> 
> The issue:
> 
>    Filter     1 --- 1  Filter Renderer         1 --- n  List of FilterBuilder
>       | 1 - n               | 1 - n                              | 1 - n
> FilterElement 1 --- 1 Filter Primitve Renderer              FilterEffects
> 
> When an attribute of the FilterElement changes, it needs to update its FilterEffects (created by this FilterElement before).
Right.

> 
> Since only the FilterRenderer access the currently existing FilterEffects, all FilterElements needs to implement this function:
> 
> void SVGFE*::setFilterEffectAttribute(FilterEffect* effect, const QualifiedName& attrName)
> 
> The function is called when we call setAllFilterEffectsAttribute(attrName) in svgAttributeChanged (thus we can put the assert to SVGFilterPrimitiveStandardAttributes.cpp:76)
Ok, looks like I misunderstand your code. For clarification:
* An attribute of an effect changes state, lets say surface-scale on the diffuseLigthning effect.
* SVGFEDiffuseLightingElement::svgAttributeChanged calls setAllFilterEffectsAttribute(attrName)
* inline void setAllFilterEffectsAttribute calls setAllFilterEffectsAttribute(primitiveRenderer, attribute); of the parent renderer RenderSVGFilterResource (primitiveRenderer is the renderer of the effect element)
* here we run through all FilterBuilder and call setFilterEffectAttribute(effect, attribute); in the DiffuseLightning element of above for setting the new value in FilterEffect*, and clear all results of all effects.

A bit confusing. But the problem is that we can have different instances of FilterEffect* for one SVGFE*Element and its RenderSVGResourceFilterPrimitive :-/

I'd like to change your concept a bit:

* SVGFEDiffuseLightingElement::svgAttributeChanged calls renderer()->primitiveAttributeChanged(attrName);
* RenderSVGResourceFilterPrimitive::primitiveAttributeChanged(const QualifiedName& attrName) calls parent()->primitveAttributeChanged(this, const QualifiedName& attrName);
* RenderSVGResourceFilter::primitveAttributeChanged(const RenderSVGResourceFilterPrimitive* renderer, const QualifiedName& attrName) is basically your RenderSVGResourceFilter::setAllFilterEffectsAttribute

Would that work? I think it is better to do it that way instead of calling a function in SVGFilterPrimitiveAttr.

> 
> Note: the other way, accessing the FilterElement from a FilterEffect would cause layering violation.
Would this even be possible? Nevertheless this is not the way to go, since FilterEffect must be independent of SVG. - Ah, thats what you mean with layering violation :-P

> 
> This patch demonstrates the concept of updating the FilterEffects when the diffuseConstant of DiffuseLighting updates (and invalidates all filter images dependent of this particular DiffuseLighting FilterEffect - this is done automatically). So the FilterBuilders are not changed at all, only some attributes changed, and some existing images freed.
Yes. And the last point confused me, but now I understand your concept. There is still a problem I guess. What if an attribute causes the smallest effect rect to be bigger? E.g. The stdDeviation of feGaussianBlur increases. Doesn't it mean, that the results of determineFilterPrimitiveSubregion() are wrong? Doesn't this cause an invalidation, because our code in RenderSVGResourceFilter::applyResource() depends on these results? Something I haven't thought about yet :-(

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.