cc: Replace recorded region with direct map lookup

cc/resources/picture_pile_base.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/picture_pile_base.h"
 
 #include <algorithm>
 #include <set>
 #include <vector>
 
@@ skipping 26 matching lines @@
 }  // namespace
 
 namespace cc {
 
 PicturePileBase::PicturePileBase()
     : min_contents_scale_(0),
       background_color_(SkColorSetARGBInline(0, 0, 0, 0)),
       slow_down_raster_scale_factor_for_debug_(0),
       contents_opaque_(false),
       show_debug_picture_borders_(false),
-      clear_canvas_with_debug_color_(kDefaultClearCanvasSetting) {
+      clear_canvas_with_debug_color_(kDefaultClearCanvasSetting),
+      has_any_recordings_(false) {
   tiling_.SetMaxTextureSize(gfx::Size(kBasePictureSize, kBasePictureSize));
   tile_grid_info_.fTileInterval.setEmpty();
   tile_grid_info_.fMargin.setEmpty();
   tile_grid_info_.fOffset.setZero();
 }
 
 PicturePileBase::PicturePileBase(const PicturePileBase* other)
     : picture_map_(other->picture_map_),
       tiling_(other->tiling_),
-      recorded_region_(other->recorded_region_),
+      recorded_viewport_(other->recorded_viewport_),
       min_contents_scale_(other->min_contents_scale_),
       tile_grid_info_(other->tile_grid_info_),
       background_color_(other->background_color_),
       slow_down_raster_scale_factor_for_debug_(
           other->slow_down_raster_scale_factor_for_debug_),
       contents_opaque_(other->contents_opaque_),
       show_debug_picture_borders_(other->show_debug_picture_borders_),
-      clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_) {
-}
+      clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_),
+      has_any_recordings_(other->has_any_recordings_) {}
 
-PicturePileBase::PicturePileBase(
-    const PicturePileBase* other, unsigned thread_index)
+PicturePileBase::PicturePileBase(const PicturePileBase* other,
+                                 unsigned thread_index)
     : tiling_(other->tiling_),
-      recorded_region_(other->recorded_region_),
+      recorded_viewport_(other->recorded_viewport_),
       min_contents_scale_(other->min_contents_scale_),
       tile_grid_info_(other->tile_grid_info_),
       background_color_(other->background_color_),
       slow_down_raster_scale_factor_for_debug_(
           other->slow_down_raster_scale_factor_for_debug_),
       contents_opaque_(other->contents_opaque_),
       show_debug_picture_borders_(other->show_debug_picture_borders_),
-      clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_) {
+      clear_canvas_with_debug_color_(other->clear_canvas_with_debug_color_),
+      has_any_recordings_(other->has_any_recordings_) {
   for (PictureMap::const_iterator it = other->picture_map_.begin();
        it != other->picture_map_.end();
        ++it) {
     picture_map_[it->first] = it->second.CloneForThread(thread_index);
   }
 }
 
 PicturePileBase::~PicturePileBase() {
 }
 
@@ skipping 14 matching lines @@
        it != picture_map_.end();
        ++it) {
     const PictureMapKey& key = it->first;
     if (key.first < min_toss_x && key.second < min_toss_y)
       continue;
     to_erase.push_back(key);
   }
 
   for (size_t i = 0; i < to_erase.size(); ++i)
     picture_map_.erase(to_erase[i]);
+
+  // Don't waste time in Resize figuring out what these hints should be.

[vmpstr, 2014/03/12 23:57:35]

Is this because Resize is (always?) followed by Update? What I mean to say is
that it's not that expensive to recompute this, since we're already walking
through the whole picture_map_, but if we always follow-up with Update, then I
can see how that time is wasted.

[enne (OOO), 2014/03/13 01:35:43]

Well, calculating what the "viewport" here is would require rounding the resized
viewport rect to tile boundaries.  I just didn't want to do the math or figure
out what the right rect was exactly.

+  recorded_viewport_ = gfx::Rect();
+  if (new_size.IsEmpty())
+    has_any_recordings_ = false;

