Bug 49863 – FilterPrimitives should have a pointer to its Filter object

• Component: SVG
• Status: RESOLVED FIXED

[Renata Hodovan]

FilterPrimitives should have a renderer, and they need a member pointer to their Filter objects for Layout() calls.

[Renata Hodovan]

Created attachment 74479 [details]
Patch

[Dirk Schulze]

Comment on attachment 74479 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=74479&action=review

Great to see progress! The patch looks good. r=me

Just one note:

> WebCore/platform/graphics/filters/FEDisplacementMap.cpp:112
> +    float scaleX = filter()->applyHorizontalScale(m_scale / 255);
> +    float scaleY = filter()->applyVerticalScale(m_scale / 255);
> +    float scaleAdjustmentX = filter()->applyHorizontalScale(0.5f - 0.5f * m_scale);
> +    float scaleAdjustmentY = filter()->applyVerticalScale(0.5f - 0.5f * m_scale);
>      int stride = imageRect.width() * 4;

Can you store them in a local var? Filter* filter = this->filter() ? The same on several other places where you call filter() multiple times.

[Renata Hodovan]

(In reply to comment #2)
> (From update of attachment 74479 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=74479&action=review
> 
> Great to see progress! The patch looks good. r=me
> 
> Just one note:
> 
> > WebCore/platform/graphics/filters/FEDisplacementMap.cpp:112
> > +    float scaleX = filter()->applyHorizontalScale(m_scale / 255);
> > +    float scaleY = filter()->applyVerticalScale(m_scale / 255);
> > +    float scaleAdjustmentX = filter()->applyHorizontalScale(0.5f - 0.5f * m_scale);
> > +    float scaleAdjustmentY = filter()->applyVerticalScale(0.5f - 0.5f * m_scale);
> >      int stride = imageRect.width() * 4;
> 
> Can you store them in a local var? Filter* filter = this->filter() ? The same on several other places where you call filter() multiple times.

It's done and landed.

[Renata Hodovan]

This is landed in <http://trac.webkit.org/changeset/72474>

[Nikolas Zimmermann]

Comment on attachment 74479 [details]
Patch

Is the Filter pointer always guaranteed to be alive for the whole life cycle of a FilterEffect class?
Filter is RefCounted, and I want to be sure that someone holds the ref to the Filter while FilterEffects are built.

Is there any time we could reset the m_filter to 0? Currently m_filter is only initialized, never cleared anywhere, so I'd like to be sure that its guarenteed to live longer than the FilterEffects...

[Dirk Schulze]

(In reply to comment #5)
> (From update of attachment 74479 [details])
> Is the Filter pointer always guaranteed to be alive for the whole life cycle of a FilterEffect class?
> Filter is RefCounted, and I want to be sure that someone holds the ref to the Filter while FilterEffects are built.
> 
> Is there any time we could reset the m_filter to 0? Currently m_filter is only initialized, never cleared anywhere, so I'd like to be sure that its guarenteed to live longer than the FilterEffects...

FilterBuilder (holds all effects) and Filter are stored in FilterData on RenderSVGResourceFilter and get cleared together.