Cache gpu suitability in DisplayItemList, remove SetUnsuitable...ForTesting

cc/playback/display_item_list.cc

 // Copyright 2014 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/playback/display_item_list.h"
 
 #include <string>
 
 #include "base/numerics/safe_conversions.h"
 #include "base/trace_event/trace_event.h"
@@ skipping 37 matching lines @@
       !settings.use_cached_picture || DisplayItemsTracingEnabled()));
 }
 
 DisplayItemList::DisplayItemList(gfx::Rect layer_rect,
                                  const DisplayItemListSettings& settings,
                                  bool retain_individual_display_items)
     : items_(LargestDisplayItemSize(), kDefaultNumDisplayItemsToReserve),
       use_cached_picture_(settings.use_cached_picture),
       retain_individual_display_items_(retain_individual_display_items),
       layer_rect_(layer_rect),
-      all_items_are_suitable_for_gpu_rasterization_(true),
+      is_suitable_for_gpu_rasterization_(true),
       approximate_op_count_(0),
       picture_memory_usage_(0),
       external_memory_usage_(0) {
 #if DCHECK_IS_ON()
   needs_process_ = false;
 #endif
   if (use_cached_picture_) {
     SkRTreeFactory factory;
     recorder_.reset(new SkPictureRecorder());
     canvas_ = skia::SharePtr(recorder_->beginRecording(
@@ skipping 47 matching lines @@
     // after |items_| grows too large and we process it.
     DCHECK(items_.empty());
   }
 }
 
 void DisplayItemList::ProcessAppendedItems() {
 #if DCHECK_IS_ON()
   needs_process_ = false;
 #endif
   for (const DisplayItem* item : items_) {
-    all_items_are_suitable_for_gpu_rasterization_ &=
-        item->is_suitable_for_gpu_rasterization();
-    approximate_op_count_ += item->approximate_op_count();
-
     if (use_cached_picture_) {
+      // When using a cached picture we will calculate gpu suitability on the
+      // entire cached picture instead of the items. This is more permissive
+      // than since none of the items might individually trigger a veto even
+      // though they collectively have enough "bad" operations that a
+      // corresponding Picture would get vetoed. See crbug.com/513016.
       DCHECK(canvas_);
+      approximate_op_count_ += item->approximate_op_count();
       item->Raster(canvas_.get(), gfx::Rect(), NULL);
+    } else {
+      is_suitable_for_gpu_rasterization_ &=
+          item->is_suitable_for_gpu_rasterization();
+      approximate_op_count_ += item->approximate_op_count();
     }
 
     if (retain_individual_display_items_) {
       // Warning: this double-counts SkPicture data if use_cached_picture_ is
       // also true.
       external_memory_usage_ += item->external_memory_usage();
     }
   }
 
   if (!retain_individual_display_items_)
     items_.clear();
 }
 
 void DisplayItemList::RasterIntoCanvas(const DisplayItem& item) {
   DCHECK(canvas_);
   DCHECK(!retain_individual_display_items_);
-  all_items_are_suitable_for_gpu_rasterization_ &=
+  is_suitable_for_gpu_rasterization_ &=

[danakj, 2015/09/17 21:11:32]

why do we do this here too? don't we want to compute all of this at recording
time? we use it to decide about how to raster

[pdr., 2015/09/18 22:40:14]

This was a mistake. I had refactored this so ProcessAppendedItems called
RasterIntoCanvas but un-factored it and forgot this line. Good catch.

       item.is_suitable_for_gpu_rasterization();
   approximate_op_count_ += item.approximate_op_count();
 
   item.Raster(canvas_.get(), gfx::Rect(), NULL);
 }
 
 bool DisplayItemList::RetainsIndividualDisplayItems() const {
   return retain_individual_display_items_;
 }
 
@@ skipping 12 matching lines @@
   if (use_cached_picture_) {
     // Convert to an SkPicture for faster rasterization.
     DCHECK(use_cached_picture_);
     DCHECK(!picture_);
     picture_ = skia::AdoptRef(recorder_->endRecordingAsPicture());
     DCHECK(picture_);
     picture_memory_usage_ =
         SkPictureUtils::ApproximateBytesUsed(picture_.get());
     recorder_.reset();
     canvas_.clear();
+    is_suitable_for_gpu_rasterization_ =
+        picture_->suitableForGpuRasterization(NULL);

[danakj, 2015/09/17 21:11:32]

nullptr

[pdr., 2015/09/18 22:40:14]

We should make a pass through and remove all the NULLs so I can copy&paste with
reckless abandon.

   }
 }
 
 bool DisplayItemList::IsSuitableForGpuRasterization() const {
   DCHECK(ProcessAppendedItemsCalled());
-  if (use_cached_picture_)
-    return picture_->suitableForGpuRasterization(NULL);
-
-  // This is more permissive than Picture's implementation, since none of the
-  // items might individually trigger a veto even though they collectively have
-  // enough "bad" operations that a corresponding Picture would get vetoed. See
-  // crbug.com/513016.
-  return all_items_are_suitable_for_gpu_rasterization_;
+  return is_suitable_for_gpu_rasterization_;
 }
 
 int DisplayItemList::ApproximateOpCount() const {
   DCHECK(ProcessAppendedItemsCalled());
   return approximate_op_count_;
 }
 
 size_t DisplayItemList::ApproximateMemoryUsage() const {
   DCHECK(ProcessAppendedItemsCalled());
   // We double-count in this case. Produce zero to avoid being misleading.
@@ skipping 71 matching lines @@
   DCHECK(picture_);
   DCHECK(!images_);
   images_ = make_scoped_ptr(new DiscardableImageMap(grid_cell_size));
   if (!picture_->willPlayBackBitmaps())
     return;
 
   images_->GatherImagesFromPicture(picture_.get(), layer_rect_);
 }
 
 }  // namespace cc