Bug 49863

Summary: FilterPrimitives should have a pointer to its Filter object
Product: WebKit Reporter: Renata Hodovan <rhodovan.u-szeged>
Component: SVGAssignee: Renata Hodovan <rhodovan.u-szeged>
Status: RESOLVED FIXED    
Severity: Normal CC: kling, krit, mdelaney7, zherczeg, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 68469, 26389    
Attachments:
Description Flags
Patch krit: review+

Renata Hodovan
Reported 2010-11-20 03:52:37 PST
FilterPrimitives should have a renderer, and they need a member pointer to their Filter objects for Layout() calls.
Attachments
Patch (100.13 KB, patch)
2010-11-20 04:19 PST, Renata Hodovan 		krit: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Renata Hodovan
Comment 1 2010-11-20 04:19:11 PST
Created attachment 74479 [details] Patch
Dirk Schulze
Comment 2 2010-11-20 04:55:31 PST
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
Comment 3 2010-11-20 06:22:03 PST
(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
Comment 4 2010-11-20 07:50:17 PST
This is landed in <http://trac.webkit.org/changeset/72474>
Nikolas Zimmermann
Comment 5 2010-11-20 09:19:01 PST
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
Comment 6 2010-11-20 09:38:05 PST
(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.
Note You need to log in before you can comment on or make changes to this bug.