cc : Fix occlusion tracking bug for contributing surface

cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

Committed: https://crrev.com/69c77aa0e63a8a0f3feff43e942b972fcfe93581
Cr-Commit-Position: refs/heads/master@{#405611}

[jaydasika, 2016-07-14 18:28:12]

Description was changed from

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

BUG=501135
==========

to

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

[jaydasika, 2016-07-14 18:36:03]

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

[jaydasika, 2016-07-14 18:36:24]

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

[jaydasika, 2016-07-14 18:36:24]

PTAL

[jaydasika, 2016-07-14 18:36:32]

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

[commit-bot: I haz the power, 2016-07-14 18:36:54]

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-07-14 18:36:56]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.blink
   For more details, see http://crbug.com/617627.

[enne (OOO), 2016-07-14 19:51:41]

https://codereview.chromium.org/2147283002/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2147283002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:965: // We are calculating transform between two
render surfaces. So, we
I think Apply and Remove look too similar but do very different things, re:
pre-concat vs post-concat.  I think you should either not make them functions
and do that work inline here or add some additional words to the function names
ApplySublayerScaleAtTarget or some such that make it clear that they are not
inverses of each other.

[commit-bot: I haz the power, 2016-07-14 19:52:41]

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

[commit-bot: I haz the power, 2016-07-14 19:52:42]

Dry run: This issue passed the CQ dry run.

[jaydasika, 2016-07-14 21:00:46]

https://codereview.chromium.org/2147283002/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2147283002/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:965: // We are calculating transform between two
render surfaces. So, we
On 2016/07/14 19:51:41, enne wrote:
> I think Apply and Remove look too similar but do very different things, re:
> pre-concat vs post-concat.  I think you should either not make them functions
> and do that work inline here or add some additional words to the function
names
> ApplySublayerScaleAtTarget or some such that make it clear that they are not
> inverses of each other.

I have renamed them to PostConcatSublayerScale and ConcatInverseSublayerScale.
In a follow-up, I will make all callers of PostConcat send the target effect
node id to it directly. So these functions will just grab the sublayer scale
from the effect node they are send and concat/postConcat to the transform.

https://codereview.chromium.org/2147283002/diff/40001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2147283002/diff/40001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:994: draw_transform * it->DrawTransform());
Missed this earlier.

[jaydasika, 2016-07-14 21:01:04]

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

[commit-bot: I haz the power, 2016-07-14 21:02:12]

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-07-14 21:02:14]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.blink
   For more details, see http://crbug.com/617627.

[enne (OOO), 2016-07-14 21:15:31]

https://codereview.chromium.org/2147283002/diff/40001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2147283002/diff/40001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:994: draw_transform * it->DrawTransform());
On 2016/07/14 at 21:00:46, jaydasika wrote:
> Missed this earlier.

Does a test catch this detail?

[jaydasika, 2016-07-14 21:30:08]

https://codereview.chromium.org/2147283002/diff/40001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/2147283002/diff/40001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:994: draw_transform * it->DrawTransform());
On 2016/07/14 21:15:31, enne wrote:
> On 2016/07/14 at 21:00:46, jaydasika wrote:
> > Missed this earlier.
> 
> Does a test catch this detail?

No. Now it does.

[jaydasika, 2016-07-14 21:30:19]

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

[commit-bot: I haz the power, 2016-07-14 21:31:04]

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-07-14 21:31:10]

Your CL relies on deprecated CQ feature(s):
 * Specifying master names in CQ_INCLUDE_TRYBOTS part of description without
"master." prefix is deprecated:
     tryserver.blink
   For more details, see http://crbug.com/617627.

[jaydasika, 2016-07-14 21:31:53]

Description was changed from

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

to

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

BUG=501135
==========

[enne (OOO), 2016-07-14 21:33:03]

lgtm

[jaydasika, 2016-07-14 21:47:13]

Description was changed from

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

BUG=501135
==========

to

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

[jaydasika, 2016-07-14 21:48:39]

Description was changed from

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

to

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

[commit-bot: I haz the power, 2016-07-14 22:42:22]

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

[commit-bot: I haz the power, 2016-07-14 22:42:23]

Dry run: This issue passed the CQ dry run.

[jaydasika, 2016-07-14 22:42:58]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-07-14 22:43:35]

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-07-14 22:49:04]

Description was changed from

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

to

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

[commit-bot: I haz the power, 2016-07-14 22:49:06]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-14 22:49:13]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-14 22:50:36]

Description was changed from

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

to

==========
cc : Fix occlusion tracking bug for contributing surface

We currently assume that the occlusion calculated for contributing
surface (the second last element of the stack in occlusion tracker) is
always the occlusion by its target. This is not true.

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

Committed: https://crrev.com/69c77aa0e63a8a0f3feff43e942b972fcfe93581
Cr-Commit-Position: refs/heads/master@{#405611}
==========

[commit-bot: I haz the power, 2016-07-14 22:50:37]

Patchset 4 (id:??) landed as
https://crrev.com/69c77aa0e63a8a0f3feff43e942b972fcfe93581
Cr-Commit-Position: refs/heads/master@{#405611}