New issue
Advanced search Search tips

Issue 753639 link

Starred by 1 user
Status: Fixed
Owner:
khushals...@chromium.org
Closed: Sep 2017
Cc:
vmp...@chromium.org
ericrk@chromium.org
cblume@chromium.org
enne@chromium.org
khushals...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Remove use of SkImage uniqueIDs for image decode caching.

Project Member Reported by khushals...@chromium.org, Aug 9 2017 
Currently caching of image decodes is keyed on the SkImage uniqueID for an image, both in cc and skia. As we remove the use of SkImageGenerator backed images to transport lazy decoded images from blink to cc, these images are no longer represented as SkImages in PaintImage.

In order to eliminate the need for these uniqueIDs, 2 things need to be fixed:

1) cc's decode cache key: The cache key used for an image frame needs to be based on an id that includes (paint_image_id, content_id, frame_index). The content_id is necessary to distinguish partially decoded frames of the same image.

2) remove skia cache usage: For images not being decoded in cc, i.e., encoded images in PaintRecords in PaintShaders/PaintImages, skia caches the decode using this uniqueID. Once we capture all images in cc, this will no longer be necessary.
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 25 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/653954392a8a6b43036f22fb402ce76a9d55c262

commit 653954392a8a6b43036f22fb402ce76a9d55c262
Author: Khushal <khushalsagar@chromium.org>
Date: Mon Sep 25 22:20:06 2017

blink: Remove uniqueID caching for SkImages created by BitmapImage.

Currently blink caches SkImage uniqueIDs created by BitmapImage to
ensure reuse of cached decoded data for the frames of this image. The 2
systems that can cache the decode are cc and skia.

1) cc: The image decode cache in cc uses PaintImage::FrameKey to cache
the decodes and does not rely on SkImage uniqueIDs anymore.

2) skia: Skia's decode caching is tied to the lifetime of the SkImage,
which invalidates skia's cache in its dtor. This means that reusing the
same uniqueID will still result in purging of the decode from skia's
cache.

Also note that the only case where the reuse of uniqueID is important
is for each frame of a multiframe image. For static images,
BitmapImage already ensures caching of the decode by storing the
PaintImage which internally keeps a reference to the SkImage, and is
cleared in DestroyDecodedData. All cases which require animation of
images go through cc and use cc's image decode cache. The few cases,
for instance Canvas2D, which decode on the main thread and use skia's
cache only use the first frame of an animated image.

R=chrishtr@chromium.org, fmalita@chromium.org

Bug:  753639 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I78dcf84a8dad25c7742526af31c05ee381da7cf4
Reviewed-on: https://chromium-review.googlesource.com/677916
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504185}
[modify] https://crrev.com/653954392a8a6b43036f22fb402ce76a9d55c262/cc/paint/paint_image.cc
[modify] https://crrev.com/653954392a8a6b43036f22fb402ce76a9d55c262/cc/paint/paint_image.h
[modify] https://crrev.com/653954392a8a6b43036f22fb402ce76a9d55c262/cc/paint/paint_image_builder.h
[modify] https://crrev.com/653954392a8a6b43036f22fb402ce76a9d55c262/cc/paint/skia_paint_image_generator.cc
[modify] https://crrev.com/653954392a8a6b43036f22fb402ce76a9d55c262/cc/paint/skia_paint_image_generator.h
[modify] https://crrev.com/653954392a8a6b43036f22fb402ce76a9d55c262/third_party/WebKit/Source/platform/graphics/BitmapImage.cpp
[modify] https://crrev.com/653954392a8a6b43036f22fb402ce76a9d55c262/third_party/WebKit/Source/platform/graphics/BitmapImageTest.cpp
[modify] https://crrev.com/653954392a8a6b43036f22fb402ce76a9d55c262/third_party/WebKit/Source/platform/graphics/FrameData.h

Comment 2 by khushals...@chromium.org, Sep 25 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment