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

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

Currently if the PictureLayerImpl's transform scale changes ever
outside of a CSS animation, we lock the raster scale to 1 to prevent
any re-raster work from happening.

This leads to really inconsistent/buggy/unpredictable behaviour for
web developers. Sometimes this only becomes visible after switching
tabs and back, causing the old tiling to be evicted.

This optimization was added when CSS animations were very new to the
web, now they are more common and we should expect developers to use
CSS animations if they want to change the scale of an element frequently
as this provides the needed hints to the compositor about how to raster
the content. We cargo-culted this into the impl-side painting
implementation and tweaked it from there.

This patch removes the hueristic entirely around fixing the raster scale,
meaning that we'll always raster at the scale javascript provides on the
layer. This eliminates many "things are blurry" problems from the web
(see bugs 556533, 413636, 368201, and 542166 for some examples).

R=enne, vmpstr
BUG=556533
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/66978ff65abbb473f8662444c2dc0823e0f45795
Cr-Commit-Position: refs/heads/master@{#361551}

[danakj, 2015-11-24 22:03:48]

Description was changed from

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

Currently if the PictureLayerImpl's transform scale changes ever
outside of a CSS animation, we lock the raster scale to 1 to prevent
any re-raster work from happening.

This leads to really inconsistent/buggy/unpredictable behaviour for
web developers. Sometimes this only becomes visible after switching
tabs and back, causing the old tiling to be evicted.

This optimization was added when CSS animations were very new to the
web, now they are more common and we should expect developers to use
CSS animations if they want to change the scale of an element frequently
as this provides the needed hints to the compositor about how to raster
the content.

This patch removes the hueristic entirely around fixing the raster scale,
meaning that we'll always raster at the scale javascript provides on the
layer. This eliminates many "things are blurry" problems from the web
(see bugs 556533, 413636, and 542166 for some examples).

R=enne, vmpstr
BUG=556533
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

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

Currently if the PictureLayerImpl's transform scale changes ever
outside of a CSS animation, we lock the raster scale to 1 to prevent
any re-raster work from happening.

This leads to really inconsistent/buggy/unpredictable behaviour for
web developers. Sometimes this only becomes visible after switching
tabs and back, causing the old tiling to be evicted.

This optimization was added when CSS animations were very new to the
web, now they are more common and we should expect developers to use
CSS animations if they want to change the scale of an element frequently
as this provides the needed hints to the compositor about how to raster
the content.

This patch removes the hueristic entirely around fixing the raster scale,
meaning that we'll always raster at the scale javascript provides on the
layer. This eliminates many "things are blurry" problems from the web
(see bugs 556533, 413636, 368201, and 542166 for some examples).

R=enne, vmpstr
BUG=556533
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[danakj, 2015-11-24 22:04:42]

Description was changed from

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

Currently if the PictureLayerImpl's transform scale changes ever
outside of a CSS animation, we lock the raster scale to 1 to prevent
any re-raster work from happening.

This leads to really inconsistent/buggy/unpredictable behaviour for
web developers. Sometimes this only becomes visible after switching
tabs and back, causing the old tiling to be evicted.

This optimization was added when CSS animations were very new to the
web, now they are more common and we should expect developers to use
CSS animations if they want to change the scale of an element frequently
as this provides the needed hints to the compositor about how to raster
the content.

This patch removes the hueristic entirely around fixing the raster scale,
meaning that we'll always raster at the scale javascript provides on the
layer. This eliminates many "things are blurry" problems from the web
(see bugs 556533, 413636, 368201, and 542166 for some examples).

R=enne, vmpstr
BUG=556533
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

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

Currently if the PictureLayerImpl's transform scale changes ever
outside of a CSS animation, we lock the raster scale to 1 to prevent
any re-raster work from happening.

This leads to really inconsistent/buggy/unpredictable behaviour for
web developers. Sometimes this only becomes visible after switching
tabs and back, causing the old tiling to be evicted.

This optimization was added when CSS animations were very new to the
web, now they are more common and we should expect developers to use
CSS animations if they want to change the scale of an element frequently
as this provides the needed hints to the compositor about how to raster
the content. We cargo-culted this into the impl-side painting
implementation and tweaked it from there.

This patch removes the hueristic entirely around fixing the raster scale,
meaning that we'll always raster at the scale javascript provides on the
layer. This eliminates many "things are blurry" problems from the web
(see bugs 556533, 413636, 368201, and 542166 for some examples).

R=enne, vmpstr
BUG=556533
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

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

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

https://codereview.chromium.org/1474853002/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl_unittest.cc:1182: // with each of the 3 tilings
mentioned here, but since we're animating none
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:59]

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

https://codereview.chromium.org/1474853002/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl_unittest.cc:1182: // with each of the 3 tilings
mentioned here, but since we're animating none
On 2015/11/24 22:05:30, enne wrote:
> 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?

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).

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

lgtm % enne

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

https://codereview.chromium.org/1474853002/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl_unittest.cc:1207:
active_layer_->tilings()->tiling_at(3)->contents_scale());
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]

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

https://codereview.chromium.org/1474853002/diff/1/cc/layers/picture_layer_imp...
cc/layers/picture_layer_impl_unittest.cc:1207:
active_layer_->tilings()->tiling_at(3)->contents_scale());
On 2015/11/24 22:14:40, vmpstr wrote:
> Out of curiosity, why is the low res pair of the HIGH_RESOLUTION tiling not
> marked LOW_RESOLUTION? Are we in an animation here?

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

[enne (OOO), 2015-11-24 22:25:10]

lgtm, thanks!

[danakj, 2015-11-24 22:25:20]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2015-11-24 22:27:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1474853002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1474853002/1

[commit-bot: I haz the power, 2015-11-25 03:25:12]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-11-25 03:26:32]

Patchset 1 (id:??) landed as
https://crrev.com/66978ff65abbb473f8662444c2dc0823e0f45795
Cr-Commit-Position: refs/heads/master@{#361551}

[danakj, 2016-04-19 23:38:00]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1894173004/ by danakj@chromium.org.

The reason for reverting is:
https://bugs.chromium.org/p/chromium/issues/detail?id=600482.