cc : Stop calculating visible rects on main thread

cc : Stop calculating visible rects on main thread

Because they are unused outside tests.
This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more
realistic (cc doesn't skip such layers).

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/0f02fed2f4a7503c715501f74f32b0654fa15c5d
Cr-Commit-Position: refs/heads/master@{#385247}

[jaydasika, 2016-04-02 02:35:32]

Description was changed from

==========
cc : Stop calculating visible rects on main thread

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

to

==========
cc : Stop calculating visible rects on main thread

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-02 02:54:09]

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

[jaydasika, 2016-04-02 02:54:10]

WIP, but want to know if anyone has thoughts on how we split stuff in
draw_proeprty_utils into logical units.

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:753: void
BuildPropertyTreesAndComputeVisibleRects(
Wanted to delete this function but calling the function in property tree builder
directly from layer_tree_host_common_test is giving a linker error that I don't
understand.

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.h (right):

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
cc/trees/draw_property_utils.h:74: void CC_EXPORT Update(Layer* root_layer,
need to name this better :) It updates property trees, render surfaces and finds
layers that need update.

[ajuma, 2016-04-04 14:15:05]

rasterize_and_record_benchmark.cc and invalidation_benchmark.cc also use visible
layer rects, so need to be sorted out (to figure out whether the dependency on
visible layer rects can be removed). For example, the rasterize_and_record
benchmark uses visible layer rects to skip layers (that have an empty visible
rect) but that doesn't match what cc does in reality, so it's probably more
realistic to remove that skipping.

For this CL it's fine to just make them directly call the function that computes
visible rects (since that's what they're implicitly doing now anyway).

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:753: void
BuildPropertyTreesAndComputeVisibleRects(
On 2016/04/02 02:54:10, jaydasika wrote:
> Wanted to delete this function but calling the function in property tree
builder
> directly from layer_tree_host_common_test is giving a linker error that I
don't
> understand.

It's probably just that PropertyTreeBuilder isn't declared with CC_EXPORT.

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.h (right):

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
cc/trees/draw_property_utils.h:74: void CC_EXPORT Update(Layer* root_layer,
On 2016/04/02 02:54:10, jaydasika wrote:
> need to name this better :) It updates property trees, render surfaces and
finds
> layers that need update.

How about splitting this into three functions?

[jaydasika, 2016-04-04 18:41:09]

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:753: void
BuildPropertyTreesAndComputeVisibleRects(
On 2016/04/04 14:15:05, ajuma wrote:
> On 2016/04/02 02:54:10, jaydasika wrote:
> > Wanted to delete this function but calling the function in property tree
> builder
> > directly from layer_tree_host_common_test is giving a linker error that I
> don't
> > understand.
> 
> It's probably just that PropertyTreeBuilder isn't declared with CC_EXPORT.
That was the reason. Thanks

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.h (right):

https://codereview.chromium.org/1855013002/diff/40001/cc/trees/draw_property_...
cc/trees/draw_property_utils.h:74: void CC_EXPORT Update(Layer* root_layer,
On 2016/04/04 14:15:05, ajuma wrote:
> On 2016/04/02 02:54:10, jaydasika wrote:
> > need to name this better :) It updates property trees, render surfaces and
> finds
> > layers that need update.
> 
> How about splitting this into three functions?

Done.

[jaydasika, 2016-04-04 19:05:30]

Description was changed from

==========
cc : Stop calculating visible rects on main thread

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Stop calculating visible rects on main thread

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ajuma, 2016-04-04 20:47:18]

https://codereview.chromium.org/1855013002/diff/80001/cc/debug/rasterize_and_...
File cc/debug/rasterize_and_record_benchmark.cc (left):

https://codereview.chromium.org/1855013002/diff/80001/cc/debug/rasterize_and_...
cc/debug/rasterize_and_record_benchmark.cc:119: return;
This seems like the most realistic thing to do (since cc doesn't actually skip
recording layers with empty visible rects). This might change benchmark results
though, so be sure to mention this in the description.

https://codereview.chromium.org/1855013002/diff/80001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/1855013002/diff/80001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:726: // don't need to be updated.
This comment doesn't apply anymore (since we're not making up a list of layers
that need visible rects). So we could just add these layers above.

https://codereview.chromium.org/1855013002/diff/80001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:745: visible_layer_list.push_back(layer.get());
Can we avoid this copy? Maybe use templating so that CalculateVisibleRects
doesn't care whether it gets a list of pointers or a list of ScopedRefPtrs?

[jaydasika, 2016-04-04 20:57:15]

Description was changed from

==========
cc : Stop calculating visible rects on main thread

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Stop calculating visible rects on main thread

This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more realistic as cc doesn't
skip such layers.

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-04 20:57:56]

Description was changed from

==========
cc : Stop calculating visible rects on main thread

This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more realistic as cc doesn't
skip such layers.

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Stop calculating visible rects on main thread

This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more 
realistic (cc doesn't skip such layers).

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-04 21:29:17]

https://codereview.chromium.org/1855013002/diff/80001/cc/debug/rasterize_and_...
File cc/debug/rasterize_and_record_benchmark.cc (left):

https://codereview.chromium.org/1855013002/diff/80001/cc/debug/rasterize_and_...
cc/debug/rasterize_and_record_benchmark.cc:119: return;
On 2016/04/04 20:47:17, ajuma wrote:
> This seems like the most realistic thing to do (since cc doesn't actually skip
> recording layers with empty visible rects). This might change benchmark
results
> though, so be sure to mention this in the description.

Done.

https://codereview.chromium.org/1855013002/diff/80001/cc/trees/draw_property_...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/1855013002/diff/80001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:726: // don't need to be updated.
On 2016/04/04 20:47:18, ajuma wrote:
> This comment doesn't apply anymore (since we're not making up a list of layers
> that need visible rects). So we could just add these layers above.

