Animation start scale should be considered for the raster scale

Animation start scale should be considered for the raster scale

In zoom-in scenarios the start animation should be taken in to account to
calculate raster scale else the first animation frame is rastered at a
low raster scale which leads to blurred content.

Added starting_animation_contents_scale to cc::DrawProperties

BUG=422341
Committed: https://crrev.com/f0ebecee83117f2bf62b4124491910c4654491d0
Cr-Commit-Position: refs/heads/master@{#328258}

[patro, 2015-04-23 17:56:42]

behara.ms@samsung.com changed reviewers:
+ danakj@chromium.org

[patro, 2015-04-23 17:56:43]

Kindly Review

Thanks
Shyam Patro

[danakj, 2015-04-23 18:24:07]

danakj@chromium.org changed reviewers:
+ ajuma@chromium.org

[danakj, 2015-04-23 18:24:08]

+ajuma

Needs unit tests.

https://codereview.chromium.org/1076313006/diff/1/cc/animation/layer_animatio...
File cc/animation/layer_animation_controller.h (right):

https://codereview.chromium.org/1076313006/diff/1/cc/animation/layer_animatio...
cc/animation/layer_animation_controller.h:137: // Animation start scale
better comment like the one below?

https://codereview.chromium.org/1076313006/diff/1/cc/layers/draw_properties.h
File cc/layers/draw_properties.h (right):

https://codereview.chromium.org/1076313006/diff/1/cc/layers/draw_properties.h...
cc/layers/draw_properties.h:160: // The scale during the layer animation start
at which content
wrap at 80

https://codereview.chromium.org/1076313006/diff/1/cc/layers/picture_layer_imp...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/1076313006/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl.cc:1004: else if (can_raster_at_maximum_scale)
I think we want to prefer the max scale here?

https://codereview.chromium.org/1076313006/diff/1/cc/trees/layer_tree_host_co...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/1076313006/diff/1/cc/trees/layer_tree_host_co...
cc/trees/layer_tree_host_common.cc:1115: std::max(ancestor_transform_scales.x(),
ancestor_transform_scales.y());
do this max once?

[ajuma, 2015-04-23 19:17:01]

https://codereview.chromium.org/1076313006/diff/1/cc/animation/keyframed_anim...
File cc/animation/keyframed_animation_curve.cc (right):

https://codereview.chromium.org/1076313006/diff/1/cc/animation/keyframed_anim...
cc/animation/keyframed_animation_curve.cc:392: gfx::Vector3dF
target_scale_for_segment;
start_scale_for_segment?

https://codereview.chromium.org/1076313006/diff/1/cc/layers/picture_layer_imp...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/1076313006/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl.cc:1002: if (can_raster_at_starting_scale)
Maybe this should be 'should_raster_at_starting_scale', since it already takes
into account whether the starting scale is larger than the maximum scale?

[patro, 2015-04-24 11:47:27]

https://codereview.chromium.org/1076313006/diff/1/cc/animation/keyframed_anim...
File cc/animation/keyframed_animation_curve.cc (right):

https://codereview.chromium.org/1076313006/diff/1/cc/animation/keyframed_anim...
cc/animation/keyframed_animation_curve.cc:392: gfx::Vector3dF
target_scale_for_segment;
On 2015/04/23 19:17:01, ajuma wrote:
> start_scale_for_segment?

Done.

https://codereview.chromium.org/1076313006/diff/1/cc/animation/layer_animatio...
File cc/animation/layer_animation_controller.h (right):

https://codereview.chromium.org/1076313006/diff/1/cc/animation/layer_animatio...
cc/animation/layer_animation_controller.h:137: // Animation start scale
On 2015/04/23 18:24:08, danakj wrote:
> better comment like the one below?

Done.

https://codereview.chromium.org/1076313006/diff/1/cc/layers/draw_properties.h
File cc/layers/draw_properties.h (right):

https://codereview.chromium.org/1076313006/diff/1/cc/layers/draw_properties.h...
cc/layers/draw_properties.h:160: // The scale during the layer animation start
at which content
On 2015/04/23 18:24:08, danakj wrote:
> wrap at 80

Done.

https://codereview.chromium.org/1076313006/diff/1/cc/layers/picture_layer_imp...
File cc/layers/picture_layer_impl.cc (right):

https://codereview.chromium.org/1076313006/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl.cc:1002: if (can_raster_at_starting_scale)
On 2015/04/23 19:17:01, ajuma wrote:
> Maybe this should be 'should_raster_at_starting_scale', since it already takes
> into account whether the starting scale is larger than the maximum scale?

Done.

https://codereview.chromium.org/1076313006/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl.cc:1004: else if (can_raster_at_maximum_scale)
On 2015/04/23 18:24:08, danakj wrote:
> I think we want to prefer the max scale here?

maximum_scale = max(all scales excluding start_scale)

For scale-down animation start_scale > max_scale so we have to raster at
starting scale to get crisp content. For remaining cases we can raster at
maximum_scale to get crisp content...Please correct me if i am wrong

https://codereview.chromium.org/1076313006/diff/1/cc/trees/layer_tree_host_co...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/1076313006/diff/1/cc/trees/layer_tree_host_co...
cc/trees/layer_tree_host_common.cc:1115: std::max(ancestor_transform_scales.x(),
ancestor_transform_scales.y());
On 2015/04/23 18:24:08, danakj wrote:
> do this max once?

Done.

[ajuma, 2015-04-24 13:24:02]

Thanks for adding tests. Please also add a test for the logic that was added to
PictureLayerImpl::RecalculateRasterScales (see
PictureLayerImplTest.HighResTilingDuringAnimationForCpuRasterization for an
example).

https://codereview.chromium.org/1076313006/diff/20001/cc/animation/layer_anim...
File cc/animation/layer_animation_controller_unittest.cc (right):

https://codereview.chromium.org/1076313006/diff/20001/cc/animation/layer_anim...
cc/animation/layer_animation_controller_unittest.cc:1984: //
AnimationStartScale.
The finished animation has a start of scale of 1, so this doesn't seem to be
testing that the finished animation isn't considered (since even if it was being
considered, the result wouldn't change).

[patro, 2015-05-04 12:44:06]

Addressed comments and added new test case

Kindly Review

Thanks
Shyam

https://codereview.chromium.org/1076313006/diff/20001/cc/animation/layer_anim...
File cc/animation/layer_animation_controller_unittest.cc (right):

https://codereview.chromium.org/1076313006/diff/20001/cc/animation/layer_anim...
cc/animation/layer_animation_controller_unittest.cc:1984: //
AnimationStartScale.
On 2015/04/24 13:24:02, ajuma wrote:
> The finished animation has a start of scale of 1, so this doesn't seem to be
> testing that the finished animation isn't considered (since even if it was
being
> considered, the result wouldn't change).

The finished animation has starting scale of 6.0f(The animation direction is
DIRECTION_REVERSE).
The active animation should have lower scale than finished animation so that we
can be sure that it takes only active animations into account.

[ajuma, 2015-05-04 13:37:59]

Thanks, lgtm.

[patro, 2015-05-05 01:37:15]

The CQ bit was checked by behara.ms@samsung.com

[commit-bot: I haz the power, 2015-05-05 01:37:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1076313006/40001

[commit-bot: I haz the power, 2015-05-05 02:52:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-05 02:53:17]

Patchset 3 (id:??) landed as
https://crrev.com/f0ebecee83117f2bf62b4124491910c4654491d0
Cr-Commit-Position: refs/heads/master@{#328258}