cc: Optimize ImageDecodeCacheKey::FromDrawImage.

cc/tiles/software_image_decode_cache.cc

Index: cc/tiles/software_image_decode_cache.cc
diff --git a/cc/tiles/software_image_decode_cache.cc b/cc/tiles/software_image_decode_cache.cc
index aa2066592517f744eaaaa1020d2ce9bf5d26c61d..53758c02552fe6e00642649291ca228f984df56b 100644
--- a/cc/tiles/software_image_decode_cache.cc
+++ b/cc/tiles/software_image_decode_cache.cc
@@ -51,7 +51,10 @@ const size_t kSuspendedMaxItemsInCache = 0;
 // The second part that much be true is that we cache only the needed subrect if
 // the total size needed for the subrect is at most kMemoryRatioToSubrect *
 // (size needed for the full original image).
+// Note that at least one of the dimensions has to be at least
+// kMinDimensionToSubrect before an image can breach the threshold.
 const size_t kMemoryThresholdToSubrect = 64 * 1024 * 1024;
+const int kMinDimensionToSubrect = 4 * 1024;
 const float kMemoryRatioToSubrect = 0.5f;
 
 class AutoRemoveKeyFromTaskMap {
@@ -176,6 +179,17 @@ void RecordLockExistingCachedImageHistogram(TilePriority::PriorityBin bin,
   }
 }
 
+gfx::Rect GetSrcRect(const DrawImage& image) {

[enne (OOO), 2017/02/02 00:36:41]

Does this end up being cheaper than the code that it replaces? It's just saving
the work of constructing objects that don't get inlined?

The other bit where you don't bother looking at rects for small images is
understandable to me why it would make things faster, but was curious how much
this helped.

[ericrk, 2017/02/02 00:37:56]

Can we just store this on DrawImage rather than src_rect? seems like the only
uses of src_rect are to compute this.

[vmpstr, 2017/02/02 00:52:45]

Hmm, I'm a little hesitant to do this, since there are other call sites (like
mip calculation and gpu cache). It's not clear for example that returning empty
when rects don't intersect would be a reasonable thing. src_rect as far as i've
been using it on draw image is more of an input thing.

I can try it out if you think it's better. WDYT?

[vmpstr, 2017/02/02 00:52:45]

This alone speeds up the test by about 25%. There are two things here:
1. We skip the "IsEmpty" checks, which works because we know we don't get empty
images even if we do, we know they are at least at (0, 0) (they don't go
negative), so the check on 188 covers the empty checks.
2. We skip gfx::SkIRectToRect, which showed up surprisingly hot (10% of the
test)

[ericrk, 2017/02/02 22:21:28]

I see, the full computations aren't needed everywhere.. keeping it as-is sgtm.

+  const SkIRect& src_rect = image.src_rect();

[ericrk, 2017/02/02 00:37:56]

DrawImage::src_rect() returns a "const SkIRect", not a "const SkIRect&", so this
will cause a copy - can we update it to return const&? may make this even faster
:D

[vmpstr, 2017/02/02 00:52:45]

Done, the perf results are comparable. 

[ericrk, 2017/02/02 22:21:28]

Done.

+  int x = std::max(0, src_rect.x());
+  int y = std::max(0, src_rect.y());
+  int right = std::min(image.image()->width(), src_rect.right());
+  int bottom = std::min(image.image()->height(), src_rect.bottom());
+  if (x >= right || y >= bottom)
+    return gfx::Rect();
+  return gfx::Rect(x, y, right - x, bottom - y);
+}
+
 }  // namespace
 
 SoftwareImageDecodeCache::SoftwareImageDecodeCache(
@@ -898,10 +912,7 @@ ImageDecodeCacheKey ImageDecodeCacheKey::FromDrawImage(const DrawImage& image) {
   // otherwise we might end up with uninitialized memory in the decode process.
   // Note that the scale is still unchanged and the target size is now a
   // function of the new src_rect.
-  gfx::Rect src_rect = gfx::IntersectRects(
-      gfx::SkIRectToRect(image.src_rect()),
-      gfx::Rect(image.image()->width(), image.image()->height()));
-
+  const gfx::Rect& src_rect = GetSrcRect(image);
   gfx::Size target_size(
       SkScalarRoundToInt(std::abs(src_rect.width() * scale.width())),
       SkScalarRoundToInt(std::abs(src_rect.height() * scale.height())));
@@ -950,7 +961,9 @@ ImageDecodeCacheKey ImageDecodeCacheKey::FromDrawImage(const DrawImage& image) {
   bool can_use_original_decode =
       quality == kLow_SkFilterQuality || quality == kNone_SkFilterQuality;
   bool should_use_subrect = false;
-  if (can_use_original_decode) {
+  if (can_use_original_decode &&
+      (image.image()->width() >= kMinDimensionToSubrect ||
+       image.image()->height() >= kMinDimensionToSubrect)) {
     base::CheckedNumeric<size_t> checked_original_size = 4u;
     checked_original_size *= image.image()->width();
     checked_original_size *= image.image()->height();