cc: Stop locking the raster scale factor at 1 after any change.

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 bbb2fb217cc98d24cafe8b41380e4024a963ed23..7c48d4d7fc7cba4929ad57f9c3b26a799b336834 100644
--- a/cc/layers/picture_layer_impl_unittest.cc
+++ b/cc/layers/picture_layer_impl_unittest.cc
@@ -1172,44 +1172,72 @@ TEST_F(PictureLayerImplTest, CleanUpTilings) {
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
   ASSERT_EQ(4u, active_layer_->tilings()->num_tilings());
 
-  // Now move the ideal scale to 0.5. Our target stays 1.2.
+  // Now move the ideal scale to 0.5.
   SetContentsScaleOnBothLayers(0.5f, 1.f, page_scale, 1.f, 0.f, false);
+  ASSERT_EQ(6u, active_layer_->tilings()->num_tilings());
 
-  // The high resolution tiling is between target and ideal, so is not
-  // removed.  The low res tiling for the old ideal=1.0 scale is removed.
-  used_tilings.clear();
-  active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
-  ASSERT_EQ(3u, active_layer_->tilings()->num_tilings());
-
-  // Now move the ideal scale to 1.0. Our target stays 1.2.
-  SetContentsScaleOnBothLayers(1.f, 1.f, page_scale, 1.f, 0.f, false);
+  // And begin an animation to 1.2. There are now 3 tilings, 0.5, 1.0 and 1.2.
+  // The the 0.5 is ideal, but the 1.2 is high res. 1.0 sits between them.
+  // high-res tiling is in the middle. There are also ex-low-res tilings paired
+  // with each of the 3 tilings mentioned here, but since we're animating none

[enne (OOO), 2015/11/24 22:05:30]

Maybe offtopic, but do we really need these low res tilings? Especially like the
1.2 one, shouldn't the 1.0 low res tiling be plenty ok (and maybe even the 0.5
one ok for all of them)?

This wasn't a problem really in the past, but now that we change the scale more
often, this seems like it'd be a problem?

[danakj, 2015/11/24 22:06:58]

They get cleaned up by CleanUpTilingsOnActiveLayer when we draw. Notice this
test didn't call that when it changed scale to 0.5 and back.

So I think we should clean up reasonably because we will draw for each scale
change in practice (for each commit).

+  // are marked as LOW_RESOLUTION.
+  SetContentsScaleOnBothLayers(0.5, 1.f, page_scale, 1.2f, 0.f, true);
+  ASSERT_EQ(6u, active_layer_->tilings()->num_tilings());
+  EXPECT_EQ(HIGH_RESOLUTION,
+            active_layer_->tilings()->tiling_at(0)->resolution());
+  EXPECT_EQ(1.2f,  // High-res, though not the ideal scale.
+            active_layer_->tilings()->tiling_at(0)->contents_scale());
+  EXPECT_EQ(1.f,  // Tiling in the middle attack.
+            active_layer_->tilings()->tiling_at(1)->contents_scale());
+  EXPECT_EQ(0.5f,  // Ideal scale, but not considered high-res.
+            active_layer_->tilings()->tiling_at(2)->contents_scale());
 
-  // All the tilings are between are target and the ideal, so they are not
-  // removed.
+  EXPECT_EQ(NON_IDEAL_RESOLUTION,
+            active_layer_->tilings()->tiling_at(1)->resolution());
+  EXPECT_EQ(NON_IDEAL_RESOLUTION,
+            active_layer_->tilings()->tiling_at(2)->resolution());
+  EXPECT_EQ(NON_IDEAL_RESOLUTION,
+            active_layer_->tilings()->tiling_at(3)->resolution());
+  EXPECT_EQ(NON_IDEAL_RESOLUTION,
+            active_layer_->tilings()->tiling_at(4)->resolution());
+  EXPECT_EQ(NON_IDEAL_RESOLUTION,
+            active_layer_->tilings()->tiling_at(5)->resolution());
+
+  EXPECT_EQ(1.2f * low_res_factor,  // Low-res, paired with current high-res.
+            active_layer_->tilings()->tiling_at(3)->contents_scale());

[vmpstr, 2015/11/24 22:14:40]

Out of curiosity, why is the low res pair of the HIGH_RESOLUTION tiling not
marked LOW_RESOLUTION? Are we in an animation here?

[danakj, 2015/11/24 22:16:20]

Right, the true on line 1184 is animation. The comment on 1182 tries to explain
this.

+  EXPECT_EQ(1.f * low_res_factor,  // Ex-low-res, paired with the middle one.
+            active_layer_->tilings()->tiling_at(4)->contents_scale());
+  EXPECT_EQ(0.5f * low_res_factor,  // Ex-low-res, paired with current ideal.
+            active_layer_->tilings()->tiling_at(5)->contents_scale());
+
+  // The high resolution tiling is between target and ideal, so is not removed.
+  // The ex-low-res tilings are all smaller scale, so they are removed.
   used_tilings.clear();
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
   ASSERT_EQ(3u, active_layer_->tilings()->num_tilings());
 
