Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4653)

Unified Diff: cc/trees/occlusion_tracker_unittest.cc

Issue 2119043002: cc: Correct inverted directions in occlusion tracker background filter logic. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 4 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « cc/trees/occlusion_tracker.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: cc/trees/occlusion_tracker_unittest.cc
diff --git a/cc/trees/occlusion_tracker_unittest.cc b/cc/trees/occlusion_tracker_unittest.cc
index 766df2da1743604e315c5435d482e085e386c88c..539f0a397f7a6c4c2e5304744607c7063915b894 100644
--- a/cc/trees/occlusion_tracker_unittest.cc
+++ b/cc/trees/occlusion_tracker_unittest.cc
@@ -1540,6 +1540,125 @@ class OcclusionTrackerTestDontOccludePixelsNeededForBackgroundFilter
ALL_OCCLUSIONTRACKER_TEST(
OcclusionTrackerTestDontOccludePixelsNeededForBackgroundFilter);
+class OcclusionTrackerTestDontOccludePixelsNeededForDropShadowBackgroundFilter
+ : public OcclusionTrackerTest {
+ protected:
+ explicit OcclusionTrackerTestDontOccludePixelsNeededForDropShadowBackgroundFilter(
+ bool opaque_layers)
+ : OcclusionTrackerTest(opaque_layers) {}
+ void RunMyTest() override {
+ gfx::Transform scale_by_half;
+ scale_by_half.Scale(0.5, 0.5);
+
+ FilterOperations filters;
+ filters.Append(FilterOperation::CreateDropShadowFilter(gfx::Point(10, 10),
+ 5, SK_ColorBLACK));
+
+ enum Direction {
+ LEFT,
+ RIGHT,
+ TOP,
+ BOTTOM,
+ LAST_DIRECTION = BOTTOM,
+ };
+
+ for (int i = 0; i <= LAST_DIRECTION; ++i) {
+ SCOPED_TRACE(i);
+
+ // Make a 50x50 filtered surface that is adjacent to occluding layers
+ // which are above it in the z-order in various configurations. The
+ // surface is scaled to test that the pixel moving is done in the target
+ // space, where the background filter is applied.
+ TestContentLayerImpl* parent = this->CreateRoot(
+ this->identity_matrix, gfx::PointF(), gfx::Size(200, 200));
+ LayerImpl* filtered_surface = this->CreateDrawingLayer(
+ parent, scale_by_half, gfx::PointF(50.f, 50.f), gfx::Size(100, 100),
+ false);
+ filtered_surface->test_properties()->background_filters = filters;
+ gfx::Rect occlusion_rect;
+ switch (i) {
+ case LEFT:
+ occlusion_rect = gfx::Rect(0, 0, 50, 200);
+ break;
+ case RIGHT:
+ occlusion_rect = gfx::Rect(100, 0, 50, 200);
enne (OOO) 2016/07/06 18:20:30 Shouldn't an adjacent occlusion layer be (150, 0)
jbroman 2016/07/06 18:42:48 That would also work. This matches what the test a
enne (OOO) 2016/07/06 19:50:00 I'm just saying the comment above that this is adj
jbroman 2016/07/06 20:26:06 Turns out it _is_ adjacent, because it's only 50x5
+ break;
+ case TOP:
+ occlusion_rect = gfx::Rect(0, 0, 200, 50);
+ break;
+ case BOTTOM:
+ occlusion_rect = gfx::Rect(0, 100, 200, 50);
+ break;
+ }
+
+ LayerImpl* occluding_layer = this->CreateDrawingLayer(
+ 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);
+
+ EXPECT_EQ(occlusion_rect.ToString(),
+ occlusion.occlusion_from_inside_target().ToString());
+ EXPECT_TRUE(occlusion.occlusion_from_outside_target().IsEmpty());
+
+ this->VisitLayer(filtered_surface, &occlusion);
+
+ // 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(),
+ 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. So the pixels it
+ // needs should be removed some the occluded area so that when we get to
enne (OOO) 2016/07/06 18:20:30 grammar?
jbroman 2016/07/06 18:42:48 Copied from the previous unit test. Would you like
enne (OOO) 2016/07/06 19:49:59 Yes, please!
jbroman 2016/07/06 20:26:06 Reworded in a way that's hopefully clearer.
+ // the parent they are drawn.
+ this->VisitContributingSurface(filtered_surface, &occlusion);
+ this->EnterLayer(parent, &occlusion);
+
+ gfx::Rect expected_occlusion;
+ switch (i) {
+ case LEFT:
+ // The right half of the occlusion is close enough to cast a shadow
+ // that would be visible in the background filter. The shadow reaches
+ // 3*5 + 10 = 25 pixels to the right.
+ expected_occlusion = gfx::Rect(0, 0, 25, 200);
+ 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);
+ 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());
+
+ this->DestroyLayers();
+ }
+ }
+};
+
+ALL_OCCLUSIONTRACKER_TEST(
+ OcclusionTrackerTestDontOccludePixelsNeededForDropShadowBackgroundFilter);
+
class OcclusionTrackerTestTwoBackgroundFiltersReduceOcclusionTwice
: public OcclusionTrackerTest {
protected:
« no previous file with comments | « cc/trees/occlusion_tracker.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698