cc: Compute draw transforms dynamically.

cc: Compute draw transforms dynamically.

This makes cc stop pre-computing all target space transforms and only compute
and cache transforms on demand. The intermediate results will be cached in
cc::PropertyTreesCachedData. The cache is invalidated every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=624120
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/1449b69d9135cfe3223b171f09677434e23906c3
Cr-Commit-Position: refs/heads/master@{#426235}

[sunxd, 2016-08-22 22:26:18]

Description was changed from

==========
WIP: perf test cache strategy

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

to

==========
WIP: perf test cache strategy

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
==========

[sunxd, 2016-08-22 22:26:18]

Description was changed from

==========
WIP: perf test cache strategy

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
==========

to

==========
WIP: perf test cache strategy

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[sunxd, 2016-09-02 16:54:49]

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

[commit-bot: I haz the power, 2016-09-02 16:55:03]

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

[sunxd, 2016-09-02 17:02:19]

Description was changed from

==========
WIP: perf test cache strategy

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Compute draw transforms dynamically behind a flag.

This makes cc stop pre-computing all target space transforms if the flag
verify_transform_tree_calculations is enabled.

Cc will instead compute draw transforms only when we need it and cache all
intermediate results in cc::PropertyTreesCachedData. The cache is invalidated
every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[sunxd, 2016-09-02 17:02:23]

The CQ bit was unchecked by sunxd@chromium.org

[sunxd, 2016-09-02 17:04:59]

Description was changed from

==========
cc: Compute draw transforms dynamically behind a flag.

This makes cc stop pre-computing all target space transforms if the flag
verify_transform_tree_calculations is enabled.

Cc will instead compute draw transforms only when we need it and cache all
intermediate results in cc::PropertyTreesCachedData. The cache is invalidated
every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Compute draw transforms dynamically behind a flag.

This makes cc stop pre-computing all target space transforms if the flag
verify_transform_tree_calculations is enabled.

Cc will instead compute draw transforms only when we need it and cache all
intermediate results in cc::PropertyTreesCachedData. The cache is invalidated
every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=624120
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[sunxd, 2016-09-02 17:04:59]

sunxd@chromium.org changed reviewers:
+ ajuma@chromium.org, jaydasika@chromium.org, vollick@chromium.org

[sunxd, 2016-09-02 17:07:52]

The changes in draw_property perf tests will be removed after running a final
telemetry before CL commit.

[jaydasika, 2016-09-02 18:10:01]

https://codereview.chromium.org/2266223002/diff/180001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (left):

https://codereview.chromium.org/2266223002/diff/180001/cc/trees/property_tree...
cc/trees/property_tree.cc:330: combined_transform = ToTarget(current->id,
kInvalidNodeId);
This is not behind a flag. Is it fine to remove it from a performance standpoint
?

[sunxd, 2016-09-02 18:13:10]

https://codereview.chromium.org/2266223002/diff/180001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (left):

https://codereview.chromium.org/2266223002/diff/180001/cc/trees/property_tree...
cc/trees/property_tree.cc:330: combined_transform = ToTarget(current->id,
kInvalidNodeId);
On 2016/09/02 18:10:01, jaydasika wrote:
> This is not behind a flag. Is it fine to remove it from a performance
standpoint
> ?

Removing it does make performance suffer. I can make it only runs when the flag
is not enabled.

But after talking with Ali, I decide to remove the old path as well as the flag
in property trees, as it might be more clear to land the patch to execute the
new path rather than to land the patch and enable it later.

[sunxd, 2016-09-02 21:50:40]

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

[commit-bot: I haz the power, 2016-09-02 21:51:17]

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

[sunxd, 2016-09-02 21:51:54]

Actually ComputeTransform, CombineTransformsBetween and
CombineTransformsInverseBetween are dead code after this patch, I'm thinking
about removing them.

[commit-bot: I haz the power, 2016-09-02 22:09:14]

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

[commit-bot: I haz the power, 2016-09-02 22:09:15]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sunxd, 2016-09-02 22:23:57]

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

[commit-bot: I haz the power, 2016-09-02 22:24:29]

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

[sunxd, 2016-09-02 22:28:55]

Description was changed from

==========
cc: Compute draw transforms dynamically behind a flag.

This makes cc stop pre-computing all target space transforms if the flag
verify_transform_tree_calculations is enabled.

Cc will instead compute draw transforms only when we need it and cache all
intermediate results in cc::PropertyTreesCachedData. The cache is invalidated
every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=624120
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Compute draw transforms dynamically.

This makes cc stop pre-computing all target space transforms and only compute
and cache transforms on demand. The intermediate results will be cached in
cc::PropertyTreesCachedData. The cache is invalidated every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=624120
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-09-02 22:46:44]

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

