pyface icon indicating copy to clipboard operation
pyface copied to clipboard

Rationalize image searching and caching

Open corranwebster opened this issue 3 years ago • 0 comments

The current image infrastructure has a number of different caches for images:

  • the Image trait caches str -> ImageResource lookups: https://github.com/enthought/pyface/blob/2dae9b473ea05df4917919582f52c590e4d1dceb/pyface/ui_traits.py#L57
  • the convert_bitmap function in pyface.ui_traits also creates a cache of ImageResource -> bitmap lookups, but does not specify sizes, so gets whatever the natural size of the image is: https://github.com/enthought/pyface/blob/2dae9b473ea05df4917919582f52c590e4d1dceb/pyface/ui_traits.py#L81
  • there is the IImageCache class used by Action instances to cache filename -> image/bitmap lookups for images of a particular size: https://github.com/enthought/pyface/blob/2dae9b473ea05df4917919582f52c590e4d1dceb/pyface/i_image_cache.py#L17-L70

There may well be others.

There is a certain amount of inefficiency in all of this:

  • ImageCache instance are unique for each ToolbarManager and so are only caching images for a particular toolbar widget; in particular different windows with the same toolbar structure won't share caches, even though they may have all the same images.
  • To create an image for the ImageCache, code currently calls ImageResource.create_image, ignores the result but grabs the path which is now populated, and passes that to the ImageCache which re-loads the image and scales it, despite the fact that ImageResource has code to search for similar images of different sizes (using "images/WxH" subdirectories).
  • worse, ImageResource stores a private _ref, which is a reference to an image file, but sizes may have multiple files, so the _ref is only good for one size (I think the last requested), so the absolute path used by the ImageCache is not even unique.

This doesn't even consider ImageLibrary instances.

This all needs to be simplified and rationalized and made so that it actually caches effectively.

corranwebster avatar Mar 25 '21 13:03 corranwebster