Back PaintRecord with PaintOpBuffer instead of SkPicture

cc/paint/display_item_list.cc

Index: cc/paint/display_item_list.cc
diff --git a/cc/paint/display_item_list.cc b/cc/paint/display_item_list.cc
index 83599482f53461dd77ef6952fa45231f5ecdb68c..4e6bc85ef4cffb69916ef5de8a9fb58dcfa048f6 100644
--- a/cc/paint/display_item_list.cc
+++ b/cc/paint/display_item_list.cc
@@ -100,9 +100,13 @@ NOINLINE DISABLE_CFI_PERF void RasterItem(const DisplayItem& base_item,
       if (canvas->quickReject(item.picture->cullRect()))
         break;
 
-      // SkPicture always does a wrapping save/restore on the canvas, so it is
-      // not necessary here.
+      // TODO(enne): Maybe the PaintRecord itself could know whether this
+      // was needed?  It's not clear whether these save/restore semantics
+      // that SkPicture handles during playback are things that should be
+      // kept around.
+      canvas->save();
       item.picture->playback(canvas, callback);
+      canvas->restore();
       break;
     }
     case DisplayItem::FLOAT_CLIP: {
@@ -176,6 +180,33 @@ void DisplayItemList::Raster(SkCanvas* canvas,
   canvas->restore();
 }
 
+// Atttempts to merge a CompositingDisplayItem and DrawingDisplayItem
+// into a single "draw with alpha".  This function returns true if
+// it was successful.  If false, then the caller is responsible for
+// drawing these items.  This is a DisplayItemList version of the
+// SkRecord optimization SkRecordNoopSaveLayerDrawRestores.
+static bool MergeAndDrawIfPossible(const CompositingDisplayItem& save_item,
+                                   const DrawingDisplayItem& draw_item,
+                                   SkCanvas* canvas) {
+  if (save_item.color_filter)
+    return false;
+  if (save_item.xfermode != SkBlendMode::kSrcOver)
+    return false;
+  // TODO(enne): I believe that lcd_text_requires_opaque_layer is not
+  // relevant here and that lcd text is preserved post merge, but I haven't
+  // tested that.
+  const PaintRecord* record = draw_item.picture.get();
+  if (record->approximateOpCount() != 1)
+    return false;
+
+  const PaintOp* op = record->GetFirstOp();
+  if (!op->IsDrawOp())
+    return false;
+
+  op->RasterWithAlpha(canvas, save_item.alpha);
+  return true;
+}
+
 void DisplayItemList::Raster(SkCanvas* canvas,
                              SkPicture::AbortCallback* callback) const {
   gfx::Rect canvas_playback_rect;
@@ -184,14 +215,38 @@ void DisplayItemList::Raster(SkCanvas* canvas,
 
   std::vector<size_t> indices;
   rtree_.Search(canvas_playback_rect, &indices);
-  for (size_t index : indices) {
-    RasterItem(items_[index], canvas, callback);
-
+  for (size_t i = 0; i < indices.size(); ++i) {
     // We use a callback during solid color analysis on the compositor thread to
     // break out early. Since we're handling a sequence of pictures via rtree
     // query results ourselves, we have to respect the callback and early out.
     if (callback && callback->abort())
       break;
+
+    const DisplayItem& item = items_[indices[i]];
+    // Optimize empty begin/end compositing and merge begin/draw/end compositing
+    // where possible.
+    // TODO(enne): remove empty clips here too?
+    // TODO(enne): does this happen recursively? Or is this good enough?
+    if (i < indices.size() - 2 && item.type == DisplayItem::COMPOSITING) {
+      const DisplayItem& second = items_[indices[i + 1]];
+      if (second.type == DisplayItem::END_COMPOSITING) {
+        i++;
+        // TODO(enne): does this ever happen? Is this worth catching?
+        continue;
+      }
+      const DisplayItem& third = items_[indices[i + 2]];
+      if (second.type == DisplayItem::DRAWING &&
+          third.type == DisplayItem::END_COMPOSITING) {
+        if (MergeAndDrawIfPossible(
+                static_cast<const CompositingDisplayItem&>(item),
+                static_cast<const DrawingDisplayItem&>(second), canvas)) {
+          i += 2;
+          continue;
+        }
+      }
+    }
+
+    RasterItem(item, canvas, callback);
   }
 }