Bug 106158 - Change SVG image cache to be per-size instead of per-renderer
Summary: Change SVG image cache to be per-size instead of per-renderer
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 420+
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philip Rogers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-04 18:19 PST by Philip Rogers
Modified: 2013-02-18 17:26 PST (History)
4 users (show)

See Also:


Attachments
Preview patch - move SVGImageCache into SVGImage (36.69 KB, patch)
2013-01-10 23:41 PST, Philip Rogers
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Rogers 2013-01-04 18:19:48 PST
The SVG image cache is currently per-renderer which makes it almost useless. Consider this example of 60 <img> tags with an SVG tiger:
http://philbit.com/SvgImagePerformance/image.html

We can knock this benchmark out of the park if we change the cache to be global or per-image-resource.
Comment 1 Philip Rogers 2013-01-10 23:41:15 PST
Created attachment 182273 [details]
Preview patch - move SVGImageCache into SVGImage

Not yet ready for review.
Comment 2 Philip Rogers 2013-01-13 17:21:26 PST
Update for posterity:
A caveat of caching per-size instead of per-renderer is we cannot have two users of an image rendering at different animation states. This is already our behavior but I don't think it has ever been explicitly stated (and it's untested).

I talked with dholbert on IRC and Mozilla's interpretation of the spec is that there is a single document animation timeline that all image renderers use. Therefore, like us, Firefox does not support different animation states. Opera does support different animation states, and IE10 does not support SMIL animations at all. Caching per-size will maintain our current behavior and we will continue to match Firefox.
Comment 3 Philip Rogers 2013-01-13 18:58:21 PST
Thorton (via email) pointed out that Nikolas' original SVGImageCache patch mentions a related issue:
// FIXME: This doesn't work correctly with animations. If an image contains animations, that say run for 2 seconds,
// and we currently have one <img> that displays us. If we open another document referencing the same SVGImage it
// will display the document at a time where animations already ran - even though it has its own ImageBuffer.
// We currently don't implement SVGSVGElement::setCurrentTime, and can NOT go back in time, once animations started.
// There's no way to fix this besides avoiding style/attribute mutations from SVGAnimationElement.

This is slightly different than the situation I was thinking of (it's multiple documents referencing a single image instead of multiple <img>s referencing a single cached image), but it will affect our cache. A solution is to simply track the document as part of the key in our cache.

Firefox also seems to be affected by this so I filed a bug: https://bugzilla.mozilla.org/show_bug.cgi?id=830211