Skip to content

Commit

Permalink
Do not map empty rect in FilterOperation.
Browse files Browse the repository at this point in the history
If the input rect is empty, we do not want to map the rect to a
non-empty rect. Because it could cause problem, for example, in
damage_tracker to have a damaged region due to having pixel-moving
filters.

DamageTrackerTest.VerifyDamageForBackgroundBlurredChild

Bug: 745101
Test: DamageTrackerTest.VerifyDamageForImageFilter and
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I8719f5ad2e6da08b3a95df45c5194532c3bc5bd4
Reviewed-on: https://chromium-review.googlesource.com/587090
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Ali Juma <ajuma@chromium.org>
Reviewed-by: Stephen White <senorblanco@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490580}
  • Loading branch information
wutao authored and Commit Bot committed Jul 29, 2017
1 parent a20d78e commit 92b4855
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
8 changes: 4 additions & 4 deletions cc/trees/damage_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ void DamageTracker::ComputeSurfaceDamage(RenderSurfaceImpl* render_surface) {

gfx::Rect damage_rect;
bool is_rect_valid = damage_for_this_update_.GetAsRect(&damage_rect);
if (is_rect_valid) {
if (is_rect_valid && !damage_rect.IsEmpty()) {
damage_rect = render_surface->Filters().MapRect(
damage_rect, render_surface->SurfaceScale().matrix());
damage_for_this_update_ = DamageAccumulator();
Expand Down Expand Up @@ -331,9 +331,9 @@ void DamageTracker::ExpandDamageInsideRectWithFilters(
const FilterOperations& filters) {
gfx::Rect damage_rect;
bool is_valid_rect = damage_for_this_update_.GetAsRect(&damage_rect);
// If the damage accumulated so far isn't a valid rect, then there is no point
// in trying to make it bigger.
if (!is_valid_rect)
// If the damage accumulated so far isn't a valid rect or empty, then there is
// no point in trying to make it bigger.
if (!is_valid_rect || damage_rect.IsEmpty())
return;

// Compute the pixels in the background of the surface that could be affected
Expand Down
33 changes: 33 additions & 0 deletions cc/trees/damage_tracker_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,19 @@ TEST_F(DamageTrackerTest, VerifyDamageForImageFilter) {

// gfx::Rect(0, 0, 1, 1), expanded by 6px for the 2px blur filter.
EXPECT_EQ(gfx::Rect(-6, -6, 13, 13), child_damage_rect);

// CASE 2: No changes, so should not damage the surface.
ClearDamageForAllSurfaces(root);
EmulateDrawingOneFrame(root);

EXPECT_TRUE(GetRenderSurface(root)->damage_tracker()->GetDamageRectIfValid(
&root_damage_rect));
EXPECT_TRUE(GetRenderSurface(child)->damage_tracker()->GetDamageRectIfValid(
&child_damage_rect));

// Should not be expanded by the blur filter.
EXPECT_EQ(gfx::Rect(), root_damage_rect);
EXPECT_EQ(gfx::Rect(), child_damage_rect);
}

TEST_F(DamageTrackerTest, VerifyDamageForTransformedImageFilter) {
Expand Down Expand Up @@ -847,6 +860,26 @@ TEST_F(DamageTrackerTest, VerifyDamageForBackgroundBlurredChild) {
// 100,100.
expected_damage_rect = gfx::Rect(100, 100, 7, 7);
EXPECT_EQ(expected_damage_rect.ToString(), root_damage_rect.ToString());

// CASE 7: No changes, so should not damage the surface.
ClearDamageForAllSurfaces(root);
// We want to make sure that the background filter doesn't cause empty damage
// to get expanded. We position child1 so that an expansion of the empty rect
// would have non-empty intersection with child1 in its target space (root
// space).
child1->SetPosition(gfx::PointF());
root->layer_tree_impl()->property_trees()->needs_rebuild = true;
EmulateDrawingOneFrame(root);

gfx::Rect child_damage_rect;
EXPECT_TRUE(GetRenderSurface(root)->damage_tracker()->GetDamageRectIfValid(
&root_damage_rect));
EXPECT_TRUE(GetRenderSurface(child1)->damage_tracker()->GetDamageRectIfValid(
&child_damage_rect));

// Should not be expanded by the blur filter.
EXPECT_EQ(gfx::Rect(), root_damage_rect);
EXPECT_EQ(gfx::Rect(), child_damage_rect);
}

TEST_F(DamageTrackerTest, VerifyDamageForAddingAndRemovingLayer) {
Expand Down

0 comments on commit 92b4855

Please sign in to comment.