cc: Move draw params from SetExternalDrawConstraints to OnDraw

cc/layers/picture_layer_impl_unittest.cc

Index: cc/layers/picture_layer_impl_unittest.cc
diff --git a/cc/layers/picture_layer_impl_unittest.cc b/cc/layers/picture_layer_impl_unittest.cc
index cb8bc3ac116aa253591c5e848dc7bdbf983f9b3b..b7e0ba13b67253bf48ffacb548e2df04abab2f68 100644
--- a/cc/layers/picture_layer_impl_unittest.cc
+++ b/cc/layers/picture_layer_impl_unittest.cc
@@ -487,17 +487,11 @@ TEST_F(PictureLayerImplTest, ExternalViewportRectForPrioritizingTiles) {
 
   // Update tiles with viewport for tile priority as (0, 0, 100, 100) and the
   // identify transform for tile priority.
-  bool resourceless_software_draw = false;
-  gfx::Rect viewport = gfx::Rect(layer_bounds),
-            viewport_rect_for_tile_priority = gfx::Rect(0, 0, 100, 100);
+  gfx::Rect viewport_rect_for_tile_priority = gfx::Rect(0, 0, 100, 100);
   gfx::Transform transform, transform_for_tile_priority;
 
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport_rect_for_tile_priority,
-                                        transform_for_tile_priority,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport_rect_for_tile_priority,
+                                        transform_for_tile_priority);
   bool update_lcd_text = false;
   host_impl_.active_tree()->UpdateDrawProperties(update_lcd_text);
 
@@ -525,12 +519,8 @@ TEST_F(PictureLayerImplTest, ExternalViewportRectForPrioritizingTiles) {
   viewport_rect_for_tile_priority = gfx::Rect(200, 200, 100, 100);
   transform_for_tile_priority.Translate(100, 100);
   transform_for_tile_priority.Rotate(45);
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport_rect_for_tile_priority,
-                                        transform_for_tile_priority,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport_rect_for_tile_priority,
+                                        transform_for_tile_priority);
   host_impl_.active_tree()->UpdateDrawProperties(update_lcd_text);
 
   gfx::Transform screen_to_view(gfx::Transform::kSkipInitialization);
@@ -571,12 +561,7 @@ TEST_F(PictureLayerImplTest, InvalidViewportForPrioritizingTiles) {
   bool resourceless_software_draw = false;
   gfx::Rect viewport = gfx::Rect(layer_bounds);
   gfx::Transform transform;
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport,
-                                        transform,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport, transform);
   active_layer_->draw_properties().visible_layer_rect = viewport;
   active_layer_->draw_properties().screen_space_transform = transform;
   active_layer_->UpdateTiles(resourceless_software_draw);
@@ -595,12 +580,7 @@ TEST_F(PictureLayerImplTest, InvalidViewportForPrioritizingTiles) {
   transform.Translate(1.f, 1.f);
   active_layer_->draw_properties().visible_layer_rect = viewport;
   active_layer_->draw_properties().screen_space_transform = transform;
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport,
-                                        transform,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport, transform);
   active_layer_->UpdateTiles(resourceless_software_draw);
 
   // Transform for tile priority is updated.
