cc: Avoid duplicate work when computing draw properties using property trees

cc: Avoid duplicate work when computing draw properties using property trees

This adds a ComputeLayerDrawPropertiesUsingPropertyTrees function and a
ComputeSurfaceDrawPropertiesUsingPropertyTrees function that compute
these properties together, avoiding re-computing values that are needed
by the computation of more than one property.

BUG=497817
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/d301953f7a9c5ce542ebca832a26b234df81ce07
Cr-Commit-Position: refs/heads/master@{#347400}

[ajuma, 2015-08-26 14:25:25]

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

[jaydasika, 2015-08-26 15:16:44]

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

https://codereview.chromium.org/1310283002/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:2550: ? layer->drawable_content_rect()
All the draw properties here are the ones calculated in CDP(drawable content
rect here, is_clipped, clip_rect and draw_transform below). Are you using it
because the property trees ones are not calculated before this point?

[ajuma, 2015-08-26 15:21:32]

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

https://codereview.chromium.org/1310283002/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:2550: ? layer->drawable_content_rect()
On 2015/08/26 15:16:44, jaydasika wrote:
> All the draw properties here are the ones calculated in CDP(drawable content
> rect here, is_clipped, clip_rect and draw_transform below). Are you using it
> because the property trees ones are not calculated before this point?

No, I changed this so that once we switch from CDP to property trees, we reuse
the values we've already computed for each layer rather than recomputing them
again. Changing this now makes any perf measurements more realistic (since we're
not really going to be recomputing them), and also allows removing the
externally exposed versions of the <Property>FromPropertyTrees functions.

[jaydasika, 2015-08-26 15:27:42]

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

https://codereview.chromium.org/1310283002/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:2550: ? layer->drawable_content_rect()
On 2015/08/26 15:21:32, ajuma wrote:
> On 2015/08/26 15:16:44, jaydasika wrote:
> > All the draw properties here are the ones calculated in CDP(drawable content
> > rect here, is_clipped, clip_rect and draw_transform below). Are you using it
> > because the property trees ones are not calculated before this point?
> 
> No, I changed this so that once we switch from CDP to property trees, we reuse
> the values we've already computed for each layer rather than recomputing them
> again. Changing this now makes any perf measurements more realistic (since
we're
> not really going to be recomputing them), and also allows removing the
> externally exposed versions of the <Property>FromPropertyTrees functions.

This is inside CalculateRenderSurfaceLayerList which is called before
VerifyPropertyTrees where the draw proeprties using property trees are computed.
So, when we switch from CDP to property trees, we haven't yet calculated the
draw properties at this point.

[ajuma, 2015-08-26 15:31:38]

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

https://codereview.chromium.org/1310283002/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:2550: ? layer->drawable_content_rect()
On 2015/08/26 15:27:42, jaydasika wrote:
> On 2015/08/26 15:21:32, ajuma wrote:
> > On 2015/08/26 15:16:44, jaydasika wrote:
> > > All the draw properties here are the ones calculated in CDP(drawable
content
> > > rect here, is_clipped, clip_rect and draw_transform below). Are you using
it
> > > because the property trees ones are not calculated before this point?
> > 
> > No, I changed this so that once we switch from CDP to property trees, we
reuse
> > the values we've already computed for each layer rather than recomputing
them
> > again. Changing this now makes any perf measurements more realistic (since
> we're
> > not really going to be recomputing them), and also allows removing the
> > externally exposed versions of the <Property>FromPropertyTrees functions.
> 
> This is inside CalculateRenderSurfaceLayerList which is called before
> VerifyPropertyTrees where the draw proeprties using property trees are
computed.
> So, when we switch from CDP to property trees, we haven't yet calculated the
> draw properties at this point. 

Right, when we switch, the property trees work will happen where CDP happens now
(that is, in between CalculateRenderTarget and CalculateRenderSurfaceLayerList),
not where verification happens now.

[jaydasika, 2015-08-26 15:54:03]

lgtm

lgtm.

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

https://codereview.chromium.org/1310283002/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:2550: ? layer->drawable_content_rect()
On 2015/08/26 15:31:38, ajuma wrote:
> On 2015/08/26 15:27:42, jaydasika wrote:
> > On 2015/08/26 15:21:32, ajuma wrote:
> > > On 2015/08/26 15:16:44, jaydasika wrote:
> > > > All the draw properties here are the ones calculated in CDP(drawable
> content
> > > > rect here, is_clipped, clip_rect and draw_transform below). Are you
using
> it
> > > > because the property trees ones are not calculated before this point?
> > > 
> > > No, I changed this so that once we switch from CDP to property trees, we
> reuse
> > > the values we've already computed for each layer rather than recomputing
> them
> > > again. Changing this now makes any perf measurements more realistic (since
> > we're
> > > not really going to be recomputing them), and also allows removing the
> > > externally exposed versions of the <Property>FromPropertyTrees functions.
> > 
> > This is inside CalculateRenderSurfaceLayerList which is called before
> > VerifyPropertyTrees where the draw proeprties using property trees are
> computed.
> > So, when we switch from CDP to property trees, we haven't yet calculated the
> > draw properties at this point. 
> 
> Right, when we switch, the property trees work will happen where CDP happens
now
> (that is, in between CalculateRenderTarget and
CalculateRenderSurfaceLayerList),
> not where verification happens now.

Okay.

https://codereview.chromium.org/1310283002/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:2626:
DrawableContentRectOfSurface(layer->render_surface()));
We already have a RenderSurfaceImpl::DrawableContentRect(), that can be used
here and we can remove this function from draw_property_utils. But, we can do it
after switching from CDP to property trees, since it is not going to cause any
difference in perf computation. In either case, we should remember to remove it.

