cc: Remove usage of ComputeVisibleRects on main thread

cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.
3) Remove the usage in benchmarks.

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

Committed: https://crrev.com/713aedbdd27d1bd0d431a02220748c740b471af9
Cr-Commit-Position: refs/heads/master@{#411162}

[sunxd, 2016-08-09 15:50:37]

Description was changed from

==========
cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.

And follow up CL will remove the usage in benchmarks.

BUG=581827
==========

to

==========
cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.

And follow up CL will remove the usage in benchmarks.

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

[sunxd, 2016-08-09 15:50:46]

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

[commit-bot: I haz the power, 2016-08-09 15:51:16]

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-08-09 17:08:38]

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

[commit-bot: I haz the power, 2016-08-09 17:08:39]

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

[sunxd, 2016-08-09 17:42:34]

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

[commit-bot: I haz the power, 2016-08-09 17:43:32]

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

[sunxd, 2016-08-09 19:16:13]

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

[sunxd, 2016-08-09 19:17:50]

https://codereview.chromium.org/2225973003/diff/40001/cc/debug/invalidation_b...
File cc/debug/invalidation_benchmark.cc (right):

https://codereview.chromium.org/2225973003/diff/40001/cc/debug/invalidation_b...
cc/debug/invalidation_benchmark.cc:83: gfx::Rect visible_layer_rect =
MathUtil::MapEnclosingClippedRect(
I forgot to make it a projection here. Please ignore it before the next patch.

[sunxd, 2016-08-09 19:26:57]

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

[commit-bot: I haz the power, 2016-08-09 19:27:48]

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

[sunxd, 2016-08-09 19:33:44]

Description was changed from

==========
cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.

And follow up CL will remove the usage in benchmarks.

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

to

==========
cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.
3) Remove the usage in benchmarks.

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

[jaydasika, 2016-08-09 20:39:59]

lgtm % ajuma

[ajuma, 2016-08-09 20:44:02]

Generally looks great.

https://codereview.chromium.org/2225973003/diff/60001/cc/debug/invalidation_b...
File cc/debug/invalidation_benchmark.cc (right):

https://codereview.chromium.org/2225973003/diff/60001/cc/debug/invalidation_b...
cc/debug/invalidation_benchmark.cc:78: gfx::Rect layer_rect =
gfx::Rect(position, layer->bounds());
The screen space transform already accounts for the layer's position, so we
shouldn't need to use |position| here.

https://codereview.chromium.org/2225973003/diff/60001/cc/debug/invalidation_b...
cc/debug/invalidation_benchmark.cc:80: transform_tree.ToScreen(index),
layer_rect);
This should use Layer::screen_space_transform (since transform_tree.ToScreen
doesn't account for the layer's offset to its transform parent).

https://codereview.chromium.org/2225973003/diff/60001/cc/debug/invalidation_b...
cc/debug/invalidation_benchmark.cc:82:
gfx::Rect(layer->layer_tree_host()->device_viewport_size()));
I wonder if it would be simpler to just start with the viewport, project that
into layer space, and intersect with the layer's bounds.

https://codereview.chromium.org/2225973003/diff/60001/cc/debug/invalidation_b...
cc/debug/invalidation_benchmark.cc:84: transform_tree.FromScreen(index),
screen_space_rect);
And this needs to use the inverse of Layer::screen_space_transform.

https://codereview.chromium.org/2225973003/diff/60001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/2225973003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:284: for (auto layer : *layer_list)
{
How about using std::find here?

https://codereview.chromium.org/2225973003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:7934:
animated->SetTransform(zero_matrix);
If you call OnTransformAnimated (instead of SetTransform) here, I think we can
avoid the rebuild below.

[commit-bot: I haz the power, 2016-08-09 21:14:43]

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

[commit-bot: I haz the power, 2016-08-09 21:14:45]

Dry run: This issue passed the CQ dry run.

[sunxd, 2016-08-10 17:19:37]

https://codereview.chromium.org/2225973003/diff/60001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/2225973003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:284: for (auto layer : *layer_list)
{
On 2016/08/09 20:44:02, ajuma wrote:
> How about using std::find here?

Done.

https://codereview.chromium.org/2225973003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:7934:
animated->SetTransform(zero_matrix);
On 2016/08/09 20:44:02, ajuma wrote:
> If you call OnTransformAnimated (instead of SetTransform) here, I think we can
> avoid the rebuild below.

Done.

[ajuma, 2016-08-10 19:18:18]

Thanks, lgtm

[sunxd, 2016-08-10 21:03:40]

The CQ bit was checked by sunxd@chromium.org

[sunxd, 2016-08-10 21:03:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
jaydasika@chromium.org
Link to the patchset: https://codereview.chromium.org/2225973003/#ps80001
(title: "Address comments and fix perf regressions")

[commit-bot: I haz the power, 2016-08-10 21:04:16]

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-08-10 22:22:33]

Description was changed from

==========
cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.
3) Remove the usage in benchmarks.

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

to

==========
cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.
3) Remove the usage in benchmarks.

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

[commit-bot: I haz the power, 2016-08-10 22:22:34]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-10 22:24:30]

Description was changed from

==========
cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.
3) Remove the usage in benchmarks.

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

to

==========
cc: Remove usage of ComputeVisibleRects on main thread

Target space transform will not be computed on main thread. Currently we
only use it in tests and subtree skipping logic.

This CL removes the usage of ComputeVisibleRects on main thread:
1) Do not skip for back face visibility on main thread;
2) Remove main thread ComputeVisibleRects in unit tests.
3) Remove the usage in benchmarks.

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

Committed: https://crrev.com/713aedbdd27d1bd0d431a02220748c740b471af9
Cr-Commit-Position: refs/heads/master@{#411162}
==========

[commit-bot: I haz the power, 2016-08-10 22:24:31]

Patchset 5 (id:??) landed as
https://crrev.com/713aedbdd27d1bd0d431a02220748c740b471af9
Cr-Commit-Position: refs/heads/master@{#411162}