cc: Separate computation of clip rect and visible rect to two functions

cc: Separate computation of clip rect and visible rect to two functions

Separate clip rect and visible rect calculation so visible rect logic
would be easier to follow.

R=ajuma
BUG=594675
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[weiliangc, 2016-05-25 19:37:36]

Description was changed from

==========
cc: Separate computation of clip rect and visible rect to two functions

Separate clip rect and visible rect calculation so visible rect logic
would be easier to follow.

R=ajuma
BUG=594675
==========

to

==========
cc: Separate computation of clip rect and visible rect to two functions

Separate clip rect and visible rect calculation so visible rect logic
would be easier to follow.

R=ajuma
BUG=594675
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[weiliangc, 2016-05-25 19:40:18]

Still not too happy about the amount of code in visible rect calc but seems
reasonable first step?

[ajuma, 2016-05-25 20:40:19]

lgtm

On 2016/05/25 19:40:18, weiliangc wrote:
> Still not too happy about the amount of code in visible rect calc but seems
> reasonable first step?

Agreed, there's still a lot of code, but it's easier to understand now.

https://codereview.chromium.org/2008283003/diff/1/cc/trees/draw_property_util...
File cc/trees/draw_property_utils.cc (right):

https://codereview.chromium.org/2008283003/diff/1/cc/trees/draw_property_util...
cc/trees/draw_property_utils.cc:67: DCHECK(layer->clip_tree_index() ==
clip_node->id);
Please also DCHECK that the clip node target_id is != target_node_id.

https://codereview.chromium.org/2008283003/diff/1/cc/trees/draw_property_util...
cc/trees/draw_property_utils.cc:120: clip_node->data.resets_clip &&
!clip_node->data.applies_local_clip;
One more simplification: I believe is_unclipped actually implies fully_visible,
so I think we just need to check fully_visible instead of checking both.

https://codereview.chromium.org/2008283003/diff/1/cc/trees/draw_property_util...
cc/trees/draw_property_utils.cc:218: clip_node->data.resets_clip &&
!clip_node->data.applies_local_clip;
Here too, I think we only need to check fully_visible.

https://codereview.chromium.org/2008283003/diff/1/cc/trees/draw_property_util...
cc/trees/draw_property_utils.cc:236: // necessarily have the same target (the
root).
nit: This comment can be deleted since it was referring to resourceless software
mode, which is now handled separately above.

https://codereview.chromium.org/2008283003/diff/1/cc/trees/draw_property_util...
cc/trees/draw_property_utils.cc:264:
gfx::ToEnclosingRect(clip_node->data.clip_in_target_space);
Nit: use { } for the if/else since they're multiline

[weiliangc, 2016-06-06 17:48:47]

rebased and addressed all comments.

[weiliangc, 2016-06-06 18:08:44]

The CQ bit was checked by weiliangc@chromium.org

[weiliangc, 2016-06-06 18:08:45]

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/2008283003/#ps40001
(title: "address review comments")

[commit-bot: I haz the power, 2016-06-06 18:09:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2008283003/40001

[commit-bot: I haz the power, 2016-06-06 18:39:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-06 18:41:35]

Description was changed from

==========
cc: Separate computation of clip rect and visible rect to two functions

Separate clip rect and visible rect calculation so visible rect logic
would be easier to follow.

R=ajuma
BUG=594675
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc: Separate computation of clip rect and visible rect to two functions

Separate clip rect and visible rect calculation so visible rect logic
would be easier to follow.

R=ajuma
BUG=594675
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/a6dee63104bb2e8cc99bd8146dfdee0bdd3323db
Cr-Commit-Position: refs/heads/master@{#398078}
==========

[commit-bot: I haz the power, 2016-06-06 18:41:36]

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