[ajuma, 2015-08-26 17:06:43]

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

https://codereview.chromium.org/1310283002/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:2626:
DrawableContentRectOfSurface(layer->render_surface()));
On 2015/08/26 15:54:03, jaydasika wrote:
> We already have a RenderSurfaceImpl::DrawableContentRect(), that can be used
> here and we can remove this function from draw_property_utils. But, we can do
it
> after switching from CDP to property trees, since it is not going to cause any
> difference in perf computation. In either case, we should remember to remove
it.
>  

Thanks, good catch. Removed the function from draw_property_utils.

[ajuma, 2015-09-02 17:08:04]

enne, PTAL.

[enne (OOO), 2015-09-02 21:51:35]

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/draw_property...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/draw_property...
cc/trees/draw_property_utils.cc:846: void
ComputeLayerDrawPropertiesUsingPropertyTrees(
I love how simple this function is!

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/draw_property...
File cc/trees/draw_property_utils.h (right):

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/draw_property...
cc/trees/draw_property_utils.h:85: struct CC_EXPORT SurfaceDrawProperties {
It's a bit odd to have this here and DrawProperties elsewhere.  Should this get
its own file? Or go live in draw_properties too?

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_common.cc:2314: // content_rect has to be computed
recursively, so is computed separately from
It may need to be computed differently, but it does still seem like a "surface
draw property" to me.  Is it possible to combine them?

[ajuma, 2015-09-03 15:06:49]

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/draw_property...
File cc/trees/draw_property_utils.h (right):

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/draw_property...
cc/trees/draw_property_utils.h:85: struct CC_EXPORT SurfaceDrawProperties {
On 2015/09/02 21:51:35, enne wrote:
> It's a bit odd to have this here and DrawProperties elsewhere.  Should this
get
> its own file? Or go live in draw_properties too?

Moved to its own file.

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/1310283002/diff/120001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_common.cc:2314: // content_rect has to be computed
recursively, so is computed separately from
On 2015/09/02 21:51:35, enne wrote:
> It may need to be computed differently, but it does still seem like a "surface
> draw property" to me.  Is it possible to combine them?

Added content_rect to RenderSurfaceDrawProperties.

[enne (OOO), 2015-09-04 04:26:09]

lgtm

[ajuma, 2015-09-04 13:01:16]

The CQ bit was checked by ajuma@chromium.org

[ajuma, 2015-09-04 13:01:16]

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/1310283002/#ps140001
(title: "Move RenderSurfaceDrawProperties to its own file")

[commit-bot: I haz the power, 2015-09-04 13:01:25]

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

[commit-bot: I haz the power, 2015-09-04 14:20:28]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-09-04 14:21:07]

Patchset 8 (id:??) landed as
https://crrev.com/d301953f7a9c5ce542ebca832a26b234df81ce07
Cr-Commit-Position: refs/heads/master@{#347400}