[commit-bot: I haz the power, 2016-09-02 22:46:45]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[sunxd, 2016-09-06 20:41:58]

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

[commit-bot: I haz the power, 2016-09-06 20:43:10]

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

[sunxd, 2016-09-06 22:14:22]

I'm thinking about adding the getdrawtransforms test in next patch.

[commit-bot: I haz the power, 2016-09-06 22:55:07]

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

[commit-bot: I haz the power, 2016-09-06 22:55:08]

Dry run: This issue passed the CQ dry run.

[ajuma, 2016-09-07 14:51:23]

https://codereview.chromium.org/2266223002/diff/260001/cc/trees/draw_property...
File cc/trees/draw_property_utils.cc (left):

https://codereview.chromium.org/2266223002/diff/260001/cc/trees/draw_property...
cc/trees/draw_property_utils.cc:902: DCHECK(success);
In which cases is this DCHECK now failing?

https://codereview.chromium.org/2266223002/diff/260001/cc/trees/property_tree.h
File cc/trees/property_tree.h (right):

https://codereview.chromium.org/2266223002/diff/260001/cc/trees/property_tree...
cc/trees/property_tree.h:494: mutable std::vector<std::map<int,
DrawTransformData>> draw_transforms;
As discussed offline, please compare the performance of using unordered_map vs
map here on the telemetry cluster.

[sunxd, 2016-09-07 15:20:02]

https://codereview.chromium.org/2266223002/diff/260001/cc/trees/draw_property...
File cc/trees/draw_property_utils.cc (left):

https://codereview.chromium.org/2266223002/diff/260001/cc/trees/draw_property...
cc/trees/draw_property_utils.cc:902: DCHECK(success);
On 2016/09/07 14:51:23, ajuma wrote:
> In which cases is this DCHECK now failing?

It only fails when the target's surface content scale is 0, or partly 0. The
test that triggers this is
PaintArtifactCompositorTestWithPropertyTrees.NestedClips.

https://codereview.chromium.org/2266223002/diff/260001/cc/trees/property_tree.h
File cc/trees/property_tree.h (right):

https://codereview.chromium.org/2266223002/diff/260001/cc/trees/property_tree...
cc/trees/property_tree.h:494: mutable std::vector<std::map<int,
DrawTransformData>> draw_transforms;
On 2016/09/07 14:51:23, ajuma wrote:
> As discussed offline, please compare the performance of using unordered_map vs
> map here on the telemetry cluster.

Acknowledged.

[sunxd, 2016-09-07 20:17:03]

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

[commit-bot: I haz the power, 2016-09-07 20:18:02]

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

[sunxd, 2016-09-08 14:42:44]

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

[commit-bot: I haz the power, 2016-09-08 14:42: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, 2016-09-08 14:46:00]

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

[commit-bot: I haz the power, 2016-09-08 14:46:03]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sunxd, 2016-09-29 22:34:46]

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

[commit-bot: I haz the power, 2016-09-29 22:35:05]

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, 2016-09-29 22:46:52]

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

[commit-bot: I haz the power, 2016-09-29 22:46:53]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sunxd, 2016-09-29 23:06:47]

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

[commit-bot: I haz the power, 2016-09-29 23:07:16]

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

[sunxd, 2016-09-30 03:12:42]

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

[commit-bot: I haz the power, 2016-09-30 03:12:55]

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, 2016-09-30 03:41:15]

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

[commit-bot: I haz the power, 2016-09-30 03:41:15]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sunxd, 2016-09-30 05:39:11]

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

[commit-bot: I haz the power, 2016-09-30 05:39:30]

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, 2016-09-30 07:26:13]

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

[commit-bot: I haz the power, 2016-09-30 07:26:15]

Dry run: This issue passed the CQ dry run.

[sunxd, 2016-10-12 17:10:19]

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

[commit-bot: I haz the power, 2016-10-12 17:10:54]

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, 2016-10-12 17:20:44]

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

[commit-bot: I haz the power, 2016-10-12 17:20:46]

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

[sunxd, 2016-10-12 17:25:52]

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

[commit-bot: I haz the power, 2016-10-12 17:26:31]

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

[sunxd, 2016-10-12 18:42:07]

PTAL.

[sunxd, 2016-10-12 19:06:41]

Patchset #48 (id:940001) has been deleted

[sunxd, 2016-10-12 19:08:58]

On 2016/10/12 18:42:07, sunxd wrote:
> PTAL.

The current perf numbers can be found at:
https://storage.cloud.google.com/cluster-telemetry/tasks/chromium_perf_runs/s...

The numbers are the time we spend on impl thread cdp on android bots, the unit
is ms.

