Rasterize at maximum scale for scale animations on CPU-rasterized layers

cc/layers/picture_layer_impl.cc

Index: cc/layers/picture_layer_impl.cc
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
index 7017d864c2ea5030090005425d7fa26726d91c70..8e1d37318d420cad19523f330cd67b3cf998a9ec 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -413,6 +413,7 @@ void PictureLayerImpl::CalculateContentsScale(
     float ideal_contents_scale,
     float device_scale_factor,
     float page_scale_factor,
+    float maximum_animation_scale_factor,
     bool animating_transform_to_screen,
     float* contents_scale_x,
     float* contents_scale_y,
@@ -455,7 +456,7 @@ void PictureLayerImpl::CalculateContentsScale(
   ideal_device_scale_ = ideal_device_scale;
   ideal_source_scale_ = std::max(ideal_source_scale, min_source_scale);
 
-  ManageTilings(animating_transform_to_screen);
+  ManageTilings(animating_transform_to_screen, maximum_animation_scale_factor);
 
   // The content scale and bounds for a PictureLayerImpl is somewhat fictitious.
   // There are (usually) several tilings at different scales.  However, the
@@ -917,7 +918,8 @@ inline float PositiveRatio(float float1, float float2) {
 
 }  // namespace
 
-void PictureLayerImpl::ManageTilings(bool animating_transform_to_screen) {
+void PictureLayerImpl::ManageTilings(bool animating_transform_to_screen,
+                                     float maximum_animation_scale_factor) {
   DCHECK(ideal_contents_scale_);
   DCHECK(ideal_page_scale_);
   DCHECK(ideal_device_scale_);
@@ -947,7 +949,8 @@ void PictureLayerImpl::ManageTilings(bool animating_transform_to_screen) {
   if (!layer_tree_impl()->device_viewport_valid_for_tile_management())
     return;
 
-  RecalculateRasterScales(animating_transform_to_screen);
+  RecalculateRasterScales(animating_transform_to_screen,
+                          maximum_animation_scale_factor);
 
   PictureLayerTiling* high_res = NULL;
   PictureLayerTiling* low_res = NULL;
@@ -1000,7 +1003,7 @@ bool PictureLayerImpl::ShouldAdjustRasterScale(
   // tree. This will allow CSS scale changes to get re-rastered at an
   // appropriate rate.
 
-  if (raster_source_scale_was_animating_ && !animating_transform_to_screen)
+  if (raster_source_scale_was_animating_ != animating_transform_to_screen)
     return true;
 
   if (animating_transform_to_screen &&
@@ -1048,7 +1051,8 @@ float PictureLayerImpl::SnappedContentsScale(float scale) {
 }
 
 void PictureLayerImpl::RecalculateRasterScales(
-    bool animating_transform_to_screen) {
+    bool animating_transform_to_screen,
+    float maximum_animation_scale_factor) {
   raster_device_scale_ = ideal_device_scale_;
   raster_source_scale_ = ideal_source_scale_;
 
@@ -1072,11 +1076,18 @@ void PictureLayerImpl::RecalculateRasterScales(
   raster_contents_scale_ =
       std::max(raster_contents_scale_, MinimumContentsScale());
 
-  // Don't allow animating CSS scales to drop below 1 if we're not
-  // re-rasterizing during the animation.
+  // If we're not re-rasterizing during animation, rasterize at the maximum
+  // scale that will occur during the animation, if the maximum scale is
+  // known.
   if (animating_transform_to_screen && !ShouldUseGpuRasterization()) {
-    raster_contents_scale_ = std::max(
-        raster_contents_scale_, 1.f * ideal_page_scale_ * ideal_device_scale_);
+    if (maximum_animation_scale_factor > 0.f) {
+      raster_contents_scale_ =
+          std::max(raster_contents_scale_, maximum_animation_scale_factor);

[enne (OOO), 2014/04/08 19:18:47]

Maybe I'm missing something, but this seems incorrect with respect to device and
page scale.

maximum_animation_scale_factor in CalcDrawProperties looks like it includes
device and page scale because it takes scales from the parent_matrix.  However,
raster contents scale does not include device and page scale in this line of
code.

This is a bikeshed request, but can you name maximum_animation_scale_factor to
be what it is? It no longer seems to be a factor.  Is it
maximum_animation_raster_scale? Is it maximum_animation_contents_scale?

[ajuma, 2014/04/08 19:48:09]

I think I'm confused. ideal_contents_scale is calculated in CalcDrawProperties
as the scale of combined_transform. For LayerImpls, UpdateLayerContentsScale
passes this value directly to CalculateContentsScale, where it is saved as
ideal_contents_scale_, and then set directly to raster_contents_scale_ above (in
the non-pinching case). So it seems like the behavior in the animated case
(taking a scale from parent_transform and from the layer's transform) is
consistent with the non-animated case (taking a scale from combined_transform).
Am I misunderstanding the logic in the non-animated case?

[enne (OOO), 2014/04/08 19:53:19]

I'm trying to assert that maximum_animation_scale_factor is really
maximum_animation_contents_scale based on the way you calculate it in
CalcDrawProps, but you are using it right here as if it were
maximum_animation_raster_scale.  I think if you adjust your test to use non-1
values for device and page scale, you'll see the issue.

[ajuma, 2014/04/08 20:30:40]

Renamed to maximum_animation_contents_scale.

+    } else {
+      raster_contents_scale_ =
+          std::max(raster_contents_scale_,
+                   1.f * ideal_page_scale_ * ideal_device_scale_);
+    }
   }
 
   // If this layer would only create one tile at this content scale,