flutter_svg icon indicating copy to clipboard operation
flutter_svg copied to clipboard

Improve caching

Open dnfield opened this issue 7 years ago • 2 comments

  • [ ] Make caching depend on picture size rather than number of pictures; depends on flutter/engine#5378
  • [ ] Make caching strategy injectable/swappable

dnfield avatar Jun 12 '18 15:06 dnfield

I would suggest that there's an inherent issue with the assumptions of the implementation of SvgPicture's cache - it's optimized for an svg that doesn't expect to change, such that if we do a fade animation on an svg, it reloads the asset for every frame of the animation, and caches the output of the newly colored render. What we usually want is for the base image to be loaded and cached, and then for each render apply the color tint. (We rarely move an svg, and frequently color-fade them.)

Optionally it could cache both the base image and the tinted image, but caching only the part we don't reuse, and reloading and rerendering from scratch for every color-frame is a really weird behavior that won't be great for performance.

There are four layers it could cache at:

  1. cache the loaded data, so we only load once. There is ~never a need to invalidate this (maybe if it came from network such that the file might change).
  2. cache some kind of pre-parsed loaded data, the Picture, in Dart terms. There is never a need to invalidate this, but I'm not sure if there's an appropriate storage structure for it because it's not using a Picture. Maybe the DrawableRoot?
  3. cache the rendered base image. This is invalidated if we change size.
  4. cache the rendered colored image. This is invalidated if we change size or color.

I believe the SvgPicture implementation uses option 4, and only option 4, so if we animate fading between color values then for each frame we reload, reparse, rerender, reapply the tint, cache that image, and finally paint it onto the display one time.

(I haven't dug into it super deeply; the reason I believe option 4 is what's happening is that in a unit test, if you change the color of an svg you have to waitForPendingImages to get a subsequent screenshot to show the updated color, which implies that some out-of-process operation is happening. We can get the same effect by wrapping an uncolored svg in a ColorFiltered widget, which allows for animating the color without waitForPendingImages, which is approximately equivalent to using caching option 3.)

ravenblackx avatar Sep 11 '20 17:09 ravenblackx

You're right that there are a bunch of problems here, but you might want to consider just using a ColorFilter widget around the SvgPicture rather than providing a color/color filter parameter to the SVG.

dnfield avatar Sep 11 '20 18:09 dnfield