Improve overdraw with multiple windows

cc/trees/occlusion_tracker_unittest.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/trees/occlusion_tracker.h"
 
 #include <stddef.h>
 
 #include "cc/animation/animation_host.h"
 #include "cc/base/filter_operation.h"
@@ skipping 1420 matching lines @@
           parent, this->identity_matrix, gfx::PointF(occlusion_rect.origin()),
           occlusion_rect.size(), true);
       occluding_layer->test_properties()->force_render_surface = false;
       this->CalcDrawEtc(parent);
 
       TestOcclusionTrackerWithClip occlusion(gfx::Rect(0, 0, 200, 200));
 
       // This layer occludes pixels directly beside the filtered_surface.
       // Because filtered surface blends pixels in a radius, it will need to see
       // some of the pixels (up to radius far) underneath the occluding layers.
       this->VisitLayer(occluding_layer, &occlusion);

[danakj, 2017/05/17 15:15:56]

This line would create an occluded area of 0,0,50,200 in the root target (from
occluding_layer)

 
       EXPECT_EQ(occlusion_rect.ToString(),
                 occlusion.occlusion_from_inside_target().ToString());
       EXPECT_TRUE(occlusion.occlusion_from_outside_target().IsEmpty());
 
       this->VisitLayer(filtered_surface, &occlusion);

[danakj, 2017/05/17 15:15:56]

filtered_surface is not opaque, so no occlusion added. but this moves our
current context to be relative to the filtered_surface target instead of the
root.

the occlusion from the root surface is now |occlusion_from_outside_target|,
which is (-50, -50, 50, 200) because in this context the the origin is the
top-left of the filtered_surface's target

 
       // The occlusion is used fully inside the surface.
       gfx::Rect occlusion_inside_surface =
           occlusion_rect - gfx::Vector2d(50, 50);
       EXPECT_TRUE(occlusion.occlusion_from_inside_target().IsEmpty());
       EXPECT_EQ(occlusion_inside_surface.ToString(),

[danakj, 2017/05/17 15:15:56]

which is verified here

                 occlusion.occlusion_from_outside_target().ToString());
 
       // The surface has a background filter, so it needs pixels that are
       // currently considered occluded in order to be drawn. The pixels it
       // needs should be removed from the occluded area, so that they are drawn
       // when we get to the parent.
       this->VisitContributingSurface(filtered_surface, &occlusion);

[danakj, 2017/05/17 15:15:57]

this is moving the current context back to the root target, and applying any
occlusion or state from inside the filtered_surface's target to the root.

part of that is the fact there's this background filter, so we apply it by
reducing the occlusion in the root target.

I don't think that your patch should have changed anything up till here, since
there's only been a single rect occluding anything, and that's seen by the
expectations not changing.

       this->EnterLayer(parent, &occlusion);

[danakj, 2017/05/17 15:15:57]

this enters the root layer to ask what parts of it are still considered
occluded.

without the background filter, this would have been 0,0,50,200. but we had to
shrink this rect from the right side of it because the background filter is
going to expose some of those pixels.

before, we shrank that rect uniformly by 25px. leaving us with 0,0,25,200.

After this patch tho, we have 0,25,45,80. That means we shrank the rect from the
right by less, only by 5 pixels now. But also from the top by 25, shrank it from
the bottom by 95.

This is super surprising to me. Why would changing how we add rects to the
region mean that this now subtracts a smaller area from the right, and is
subtracting from the top and bottom.

Also, subtracting less from the right means we're considering pixels as occluded
that used to be not. The old comment said that the background filter exposes 25
pixels, but now we're only exposing 5 pixels from the right, so we'll keep
occluded pixels that the filter uses?

 
       gfx::Rect expected_occlusion;
       switch (i) {
         case LEFT:
-          // The right half of the occlusion is close enough to cast a shadow

[danakj, 2017/05/15 21:47:43]

I think this part of the comment should be kept, explaining that the shadow is
visible in the background filter.

[yiyix, 2017/05/17 03:00:32]

Done.

-          // that would be visible in the background filter. The shadow reaches
-          // 3*5 + 10 = 25 pixels to the right.

[danakj, 2017/05/15 21:47:43]

But I don't actually understand what's shrinking the rect more from what you
wrote there. Theres a (0,0,50,200) area that could occlude. The shadow shrinks
that on the left side by 25. But what shrinks it from the right and bottom
sides?

Did we pick something else to use as the occlusion instead of |occluding_layer|?

[yiyix, 2017/05/17 03:00:32]

From what I understand, there are 2 rects: occluding_layer: (0, 0, 50. 200) &&
filtered_surface: (50, 50, 100, 100). The filter surface is first transformed to
(50, 50, 50, 50) by |scale_by_half|, then it become (25, 25, 80, 80) after
applying filter. After "expanding the non-opaque pixels outside the rect", the
rect in filter surface become (25, 25, 20, 80). This process is not explained in
the original test description. I can put some transformation details in the
description.

-          expected_occlusion = gfx::Rect(0, 0, 25, 200);
+          // The occlusion needs for the shadow reaches 3*5+10 pixel to the

[danakj, 2017/05/15 21:47:42]

pixels

[yiyix, 2017/05/17 03:00:32]

Done.

+          // right and the rect in filtered_surface which is (25, 25, 20*80), so

[danakj, 2017/05/15 21:47:43]

20, 80

[yiyix, 2017/05/17 03:00:32]

Done.

+          // the expected occlusion is (0, 25, 45*80).

[danakj, 2017/05/15 21:47:43]

45, 80  (it reads like a triple with a product otherwise without more context)

[yiyix, 2017/05/17 03:00:32]

Done.

+          expected_occlusion = gfx::Rect(0, 25, 45, 80);
           break;
         case RIGHT:
           // The shadow spreads 3*5 - 10 = 5 pixels to the left, so the
           // occlusion must recede by 5 to account for that.
           expected_occlusion = gfx::Rect(105, 0, 45, 200);
           break;
         case TOP:
           // Similar to LEFT.
-          expected_occlusion = gfx::Rect(0, 0, 200, 25);
+          expected_occlusion = gfx::Rect(25, 0, 80, 45);
           break;
         case BOTTOM:
           // Similar to RIGHT.
           expected_occlusion = gfx::Rect(0, 105, 200, 45);
           break;
       }
 
       EXPECT_EQ(expected_occlusion.ToString(),
                 occlusion.occlusion_from_inside_target().ToString());
       EXPECT_TRUE(occlusion.occlusion_from_outside_target().IsEmpty());
@@ skipping 944 matching lines @@
     EXPECT_EQ(gfx::Rect(),
               occlusion.UnoccludedSurfaceContentRect(
                   surface, gfx::Rect(80, 70, 50, 50)));
   }
 };
 
 ALL_OCCLUSIONTRACKER_TEST(OcclusionTrackerTestUnoccludedSurfaceQuery)
 
 }  // namespace
 }  // namespace cc