Connect LTHI::ScrollAnimated to cc::Viewport

cc/layers/viewport.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/layers/viewport.h"
 
 #include "base/logging.h"
 #include "cc/input/top_controls_manager.h"
 #include "cc/trees/layer_tree_host_impl.h"
 #include "cc/trees/layer_tree_impl.h"
@@ skipping 46 matching lines @@
       scroll_tree.Node(OuterScrollLayer()->scroll_tree_index());
   pending_content_delta -= host_impl_->ScrollSingleNode(
       outer_node, pending_content_delta, viewport_point, is_direct_manipulation,
       &scroll_tree);
   result.consumed_delta = delta - AdjustOverscroll(pending_content_delta);
 
   result.content_scrolled_delta = content_delta - pending_content_delta;
   return result;
 }
 
+bool Viewport::ShouldAnimateViewport(const gfx::Vector2dF& viewport_delta,
+                                     const gfx::Vector2dF& pending_delta) {
+  float max_dim_viewport_delta =
+      std::abs(viewport_delta.x()) > std::abs(viewport_delta.y())
+          ? viewport_delta.x()
+          : viewport_delta.y();
+  float max_dim_pending_delta =
+      std::abs(pending_delta.x()) > std::abs(pending_delta.y())
+          ? pending_delta.x()
+          : pending_delta.y();
+  return std::abs(max_dim_viewport_delta) > std::abs(max_dim_pending_delta);

[bokan, 2016/04/06 13:35:52]

If you want to animate the viewport which will scroll the most, shouldn't you
compare the sums of abs(x) and abs(y)? If you have viewport_delta(99, 99) and
pending_delta(0, 100), you'll return false even though the viewport will consume
198 but the pending is only 100.

[ymalik, 2016/04/06 15:33:45]

I think the animation duration is determined by the maximum dimension.
https://code.google.com/p/chromium/codesearch#chromium/src/cc/animation/scrol...

[bokan, 2016/04/06 19:38:23]

Ok, this is fine then. IMO, std::max(viewport_delta.x(), viewport_delta.y())
would be clearer.

+}
+
+gfx::Vector2dF Viewport::ScrollAnimated(const gfx::Vector2dF& delta) {
+  ScrollTree& scroll_tree =
+      host_impl_->active_tree()->property_trees()->scroll_tree;
+
+  float scale_factor = host_impl_->active_tree()->current_page_scale_factor();
+  gfx::Vector2dF scaled_delta = delta;
+  scaled_delta.Scale(1.f / scale_factor);
+
+  ScrollNode* inner_node =
+      scroll_tree.Node(InnerScrollLayer()->scroll_tree_index());
+  LayerTreeHostImpl::ScrollAmount inner_scroll =
+      host_impl_->ComputeScrollDelta(inner_node, delta);
+
+  // Animate the viewport to which the majority of scroll delta will be applied.
+  // The animation system only supports running one scroll offset animation.
+  gfx::Vector2dF pending_delta = scaled_delta - inner_scroll.delta;
+  bool animating_inner_viewport =
+      ShouldAnimateViewport(inner_scroll.delta, pending_delta);

[bokan, 2016/04/06 13:35:52]

Shouldn't this compare inner_scroll.delta to outer_scroll.delta? The outer
viewport might not be able to accept all or any of that pending_delta.

[ymalik, 2016/04/06 15:33:45]

Yes, you're right! Done.

+  pending_delta.Scale(scale_factor);
+
+  ScrollNode* outer_node =
+      scroll_tree.Node(OuterScrollLayer()->scroll_tree_index());
+  LayerTreeHostImpl::ScrollAmount outer_scroll =
+      host_impl_->ComputeScrollDelta(outer_node, pending_delta);
+
+  if (inner_scroll.delta.IsZero() && outer_scroll.delta.IsZero())
+    return gfx::Vector2dF(0, 0);
+
+  bool will_animate = false;
+  if (animating_inner_viewport) {
+    scroll_tree.ScrollBy(outer_node, outer_scroll.delta,

[bokan, 2016/04/06 13:35:52]

Won't this cause a visible "jump"? I would guess that scrolling only one at a
time but animated would feel better...

[ymalik, 2016/04/06 15:33:45]

The jump doesn't look terribly bad IMO. It also only happens when we reach the
edge of the visual viewport. It's still better than the current behavior where
we only scroll the outer viewport. We will need to have some sort of state logic
to start the second animation once the first one is finished, which may be a lot
of work.

Added comments to address this. 

[bokan, 2016/04/06 19:38:23]

Acknowledged.

+                         host_impl_->active_tree());
+    will_animate = host_impl_->ScrollAnimationCreate(inner_node, inner_scroll);
+  } else {
+    scroll_tree.ScrollBy(inner_node, inner_scroll.delta,
+                         host_impl_->active_tree());
+    will_animate = host_impl_->ScrollAnimationCreate(outer_node, outer_scroll);
+  }
+
+  if (will_animate) {
+    // Consume entire scroll delta as long as we are starting an animation.
+    return delta;
+  }
+
+  pending_delta = scaled_delta - inner_scroll.delta - outer_scroll.delta;
+  pending_delta.Scale(scale_factor);
+  return pending_delta;
+}
+
 void Viewport::SnapPinchAnchorIfWithinMargin(const gfx::Point& anchor) {
   gfx::SizeF viewport_size = gfx::SizeF(
       host_impl_->active_tree()->InnerViewportContainerLayer()->bounds());
 
   if (anchor.x() < kPinchZoomSnapMarginDips)
     pinch_anchor_adjustment_.set_x(-anchor.x());
   else if (anchor.x() > viewport_size.width() - kPinchZoomSnapMarginDips)
     pinch_anchor_adjustment_.set_x(viewport_size.width() - anchor.x());
 
   if (anchor.y() < kPinchZoomSnapMarginDips)
@@ skipping 100 matching lines @@
 
 LayerImpl* Viewport::InnerScrollLayer() const {
   return host_impl_->InnerViewportScrollLayer();
 }
 
 LayerImpl* Viewport::OuterScrollLayer() const {
   return host_impl_->OuterViewportScrollLayer();
 }
 
 }  // namespace cc