Bug 99785 – [chromium] Improve cached image indexing in ImageDecodingStore

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: Images

[Hin-Chung Lam]

After https://bugs.webkit.org/show_bug.cgi?id=94240 is landed. ImageDecodingStore becomes the infrastructure for a centralized image cache.

There are a couple things can be improved:
1. Better decoupling ImageFrameGenerator from ImageDecodingStore
2. Better indexing of image scale

[Hin-Chung Lam]

Let's start the discussion here for the details of patching ImageDecodingStore.

Problem 1 - Decouple ImageFrameGenerator from ImageDecodingStore

ImageDecodingStore::lockPixels() take a pointer to ImageFrameGenerator. This means lockPixel() does two functions 1) lookup for scaled image fragment 2) generation of image fragment as needed.

These two tasks should be better isolated. Which means we should have a cache lookup function in ImageDecodingStore such that LazyDecodingPixelRef can call, and then a function to save a scaled image fragment to the cache. Something like:

SkBitmap ImageDecodingStore::cacheLookup(int imageId, SkISize scaledSize, SkIRect scaledFragment);
void ImageDecodingStore::saveImageFragment(int imageId, SkISize, SkIRect, SkBitmap);

Attention needs to be taken such that there can be only one SkBitmap being locked. This should be the case as this code is running on a single thread.

Problem 2 - Efficient cache lookup

This problem is the choice of data structure, key for indexing the data structure and what to cache.

ImageDecodingStore now always save the full size image to the cache. We should investigate whether to cache the entire scaled image or just the fragment. And then decide data structure (like map) to store image fragments.

[Stephen White]

Here's what I'd like to see:  In order to decouple cacheing from deferred decoding, access the ScaledImageFragment cache directly from NativeImageSkia.  Something like this:

- from NativeImageSkia::resizedBitmap():
  - look up a ScaledImageFragment by hash(NativeImageSkia* + scaledImageSize + scaledImageSubset)
  - if it's in cache, use it
  - if not, check for deferred image decoded pixelref
    - if there:
      - decode it (if necessary), resize it and populate the cache with a new ScaledImageFragment
    - if not, deferred decoding is off, and we have a full-size already decoded in the pixelref
      - resize it and populate the cache with a new ScaledImageFragment

[Hin-Chung Lam]

Nick is going to take charge of the area of caching. I would like to hear his comments on restructuring this bit of code.

[Hin-Chung Lam]

Mark this as duplicated. Combining this bug with 99784.

*** This bug has been marked as a duplicate of bug 99784 ***