Tweak the math in ScrollOffsetAnimationCurve::UpdateTarget.

cc/animation/scroll_offset_animation_curve.cc

 // Copyright 2013 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/animation/scroll_offset_animation_curve.h"
 
 #include <algorithm>
 #include <cmath>
 
 #include "base/logging.h"
 #include "cc/animation/timing_function.h"
 #include "cc/base/time_util.h"
 #include "ui/gfx/animation/tween.h"
 
 const double kConstantDuration = 12.0;
 const double kDurationDivisor = 60.0;
 
 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();
 }
 
 static base::TimeDelta SegmentDuration(
     const gfx::Vector2dF& delta,
     ScrollOffsetAnimationCurve::DurationBehavior behavior) {
   if (behavior == ScrollOffsetAnimationCurve::DurationBehavior::DELTA_BASED) {
     // The duration of a JS scroll animation depends on the size of the scroll.
     // The exact relationship between the size and the duration isn't specified
@@ skipping 86 matching lines @@
       static_cast<TimingFunction*>(timing_function_->Clone().release()));
   scoped_ptr<ScrollOffsetAnimationCurve> curve_clone =
       Create(target_value_, std::move(timing_function), duration_behavior_);
   curve_clone->initial_value_ = initial_value_;
   curve_clone->total_animation_duration_ = total_animation_duration_;
   curve_clone->last_retarget_ = last_retarget_;
   curve_clone->has_set_initial_value_ = has_set_initial_value_;
   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) {
   gfx::ScrollOffset current_position =
       GetValue(base::TimeDelta::FromSecondsD(t));
   gfx::Vector2dF old_delta = target_value_.DeltaFrom(initial_value_);
   gfx::Vector2dF new_delta = new_target.DeltaFrom(current_position);
 
   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;
   total_animation_duration_ = base::TimeDelta::FromSecondsD(t + new_duration);
   last_retarget_ = base::TimeDelta::FromSecondsD(t);
   timing_function_ = EaseOutWithInitialVelocity(new_velocity);
 }
 
 }  // namespace cc