[commit-bot: I haz the power, 2016-10-12 19:25:10]

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

[commit-bot: I haz the power, 2016-10-12 19:25:12]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ajuma, 2016-10-12 20:08:48]

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:180: needs_update_ = needs_update;
Can we make needs_update_ private to make sure it's only changed by this method?

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2203: if (transform_data.target_id == dest_id ||
transform_data.target_id == -1) {
Which cases is the "-1" check supposed to handle?

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2204: return
cached_data_.draw_transforms[transform_id][cnt];
Would it work to return |transform_data| here (so no need for the counter
variable)?

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2225: data.target_id != -1);
In what situations do we have target_id -1 (and why does that imply that the
update numbers don't match)?

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2241: if (dest_id != TransformTree::kRootNodeId)
https://codereview.chromium.org/2408243002/ might affect this logic

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree.h
File cc/trees/property_tree.h (left):

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.h:551: 
Nit: this line shouldn't be deleted

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree.h
File cc/trees/property_tree.h (right):

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.h:526: bool invertible;
Having this be true when we don't know if the transform is invertible seems a
bit confusing. But it looks like this is only used by the caching code to avoid
re-computing inverses of things we know are uninvertible? In that case, could we
rename this to something like "might_be_invertible"?

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.h:650: DrawTransformData&
FetchDrawTransformsDataFromCache(int transform_id,
Can this be private?

https://codereview.chromium.org/2266223002/diff/960001/tools/perf/benchmarks/...
File tools/perf/benchmarks/draw_properties.py (right):

https://codereview.chromium.org/2266223002/diff/960001/tools/perf/benchmarks/...
tools/perf/benchmarks/draw_properties.py:3: # found in the LICENSE file.
Don't forget to remove this file from your patch.

https://codereview.chromium.org/2266223002/diff/960001/tools/perf/measurement...
File tools/perf/measurements/draw_properties.py (right):

https://codereview.chromium.org/2266223002/diff/960001/tools/perf/measurement...
tools/perf/measurements/draw_properties.py:3: # found in the LICENSE file.
Do these changes need to land with this patch?

[sunxd, 2016-10-12 21:08:19]

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:180: needs_update_ = needs_update;
On 2016/10/12 20:08:47, ajuma wrote:
> Can we make needs_update_ private to make sure it's only changed by this
method?

I think needs_update_ is already private. TransformTree can access that because
it's a friend class of PropertyTree<>.

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2203: if (transform_data.target_id == dest_id ||
transform_data.target_id == -1) {
On 2016/10/12 20:08:47, ajuma wrote:
> Which cases is the "-1" check supposed to handle?

In PropertyTrees::ResetCachedData, we initialize draw_transforms with 1 element.
The first time when we access the cache, we would hit a -1 target_id.

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2204: return
cached_data_.draw_transforms[transform_id][cnt];
On 2016/10/12 20:08:47, ajuma wrote:
> Would it work to return |transform_data| here (so no need for the counter
> variable)?

Done.

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2225: data.target_id != -1);
On 2016/10/12 20:08:47, ajuma wrote:
> In what situations do we have target_id -1 (and why does that imply that the
> update numbers don't match)?

As stated in the comments in Fetch function, it is -1 when we compute for the
first time after a cache reset.

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2241: if (dest_id != TransformTree::kRootNodeId)
On 2016/10/12 20:08:47, ajuma wrote:
> https://codereview.chromium.org/2408243002/ might affect this logic

Is it safe that we do not apply root surface contents scale as in that CL the
scale factor is stored in the transform root.

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree.h
File cc/trees/property_tree.h (left):

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.h:551: 
On 2016/10/12 20:08:47, ajuma wrote:
> Nit: this line shouldn't be deleted

Done.

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree.h
File cc/trees/property_tree.h (right):

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.h:526: bool invertible;
On 2016/10/12 20:08:47, ajuma wrote:
> Having this be true when we don't know if the transform is invertible seems a
> bit confusing. But it looks like this is only used by the caching code to
avoid
> re-computing inverses of things we know are uninvertible? In that case, could
we
> rename this to something like "might_be_invertible"?

Done.

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.h:650: DrawTransformData&
FetchDrawTransformsDataFromCache(int transform_id,
On 2016/10/12 20:08:47, ajuma wrote:
> Can this be private?

Done.

https://codereview.chromium.org/2266223002/diff/960001/tools/perf/benchmarks/...
File tools/perf/benchmarks/draw_properties.py (right):

https://codereview.chromium.org/2266223002/diff/960001/tools/perf/benchmarks/...
tools/perf/benchmarks/draw_properties.py:3: # found in the LICENSE file.
On 2016/10/12 20:08:47, ajuma wrote:
> Don't forget to remove this file from your patch.

Acknowledged.

https://codereview.chromium.org/2266223002/diff/960001/tools/perf/measurement...
File tools/perf/measurements/draw_properties.py (right):

https://codereview.chromium.org/2266223002/diff/960001/tools/perf/measurement...
tools/perf/measurements/draw_properties.py:3: # found in the LICENSE file.
On 2016/10/12 20:08:48, ajuma wrote:
> Do these changes need to land with this patch?

No, I'll have a final run and remove this change.

[ajuma, 2016-10-12 21:37:41]

Thanks, lgtm assuming the final cluster telemetry results look reasonable.

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/2266223002/diff/960001/cc/trees/property_tree...
cc/trees/property_tree.cc:2203: if (transform_data.target_id == dest_id ||
transform_data.target_id == -1) {
On 2016/10/12 21:08:19, sunxd wrote:
> On 2016/10/12 20:08:47, ajuma wrote:
> > Which cases is the "-1" check supposed to handle?
> 
> In PropertyTrees::ResetCachedData, we initialize draw_transforms with 1
element.
> The first time when we access the cache, we would hit a -1 target_id.

Ah! Please add a comment about this.

[sunxd, 2016-10-12 22:23:18]

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

[commit-bot: I haz the power, 2016-10-12 22:23:44]

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, 2016-10-13 00:32:01]

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

[commit-bot: I haz the power, 2016-10-13 00:32:03]

Dry run: This issue passed the CQ dry run.

[sunxd, 2016-10-18 15:19:18]

The CQ bit was checked by sunxd@chromium.org

[sunxd, 2016-10-18 15:19:19]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2266223002/#ps1060001
(title: "Remove script changes.")

[commit-bot: I haz the power, 2016-10-18 15:19:37]

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, 2016-10-18 15:29:22]

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

[commit-bot: I haz the power, 2016-10-18 15:29:23]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[sunxd, 2016-10-19 14:24:03]

The CQ bit was checked by sunxd@chromium.org

[commit-bot: I haz the power, 2016-10-19 14:24:32]

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, 2016-10-19 14:43:05]

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

[commit-bot: I haz the power, 2016-10-19 14:43:07]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ian Vollick, 2016-10-19 15:05:46]

On 2016/10/19 14:43:07, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

platform/ lgtm.

[sunxd, 2016-10-19 15:06:39]

The CQ bit was checked by sunxd@chromium.org

[commit-bot: I haz the power, 2016-10-19 15:06:59]

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, 2016-10-19 15:25:42]

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

