cc: Fix draw transform computation for non-drawn layers

cc: Fix draw transform computation for non-drawn layers

When property trees are enabled, draw properties are only
computed for drawn layers. This means that when a non-drawn
layer's draw transform is needed, it must be computed
on-demand using the transform tree rather than being read
from the layer's draw properties.

BUG=560275
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[ajuma, 2015-11-26 19:32:04]

Description was changed from

==========
cc: Fix draw transform computation for non-drawn layers

BUG=560275
==========

to

==========
cc: Fix draw transform computation for non-drawn layers

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

[ajuma, 2015-11-27 19:37:56]

Description was changed from

==========
cc: Fix draw transform computation for non-drawn layers

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

to

==========
cc: Fix draw transform computation for non-drawn layers

When property trees are enabled, draw properties are only
computed for drawn layers. This means that when a non-drawn
layer's draw transform is needed, it must be computed
on-demand using the transform tree rather than being read
from the layer's draw properties.

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

[ajuma, 2015-11-27 19:37:56]

ajuma@chromium.org changed reviewers:
+ weiliangc@chromium.org

[ajuma, 2015-11-27 19:39:31]

There are similar bugs involving screen_space_transform, which I'm planning on
fixing in a separate CL.

The tests added in this CL fail when property trees are enabled without the
patch.

[weiliangc, 2015-11-27 20:24:29]

Does LayerDrawTransform() replaces all calls to LayerImpl's draw_transform()?
Can we just remove the LayerImpl::draw_transform()?

[ajuma, 2015-11-27 20:47:25]

On 2015/11/27 20:24:29, weiliangc wrote:
> Does LayerDrawTransform() replaces all calls to LayerImpl's draw_transform()?
> Can we just remove the LayerImpl::draw_transform()?

There are still other callers of LayerImpl::draw_transform(). I was a bit
worried about making these calls more expensive (draw_transform() just returns a
const ref, LayerDrawTransform always at least makes a copy). But maybe a good
solution is to use draw_properties().target_space_transform directly in places
where this extra cost might matter (mainly inside CDP itself, where it's safe to
assume that ancestor draw_properties are valid), remove draw_transform(), and
move LayerDrawTransform to LayerImpl. WDYT?

[weiliangc, 2015-11-27 20:52:21]

On 2015/11/27 at 20:47:25, ajuma wrote:
> On 2015/11/27 20:24:29, weiliangc wrote:
> > Does LayerDrawTransform() replaces all calls to LayerImpl's
draw_transform()?
> > Can we just remove the LayerImpl::draw_transform()?
> 
> There are still other callers of LayerImpl::draw_transform(). I was a bit
worried about making these calls more expensive (draw_transform() just returns a
const ref, LayerDrawTransform always at least makes a copy). But maybe a good
solution is to use draw_properties().target_space_transform directly in places
where this extra cost might matter (mainly inside CDP itself, where it's safe to
assume that ancestor draw_properties are valid), remove draw_transform(), and
move LayerDrawTransform to LayerImpl. WDYT?

Sounds good. Was worried if people mistakenly using draw_transform() with stale
value. This plan sounds nice.

[ajuma, 2015-11-27 23:12:49]

Patchset #5 (id:80001) has been deleted

[ajuma, 2015-11-30 16:56:53]

Patchset #5 (id:100001) has been deleted

[ajuma, 2015-11-30 17:55:52]

Removed LayerImpl::draw_transform(), and moved LayerDrawTransform to LayerImpl
(and just called it DrawTransform).

https://codereview.chromium.org/1479883002/diff/120001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.h (right):

https://codereview.chromium.org/1479883002/diff/120001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.h:226: void IncrementRenderSurfaceListIdForTesting();
In several unit tests, the render surface list id passed to CDP wasn't coming
from LayerTreeImpl, so LayerImpl::IsDrawnRenderSurfaceLayerListMember was
returning the wrong thing (meaning LayerImpl::DrawTransform also did the wrong
thing). I've added this method so tests can use LayerTreeImpl's id rather than
creating their own.

[weiliangc, 2015-11-30 19:16:38]

LGTM.

Thanks for fixing the unittests!

[ajuma, 2015-11-30 19:31:36]

The CQ bit was checked by ajuma@chromium.org

[commit-bot: I haz the power, 2015-11-30 19:34:38]

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

[commit-bot: I haz the power, 2015-11-30 19:43:51]

Description was changed from

==========
cc: Fix draw transform computation for non-drawn layers

When property trees are enabled, draw properties are only
computed for drawn layers. This means that when a non-drawn
layer's draw transform is needed, it must be computed
on-demand using the transform tree rather than being read
from the layer's draw properties.

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

to

==========
cc: Fix draw transform computation for non-drawn layers

When property trees are enabled, draw properties are only
computed for drawn layers. This means that when a non-drawn
layer's draw transform is needed, it must be computed
on-demand using the transform tree rather than being read
from the layer's draw properties.

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

[commit-bot: I haz the power, 2015-11-30 19:43:51]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2015-11-30 19:45:14]

Description was changed from

==========
cc: Fix draw transform computation for non-drawn layers

When property trees are enabled, draw properties are only
computed for drawn layers. This means that when a non-drawn
layer's draw transform is needed, it must be computed
on-demand using the transform tree rather than being read
from the layer's draw properties.

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

to

==========
cc: Fix draw transform computation for non-drawn layers

When property trees are enabled, draw properties are only
computed for drawn layers. This means that when a non-drawn
layer's draw transform is needed, it must be computed
on-demand using the transform tree rather than being read
from the layer's draw properties.

BUG=560275
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[commit-bot: I haz the power, 2015-11-30 19:45:15]

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