cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

After the move, FindScrollLayerForDeviceViewportPoint no longer
depends on LayerImpl, it iterates completely on scroll tree now.

BUG=568830
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[sunxd, 2016-02-11 21:41:32]

Description was changed from

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

BUG=568830
==========

to

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

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

[sunxd, 2016-02-11 22:24:55]

Description was changed from

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

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

to

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

After the move, FindScrollLayerForDeviceViewportPoint no longer depends on
LayerImpl, it iterates completely on scroll tree now.

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

[sunxd, 2016-02-11 22:24:55]

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

[sunxd, 2016-02-11 22:25:14]

Description was changed from

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

After the move, FindScrollLayerForDeviceViewportPoint no longer depends on
LayerImpl, it iterates completely on scroll tree now.

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

to

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

After the move, FindScrollLayerForDeviceViewportPoint no longer
depends on LayerImpl, it iterates completely on scroll tree now.

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

[jaydasika, 2016-02-11 23:38:56]

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/property_tree....
cc/trees/property_tree.cc:1286: gfx::Transform
ScrollTree::ScreenSpaceTransform(int scroll_node_id) const {
Question : Is Scroll Node creation reasons a subset of Transform node creation
reasons ?
I looked at the code and  we don't create a transform node for having a non fast
scrollable region but we create a scroll node. In that case, the screen space
transform computed here should also include the layer's offset to transform
parent).

[sunxd, 2016-02-12 15:33:30]

I think we are ok without referring to offset_to_transform_parent in TryScroll,
but this needs to be verified.

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/property_tree....
cc/trees/property_tree.cc:1286: gfx::Transform
ScrollTree::ScreenSpaceTransform(int scroll_node_id) const {
On 2016/02/11 23:38:56, jaydasika wrote:
> Question : Is Scroll Node creation reasons a subset of Transform node creation
> reasons ?
> I looked at the code and  we don't create a transform node for having a non
fast
> scrollable region but we create a scroll node. In that case, the screen space
> transform computed here should also include the layer's offset to transform
> parent). 

So we are only interested with ScreenSpaceTransform in non-fast scrollable
region case in TryScroll (only caller of this function, TryScroll also checks if
this transform is non-invertible, but if it is, it will have a transform node,
otherwise we are not interested.). It seems that non-fast scrollable region can
only be set on scrollable layers
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...),
but I'm not very sure about this.

If we do have scroll tree nodes that doesn't have a corresponding transform node
and we are interested in its ScreenSpaceTransform, we can put
offset_to_transform_parent in scroll tree, and update it when changing the value
in the layers.

[ajuma, 2016-02-12 17:03:02]

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.h (right):

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.h:606: InputHandler::ScrollStatus TryScroll(const
gfx::PointF& device_viewport_point,
screen_space_point

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/property_tree....
cc/trees/property_tree.cc:1286: gfx::Transform
ScrollTree::ScreenSpaceTransform(int scroll_node_id) const {
On 2016/02/12 15:33:30, sunxd wrote:
> On 2016/02/11 23:38:56, jaydasika wrote:
> > Question : Is Scroll Node creation reasons a subset of Transform node
creation
> > reasons ?
> > I looked at the code and  we don't create a transform node for having a non
> fast
> > scrollable region but we create a scroll node. In that case, the screen
space
> > transform computed here should also include the layer's offset to transform
> > parent). 
> 
> So we are only interested with ScreenSpaceTransform in non-fast scrollable
> region case in TryScroll (only caller of this function, TryScroll also checks
if
> this transform is non-invertible, but if it is, it will have a transform node,
> otherwise we are not interested.). It seems that non-fast scrollable region
can
> only be set on scrollable layers
>
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...),
> but I'm not very sure about this.
> 
> If we do have scroll tree nodes that doesn't have a corresponding transform
node
> and we are interested in its ScreenSpaceTransform, we can put
> offset_to_transform_parent in scroll tree, and update it when changing the
value
> in the layers.

For TryScroll we might be ok, but I'm not sure if that's true for other
scrolling code. If we're exposing a ScreenSpaceTransform method on ScrollTree,
it seems safer to make sure it works for all scroll nodes.

https://codereview.chromium.org/1689293002/diff/20001/cc/trees/property_tree....
cc/trees/property_tree.cc:1292: screen_space_transform.FlattenTo2d();
I believe this already happens when to_screen is computed in the transform tree.

However, for layers that don't have transform nodes, you need to flatten if
LayerImpl::should_flatten_transform_from_property_trees is true.

[sunxd, 2016-02-16 18:57:45]

Merged with origin/master branch, and fixed one line in TryScroll.

[ajuma, 2016-02-16 19:42:34]

On 2016/02/16 18:57:45, sunxd wrote:
> Merged with origin/master branch, and fixed one line in TryScroll.

Thanks, lgtm.

[sunxd, 2016-02-16 21:26:58]

The CQ bit was checked by sunxd@chromium.org

[commit-bot: I haz the power, 2016-02-16 21:29:51]

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

[commit-bot: I haz the power, 2016-02-16 22:08:40]

Description was changed from

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

After the move, FindScrollLayerForDeviceViewportPoint no longer
depends on LayerImpl, it iterates completely on scroll tree now.

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

to

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

After the move, FindScrollLayerForDeviceViewportPoint no longer
depends on LayerImpl, it iterates completely on scroll tree now.

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

[commit-bot: I haz the power, 2016-02-16 22:08:41]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-02-16 22:55:13]

Description was changed from

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

After the move, FindScrollLayerForDeviceViewportPoint no longer
depends on LayerImpl, it iterates completely on scroll tree now.

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

to

==========
cc: Move TryScroll from LayerImpl to LayerTreeHostImpl

After the move, FindScrollLayerForDeviceViewportPoint no longer
depends on LayerImpl, it iterates completely on scroll tree now.

BUG=568830
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[commit-bot: I haz the power, 2016-02-16 22:55:13]

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