Invalidate subsequence caching & empty paint phases if clips changed.

Invalidate subsequence caching & empty paint phases if clips changed.

Subsequence caching & empty paint phase optimizations need to be invalidated i
any ancestor clips updated, since those clips are used to:

a. Adjust painted output (SPInvalidation only, not SPv2), and
b. Cull out invisible content (SPInvalidation and SPv2)

Previously, we detected that by computing clip rects during paint. This CL
moves it to the pre-paint tree walk.

The CL does not yet work for SPv2, because it does not include computation of
the enclosing transformed ancestor, which will be the SPv2 analogue of
paintInvalidationContainer.

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

[chrishtr, 2017-01-23 21:36:46]

Description was changed from

==========
none


none


Merge branch 'master' into repaintonclip


none


none

BUG=
==========

to

==========
none


none


Merge branch 'master' into repaintonclip


none


none

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

[chrishtr, 2017-01-23 21:39:52]

Description was changed from

==========
none


none


Merge branch 'master' into repaintonclip


none


none

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

to

==========
Set needsRepaint if any property tree state in an ancestor changed.

Subsequence caching needs to be invalidated if any ancestor clips updated, since
those clips are used to:

a. Adjust painted output (SPInvalidation only, not SPv2), and
b. Cull out invisible content (SPInvalidation and SPv2)

Previously, we detected that by computing clip rects. Instead, re-use the
dirty bit machinery in the paint property tree walk for this purpose, because
that machinery forces a subtree update if any paint property node changed.

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

[chrishtr, 2017-01-23 21:41:10]

Description was changed from

==========
Set needsRepaint if any property tree state in an ancestor changed.

Subsequence caching needs to be invalidated if any ancestor clips updated, since
those clips are used to:

a. Adjust painted output (SPInvalidation only, not SPv2), and
b. Cull out invisible content (SPInvalidation and SPv2)

Previously, we detected that by computing clip rects. Instead, re-use the
dirty bit machinery in the paint property tree walk for this purpose, because
that machinery forces a subtree update if any paint property node changed.

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

to

==========
Invalidate subsequence caching & empty paint phases if property tree node
changed.

Subsequence caching & empty paint phase optimizations need to be invalidated i
any ancestor clips updated, since those clips are used to:

a. Adjust painted output (SPInvalidation only, not SPv2), and
b. Cull out invisible content (SPInvalidation and SPv2)

Previously, we detected that by computing clip rects. Instead, re-use the
dirty bit machinery in the paint property tree walk for this purpose, because
that machinery forces a subtree update if any paint property node changed.

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

[chrishtr, 2017-01-23 21:41:16]

The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 21:41:56]

Dry run: 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-23 22:58:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-23 22:58:33]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[chrishtr, 2017-01-24 00:37:20]

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

[chrishtr, 2017-01-24 00:37:21]

For discussion. We might be able to find a better solution.

Pros of this approach:

- Very simple and "obviously" correct, assuming correctness of the code that
force-recomputes paint properties for subtrees when anything changes
- Doesn't require checking clipping rects all the time
- Is cleaner and more precise than the approach of checking clip rects

Con of this approach:

- Ties the paint property tree force-recompute-children to invalidation of
optimizations
for avoiding certain paint phases and subsequence caching

WDYT?

[pdr., 2017-01-24 07:23:12]

On 2017/01/24 at 00:37:21, chrishtr wrote:
> For discussion. We might be able to find a better solution.
> 
> Pros of this approach:
> 
> - Very simple and "obviously" correct, assuming correctness of the code that
force-recomputes paint properties for subtrees when anything changes
> - Doesn't require checking clipping rects all the time
> - Is cleaner and more precise than the approach of checking clip rects
> 
> Con of this approach:
> 
> - Ties the paint property tree force-recompute-children to invalidation of
optimizations
> for avoiding certain paint phases and subsequence caching
> 
> WDYT?

What if an ancestor clip changes but is not added or removed (i.e., no
structural change to the property tree, only a change of values)? In that case
we may early-out before getting to the update. Is that okay?

How important is not over-invalidating for performance? By using a targeted bit
we could save a lot of setNeedsRepaint calls.

[chrishtr, 2017-01-25 01:32:56]

Update: Philip and I discussed at some length.

To reiterate, the task is to decide if any clip that applies to a PaintLayer or
one of
its non-composited descendants changes. This requires computing clips for self, 
in-flow, abspos and fixedpos descendants.

The approach in patch set 1 of this CL is not a complete solution, because the
force-subtree bits in the property tree builder are not set when changing the
value of a node, just when we add or remove one. But even if I fixed that, this
patch would cause a lot of over-invalidation of subsequence caching, such as
when animating a transformed element that contains a stacking context.

We arrived at what seems like a good solution: make a new method on
PrePaintTreeWalk
that takes in these parameters: paintInvalidationContainer,
paintPropertyBuilderContext, enclosingTransformNode, and computes the clips
between
self and paintInvalidationContainer (or enclosingTransform node for SPv2 mode)
for
the regular, abspos and fixedpos PropertyTreeStates of the
paintPropertyBuilderContext,
then compares them with the cached ClipRects object on PaintLayer. If changed,
it
invalidates subsequence caching and elided paint phase optimizations on that
PaintLayer.

This is basically the same as the way it's implemented today in
PaintLayerPainter, except
it will be performant for SPv2.

[chrishtr, 2017-01-26 22:46:48]

Description was changed from

==========
Invalidate subsequence caching & empty paint phases if property tree node
changed.

Subsequence caching & empty paint phase optimizations need to be invalidated i
any ancestor clips updated, since those clips are used to:

a. Adjust painted output (SPInvalidation only, not SPv2), and
b. Cull out invisible content (SPInvalidation and SPv2)

Previously, we detected that by computing clip rects. Instead, re-use the
dirty bit machinery in the paint property tree walk for this purpose, because
that machinery forces a subtree update if any paint property node changed.

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

to

==========
Invalidate subsequence caching & empty paint phases if clips changed.

Subsequence caching & empty paint phase optimizations need to be invalidated i
any ancestor clips updated, since those clips are used to:

a. Adjust painted output (SPInvalidation only, not SPv2), and
b. Cull out invisible content (SPInvalidation and SPv2)

Previously, we detected that by computing clip rects during paint. This CL
moves it to the pre-paint tree walk.

The CL does not yet work for SPv2, because it does not include computation of
the enclosing transformed ancestor, which will be the SPv2 analogue of
paintInvalidationContainer.

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

[chrishtr, 2017-01-31 21:07:26]

The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 21:08:26]

Dry run: 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-31 21:10:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 21:10:47]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[chrishtr, 2017-01-31 21:29:06]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2017-01-31 21:29:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 21:29:58]

This CL has an open dependency (Issue 2658513002 Patch 20001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.