cc: Fix low-res impl-side painting artifacts

cc/picture_pile_impl.cc

Index: cc/picture_pile_impl.cc
diff --git a/cc/picture_pile_impl.cc b/cc/picture_pile_impl.cc
index 05f9584ae58bb1f82306f7d244053b42a32909d1..d35e34dc19944565768afe371f371828a4f1253e 100644
--- a/cc/picture_pile_impl.cc
+++ b/cc/picture_pile_impl.cc
@@ -15,7 +15,8 @@ scoped_refptr<PicturePileImpl> PicturePileImpl::Create() {
   return make_scoped_refptr(new PicturePileImpl());
 }
 
-PicturePileImpl::PicturePileImpl() {
+PicturePileImpl::PicturePileImpl()
+    : min_contents_scale_(1) {
 }
 
 PicturePileImpl::~PicturePileImpl() {
@@ -40,6 +41,7 @@ scoped_refptr<PicturePileImpl> PicturePileImpl::CloneForDrawing() const {
   for (PicturePile::Pile::const_iterator i = pile_.begin();
        i != pile_.end(); ++i)
     clone->pile_.push_back((*i)->Clone());
+  clone->min_contents_scale_ = min_contents_scale_;
 
   return clone;
 }
@@ -49,26 +51,36 @@ void PicturePileImpl::Raster(
     gfx::Rect content_rect,
     float contents_scale,
     RenderingStats* stats) {
+
+  if (!pile_.size())
+    return;
+
+  DCHECK(contents_scale >= min_contents_scale_);
+
   base::TimeTicks rasterizeBeginTime = base::TimeTicks::Now();
 
   // TODO(enne): do this more efficiently, i.e. top down with Skia clips
   canvas->save();
   canvas->translate(-content_rect.x(), -content_rect.y());
-  SkRect layer_skrect = SkRect::MakeXYWH(
-      content_rect.x(),
-      content_rect.y(),
-      content_rect.width(),
-      content_rect.height());
-  canvas->clipRect(layer_skrect);
-  canvas->scale(contents_scale, contents_scale);
+  canvas->clipRect(gfx::RectToSkRect(content_rect));

[danakj, 2013/01/05 00:04:58]

Is this right after the translate() ? Or should this clip rect be at 0,0?

[enne (OOO), 2013/01/06 03:57:18]

Hmm.  This should go before.  Done.

[enne (OOO), 2013/01/06 04:30:31]

I was right the first time, sorry.

The translation is to go from canvas space to content space.  So that things
drawn into content space get correctly translated back to the 0, 0, w, h of the
canvas.  So, the clip comes afterwards.

 
-  gfx::Rect layer_rect = gfx::ToEnclosedRect(gfx::ScaleRect(gfx::RectF(content_rect), 1 / contents_scale));
+  gfx::Rect layer_rect = gfx::ToEnclosingRect(

[danakj, 2013/01/05 00:04:58]

Why enclosing? We do enclosing when going from layer to content space (ie
Layer::layerRectToContentRect()), so isn't this supposed to be enclosed?

[enne (OOO), 2013/01/06 03:57:18]

Good question.  I think what I should do is just see if content_clip intersects
with content_rect and not even calculate layer_rect.

+      gfx::ScaleRect(gfx::RectF(content_rect), 1 / contents_scale));
 
   for (PicturePile::Pile::const_iterator i = pile_.begin();
        i != pile_.end(); ++i) {
-    if (!(*i)->LayerRect().Intersects(layer_rect))
+    gfx::Rect picture_layer_rect = (*i)->LayerRect();
+    if (!picture_layer_rect.Intersects(layer_rect))
       continue;
-    (*i)->Raster(canvas);
+
+    // This is intentionally *enclosed* rect, so that the clip is aligned on
+    // integral post-scale content pixels and does not extend past the edges of
+    // the picture's layer rect.  The min_contents_scale enforces that enough
+    // buffer pixels have been added such that the enclosed rect encompasses all
+    // invalidated pixels at that scale level.
+    gfx::Rect content_clip = gfx::ToEnclosedRect(
+        gfx::ScaleRect(picture_layer_rect, contents_scale));
+    (*i)->Raster(canvas, content_clip, contents_scale);
 
     SkISize deviceSize = canvas->getDeviceSize();
     stats->totalPixelsRasterized += deviceSize.width() * deviceSize.height();