Tweak the math in ScrollOffsetAnimationCurve::UpdateTarget.

cc/animation/scroll_offset_animation_curve.cc

Index: cc/animation/scroll_offset_animation_curve.cc
diff --git a/cc/animation/scroll_offset_animation_curve.cc b/cc/animation/scroll_offset_animation_curve.cc
index 6cd823f03c7f832cb698cd6bf3e2970f59740d4e..7b8708198969c071d1af3052c3aed6746f31e973 100644
--- a/cc/animation/scroll_offset_animation_curve.cc
+++ b/cc/animation/scroll_offset_animation_curve.cc
@@ -19,6 +19,8 @@ namespace cc {
 
 namespace {
 
+const double kEpsilon = 0.01f;
+
 static float MaximumDimension(const gfx::Vector2dF& delta) {
   return std::abs(delta.x()) > std::abs(delta.y()) ? delta.x() : delta.y();
 }
@@ -125,6 +127,31 @@ scoped_ptr<AnimationCurve> ScrollOffsetAnimationCurve::Clone() const {
   return std::move(curve_clone);
 }
 
+static double VelocityBasedDurationBound(gfx::Vector2dF old_delta,
+                                         double old_velocity,
+                                         double old_duration,
+                                         gfx::Vector2dF new_delta) {
+  double old_delta_max_dimension = MaximumDimension(old_delta);
+  double new_delta_max_dimension = MaximumDimension(new_delta);
+
+  // If we are already at the target, stop animating.
+  if (std::abs(new_delta_max_dimension) < kEpsilon)
+    return 0;
+
+  // Guard against division by zero.
+  if (std::abs(old_delta_max_dimension) < kEpsilon ||
+      std::abs(old_velocity) < kEpsilon) {
+    return std::numeric_limits<double>::infinity();
+  }
+
+  // Estimate how long it will take to reach the new target at our present
+  // velocity, with some fudge factor to account for the "ease out".
+  double bound = old_duration / old_velocity * new_delta_max_dimension /
+                 old_delta_max_dimension * 2.5f;

[ymalik, 2015/12/30 19:30:18]

I am trying to understand how this gives us the estimate mentioned in the
comment. Should this not simply be new_delta_max_dimension / old_velocity? What
are the other params for? 

[skobes, 2015/12/30 19:44:20]

Sorry it's a little confusing.  The old_velocity is actually the slope on the
curve normalized to (0,0)-(1,1), so we have to multiply it by (old_delta /
old_duration) to get a "true" px/sec velocity.

[ymalik, 2015/12/30 20:09:59]

Ah I see. Perhaps putting it in parentheses would make that more clear?
new_delta / (old_velocity * old_delta / old_duration). 
Or maybe renaming old_velocity = normalized_velocity * old_delta / old_duration
to make that even more clear. 

Up to you though :)

[skobes, 2015/12/30 20:19:43]

Done.

+
+  return bound < kEpsilon ? std::numeric_limits<double>::infinity() : bound;

[ajuma, 2015/12/30 19:58:23]

Should this be returning zero rather than infinity for tiny bounds? (Actually,
do we need a special case at all, given that the caller already handles the case
where the duration is smaller than epsilon?)

[skobes, 2015/12/30 20:07:30]

The important thing is really to return infinity when bound is negative, meaning
the current velocity is in the wrong direction.

I think bound will never be (close to) 0 here, since we have already handled
std::abs(new_delta_max_dimension) < kEpsilon, and old_duration will not be 0.

I've updated the check to use 0 instead of kEpsilon for clarity, and added a
comment.

+}
+
 void ScrollOffsetAnimationCurve::UpdateTarget(
     double t,
     const gfx::ScrollOffset& new_target) {
@@ -135,22 +162,30 @@ void ScrollOffsetAnimationCurve::UpdateTarget(
 
   double old_duration =
       (total_animation_duration_ - last_retarget_).InSecondsF();
-  double new_duration =
-      SegmentDuration(new_delta, duration_behavior_).InSecondsF();
-
   double old_velocity = timing_function_->Velocity(
       (t - last_retarget_.InSecondsF()) / old_duration);
 
+  // Use the velocity-based duration bound when it is less than the constant
+  // segment duration. This minimizes the "rubber-band" bouncing effect when
+  // old_velocity is large and new_delta is small.
+  double new_duration =
+      std::min(SegmentDuration(new_delta, duration_behavior_).InSecondsF(),
+               VelocityBasedDurationBound(old_delta, old_velocity, old_duration,
+                                          new_delta));
+
+  if (new_duration < kEpsilon) {
+    // We are already at or very close to the new target. Stop animating.
+    target_value_ = new_target;
+    total_animation_duration_ = base::TimeDelta::FromSecondsD(t);

[ymalik, 2015/12/30 19:30:18]

I know that the premise of this condition is that we are either at or close to
the new target, but should it also update the last_retarget_ and others like at
the end of the function in case UpdateTarget gets called again in time <
epsilon?

[skobes, 2015/12/30 19:44:20]

We don't want to treat this as a new segment, since its delta and duration are
both near 0 and we can't make a meaningful timing function.  Instead we are
making the curve report that it has reached the end of the current segment.

[ymalik, 2015/12/30 20:09:59]

Makes sense. Thanks!

+    return;
+  }
+
   // TimingFunction::Velocity gives the slope of the curve from 0 to 1.
   // To match the "true" velocity in px/sec we must adjust this slope for
   // differences in duration and scroll delta between old and new curves.
-  const double kEpsilon = 0.01f;
-  double new_delta_max_dimension = MaximumDimension(new_delta);
   double new_velocity =
-      new_delta_max_dimension < kEpsilon  // Guard against division by 0.
-          ? old_velocity
-          : old_velocity * (new_duration / old_duration) *
-                (MaximumDimension(old_delta) / new_delta_max_dimension);
+      old_velocity * (new_duration / old_duration) *
+      (MaximumDimension(old_delta) / MaximumDimension(new_delta));
 
   initial_value_ = current_position;
   target_value_ = new_target;