Summary: | SVGPaintServerPatternCg should cache CGPatternRef (m_pattern) for speed | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||||||
Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | mitz | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 420+ | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2006-12-21 04:03:54 PST
Created attachment 12058 [details]
Initial patch
No new regressions -> CGPatternRef's are properly cached now.
Comment on attachment 12058 [details]
Initial patch
Looks like it's leaking the last pattern and space used when the SVGPaintServerPattern is destroyed. Also, isn't it possible for an ImageBuffer to be shared by two SVGPaintServerPatterns?
(In reply to comment #3) > (From update of attachment 12058 [details] [edit]) > Also, isn't it possible for an ImageBuffer > to be shared by two SVGPaintServerPatterns? > Looking at the current code, it does indeed seem impossible, so keeping the flag on the ImageBuffer should work. However, having the "contents changed" flag on the ImageBuffer still doesn't feel right for a general ImageBuffer object. As far as I can see, the current code always creates the ImageBuffer, renders into it and then calls setTile, and never draws to the ImageBuffer afterwards, let alone does that without calling setTile again. In this case, it is sufficient (as far as image buffer contents are concerned; see below) for the paint server to invalidate its cache only in setTile(). I noticed that the cached CGPattern depends on the bbox and the tile's size, but you don't invalidate the cache in setBBox(). Is that (also) guaranteed not to be called after setTile()? Created attachment 12062 [details]
Updated patch
Mitz is correct, fixed the issues he mentioned.
Created attachment 12063 [details]
Final patch
Seperating the alpha handling changes, as they don't belong in this patchset.
Created attachment 12064 [details]
Final patch (II)
Eric mentioned some more optimizations. Fixed.
Comment on attachment 12064 [details]
Final patch (II)
We discussed a couple further optimizations. Wow what a pretty patch. :)
Will revert this patch again, as it broke pattern zooming. Either we need a more clever logic for the caching or just don't cache. Needs to be sharked to actually decide it. |