-  // Now move the ideal scale to 1.1 on the active layer. Our target stays 1.2.
-  SetupDrawPropertiesAndUpdateTiles(active_layer_, 1.1f, 1.f, page_scale, 1.f,
-                                    0.f, false);
+  // Now move the ideal scale to 1.0 on the active layer. The high-res tiling
+  // stays 1.2.
+  SetupDrawPropertiesAndUpdateTiles(active_layer_, 1.f, 1.f, page_scale, 1.2f,
+                                    0.f, true);
 
-  // Because the pending layer's ideal scale is still 1.0, our tilings fall
-  // in the range [1.0,1.2] and are kept.
+  // All the tilings are between are target and the ideal, so they are not
+  // Because the pending layer's ideal scale is still 0.5, all tilings fall
+  // in the range [0.5,1.2] and are kept.
   used_tilings.clear();
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
   ASSERT_EQ(3u, active_layer_->tilings()->num_tilings());
 
-  // Move the ideal scale on the pending layer to 1.1 as well. Our target stays
-  // 1.2 still.
-  SetupDrawPropertiesAndUpdateTiles(pending_layer_, 1.1f, 1.f, page_scale, 1.f,
+  // Move the ideal scale on the pending layer to 1.0 as well. Our high-res
+  // stays 1.2 still.
+  SetupDrawPropertiesAndUpdateTiles(pending_layer_, 1.f, 1.f, page_scale, 1.f,
                                     0.f, false);
 
-  // Our 1.0 tiling now falls outside the range between our ideal scale and our
-  // target raster scale. But it is in our used tilings set, so nothing is
+  // Our 0.5 tiling now falls outside the range between our ideal scale and our
+  // high-res raster scale. But it is in our used tilings set, so nothing is
   // deleted.
   used_tilings.clear();
-  used_tilings.push_back(active_layer_->tilings()->tiling_at(1));
+  used_tilings.push_back(active_layer_->tilings()->tiling_at(2));
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
   ASSERT_EQ(3u, active_layer_->tilings()->num_tilings());
 
@@ -2029,8 +2057,9 @@ TEST_F(PictureLayerImplTest,
   gfx::Size viewport_size(400, 400);
 
   host_impl_.SetViewportSize(viewport_size);
-  SetInitialDeviceScaleFactor(2.f);
 
+  // Start with an ideal and raster contents scale of 2.
+  SetInitialDeviceScaleFactor(2.f);
   SetupDefaultTreesWithFixedTileSize(layer_bounds, tile_size, Region());
 
   // One ideal tile exists, this will get used when drawing.
@@ -2040,18 +2069,17 @@ TEST_F(PictureLayerImplTest,
   host_impl_.tile_manager()->InitializeTilesWithResourcesForTesting(
       ideal_tiles);
 
-  // Due to layer scale throttling, the raster contents scale is changed to 1,
-  // while the ideal is still 2.
-  SetupDrawPropertiesAndUpdateTiles(active_layer_, 1.f, 1.f, 1.f, 1.f, 0.f,
-                                    false);
+  // Due to the animation, the raster contents scale moves to 1, while the
+  // ideal will still be 2.
   SetupDrawPropertiesAndUpdateTiles(active_layer_, 2.f, 1.f, 1.f, 1.f, 0.f,
-                                    false);
+                                    true);
 
   EXPECT_EQ(1.f, active_layer_->HighResTiling()->contents_scale());
   EXPECT_EQ(1.f, active_layer_->raster_contents_scale());
   EXPECT_EQ(2.f, active_layer_->ideal_contents_scale());
 
   // Both tilings still exist.
+  ASSERT_GE(active_layer_->tilings()->num_tilings(), 2u);
   EXPECT_EQ(2.f, active_layer_->tilings()->tiling_at(0)->contents_scale());
   EXPECT_EQ(1.f, active_layer_->tilings()->tiling_at(1)->contents_scale());
 
@@ -3322,23 +3350,22 @@ TEST_F(PictureLayerImplTest, RasterScaleChangeWithoutAnimation) {
                                starting_animation_scale, animating_transform);
   EXPECT_BOTH_EQ(HighResTiling()->contents_scale(), 2.f);
 
-  // Changing the source scale without being in an animation will cause
-  // the layer to reset its source scale to 1.f.
+  // Changing the source scale without being in an animation should be
+  // immediately honored.
   contents_scale = 3.f;
 
   SetContentsScaleOnBothLayers(contents_scale, device_scale, page_scale,
                                maximum_animation_scale,
                                starting_animation_scale, animating_transform);
-  EXPECT_BOTH_EQ(HighResTiling()->contents_scale(), 1.f);
+  EXPECT_BOTH_EQ(HighResTiling()->contents_scale(), 3.f);
 
-  // Further changes to the source scale will no longer be reflected in the
-  // contents scale.
+  // Further changes to the source scale will continue to be honored.
   contents_scale = 0.5f;
 
   SetContentsScaleOnBothLayers(contents_scale, device_scale, page_scale,
                                maximum_animation_scale,
                                starting_animation_scale, animating_transform);
-  EXPECT_BOTH_EQ(HighResTiling()->contents_scale(), 1.f);
+  EXPECT_BOTH_EQ(HighResTiling()->contents_scale(), 0.5f);
 }
 
 TEST_F(PictureLayerImplTest, LowResReadyToDrawNotEnoughToActivate) {
@@ -3708,52 +3735,58 @@ TEST_F(NoLowResPictureLayerImplTest, CleanUpTilings) {
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
   ASSERT_EQ(2u, active_layer_->tilings()->num_tilings());
 
-  // Now move the ideal scale to 0.5. Our target stays 1.2.
+  // Now move the ideal scale to 0.5.
   SetContentsScaleOnBothLayers(0.5f, device_scale, page_scale, 1.f, 0.f, false);
+  ASSERT_EQ(3u, active_layer_->tilings()->num_tilings());
 
-  // The high resolution tiling is between target and ideal, so is not
-  // removed.  The low res tiling for the old ideal=1.0 scale is removed.
-  used_tilings.clear();
-  active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
-  ASSERT_EQ(2u, active_layer_->tilings()->num_tilings());
-
-  // Now move the ideal scale to 1.0. Our target stays 1.2.
-  SetContentsScaleOnBothLayers(1.f, device_scale, page_scale, 1.f, 0.f, false);
+  // And begin an animation to 1.2. There are now 3 tilings, 0.5, 1.0 and 1.2.
+  // The the 0.5 is ideal, but the 1.2 is high res. 1.0 sits between them.
+  // high-res tiling is in the middle.
+  SetContentsScaleOnBothLayers(0.5, device_scale, page_scale, 1.2f, 0.f, true);
+  EXPECT_EQ(HIGH_RESOLUTION,
+            active_layer_->tilings()->tiling_at(0)->resolution());
+  EXPECT_EQ(1.2f,  // High-res, though not the ideal scale.
+            active_layer_->tilings()->tiling_at(0)->contents_scale());
+  EXPECT_EQ(1.f,  // Tiling in the middle attack.
+            active_layer_->tilings()->tiling_at(1)->contents_scale());
+  EXPECT_EQ(0.5f,  // Ideal scale, but not considered high-res.
+            active_layer_->tilings()->tiling_at(2)->contents_scale());
 
-  // All the tilings are between are target and the ideal, so they are not
+  // The high resolution tiling is between target and ideal, so is not
   // removed.
   used_tilings.clear();
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
-  ASSERT_EQ(2u, active_layer_->tilings()->num_tilings());
+  ASSERT_EQ(3u, active_layer_->tilings()->num_tilings());
 
-  // Now move the ideal scale to 1.1 on the active layer. Our target stays 1.2.
-  SetupDrawPropertiesAndUpdateTiles(active_layer_, 1.1f, device_scale,
-                                    page_scale, 1.f, 0.f, false);
+  // Now move the ideal scale to 1.0 on the active layer. The high-res tiling
+  // stays 1.2.
+  SetupDrawPropertiesAndUpdateTiles(active_layer_, 1.f, device_scale,
+                                    page_scale, 1.2f, 0.f, true);
 
-  // Because the pending layer's ideal scale is still 1.0, our tilings fall
-  // in the range [1.0,1.2] and are kept.
+  // Because the pending layer's ideal scale is still 0.5, all tilings fall
+  // in the range [0.5,1.2] and are kept.
   used_tilings.clear();
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
-  ASSERT_EQ(2u, active_layer_->tilings()->num_tilings());
+  ASSERT_EQ(3u, active_layer_->tilings()->num_tilings());
 
-  // Move the ideal scale on the pending layer to 1.1 as well. Our target stays
-  // 1.2 still.
-  SetupDrawPropertiesAndUpdateTiles(pending_layer_, 1.1f, device_scale,
+  // Move the ideal scale on the pending layer to 1.0 as well. Our high-res
+  // stays 1.2 still.
+  SetupDrawPropertiesAndUpdateTiles(pending_layer_, 1.f, device_scale,
                                     page_scale, 1.f, 0.f, false);
 
-  // Our 1.0 tiling now falls outside the range between our ideal scale and our
-  // target raster scale. But it is in our used tilings set, so nothing is
+  // Our 0.5 tiling now falls outside the range between our ideal scale and our
+  // high-res raster scale. But it is in our used tilings set, so nothing is
   // deleted.
   used_tilings.clear();
-  used_tilings.push_back(active_layer_->tilings()->tiling_at(1));
+  used_tilings.push_back(active_layer_->tilings()->tiling_at(2));
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
-  ASSERT_EQ(2u, active_layer_->tilings()->num_tilings());
+  ASSERT_EQ(3u, active_layer_->tilings()->num_tilings());
 
   // If we remove it from our used tilings set, it is outside the range to keep
   // so it is deleted.
   used_tilings.clear();
   active_layer_->CleanUpTilingsOnActiveLayer(used_tilings);
-  ASSERT_EQ(1u, active_layer_->tilings()->num_tilings());
+  ASSERT_EQ(2u, active_layer_->tilings()->num_tilings());
 }
 
 TEST_F(NoLowResPictureLayerImplTest, ReleaseResources) {