diff options
Diffstat (limited to 'chrome/renderer')
-rw-r--r-- | chrome/renderer/paint_aggregator.cc | 25 | ||||
-rw-r--r-- | chrome/renderer/paint_aggregator.h | 3 | ||||
-rw-r--r-- | chrome/renderer/paint_aggregator_unittest.cc | 204 | ||||
-rw-r--r-- | chrome/renderer/render_widget.cc | 11 |
4 files changed, 161 insertions, 82 deletions
diff --git a/chrome/renderer/paint_aggregator.cc b/chrome/renderer/paint_aggregator.cc index 991053f..23a9025 100644 --- a/chrome/renderer/paint_aggregator.cc +++ b/chrome/renderer/paint_aggregator.cc @@ -34,6 +34,10 @@ static const float kMaxRedundantPaintToScrollArea = 0.8f; // a paint rect can be significant. static const size_t kMaxPaintRects = 5; +// If the combined area of paint rects divided by the area of the union of all +// paint rects exceeds this threshold, then we will combine the paint rects. +static const float kMaxPaintRectsAreaRatio = 0.3f; + PaintAggregator::PendingUpdate::PendingUpdate() {} PaintAggregator::PendingUpdate::~PendingUpdate() {} @@ -89,6 +93,25 @@ void PaintAggregator::ClearPendingUpdate() { update_ = PendingUpdate(); } +void PaintAggregator::PopPendingUpdate(PendingUpdate* update) { + // Combine paint rects if their combined area is not sufficiently less than + // the area of the union of all paint rects. We skip this if there is a + // scroll rect since scrolling benefits from smaller paint rects. + if (update_.scroll_rect.IsEmpty() && update_.paint_rects.size() > 1) { + int paint_area = 0; + gfx::Rect union_rect; + for (size_t i = 0; i < update_.paint_rects.size(); ++i) { + paint_area += update_.paint_rects[i].size().GetArea(); + union_rect = union_rect.Union(update_.paint_rects[i]); + } + int union_area = union_rect.size().GetArea(); + if (float(paint_area) / float(union_area) > kMaxPaintRectsAreaRatio) + CombinePaintRects(); + } + *update = update_; + 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) { @@ -127,7 +150,7 @@ void PaintAggregator::InvalidateRect(const gfx::Rect& 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 + // paint_rects at the time when PopPendingUpdate() is called may mask // certain performance problems. HISTOGRAM_COUNTS_100("MPArch.RW_IntermediatePaintRectCount", update_.paint_rects.size()); diff --git a/chrome/renderer/paint_aggregator.h b/chrome/renderer/paint_aggregator.h index 119e0b5..4924b83 100644 --- a/chrome/renderer/paint_aggregator.h +++ b/chrome/renderer/paint_aggregator.h @@ -43,7 +43,8 @@ class PaintAggregator { bool HasPendingUpdate() const; void ClearPendingUpdate(); - const PendingUpdate& GetPendingUpdate() const { return update_; } + // Fills |update| and clears the pending update. + void PopPendingUpdate(PendingUpdate* update); // The given rect should be repainted. void InvalidateRect(const gfx::Rect& rect); diff --git a/chrome/renderer/paint_aggregator_unittest.cc b/chrome/renderer/paint_aggregator_unittest.cc index 25503c8..3473dea 100644 --- a/chrome/renderer/paint_aggregator_unittest.cc +++ b/chrome/renderer/paint_aggregator_unittest.cc @@ -17,17 +17,41 @@ TEST(PaintAggregator, SingleInvalidation) { greg.InvalidateRect(rect); EXPECT_TRUE(greg.HasPendingUpdate()); - EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - ASSERT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_EQ(rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_TRUE(update.scroll_rect.IsEmpty()); + ASSERT_EQ(1U, update.paint_rects.size()); + + EXPECT_EQ(rect, update.paint_rects[0]); } TEST(PaintAggregator, DoubleDisjointInvalidation) { PaintAggregator greg; - gfx::Rect r1(2, 4, 2, 4); - gfx::Rect r2(4, 2, 4, 2); + gfx::Rect r1(2, 4, 2, 40); + gfx::Rect r2(4, 2, 40, 2); + + greg.InvalidateRect(r1); + greg.InvalidateRect(r2); + + gfx::Rect expected_bounds = r1.Union(r2); + + EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); + + EXPECT_TRUE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(2U, update.paint_rects.size()); + + EXPECT_EQ(expected_bounds, update.GetPaintBounds()); +} + +TEST(PaintAggregator, DisjointInvalidationsCombined) { + PaintAggregator greg; + + gfx::Rect r1(2, 4, 2, 2); + gfx::Rect r2(4, 2, 2, 2); greg.InvalidateRect(r1); greg.InvalidateRect(r2); @@ -35,10 +59,13 @@ TEST(PaintAggregator, DoubleDisjointInvalidation) { gfx::Rect expected_bounds = r1.Union(r2); EXPECT_TRUE(greg.HasPendingUpdate()); - EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - ASSERT_EQ(2U, greg.GetPendingUpdate().paint_rects.size()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_EQ(expected_bounds, greg.GetPendingUpdate().GetPaintBounds()); + EXPECT_TRUE(update.scroll_rect.IsEmpty()); + ASSERT_EQ(1U, update.paint_rects.size()); + + EXPECT_EQ(expected_bounds, update.paint_rects[0]); } TEST(PaintAggregator, SingleScroll) { @@ -49,15 +76,18 @@ TEST(PaintAggregator, SingleScroll) { greg.ScrollRect(delta.x(), delta.y(), rect); EXPECT_TRUE(greg.HasPendingUpdate()); - EXPECT_TRUE(greg.GetPendingUpdate().paint_rects.empty()); - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); + + EXPECT_TRUE(update.paint_rects.empty()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); - EXPECT_EQ(rect, greg.GetPendingUpdate().scroll_rect); + EXPECT_EQ(rect, update.scroll_rect); - EXPECT_EQ(delta.x(), greg.GetPendingUpdate().scroll_delta.x()); - EXPECT_EQ(delta.y(), greg.GetPendingUpdate().scroll_delta.y()); + EXPECT_EQ(delta.x(), update.scroll_delta.x()); + EXPECT_EQ(delta.y(), update.scroll_delta.y()); - gfx::Rect resulting_damage = greg.GetPendingUpdate().GetScrollDamage(); + gfx::Rect resulting_damage = update.GetScrollDamage(); gfx::Rect expected_damage(1, 2, 1, 4); EXPECT_EQ(expected_damage, resulting_damage); } @@ -72,17 +102,20 @@ TEST(PaintAggregator, DoubleOverlappingScroll) { greg.ScrollRect(delta2.x(), delta2.y(), rect); EXPECT_TRUE(greg.HasPendingUpdate()); - EXPECT_TRUE(greg.GetPendingUpdate().paint_rects.empty()); - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_EQ(rect, greg.GetPendingUpdate().scroll_rect); + EXPECT_TRUE(update.paint_rects.empty()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + + EXPECT_EQ(rect, update.scroll_rect); gfx::Point expected_delta(delta1.x() + delta2.x(), delta1.y() + delta2.y()); - EXPECT_EQ(expected_delta.x(), greg.GetPendingUpdate().scroll_delta.x()); - EXPECT_EQ(expected_delta.y(), greg.GetPendingUpdate().scroll_delta.y()); + EXPECT_EQ(expected_delta.x(), update.scroll_delta.x()); + EXPECT_EQ(expected_delta.y(), update.scroll_delta.y()); - gfx::Rect resulting_damage = greg.GetPendingUpdate().GetScrollDamage(); + gfx::Rect resulting_damage = update.GetScrollDamage(); gfx::Rect expected_damage(1, 2, 2, 4); EXPECT_EQ(expected_damage, resulting_damage); } @@ -113,10 +146,13 @@ TEST(PaintAggregator, DiagonalScroll) { greg.ScrollRect(delta.x(), delta.y(), rect); EXPECT_TRUE(greg.HasPendingUpdate()); - EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - ASSERT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); + + EXPECT_TRUE(update.scroll_rect.IsEmpty()); + ASSERT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(rect, update.paint_rects[0]); } TEST(PaintAggregator, ContainedPaintAfterScroll) { @@ -129,13 +165,15 @@ TEST(PaintAggregator, ContainedPaintAfterScroll) { greg.InvalidateRect(paint_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); // expecting a paint rect inside the scroll rect - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); - EXPECT_EQ(paint_rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(scroll_rect, update.scroll_rect); + EXPECT_EQ(paint_rect, update.paint_rects[0]); } TEST(PaintAggregator, ContainedPaintBeforeScroll) { @@ -148,15 +186,17 @@ TEST(PaintAggregator, ContainedPaintBeforeScroll) { greg.ScrollRect(2, 0, scroll_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); // Expecting a paint rect inside the scroll rect - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); paint_rect.Offset(2, 0); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); - EXPECT_EQ(paint_rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(scroll_rect, update.scroll_rect); + EXPECT_EQ(paint_rect, update.paint_rects[0]); } TEST(PaintAggregator, ContainedPaintsBeforeAndAfterScroll) { @@ -174,13 +214,15 @@ TEST(PaintAggregator, ContainedPaintsBeforeAndAfterScroll) { gfx::Rect expected_paint_rect = paint_rect2; EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); // Expecting a paint rect inside the scroll rect - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); - EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(scroll_rect, update.scroll_rect); + EXPECT_EQ(expected_paint_rect, update.paint_rects[0]); } TEST(PaintAggregator, LargeContainedPaintAfterScroll) { @@ -193,11 +235,13 @@ TEST(PaintAggregator, LargeContainedPaintAfterScroll) { greg.InvalidateRect(paint_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_TRUE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(scroll_rect, update.paint_rects[0]); } TEST(PaintAggregator, LargeContainedPaintBeforeScroll) { @@ -210,11 +254,13 @@ TEST(PaintAggregator, LargeContainedPaintBeforeScroll) { greg.ScrollRect(0, 1, scroll_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_TRUE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(scroll_rect, update.paint_rects[0]); } TEST(PaintAggregator, OverlappingPaintBeforeScroll) { @@ -229,11 +275,13 @@ TEST(PaintAggregator, OverlappingPaintBeforeScroll) { gfx::Rect expected_paint_rect = scroll_rect.Union(paint_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_TRUE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(expected_paint_rect, update.paint_rects[0]); } TEST(PaintAggregator, OverlappingPaintAfterScroll) { @@ -248,11 +296,13 @@ TEST(PaintAggregator, OverlappingPaintAfterScroll) { gfx::Rect expected_paint_rect = scroll_rect.Union(paint_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_TRUE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_TRUE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(expected_paint_rect, update.paint_rects[0]); } TEST(PaintAggregator, DisjointPaintBeforeScroll) { @@ -265,12 +315,14 @@ TEST(PaintAggregator, DisjointPaintBeforeScroll) { greg.ScrollRect(2, 0, scroll_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(paint_rect, greg.GetPendingUpdate().paint_rects[0]); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); + EXPECT_EQ(paint_rect, update.paint_rects[0]); + EXPECT_EQ(scroll_rect, update.scroll_rect); } TEST(PaintAggregator, DisjointPaintAfterScroll) { @@ -283,12 +335,14 @@ TEST(PaintAggregator, DisjointPaintAfterScroll) { greg.InvalidateRect(paint_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(paint_rect, greg.GetPendingUpdate().paint_rects[0]); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); + EXPECT_EQ(paint_rect, update.paint_rects[0]); + EXPECT_EQ(scroll_rect, update.scroll_rect); } TEST(PaintAggregator, ContainedPaintTrimmedByScroll) { @@ -304,12 +358,14 @@ TEST(PaintAggregator, ContainedPaintTrimmedByScroll) { gfx::Rect expected_paint_rect(6, 4, 4, 6); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); + EXPECT_EQ(expected_paint_rect, update.paint_rects[0]); + EXPECT_EQ(scroll_rect, update.scroll_rect); } TEST(PaintAggregator, ContainedPaintEliminatedByScroll) { @@ -322,11 +378,13 @@ TEST(PaintAggregator, ContainedPaintEliminatedByScroll) { greg.ScrollRect(6, 0, scroll_rect); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_TRUE(greg.GetPendingUpdate().paint_rects.empty()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_TRUE(update.paint_rects.empty()); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); + EXPECT_EQ(scroll_rect, update.scroll_rect); } TEST(PaintAggregator, ContainedPaintAfterScrollTrimmedByScrollDamage) { @@ -342,13 +400,15 @@ TEST(PaintAggregator, ContainedPaintAfterScrollTrimmedByScrollDamage) { gfx::Rect expected_paint_rect(4, 0, 2, 10); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_EQ(1U, greg.GetPendingUpdate().paint_rects.size()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_EQ(1U, update.paint_rects.size()); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); - EXPECT_EQ(expected_scroll_damage, greg.GetPendingUpdate().GetScrollDamage()); - EXPECT_EQ(expected_paint_rect, greg.GetPendingUpdate().paint_rects[0]); + EXPECT_EQ(scroll_rect, update.scroll_rect); + EXPECT_EQ(expected_scroll_damage, update.GetScrollDamage()); + EXPECT_EQ(expected_paint_rect, update.paint_rects[0]); } TEST(PaintAggregator, ContainedPaintAfterScrollEliminatedByScrollDamage) { @@ -363,10 +423,12 @@ TEST(PaintAggregator, ContainedPaintAfterScrollEliminatedByScrollDamage) { gfx::Rect expected_scroll_damage(0, 0, 4, 10); EXPECT_TRUE(greg.HasPendingUpdate()); + PaintAggregator::PendingUpdate update; + greg.PopPendingUpdate(&update); - EXPECT_FALSE(greg.GetPendingUpdate().scroll_rect.IsEmpty()); - EXPECT_TRUE(greg.GetPendingUpdate().paint_rects.empty()); + EXPECT_FALSE(update.scroll_rect.IsEmpty()); + EXPECT_TRUE(update.paint_rects.empty()); - EXPECT_EQ(scroll_rect, greg.GetPendingUpdate().scroll_rect); - EXPECT_EQ(expected_scroll_damage, greg.GetPendingUpdate().GetScrollDamage()); + EXPECT_EQ(scroll_rect, update.scroll_rect); + EXPECT_EQ(expected_scroll_damage, update.GetScrollDamage()); } diff --git a/chrome/renderer/render_widget.cc b/chrome/renderer/render_widget.cc index ee1ccd9..13c194d 100644 --- a/chrome/renderer/render_widget.cc +++ b/chrome/renderer/render_widget.cc @@ -467,8 +467,8 @@ void RenderWidget::DoDeferredUpdate() { // OK, save the pending update to a local since painting may cause more // invalidation. Some WebCore rendering objects only layout when painted. - PaintAggregator::PendingUpdate update = paint_aggregator_.GetPendingUpdate(); - paint_aggregator_.ClearPendingUpdate(); + PaintAggregator::PendingUpdate update; + paint_aggregator_.PopPendingUpdate(&update); gfx::Rect scroll_damage = update.GetScrollDamage(); gfx::Rect bounds = update.GetPaintBounds().Union(scroll_damage); @@ -505,13 +505,6 @@ void RenderWidget::DoDeferredUpdate() { 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. See bug 29589. - if (update.scroll_rect.IsEmpty()) { - update.paint_rects.clear(); - update.paint_rects.push_back(bounds); - } - // The scroll damage is just another rectangle to paint and copy. copy_rects.swap(update.paint_rects); if (!scroll_damage.IsEmpty()) |