Done.

https://codereview.chromium.org/1855013002/diff/80001/cc/trees/draw_property_...
cc/trees/draw_property_utils.cc:745: visible_layer_list.push_back(layer.get());
On 2016/04/04 20:47:17, ajuma wrote:
> Can we avoid this copy? Maybe use templating so that CalculateVisibleRects
> doesn't care whether it gets a list of pointers or a list of ScopedRefPtrs?

Done.

[ajuma, 2016-04-04 21:31:16]

Thanks! lgtm

[jaydasika, 2016-04-04 21:32:58]

Description was changed from

==========
cc : Stop calculating visible rects on main thread

This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more 
realistic (cc doesn't skip such layers).

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Stop calculating visible rects on main thread

Because they are unused outside tests.
This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more 
realistic (cc doesn't skip such layers).

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-04 21:33:12]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-04-04 21:33:32]

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

[commit-bot: I haz the power, 2016-04-04 23:26:26]

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

[commit-bot: I haz the power, 2016-04-04 23:26:27]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/8...)

[jaydasika, 2016-04-05 00:57:12]

Made one more change to address layout test failures. Mask layers always get
added to update_layer_list.

[ajuma, 2016-04-05 13:24:47]

On 2016/04/05 00:57:12, jaydasika wrote:
> Made one more change to address layout test failures. Mask layers always get
> added to update_layer_list.

Please add a unit test for this.

[jaydasika, 2016-04-05 17:16:06]

On 2016/04/05 13:24:47, ajuma wrote:
> On 2016/04/05 00:57:12, jaydasika wrote:
> > Made one more change to address layout test failures. Mask layers always get
> > added to update_layer_list.
> 
> Please add a unit test for this.

Done.

[ajuma, 2016-04-05 17:22:11]

Thanks, lgtm

[jaydasika, 2016-04-05 18:08:39]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-04-05 18:08:56]

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

[commit-bot: I haz the power, 2016-04-05 19:07:16]

Description was changed from

==========
cc : Stop calculating visible rects on main thread

Because they are unused outside tests.
This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more 
realistic (cc doesn't skip such layers).

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Stop calculating visible rects on main thread

Because they are unused outside tests.
This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more 
realistic (cc doesn't skip such layers).

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-04-05 19:07:18]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-04-05 19:08:33]

Description was changed from

==========
cc : Stop calculating visible rects on main thread

Because they are unused outside tests.
This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more 
realistic (cc doesn't skip such layers).

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Stop calculating visible rects on main thread

Because they are unused outside tests.
This CL also stops skipping layers with empty visible rects
in record_and_rasterize benchmark because that is more
realistic (cc doesn't skip such layers).

BUG=586268
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/0f02fed2f4a7503c715501f74f32b0654fa15c5d
Cr-Commit-Position: refs/heads/master@{#385247}
==========

[commit-bot: I haz the power, 2016-04-05 19:08:35]

Patchset 9 (id:??) landed as
https://crrev.com/0f02fed2f4a7503c715501f74f32b0654fa15c5d
Cr-Commit-Position: refs/heads/master@{#385247}