[vmpstr, 2014/03/12 23:57:35]

This one in particular, I think if you add a has_any_recordings_ |=
it->second.GetPicture() where the continue is on line 110 would eliminate some
false positives.

[enne (OOO), 2014/03/13 01:35:43]

Done.

 }
 
 void PicturePileBase::SetMinContentsScale(float min_contents_scale) {
   DCHECK(min_contents_scale);
   if (min_contents_scale_ == min_contents_scale)
     return;
 
   // Picture contents are played back scaled. When the final contents scale is
   // less than 1 (i.e. low res), then multiple recorded pixels will be used
   // to raster one final pixel.  To avoid splitting a final pixel across
@@ skipping 35 matching lines @@
     return;
 
   Clear();
   tiling_.SetBorderTexels(new_buffer_pixels);
 }
 
 void PicturePileBase::Clear() {
   picture_map_.clear();
 }
 
-void PicturePileBase::UpdateRecordedRegion() {
-  recorded_region_.Clear();
-  for (PictureMap::const_iterator it = picture_map_.begin();
-       it != picture_map_.end();
-       ++it) {
-    if (it->second.GetPicture()) {
-      const PictureMapKey& key = it->first;
-      recorded_region_.Union(tile_bounds(key.first, key.second));
-    }
-  }
-}
-
 bool PicturePileBase::HasRecordingAt(int x, int y) {
   PictureMap::const_iterator found = picture_map_.find(PictureMapKey(x, y));
   if (found == picture_map_.end())
     return false;
   return !!found->second.GetPicture();
 }
 
 bool PicturePileBase::CanRaster(float contents_scale,
                                 const gfx::Rect& content_rect) {
   if (tiling_.total_size().IsEmpty())
     return false;
   gfx::Rect layer_rect = gfx::ScaleToEnclosingRect(
       content_rect, 1.f / contents_scale);
   layer_rect.Intersect(gfx::Rect(tiling_.total_size()));
-  return recorded_region_.Contains(layer_rect);
+
+  // Common case inside of viewport to avoid the slower map lookups.
+  if (recorded_viewport_.Contains(layer_rect)) {
+    // Sanity check that there are no false positives in recorded_viewport_.
+    DCHECK(CanRasterSlowTileCheck(layer_rect));
+    return true;
+  }
+
+  return CanRasterSlowTileCheck(layer_rect);
+}
+
+bool PicturePileBase::CanRasterSlowTileCheck(

[vmpstr, 2014/03/12 23:57:35]

I don't really buy that this function is that slow :) At least when compared to
the previous version of "CanRaster"

[enne (OOO), 2014/03/13 01:35:43]

PicturePileBase::CanRasterNotAsSlowAsPreviousVersionButProbablyNotTooSpeedySinceItIsDoingSomeHashMapLookups

[vmpstr, 2014/03/13 16:30:35]

I like it!

+    const gfx::Rect& layer_rect) const {
+  for (TilingData::Iterator tile_iter(&tiling_, layer_rect); tile_iter;
+       ++tile_iter) {
+    PictureMap::const_iterator map_iter = picture_map_.find(tile_iter.index());
+    if (map_iter == picture_map_.end())
+      return false;
+    if (!map_iter->second.GetPicture())
+      return false;
+  }
+  return true;
 }
 
 gfx::Rect PicturePileBase::PaddedRect(const PictureMapKey& key) {
   gfx::Rect tile = tiling_.TileBounds(key.first, key.second);
   return PadRect(tile);
 }
 
 gfx::Rect PicturePileBase::PadRect(const gfx::Rect& rect) {
   gfx::Rect padded_rect = rect;
   padded_rect.Inset(
@@ skipping 71 matching lines @@
     info.picture_ = picture_->GetCloneForDrawingOnThread(thread_index);
   return info;
 }
 
 float PicturePileBase::PictureInfo::GetInvalidationFrequency() const {
   return invalidation_history_.count() /
          static_cast<float>(INVALIDATION_FRAMES_TRACKED);
 }
 
 }  // namespace cc