Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3028)

Unified Diff: cc/tiles/image_decode_controller.cc

Issue 1682803003: cc: ImageDecodes: handle low quality filters. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: remove old code Created 4 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: cc/tiles/image_decode_controller.cc
diff --git a/cc/tiles/image_decode_controller.cc b/cc/tiles/image_decode_controller.cc
index dac068c0833aca087f2cd54c05baa579cdb26d69..56a88143862d5f05003a972716c5ba5d38712b4e 100644
--- a/cc/tiles/image_decode_controller.cc
+++ b/cc/tiles/image_decode_controller.cc
@@ -78,6 +78,9 @@ typename std::deque<Type>::iterator FindImage(
}
SkSize GetScaleAdjustment(const ImageDecodeControllerKey& key) {
+ if (key.can_use_original_decode())
+ return SkSize::Make(1.f, 1.f);
+
float x_scale =
key.target_size().width() / static_cast<float>(key.src_rect().width());
float y_scale =
@@ -143,7 +146,7 @@ bool ImageDecodeController::GetTaskForImageAndRef(
// If we already have the image in cache, then we can return it.
auto decoded_it = FindImage(&decoded_images_, key);
bool new_image_fits_in_memory =
- locked_images_budget_.AvailableMemoryBytes() >= key.target_bytes();
+ locked_images_budget_.AvailableMemoryBytes() >= key.locked_bytes();
if (decoded_it != decoded_images_.end()) {
if (decoded_it->second->is_locked() ||
(new_image_fits_in_memory && decoded_it->second->Lock())) {
@@ -195,8 +198,8 @@ void ImageDecodeController::RefImage(const ImageKey& key) {
lock_.AssertAcquired();
int ref = ++decoded_images_ref_counts_[key];
if (ref == 1) {
- DCHECK_GE(locked_images_budget_.AvailableMemoryBytes(), key.target_bytes());
- locked_images_budget_.AddUsage(key.target_bytes());
+ DCHECK_GE(locked_images_budget_.AvailableMemoryBytes(), key.locked_bytes());
+ locked_images_budget_.AddUsage(key.locked_bytes());
}
}
@@ -219,7 +222,7 @@ void ImageDecodeController::UnrefImage(const DrawImage& image) {
--ref_count_it->second;
if (ref_count_it->second == 0) {
decoded_images_ref_counts_.erase(ref_count_it);
- locked_images_budget_.SubtractUsage(key.target_bytes());
+ locked_images_budget_.SubtractUsage(key.locked_bytes());
auto decoded_image_it = FindImage(&decoded_images_, key);
// If we've never decoded the image before ref reached 0, then we wouldn't
@@ -266,7 +269,7 @@ void ImageDecodeController::DecodeImage(const ImageKey& key,
scoped_refptr<DecodedImage> decoded_image;
{
base::AutoUnlock unlock(lock_);
- decoded_image = DecodeImageInternal(key, image.image());
+ decoded_image = DecodeImageInternal(key, image);
}
// Erase the pending task from the queue, since the task won't be doing
@@ -306,37 +309,108 @@ void ImageDecodeController::DecodeImage(const ImageKey& key,
scoped_refptr<ImageDecodeController::DecodedImage>
ImageDecodeController::DecodeImageInternal(const ImageKey& key,
- const SkImage* image) {
+ const DrawImage& draw_image) {
TRACE_EVENT1("disabled-by-default-cc.debug",
"ImageDecodeController::DecodeImageInternal", "key",
key.ToString());
- // Get the decoded image first (at the original scale).
- SkImageInfo decoded_info = SkImageInfo::MakeN32Premul(
- key.src_rect().width(), key.src_rect().height());
- scoped_ptr<uint8_t[]> decoded_pixels;
- {
- TRACE_EVENT0(
- "disabled-by-default-cc.debug",
- "ImageDecodeController::DecodeImageInternal - allocate decoded pixels");
- decoded_pixels.reset(
- new uint8_t[decoded_info.minRowBytes() * decoded_info.height()]);
+ const SkImage* image = draw_image.image();
+
+ // If we can use the original decode, then we don't need to do scaling. We can
+ // just read pixels into the final memory.
+ if (key.can_use_original_decode()) {
+ SkImageInfo decoded_info =
+ SkImageInfo::MakeN32Premul(image->width(), image->height());
+ scoped_ptr<base::DiscardableMemory> decoded_pixels;
+ {
+ TRACE_EVENT0("disabled-by-default-cc.debug",
+ "ImageDecodeController::DecodeImageInternal - allocate "
+ "decoded pixels");
+ decoded_pixels =
+ base::DiscardableMemoryAllocator::GetInstance()
+ ->AllocateLockedDiscardableMemory(decoded_info.minRowBytes() *
+ decoded_info.height());
+ }
+ {
+ TRACE_EVENT0("disabled-by-default-cc.debug",
+ "ImageDecodeController::DecodeImageInternal - read pixels");
+ bool result = image->readPixels(decoded_info, decoded_pixels->data(),
+ decoded_info.minRowBytes(), 0, 0,
+ SkImage::kDisallow_CachingHint);
+
+ if (!result) {
+ decoded_pixels->Unlock();
+ return nullptr;
+ }
+ }
+
+ return make_scoped_refptr(new DecodedImage(
+ decoded_info, std::move(decoded_pixels), SkSize::Make(0, 0)));
}
- {
- TRACE_EVENT0("disabled-by-default-cc.debug",
- "ImageDecodeController::DecodeImageInternal - read pixels");
- bool result = image->readPixels(
- decoded_info, decoded_pixels.get(), decoded_info.minRowBytes(),
- key.src_rect().x(), key.src_rect().y(), SkImage::kAllow_CachingHint);
- if (!result)
- return nullptr;
+ // If we get here, that means we couldn't use the original sized decode for
+ // whatever reason. However, in all cases we do need an original decode to
+ // either do a scale or to extract a subrect from the image. So, what we can
+ // do is construct a key that would require a full sized decode, then get that
+ // decode via GetDecodedImageForDrawInternal(), use it, and unref it. This
+ // ensures that if the original sized decode is already available in any of
+ // the caches, we reuse that. We also ensure that all the proper locking takes
+ // place. If, on the other hand, the decode was not available,
+ // GetDecodedImageForDrawInternal() would decode the image, and unreffing it
+ // later ensures that we will store the discardable memory unlocked in the
+ // cache to be used by future requests.
+ SkSize identity_scale = SkSize::Make(1.f, 1.f);
+ bool matrix_has_perspective = false;
+ bool matrix_is_decomposable = true;
+ gfx::Rect full_image_rect(image->width(), image->height());
+ DrawImage original_size_draw_image(
+ image, gfx::RectToSkIRect(full_image_rect), identity_scale,
+ kLow_SkFilterQuality, matrix_has_perspective, matrix_is_decomposable);
+ ImageKey original_size_key =
+ ImageKey::FromDrawImage(original_size_draw_image);
+ // Sanity checks.
+ DCHECK(original_size_key.can_use_original_decode());
+
+ auto decoded_draw_image = GetDecodedImageForDrawInternal(
+ original_size_key, original_size_draw_image);
+ if (!decoded_draw_image.image()) {
+ DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
+ return nullptr;
+ }
+
+ scoped_ptr<uint8_t[]> decoded_subrect_pixels;
+ SkPixmap decoded_pixmap;
+ bool result;
+ if (key.src_rect() == full_image_rect) {
+ result = decoded_draw_image.image()->peekPixels(&decoded_pixmap);
+ } else {
+ SkImageInfo decoded_info = SkImageInfo::MakeN32Premul(
+ key.src_rect().width(), key.src_rect().height());
+ {
+ TRACE_EVENT0("disabled-by-default-cc.debug",
+ "ImageDecodeController::DecodeImageInternal - allocate "
+ "decoded pixels");
+ decoded_subrect_pixels.reset(
+ new uint8_t[decoded_info.minRowBytes() * decoded_info.height()]);
+ }
+ {
+ TRACE_EVENT0("disabled-by-default-cc.debug",
+ "ImageDecodeController::DecodeImageInternal - read pixels");
+ result =
+ image->readPixels(decoded_info, decoded_subrect_pixels.get(),
+ decoded_info.minRowBytes(), key.src_rect().x(),
+ key.src_rect().y(), SkImage::kDisallow_CachingHint);
+ }
+ decoded_pixmap = SkPixmap(decoded_info, decoded_subrect_pixels.get(),
+ decoded_info.minRowBytes());
}
- SkPixmap decoded_pixmap(decoded_info, decoded_pixels.get(),
- decoded_info.minRowBytes());
+ // Since the decoded_draw_image has locked memory, it should always succeed on
+ // both peekPixels and readPixels.
+ DCHECK(result);
- // Now scale the pixels into the destination size.
+ // Now we have a decoded_pixmap which represents the src_rect at the original
+ // scale. All we need to do is scale it.
DCHECK(!key.target_size().IsEmpty());
SkImageInfo scaled_info = SkImageInfo::MakeN32Premul(
key.target_size().width(), key.target_size().height());
@@ -357,9 +431,15 @@ ImageDecodeController::DecodeImageInternal(const ImageKey& key,
TRACE_EVENT0("disabled-by-default-cc.debug",
"ImageDecodeController::DecodeImageInternal - scale pixels");
bool result =
- decoded_pixmap.scalePixels(scaled_pixmap, kHigh_SkFilterQuality);
+ decoded_pixmap.scalePixels(scaled_pixmap, key.filter_quality());
DCHECK(result);
}
+
+ // Release the original sized decode. Any other intermediate result to release
+ // would be the subrect memory. However, that's in a scoped_ptr and will be
+ // deleted automatically when we return.
+ DrawWithImageFinished(original_size_draw_image, decoded_draw_image);
+
return make_scoped_refptr(
new DecodedImage(scaled_info, std::move(scaled_pixels),
SkSize::Make(-key.src_rect().x(), -key.src_rect().y())));
@@ -369,7 +449,7 @@ DecodedDrawImage ImageDecodeController::GetDecodedImageForDraw(
const DrawImage& draw_image) {
ImageKey key = ImageKey::FromDrawImage(draw_image);
TRACE_EVENT1("disabled-by-default-cc.debug",
- "ImageDecodeController::GetDecodedImageAndRef", "key",
+ "ImageDecodeController::GetDecodedImageForDraw", "key",
key.ToString());
if (!CanHandleImage(key, draw_image))
return DecodedDrawImage(draw_image.image(), draw_image.filter_quality());
@@ -378,6 +458,15 @@ DecodedDrawImage ImageDecodeController::GetDecodedImageForDraw(
if (key.target_size().IsEmpty())
return DecodedDrawImage(nullptr, kNone_SkFilterQuality);
+ return GetDecodedImageForDrawInternal(key, draw_image);
+}
+
+DecodedDrawImage ImageDecodeController::GetDecodedImageForDrawInternal(
+ const ImageKey& key,
+ const DrawImage& draw_image) {
+ TRACE_EVENT1("disabled-by-default-cc.debug",
+ "ImageDecodeController::GetDecodedImageForDrawInternal", "key",
+ key.ToString());
base::AutoLock lock(lock_);
auto decoded_images_it = FindImage(&decoded_images_, key);
// If we found the image and it's locked, then return it. If it's not locked,
@@ -403,9 +492,11 @@ DecodedDrawImage ImageDecodeController::GetDecodedImageForDraw(
DCHECK(at_raster_images_it->second->is_locked());
RefAtRasterImage(key);
SanityCheckState(__LINE__, true);
+ const scoped_refptr<DecodedImage>& at_raster_decoded_image =
+ at_raster_images_it->second;
auto decoded_draw_image =
- DecodedDrawImage(at_raster_images_it->second->image(),
- at_raster_images_it->second->src_rect_offset(),
+ DecodedDrawImage(at_raster_decoded_image->image(),
+ at_raster_decoded_image->src_rect_offset(),
GetScaleAdjustment(key), kLow_SkFilterQuality);
decoded_draw_image.set_at_raster_decode(true);
return decoded_draw_image;
@@ -421,7 +512,7 @@ DecodedDrawImage ImageDecodeController::GetDecodedImageForDraw(
// on the compositor thread. This means holding on to the lock might stall
// the compositor thread for the duration of the decode!
base::AutoUnlock unlock(lock_);
- decoded_image = DecodeImageInternal(key, draw_image.image());
+ decoded_image = DecodeImageInternal(key, draw_image);
// Skip the image if we couldn't decode it.
if (!decoded_image)
@@ -548,18 +639,9 @@ bool ImageDecodeController::CanHandleImage(const ImageKey& key,
bool ImageDecodeController::CanHandleFilterQuality(
SkFilterQuality filter_quality) {
- // We don't need to handle low quality filters.
- if (filter_quality == kLow_SkFilterQuality ||
- filter_quality == kNone_SkFilterQuality) {
- return false;
- }
-
// TODO(vmpstr): We need to start caching mipmaps for medium quality and
// caching the interpolated values from those. For now, we don't have this.
- if (filter_quality == kMedium_SkFilterQuality)
- return false;
- DCHECK(filter_quality == kHigh_SkFilterQuality);
- return true;
+ return filter_quality != kMedium_SkFilterQuality;
}
void ImageDecodeController::ReduceCacheUsage() {
@@ -618,9 +700,16 @@ void ImageDecodeController::SanityCheckState(int line, bool lock_acquired) {
MemoryBudget budget(kLockedMemoryLimitBytes);
for (const auto& annotated_image : decoded_images_) {
- auto ref_it = decoded_images_ref_counts_.find(annotated_image.first);
+ DCHECK_EQ(1, std::count_if(
+ decoded_images_.begin(), decoded_images_.end(),
+ [&annotated_image](const AnnotatedDecodedImage& image) {
+ return image.first == annotated_image.first;
+ }))
+ << line;
+ auto key = annotated_image.first;
+ auto ref_it = decoded_images_ref_counts_.find(key);
if (annotated_image.second->is_locked()) {
- budget.AddUsage(annotated_image.first.target_bytes());
+ budget.AddUsage(annotated_image.first.locked_bytes());
DCHECK(ref_it != decoded_images_ref_counts_.end()) << line;
} else {
DCHECK(ref_it == decoded_images_ref_counts_.end() ||
@@ -679,27 +768,61 @@ ImageDecodeControllerKey ImageDecodeControllerKey::FromDrawImage(
}
}
- return ImageDecodeControllerKey(image.image()->uniqueID(),
- gfx::SkIRectToRect(image.src_rect()),
- target_size, quality);
+ gfx::Rect src_rect = gfx::SkIRectToRect(image.src_rect());
+ gfx::Size full_image_size(image.image()->width(), image.image()->height());
+ gfx::Rect full_image_rect(full_image_size);
+ bool scale_needs_caching =
+ quality != kLow_SkFilterQuality && quality != kNone_SkFilterQuality;
+ bool is_full_image_rect = full_image_rect == src_rect;
+ bool scale_is_required = src_rect.width() != target_size.width() ||
+ src_rect.height() != target_size.height();
+ bool can_use_original_decode =
+ !scale_needs_caching && (is_full_image_rect || !scale_is_required);
+
+ return ImageDecodeControllerKey(image.image()->uniqueID(), src_rect,
+ target_size, quality, can_use_original_decode,
+ full_image_size);
}
ImageDecodeControllerKey::ImageDecodeControllerKey(
uint32_t image_id,
const gfx::Rect& src_rect,
const gfx::Size& target_size,
- SkFilterQuality filter_quality)
+ SkFilterQuality filter_quality,
+ bool can_use_original_decode,
+ const gfx::Size& original_size)
: image_id_(image_id),
src_rect_(src_rect),
target_size_(target_size),
- filter_quality_(filter_quality) {}
+ filter_quality_(filter_quality),
+ can_use_original_decode_(can_use_original_decode),
+ original_size_(original_size) {
+ if (can_use_original_decode_) {
+ hash_ = image_id_;
enne (OOO) 2016/02/11 00:06:06 Do you need to hash this to get a good distributio
vmpstr 2016/02/11 00:23:46 Sure :D
+ } else {
+ // TODO(vmpstr): This is a mess. Maybe it's faster to just search the vector
+ // always (forwards or backwards to account for LRU).
+ uint64_t src_rect_hash = base::HashInts(
+ static_cast<uint64_t>(base::HashInts(src_rect_.x(), src_rect_.y())),
+ static_cast<uint64_t>(
+ base::HashInts(src_rect_.width(), src_rect_.height())));
+
+ uint64_t target_size_hash =
+ base::HashInts(target_size_.width(), target_size_.height());
+
+ hash_ = base::HashInts(base::HashInts(src_rect_hash, target_size_hash),
+ base::HashInts(image_id_, filter_quality_));
+ }
+}
std::string ImageDecodeControllerKey::ToString() const {
std::ostringstream str;
str << "id[" << image_id_ << "] src_rect[" << src_rect_.x() << ","
<< src_rect_.y() << " " << src_rect_.width() << "x" << src_rect_.height()
<< "] target_size[" << target_size_.width() << "x"
- << target_size_.height() << "] filter_quality[" << filter_quality_ << "]";
+ << target_size_.height() << "] filter_quality[" << filter_quality_
+ << "] can_use_original_decode [" << can_use_original_decode_ << "] hash ["
+ << hash_ << "]";
return str.str();
}

Powered by Google App Engine
This is Rietveld 408576698