@@ -613,12 +593,7 @@ TEST_F(PictureLayerImplTest, InvalidViewportForPrioritizingTiles) {
   // Keep expanded viewport but mark it valid. Should update tile viewport.
   host_impl_.AdvanceToNextFrame(base::TimeDelta::FromMilliseconds(200));
   resourceless_software_draw = false;
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport,
-                                        transform,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport, transform);
   active_layer_->UpdateTiles(resourceless_software_draw);
 
   EXPECT_TRANSFORMATION_MATRIX_EQ(transform,
@@ -636,14 +611,11 @@ TEST_F(PictureLayerImplTest, ViewportRectForTilePriorityIsCached) {
 
   host_impl_.AdvanceToNextFrame(base::TimeDelta::FromMilliseconds(200));
 
-  bool resourceless_software_draw = false;
-  gfx::Rect viewport = gfx::Rect(layer_bounds);
   gfx::Rect viewport_rect_for_tile_priority(0, 0, 100, 100);
-  gfx::Transform transform, transform_for_tile_priority;
+  gfx::Transform transform_for_tile_priority;
 
-  host_impl_.SetExternalDrawConstraints(
-      transform, viewport, viewport, viewport_rect_for_tile_priority,
-      transform_for_tile_priority, resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport_rect_for_tile_priority,
+                                        transform_for_tile_priority);
   bool update_lcd_text = false;
   host_impl_.active_tree()->UpdateDrawProperties(update_lcd_text);
 
@@ -653,9 +625,8 @@ TEST_F(PictureLayerImplTest, ViewportRectForTilePriorityIsCached) {
   host_impl_.AdvanceToNextFrame(base::TimeDelta::FromMilliseconds(200));
 
   gfx::Rect another_viewport_rect_for_tile_priority(11, 11, 50, 50);
-  host_impl_.SetExternalDrawConstraints(
-      transform, viewport, viewport, another_viewport_rect_for_tile_priority,
-      transform_for_tile_priority, resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(another_viewport_rect_for_tile_priority,
+                                        transform_for_tile_priority);
 
   // Didn't call UpdateDrawProperties yet. The viewport rect for tile priority
   // should remain to be the previously cached value.
@@ -1778,15 +1749,10 @@ TEST_F(NoLowResPictureLayerImplTest, MarkRequiredOffscreenTiles) {
   gfx::Size layer_bounds(200, 200);
 
   gfx::Transform transform;
-  gfx::Transform transform_for_tile_priority;
   bool resourceless_software_draw = false;
   gfx::Rect viewport(0, 0, 100, 200);
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport,
-                                        transform,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport, transform);
+  host_impl_.SetExternalViewporForTesting(viewport);

[danakj, 2015/11/24 21:17:30]

Why is this needed for this test?

[boliu, 2015/11/24 23:27:37]

Good question..

It's needed for the for the EXPECT_EQ(viewport,
pending_layer_->visible_rect_for_tile_priority()) a few lines below.

And I think this is somewhat expected. visible_rect_for_tile_priority is a bit
of a misnomer I think. It's not derived from
viewport/transform_for_tile_priority; rather it's the last visible_rect
calculated when draw constraints was valid for tile priority (ie not software
draw).

What a mouthful.. :|

Conceptually, it should be the visible rect calculated with
viewport/transform_for_tile_priority. But I guess a second CalcDrawProps is too
expensive?

[danakj, 2015/12/10 22:33:54]

There's a typo "SetExternalViewporForTesting" -> Viewport

On 2015/11/24 23:27:37, boliu wrote:
So... at least an explanation like this should live as a comment. I'm trying to
parse/apply it all myself..

This sets the external_viewport_ which changes the LTHI::DeviceViewport() which
is used in various places.

The EXPECT below is verifying that DeviceViewport is used for the visible rect
when computing visible_content_rect?

We get a tiling with 4 tiles, two in the DeviceViewport and 2 outside. We check
that the tiles inside the DeviceViewport are required for activation and the
ones outside are not?

So I am not sure that this test is the right place to be testing that setting
external_viewport changes the viewport used for the layer's visible rect.
Testing that it good, but this test is checking requires for activation stuff.
This test could just use SetViewport?

I'd rather a separate test that just does OnDraw and checks visible rects.

 
   scoped_refptr<FakeDisplayListRasterSource> pending_raster_source =
       FakeDisplayListRasterSource::CreateFilled(layer_bounds);
@@ -1840,12 +1806,8 @@ TEST_F(NoLowResPictureLayerImplTest,
   gfx::Transform transform;
   gfx::Transform transform_for_tile_priority;
   bool resourceless_software_draw = false;
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        external_viewport_for_tile_priority,
-                                        transform_for_tile_priority,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(external_viewport_for_tile_priority,
+                                        transform_for_tile_priority);
   host_impl_.AdvanceToNextFrame(base::TimeDelta::FromMilliseconds(1));
   bool update_lcd_text = false;
   host_impl_.pending_tree()->UpdateDrawProperties(update_lcd_text);
@@ -3590,12 +3552,7 @@ TEST_F(NoLowResPictureLayerImplTest, InvalidViewportForPrioritizingTiles) {
   bool resourceless_software_draw = false;
   gfx::Rect viewport = gfx::Rect(layer_bounds);
   gfx::Transform transform;
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport,
-                                        transform,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport, transform);
   active_layer_->draw_properties().visible_layer_rect = viewport;
   active_layer_->draw_properties().screen_space_transform = transform;
   active_layer_->UpdateTiles(resourceless_software_draw);
@@ -3614,12 +3571,7 @@ TEST_F(NoLowResPictureLayerImplTest, InvalidViewportForPrioritizingTiles) {
   transform.Translate(1.f, 1.f);
   active_layer_->draw_properties().visible_layer_rect = viewport;
   active_layer_->draw_properties().screen_space_transform = transform;
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport,
-                                        transform,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport, transform);
   active_layer_->UpdateTiles(resourceless_software_draw);
 
   // Transform for tile priority is updated.
@@ -3632,12 +3584,7 @@ TEST_F(NoLowResPictureLayerImplTest, InvalidViewportForPrioritizingTiles) {
   // Keep expanded viewport but mark it valid. Should update tile viewport.
   host_impl_.AdvanceToNextFrame(base::TimeDelta::FromMilliseconds(200));
   resourceless_software_draw = false;
-  host_impl_.SetExternalDrawConstraints(transform,
-                                        viewport,
-                                        viewport,
-                                        viewport,
-                                        transform,
-                                        resourceless_software_draw);
+  host_impl_.SetExternalDrawConstraints(viewport, transform);
   active_layer_->UpdateTiles(resourceless_software_draw);
 
   EXPECT_TRANSFORMATION_MATRIX_EQ(transform,
@@ -4745,13 +4692,8 @@ TEST_F(PictureLayerImplTest, ScrollPastLiveTilesRectAndBack) {
   ActivateTree();
   EXPECT_TRUE(active_layer_->HighResTiling()->has_tiles());
 
-  host_impl_.SetExternalDrawConstraints(
-      gfx::Transform(),              // transform
-      gfx::Rect(),                   // clip
-      gfx::Rect(),                   // viewport
-      gfx::Rect(0, 5000, 100, 100),  // viewport_rect_for_tile_priority
-      gfx::Transform(),              // transform_for_tile_priority
-      false);
+  host_impl_.SetExternalDrawConstraints(gfx::Rect(0, 5000, 100, 100),
+                                        gfx::Transform());
 
   SetupPendingTreeWithFixedTileSize(pending_raster_source, tile_size,
                                     gfx::Rect());
@@ -4762,13 +4704,8 @@ TEST_F(PictureLayerImplTest, ScrollPastLiveTilesRectAndBack) {
   EXPECT_FALSE(active_layer_->HighResTiling()->has_tiles());
   EXPECT_TRUE(active_layer_->HighResTiling()->live_tiles_rect().IsEmpty());
 
-  host_impl_.SetExternalDrawConstraints(
-      gfx::Transform(),             // transform
-      gfx::Rect(),                  // clip
-      gfx::Rect(),                  // viewport
-      gfx::Rect(0, 110, 100, 100),  // viewport_rect_for_tile_priority
-      gfx::Transform(),             // transform_for_tile_priority
-      false);
+  host_impl_.SetExternalDrawConstraints(gfx::Rect(0, 110, 100, 100),
+                                        gfx::Transform());
 
   SetupPendingTreeWithFixedTileSize(pending_raster_source, tile_size,
                                     gfx::Rect());