[commit-bot: I haz the power, 2016-10-19 15:25:44]

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-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[sunxd, 2016-10-19 15:31:48]

The CQ bit was checked by sunxd@chromium.org

[sunxd, 2016-10-19 15:31:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
vollick@chromium.org, ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2266223002/#ps1080001
(title: "Rebase!")

[commit-bot: I haz the power, 2016-10-19 15:32:17]

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, 2016-10-19 17:31:19]

Description was changed from

==========
cc: Compute draw transforms dynamically.

This makes cc stop pre-computing all target space transforms and only compute
and cache transforms on demand. The intermediate results will be cached in
cc::PropertyTreesCachedData. The cache is invalidated every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=624120
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Compute draw transforms dynamically.

This makes cc stop pre-computing all target space transforms and only compute
and cache transforms on demand. The intermediate results will be cached in
cc::PropertyTreesCachedData. The cache is invalidated every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=624120
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[commit-bot: I haz the power, 2016-10-19 17:31:21]

Committed patchset #54 (id:1080001)

[commit-bot: I haz the power, 2016-10-21 13:09:52]

Description was changed from

==========
cc: Compute draw transforms dynamically.

This makes cc stop pre-computing all target space transforms and only compute
and cache transforms on demand. The intermediate results will be cached in
cc::PropertyTreesCachedData. The cache is invalidated every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=624120
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
cc: Compute draw transforms dynamically.

This makes cc stop pre-computing all target space transforms and only compute
and cache transforms on demand. The intermediate results will be cached in
cc::PropertyTreesCachedData. The cache is invalidated every frame.

The following steps are removing transform tree cached data and stoping
adding transform nodes for render surfaces.

BUG=624120
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/1449b69d9135cfe3223b171f09677434e23906c3
Cr-Commit-Position: refs/heads/master@{#426235}
==========

[commit-bot: I haz the power, 2016-10-21 13:09:54]

Patchset 54 (id:??) landed as
https://crrev.com/1449b69d9135cfe3223b171f09677434e23906c3
Cr-Commit-Position: refs/heads/master@{#426235}