Coalesce damage rects that share an edge.

This fixes the Bloat HTTP page cycler regression, and allows me to remove the
hack I checked in to disable multiple-paints due to an observed DHTML page
cycler regression.

I added a new histogram, RW_IntermediatePaintRectCount, that would have shown
the problem clearly had it been there before.

I included some cleanup in PaintAggregator:
1- rename "r" to "existing_rect" for clarity
2- check for contained (i.e., redundant) invalidates up front

I also changed the opacity of the paint rects used when --show-paint-rects is
specified.  I find myself dogfooding with this command line option enabled, and
I want it to be a little less annoying but still just as useful.

R=brettw
BUG=29477
TEST=none

Review URL: http://codereview.chromium.org/464057

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33949 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 71 insertions, 23 deletions

diff --git a/base/gfx/rect.cc b/base/gfx/rect.cc
index 1e067e7..2a35575 100644
--- a/base/gfx/rect.cc
+++ b/base/gfx/rect.cc
@@ -225,6 +225,13 @@ Point Rect::CenterPoint() const {
   return Point(x() + (width() + 1) / 2, y() + (height() + 1) / 2);
 }
 
+bool Rect::SharesEdgeWith(const gfx::Rect& rect) const {
+  return (y() == rect.y() && height() == rect.height() &&
+             (x() == rect.right() || right() == rect.x())) ||
+         (x() == rect.x() && width() == rect.width() &&
+             (y() == rect.bottom() || bottom() == rect.y()));
+}
+
 }  // namespace gfx
 
 std::ostream& operator<<(std::ostream& out, const gfx::Rect& r) {
diff --git a/base/gfx/rect.h b/base/gfx/rect.h
index 2eef76c..c3c01ad 100644
--- a/base/gfx/rect.h
+++ b/base/gfx/rect.h
@@ -149,6 +149,10 @@ class Rect {
   // Returns the center of this rectangle.
   Point CenterPoint() const;
 
+  // Returns true if this rectangle shares an entire edge (i.e., same width or
+  // same height) with the given rectangle, and the rectangles do not overlap.
+  bool SharesEdgeWith(const gfx::Rect& rect) const;
+
  private:
   gfx::Point origin_;
   gfx::Size size_;
diff --git a/base/gfx/rect_unittest.cc b/base/gfx/rect_unittest.cc
index 50c789d..3562883 100644
--- a/base/gfx/rect_unittest.cc
+++ b/base/gfx/rect_unittest.cc
@@ -278,3 +278,37 @@ TEST(RectTest, IsEmpty) {
   EXPECT_FALSE(gfx::Rect(0, 0, 10, 10).IsEmpty());
   EXPECT_FALSE(gfx::Rect(0, 0, 10, 10).size().IsEmpty());
 }
+
+TEST(RectTest, SharesEdgeWith) {
+  gfx::Rect r(2, 3, 4, 5);
+
+  // Must be non-overlapping
+  EXPECT_FALSE(r.SharesEdgeWith(r));
+
+  gfx::Rect just_above(2, 1, 4, 2);
+  gfx::Rect just_below(2, 8, 4, 2);
+  gfx::Rect just_left(0, 3, 2, 5);
+  gfx::Rect just_right(6, 3, 2, 5);
+
+  EXPECT_TRUE(r.SharesEdgeWith(just_above));
+  EXPECT_TRUE(r.SharesEdgeWith(just_below));
+  EXPECT_TRUE(r.SharesEdgeWith(just_left));
+  EXPECT_TRUE(r.SharesEdgeWith(just_right));
+
+  // Wrong placement
+  gfx::Rect same_height_no_edge(0, 0, 1, 5);
+  gfx::Rect same_width_no_edge(0, 0, 4, 1);
+
+  EXPECT_FALSE(r.SharesEdgeWith(same_height_no_edge));
+  EXPECT_FALSE(r.SharesEdgeWith(same_width_no_edge));
+
+  gfx::Rect just_above_no_edge(2, 1, 5, 2);  // too wide
+  gfx::Rect just_below_no_edge(2, 8, 3, 2);  // too narrow
+  gfx::Rect just_left_no_edge(0, 3, 2, 6);   // too tall
+  gfx::Rect just_right_no_edge(6, 3, 2, 4);  // too short
+
+  EXPECT_FALSE(r.SharesEdgeWith(just_above_no_edge));
+  EXPECT_FALSE(r.SharesEdgeWith(just_below_no_edge));
+  EXPECT_FALSE(r.SharesEdgeWith(just_left_no_edge));
+  EXPECT_FALSE(r.SharesEdgeWith(just_right_no_edge));
+}
diff --git a/chrome/renderer/paint_aggregator.cc b/chrome/renderer/paint_aggregator.cc
index af543af..8f77b15 100644
--- a/chrome/renderer/paint_aggregator.cc
+++ b/chrome/renderer/paint_aggregator.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/renderer/paint_aggregator.h"
 
+#include "base/histogram.h"
 #include "base/logging.h"
 
 // ----------------------------------------------------------------------------
@@ -81,21 +82,29 @@ void PaintAggregator::ClearPendingUpdate() {
 void PaintAggregator::InvalidateRect(const gfx::Rect& rect) {
   // Combine overlapping paints using smallest bounding box.
   for (size_t i = 0; i < update_.paint_rects.size(); ++i) {
-    gfx::Rect r = update_.paint_rects[i];
-    if (rect.Intersects(r)) {
-      if (!r.Contains(rect)) {  // Optimize for redundant paint.
-        update_.paint_rects.erase(update_.paint_rects.begin() + i);
-        InvalidateRect(rect.Union(r));
-      }
+    const gfx::Rect& existing_rect = update_.paint_rects[i];
+    if (existing_rect.Contains(rect))  // Optimize out redundancy.
+      return;
+    if (rect.Intersects(existing_rect) || rect.SharesEdgeWith(existing_rect)) {
+      // Re-invalidate in case the union intersects other paint rects.
+      gfx::Rect combined_rect = existing_rect.Union(rect);
+      update_.paint_rects.erase(update_.paint_rects.begin() + i);
+      InvalidateRect(combined_rect);
       return;
     }
   }
 
-  // Add a non-overlapping paint.
-  // TODO(darin): Limit the size of this vector?
-  // TODO(darin): Coalesce adjacent rects.
+  // Add a non-overlapping paint.  TODO(darin): Limit the size of this vector?
   update_.paint_rects.push_back(rect);
 
+  // Track how large the paint_rects vector grows during an invalidation
+  // sequence.  Note: A subsequent invalidation may end up being combined
+  // with all existing paints, which means that tracking the size of
+  // paint_rects at the time when GetPendingUpdate() is called may mask
+  // certain performance problems.
+  HISTOGRAM_COUNTS_10000("MPArch.RW_IntermediatePaintRectCount",
+                          update_.paint_rects.size());
+
   // If the new paint overlaps with a scroll, then it forces an invalidation of
   // the scroll.  If the new paint is contained by a scroll, then trim off the
   // scroll damage to avoid redundant painting.
@@ -182,13 +191,13 @@ bool PaintAggregator::ShouldInvalidateScrollRect(const gfx::Rect& rect) const {
   // rect comes too close to the area of the scroll_rect.  If so, then we
   // might as well invalidate the scroll rect.
 
-  int paint_area = rect.width() * rect.height();
+  int paint_area = rect.size().GetArea();
   for (size_t i = 0; i < update_.paint_rects.size(); ++i) {
-    const gfx::Rect& r = update_.paint_rects[i];
-    if (update_.scroll_rect.Contains(r))
-      paint_area += r.width() * r.height();
+    const gfx::Rect& existing_rect = update_.paint_rects[i];
+    if (update_.scroll_rect.Contains(existing_rect))
+      paint_area += existing_rect.size().GetArea();
   }
-  int scroll_area = update_.scroll_rect.width() * update_.scroll_rect.height();
+  int scroll_area = update_.scroll_rect.size().GetArea();
   if (float(paint_area) / float(scroll_area) > kMaxRedundantPaintToScrollArea)
     return true;
 
diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc
index 388dd59..5d56a41 100644
--- a/chrome/renderer/render_widget.cc
+++ b/chrome/renderer/render_widget.cc
@@ -393,7 +393,7 @@ void RenderWidget::PaintDebugBorder(const gfx::Rect& rect,
 
   SkPaint paint;
   paint.setStyle(SkPaint::kStroke_Style);
-  paint.setColor(SkColorSetARGB(0x7F, 0xFF, 0, 0));
+  paint.setColor(SkColorSetARGB(0x3F, 0xFF, 0, 0));
   paint.setStrokeWidth(1);
 
   SkIRect irect;
@@ -484,14 +484,8 @@ void RenderWidget::DoDeferredUpdate() {
     bounds.set_width(canvas->getDevice()->width());
     bounds.set_height(canvas->getDevice()->height());
 
-    HISTOGRAM_COUNTS_100("MPArch.RW_PaintRectCount", update.paint_rects.size());
-
-    // TODO(darin): re-enable painting multiple damage rects once the
-    // page-cycler regressions are resolved.
-    if (update.scroll_rect.IsEmpty()) {
-      update.paint_rects.clear();
-      update.paint_rects.push_back(bounds);
-    }
+    HISTOGRAM_COUNTS_10000("MPArch.RW_PaintRectCount",
+                           update.paint_rects.size());
 
     for (size_t i = 0; i < update.paint_rects.size(); ++i)
       PaintRect(update.paint_rects[i], bounds.origin(), canvas.get());