cc: Discard SkCanvas (frame-buffer) before rasterization.

cc/resources/picture_pile_impl.cc

 // Copyright 2012 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 <algorithm>
 #include <limits>
 
 #include "base/debug/trace_event.h"
 #include "cc/base/region.h"
 #include "cc/debug/debug_colors.h"
@@ skipping 79 matching lines @@
     RenderingStatsInstrumentation* stats_instrumentation) {
   RasterCommon(
       canvas, canvas, canvas_rect, contents_scale, stats_instrumentation, true);
 }
 
 void PicturePileImpl::RasterToBitmap(
     SkCanvas* canvas,
     const gfx::Rect& canvas_rect,
     float contents_scale,
     RenderingStatsInstrumentation* rendering_stats_instrumentation) {
+  DCHECK(canvas->getBaseLayerSize().equals(canvas_rect.width(),
+                                           canvas_rect.height()));

[reveman, 2014/05/09 00:56:29]

Should we really require this? In that case, should |canvas_rect| be
|canvas_origin| instead of passing more data to this function than needed?

[alokp, 2014/05/09 20:13:04]

We do not require it as far as the API is concerned. But in practice we always
create a new canvas for each raster. The API allows you to offset multiple
rasters into a single canvas. But we have gradually started making the
assumption that it is OK to write outside canvas_rect - see line 143 below where
we clear the entire canvas. I wanted to add a DCHECK to ensure this assumption.

You are right that we should change the API to make this assumption explicit.
But I think it is outside the scope of this patch.

[reveman, 2014/05/10 00:34:52]

I think I prefer if you didn't add this DCHECK. Without it, this API can be
interpreted as "Here's a canvas of some size, you can do whatever with it but
canvas_rect is the area I like correct contents to be produced for". If this
flexibility is unnecessary, I'd rather see us change the API right away than add
temporary DCHECKs.

[alokp, 2014/05/12 18:45:33]

OK. Removed DCHECK. I think we should make it clear through documentation or
otherwise that this function may write over area outside canvas_rect.

[reveman, 2014/05/12 18:51:38]

Yea, that wouldn't hurt. Feel free to add a comment to picture_pile_impl.h as
part of this patch if you like.

+  canvas->discard();
   if (clear_canvas_with_debug_color_) {
     // Any non-painted areas will be left in this color.
     canvas->clear(DebugColors::NonPaintedFillColor());
   }
 
   // If this picture has opaque contents, it is guaranteeing that it will
   // draw an opaque rect the size of the layer.  If it is not, then we must
   // clear this canvas ourselves.
   if (contents_opaque_ || contents_fill_bounds_completely_) {
     // Even if completely covered, for rasterizations that touch the edge of the
@@ skipping 333 matching lines @@
        ++it) {
     Picture* picture = it->second.GetPicture();
     if (picture && (processed_pictures.count(picture) == 0)) {
       picture->EmitTraceSnapshot();
       processed_pictures.insert(picture);
     }
   }
 }
 
 }  // namespace cc