Paint invisible layer content in presence of composited animations.

Paint invisible layer content in presence of composited animations.

For SPv2 we must paint layer content even if the content is invisible
w.r.t. opacity.

This is required (a) so that we have a paint chunk, which will produce a layer,
which is required by the animation subsystem, and (b) so that cc has the paint
commands needed in order to actually enact the animation compositor-side through
mutating the property tree state and re-rastering as needed.

In SPv1 this code path (PaintLayerPainter::paintedOutputInvisible) is skipped
when painting a layer for composited animated content. Instead of calling
PaintLayerPainter::paint(), we end up painting the GraphicsLayer for the
composited animation via CompositedLayerMapping::doPaintTask ->
PaintLayerPainter::paintLayerContents.

BUG=684842
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2652273004
Cr-Commit-Position: refs/heads/master@{#446605}
Committed: https://chromium.googlesource.com/chromium/src/+/9ce0839c605303284391c7efacccdcc00dc38437

[wkorman, 2017-01-26 22:45:06]

Description was changed from

==========
Paint invisible layer content in presence of composited animations.

For SPv2 we must paint layer content even if the content is invisible
w.r.t. opacity.

This is required (a) so that we have a paint chunk, which will produce a layer,
which is required by the animation subsystem, and (b) so that cc has the paint
commands needed in order to actually enact the animation compositor-side through
mutating the property tree state and re-rastering as needed.

In SPv1 this code path (PaintLayerPainter::paintedOutputInvisible) is skipped
when painting a layer for composited animated content. Instead of calling
PaintLayerPainter::paint(), we end up painting the GraphicsLayer for the
composited animation via CompositedLayerMapping::doPaintTask ->
PaintLayerPainter::paintLayerContents.

BUG=684842
==========

to

==========
Paint invisible layer content in presence of composited animations.

For SPv2 we must paint layer content even if the content is invisible
w.r.t. opacity.

This is required (a) so that we have a paint chunk, which will produce a layer,
which is required by the animation subsystem, and (b) so that cc has the paint
commands needed in order to actually enact the animation compositor-side through
mutating the property tree state and re-rastering as needed.

In SPv1 this code path (PaintLayerPainter::paintedOutputInvisible) is skipped
when painting a layer for composited animated content. Instead of calling
PaintLayerPainter::paint(), we end up painting the GraphicsLayer for the
composited animation via CompositedLayerMapping::doPaintTask ->
PaintLayerPainter::paintLayerContents.

BUG=684842
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[wkorman, 2017-01-26 22:47:56]

wkorman@chromium.org changed reviewers:
+ chrishtr@chromium.org, pdr@chromium.org

[wkorman, 2017-01-26 22:47:56]

Will add tests presuming this passes initial review.

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp (left):

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:514:
CompositingReasonFinder::requiresCompositingForEffectAnimation(style);
This was a bug -- we were assigning the bool result to an int reason var. Now
fixed below.

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/paint/EffectPaintPropertyNode.h
(right):

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/paint/EffectPaintPropertyNode.h:116:
CompositingReasons compositingReasons() const {
If we'd prefer not to expose compositing reasons directly from the node we could
add accessors to each of these such as requiresCompositingForAnimation() that
performs the check internally.

[pdr., 2017-01-26 22:49:35]

LGTM

[pdr., 2017-01-26 22:50:38]

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp (right):

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:74: if
(layoutObject.styleRef().opacity())
Do you need to check opacity > kMinimumVisibleOpacity here?

[wkorman, 2017-01-26 23:09:12]

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp (right):

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:74: if
(layoutObject.styleRef().opacity())
On 2017/01/26 at 22:50:38, pdr. wrote:
> Do you need to check opacity > kMinimumVisibleOpacity here?

http://crrev.com/2523103003 added PaintLayerPainterTest.DontPaintWithTinyOpacity
which explicitly checks that for SPv2 any opacity, even one <
kMinimumVisibleOpacity, paints for SPv2. I'm presuming the author had a specific
reason for that, so would like to preserve that logic in this change, though it
does seem like the optimization conceptually should be valid for SPv2 as well.

[pdr., 2017-01-27 00:48:45]

On 2017/01/26 at 23:09:12, wkorman wrote:
>
https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp (right):
> 
>
https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:74: if
(layoutObject.styleRef().opacity())
> On 2017/01/26 at 22:50:38, pdr. wrote:
> > Do you need to check opacity > kMinimumVisibleOpacity here?
> 
> http://crrev.com/2523103003 added
PaintLayerPainterTest.DontPaintWithTinyOpacity which explicitly checks that for
SPv2 any opacity, even one < kMinimumVisibleOpacity, paints for SPv2. I'm
presuming the author had a specific reason for that, so would like to preserve
that logic in this change, though it does seem like the optimization
conceptually should be valid for SPv2 as well.

OK SGTM LGTM

[wkorman, 2017-01-27 03:53:12]

Added tests.

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp (left):

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp:514:
CompositingReasonFinder::requiresCompositingForEffectAnimation(style);
On 2017/01/26 at 22:47:56, wkorman wrote:
> This was a bug -- we were assigning the bool result to an int reason var. Now
fixed below.

Added check for this to test case.

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/graphics/paint/EffectPaintPropertyNode.h
(right):

https://codereview.chromium.org/2652273004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/paint/EffectPaintPropertyNode.h:116:
CompositingReasons compositingReasons() const {
On 2017/01/26 at 22:47:56, wkorman wrote:
> If we'd prefer not to expose compositing reasons directly from the node we
could add accessors to each of these such as requiresCompositingForAnimation()
that performs the check internally.

I made this change as I think it's cleaner.

[wkorman, 2017-01-27 03:53:24]

The CQ bit was checked by wkorman@chromium.org

[wkorman, 2017-01-27 03:53:24]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2652273004/#ps20001
(title: "Add tests and sync to head.")

[commit-bot: I haz the power, 2017-01-27 03:53:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 05:48:08]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485489204170010,
"parent_rev": "cc1d4004d36827409a7485fb1672ffcc3e7c2c99", "commit_rev":
"9ce0839c605303284391c7efacccdcc00dc38437"}

[commit-bot: I haz the power, 2017-01-27 05:48:43]

Description was changed from

==========
Paint invisible layer content in presence of composited animations.

For SPv2 we must paint layer content even if the content is invisible
w.r.t. opacity.

This is required (a) so that we have a paint chunk, which will produce a layer,
which is required by the animation subsystem, and (b) so that cc has the paint
commands needed in order to actually enact the animation compositor-side through
mutating the property tree state and re-rastering as needed.

In SPv1 this code path (PaintLayerPainter::paintedOutputInvisible) is skipped
when painting a layer for composited animated content. Instead of calling
PaintLayerPainter::paint(), we end up painting the GraphicsLayer for the
composited animation via CompositedLayerMapping::doPaintTask ->
PaintLayerPainter::paintLayerContents.

BUG=684842
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Paint invisible layer content in presence of composited animations.

For SPv2 we must paint layer content even if the content is invisible
w.r.t. opacity.

This is required (a) so that we have a paint chunk, which will produce a layer,
which is required by the animation subsystem, and (b) so that cc has the paint
commands needed in order to actually enact the animation compositor-side through
mutating the property tree state and re-rastering as needed.

In SPv1 this code path (PaintLayerPainter::paintedOutputInvisible) is skipped
when painting a layer for composited animated content. Instead of calling
PaintLayerPainter::paint(), we end up painting the GraphicsLayer for the
composited animation via CompositedLayerMapping::doPaintTask ->
PaintLayerPainter::paintLayerContents.

BUG=684842
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2652273004
Cr-Commit-Position: refs/heads/master@{#446605}
Committed:
https://chromium.googlesource.com/chromium/src/+/9ce0839c605303284391c7efaccc...
==========

[commit-bot: I haz the power, 2017-01-27 05:48:44]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/9ce0839c605303284391c7efaccc...