Bug 99785 - [chromium] Improve cached image indexing in ImageDecodingStore
Summary: [chromium] Improve cached image indexing in ImageDecodingStore
Status: RESOLVED DUPLICATE of bug 99784
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nick Carter
URL:
Keywords:
Depends on: 94240 102019 102021
Blocks: 99784 99790
  Show dependency treegraph
 
Reported: 2012-10-18 18:08 PDT by Hin-Chung Lam
Modified: 2012-11-25 22:01 PST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hin-Chung Lam 2012-10-18 18:08:43 PDT
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
Comment 1 Hin-Chung Lam 2012-10-18 18:17:23 PDT
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.
Comment 2 Stephen White 2012-10-19 08:01:20 PDT
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
Comment 3 Hin-Chung Lam 2012-10-19 14:07:27 PDT
Nick is going to take charge of the area of caching. I would like to hear his comments on restructuring this bit of code.
Comment 4 Hin-Chung Lam 2012-11-25 22:01:54 PST
Mark this as duplicated. Combining this bug with 99784.

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