cc: Rename clip rects in CalcDrawProps.

cc: Rename clip rects in CalcDrawProps.

Name these clip rects after the space they are in.

R=enne
BUG=263512
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=213328

[enne (OOO), 2013-07-23 22:53:34]

Name changes in general lgtm

https://codereview.chromium.org/19664009/diff/6001/cc/trees/layer_tree_host_c...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/19664009/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common.cc:867: bool ancestor_clips_subtree,
ancestor_clips_descendants to match layer_or_ancestor_clips_descendants?

https://codereview.chromium.org/19664009/diff/6001/cc/trees/layer_tree_host_c...
cc/trees/layer_tree_host_common.cc:1360: clip_rect_in_ancestor_target_space =
rect_in_target_space;
This *does* look awfully suspicious.

[danakj, 2013-07-23 22:55:39]

PTAL

[enne (OOO), 2013-07-23 23:10:38]

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common.cc:168: // its own render target's space to the
current render target's space.
'its' is a little ambiguous here and makes this comment hard to parse.

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common.cc:1277:
target_surface_clip_rect_in_target_space =
Too many targets here makes this renaming less clear.

clip_rect_in_target_space?
clip_rect_for_layer_and_descendants_in_target_space?

[danakj, 2013-07-23 23:11:56]

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common.cc:1277:
target_surface_clip_rect_in_target_space =
On 2013/07/23 23:10:38, enne wrote:
> Too many targets here makes this renaming less clear.
> 
> clip_rect_in_target_space?
> clip_rect_for_layer_and_descendants_in_target_space?

So this isn't the clip rect of the layer, it is the clip rect of the target
surface in it's own target. I think you need these targets to say what it is.

[danakj, 2013-07-23 23:19:48]

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common.cc:168: // its own render target's space to the
current render target's space.
On 2013/07/23 23:10:38, enne wrote:
> 'its' is a little ambiguous here and makes this comment hard to parse.

I tried doing something more precise here, PTAL.

[enne (OOO), 2013-07-23 23:21:44]

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common.cc:1277:
target_surface_clip_rect_in_target_space =
On 2013/07/23 23:11:56, danakj wrote:
> On 2013/07/23 23:10:38, enne wrote:
> > Too many targets here makes this renaming less clear.
> > 
> > clip_rect_in_target_space?
> > clip_rect_for_layer_and_descendants_in_target_space?
> 
> So this isn't the clip rect of the layer, it is the clip rect of the target
> surface in its own target. I think you need these targets to say what it is.

clip_rect_from_ancestor_for_layer_and_descendants_in_target_space.  ;)

More seriously, since you're transforming
clip_rect_from_ancestor_in_ancestor_target_space, what about
clip_rect_from_ancestor_in_target_space?

[danakj, 2013-07-23 23:22:18]

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common.cc:1277:
target_surface_clip_rect_in_target_space =
On 2013/07/23 23:11:56, danakj wrote:
> On 2013/07/23 23:10:38, enne wrote:
> > Too many targets here makes this renaming less clear.
> > 
> > clip_rect_in_target_space?
> > clip_rect_for_layer_and_descendants_in_target_space?
> 
> So this isn't the clip rect of the layer, it is the clip rect of the target
> surface in it's own target. I think you need these targets to say what it is.

Sorry, more precisely:

This is the clip rect of the current layer's target surface in that surface's
target, but translated into the current layer's target space. That's why i have
it as

"target surface's clip rect" in (current layer's) "target space"

[enne (OOO), 2013-07-23 23:24:31]

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/19664009/diff/10001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_common.cc:168: // its own render target's space to the
current render target's space.
On 2013/07/23 23:19:48, danakj wrote:
> On 2013/07/23 23:10:38, enne wrote:
> > 'its' is a little ambiguous here and makes this comment hard to parse.
> 
> I tried doing something more precise here, PTAL.

Thanks.  That's a lot better.

[danakj, 2013-07-23 23:40:48]

target_surface_clip_rect_in_target_space =>
clip_rect_for_target_surface_in_target_space HTH

[commit-bot: I haz the power, 2013-07-24 00:05:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/19664009/27001

[commit-bot: I haz the power, 2013-07-24 03:16:07]

Change committed as 213328