cc: Fix CanRaster dcheck failures ONCE AND FOR ALL.

cc/resources/picture_pile.cc

Index: cc/resources/picture_pile.cc
diff --git a/cc/resources/picture_pile.cc b/cc/resources/picture_pile.cc
index ae67cd482163bb5839f2a17961779246dd23e15d..dc4274c2cca7c0a22c7a6c858f1ce714e6f6d0c3 100644
--- a/cc/resources/picture_pile.cc
+++ b/cc/resources/picture_pile.cc
@@ -226,34 +226,117 @@ bool PicturePile::UpdateAndExpandInvalidation(
     gfx::Rect old_tiling_rect_over_tiles =
         tiling_.ExpandRectToTileBounds(gfx::Rect(old_tiling_size));
     if (min_toss_x < tiling_.num_tiles_x()) {
-      int unrecorded_left = std::max(tiling_.TilePositionX(min_toss_x),
-                                     interest_rect_over_tiles.right());
+      // The bounds which we want to invalidate are the tiles along the old
+      // edge of the pile. In the picture below, it would be the bounding box
+      // of tiles {h,i,j}.
+      //
+      //  old pile edge-v  new pile edge-v
+      // ---------------+ - - - - - - - -+
+      // mmppssvvyybbeeh|h               .

[enne (OOO), 2014/08/25 17:48:47]

If I'm reading this diagram correctly, min_toss_x is between ee and hh, e.g.
"ee|hh"?

[danakj, 2014/08/25 18:03:39]

The old pile edge was in the middle of the h tile in this picture, so some new
pixels on h are exposed. The min_toss_x will be == h because that tile has new
pixels.

[enne (OOO), 2014/08/25 18:08:13]

...so you're agreeing with me?

[danakj, 2014/08/25 18:13:35]

Ya, but for clarify min_toss_x would be == h not between? But ya same thing.

+      // mmppssvvyybbeeh|h               .
+      // nnqqttwwzzccffi|i               .
+      // nnqqttwwzzccffi|i               .
+      // oorruuxxaaddggj|j               .
+      // oorruuxxaaddggj|j               .
+      // ---------------+ - - - - - - - -+ <- old pile edge
+      //                                 .
+      //  - - - - - - - - - - - - - - - -+ <- new pile edge
+      //
+      // If you were to slide a vertical beam from the left edge of the
+      // invalidation rect toward the right, it would either hit the right edge
+      // of the invalidation rect, or the interest rect (expanded to the bounds
+      // of the tiles it touches).
+      // +--------------------------------------+
+      // |===>      INVALIDATION RECT           |

[enne (OOO), 2014/08/25 17:48:47]

This bit took me a while to understand.  Can you include the new pile edge in
this diagram too?

Also, it's not really "invalidation rect" it's like "potential invalidation
rect" or even like "old pile edge rect"? I think the word "invalidation" here is
confusing (for me) since this code is trying to generate an invalidation rect.

[danakj, 2014/08/25 18:03:39]

Sure, this rect is no longer the entire pile, this is only the last column of
tiles on the old pile. The old pile rect's edge will be a vertical line anywhere
inside this rect. I can add something and describe that better.
Hm, ok, it's the rect expanded to tile bounds of all tiles in the "to be dropped
row/column" and we are invalidating those tiles. Hence how I got there. We
invalidate everything inside it, except if we're going to re-record and keep the
tile alive. Old pile edge rect sounds okay to me.

+      // |===>                                  |
+      // |===>+-----------------+               |
+      // |===>| INTEREST RECT   |               |

[enne (OOO), 2014/08/25 17:48:47]

This may just be the coffee speaking, but I think I understand the "what" of
this enough to say that I don't understand the "why" of this algorithm.

Everything in the invalidation rect in this diagram is a picture pile tile that
has either been entirely or partially exposed to new content due to the bounds
change.  Why are you avoiding invalidations inside of the interest rect?

[danakj, 2014/08/25 18:03:39]

Outside of the interest rect we invalidate the entire tile, because we're
dropping the tile, it won't exist anymore so all pixels in the tile are bad.

If the tile intersects the intersect rect, we will re-record it below so the
tile still exists and impl can keep around recordings that use it. So for those
tiles we only invalidate new pixels. If old pixels are used by a recording tile,
and not new pixels, that recording tile doesn't need to change.

[enne (OOO), 2014/08/25 18:08:13]

What do you mean by "impl can keep around recordings that use it"?

Here's what I don't understand: why do you need to special case the interest
rect? It seems like if you invalidated everything in that big "invalidation"
rect and rerecorded new tiles in the interest rect, everything would still work
ok.

[danakj, 2014/08/25 18:13:35]

If a raster tile used some of the old pixels on one of those tiles, but not any
of the new pixels, invalidating the old pixels would cause us to drop the raster
tile and reraster it.

[enne (OOO), 2014/08/25 18:24:17]

Something still doesn't seem right here to me.  I don't understand why the
interest rect is relevant.  The diagram below where you start at the old pile
edge is what keeps you from rerastering existing content, since all existing
content is by definition inside of the old bounds.

[danakj, 2014/08/25 18:29:55]

There are 2 possible outcomes for each tile on the edge of the old pile here:

1. We drop the tile and it no longer exists
- This is because the interest rect doesn't overlap.
- In this case we need to drop Tiles on the impl side that would try to use
these recording tiles since they are now destroyed.

2. We drop the tile, but we re-record it below
- This is because the interest rect touches the recording tile
- In this case the pixels in the tile and inside the old pile bounds are still
valid and are not changed (modulo external invalidation).
- Since the old pixels are still valid, we don't want to add invalidation for
them as that would cause unneeded reraster.
- So we only invalidate the new pixels outside the old pile bounds.

Case 2 is handled below and is talked about by that diagram. This part is
dealing with case 1. Either a tile intersects the interest rect or it does not.
Maybe this diagram is poor, because the interest rect will always be either
entirely overlapping the "INVALIDATION RECT TBR" horizontally, or it won't
overlap at all. It can't be partially horizontally overlapping the tiles in the
old-last-column, since the rect is expanded to tile bounds.

Does that help?

+      // +----|-----------------|---------------+
+      // We want to invalidate everything between the top and bottom of the
+      // invalidation rect from the left edge, until the first point where
+      // either the interest rect or the other end of the invalidation rect
+      // is reached. The |left_until| variable is this point.
+      //
+      // Repeat this for each of the four edges of the invalidation rect. In the
+      // picture above, the beam from the bottom would produce no invalidation
+      // since it would immediately hit the interest rect.
+      //
+      // Lastly, we need to consider tiles that do intersect the interest rect.
+      // For those tiles, we want to invalidate exactly the newly exposed
+      // pixels. In the picture below, the left and bottom edges touch the
+      // interest rect, so no invalidation would be added from those edges.
+      // However everyting to the right of the old pile edge are newly visible
+      // pixels and should be invalidated. The last step is to add that entire
+      // rect to the invalidation. This will overlap, or even be contained
+      // within the four rects previously added, if the interest rect intersects
+      // any tiles within the bounds of the invalidation rect.
+      //
+      //    v-old pile edge                     v-new pile edge
+      // +--.-----------------------------------+
+      // |  .===>   INVALIDATION RECT           |
+      // |  .===>                               |
+      // ---.===>---------------+               |
+      // |  .===> INTEREST RECT |               |
+      // +--.-------------------|---------------+
+
+      int left = tiling_.TilePositionX(min_toss_x);
+      int right = left + tiling_.TileSizeX(min_toss_x);
+      int top = old_tiling_rect_over_tiles.y();
+      int bottom = old_tiling_rect_over_tiles.bottom();
+
+      int left_until = std::min(interest_rect_over_tiles.x(), right);
+      int right_until = std::max(interest_rect_over_tiles.right(), left);
+      int top_until = std::min(interest_rect_over_tiles.y(), bottom);
+      int bottom_until = std::max(interest_rect_over_tiles.bottom(), top);
+
       int exposed_left = old_tiling_size.width();
-      int left = std::min(unrecorded_left, exposed_left);
-      int tile_right =
-          tiling_.TilePositionX(min_toss_x) + tiling_.TileSizeX(min_toss_x);
-      int exposed_right = tiling_size().width();
-      int right = std::min(tile_right, exposed_right);
-      gfx::Rect right_side(left,
-                           old_tiling_rect_over_tiles.y(),
-                           right - left,
-                           old_tiling_rect_over_tiles.height());
-      resize_invalidation.Union(right_side);
+      int exposed_left_until = right;
+      DCHECK_GE(exposed_left, left);
+
+      gfx::Rect left_rect(left, top, left_until - left, bottom - top);
+      gfx::Rect right_rect(right_until, top, right - right_until, bottom - top);
+      gfx::Rect top_rect(left, top, right - left, top_until - top);
+      gfx::Rect bottom_rect(
+          left, bottom_until, right - left, bottom - bottom_until);
+      gfx::Rect exposed_rect(
+          exposed_left, top, exposed_left_until - exposed_left, bottom - top);
+      resize_invalidation.Union(left_rect);
+      resize_invalidation.Union(right_rect);
+      resize_invalidation.Union(top_rect);
+      resize_invalidation.Union(bottom_rect);
+      resize_invalidation.Union(exposed_rect);
     }
     if (min_toss_y < tiling_.num_tiles_y()) {
-      int unrecorded_top = std::max(tiling_.TilePositionY(min_toss_y),
-                                    interest_rect_over_tiles.bottom());
+      // The same thing occurs here as in the case above, but the invalidation
+      // rect is the bounding box around the bottom row of tiles in the old
+      // pile. This would be tiles {o,r,u,x,a,d,g,j} in the above picture.
+
+      int top = tiling_.TilePositionY(min_toss_y);
+      int bottom = top + tiling_.TileSizeY(min_toss_y);
+      int left = old_tiling_rect_over_tiles.x();
+      int right = old_tiling_rect_over_tiles.right();
+
+      int top_until = std::min(interest_rect_over_tiles.y(), bottom);
+      int bottom_until = std::max(interest_rect_over_tiles.bottom(), top);
+      int left_until = std::min(interest_rect_over_tiles.x(), right);
+      int right_until = std::max(interest_rect_over_tiles.right(), left);
+
       int exposed_top = old_tiling_size.height();
-      int top = std::min(unrecorded_top, exposed_top);
-      int tile_bottom =
-          tiling_.TilePositionY(min_toss_y) + tiling_.TileSizeY(min_toss_y);
-      int exposed_bottom = tiling_size().height();
-      int bottom = std::min(tile_bottom, exposed_bottom);
-      gfx::Rect bottom_side(old_tiling_rect_over_tiles.x(),
-                            top,
-                            old_tiling_rect_over_tiles.width(),
-                            bottom - top);
-      resize_invalidation.Union(bottom_side);
+      int exposed_top_until = bottom;
+      DCHECK_GE(exposed_top, top);
+
+      gfx::Rect left_rect(left, top, left_until - left, bottom - top);
+      gfx::Rect right_rect(right_until, top, right - right_until, bottom - top);
+      gfx::Rect top_rect(left, top, right - left, top_until - top);
+      gfx::Rect bottom_rect(
+          left, bottom_until, right - left, bottom - bottom_until);
+      gfx::Rect exposed_rect(
+          left, exposed_top, right - left, exposed_top_until - exposed_top);
+      resize_invalidation.Union(left_rect);
+      resize_invalidation.Union(right_rect);
+      resize_invalidation.Union(top_rect);
+      resize_invalidation.Union(bottom_rect);
+      resize_invalidation.Union(exposed_rect);
     }
   }